Harmonisation des doublons usagers suspectés

[adipasquale]

Ceci est une grosse PR, elle m'a pris un long moment et beaucoup d'allers-retours. Le code n'est pas encore parfait, mais ça va dans la bonne direction par rapport à aujourd'hui je pense.

Contexte

Un usager peut-être créé de plusieurs manières :

• inscription par l'usager normale
• inscription par l'usager avec FranceConnect
• création par un agent depuis le formulaire usager
• création par un agent depuis la modale dans le tunnel RDV
• création par un agent depuis l'API

Il y a différents cas d'usagers doublons que l'on veut gérer :

• sur base de l'email : interdits strictements partout
• sur base du numéro de téléphone : plus souple
• sur base de l'identité (nom prénom date de naissance) : plus souple

Lorsqu'un doublon est détecté on veut réagir différement selon le chemin emprunté :

• depuis l'admin, on veut pouvoir avertir des suspicions contournables, et on veut aussi pouvoir offrir des actions contextuelles : "aller modifier l'usager suspect plutôt qu'en créer un nouveau", "Choisir cet usager pour le RDV courant plutôt que d'en créer un nouveau" et enfin "associer cet usager suspect existant dans une autre orga à l'orga courante plutôt que d'en créer un nouveau"
• depuis les autres chemins, on veut pour l'instant uniquement respecter les interdictions strictes (càd uniquement sur base de l'email aujourd'hui). On n'inclut pas d'infos sur les avertissements contournables dans l'API, l'inscription via FC ni l'inscription normale des usagers.

Enfin, il faut aussi prendre en compte les chemins de mise à jour, dans lesquels on veut réagir de la même manière mais uniquement en cas de modification des champs menant à l'apparition du doublon. On ne veut pas ré-alerter de la présence d'un doublon sur base du numéro de téléphone si l'agent vient de changer l'adresse par ex.

cf screenshots tout en bas pour avoir une idée visuelle

Objectifs de cette PR

• harmoniser ce qui est interdit et autorisé depuis les différents chemins
• harmoniser la manière d'afficher les autorisations contournables avec le reste de l'admin (et harmoniser le code associé) cf refacto duplicate user warning #1214
• réparer le bug de réconciliation FC cf reparer reconciliation sur email pour FranceConnect #1237

Changements fonctionnels

• Aujourd'hui les doublons sur base de l'identité sont strictement interdits, ce qui ne me paraît pas correct (c'est d'ailleurs ce qu'on veut explicitement autoriser pour réparer le bug FC cf reparer reconciliation sur email pour FranceConnect #1237) . Je transforme donc cette interdiction stricte en avertissement contournable côté admin, et en autorisation silencieuse pour les autres chemins (inscription usagers, API, FranceConnect) ...
• Aujourd'hui tous les doublons sont interdits coté inscription usagers, sur base de l'email, du numéro de tel et de l'identité. C'est à mon avis un side effect innattendu plus qu'un choix. Je change ça pour que les inscriptions usagers ne respectent plus que les règles strictes (càd l'email uniquement)

Refactorings code

Form Object Admin::UserForm

introduction d'un Form Object Admin::UserForm dont le rôle est de rajouter des validations et warnings sur la création et update d'User, uniquement depuis les formulaires web de l'admin.

Ces validations et warnings étaient jusqu'ici faites dans le modèle User, ce qui ne permettait pas de se comporter différement selon le chemin emprunté.

Mon utilisation idéale d'un Form Object (FO) serait qu'il remplace complètement l'AR record qu'il wrappe dans le contrôleur et la vue, que l'on puisse faire simple_form_for(user_form), user_form.save, afficher les warnings du FO plutot que du record. Malheureusement ça ne se comporte pas très bien avec simple_form, principalement parce qu'il ne reconnaît pas bien les associations, même en déléguant bien du FO vers le record. Je fais donc un usage un peu hybride des deux objets, ce qui n'est pas très agréable :/

À cause de cette remarque précédente, ce FO rajoute des erreurs et des warnings sur le record (plutôt que d'avoir ses propres warnings et erreurs, et de les fusionner avec ceux du record). En plus, on fait un truc un peu hacky avec le active_warnings_confirm_decision qui vient de la gem activemodel-caution : il faut bien qu'il soit setté au niveau des params du FO et pas de ceux du record, car il faut le mettre au même niveau que l'appel à caution 😓 tricky

Contrôleur admin/users_controler

• on remplace les instanciations de user par des instanciations du UserForm
• on doit passer des view_locals qui vont être utilisées dans la génération des messages interactifs d'erreurs et de warnings. Ce n'est pas très agréable mais je n'ai pas d'autre solution en tête.
• on peut remplacer les manips custom avec skip_duplicate_warnings par le passage au bon niveau du active_warnings_confirm_decision qui vient de la gem activemodel-caution

Modèle User

beaucoup de nettoyage de vilaines méthodes :)

Screenshots

(après modifications décrites dans cette PR)

Screenshot avant après refacto pour utiliser le système d'avertissements générique

Screenshots admin/users#new meme orga

Screenshots admin/users#new different orga

Screenshots dans le tunnel RDV

[n-b]

(notes pour moi)

[n-b]

@yaf Sinon c’est bon pour moi. J’ai l’impression que la modale nous coûte assez cher en complexité du code, mais c’est pas l’objet ici.

Je veux bien que tu jettes un coup d’œil rapide :)

[yaf]

Je vais prendre un peu le temps de faire quelques tests si c'est ok pour toi d'attendre un peu.

[yaf]

Il y a une erreur 500 lorsque je pose un rendez-vous depuis l'agenda
https://sentry.io/organizations/rdv-solidarites/issues/2178934294/?project=1811205&referrer=RegressionActivityEmail

Bizarre d'ailleurs de ne pas avoir un test qui claque ici du coup 🤔 S'il y a bien un truc qui est sur le chemin critique, c'est bien poser un rendez-vous sur l'agenda :)

[n-b]

Mm, je vois la 500 sur la review app mais pas en local. (Au moins ça explique pourquoi les tests n’échouent pas.)

[yaf]

Ah oui. Ok, c'est peut-être lié à une migration ou une donnée inconsistante alors 🤔

[yaf]

Ça fait du bien ce petit nettoyage.

Je ne suis pas à l'aise avec cette histoire de modal. Mais je crois que pour s'en passer nous avons besoin de réfléchir un peu en termes de design

Pour l'usage des FormObjects, je ne suis pas encore pleinement convaincu, mais je n'ai pas non plus d'argument contre. Je suis prêt à essayer encore un peu :)