Merge pull request #7107 from betagouv/main

2022-04-04-01

app/controllers/experts/avis_controller.rb

@@ -6,7 +6,7 @@ module Experts
     before_action :authenticate_expert!, except: [:sign_up, :update_expert]
     before_action :check_if_avis_revoked, only: [:show]
     before_action :redirect_if_no_sign_up_needed, only: [:sign_up, :update_expert]
-    before_action :set_avis_and_dossier, only: [:show, :instruction, :messagerie, :create_commentaire, :update, :telecharger_pjs]
+    before_action :set_avis_and_dossier, only: [:show, :instruction, :messagerie, :create_commentaire, :delete_commentaire, :update, :telecharger_pjs]
 
     A_DONNER_STATUS = 'a-donner'
     DONNES_STATUS   = 'donnes'
@@ -113,6 +113,22 @@ module Experts
       end
     end
 
+    def delete_commentaire
+      commentaire = avis.dossier.commentaires.find(params[:commentaire])
+      if commentaire.sent_by?(current_expert)
+        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_expert_avis_path(avis.procedure, avis))
+    rescue Discard::RecordNotDiscarded
+      flash[:alert] = I18n.t('views.shared.commentaires.destroy.alert_reasons.already_discarded')
+      redirect_to(messagerie_expert_avis_path(avis.procedure, avis))
+    end
+
     def bilans_bdf
       if avis.dossier.etablissement&.entreprise_bilans_bdf.present?
         extension = params[:format]

app/lib/download_manager/parallel_download_queue.rb

@@ -36,11 +36,12 @@ module DownloadManager
         File.write(attachment_path, attachment.file.read, mode: 'wb')
       else
         request = Typhoeus::Request.new(attachment.url)
-        request.on_body do |chunk|
-          File.write(attachment_path, chunk, mode: 'a+b')
-        end
         request.on_complete do |response|
-          unless response.success?
+          if response.success?
+            File.open(attachment_path, mode: "wb") do |fd|
+              fd.write(response.body)
+            end
+          else
             File.delete(attachment_path) if File.exist?(attachment_path) # -> case of retries failed, must cleanup partialy downloaded file
             on_error.call(attachment, path_in_download_dir, response.code)
           end

app/models/commentaire.rb

@@ -80,7 +80,7 @@ class Commentaire < ApplicationRecord
   end
 
   def soft_deletable?(connected_user)
-    sent_by?(connected_user) && sent_by_instructeur? && !discarded?
+    sent_by?(connected_user) && (sent_by_instructeur? || sent_by_expert?) && !discarded?
   end
 
   def file_url
@@ -97,10 +97,8 @@ 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?
+    if sent_by_instructeur? || sent_by_expert?
       notify_user(wait: 5.minutes)
-    elsif sent_by_expert?
-      notify_user
     end
   end
 

app/services/pieces_justificatives_service.rb

@@ -4,16 +4,14 @@ class PiecesJustificativesService
     pjs_commentaires = pjs_for_commentaires(dossier)
     pjs_dossier = pjs_for_dossier(dossier, for_expert)
 
-    (pjs_champs + pjs_commentaires + pjs_dossier)
+    pjs_champs + pjs_commentaires + pjs_dossier.filter(&:attached?)
-      .filter(&:attached?)
   end
 
   def self.liste_pieces_justificatives(dossier)
     pjs_champs = pjs_for_champs(dossier)
     pjs_commentaires = pjs_for_commentaires(dossier)
 
-    (pjs_champs + pjs_commentaires)
+    pjs_champs + pjs_commentaires
-      .filter(&:attached?)
   end
 
   def self.pieces_justificatives_total_size(dossier)
@@ -121,21 +119,27 @@ class PiecesJustificativesService
   private
 
   def self.pjs_for_champs(dossier, for_expert = false)
-    allowed_champs = for_expert ? dossier.champs : dossier.champs + dossier.champs_private
+    champs = Champ
+      .joins(:piece_justificative_file_attachment)
+      .where(type: "Champs::PieceJustificativeChamp", dossier: dossier)
 
-    allowed_child_champs = allowed_champs
+    if for_expert
-      .filter { |c| c.type_champ == TypeDeChamp.type_champs.fetch(:repetition) }
+      champs = champs.where(private: false)
-      .flat_map(&:champs)
+    end
 
-    (allowed_champs + allowed_child_champs)
+    ActiveStorage::Attachment
-      .filter { |c| c.type_champ == TypeDeChamp.type_champs.fetch(:piece_justificative) }
+      .includes(:blob)
-      .map(&:piece_justificative_file)
+      .where(record_type: "Champ", record_id: champs.ids)
   end
 
   def self.pjs_for_commentaires(dossier)
-    dossier
+    commentaires = Commentaire
-      .commentaires
+      .joins(:piece_jointe_attachment)
-      .map(&:piece_jointe)
+      .where(dossier: dossier)
+
+    ActiveStorage::Attachment
+      .includes(:blob)
+      .where(record_type: "Commentaire", record_id: commentaires.ids)
   end
 
   def self.pjs_for_dossier(dossier, for_expert = false)

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

@@ -12,7 +12,8 @@
 
   .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
+      - path = connected_user.is_a?(Instructeur) ? instructeur_commentaire_path(commentaire.dossier.procedure, commentaire.dossier, commentaire) : delete_commentaire_expert_avis_path(@avis.procedure, @avis, commentaire: commentaire)
+      = button_to path, method: :delete, class: 'button danger', data: { confirm: t('views.shared.commentaires.destroy.confirm') } do
         %span.icon.delete
         = t('views.shared.commentaires.destroy.button')
 

config/routes.rb

@@ -310,6 +310,7 @@ Rails.application.routes.draw do
             get 'instruction'
             get 'messagerie'
             post 'commentaire' => 'avis#create_commentaire'
+            delete 'delete_commentaire' => 'avis#delete_commentaire'
             post 'avis' => 'avis#create_avis'
             get 'bilans_bdf'
             get 'telecharger_pjs' => 'avis#telecharger_pjs'

spec/models/commentaire_spec.rb

@@ -95,8 +95,8 @@ describe Commentaire do
     context "with a commentaire created by an expert" do
       let(:commentaire) { CommentaireService.build(expert, dossier, body: "Mon commentaire") }
 
-      it "calls notify_user" do
+      it "calls notify_user with delay so expert can destroy his comment in case of failure" do
-        expect(commentaire).to receive(:notify_user).with(no_args)
+        expect(commentaire).to receive(:notify_user).with(wait: 5.minutes)
         commentaire.save
       end
     end

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

@@ -86,4 +86,51 @@ describe 'shared/dossiers/messages/message.html.haml', type: :view do
       end
     end
   end
+
+  context 'with an expert message' do
+    describe 'delete message button for expert' do
+      let(:expert) { create(:expert) }
+      let(:procedure) { create(:procedure) }
+      let(:dossier) { create(:dossier, :en_construction, commentaires: [commentaire], procedure: procedure) }
+      let(:experts_procedure) { create(:experts_procedure, procedure: procedure, expert: expert) }
+      let!(:avis) { create(:avis, email: nil, experts_procedure: experts_procedure) }
+      subject { render 'shared/dossiers/messages/message.html.haml', commentaire: commentaire, messagerie_seen_at: seen_at, connected_user: expert, show_reply_button: true }
+      let(:form_url) { delete_commentaire_expert_avis_path(avis.procedure, avis, commentaire: commentaire) }
+
+      before do
+        assign(:avis, avis)
+      end
+
+      context 'on a procedure where commentaire had been written by connected expert' do
+        let(:commentaire) { create(:commentaire, expert: expert, 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 expert and discarded' do
+        let(:commentaire) { create(:commentaire, expert: expert, 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 instructeur' do
+        let(:commentaire) { create(:commentaire, instructeur: create(:instructeur), 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 expert' do
+        let(:commentaire) { create(:commentaire, expert: create(:expert), body: 'Second message') }
+
+        it { is_expected.not_to have_selector("form[action=\"#{form_url}\"]") }
+      end
+    end
+  end
 end