Conversation
There was a problem hiding this comment.
Pull request overview
Ce PR vise à permettre l’ajout de nouveaux scopes sur un HabilitationType même lorsque ses champs structurels sont verrouillés (ex. présence de demandes liées), en adaptant à la fois l’UI d’édition et la sanitization côté controller, et en ajoutant une couverture Cucumber.
Changes:
- Ajout d’un scénario Cucumber couvrant l’ajout d’un scope sur un type avec des demandes liées.
- Adaptation de
Admin::HabilitationTypesController#sanitize_locked_scopespour conserver les scopes existants tout en acceptant de nouveaux scopes. - Mise à jour du composant d’édition des scopes pour réactiver l’ajout de scopes en mode “structural locked” et préserver la valeur via un champ hidden.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| features/step_definitions/habilitation_types_steps.rb | Ajoute un step de création d’un type avec scopes + demandes liées pour les tests e2e. |
| features/admin/habilitation_types.feature | Ajoute un scénario JS vérifiant l’ajout d’un scope sur un type avec demandes liées. |
| app/controllers/admin/habilitation_types_controller.rb | Modifie la sanitization pour autoriser l’ajout de nouveaux scopes quand les champs structurels sont verrouillés. |
| app/components/molecules/admin/scopes_editor_component.html.erb | Réactive le template/bouton d’ajout de scope et ajoute un champ hidden pour conserver value quand le champ est désactivé. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jbfeldis
force-pushed
the
feature/dp-1641-ajouter-des-scopes-quand-je-modifie-une-definition-avec-des
branch
from
e3d744f to
6ace454
Compare
jbfeldis
changed the title
Isalafont
left a comment
Isalafont
left a comment
There was a problem hiding this comment.
Peut être que ça mérite juste un test supplémentaire pour s'assurer que l'ajout de scopes n'efface pas un scope déjà existant ?
D'ailleurs, est ce que l'on couvre le cas d'une value en conflit ? Est ce qu'on lève un message d'erreur pour avertir l'utilisateur ?
C'est peut être overkill à ce stade...
A voir si c'est plus simple de faire un test rspec pour ça ou si un test cucumber en vaut la peine ?
Sinon après ce sont plus des interrogations plus que des points bloquant en fait :)
| Et la page ne contient pas "Ajouter un scope" | ||
|
|
||
| @javascript | ||
| Scénario: Je peux ajouter un scope à un type avec des demandes liées |
There was a problem hiding this comment.
Est ce que l'on ne rajouterait pas un test supplémentaire pour garantissant que l'ajout d'un nouveau scope n'écrase pas un scope déjà existant ?
Quelque chose comme :
Et que je me rends sur le chemin "/admin/types-habilitation/api-protegee"
Alors la page contient "Nouveau scope"
Et la page contient "Revenu fiscal"
| def extract_new_scopes(existing, submitted_scopes) | ||
| existing_values = existing.to_set { |s| s['value'] } | ||
| submitted_scopes | ||
| .reject { |s| existing_values.include?(s[:value]) } |
There was a problem hiding this comment.
Ce n'est pas bloquant mais
Si je comprends bien ici :
Lorsqu'un admin tape une valeur déjà existante, le nouveau scope sera silencieusement ignoré sans message d'erreur ou autre.
Est ce que l'on hardenerai pas cela ? Peut être en mettant en place une validation sur une value unique ou alors en levant un message d'erreur ?
Uh oh!
There was an error while loading. Please reload this page.