Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Permettre aux usagers de répondre directement aux agents #2473

Merged
merged 1 commit into from
May 30, 2022

Conversation

francois-ferrandis
Copy link
Contributor

@francois-ferrandis francois-ferrandis commented May 12, 2022

Closes #1649

Le fonctionnement en gros : lorsqu'un⋅e usagèr⋅e reçoit un mail de notif, iel peut y répondre par mail et sa réponse est transmise à / aux agent(s) liés au RDV.

Fonctionnalités

  • Sécuriser l'adresse de callback appelée par Sendinblue (utiliser un password)
  • Retrouver un RDV à partir du token présent dans l'adresse
  • Envoyer un mail de notif aux agents du RDV avec
    • le contenu de la réponse
    • un lien vers le RDV
    • la liste des pièces jointes présentes
    • l'e-mail original en pièce jointe
  • Faire en sorte que les e-mails de notification envoyés aux usagers aient bien un champ "reply to" qui contient le token*
  • Envoyer le mail de façon asynchrone
  • S'assurer que la citation du mail de notif original est ignorée lorsque l'on transmet le mail de réponse (utiliser le champ ExtractedMarkdownMessage de Sendinblue)
  • Envoyer un seul mail de notif à tous les agents, plutôt que des mails séparés

Aperçu du mail transmis aux agents

image

Choix techniques

Où stocker le token ?

Afin de pouvoir retrouver un RDV à partir de l'e-mail de réponse, il fallait stocker un identifiant dans la base et faire en sorte de le joindre au mail de réponse, en utilisant une adresse du type rdv+UUID@reply.rdv-solidarites.fr.

Il était possible de stocker le token dans :

  • un Rdv
  • un RdvsUser
  • un Receipt

J'ai fait le choix de stocker le token dans Rdv pour le moment, principalement car les mailers n'ont pas accès aux RdvsUsers et au Receipts, et qu'il était donc difficile de générer l'adresse de ReplyTo depuis les mailers.

Je me suis aussi dit que si une personne était ajoutée par erreur à un RDV puis supprimée juste ensuite, elle allait recevoir un mail de notification pour la création, et il était alors préférable qu'elle puisse répondre, ce qui aurait été impossible si le token était stocké dans un RdvsUser.

Je me dis aussi que si on change d'avis (par exemple stocker le token dans les Receipts), la migration sera simple.

Comment router les mails reçus à rdv+***@reply.rdv-solidarites.fr ?

Deux options s'offrent à nous :

  1. créer une boite mail chez Gandi, et faire du polling dessus pour récupérer les nouveaux mails, puis les traiter.
  2. utiliser un service externe qui permet que tous les e-mails envoyés à un sous-domaine soient détectés et transmis en HTTP à notre appli

La solution retenue pour le moment est la seconde, à travers le système proposé par Sendinblue (qui est le seul non américain à proposer ce service, et qui se trouve être déjà notre outil d'envoi de mails transactionnels). Ce choix nous a paru le plus simple, mais rien ne nous empêchera de changer à l'avenir.

J'ai testé l'envoi de mails depuis mes adresses perso vers le domaine @reply.rdv-solidarites.fr avec un webhook en place qui transmet à une URL de test proposée par webhook.site.

Note : les webhooks Sendinblue de type "inbound" n’apparaissent pas dans leur interface web, mais sont bien listés via leur API (il faut bien demander les webhooks de type inbound et non transactional).

Sous quel format transmettre le contenu de la réponse ?

Lorsque l'usagè⋅re répond au mail de notification, son client mail va faire en sorte de citer le mail original et de permettre de répondre au dessus de cette citation. Cette citation ne nous intéresse pas, ou du moins nous ne souhaitons pas la faire apparaître lorsque nous notifions l'agent de la réponse.

La bonne nouvelle, c'est que Sendinblue nous fournit dans son payload une version Markdown du mail, dans laquelle ils ont déjà exclu la citation. C'est donc ce champ (ExtractedMarkdownMessage) que nous allons utiliser pour transmettre la réponse.

Checklist

Avant la revue :

  • Préparer des captures de l’interface avant et après
  • Nettoyer les commits pour faciliter la relecture
  • Supprimer les éventuels logs de test et le code mort

Revue :

  • Relecture du code
  • Test sur la review app / en local

@francois-ferrandis francois-ferrandis self-assigned this May 12, 2022
Copy link
Contributor

@n-b n-b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Je note ici pour en discuter plus tard)

Si je recolle les morceaux, c’est SendInBlue qui reçoit l’email et nous appelle sur /email_reply avec un le mail dans le body en json, c’est bien ça?

  • comment on authentifie SIB ?
  • qu’est-ce qu’on fait si on ne trouve pas l’identifiant dans un email ? On transfère à contact ?
  • est-ce qu’on utilise rdv.id ou bien un identifiant plus spécifique au message ?
  • est-ce qu’on peut utiliser le header MessageID? Est-ce que ça a du sens ?
  • est-ce qu’on garde un receipt du message transféré?

email_payload = params["items"].first
to_address = email_payload["To"].first["Address"]
rdv_identifier = to_address.match(/hello\+([A-Za-z0-9]*)@reply\.rdv-solidarites\.fr/).captures.first
raise rdv_identifier
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahah, “raise” c’est pour debugguer sans faire de vue?

Copy link
Contributor Author

@francois-ferrandis francois-ferrandis May 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oui, je débug pas mal à base de raise dans les phases expérimentales, ça a pour avantage d'afficher la valeur que tu sois dans un test ou dans un navigateur. Ça a aussi pour avantage que tu risques moins de laisser traîner un raise qu'un puts. 😉

Je n'irais pas jusqu'à dire que je recommande cette méthode, mais c'est une habitude et jusqu'ici ça fait bien la job.

@francois-ferrandis
Copy link
Contributor Author

francois-ferrandis commented May 16, 2022

(Je note ici pour en discuter plus tard)

Si je recolle les morceaux, c’est SendInBlue qui reçoit l’email et nous appelle sur /email_reply avec un le mail dans le body en json, c’est bien ça?

  • comment on authentifie SIB ?

  • qu’est-ce qu’on fait si on ne trouve pas l’identifiant dans un email ? On transfère à contact ?

  • est-ce qu’on utilise rdv.id ou bien un identifiant plus spécifique au message ?

  • est-ce qu’on peut utiliser le header MessageID? Est-ce que ça a du sens ?

  • est-ce qu’on garde un receipt du message transféré?

Que de bonnes questions !

Je me demandais aussi, lorsqu'on envoie le mail, en fait on passe par Sendinblue, donc on se retrouve à avoir des mails persos non-transactionnels dans nos logs d'envoi d'e-mails transactionnels. Un bon gros sujet au final !

@gitguardian
Copy link

gitguardian bot commented May 16, 2022

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
Once a secret has been leaked into a git repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!

@francois-ferrandis francois-ferrandis force-pushed the email-reply-relay-FRF branch 3 times, most recently from 0254aec to 72aecf4 Compare May 19, 2022 08:31
@francois-ferrandis francois-ferrandis force-pushed the email-reply-relay-FRF branch 2 times, most recently from 6582e3b to 1303e29 Compare May 24, 2022 08:19
Comment on lines -61 to +60
after_commit :reload_uuid, on: :create
before_validation { self.uuid ||= SecureRandom.uuid }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J'ai trouvé plus simple de générer l'UUID en Ruby plutôt que de devoir recharger après création l'UUID généré par Postgres.

@francois-ferrandis francois-ferrandis force-pushed the email-reply-relay-FRF branch 3 times, most recently from cad74c6 to 1371d3a Compare May 25, 2022 06:45
@francois-ferrandis francois-ferrandis marked this pull request as ready for review May 25, 2022 06:45
Copy link
Contributor

@victormours victormours left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trop bien ! Ça va faire gagner un temps ouf à l'équipe support.
Il y a peut-être des améliorations mineures qui sont possibles sur les clés de traduction, mais rien de bloquant.

private

def authenticate_sendinblue
return if ActiveSupport::SecurityUtils.secure_compare(ENV["SENDINBLUE_INBOUND_PASSWORD"], params[:password])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😍 magnifique ce secure_compare

notify_agent_of_user_reply:
title: Message d'usager⋅e au sujet de votre RDV %{date}
intro: "Dans le cadre du RDV du %{date}, l'usager⋅e %{author} a envoyé la réponse suivante par e-mail :"
attachments: Le mail de l'usager avait en pièce jointe "%{attachment_names}". Vous pouvez lui demander de les transmettre à nouveau.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
attachments: Le mail de l'usager avait en pièce jointe "%{attachment_names}". Vous pouvez lui demander de les transmettre à nouveau.
attachments: Le mail de l'usager avait en pièce jointe "%{attachment_names}". Les pièces jointes n'ont pas pu être ajoutées à ce mail automatique, mais vous pouvez lui demander de les transmettre à nouveau.

Copy link

@yaf yaf May 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je n'ai pas de proposition de formulation, mais un truc me semble très peu clair dans cette phrase :

Est-ce que la pièce jointe n'a pas pu être ajouté ici accidentellement ou bien c'est jamais possible ?

et aussi

Je demande à l'usager de tranferer « encore ? » la pièce jointe sur le même mail ?

Peut-être suggérer d'utiliser un outil de partage de fichier chiffré de bout en bout comme https://drop.chapril.org/

ou un de la liste des chatons https://www.chatons.org/search/by-service?service_type_target_id=148&field_alternatives_aux_services_target_id=All&field_software_target_id=All&field_is_shared_value=All&title=
(attention, tous ne propose pas un outil qui fait du chiffrement).

forward_to_default_mailbox:
title: Message d'usager⋅e en réponse à un e-mail de notification
intro: "L'usager⋅e %{author} a répondu à un e-mail de notification :"
attachments: Le mail de l'usager avait en pièce jointe "%{attachment_names}". Vous pouvez lui demander de les transmettre à nouveau.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
attachments: Le mail de l'usager avait en pièce jointe "%{attachment_names}". Vous pouvez lui demander de les transmettre à nouveau.
attachments: Le mail de l'usager avait en pièce jointe "%{attachment_names}". Les pièces jointes n'ont pas pu être ajoutées à ce mail automatique, mais vous pouvez lui demander de les transmettre à nouveau.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plutôt que de dupliquer les clés de traductions, on pourrait les réutiliser ?

travel_to(Time.zone.parse("2022-05-17 16:00:00"))
end

let!(:user) { create(:user, email: "bene_ficiaire@lapin.fr", first_name: "Bénédicte", last_name: "Ficiaire") }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😄

@francois-ferrandis francois-ferrandis force-pushed the email-reply-relay-FRF branch 6 times, most recently from a15afaf to 70052a0 Compare May 30, 2022 09:30
@francois-ferrandis francois-ferrandis merged commit 430590c into production May 30, 2022
@francois-ferrandis francois-ferrandis deleted the email-reply-relay-FRF branch May 30, 2022 10:18
francois-ferrandis pushed a commit that referenced this pull request Jun 1, 2022
Reprise des informations laissé par François dans la PR #2473
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Permettre aux usagers de répondre directement aux agents
4 participants