annuaire-entreprises-site
annuaire-entreprises-site copied to clipboard
Add OpenData from API Entreprise for agent
- [x] Add a new scope
opendata
- [x] Add EORI client & model
- [x] Add carte professionnelle des travaux publics
- [x] Add Qualibat
- [x] Add Qualifelec
- [x] Add Opqibi
- [x] Date de naissance
- [x] Add link & label from résumé to certification for agent
- [x] Add E2E test to check that standard users don't get to see protected data
I like the idea of IUnauthorized but I dont think it is working :
- you end up with a lot more session being pass as arguments
- you still need to handle IUnauthorized in view as
- you will still need to have a way for the view to prevent API call based on rights. See conformite for instance. Otherwise it will lead to many unnecessary calls
- As API calls from API Entreprise tend to be very slow (appart from the test env) we will need to use async a lot
I like the see more, but we need something much more obvious visually. I missed it several time in local trying to force trigger it, before eventually I realised I had it in front of me, I just did not see it.
Maybe github can be an inspiration on this, I find that their collapsed comment component is quite explicit
Another solution is to have it built in the <DataSection/>. Give it a max height etc.
General :
- every call should be made async as API Entreprise tend to be much slower than sirene and the whole point of the /app router migration was to allow more async call from /entreprise page
- notAuthorized is an interesting idea, but the implem only really shine for server side calls. For async calls I feel like it doesnot really help as you still need some kind of browser validation and it require to share the session to the model, which I tried to avoid
- this refacto needs more reflection and might need a dedicated PR
Actes :
- see more is a really good idea, but this implementation is not obvious enough (I add a really hard time finding it, knowing it was there)
- I think this deserve a dedicated PR
Dirigeant
- we should move them into a dedicated section, not replace the rne section, even if it creates duplicates as RNE and RCS are not the same and dont hold the same meaning.
- we need to add a short explanatation as to why we also show RCS
Labels & certif
- we need a way to display badges on /entreprise page and/or /etablissement page (if they make sense at siret level)
every call should be made async as API Entreprise tend to be much slower than sirene and the whole point of the /app router migration was to allow more async call from /entreprise page
This is only the case for agent. If you believe it's too much of trade off, we could use async server component. This would enable us to load the page first and then render, without having to move calls to an API route first.
require to share the session to the model, which I tried to avoid
I think it is the point. The view shouldn't have to know which API call to make depending on authorization level. This is a specific domain logic that should be inside the model. If you think sharing the session with the model is not a good pattern (I do agree, but I did it for the sake of simplicity :p), we can refactor, and only share the User
model, or just the scopes
.
see more is a really good idea, but this implementation is not obvious enough (I add a really hard time finding it, knowing it was there)
We cannot do without because otherwise, the carteProfessionnelle
document would be pushed down very far on the page, especially when there is a lot of actes (like it does with large companies). We can however fix it's visibility and affordability.
we should move them into a dedicated section, not replace the rne section, even if it creates duplicates as RNE and RCS are not the same and dont hold the same meaning.
From a data perspective, I agree, but does-it from a user-centric standpoint ? I'm only asking, to understand better. In which case the RCS would differ from the RNE ?
we need a way to display badges on /entreprise page and/or /etablissement page (if they make sense at siret level)
Yes !