Skip to content

refactor admin RDVs forms : extracted Concern + form objects

Vincent Agnano requested to merge refactor/agent-rdv-wizard-save into master

Created by: adipasquale

preliminary to #994

️ Désolé c'est encore une PR un peu compliquée, et cette fois pas découpée en plusieurs commits, je me suis perdu et ait tout squashed. Je pourrais a minima extraire les bonus, notamment le renaming pour clarifier , tu me dis si c'est trop dur à reviez @yaf

J'ai sacrément tartiné sur la description de cette PR mais je pense qu'il faut bien lire ça avant de regarder le code, sinon ça va être bien abstrait !

Objectifs

1er objectif : corriger la place du warning de POs qui overlappent sur les RDVs

On parle ici du de l'avertissement (~cad la validation souple) qui verifie si un RDV présente une incohérence avec une plage d'ouverture dans un autre lieu. Cet avertissement est aujourd'hui défini au niveau du modèle RDV. Or, on ne veut pas déclencher ce warning si c'est un usager qui créé un RDV. On ne fournit pas d'interface pour contourner les avertissements côté usager, donc il serait bloqué.

Par chance, cet avertissement a peu de chances de se déclencher côté usager aujourd'hui, puisqu'un usager prend toujours RDV sur un créneau appartenant à une plage d'ouverture. Il pourrait éventuellement se déclencher si un agent a deux plages d'ouvertures en conflit.

Ca parait plus sur d'extraire cet avertissement depuis le modèle ailleurs, pour qu'il ne soit déclenché que par les actions agents. La bonne place semble être un "form object" c'est à dire un objet proxy du modèle qui surcharge et rajoute des validations.

Une autre justification d'extraire cet avertissement c'est qu'on va en rajouter d'autres qu'il ne faut vraiment pas déclencher côté usager : notamment l'avertissement qu'un agent s'apprête à créer un RDV qui commence peu de temps après la fin d'un autre (pour éviter les trous). Cet avertissement se déclencherait souvent s'il était appliqué côté usagers.

Enfin dernière raison d'extraire ça : faire de la place dans Rdv qui est gros.

2e objectif : fusionner les messages d'avertissements

Jusqu'ici le warning "overlapping PO" s'affichait en deux parties :

  1. un message de warning dans le bloc jaune
  2. un détail des plages d'ouvertures en conflit en dessous
Screenshot_2020-12-09_at_16 46 28

On veut fusionner 2 dans 1 :

same-orga

Les avantages:

  • plus simple à lire, une seule info au lieu de 2
  • évite la manip bizarre du content_for pour injecter 2 au bon endroit
  • plus extensible pour l'avenir vu qu'on va rajouter des messages d'avertissement dans la PR à venir.

Détails d'implémentation

création et update

On a envie que ces avertissements spécifiques s'affichent côté admin uniquement, mais à la fois sur les créations et les updates. le AgentRdvWizard actuel n'est donc pas le bon form object pour accueillir le nouveau comportement, il est utilisé uniquement à la création.

J'ai introduit :

  • un Admin::RdvFormConcern qui contient le comportment partagé avec la définition de ces avertissements + les délégations nécessaires au RDV
  • un Admin::EditRdvForm qui inclut ce concern et fait peu de choses de plus, utilisé pour l'update
  • inclut le concern dans le AgentRdvWizard::Step3

appeler agent_rdv_wizard.save plutot que rdv.save

  • la route admin/rdvs#create est supprimée, maintenant la step3 du rdv wizard postera vers admin/rdv_wizard_steps#create comme les autres steps
  • on appelle maintenant .save sur agent_rdv_wizard plutot que sur le rdv. Cela retire le besoin de déclencher manuellement les validations, entre autres
  • ça demande de rajouter quelques délégations dans l'objet RdvWizardForm et de faire remonter les erreurs du Rdv vers ce form object

Fusion des messages d'avertissements de plages d'ouvertures en conflit

J'avais séparé en deux pour contourner des limitations d'AR car je n'arrivais pas à utiliser le contexte de la vue dans le 1. directement. En effet 1 venant d'un message de validation, il était construit dans le modèle Rdv, où ce n'est pas évident d'avoir le contexte de la vue. On doit faire appel aux helpers de vues, apeller le policy_scope etc.

J'ai détaché un nouvel object PlageOuverturePresenter qui est en charge d'afficher des infos a propos d'une plage d'ouverture dans un contexte de vue (agent + orga). C'est lui qui construit le message d'avertissement selon tous les differents cas qui peuvent se produire. Ce Presenter inclut des helpers de vues et se charge d'appeler pundit

dans le contexte d'une autre orga que celle de la PO :

different-orga

Bonus

  • on renomme et déplace ce form object vers Admin::RdvWizardForm
  • Implémentation du authorize sur les routes admin/rdv_wizard_steps, il n'y avait pas de raison de ne pas le faire.
  • nouvelles methodes AgentRdvWizard#success_path et #success_flash pour éviter d'avoir à brancher dans le controleur selon la step

Merge request reports