Closed dzc34 closed 3 weeks ago
@tchak @krichtof ticket complété
@dzc34 ; on commence a se demander si on ouvrirait pas ce sujet de notre coté. je me demande ou vous en êtes :-)
Avec un peu de retard, le résumé de la réunion de cadrage du 26/09/2022 entre équipe DS (@emsnytech @krichtof @tchak), Synbioz (@akarzim) et l'Adullact (@dzc34).
Avec un peu de retard, le résumé de la réunion de cadrage technique du 29/09/2022 entre équipe DS (@krichtof @tchak), Synbioz (@akarzim) et l'Adullact (@dzc34).
(7)(6)(5)(4)(3)(2)(1) ----> créer un ticket dédié à chaque fois
pour "Envoyer un message identique sur plusieurs dossiers"
pour "Demander un avis aux même expert"
@tchak ; l'impression que le préambule a tous ça c'est un refacto pour isoler les appels db/side effects. t'avais une cible la dessous ou pas ?
remarque de @akarzim sur le refacto
refacto à faire pour isoler les "side effects"
La refacto proposée devrait porter sur plusieurs axes :
- encapsuler les requêtes DB dans des transactions ;
- logguer de façon asynchrone ;
- optimisation de certaines requêtes (?) ;
- extraire la logique dans des services pour soulager le modèle dossier qui fait près de 1300 lignes (?) ;
- consolider les tests.
notes techniques de @akarzim pour l'étape 1 et 1 question :
Opérations possibles sur un dossier par un instructeur
# app/models/dossier_operation_log.rb enum operation: { changer_groupe_instructeur: 'changer_groupe_instructeur', # par l'administrateur uniquement passer_en_instruction: 'passer_en_instruction', repasser_en_construction: 'repasser_en_construction', repasser_en_instruction: 'repasser_en_instruction', accepter: 'accepter', refuser: 'refuser', classer_sans_suite: 'classer_sans_suite', supprimer: 'supprimer', restaurer: 'restaurer', modifier_annotation: 'modifier_annotation', demander_un_avis: 'demander_un_avis', archiver: 'archiver', desarchiver: 'desarchiver' }
1ère étape (facile) -> pas de mail
Archivage
# app/controllers/instructeurs/dossiers_controller.rb def archive dossier.archiver!(current_instructeur) redirect_back(fallback_location: instructeur_procedure_path(procedure)) end # app/models/dossier.rb def archiver!(author) update!(archived: true) log_dossier_operation(author, :archiver) end
# app/controllers/instructeurs/dossiers_controller.rb def unarchive dossier.desarchiver!(current_instructeur) redirect_back(fallback_location: instructeur_procedure_path(procedure)) end # app/models/dossier.rb def desarchiver!(author) update!(archived: false) log_dossier_operation(author, :desarchiver) end
Suivre un dossier
# app/controllers/instructeurs/dossiers_controller.rb def follow current_instructeur.follow(dossier) flash.notice = 'Dossier suivi' redirect_back(fallback_location: instructeur_procedure_path(procedure)) end # app/models/instructeur.rb def follow(dossier) begin followed_dossiers << dossier # If the user tries to follow a dossier she already follows, # we just fail silently: it means the goal is already reached. rescue ActiveRecord::RecordNotUnique # Database uniqueness constraint rescue ActiveRecord::RecordInvalid => e # ActiveRecord validation raise unless e.record.errors.details.dig(:instructeur_id, 0, :error) == :taken end end
# app/controllers/instructeurs/dossiers_controller.rb def unfollow current_instructeur.unfollow(dossier) flash.notice = "Vous ne suivez plus le dossier nº #{dossier.id}" redirect_back(fallback_location: instructeur_procedure_path(procedure)) end # app/models/instructeur.rb def unfollow(dossier) f = follows.find_by(dossier: dossier) if f.present? f.update(unfollowed_at: Time.zone.now) end end
Question : pas de log pour l'action de (ne plus) suivre un dossier ?
notes techniques de @akarzim pour l'étape 2 et 2 questions / remarques
2ème étape ---> mails envoyés dans certains cas
Suppression / Restauration
# app/controllers/instructeurs/dossiers_controller.rb def destroy if dossier.termine? dossier.hide_and_keep_track!(current_instructeur, :instructeur_request) flash.notice = t('instructeurs.dossiers.deleted_by_instructeur') else flash.alert = t('instructeurs.dossiers.impossible_deletion') end redirect_back(fallback_location: instructeur_procedure_path(procedure)) end # app/models/dossier.rb def hide_and_keep_track!(author, reason) transaction do if author_is_administration(author) && can_be_deleted_by_administration?(reason) update(hidden_by_administration_at: Time.zone.now, hidden_by_reason: reason) elsif author_is_user(author) && can_be_deleted_by_user? update(hidden_by_user_at: Time.zone.now, dossier_transfer_id: nil, hidden_by_reason: reason) else raise "Unauthorized dossier hide attempt Dossier##{id} by #{author} for reason #{reason}" end log_dossier_operation(author, :supprimer, self) end if en_construction? && !hidden_by_administration? administration_emails = followers_instructeurs.present? ? followers_instructeurs.map(&:email) : procedure.administrateurs.map(&:email) administration_emails.each do |email| DossierMailer.notify_en_construction_deletion_to_administration(self, email).deliver_later end end end
# app/controllers/instructeurs/dossiers_controller.rb def restore dossier = current_instructeur.dossiers.find(params[:dossier_id]) dossier.restore(current_instructeur) flash.notice = t('instructeurs.dossiers.restore') if dossier.termine? redirect_to instructeur_procedure_path(procedure, statut: :traites) else redirect_back(fallback_location: instructeur_procedure_path(procedure)) end end # app/models/dossier.rb def restore(author) transaction do if author_is_administration(author) update(hidden_by_administration_at: nil) elsif author_is_user(author) update(hidden_by_user_at: nil) end if !hidden_by_user? && !hidden_by_administration? update(hidden_by_reason: nil) end log_dossier_operation(author, :restaurer, self) end end
Questions / Remarques :
- Les transactions c'est bien, mais ne devrait-on pas envisager d'en exclure les logs ? Dit autrement, si le log crash, faut-il annuler la mise à jour ?
- Pourquoi n'envoie-t-on pas d'email aux administrateurs quand un dossier est restauré ?
notes techniques de @akarzim pour l'étape 3
3ème étape ---> mails envoyés
# app/controllers/instructeurs/dossiers_controller.rb def passer_en_instruction begin dossier.passer_en_instruction!(instructeur: current_instructeur) flash.notice = 'Dossier passé en instruction.' rescue AASM::InvalidTransition => e flash.alert = aasm_error_message(e, target_state: :en_instruction) end @dossier = dossier render :change_state end # app/models/dossier.rb aasm whiny_persistence: true, column: :state, enum: true do state :brouillon, initial: true state :en_construction state :en_instruction state :accepte state :refuse state :sans_suite event :passer_en_instruction, after: :after_passer_en_instruction do transitions from: :en_construction, to: :en_instruction end end def after_passer_en_instruction(h) instructeur = h[:instructeur] disable_notification = h.fetch(:disable_notification, false) instructeur.follow(self) self.en_construction_close_to_expiration_notice_sent_at = nil self.conservation_extension = 0.days self.en_instruction_at = self.traitements .passer_en_instruction(instructeur: instructeur) .processed_at save! if !procedure.declarative_accepte? && !disable_notification NotificationMailer.send_en_instruction_notification(self).deliver_later end log_dossier_operation(instructeur, :passer_en_instruction) end
Remarques :
- Les autres actions sont très similaires ;
- Plus complexe ici, on passe par une machine à états sur le dossier ;
- On touche à plusieurs objets ici (dossier, instructeur, traitements), il faudra penser à encapsuler ça dans une transaction ;
- La notification mail est faite de manière asynchrone ;
- Nous devrions très certainement faire de même pour les logs.
notes techniques de @akarzim pour l'étape 4
4ème étape --> mails envoyés + éventuellement un fichier
# app/controllers/instructeurs/dossiers_controller.rb def terminer motivation = params[:dossier] && params[:dossier][:motivation] justificatif = params[:dossier] && params[:dossier][:justificatif_motivation] h = { instructeur: current_instructeur, motivation: motivation, justificatif: justificatif } begin case params[:process_action] when "refuser" target_state = :refuse dossier.refuser!(h) flash.notice = "Dossier considéré comme refusé." when "classer_sans_suite" target_state = :sans_suite dossier.classer_sans_suite!(h) flash.notice = "Dossier considéré comme sans suite." when "accepter" target_state = :accepte dossier.accepter!(h) flash.notice = "Dossier traité avec succès." end rescue AASM::InvalidTransition => e flash.alert = aasm_error_message(e, target_state: target_state) end @dossier = dossier render :change_state end # app/models/dossier.rb def after_accepter(h) instructeur = h[:instructeur] motivation = h[:motivation] justificatif = h[:justificatif] disable_notification = h.fetch(:disable_notification, false) self.processed_at = self.traitements .accepter(motivation: motivation, instructeur: instructeur) .processed_at save! if justificatif self.justificatif_motivation.attach(justificatif) end if attestation.nil? self.attestation = build_attestation end save! remove_titres_identite! if !disable_notification NotificationMailer.send_accepte_notification(self).deliver_later end send_dossier_decision_to_experts(self) log_dossier_operation(instructeur, :accepter, self) end
Remarques :
- Les autres actions sont très similaires ;
- On passe ici aussi par une machine à états sur le dossier ;
- On attache des fichiers ;
- On envoie des notifications mail en asynchrone ;
- On transmet la décision aux experts, en asynchrone, mais avec des requêtes supplémentaire sur les avis ;
- On nettoie certaines données de manière asynchrone ;
- Ici encore, les logs devraient être fait de manière asynchrone ;
- Et encapsuler le tout (hors async) dans une transaction serait bienvenu.
notes techniques de @akarzim pour l'étape 1
Question : pas de log pour l'action de (ne plus) suivre un dossier ?
@akarzim / @dzc34 : pas de log c'est pas une info importante (au point qu'on penser arreter de logguer sur archive
notes techniques de @akarzim pour l'étape 2
Questions / Remarques :
* Les transactions c'est bien, mais ne devrait-on pas envisager d'en exclure les logs ? Dit autrement, si le log crash, faut-il annuler la mise à jour ?
Pour des raisons de sécurité/traçabilité, il nous faut les logs, il me semble que ca va forcement de pair. @LeSim / @tchak une confirmation serait top
* Pourquoi n'envoie-t-on pas d'email aux administrateurs quand un dossier est restauré ?
Je ne sais pas, p-e que le besoin n'a jamais été remonté
notes techniques de @akarzim pour l'étape 3
Remarques :
- Les autres actions sont très similaires ;
- Plus complexe ici, on passe par une machine à états sur le dossier ;
- On touche à plusieurs objets ici (dossier, instructeur, traitements), il faudra penser à encapsuler ça dans une transaction ;
- La notification mail est faite de manière asynchrone ;
- Nous devrions très certainement faire de même pour les logs.
Remarque sur les logs en asynchrone, de ma comprehension le dossier est serialisé au moment du logs, le passage en asynchrone n'est pas impossible, mais il faudrait serializer l'etat du dossier au moment du passage en async. @LeSim / @tchak, pas enorme visibilité la dessus, possible que vous confirmiez (ou pas?)
Questions / Remarques :
* Les transactions c'est bien, mais ne devrait-on pas envisager d'en exclure les logs ? Dit autrement, si le log crash, faut-il annuler la mise à jour ?
Pour des raisons de sécurité/traçabilité, il nous faut les logs, il me semble que ca va forcement de pair. @LeSim / @tchak une confirmation serait top
Oui, je confirme, ces logs sont importants. S'ils ne sont pas écrit, on ne veut pas réaliser l'action.
Remarque sur les logs en asynchrone, de ma comprehension le dossier est serialisé au moment du logs, le passage en asynchrone n'est pas impossible, mais il faudrait serializer l'etat du dossier au moment du passage en async. @LeSim / @tchak, pas enorme visibilité la dessus, possible que vous confirmiez (ou pas?)
je n'ai pas très bien compris le coup de l'asynchrone.
Tout ce que je peux vous dire c'est que le log représente l'état du dossier au moment ou l'action est prise. Si on ne fait pas les 2 en mm temps (passage_en_instruction, log) on ne peut pas prouver que notre log est correct.
Commentaire pour le suivi Adullact (@cmayran @dzc34)
voir
Actions par lot : :hourglass_flowing_sand:
à suivre
] :heavy_check_mark:
suivis
]
traités
] :heavy_check_mark:
supprimés
] :heavy_check_mark:
expirants
] :heavy_check_mark:
archivés
] :heavy_check_mark:
:warning: (1) ---> :stop_sign: ne sera pas implémenté
explication :
- pose des questions d'UX assez complexes et des modifications conséquentes de l'interface.
- éviter l'effet newsletter
:warning: (2) ---> :stop_sign: ne sera pas implémenté
explication :
- compliqué de revenir sur plusieurs dossiers qui ont déjà été traités : pas envie de faciliter cet usage. (dixit équipe DS)
Résumé
ETQ instructeur, je souhaite faire une même action sur plusieurs dossiers (passer en instruction, clôturer, ...)
voir en commentaire la liste des actions (déjà implémentées, à implémenter ou qui ne seront pas implémentées) : --------> https://github.com/demarches-simplifiees/demarches-simplifiees.fr/issues/7805#issuecomment-1386967187
Cadre de cette contribution
Demande initiale
via l'ARNIA (août 2022) à l'Adullact
Demandes similaires
Suggestion-Votes / Instructeur - passer plusieurs dossiers à la fois en instruction (2021)
Suggestion-Votes / Instructeur - Faciliter l'archivage des dossiers (2021)
Github Milestone - Traitement multiple de dossiers (instructeurs)
Github #5468 - ETQ Instructeur, je peux faire une même action sur des dossiers multiples
Github #2715 - ETQ Instructeur, j'aimerais passer plusieurs dossiers en archivés en même temps
Github #2013 - ETQ accompagnateur je veux demander des avis par batch de dossiers
Github #1617 - ETQ Instructeur je veux effectuer des actions de masse sur les dossiers
Actuellement
Si l'instructeur doit appliquer la même action sur plusieurs dossiers, il doit ouvrir un par un chaque dossier et appliquer à chaque fois la même action (Passer en instruction, Accepter, Archiver, ...).
Comportement attendu
L'instructeur peut sélectionner plusieurs dossiers et appliquer en même temps la même action (1) sur les dossiers sélectionnés.
voir en commentaire la liste des actions (déjà implémentées, à implémenter ou qui ne seront pas implémentées) : --------> https://github.com/demarches-simplifiees/demarches-simplifiees.fr/issues/7805#issuecomment-1386967187
Cahier des charges
User story
User story par type d'action
Suivre plusieurs dossiers
Ne plus suivre plusieurs dossiers
Passer "en instruction" plusieurs dossiers
Accepter, refuser ou "classer sans suite" plusieurs dossiers
Archiver plusieurs dossiers :
Demander un avis à la même personne sur plusieurs dossiers :
Envoyer un message identique aux usagers de plusieurs dossiers :
...
Tests d'acceptation
Passer "en instruction" plusieurs dossiers
Accepter, refuser ou "classer sans suite" plusieurs dossiers
Problématique UX
Status possibles des dossiers en fonction du type liste (onglet) :
en construction
/procedures/<idProcedure>?statut=a-suivre
en construction
/procedures/<idProcedure>?statut=suivis
refusé
classé sans suite
/procedures/<idProcedure>?statut=traites
en construction
accepté
refusé
classer sans suite
/procedures/<idProcedure>?statut=tous
refusé
classer sans suite
/procedures/<idProcedure>?statut=supprimes_recemment
refusé
classer sans suite
/procedures/<idProcedure>?statut=archives
Actions possibles en fonction du status du dossier et des flags
envoyer un message
demander un avis
passer en instruction
envoyer un message
demander un avis
passer en instruction
envoyer un message
demander un avis
re-passer en construction
classer sans suite
accepté
refusé
envoyer un message
demander un avis
re-passer en construction
classer sans suite
accepté
refusé
accepté
refusé
Supprimer
accepté
refusé
accepté
refusé