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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ Rails/LexicallyScopedActionFilter:
- 'app/controllers/agents/sessions_controller.rb'
- 'app/controllers/agent_auth_controller.rb'

Rails/ActiveRecordCallbacksOrder:
Enabled: false

# This allows the standard syntax for the `change` matcher, see:
# https://relishapp.com/rspec/rspec-expectations/docs/built-in-matchers/change-matcher
Lint/AmbiguousBlockAssociation:
Expand Down Expand Up @@ -149,6 +152,7 @@ RSpec/NestedGroups:
RSpec/DescribeClass:
Exclude:
- "spec/features/**/*"
- "spec/requests/**/*"

Style/AsciiComments:
Enabled: false
Expand Down
23 changes: 23 additions & 0 deletions app/controllers/inbound_emails_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# frozen_string_literal: true

# rubocop:disable Rails/ApplicationController
class InboundEmailsController < ActionController::Base
skip_before_action :verify_authenticity_token

before_action :authenticate_sendinblue

def sendinblue
payload = request.params["items"].first
TransferEmailReplyJob.perform_later(payload)
end

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


Sentry.capture_message("Sendinblue inbound controller was called without valid password")
head :unauthorized
end
end
# rubocop:enable Rails/ApplicationController
6 changes: 6 additions & 0 deletions app/javascript/stylesheets/mail.scss
Original file line number Diff line number Diff line change
Expand Up @@ -251,3 +251,9 @@ html,
margin: 0 !important;
}
}

blockquote {
background-color: $light;
padding: $padding;
margin: 15px;
}
92 changes: 92 additions & 0 deletions app/jobs/transfer_email_reply_job.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
# frozen_string_literal: true

class TransferEmailReplyJob < ApplicationJob
queue_as :mailers

def self.reply_address_for_rdv(rdv)
"rdv+#{rdv.uuid}@reply.rdv-solidarites.fr"
end

UUID_EXTRACTOR = /rdv\+([a-f0-9\-]*)@reply\.rdv-solidarites\.fr/

def perform(sendinblue_hash)
@sendinblue_hash = sendinblue_hash.with_indifferent_access

if rdv
notify_agents
else
Sentry.capture_message("Reply email could not be forwarded to agent, it was sent to default mailbox")
forward_to_default_mailbox
end
end

private

def notify_agents
Agents::ReplyTransferMailer.notify_agent_of_user_reply(
rdv: rdv,
author: user || source_mail.header[:from],
agents: rdv.agents,
reply_body: extracted_response,
source_mail: source_mail
).deliver_now
end

def forward_to_default_mailbox
Agents::ReplyTransferMailer.forward_to_default_mailbox(
reply_body: extracted_response,
source_mail: source_mail
).deliver_now
end

def rdv
Rdv.find_by(uuid: uuid) if uuid
end

def user
rdv&.users&.find_by(email: source_mail.from.first)
end

def uuid
source_mail.to.first.match(UUID_EXTRACTOR)&.captures&.first
end

def extracted_response
# Sendinblue provides us with both
# - the RAW email body (text + HTML)
# - a smart extraction of the content in markdown format
# We chose to use the smart extract because it already does all
# the hard work of excluding the quoted reply part.
[@sendinblue_hash[:ExtractedMarkdownMessage], @sendinblue_hash[:ExtractedMarkdownSignature]].compact.join("\n\n")
end

# @return [Mail::Message]
def source_mail
payload = @sendinblue_hash

@source_mail ||= Mail.new do
headers payload[:Headers]
subject payload[:Subject]

if payload[:RawTextBody].present?
text_part do
body payload[:RawTextBody]
end
end

if payload[:RawHtmlBody].present?
html_part do
content_type "text/html; charset=UTF-8"
body payload[:RawHtmlBody]
end
end

payload.fetch(:Attachments, []).each do |attachment_payload|
attachments[attachment_payload[:Name]] = {
mime_type: attachment_payload[:ContentType],
content: "", # Sendinblue webhook does not provide the content of attachments
}
end
end
end
end
32 changes: 32 additions & 0 deletions app/mailers/agents/reply_transfer_mailer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# frozen_string_literal: true

class Agents::ReplyTransferMailer < ApplicationMailer
include DateHelper
include ActionView::Helpers::TextHelper

# @param [Rdv] rdv
# @param [User, String] author
# @param [Array<Agent>] agents
# @param [Mail::Message] source_mail
def notify_agent_of_user_reply(rdv:, author:, agents:, reply_body:, source_mail:)
@rdv = rdv
@author = author
@reply_subject = source_mail.subject
@reply_body = reply_body
@attachment_names = source_mail.attachments.map(&:filename).join(", ")
@date = relative_date(@rdv.starts_at)

mail(to: agents.map(&:email), subject: t(".title", date: @date))
end

# @param [String] reply_body
# @param [Mail::Message] source_mail
def forward_to_default_mailbox(reply_body:, source_mail:)
@author = source_mail.header[:from]
@reply_subject = source_mail.subject
@reply_body = reply_body
@attachment_names = source_mail.attachments.map(&:filename).join(", ")

mail(to: "support@rdv-solidarites.fr", subject: t(".title"))
end
end
2 changes: 1 addition & 1 deletion app/mailers/users/file_attente_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class Users::FileAttenteMailer < ApplicationMailer
@token = params[:token]
end

default to: -> { @user.email }
default to: -> { @user.email }, reply_to: -> { TransferEmailReplyJob.reply_address_for_rdv(@rdv) }

def new_creneau_available
subject = t("users.file_attente_mailer.new_creneau_available.title")
Expand Down
2 changes: 1 addition & 1 deletion app/mailers/users/rdv_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class Users::RdvMailer < ApplicationMailer
@token = params[:token]
end

default to: -> { @user.email }
default to: -> { @user.email }, reply_to: -> { TransferEmailReplyJob.reply_address_for_rdv(@rdv) }

def rdv_created
self.ics_payload = @rdv.payload(:create, @user)
Expand Down
7 changes: 1 addition & 6 deletions app/models/rdv.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def nested_lieu_attributes
# Hooks
after_save :associate_users_with_organisation
after_commit :update_agents_unknown_past_rdv_count, if: -> { past? }
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.


# Scopes
scope :not_cancelled, -> { where(status: NOT_CANCELLED_STATUSES) }
Expand Down Expand Up @@ -301,11 +301,6 @@ def update_agents_unknown_past_rdv_count
agents.each(&:update_unknown_past_rdv_count!)
end

def reload_uuid
# https://github.com/rails/rails/issues/17605
self[:uuid] = self.class.where(id: id).pick(:uuid) if attributes.key? "uuid"
end

def cant_destroy_if_receipts_exist
# This is similar to using :restrict_with_errors on the has_many :receipts relation, but we want a custom error
return if receipts.empty?
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
div
p = t("mailers.common.hello")
p = t(".intro", author: @author)
blockquote
h4 = @reply_subject
div = simple_format(@reply_body)

p = t("agents.reply_transfer_mailer.shared.attachments", attachment_names: @attachment_names) if @attachment_names.present?

p = t(".instructions")
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
div
p = t("mailers.common.hello")
p = t(".intro", date: @date, author: @author)
blockquote
h4 = @reply_subject
div = simple_format(@reply_body)

p = auto_link t("agents.reply_transfer_mailer.shared.attachments", attachment_names: @attachment_names) if @attachment_names.present?

p = t(".instructions")

.btn-wrapper
= link_to "Voir le RDV", admin_organisation_rdv_url(@rdv.organisation_id, @rdv.id), class: "btn btn-primary"
11 changes: 11 additions & 0 deletions config/locales/mailers.fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,17 @@ fr:
revoked_at_date_by_agent: Un RDV qui devait avoir lieu %{date} vient d’être annulé par %{author} pour raison administrative.
cancelled_at_date_by_agent: Un RDV qui devait avoir lieu %{date} vient d’être annulé par %{author} à la demande de l’usager.
cancelled_at_date_by_user: Un RDV qui devait avoir lieu %{date} vient d’être annulé par l’usager %{author}.
reply_transfer_mailer:
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 :"
instructions: Merci de ne pas répondre à cet e-mail. Vous pouvez contacter l'usager⋅e à l'aide des informations inclues dans le RDV. Vous trouverez en pièce jointe l'email original.
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 :"
instructions: Merci de ne pas répondre à cet e-mail. Vous trouverez en pièce jointe l'email original.
shared:
attachments: Le mail de l'usager⋅e avait en pièce jointe "%{attachment_names}". Il nous est impossible de vous transmettre ce fichier. Vous pouvez contacter l'usager⋅e pour qu'iel l'envoie à votre adresse.
users:
file_attente_mailer:
new_creneau_available:
Expand Down
2 changes: 2 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,8 @@ def format_redirect_params(params)
get "admin/organisations/:organisation_id/agents/:agent_id", to: redirect("/admin/organisations/%{organisation_id}/agent_agendas/%{agent_id}")
# rubocop:enable Style/FormatStringToken

post "/inbound_emails/sendinblue", controller: :inbound_emails, action: :sendinblue

if Rails.env.development?
namespace :lapin do
resources :sms_preview, only: %i[index] do
Expand Down
106 changes: 106 additions & 0 deletions spec/jobs/transfer_email_reply_job_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
# frozen_string_literal: true

RSpec.describe TransferEmailReplyJob do
subject(:perform_job) { described_class.perform_now(sendinblue_payload) }

before do
# Set a fixed date so we can assert on dates within email body
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.

😄

let!(:agent) { create(:agent, email: "je_suis_un_agent@departement.fr") }
let(:rdv_uuid) { "8fae4d5f-4d63-4f60-b343-854d939881a3" }
let!(:rdv) { create(:rdv, users: [user], agents: [agent], uuid: rdv_uuid) }

let(:sendinblue_valid_payload) do
# The usual payload has more info, but I removed non-essential fields for readability.
# See: https://developers.sendinblue.com/docs/inbound-parsing-api-1#sample-payload
{
Cc: [],
ReplyTo: nil,
Subject: "coucou",
Attachments: [],
Headers: {
"Message-ID": "<d6c8663e3763aa750345a76c17f435a2bd14eded.camel@lapin.fr>",
Subject: "coucou",
From: "Bénédicte Ficiaire <bene_ficiaire@lapin.fr>",
To: "rdv+8fae4d5f-4d63-4f60-b343-854d939881a3@reply.rdv-solidarites.fr",
Date: "Thu, 12 May 2022 12:22:15 +0200",
},
ExtractedMarkdownMessage: "Je souhaite annuler mon RDV",
ExtractedMarkdownSignature: nil,
RawHtmlBody: %(<html dir="ltr"><head></head><body style="text-align:left; direction:ltr;"><div>Je souhaite annuler mon RDV</div>\n</body></html>\n),
RawTextBody: "Je souhaite annuler mon RDV\n",
}
end
let(:sendinblue_payload) { sendinblue_valid_payload } # use valid payload by default

context "when all goes well" do
it "sends a notification email to the agent, containing the user reply" do
expect { perform_job }.to change { ActionMailer::Base.deliveries.size }.by(1)
transferred_email = ActionMailer::Base.deliveries.last
expect(transferred_email.to).to eq(["je_suis_un_agent@departement.fr"])
expect(transferred_email.from).to eq(["contact@rdv-solidarites.fr"])
expect(transferred_email.reply_to).to eq(["support@rdv-solidarites.fr"])
expect(transferred_email.html_part.body.to_s).to include("Dans le cadre du RDV du 20 mai, l'usager⋅e Bénédicte FICIAIRE a envoyé")
expect(transferred_email.html_part.body.to_s).to include("Je souhaite annuler mon RDV") # reply content
expect(transferred_email.html_part.body.to_s).to include(%(href="http://#{ApplicationMailer.default_url_options[:host]}/admin/organisations/#{rdv.organisation_id}/rdvs/#{rdv.id}))
end
end

context "when reply token does not match any in DB" do
let(:rdv_uuid) { "6df62597-632e-4be1-a273-708ab58e4765" }

it "sends a notification email to the default mailbox, containing the user reply" do
expect { perform_job }.to change { ActionMailer::Base.deliveries.size }.by(1)
transferred_email = ActionMailer::Base.deliveries.last
expect(transferred_email.to).to eq(["support@rdv-solidarites.fr"])
expect(transferred_email.from).to eq(["contact@rdv-solidarites.fr"])
expect(transferred_email.reply_to).to eq(["support@rdv-solidarites.fr"])
expect(transferred_email.html_part.body.to_s).to include(%(L'usager⋅e "Bénédicte Ficiaire" &lt;bene_ficiaire@lapin.fr&gt; a répondu))
expect(transferred_email.html_part.body.to_s).to include("Je souhaite annuler mon RDV") # reply content
end

it "warns Sentry" do
expect(Sentry).to receive(:capture_message).with("Reply email could not be forwarded to agent, it was sent to default mailbox")
perform_job
end
end

context "when an e-mail address does not match our pattern" do
let(:sendinblue_payload) do
sendinblue_valid_payload.tap { |hash| hash[:Headers][:To] = "nimportequoi@reply.rdv-solidarites.fr" }
end

it "is forwarded to default mailbox" do
expect { perform_job }.to change { ActionMailer::Base.deliveries.size }.by(1)
transferred_email = ActionMailer::Base.deliveries.last
expect(transferred_email.to).to eq(["support@rdv-solidarites.fr"])
expect(transferred_email.html_part.body.to_s).to include(%(L'usager⋅e "Bénédicte Ficiaire" &lt;bene_ficiaire@lapin.fr&gt; a répondu))
end
end

context "when several agents are linked to the RDV" do
let!(:other_agent) { create(:agent, email: "autre@departement.fr").tap { |a| rdv.agents << a } }

it "sends one email with all agents in the TO: field" do
perform_job
expect(ActionMailer::Base.deliveries.last.to).to match_array(["je_suis_un_agent@departement.fr", "autre@departement.fr"])
end
end

context "when attachments are present" do
let(:sendinblue_payload) do
sendinblue_valid_payload.tap do |hash|
hash[:Attachments] = [{ Name: "mon_scan.pdf", ContentType: "application/pdf" }]
end
end

it "mentions the attachments in the notification e-mail" do
expect { perform_job }.to change { ActionMailer::Base.deliveries.size }.by(1)
transferred_email = ActionMailer::Base.deliveries.last
expect(transferred_email.html_part.body.to_s).to include(%(Le mail de l'usager⋅e avait en pièce jointe "mon_scan.pdf".))
end
end
end
Loading