Amélioration de l’explication de la permission pour changer un agent de service #4903
Conversation
adipasquale
changed the title
| @@ -21,7 +21,7 @@ | |||
| = render "model_errors", model: @agent | |||
| - allowed_to_change_services = Agent::TerritoryPolicy.new(current_agent, current_territory).allow_to_manage_access_rights? | |||
|
|
|||
| = f.association :services, collection: @services, disabled: true, input_html: { class: "select2-input" }, wrapper_html: { class: "mb-0" }, hint: allowed_to_change_services ? nil : "Le changement de service est réservé aux admins de territoire" | |||
| = f.association :services, collection: @services, disabled: true, input_html: { class: "select2-input" }, wrapper_html: { class: "mb-0" }, hint: allowed_to_change_services ? nil : "Vous n’avez pas les droits nécessaires pour modifier les services de l’agent. Il faut être administrateur de territoire avec gestion des droits d’accès." | |||
There was a problem hiding this comment.
Comme le montre le screenshot, ça commence à faire beaucoup de texte à la suite pour ce champs. En plus la nomenclature n'est pas uniforme ("administrateur de territoire avec gestion des droits d’accès" vs "agent admin du territoire"). Par ailleurs, ce champs sera rarement éditable, et souvent ce ne sont pas les services que l'agent vient modifier ici.
Pour éviter de commencer cette page par un long texte qui ne sera pas pertinent dans tous les cas, on pourrait le déplacer plus bas dans le formulaire, et/ou le mettre dans une infobulle dsfr (https://www.systeme-de-design.gouv.fr/composants-et-modeles/composants/infobulle)
There was a problem hiding this comment.
@victormours je suis d’accord avec toi que ça fait trop d’infos dans ce hint !
Sur le wording je me suis un peu emmêlé les pinceaux, tu as raison. Je te propose de modifier pour dire « Le changement de service est réservé aux agents admin de territoire ayant le droit d’accès pour modifier les services ».
La proposition d’infobulle est intéressante, tu imagines ça comme un icône question mark à côté du champ et quand tu cliques tu vois le contenu complet des 4 lignes de hint actuelles ?
En changement un peu plus profond, peut-être dans une PR indépendante, je me demandais si on ne retirerait pas la possibilité de modifier le(s) service(s) dans ce formulaire.
On afficherait simplement les services textuellement.
Si l’agent connecté a les droits de modif, on lui afficherait un lien pour aller le modifier depuis l’admin de territoire.
Si l’agent connecté n’a pas les droits, on lui afficherait un texte explicatif (ou une infobulle).
Les avantages de cette piste :
- alléger le formulaire agent simple
- dans le formulaire agent côté admin de territoire, les personnes connectées ont plus de contexte et peuvent plus simplement comprendre la notion de « droits d’accès territoriaux ».
- dans le formulaire agent simple, si l’agent connecté n’est pas admin de territoire on pourrait simplement indiquer que pour modifier le service il faut être « admin de territoire » sans rentrer dans le détail des droits d’accès
- sémantiquement c’est un peu plus cohérent d’éditer le service de l’agent à l’échelle la plus haute possible puisque ce n’est pas dépendant de l’orga courante
Les inconvénients :
- pour les agents qui ont les droits, ça peut leur rajouter des clics de devoir basculer entre les formulaires côté territoire et côté orga
Qu’en penses-tu ?
There was a problem hiding this comment.
J'adore ! Je pense que c'est une très bonne idée effectivement, et l'inconvénient n'est pas si grave que ça, vu qu'on a déjà ces basculement de formulaire.
tu imagines ça comme un icône question mark à côté du champ et quand tu cliques tu vois le contenu complet des 4 lignes de hint actuelles ?
toutafé
| .card.m-2.rounded | ||
| h2.card-header | ||
| = t(".agent_services_legend") | ||
| = simple_form_for @agent, url: update_services_admin_territory_agent_path(current_territory, @agent) do |f| |
There was a problem hiding this comment.
c'est assez verbeux de mettre le formulaire en entier alors que l'agent ne peut pas s'en servir. Est-ce qu'on pourrait se contenter de mettre le texte d'info qui explique que l'agent n'a pas les permissions suffisances pour gérer les droits d'accès ?
| }, | ||
| disabled: !can_edit_services | ||
| - unless can_edit_services | ||
| .fr-alert.fr-alert--info |
There was a problem hiding this comment.
Je me demande si le composant de Mise en avant (https://www.systeme-de-design.gouv.fr/composants-et-modeles/composants/mise-en-avant) ne serait pas plus pertinent qu'une alerte ici. Dans bootstrap on n'avait pas cette option (ou en tout cas on ne s'en servait pas), mais maintenant qu'on a le dsfr, on peut distinguer les alertes (réaction de l'appli à l'action de l'utilisateur, généralement équivalent aux flash rails) des mises en avant ("information qui vient compléter le contenu consulté")
| = f.input :allow_to_manage_access_rights, as: :boolean, hint: t(".hint_allow_to_manage_access_rights"), disabled: !can_edit_access_rights | ||
| = f.input :allow_to_invite_agents, as: :boolean, hint: t(".hint_allow_to_invite_agents"), disabled: !can_edit_access_rights | ||
| - unless can_edit_access_rights | ||
| .fr-alert.fr-alert--info |
There was a problem hiding this comment.
idem pour ce formulaire et cet alert
|
@victormours j’ai mis à jour le code selon ce qu’on a dit. J’ai pris la liberté de supprimer la mention de la nouveauté de la possibilité d’affecter un agent à plusieurs services puisque cette nouveauté a maintenant plus d’un an #3846 . Ça rend le texte plus digeste donc je me suis dit que ça allait de le laisser inline plutôt que dans une infobulle. Je n’ai vraiment pas d’avis fort là dessus si tu préfères qu’on essaie une infobulle tu me dis. Et le changement de texte du hint est maintenant
c’est encore très discutable et soumis à tes retours et éventuellemetn ceux d’autres personnes si tu penses que ça vaut le coup |
There was a problem hiding this comment.
J’ai pris la liberté de supprimer la mention de la nouveauté de la possibilité d’affecter un agent à plusieurs services puisque cette nouveauté a maintenant plus d’un an #3846 .
Ça rend le texte plus digeste donc je me suis dit que ça allait de le laisser inline plutôt que dans une infobulle. Je n’ai vraiment pas d’avis fort là dessus si tu préfères qu’on essaie une infobulle tu me dis.
C'est nickel, effectivement c'est plus simple comme ça, ça me va très bien.
J'ai mis quelques commentaires sur des petits points de détail, mais ça me va qu'on shippe ça 👍
| = link_to "Accéder à la configuration des agents", edit_admin_territory_agent_path(territory_id: current_territory.id, agent_id: @agent.id) | ||
| p | ||
| - if @agent.services.count == 1 | ||
| | #{@agent.full_name} appartient à un seul service : |
There was a problem hiding this comment.
c'est un joli détail d'avoir fait ce if 😍 . Tant qu'à faire, pour rendre ce cas un petit peu plus joli on pourrait même inliner le nom du service :
| | #{@agent.full_name} appartient à un seul service : | |
| | #{@agent.full_name} appartient au service <b>#{@agent.services.first.name}</b> |
| .row | ||
| .col.text-right | ||
| - if can_edit_services | ||
| = f.submit class: "btn btn-primary", value: "Enregistrer les services", disabled: !can_edit_services |
There was a problem hiding this comment.
je crois que le disabled n'est plus nécessaire ici
| .row | ||
| .col.text-right | ||
| - if can_edit_access_rights | ||
| = f.submit class: "btn btn-primary", value: "Enregistrer les droits d'accès", disabled: !can_edit_access_rights |
Contexte
cf cette discussion mattermost avec Eva https://mattermost.incubateur.net/betagouv/pl/9jdjt888ejgxzgdmocfsp7fixr
Solution
Je propose des nouvelles tournures de phrase plus précises, mais aussi plus longues 🤷
Je propose aussi de rajouter les champs désactivés et des infos dans le formulaire d’édition d’agents côté admin de territoire
Captures d'écran
Tests
Review app
Testez avec [email protected] qui est admin de territoire mais n’a pas les droits nécessaires