Merge pull request #5386 from betagouv/5138-revoque-avis-expert

[Revoque une demande d'avis à un expert](https://github.com/betagouv/demarches-simplifiees.fr/pull/5386)

app/controllers/instructeurs/avis_controller.rb

@@ -3,6 +3,7 @@ module Instructeurs
     include CreateAvisConcern
 
     before_action :authenticate_instructeur!, except: [:sign_up, :create_instructeur]
+    before_action :check_if_avis_revoked, only: [:show]
     before_action :redirect_if_no_sign_up_needed, only: [:sign_up]
     before_action :check_avis_exists_and_email_belongs_to_avis, only: [:sign_up, :create_instructeur]
     before_action :set_avis_and_dossier, only: [:show, :instruction, :messagerie, :create_commentaire, :update]
@@ -114,6 +115,14 @@ module Instructeurs
       end
     end
 
+    def revoquer
+      avis = Avis.find(params[:id])
+      if avis.revoke_by!(current_instructeur)
+        flash.notice = "#{avis.email_to_display} ne peut plus donner son avis sur ce dossier."
+        redirect_back(fallback_location: avis_instructeur_dossier_path(avis.procedure, avis.dossier))
+      end
+    end
+
     private
 
     def set_avis_and_dossier
@@ -135,6 +144,14 @@ module Instructeurs
       end
     end
 
+    def check_if_avis_revoked
+      avis = Avis.find(params[:id])
+      if avis.revoked?
+        flash.alert = "Vous n'avez plus accès à ce dossier."
+        redirect_to url_for(root_path)
+      end
+    end
+
     def check_avis_exists_and_email_belongs_to_avis
       if !Avis.avis_exists_and_email_belongs_to_avis?(params[:id], params[:email])
         redirect_to url_for(root_path)

app/models/avis.rb

@@ -56,6 +56,24 @@ class Avis < ApplicationRecord
     dossier.procedure
   end
 
+  def revoked?
+    revoked_at.present?
+  end
+
+  def revokable_by?(revocator)
+    revocator.dossiers.include?(dossier) || revocator == claimant
+  end
+
+  def revoke_by!(revocator)
+    return false if !revokable_by?(revocator)
+
+    if answer.present?
+      update!(revoked_at: Time.zone.now)
+    else
+      destroy!
+    end
+  end
+
   private
 
   def try_to_assign_instructeur

app/views/instructeurs/shared/avis/_list.html.haml

@@ -24,10 +24,19 @@
               %h2.instructeur
                 = (avis.email_to_display == current_instructeur.email) ? 'Vous' : avis.email_to_display
                 - if avis.answer.present?
+                  - if avis.revoked?
+                    %span.waiting{ class: highlight_if_unseen_class(avis_seen_at, avis.revoked_at) }
+                      Demande d'avis révoquée le #{l(avis.revoked_at, format: '%d/%m/%y à %H:%M')}
+                  - else
+                    - if avis.revokable_by?(current_instructeur)
+                      %span.waiting= link_to("Révoquer l'avis", revoquer_instructeur_avis_path(avis.procedure, avis), data: { confirm: "Souhaitez-vous révoquer la demande d'avis à #{avis.email_to_display} ?" }, method: :patch)
                   %span.date{ class: highlight_if_unseen_class(avis_seen_at, avis.updated_at) }
                     Réponse donnée le #{l(avis.updated_at, format: '%d/%m/%y à %H:%M')}
                 - else
-                  %span.waiting En attente de réponse
+                  %span.waiting
+                    En attente de réponse
+                    - if avis.revokable_by?(current_instructeur)
+                      = link_to("Révoquer l'avis", revoquer_instructeur_avis_path(avis.procedure, avis), data: { confirm: "Souhaitez-vous révoquer la demande d'avis à #{avis.email_to_display} ?" }, method: :patch)
               - if avis.piece_justificative_file.attached?
                 = render partial: 'shared/attachment/show', locals: { attachment: avis.piece_justificative_file.attachment }
               .answer-body

config/routes.rb

@@ -318,6 +318,7 @@ Rails.application.routes.draw do
             get 'messagerie'
             post 'commentaire' => 'avis#create_commentaire'
             post 'avis' => 'avis#create_avis'
+            patch 'revoquer'
             get 'bilans_bdf'
 
             get 'sign_up/email/:email' => 'avis#sign_up', constraints: { email: /.*/ }, as: 'sign_up'

db/migrate/20200715143010_add_revoked_at_to_avis.rb

@@ -0,0 +1,5 @@
+class AddRevokedAtToAvis < ActiveRecord::Migration[6.0]
+  def change
+    add_column :avis, :revoked_at, :datetime
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2020_07_07_082256) do
+ActiveRecord::Schema.define(version: 2020_07_15_143010) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -141,6 +141,7 @@ ActiveRecord::Schema.define(version: 2020_07_07_082256) do
     t.datetime "updated_at", null: false
     t.integer "claimant_id", null: false
     t.boolean "confidentiel", default: false, null: false
+    t.datetime "revoked_at"
     t.index ["claimant_id"], name: "index_avis_on_claimant_id"
     t.index ["dossier_id"], name: "index_avis_on_dossier_id"
     t.index ["instructeur_id"], name: "index_avis_on_instructeur_id"

spec/controllers/instructeurs/avis_controller_spec.rb

@@ -36,11 +36,24 @@ describe Instructeurs::AvisController, type: :controller do
     end
 
     describe '#show' do
-      before { get :show, params: { id: avis_without_answer.id, procedure_id: procedure.id } }
+      subject { get :show, params: { id: avis_with_answer.id, procedure_id: procedure.id } }
 
-      it { expect(response).to have_http_status(:success) }
+      context 'with a valid avis' do
-      it { expect(assigns(:avis)).to eq(avis_without_answer) }
+        before { subject }
-      it { expect(assigns(:dossier)).to eq(dossier) }
+
+        it { expect(response).to have_http_status(:success) }
+        it { expect(assigns(:avis)).to eq(avis_with_answer) }
+        it { expect(assigns(:dossier)).to eq(dossier) }
+      end
+
+      context 'with a revoked avis' do
+        it "refuse l'accès au dossier" do
+          avis_with_answer.update!(revoked_at: Time.zone.now)
+          subject
+          expect(flash.alert).to eq("Vous n'avez plus accès à ce dossier.")
+          expect(response).to redirect_to(root_path)
+        end
+      end
     end
 
     describe '#instruction' do
@@ -258,6 +271,17 @@ describe Instructeurs::AvisController, type: :controller do
         end
       end
     end
+
+    describe "#revoker" do
+      let(:avis) { create(:avis, claimant: instructeur) }
+      let(:procedure) { avis.procedure }
+
+      it "revoke the dossier" do
+        patch :revoquer, params: { procedure_id: procedure.id, id: avis.id }
+
+        expect(flash.notice).to eq("#{avis.email} ne peut plus donner son avis sur ce dossier.")
+      end
+    end
   end
 
   context 'without a instructeur signed in' do

spec/models/avis_spec.rb

@@ -131,4 +131,73 @@ RSpec.describe Avis, type: :model do
       it { expect(subject.email).to eq("toto@tps.fr") }
     end
   end
+
+  describe ".revoke_by!" do
+    let(:claimant) { create(:instructeur) }
+
+    context "when no answer" do
+      let(:avis) { create(:avis, claimant: claimant) }
+
+      it "supprime l'avis" do
+        avis.revoke_by!(claimant)
+        expect(avis).to be_destroyed
+        expect(Avis.count).to eq 0
+      end
+    end
+
+    context "with answer" do
+      let(:avis) { create(:avis, :with_answer, claimant: claimant) }
+
+      it "revoque l'avis" do
+        avis.revoke_by!(claimant)
+        expect(avis).not_to be_destroyed
+        expect(avis).to be_revoked
+      end
+    end
+
+    context "by an instructeur who can't revoke" do
+      let(:avis) { create(:avis, :with_answer, claimant: claimant) }
+      let(:expert) { create(:instructeur) }
+
+      it "doesn't revoke avis and returns false" do
+        result = avis.revoke_by!(expert)
+        expect(result).to be_falsey
+        expect(avis).not_to be_destroyed
+        expect(avis).not_to be_revoked
+      end
+    end
+  end
+
+  describe "revokable_by?" do
+    let(:instructeur) { create(:instructeur) }
+    let(:instructeurs) { [instructeur] }
+    let(:procedure) { create(:procedure, :published, instructeurs: instructeurs) }
+    let(:dossier) { create(:dossier, :en_instruction, procedure: procedure) }
+    let(:claimant_expert) { create(:instructeur) }
+    let(:expert) { create(:instructeur) }
+    let(:another_expert) { create(:instructeur) }
+
+    context "when avis claimed by an expert" do
+      let(:avis) { create(:avis, dossier: dossier, claimant: claimant_expert, instructeur: expert) }
+      let(:another_avis) { create(:avis, dossier: dossier, claimant: instructeur, instructeur: another_expert) }
+      it "is revokable by this expert or any instructeurs of the dossier" do
+        expect(avis.revokable_by?(claimant_expert)).to be_truthy
+        expect(avis.revokable_by?(another_expert)).to be_falsy
+        expect(avis.revokable_by?(instructeur)).to be_truthy
+      end
+    end
+
+    context "when avis claimed by an instructeur" do
+      let(:avis) { create(:avis, dossier: dossier, claimant: instructeur, instructeur: expert) }
+      let(:another_avis) { create(:avis, dossier: dossier, claimant: expert, instructeur: another_expert) }
+      let(:another_instructeur) { create(:instructeur) }
+      let(:instructeurs) { [instructeur, another_instructeur] }
+
+      it "is revokable by any instructeur of the dossier, not by an expert" do
+        expect(avis.revokable_by?(instructeur)).to be_truthy
+        expect(avis.revokable_by?(another_expert)).to be_falsy
+        expect(avis.revokable_by?(another_instructeur)).to be_truthy
+      end
+    end
+  end
 end