Merge pull request #6636 from betagouv/6624/etq-instructeur-je-souhaite-supprimer-des-messages-envoyes-par-erreur

6624/etq-instructeur-je-souhaite-supprimer-des-messages-envoyes-par-erreur 🚀

app/controllers/instructeurs/commentaires_controller.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+module Instructeurs
+  class CommentairesController < ProceduresController
+    def destroy
+      commentaire = Dossier.find(params[:dossier_id]).commentaires.find(params[:id])
+      if commentaire.sent_by?(current_instructeur)
+        commentaire.piece_jointe.purge_later if commentaire.piece_jointe.attached?
+        commentaire.discard!
+        commentaire.update!(body: '')
+        flash[:notice] = t('views.shared.commentaires.destroy.notice')
+      else
+        flash[:alert] = I18n.t('views.shared.commentaires.destroy.alert_reasons.acl')
+      end
+      redirect_to(messagerie_instructeur_dossier_path(params[:procedure_id], params[:dossier_id]))
+    rescue Discard::RecordNotDiscarded
+      flash[:alert] = I18n.t('views.shared.commentaires.destroy.alert_reasons.already_discarded')
+      redirect_to(messagerie_instructeur_dossier_path(params[:procedure_id], params[:dossier_id]))
+    end
+  end
+end

app/helpers/commentaire_helper.rb

@@ -24,6 +24,7 @@ module CommentaireHelper
   end
 
   def pretty_commentaire(commentaire)
+    return t('views.shared.commentaires.destroy.deleted_body') if commentaire.discarded?
     body_formatted = commentaire.sent_by_system? ? commentaire.body : simple_format(commentaire.body)
     sanitize(body_formatted)
   end

app/mailers/dossier_mailer.rb

@@ -6,6 +6,7 @@ class DossierMailer < ApplicationMailer
 
   layout 'mailers/layout'
   default from: NO_REPLY_EMAIL
+  after_action :prevent_perform_deliveries, only: [:notify_new_answer]
 
   def notify_new_draft(dossier)
     I18n.with_locale(dossier.user_locale) do
@@ -20,12 +21,14 @@ class DossierMailer < ApplicationMailer
     end
   end
 
-  def notify_new_answer(dossier, body = nil)
+  def notify_new_answer
+    commentaire = params[:commentaire]
+    dossier = commentaire.dossier
     I18n.with_locale(dossier.user_locale) do
       @dossier = dossier
       @service = dossier.procedure.service
       @logo_url = attach_logo(dossier.procedure)
-      @body = body
+      @body = commentaire.body
       @subject = default_i18n_subject(dossier_id: dossier.id, libelle_demarche: dossier.procedure.libelle)
 
       mail(to: dossier.user_email_for(:notification), subject: @subject) do |format|
@@ -168,6 +171,10 @@ class DossierMailer < ApplicationMailer
 
   protected
 
+  def prevent_perform_deliveries
+    mail.perform_deliveries = false if params[:commentaire].discarded?
+  end
+
   # This is an override of `default_i18n_subject` method
   # https://api.rubyonrails.org/v5.0.0/classes/ActionMailer/Base.html#method-i-default_i18n_subject
   #

app/models/commentaire.rb

@@ -4,6 +4,7 @@
 #
 #  id             :integer          not null, primary key
 #  body           :string
+#  discarded_at   :datetime
 #  email          :string
 #  created_at     :datetime         not null
 #  updated_at     :datetime         not null
@@ -13,6 +14,7 @@
 #
 class Commentaire < ApplicationRecord
   include FileValidationConcern
+  include Discard::Model
 
   self.ignored_columns = [:user_id]
   belongs_to :dossier, inverse_of: :commentaires, touch: true, optional: false
@@ -24,7 +26,7 @@ class Commentaire < ApplicationRecord
 
   has_one_attached :piece_jointe
 
-  validates :body, presence: { message: "ne peut être vide" }
+  validates :body, presence: { message: "ne peut être vide" }, unless: :discarded?
 
   FILE_MAX_SIZE = 20.megabytes
   validates :piece_jointe,
@@ -75,7 +77,11 @@ class Commentaire < ApplicationRecord
   end
 
   def sent_by?(someone)
-    email == someone.email
+    someone.present? && email == someone&.email
+  end
+
+  def soft_deletable?(connected_user)
+    sent_by?(connected_user) && sent_by_instructeur? && !discarded?
   end
 
   def file_url
@@ -92,13 +98,15 @@ class Commentaire < ApplicationRecord
     # - If a user or an invited user posted a commentaire, do nothing,
     #   the notification system will properly
     # - Otherwise, a instructeur posted a commentaire, we need to notify the user
-    if sent_by_instructeur? || sent_by_expert?
+    if sent_by_instructeur?
+      notify_user(wait: 5.minutes)
+    elsif sent_by_expert?
       notify_user
     end
   end
 
-  def notify_user
+  def notify_user(job_options = {})
-    DossierMailer.notify_new_answer(dossier, body).deliver_later
+    DossierMailer.with(commentaire: self).notify_new_answer.deliver_later(job_options)
   end
 
   def messagerie_available?

app/views/shared/dossiers/messages/_message.html.haml

@@ -11,6 +11,11 @@
   .rich-text= pretty_commentaire(commentaire)
 
   .message-extras.flex.justify-start
+    - if commentaire.soft_deletable?(connected_user)
+      = button_to  instructeur_commentaire_path(commentaire.dossier.procedure, commentaire.dossier, commentaire), method: :delete, class: 'button danger', data: { confirm: t('views.shared.commentaires.destroy.confirm') } do
+        %span.icon.delete
+        = t('views.shared.commentaires.destroy.button')
+
     - if commentaire.piece_jointe.attached?
       .attachment-link
         = render partial: "shared/attachment/show", locals: { attachment: commentaire.piece_jointe.attachment }

config/locales/views/shared/en.yml

@@ -1,6 +1,14 @@
 en:
   views:
     shared:
+      dossiers:
+        identite_entreprise:
+          warning_for_private_info: "The establishment %{etablissement} applied his right to not publish information regarding his identity. These informaiton won't be visible from instructor services"
+      avis:
+        demande_envoyee_le: "Feedback send at %{date}"
+        demande_revoquee_le: "Feedback revoked at %{date}"
+        reponse_donnee_le: "Response sent at %{date}"
+        en_attente: "Waiting for response"
       france_connect_login:
         title: "With FranceConnect"
         description: "France connect is a solution proposed by the government to secure and simplify the connection to web services."
@@ -12,3 +20,12 @@ en:
         already_user: "I already have an account"
         create: 'Create an account'
         signin: 'Sign in'
+      commentaires:
+        destroy:
+          button: 'Destroy this message'
+          confirm: "Are you sure you want to destroy this message ?"
+          deleted_body: Message deleted
+          notice: 'Your message had been deleted'
+          alert_reasons:
+            acl: "Can not destroy message: it does not belong to you"
+            already_discarded: "Can not destroy message: it was already destroyed"

config/locales/views/shared/fr.yml

@@ -20,3 +20,12 @@ fr:
         already_user: 'J’ai déjà un compte'
         create: 'Créer un compte'
         signin: 'Connexion'
+      commentaires:
+        destroy:
+          button: 'Supprimer le message'
+          confirm: "Êtes-vous sûr de vouloir supprimer ce message ?"
+          deleted_body: Message supprimé
+          notice: 'Votre message a été supprimé'
+          alert_reasons:
+            acl: "Impossible de supprimer le message, celui ci ne vous appartient pas"
+            already_discarded: "Ce message a déjà été supprimé"

config/routes.rb

@@ -351,6 +351,8 @@ Rails.application.routes.draw do
 
         resources :dossiers, only: [:show], param: :dossier_id do
           member do
+            resources :commentaires, only: [:destroy]
+
             get 'attestation'
             get 'geo_data'
             get 'apercu_attestation'

db/migrate/20211115112933_add_discarded_at_to_commentaires.rb

@@ -0,0 +1,6 @@
+class AddDiscardedAtToCommentaires < ActiveRecord::Migration[6.1]
+  def change
+    add_column :commentaires, :discarded_at, :datetime
+    # add_index :commentaires, :discarded_at
+  end
+end

db/schema.rb

@@ -10,8 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2021_11_04_102349) do
+ActiveRecord::Schema.define(version: 2021_11_15_112933) do
-
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
   enable_extension "unaccent"
@@ -219,6 +218,7 @@ ActiveRecord::Schema.define(version: 2021_11_04_102349) do
     t.bigint "user_id"
     t.bigint "instructeur_id"
     t.bigint "expert_id"
+    t.datetime "discarded_at"
     t.index ["dossier_id"], name: "index_commentaires_on_dossier_id"
     t.index ["expert_id"], name: "index_commentaires_on_expert_id"
     t.index ["instructeur_id"], name: "index_commentaires_on_instructeur_id"

spec/controllers/instructeurs/commentaires_controller_spec.rb

@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+describe Instructeurs::CommentairesController, type: :controller do
+  let(:instructeur) { create(:instructeur) }
+  let(:procedure) { create(:procedure, :published, :for_individual, instructeurs: [instructeur]) }
+  let(:dossier) { create(:dossier, :en_construction, :with_individual, procedure: procedure) }
+
+  before { sign_in(instructeur.user) }
+
+  describe 'destroy' do
+    context 'when it works' do
+      let(:commentaire) { create(:commentaire, instructeur: instructeur, dossier: dossier) }
+      subject { delete :destroy, params: { dossier_id: dossier.id, procedure_id: procedure.id, id: commentaire.id } }
+
+      it 'redirect to dossier' do
+        expect(subject).to redirect_to(messagerie_instructeur_dossier_path(dossier.procedure, dossier))
+      end
+      it 'flash success' do
+        subject
+        expect(flash[:notice]).to eq(I18n.t('views.shared.commentaires.destroy.notice'))
+      end
+    end
+
+    context 'when dossier had been discarded' do
+      let(:commentaire) { create(:commentaire, instructeur: instructeur, dossier: dossier, discarded_at: 2.hours.ago) }
+      subject { delete :destroy, params: { dossier_id: dossier.id, procedure_id: procedure.id, id: commentaire.id } }
+
+      it 'redirect to dossier' do
+        expect(subject).to redirect_to(messagerie_instructeur_dossier_path(dossier.procedure, dossier))
+      end
+      it 'flash success' do
+        subject
+        expect(flash[:alert]).to eq(I18n.t('views.shared.commentaires.destroy.alert_reasons.already_discarded'))
+      end
+    end
+  end
+end

spec/controllers/instructeurs/dossiers_controller_spec.rb

@@ -382,6 +382,11 @@ describe Instructeurs::DossiersController, type: :controller do
     end
   end
 
+  describe '#messagerie' do
+    subject { get :messagerie, params: { procedure_id: procedure.id, dossier_id: dossier.id } }
+    it { expect(response).to have_http_status(:ok) }
+  end
+
   describe "#create_commentaire" do
     let(:saved_commentaire) { dossier.commentaires.first }
     let(:body) { "avant\napres" }

spec/mailers/dossier_mailer_spec.rb

@@ -28,8 +28,8 @@ RSpec.describe DossierMailer, type: :mailer do
 
   describe '.notify_new_answer with dossier brouillon' do
     let(:dossier) { create(:dossier, procedure: build(:simple_procedure)) }
-
+    let(:commentaire) { create(:commentaire, dossier: dossier) }
-    subject { described_class.notify_new_answer(dossier) }
+    subject { described_class.with(commentaire: commentaire).notify_new_answer }
 
     it { expect(subject.subject).to include("Nouveau message") }
     it { expect(subject.subject).to include(dossier.id.to_s) }
@@ -40,8 +40,8 @@ RSpec.describe DossierMailer, type: :mailer do
 
   describe '.notify_new_answer with dossier en construction' do
     let(:dossier) { create(:dossier, state: "en_construction", procedure: build(:simple_procedure)) }
-
+    let(:commentaire) { create(:commentaire, dossier: dossier) }
-    subject { described_class.notify_new_answer(dossier) }
+    subject { described_class.with(commentaire: commentaire).notify_new_answer }
 
     it { expect(subject.subject).to include("Nouveau message") }
     it { expect(subject.subject).to include(dossier.id.to_s) }
@@ -50,6 +50,15 @@ RSpec.describe DossierMailer, type: :mailer do
     it_behaves_like 'a dossier notification'
   end
 
+  describe '.notify_new_answer with commentaire discarded' do
+    let(:dossier) { create(:dossier, procedure: build(:simple_procedure)) }
+    let(:commentaire) { create(:commentaire, dossier: dossier, discarded_at: 2.minutes.ago) }
+
+    subject { described_class.with(commentaire: commentaire).notify_new_answer }
+
+    it { expect(subject.perform_deliveries).to be_falsy }
+  end
+
   describe '.notify_deletion_to_user' do
     let(:deleted_dossier) { build(:deleted_dossier) }
 

spec/models/commentaire_spec.rb

@@ -40,6 +40,12 @@ describe Commentaire do
     end
   end
 
+  describe "sent_by?" do
+    let(:commentaire) { build(:commentaire, instructeur: build(:instructeur)) }
+    subject { commentaire.sent_by?(nil) }
+    it { is_expected.to be_falsy }
+  end
+
   describe "#redacted_email" do
     subject { commentaire.redacted_email }
 
@@ -80,8 +86,8 @@ describe Commentaire do
     context "with a commentaire created by a instructeur" do
       let(:commentaire) { CommentaireService.build(instructeur, dossier, body: "Mon commentaire") }
 
-      it "calls notify_user" do
+      it "calls notify_user with delay so instructeur can destroy his comment in case of failure" do
-        expect(commentaire).to receive(:notify_user)
+        expect(commentaire).to receive(:notify_user).with(wait: 5.minutes)
         commentaire.save
       end
     end
@@ -90,7 +96,7 @@ describe Commentaire do
       let(:commentaire) { CommentaireService.build(expert, dossier, body: "Mon commentaire") }
 
       it "calls notify_user" do
-        expect(commentaire).to receive(:notify_user)
+        expect(commentaire).to receive(:notify_user).with(no_args)
         commentaire.save
       end
     end
@@ -99,7 +105,7 @@ describe Commentaire do
       let(:commentaire) { CommentaireService.build_with_email(CONTACT_EMAIL, dossier, body: "Mon commentaire") }
 
       it "does not call notify_user" do
-        expect(commentaire).not_to receive(:notify_user)
+        expect(commentaire).not_to receive(:notify_user).with(no_args)
         commentaire.save
       end
     end

spec/views/shared/dossiers/messages/message.html.haml_spec.rb

@@ -46,5 +46,44 @@ describe 'shared/dossiers/messages/message.html.haml', type: :view do
         it { is_expected.not_to have_text(instructeur.email) }
       end
     end
+
+    describe 'delete message button for instructeur' do
+      let(:instructeur) { create(:instructeur) }
+      let(:procedure) { create(:procedure) }
+      let(:dossier) { create(:dossier, :en_construction, commentaires: [commentaire], procedure: procedure) }
+      subject { render 'shared/dossiers/messages/message.html.haml', commentaire: commentaire, messagerie_seen_at: seen_at, connected_user: instructeur, show_reply_button: true }
+      let(:form_url) { instructeur_commentaire_path(commentaire.dossier.procedure, commentaire.dossier, commentaire) }
+
+      context 'on a procedure where commentaire had been written by connected instructeur' do
+        let(:commentaire) { create(:commentaire, instructeur: instructeur, body: 'Second message') }
+
+        it { is_expected.to have_selector("form[action=\"#{form_url}\"]") }
+      end
+
+      context 'on a procedure where commentaire had been written by connected instructeur and discarded' do
+        let(:commentaire) { create(:commentaire, instructeur: instructeur, body: 'Second message', discarded_at: 2.days.ago) }
+
+        it { is_expected.not_to have_selector("form[action=\"#{form_url}\"]") }
+        it { is_expected.not_to have_selector(".rich-text", text: I18n.t(t('views.shared.commentaires.destroy.deleted_body'))) }
+      end
+
+      context 'on a procedure where commentaire had been written by connected an user' do
+        let(:commentaire) { create(:commentaire, email: create(:user).email, body: 'Second message') }
+
+        it { is_expected.not_to have_selector("form[action=\"#{form_url}\"]") }
+      end
+
+      context 'on a procedure where commentaire had been written by connected an expert' do
+        let(:commentaire) { create(:commentaire, expert: create(:expert), body: 'Second message') }
+
+        it { is_expected.not_to have_selector("form[action=\"#{form_url}\"]") }
+      end
+
+      context 'on a procedure where commentaire had been written another instructeur' do
+        let(:commentaire) { create(:commentaire, instructeur: create(:instructeur), body: 'Second message') }
+
+        it { is_expected.not_to have_selector("form[action=\"#{form_url}\"]") }
+      end
+    end
   end
 end