refactor: fusionne les controllers de choix du mode en un wizard Wicked - #493
refactor: fusionne les controllers de choix du mode en un wizard Wicked#493lordinatrice wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Ce PR refactorise le parcours de choix du mode de candidature / type de groupement côté candidat en fusionnant les anciens contrôleurs M1/M2 dans un wizard Wicked unique, afin de faciliter l’ajout des prochaines étapes (M3+), tout en corrigeant des régressions de réécriture silencieuse du mode et de propagation du user_id en “mixte”.
Changes:
- Remplace
Candidate::ApplicationModesController+Candidate::GroupingLegalTypesControllerparCandidate::GroupingWizardController(Wicked) et met à jour les routes/redirects associés. - Introduit une logique centralisée de redirection vers l’étape requise (
MarketApplication#next_required_wizard_step) et ajoute#groupement_counterpart. - Ajoute/ajuste la couverture de tests (request specs, model/interactor specs, cucumber) et corrige la propagation du
user_idau counterpart lors du login.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| spec/requests/candidate/sessions_spec.rb | Ajuste les redirections post-magic-link et ajoute un test de lien user_id vers le counterpart groupement. |
| spec/requests/candidate/market_applications_spec.rb | Stabilise les specs en désactivant le flag par défaut et met à jour la redirection vers le wizard. |
| spec/requests/candidate/lot_selections_spec.rb | Met à jour les redirections vers l’étape wizard et isole le flag. |
| spec/requests/candidate/grouping_wizard_spec.rb | Nouveau fichier : couverture request du wizard (modes, readonly, régressions). |
| spec/requests/candidate/grouping_legal_types_spec.rb | Supprimé : remplacé par la couverture du wizard. |
| spec/requests/candidate/company_identifications_spec.rb | Met à jour la redirection vers le wizard lorsque le mode n’est pas choisi. |
| spec/requests/candidate/application_modes_spec.rb | Supprimé : remplacé par la couverture du wizard. |
| spec/requests/api/v1/market_applications_spec.rb | Ajoute des cas autour du wizard (notamment “mixte” et lien vers l’étape requise). |
| spec/models/market_application_spec.rb | Ajoute des tests pour #groupement_counterpart et #next_required_wizard_step. |
| spec/interactors/candidate/create_applications_for_mode_spec.rb | Ajoute un test de propagation du user au counterpart groupement. |
| features/step_definitions/grouping_legal_type_steps.rb | Met à jour les chemins vers la route wizard. |
| features/step_definitions/candidate_authentication_steps.rb | Ajoute des steps pour demander un magic link sur le counterpart et suivre le dernier email. |
| features/step_definitions/candidacy_mode_steps.rb | Met à jour les assertions de navigation vers les nouvelles URLs wizard. |
| features/candidacy_mode_choice.feature | Ajoute un scénario JS de reconnexion depuis le funnel “groupement” après choix “mixte”. |
| config/routes.rb | Remplace les routes dédiées par grouping_wizard/:id (show/update). |
| app/views/candidate/grouping_wizard/grouping_legal_type.html.erb | Adapte le formulaire et les liens au wizard (avec un ajustement de styles titres). |
| app/views/candidate/grouping_wizard/application_mode.html.erb | Adapte le formulaire au wizard (wizard_path). |
| app/models/market_application.rb | Ajoute #groupement_counterpart et #next_required_wizard_step. |
| app/interactors/candidate/create_applications_for_mode.rb | Lie le user du solo au counterpart groupement lors de la création mixte. |
| app/controllers/concerns/candidate/application_mode_guard.rb | Redirige vers l’étape wizard requise plutôt que l’ancienne route application_mode. |
| app/controllers/candidate/sessions_controller.rb | Lie le user au counterpart lors du login et redirige via next_required_wizard_step. |
| app/controllers/candidate/grouping_wizard_controller.rb | Nouveau contrôleur Wicked qui orchestre les étapes application_mode + grouping_legal_type. |
| app/controllers/candidate/grouping_legal_types_controller.rb | Supprimé : remplacé par le wizard. |
| app/controllers/candidate/application_modes_controller.rb | Supprimé : remplacé par le wizard. |
| app/controllers/api/v1/market_applications_controller.rb | Retourne désormais une URL wizard quand une étape groupement est requise. |
Suppressed comments (1)
app/views/candidate/grouping_wizard/grouping_legal_type.html.erb:18
- La hiérarchie des titres semble inversée ici : le
est stylé en
fr-h4(petit) alors que leest stylé en
fr-h1(très grand). Cela nuit à la cohérence visuelle et peut perturber les lecteurs d’écran (le niveau H2 paraît plus important que le H1). Je suggère de revenir à une taille H1 > H2 (comme avant le refactor).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9d31450 to
184dccd
Compare
6200111 to
d026168
Compare
Un3x
left a comment
Un3x
left a comment
There was a problem hiding this comment.
Ok pour le contenu même si je suis un peu surpris qu'on ai plus de ligne ajouté que retiré pour un refactor. Mais soit.
J'ai quand même quelques interrogations un peu cheloue :
. La connexion ne « raccroche » qu'une seule des deux candidatures 🔗
- Imagine qu'un candidat a deux dossiers jumeaux pour le même marché : un dossier « solo » et un dossier « groupement ». Quand il se connecte avec son link, il se passe quoi ? Et surtout il doit se passer quoi ?
Actuellement, le code ne sait retrouver le jumeau que dans un sens.. Résultat : seul le dossier groupement reçoit son nom, et le dossier solo reste orphelin. Faut voir quel est le comportement attendu mais j'imagine que les deux magic link seraient différent et qu'il faudrait qu'indépendament ils rattachent l'utilisateur au lien qui va avec sa candidature.
- Le bouton « Réessayer la synchronisation » fait semblant de marcher 🔄
Pour les anciens dossiers groupement terminés qui n'ont jamais choisi de forme juridique (c'était possible avant cette PR), le nouveau garde-barrière intercepte le clic sur « Réessayer » et renvoie le candidat vers l'assistant… qui le renvoie aussitôt vers la page de statut, car le dossier est déjà terminé.
Je ne pense pas que ce soit pertinent en prod mais ca pourrait l'être en sandbox/staging pour natalia. Ptet qu'il faut faire une ptite migration pour que ca matche.
- Une requête SQL pour rien à chaque page
Même quand la fonctionnalité « groupement » est désactivée (probablement le cas en production), le code va quand même chercher dans la base de données s'il existe un dossier.
- Des commentaires-bannières
Y a des commentaires qui sont là pour "flagger" une zone de responsabilité d'une classe. Bon j'suis pas un fan des commentaires mais là pour moi c'est surtout un signe que la classe a trop de responsabilité et qu'il faut découper.
| let(:completed_market_application) { create(:market_application, :completed, public_market:, siret: '73282932000074') } | ||
|
|
||
| before do | ||
| allow(FeatureFlags::Groupement).to receive(:enabled?).and_return(false) |
There was a problem hiding this comment.
Cette ligne là que t'as rajouté est la même que celle 2 ligne plus bas.
e0d4c56 to
4410352
Compare
|
Meric pour les retours. En revanche pour le refacto, le but n'était pas tant de réduire la taille mais plutot d'avoir une logique centralisée: avant, 3 endroits différents (sessions, guard, API éditeur) recalculaient chacun "quelle est la prochaine étape", avec le risque qu'ils divergent à chaque step ajouté. Maintenant il n'y en a plus qu'un seul (next_required_wizard_step), donc plus de lignes mais un seul endroit à maintenir pour nb la PR suivante (M3) qui rajoute des steps. |
7abe17d to
cf4e442
Compare
…plication Trois points d'entrée recalculaient chacun leur propre logique de "prochaine étape requise" : ApplicationModesController#next_step_path, SessionsController#first_step_path (cascade if/else dupliquée) et Api::V1::MarketApplicationsController#application_url_for (qui ne connaissait même pas grouping_legal_type). ApplicationModeGuard, lui, ne redirigeait que sur le choix du mode, laissant un candidat sauter l'étape de forme juridique du groupement en accédant directement à une étape suivante par URL. Centralise cette cascade dans MarketApplication#next_required_wizard_step et Candidate::WizardRoutable#next_required_wizard_step_path, utilisés par les 4 points d'entrée pour éviter toute divergence future à mesure que d'autres étapes s'ajoutent. Ajoute le scénario cucumber de reconnexion en mode mixte : une reconnexion via magic link pendant le funnel groupement d'une candidature mixte doit ramener sur l'étape en attente côté groupement, pas sur la candidature solo.
…ifférée en mixte sign_in_candidate ne liait le user qu'à l'application solo signée. Un candidat en mode mixte se connectant après coup laissait son dossier groupement jumeau orphelin (user_id jamais rempli), cassant la redirection du wizard pour ce dossier.
- La reconnexion via magic link ne raccrochait qu'un sens du couple mixte (solo→groupement). Ajoute solo_counterpart, symétrique de groupement_counterpart, pour couvrir la reconnexion depuis le dossier groupement. - next_required_wizard_step interrogeait groupement_counterpart même quand FeatureFlags::Groupement est désactivé, ajoutant une requête sur chaque page du parcours candidat en production. - grouping_legal_type_choice_required? ne tenait pas compte des dossiers groupement déjà complétés avant l'introduction de cette contrainte, les redirigeant en boucle vers l'étape legal_type.
…esenters dédiés already_mandataire_elsewhere? (ApplicationModesController) et la recherche du Grouping mandataire (GroupingLegalTypesController) étaient des requêtes SQL écrites en dur dans les controllers. Extrait dans Candidate::ApplicationModePresenter et Candidate::GroupingLegalTypePresenter, testés isolément.
- grouping_legal_types/show.html.erb : le h1 était stylé plus petit que le h2 (fr-h3 vs fr-h4), perturbant la structure de titres pour les lecteurs d'écran. Corrige aussi le badge "most_common" qui manquait fr-badge--no-icon. - Complète les specs de CreateApplicationsForMode, de l'API éditeur et des 2 controllers avec les cas déjà couverts par les fixes précédents mais pas encore testés explicitement (linked user en mixte déjà authentifié, cascade groupement_counterpart côté API, readonly avec counterpart complet/incomplet).
…cked-groupement-wizard
cf4e442 to
21c2921
Compare
Résumé
Candidate::ApplicationModesControlleretCandidate::GroupingLegalTypesController. Abandonne l'approche wizard Wicked initiale (trop rigide pour la logique de navigation conditionnelle mixte/groupement) au profit d'une cascade de navigation centralisée dansMarketApplication#next_required_wizard_step, réutilisée par les 4 points d'entrée du parcours (controllers, session, API éditeur).ApplicationModePresenter,GroupingLegalTypePresenter), pour garder les controllers fins et testables isolément.user_idque dans un sens du couple mixte (solo→groupement) ; ajoutesolo_counterpart, symétrique degroupement_counterpart.next_required_wizard_stepinterrogeaitgroupement_counterpartmême quandFeatureFlags::Groupementest désactivé, ajoutant une requête inutile sur chaque page du parcours candidat en production.grouping_legal_type_choice_required?ne tenait pas compte des dossiers groupement déjà complétés avant l'introduction de cette contrainte, les redirigeant en boucle vers l'étape legal_type.grouping_legal_types/show.html.erb, corrigée pour la structure lecteur d'écran.update_application_modeacceptait un PATCH même quand le mode était déjà choisi, permettant de réécrire silencieusementapplication_modesans repasser par les gardes déjà en place côté GET.Test plan
bundle exec rspec spec/requests/candidate/application_modes_spec.rb spec/requests/candidate/grouping_legal_types_spec.rb spec/requests/candidate/sessions_spec.rb— 0 échecbundle exec cucumber features/candidacy_mode_choice.feature features/grouping_legal_type.feature— tout passebin/rubocop— aucun problème