Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ETQ instructeur : un message m'incite à ajouter la colonne "Labels" dans mon tableau de suivi des dossiers #11244

Merged
merged 4 commits into from
Feb 10, 2025

Conversation

Benoit-MINT
Copy link
Contributor

@Benoit-MINT Benoit-MINT commented Jan 23, 2025

issue : #11115

Résultat :
Capture d’écran 2025-01-23 à 19 24 14

Pour info, la caractère ouvert/fermé du bandeau d'information est à la maille de l'instructeur/procédure. Donc dès lors qu'il est fermé depuis l'un des dossiers d'une procédure, celui-ci ne s'affichera plus pour les autres dossiers. En revanche, quand ce même instructeur consultera un des dossiers d'une autre procédure, celui-ci sera affiché.

Copy link

codecov bot commented Jan 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.44%. Comparing base (5700d4f) to head (cfe9388).
Report is 129 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11244      +/-   ##
==========================================
+ Coverage   84.42%   84.44%   +0.02%     
==========================================
  Files        1199     1209      +10     
  Lines       26381    26560     +179     
  Branches     4965     5000      +35     
==========================================
+ Hits        22271    22429     +158     
- Misses       4110     4131      +21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Benoit-MINT Benoit-MINT changed the title ETQ administrateur, je peux ordonner les labels / choisir leur ordre d’affichage pour l’instructeur ETQ instructeur : un message m'incite à ajouter la colonne "Labels" dans mon tableau de suivi des dossiers Jan 27, 2025
@Benoit-MINT Benoit-MINT marked this pull request as ready for review January 27, 2025 10:26
Copy link
Member

@colinux colinux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je suis pas très convaincu par le fait de matérialiser l'affichage de la notice par une colonne dédiée dans la db, et un controller pour la masquer. Je veux dire que ça me paraît cher payé. Aussi les ProcedurePresentation peuvent être réinitialisées à différents moments(type réinitialisation du tableau) et ça voudra dire réaffichage de la notice.

Est-ce qu'on ne pourrait pas avoir une heuristique différente ? Par exemple tjs afficher la notice tant que la colonne n'est pas affichée?

@Benoit-MINT
Copy link
Contributor Author

Je suis pas très convaincu par le fait de matérialiser l'affichage de la notice par une colonne dédiée dans la db, et un controller pour la masquer. Je veux dire que ça me paraît cher payé. Aussi les ProcedurePresentation peuvent être réinitialisées à différents moments(type réinitialisation du tableau) et ça voudra dire réaffichage de la notice.

Est-ce qu'on ne pourrait pas avoir une heuristique différente ? Par exemple tjs afficher la notice tant que la colonne n'est pas affichée?

Sur le principe d'enregistrer en db, je partage ton point, et j'avais pensé à la même idée de faire selon la présence de la colonne dans le tableau. J'ai rebouclé avec Marlène qui estimait que c'est plus du bonus qu'une solution de substitution (ping @marleneklok ).

Sur le choix de ProcedurePresentation, je suis parti du principe que si l'on pouvait accepter la réinitialisation de la personnalisation du tableau, on peut accepter la réinitialisation de l'affichage du message, et les deux ensemble sont pas incohérents d'ailleurs. Et en lien avec ta remarque du "cher payé", je trouvais que ça faisait "tâche" dans Instructeur.

@colinux
Copy link
Member

colinux commented Jan 29, 2025

J'ai l'impression qu'on se rejoint non ? Le bonus est moins cher dans le sens ou c'est pas cher de regarder si on a la colonne Labels dans la liste des colonnes de la PP d'un des onglets, ce qui éviterait à la fois le controller dédié + la colonne en db

@Benoit-MINT Benoit-MINT force-pushed the etq-instructeur-message-colonne-labels branch from bcdf2ff to 51c0008 Compare January 30, 2025 17:51
@Benoit-MINT Benoit-MINT force-pushed the etq-instructeur-message-colonne-labels branch from 57f375e to cfe9388 Compare January 30, 2025 18:40
%p.fr-text--sm.fr-text-mention--grey.fr-mb-0
%b Besoin d'autres labels ?
%br
Contactez les
= link_to 'administrateurs de la démarche', administrateurs_instructeur_procedure_path(dossier.procedure), class: 'fr-link fr-link--sm', **external_link_attributes
- if !@procedure_presentation.displayed_columns.any? { |c| c.column == "label_id" }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pour info, ça aurait aussi pu être

Suggested change
- if !@procedure_presentation.displayed_columns.any? { |c| c.column == "label_id" }
- if @procedure_presentation.displayed_columns.none? { |c| c.column == "label_id" }

(gain de lisibilité en évitant l'inversion de condition avec !) :

@colinux colinux added this pull request to the merge queue Feb 10, 2025
Merged via the queue into main with commit 38ad948 Feb 10, 2025
18 checks passed
@colinux colinux deleted the etq-instructeur-message-colonne-labels branch February 10, 2025 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants