Fix: mise à jour de la liste des agents d’un RDV #5169
Conversation
AntoineGirard
force-pushed
the
fix-agents-rdvs-creation-before-rdv-valid
branch
from
4c7677c to
e61f3cf
Compare
adipasquale
force-pushed
the
fix-agents-rdvs-creation-before-rdv-valid
branch
from
aad2591 to
3c3470a
Compare
| @@ -11,11 +11,15 @@ def initialize(rdv, agent_context) | |||
| end | |||
|
|
|||
| def submit(rdv_attributes) | |||
| raise ArgumentError, "agent_ids est accepté mais pas agents" if rdv_attributes.key?(:agents) | |||
There was a problem hiding this comment.
j’ai rajouté cette ligne pour expliciter qu’il faut passer agent_ids et pas agents, ce qui était le cas dans ta première version des specs @AntoineGirard .
On pourrait adopter d’autres solutions.
Une manière serait d’accepter une liste en dur d’attributs plutôt que rdv_attributes qui est un peu « dangereux ».
Une autre manière, que je pense proposer dans une PR successive est de modifier ce submit pour recevoir la liste de params du controlleur et faire le parsing directement ici. Ce formulaire est appelé à un seul endroit donc ça me semble raisonnable de décaler le parsing ici, aussi parce que le controlleur déborde un peu actuellement
| let!(:rdv2) { create(:rdv, agents: [agent_mayra], organisation:, starts_at: Time.zone.parse("2025-04-29 18:00")) } | ||
| let(:base_attributes) do | ||
| { | ||
| starts_at: Time.zone.parse("2025-04-28 10:30"), # le même horaire que rdv1, déclenche une erreur contournable |
There was a problem hiding this comment.
@AntoineGirard j’ai modifié les specs pour que le cas soit plus réaliste : on tente de changer l’heure et les agents en même temps.
There was a problem hiding this comment.
Merci @adipasquale d’avoir itéré sur le sujet
J’étais parti sur des solutions un peu plus lourdes à la mise en œuvre, mais je n’avais pas eu le temps d’aboutir à quelque chose de satisfaisant à l’époque
Pour une première itération ça me va très bien !
La solution que tu proposes dans un deuxième temps me semble parfaitement convenable aussi
github-project-automation
bot
moved this from 🔖 Ready
to 🚀 To deploy
in RDV Service Public
github-project-automation
bot
moved this from 🚀 To deploy
to ✅ Done
in RDV Service Public
* Fix: mise à jour de la liste des agents d’un RDV * improve spec * fix spec --------- Co-authored-by: Adrien Di Pasquale <[email protected]>
Contexte
En explorant un bug concernant les mails transactionnels qui ne partaient pas quand on ajoute un agent à un RDV, je me suis rendu compte d’un comportement anormal.
En tant qu’agent, si j’édite un RDV et que celui-ci n’est pas valide au moment de la soumission (et que je n’ignore pas l’avertissement), le RDV n’est pas sauvegardé mais la liste des agents oui. Le comportement est reproduit dans la spec du premier commit.
Le problème vient du
assign_attributes(attributes)positionné avant levalid?et lesave. Il déclenche unself.agent_ids = new_agent_idsqui écrit en db, ce qui est un comportement connu et documenté mais très surprenant de Rails.Solution
Je propose une solution qui améliore la situation mais ne la résoud pas à 100% : je décale ce
self.agent_ids =plus bas, après la vérification duvalid?.Il peut donc encore y avoir des états incohérents si le
.agent_ids=passe mais pas le.saveconsécutif. C’est nettement moins probable.A ma connaissance, il n’y a pas de validation sur les agents associés à un RDV au moment de l’update donc ça ne pose pas de soucis de décaler le
agent_ids=après la vérification de la validité.