From 3ea34834a74ed32864fdf6f32dc18dedf6531d3f Mon Sep 17 00:00:00 2001 From: Christophe Robillard Date: Thu, 16 Jul 2020 11:14:37 +0200 Subject: [PATCH 1/2] revoke expert avis --- .../instructeurs/avis_controller.rb | 17 ++++++++++ app/models/avis.rb | 12 +++++++ .../instructeurs/shared/avis/_list.html.haml | 9 +++++- config/routes.rb | 1 + .../20200715143010_add_revoked_at_to_avis.rb | 5 +++ db/schema.rb | 3 +- .../instructeurs/avis_controller_spec.rb | 32 ++++++++++++++++--- spec/models/avis_spec.rb | 22 +++++++++++++ 8 files changed, 95 insertions(+), 6 deletions(-) create mode 100644 db/migrate/20200715143010_add_revoked_at_to_avis.rb diff --git a/app/controllers/instructeurs/avis_controller.rb b/app/controllers/instructeurs/avis_controller.rb index 6382da31d..aebcc31df 100644 --- a/app/controllers/instructeurs/avis_controller.rb +++ b/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! + 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) diff --git a/app/models/avis.rb b/app/models/avis.rb index 7deb2cd6e..2374ef365 100644 --- a/app/models/avis.rb +++ b/app/models/avis.rb @@ -56,6 +56,18 @@ class Avis < ApplicationRecord dossier.procedure end + def revoked? + revoked_at.present? + end + + def revoke! + if answer.present? + update!(revoked_at: Time.zone.now) + else + destroy! + end + end + private def try_to_assign_instructeur diff --git a/app/views/instructeurs/shared/avis/_list.html.haml b/app/views/instructeurs/shared/avis/_list.html.haml index d86914cc1..6b5a30e07 100644 --- a/app/views/instructeurs/shared/avis/_list.html.haml +++ b/app/views/instructeurs/shared/avis/_list.html.haml @@ -24,10 +24,17 @@ %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 + %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 + = 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 diff --git a/config/routes.rb b/config/routes.rb index e781d39f2..560f5e11f 100644 --- a/config/routes.rb +++ b/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' diff --git a/db/migrate/20200715143010_add_revoked_at_to_avis.rb b/db/migrate/20200715143010_add_revoked_at_to_avis.rb new file mode 100644 index 000000000..675f8510e --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 5e8ef8db9..8954f1d5e 100644 --- a/db/schema.rb +++ b/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" diff --git a/spec/controllers/instructeurs/avis_controller_spec.rb b/spec/controllers/instructeurs/avis_controller_spec.rb index 00d1c3b55..cfdd85011 100644 --- a/spec/controllers/instructeurs/avis_controller_spec.rb +++ b/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) } - it { expect(assigns(:avis)).to eq(avis_without_answer) } - it { expect(assigns(:dossier)).to eq(dossier) } + context 'with a valid avis' do + before { subject } + + 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.revoke! + 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) } + 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 diff --git a/spec/models/avis_spec.rb b/spec/models/avis_spec.rb index aabe056a2..7c2b75ee5 100644 --- a/spec/models/avis_spec.rb +++ b/spec/models/avis_spec.rb @@ -131,4 +131,26 @@ RSpec.describe Avis, type: :model do it { expect(subject.email).to eq("toto@tps.fr") } end end + + describe ".revoke!" do + context "when no answer" do + let(:avis) { create(:avis) } + + it "supprime l'avis" do + avis.revoke! + expect(avis).to be_destroyed + expect(Avis.count).to eq 0 + end + end + + context "with answer" do + let(:avis) { create(:avis, :with_answer) } + + it "revoque l'avis" do + avis.revoke! + expect(avis).not_to be_destroyed + expect(avis).to be_revoked + end + end + end end From 52e8f32e19d0e56344b4caf59781edd8b7282224 Mon Sep 17 00:00:00 2001 From: Christophe Robillard Date: Thu, 16 Jul 2020 20:42:50 +0200 Subject: [PATCH 2/2] expert can only revoke avis claimed by him/her --- .../instructeurs/avis_controller.rb | 2 +- app/models/avis.rb | 8 ++- .../instructeurs/shared/avis/_list.html.haml | 6 +- .../instructeurs/avis_controller_spec.rb | 4 +- spec/models/avis_spec.rb | 57 +++++++++++++++++-- 5 files changed, 66 insertions(+), 11 deletions(-) diff --git a/app/controllers/instructeurs/avis_controller.rb b/app/controllers/instructeurs/avis_controller.rb index aebcc31df..322734006 100644 --- a/app/controllers/instructeurs/avis_controller.rb +++ b/app/controllers/instructeurs/avis_controller.rb @@ -117,7 +117,7 @@ module Instructeurs def revoquer avis = Avis.find(params[:id]) - if avis.revoke! + 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 diff --git a/app/models/avis.rb b/app/models/avis.rb index 2374ef365..3834469a0 100644 --- a/app/models/avis.rb +++ b/app/models/avis.rb @@ -60,7 +60,13 @@ class Avis < ApplicationRecord revoked_at.present? end - def revoke! + 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 diff --git a/app/views/instructeurs/shared/avis/_list.html.haml b/app/views/instructeurs/shared/avis/_list.html.haml index 6b5a30e07..72016270e 100644 --- a/app/views/instructeurs/shared/avis/_list.html.haml +++ b/app/views/instructeurs/shared/avis/_list.html.haml @@ -28,13 +28,15 @@ %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 - %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) + - 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 - = 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.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 diff --git a/spec/controllers/instructeurs/avis_controller_spec.rb b/spec/controllers/instructeurs/avis_controller_spec.rb index cfdd85011..0e73fc559 100644 --- a/spec/controllers/instructeurs/avis_controller_spec.rb +++ b/spec/controllers/instructeurs/avis_controller_spec.rb @@ -48,7 +48,7 @@ describe Instructeurs::AvisController, type: :controller do context 'with a revoked avis' do it "refuse l'accès au dossier" do - avis_with_answer.revoke! + 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) @@ -273,7 +273,7 @@ describe Instructeurs::AvisController, type: :controller do end describe "#revoker" do - let(:avis) { create(:avis) } + let(:avis) { create(:avis, claimant: instructeur) } let(:procedure) { avis.procedure } it "revoke the dossier" do diff --git a/spec/models/avis_spec.rb b/spec/models/avis_spec.rb index 7c2b75ee5..bd6cd64c5 100644 --- a/spec/models/avis_spec.rb +++ b/spec/models/avis_spec.rb @@ -132,25 +132,72 @@ RSpec.describe Avis, type: :model do end end - describe ".revoke!" do + describe ".revoke_by!" do + let(:claimant) { create(:instructeur) } + context "when no answer" do - let(:avis) { create(:avis) } + let(:avis) { create(:avis, claimant: claimant) } it "supprime l'avis" do - avis.revoke! + 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) } + let(:avis) { create(:avis, :with_answer, claimant: claimant) } it "revoque l'avis" do - avis.revoke! + 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