Skip to content

notify agents on RDV starting soon cancelled or date updated

Vincent Agnano requested to merge feature/notify-agents-of-cancelled-rdvs into master

Created by: adipasquale

https://trello.com/c/LW2daCrU/1085-notifications-en-cas-dannulation-pour-les-rdv-daujourdhui-ou-demain

stitched_2020_09_24-09_47_50

Ce n'est pas super satisfaisant tel quel mais je ne sais pas s'il faut aller plus loin (cf pistes plus bas). Je m'appuie sur les callbacks ActiveRecord after_update et sur la dernière version stockée par papertrail pour trouver l'auteur du changement (sous forme textuelle uniquement eg "[Agent] Jean MARC" ou "[Usager] Pat OCHE").

J'ai extrait un concern Rdv::Notifiable car la classe Rdv devenait trop longue pour le linter.

en passant, et j'ai du modifier deux choses sinon des erreurs apparaissaient sur les deliver_later des mails de notifs de RDV qui disaient 'pas d'ID, impossible d'enqueue' :

  1. j'ai du changer le after_create :notify_rdv_created en after_create_commit. La différence est que le deuxieme s'effectue une fois la transaction commitée en DB. C'est effectivement ce qui est recommandé pour ce genre de callbacks qui interagit avec un systeme externe (ici la taskqueue). cf https://guides.rubyonrails.org/active_record_callbacks.html#transaction-callbacks
  2. j'ai du changer lest after_x :notify_x en after_x, -> { notify_x }. c'est un fix un peu hacky pour contourner une limitation des callbacks AR : "Using both after_create_commit and after_update_commit in the same model will only allow the last callback defined to take effect, and will override all others.". J'ai trouvé ce contournement ici https://github.com/rails/rails/issues/29554#issuecomment-467837695 . une solution moins hacky mais vraiment peu explicite serait de faire un seul callback commun a tous after_commit et de brancher dans ce commit selon le cas effectif update / create + conditions

je suppose que c'est justement l'introduction d'un nouveau after_update_commit qui fait que notre ancien équilibre ne tient plus et qu'il faut faire ces changements. Le 1. m'a l'air judicieux et pas vraiment un hack, mais le 2. est borderline, tu me diras ce que tu en penses @yaf. J'ai cependant ajouté des specs donc on devrait être avertis si ces callbacks arrêtent de se comporter comme on l'espère.

Autres pistes d'implémentation plus propres

notif lors de l'annulation

ca serait probablement plus simple, clair et evolutif avec une state machine pour le statut des RDVs. ça permettrait de déclencher des événements lors des transitions précises plutôt que se brancher sur le after_update filtré selon les statuts avant après. Mais j'hésite a introduire cette state machine maintenant. je pense que c'est un petit chantier..

notif lors de l'update de date

je pense qu'il faudrait revoir l'UX pour que ce changement soit isolé du formulaire general de RDV, et que ça n'appelle pas la route rdv#update mais peut etre une autre route plus précise genre rdv/reschedules#create. On pourrait alors se brancher la et gerer plus explicitement selon la date du RDV avant et après update, et éventuellement en redonnant du contrôle à l'agent par ex avec une checkbox "prévenir les usagers".

Merge request reports