ETQ admin j'ai accès à une nouvelle interface de gestion des instructeurs #10878
Conversation
E-L-T
force-pushed
the
update-routage-interface
branch
from
d01f130 to
fa5c0d1
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10878 +/- ##
==========================================
+ Coverage 84.50% 84.51% +0.01%
==========================================
Files 1140 1141 +1
Lines 25246 25263 +17
Branches 4724 4725 +1
==========================================
+ Hits 21334 21351 +17
Misses 3912 3912 ☔ View full report in Codecov by Sentry. |
E-L-T
force-pushed
the
update-routage-interface
branch
6 times, most recently
from
c5bbbc7 to
283f349
Compare
There was a problem hiding this comment.
C'est super ! 👏
En plus de 2-3 petits remarques mineures commentées au fil du code :
- le message en noir Même si votre démarche est encore en test / non publiée, tous les instructeurs que vous ajouterez à la démarche seront notifiés par email. pourrait ne pas être affiché sur une démarche publiée (d'ailleurs je le vois pas sur la page des groupes)
- c'est peut-être la CI qui déconne mais ça indique qu'il n'y a aucun test qui passe par le component ImportComponent ni par le
GroupeInstructeursController#import(ça m'étonne un peu, donc c'est ptet juste l'outil de suivi qui est pas à jour)
| def template_path | ||
| if @procedure.routing_enabled? | ||
| Rails.public_path.join('csv/import-groupe-test.csv') | ||
| else | ||
| Rails.public_path.join('csv/import-instructeurs-test.csv') | ||
| end |
There was a problem hiding this comment.
| def template_path | |
| if @procedure.routing_enabled? | |
| Rails.public_path.join('csv/import-groupe-test.csv') | |
| else | |
| Rails.public_path.join('csv/import-instructeurs-test.csv') | |
| end | |
| def template_path | |
| Rails.public.path.join(template_file) | |
| end |
There was a problem hiding this comment.
le truc c'est que dans un cas il faut un / au début du path et dans l'autre il en faut pas, je sais pas trop comment gérer ça proprement
There was a problem hiding this comment.
En fait c'est bon, j'ai fait un commit de refacto
| @procedure.routing_enabled? ? 'groupes' : 'instructeurs' | ||
| end | ||
|
|
||
| def template_url |
There was a problem hiding this comment.
| def template_url | |
| def template_file |
- j'ai l'impression que ça peut passer en private
There was a problem hiding this comment.
je crois pas, parce que c'est utilisé depuis la vue, app/components/procedure/import_component/import_component.html.haml:15
Je serais plutôt pour garder cette alerte même quand la démarche est publiée, quitte à modifier le wording (enlever le début "Même si votre démarche est encore en test / non publiée," si la démarche est publiée).
ouais, bizarre, il y a déjà des tests là-dessus. Je me demande si c'est pas d'avoir déplacé le code dans un component qui fait ça |
E-L-T
force-pushed
the
update-routage-interface
branch
from
4ae0881 to
3ed7e7d
Compare
Cette PR met à jour uniquement la page de gestion des instructeurs (routage inactif)
Ça correspond à la première partie de #10766