diff --git a/app/controllers/experts/avis_controller.rb b/app/controllers/experts/avis_controller.rb index 51e4fe255..2e964badd 100644 --- a/app/controllers/experts/avis_controller.rb +++ b/app/controllers/experts/avis_controller.rb @@ -4,7 +4,7 @@ module Experts include Zipline before_action :authenticate_expert!, except: [:sign_up, :update_expert] - before_action :check_if_avis_revoked, only: [:show] + before_action :check_if_avis_revoked, except: [:index, :procedure] 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, :delete_commentaire, :update, :telecharger_pjs] @@ -12,14 +12,28 @@ module Experts DONNES_STATUS = 'donnes' def index - avis = current_expert.avis.includes(dossier: [groupe_instructeur: :procedure]).not_hidden_by_administration + avis = current_expert.avis.not_revoked.includes(dossier: [groupe_instructeur: :procedure]).not_hidden_by_administration @avis_by_procedure = avis.to_a.group_by(&:procedure) end def procedure @procedure = current_expert.procedures.find_by(id: params[:procedure_id]) - redirect_to(expert_all_avis_path, flash: { alert: "Vous n’avez pas accès à cette démarche." }) and return unless @procedure - expert_avis = current_expert.avis.includes(:dossier).not_hidden_by_administration.where(dossiers: { groupe_instructeur: GroupeInstructeur.where(procedure: @procedure.id) }) + + if @procedure.nil? + redirect_to(expert_all_avis_path, flash: { alert: "Vous n’avez pas accès à cette démarche." }) and return + end + + expert_avis = current_expert + .avis + .not_revoked + .includes(:dossier) + .not_hidden_by_administration + .where(dossiers: { groupe_instructeur: GroupeInstructeur.where(procedure: @procedure) }) + + if expert_avis.empty? + redirect_to(expert_all_avis_path, flash: { alert: "Vous n’avez pas accès à cette démarche." }) and return + end + @avis_a_donner = expert_avis.without_answer @avis_donnes = expert_avis.with_answer diff --git a/app/models/avis.rb b/app/models/avis.rb index 4a49adb0e..15d49d894 100644 --- a/app/models/avis.rb +++ b/app/models/avis.rb @@ -53,6 +53,7 @@ class Avis < ApplicationRecord scope :termine_expired, -> { unscope(:joins).where(dossier: Dossier.termine_expired) } scope :en_construction_expired, -> { unscope(:joins).where(dossier: Dossier.en_construction_expired) } scope :not_hidden_by_administration, -> { where(dossiers: { hidden_by_administration_at: nil }) } + scope :not_revoked, -> { where(revoked_at: nil) } # The form allows subtmitting avis requests to several emails at once, # hence this virtual attribute. attr_accessor :emails diff --git a/spec/controllers/experts/avis_controller_spec.rb b/spec/controllers/experts/avis_controller_spec.rb index ce419259d..4502f3ccb 100644 --- a/spec/controllers/experts/avis_controller_spec.rb +++ b/spec/controllers/experts/avis_controller_spec.rb @@ -9,11 +9,19 @@ describe Experts::AvisController, type: :controller do let(:claimant) { create(:expert) } let(:expert) { create(:expert) } let(:procedure) { create(:procedure, :published, instructeurs: [instructeur, another_instructeur, instructeur_with_instant_avis_notification]) } + let(:procedure_id) { procedure.id } let(:another_procedure) { create(:procedure, :published, instructeurs: [instructeur]) } - let(:dossier) { create(:dossier, :en_construction, procedure: procedure) } - let(:experts_procedure) { create(:experts_procedure, expert: expert, procedure: procedure) } - let!(:avis_without_answer) { create(:avis, dossier: dossier, claimant: claimant, experts_procedure: experts_procedure) } - let!(:avis_with_answer) { create(:avis, dossier: dossier, claimant: claimant, experts_procedure: experts_procedure, answer: 'yop') } + let(:dossier) { create(:dossier, :en_construction, procedure:) } + let(:experts_procedure) { create(:experts_procedure, expert:, procedure:) } + let!(:avis_without_answer) { create(:avis, dossier:, claimant:, experts_procedure:) } + let!(:avis_with_answer) { create(:avis, dossier:, claimant:, experts_procedure:, answer: 'yop') } + + let!(:revoked_procedure) { create(:procedure, :published, instructeurs: [instructeur]) } + let!(:revoked_dossier) { create(:dossier, :en_construction, procedure: revoked_procedure) } + let!(:revoked_experts_procedure) { create(:experts_procedure, expert: expert, procedure: revoked_procedure) } + let!(:revoked_avis) do + create(:avis, dossier: revoked_dossier, claimant:, experts_procedure: revoked_experts_procedure, introduction: 'revoked', revoked_at: Time.zone.now) + end before do sign_in(expert.user) @@ -21,30 +29,44 @@ describe Experts::AvisController, type: :controller do describe '#index' do before { get :index } - it { expect(response).to have_http_status(:success) } - it { expect(assigns(:avis_by_procedure).flatten).to include(procedure) } - it { expect(assigns(:avis_by_procedure).flatten).not_to include(another_procedure) } + it do + expect(response).to have_http_status(:success) + expect(assigns(:avis_by_procedure).keys).to match_array(procedure) + expect(assigns(:avis_by_procedure).values.flatten).to match_array([avis_without_answer, avis_with_answer]) + end end describe '#procedure' do context 'without filter' do let!(:oldest_avis_without_answer) { create(:avis, dossier: dossier, claimant: claimant, experts_procedure: experts_procedure, created_at: 2.years.ago) } - before { get :procedure, params: { procedure_id: procedure.id } } + before { get :procedure, params: { procedure_id: procedure_to_show.id } } - it { expect(response).to have_http_status(:success) } - it { expect(assigns(:avis_a_donner)).to match([avis_without_answer, oldest_avis_without_answer]) } - it { expect(assigns(:avis_donnes)).to match([avis_with_answer]) } - it { expect(assigns(:statut)).to eq('a-donner') } + context 'with legitimates avis' do + let(:procedure_to_show) { procedure } + + it do + expect(response).to have_http_status(:success) + expect(assigns(:avis_a_donner)).to match([avis_without_answer, oldest_avis_without_answer]) + expect(assigns(:avis_donnes)).to match([avis_with_answer]) + expect(assigns(:statut)).to eq('a-donner') + end + end + + context 'with a revoked avis' do + let(:procedure_to_show) { revoked_procedure } + + it { expect(response).to redirect_to expert_all_avis_path } + end end context 'with a statut equal to donnes' do - before { get :procedure, params: { statut: 'donnes', procedure_id: procedure.id } } + before { get :procedure, params: { statut: 'donnes', procedure_id: } } it { expect(assigns(:statut)).to eq('donnes') } end context 'with different procedure' do - subject { get :procedure, params: { statut: 'donnes', procedure_id: procedure.id } } + subject { get :procedure, params: { statut: 'donnes', procedure_id: } } it 'fails' do sign_in(create(:expert).user) @@ -56,20 +78,52 @@ describe Experts::AvisController, type: :controller do end describe '#bilans_bdf' do - before { get :bilans_bdf, params: { id: avis_without_answer.id, procedure_id: procedure.id } } + let(:avis) { avis_without_answer } + + before { get :bilans_bdf, params: { id: avis, procedure_id: } } it { expect(response).to redirect_to(instructeur_avis_path(avis_without_answer)) } + + context 'with a revoked avis' do + let(:avis) { revoked_avis } + + it { expect(response).to redirect_to(root_path) } + end + end + + describe '#telecharger_pjs' do + let(:avis) { avis_with_answer } + + subject { get :telecharger_pjs, params: { id: avis.id, procedure_id: } } + + context 'with a valid avis' do + it { is_expected.to have_http_status(:success) } + end + + context 'with a revoked avis' do + let(:avis) { revoked_avis } + + it { is_expected.to redirect_to(root_path) } + end + + context 'with a another avis' do + let(:avis) { create(:avis) } + + it { is_expected.to redirect_to(expert_all_avis_path) } + end end describe '#show' do - subject { get :show, params: { id: avis_with_answer.id, procedure_id: procedure.id } } + subject { get :show, params: { id: avis_with_answer.id, procedure_id: } } 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) } + it do + expect(response).to have_http_status(:success) + expect(assigns(:avis)).to eq(avis_with_answer) + expect(assigns(:dossier)).to eq(dossier) + end end context 'with a revoked avis' do @@ -92,14 +146,22 @@ describe Experts::AvisController, type: :controller do end describe '#instruction' do - subject { get :instruction, params: { id: avis_without_answer.id, procedure_id: procedure.id } } + subject { get :instruction, params: { id: avis_to_instruct.id, procedure_id: } } + context 'with valid avis' do + let(:avis_to_instruct) { avis_without_answer } before { subject } - it { expect(response).to have_http_status(:success) } - it { expect(assigns(:avis)).to eq(avis_without_answer) } - it { expect(assigns(:dossier)).to eq(dossier) } + + it do + expect(response).to have_http_status(:success) + expect(assigns(:avis)).to eq(avis_without_answer) + expect(assigns(:dossier)).to eq(dossier) + end end + context 'with an avis that does not belongs to current_expert' do + let(:avis_to_instruct) { avis_without_answer } + it "refuse l'accès au dossier" do sign_in(create(:expert).user) subject @@ -107,6 +169,13 @@ describe Experts::AvisController, type: :controller do expect(flash.alert).to eq("Vous n’avez pas accès à cet avis.") end end + + context 'with a revoked avis' do + let(:avis_to_instruct) { revoked_avis } + before { subject } + + it { expect(response).to redirect_to root_path } + end end context 'with destroyed claimant' do @@ -115,20 +184,25 @@ describe Experts::AvisController, type: :controller do avis_with_merged_instructeur = create(:avis, dossier: dossier, claimant: another_instructeur, experts_procedure: experts_procedure) another_instructeur.user.destroy sign_in(expert.user) - get :instruction, params: { id: avis_with_merged_instructeur.id, procedure_id: procedure.id } + get :instruction, params: { id: avis_with_merged_instructeur.id, procedure_id: } expect(response).to have_http_status(200) end end describe '#messagerie' do - subject { get :messagerie, params: { id: avis_without_answer.id, procedure_id: procedure.id } } + let(:avis) { avis_without_answer } + subject { get :messagerie, params: { id: avis.id, procedure_id: } } + context 'with valid avis' do before { subject } - it { expect(response).to have_http_status(:success) } - it { expect(assigns(:avis)).to eq(avis_without_answer) } - it { expect(assigns(:dossier)).to eq(dossier) } + it do + expect(response).to have_http_status(:success) + expect(assigns(:avis)).to eq(avis_without_answer) + expect(assigns(:dossier)).to eq(dossier) + end end + context 'with an avis that does not belongs to current_expert' do it "refuse l'accès au dossier" do sign_in(create(:expert).user) @@ -137,21 +211,36 @@ describe Experts::AvisController, type: :controller do expect(flash.alert).to eq("Vous n’avez pas accès à cet avis.") end end + + context 'with a revoked avis' do + let(:avis) { revoked_avis } + + it { is_expected.to redirect_to(root_path) } + end end describe '#update' do - context 'without attachment' do - before do - Timecop.freeze(now) - patch :update, params: { id: avis_without_answer.id, procedure_id: procedure.id, avis: { answer: 'answer' } } - avis_without_answer.reload - end - after { Timecop.return } + before { Timecop.freeze(now) } + after { Timecop.return } + let(:avis) { avis_without_answer } + + subject do + post :update, params: { id: avis.id, procedure_id:, avis: { answer: 'answer' } } + avis.reload + end + + context 'on a revoked avis' do + let(:avis) { revoked_avis } + + it { expect(subject).to redirect_to(root_path) } + end + + context 'without attachment' do it 'should be ok' do - expect(response).to redirect_to(instruction_expert_avis_path(avis_without_answer.procedure, avis_without_answer)) - expect(avis_without_answer.answer).to eq('answer') - expect(avis_without_answer.piece_justificative_file).to_not be_attached + expect(subject).to redirect_to(instruction_expert_avis_path(avis.procedure, avis)) + expect(avis.answer).to eq('answer') + expect(avis.piece_justificative_file).to_not be_attached expect(dossier.reload.last_avis_updated_at).to eq(now) expect(flash.notice).to eq('Votre réponse est enregistrée.') end @@ -161,28 +250,9 @@ describe Experts::AvisController, type: :controller do before do allow(DossierMailer).to receive(:notify_new_avis_to_instructeur).and_return(double(deliver_later: nil)) AssignTo.find_by(instructeur: instructeur_with_instant_avis_notification).update!(instant_expert_avis_email_notifications_enabled: true) - Timecop.freeze(now) instructeur_with_instant_avis_notification.follow(avis_without_answer.dossier) - patch :update, params: { id: avis_without_answer.id, procedure_id: procedure.id, avis: { answer: 'answer' } } - avis_without_answer.reload + subject end - after { Timecop.return } - - it 'The instructeur should be notified of the new avis' do - expect(DossierMailer).to have_received(:notify_new_avis_to_instructeur).once.with(avis_without_answer, instructeur_with_instant_avis_notification.email) - end - end - - context 'without attachment with an instructeur wants to be notified' do - before do - allow(DossierMailer).to receive(:notify_new_avis_to_instructeur).and_return(double(deliver_later: nil)) - AssignTo.find_by(instructeur: instructeur_with_instant_avis_notification).update!(instant_expert_avis_email_notifications_enabled: true) - Timecop.freeze(now) - instructeur_with_instant_avis_notification.follow(avis_without_answer.dossier) - patch :update, params: { id: avis_without_answer.id, procedure_id: procedure.id, avis: { answer: 'answer' } } - avis_without_answer.reload - end - after { Timecop.return } it 'The instructeur should be notified of the new avis' do expect(DossierMailer).to have_received(:notify_new_avis_to_instructeur).once.with(avis_without_answer, instructeur_with_instant_avis_notification.email) @@ -190,12 +260,11 @@ describe Experts::AvisController, type: :controller do end context 'with attachment' do - include ActiveJob::TestHelper let(:file) { fixture_file_upload('spec/fixtures/files/piece_justificative_0.pdf', 'application/pdf') } before do expect(ClamavService).to receive(:safe_file?).and_return(true) - post :update, params: { id: avis_without_answer.id, procedure_id: procedure.id, avis: { answer: 'answer', piece_justificative_file: file } } + post :update, params: { id: avis_without_answer.id, procedure_id:, avis: { answer: 'answer', piece_justificative_file: file } } perform_enqueued_jobs avis_without_answer.reload end @@ -207,11 +276,12 @@ describe Experts::AvisController, type: :controller do expect(flash.notice).to eq('Votre réponse est enregistrée.') end end + context 'with an avis that does not belongs to current_expert' do + before { sign_in(create(:expert).user) } + it "refuse l'accès au dossier" do - sign_in(create(:expert).user) - patch :update, params: { id: avis_without_answer.id, procedure_id: procedure.id, avis: { answer: 'answer' } } - expect(response).to redirect_to(expert_all_avis_path) + expect(subject).to redirect_to(expert_all_avis_path) expect(flash.alert).to eq("Vous n’avez pas accès à cet avis.") end end @@ -221,8 +291,9 @@ describe Experts::AvisController, type: :controller do let(:file) { nil } let(:scan_result) { true } let(:now) { Time.zone.parse("14/07/1789") } + let(:avis) { avis_without_answer } - subject { post :create_commentaire, params: { id: avis_without_answer.id, procedure_id: procedure.id, commentaire: { body: 'commentaire body', piece_jointe: file } } } + subject { post :create_commentaire, params: { id: avis.id, procedure_id:, commentaire: { body: 'commentaire body', piece_jointe: file } } } before do allow(ClamavService).to receive(:safe_file?).and_return(scan_result) @@ -243,11 +314,15 @@ describe Experts::AvisController, type: :controller do let(:file) { fixture_file_upload('spec/fixtures/files/piece_justificative_0.pdf', 'application/pdf') } it do - subject + expect { subject }.to change(Commentaire, :count).by(1) expect(Commentaire.last.piece_jointe.filename).to eq("piece_justificative_0.pdf") end + end - it { expect { subject }.to change(Commentaire, :count).by(1) } + context 'with a revoked avis' do + let(:avis) { revoked_avis } + + it { is_expected.to redirect_to(root_path) } end end @@ -272,70 +347,80 @@ describe Experts::AvisController, type: :controller do end describe '#create_avis' do - let!(:previous_avis) { create(:avis, dossier: dossier, claimant: claimant, experts_procedure: experts_procedure, confidentiel: previous_avis_confidentiel) } - let(:emails) { "[\"a@b.com\"]" } - let(:intro) { 'introduction' } + let(:previous_avis_confidentiel) { false } + let(:previous_revoked_at) { nil } + let!(:previous_avis) { create(:avis, dossier:, claimant:, experts_procedure:, confidentiel: previous_avis_confidentiel, revoked_at: previous_revoked_at) } + let(:emails) { '["a@b.com"]' } + let(:introduction) { 'introduction' } let(:created_avis) { Avis.last } let!(:old_avis_count) { Avis.count } let(:invite_linked_dossiers) { nil } + let(:introduction_file) { fixture_file_upload('spec/fixtures/files/piece_justificative_0.pdf', 'application/pdf') } + let(:confidentiel) { false } before do Timecop.freeze(now) - @introduction_file = fixture_file_upload('spec/fixtures/files/piece_justificative_0.pdf', 'application/pdf') - post :create_avis, params: { id: previous_avis.id, procedure_id: procedure.id, avis: { emails: emails, introduction: intro, experts_procedure: experts_procedure, confidentiel: asked_confidentiel, invite_linked_dossiers: invite_linked_dossiers, introduction_file: @introduction_file } } + post :create_avis, params: { id: previous_avis.id, procedure_id:, avis: { emails:, introduction:, experts_procedure:, confidentiel:, invite_linked_dossiers:, introduction_file: } } created_avis.reload end after { Timecop.return } + context 'from a revoked avis' do + let(:previous_revoked_at) { Time.zone.now } + + it do + expect(response).to redirect_to(root_path) + expect(Avis.last).to eq(previous_avis) + end + end + context 'when an invalid email' do - let(:previous_avis_confidentiel) { false } - let(:asked_confidentiel) { false } let(:emails) { "[\"toto.fr\"]" } - it { expect(response).to render_template :instruction } - it { expect(flash.alert).to eq(["toto.fr : Email n'est pas valide"]) } - it { expect(Avis.last).to eq(previous_avis) } - it { expect(dossier.last_avis_updated_at).to eq(nil) } + it do + expect(response).to render_template :instruction + expect(flash.alert).to eq(["toto.fr : Email n'est pas valide"]) + expect(Avis.last).to eq(previous_avis) + expect(dossier.last_avis_updated_at).to eq(nil) + end end context 'ask review with attachment' do - let(:previous_avis_confidentiel) { false } - let(:asked_confidentiel) { false } let(:emails) { "[\"toto@totomail.com\"]" } - it { expect(created_avis.introduction_file).to be_attached } - it { expect(created_avis.introduction_file.filename).to eq("piece_justificative_0.pdf") } - it { expect(created_avis.dossier.reload.last_avis_updated_at).to eq(now) } - it { expect(flash.notice).to eq("Une demande d’avis a été envoyée à toto@totomail.com") } + it do + expect(created_avis.introduction_file).to be_attached + expect(created_avis.introduction_file.filename).to eq("piece_justificative_0.pdf") + expect(created_avis.dossier.reload.last_avis_updated_at).to eq(now) + expect(flash.notice).to eq("Une demande d’avis a été envoyée à toto@totomail.com") + end end context 'with multiple emails' do - let(:asked_confidentiel) { false } - let(:previous_avis_confidentiel) { false } let(:emails) { "[\"toto.fr\",\"titi@titimail.com\"]" } - it { expect(response).to render_template :instruction } - it { expect(flash.alert).to eq(["toto.fr : Email n'est pas valide"]) } - it { expect(flash.notice).to eq("Une demande d’avis a été envoyée à titi@titimail.com") } - it { expect(Avis.count).to eq(old_avis_count + 1) } + it do + expect(response).to render_template :instruction + expect(flash.alert).to eq(["toto.fr : Email n'est pas valide"]) + expect(flash.notice).to eq("Une demande d’avis a été envoyée à titi@titimail.com") + expect(Avis.count).to eq(old_avis_count + 1) + end end context 'when the previous avis is public' do - let(:previous_avis_confidentiel) { false } - context 'when the user asked for a public avis' do - let(:asked_confidentiel) { false } - - it { expect(created_avis.confidentiel).to be(false) } - it { expect(created_avis.introduction).to eq(intro) } - it { expect(created_avis.dossier).to eq(previous_avis.dossier) } - it { expect(created_avis.claimant).to eq(expert) } - it { expect(response).to redirect_to(instruction_expert_avis_path(previous_avis.procedure, previous_avis)) } + it do + expect(created_avis.confidentiel).to be(false) + expect(created_avis.introduction).to eq(introduction) + expect(created_avis.dossier).to eq(previous_avis.dossier) + expect(created_avis.claimant).to eq(expert) + expect(response).to redirect_to(instruction_expert_avis_path(previous_avis.procedure, previous_avis)) + end end context 'when the user asked for a confidentiel avis' do - let(:asked_confidentiel) { true } + let(:confidentiel) { true } it { expect(created_avis.confidentiel).to be(true) } end @@ -345,15 +430,13 @@ describe Experts::AvisController, type: :controller do let(:previous_avis_confidentiel) { true } context 'when the user asked for a public avis' do - let(:asked_confidentiel) { false } + let(:confidentiel) { false } it { expect(created_avis.confidentiel).to be(true) } end end context 'with linked dossiers' do - let(:asked_confidentiel) { false } - let(:previous_avis_confidentiel) { false } let(:dossier) { create(:dossier, :en_construction, :with_dossier_link, procedure: procedure) } context 'when the expert doesn’t share linked dossiers' do @@ -403,10 +486,17 @@ describe Experts::AvisController, type: :controller do let(:dossier) { create(:dossier) } let(:avis) { create(:avis, dossier: dossier, experts_procedure: experts_procedure, claimant: claimant) } let(:procedure) { dossier.procedure } + let(:procedure_id) { procedure.id } describe '#sign_up' do subject do - get :sign_up, params: { id: avis.id, procedure_id: procedure.id, email: avis.expert.email } + get :sign_up, params: { id: avis.id, procedure_id:, email: avis.expert.email } + end + + context 'when the avis is revoked' do + before { avis.update(revoked_at: Time.zone.now) } + + it { is_expected.to redirect_to(root_path) } end context 'when the expert hasn’t signed up yet' do @@ -446,7 +536,7 @@ describe Experts::AvisController, type: :controller do subject do post :update_expert, params: { id: avis.id, - procedure_id: procedure.id, + procedure_id:, email: avis.expert.email, user: { password: 'my-s3cure-p4ssword' @@ -454,6 +544,12 @@ describe Experts::AvisController, type: :controller do } end + context 'when the avis is revoked' do + before { avis.update(revoked_at: Time.zone.now) } + + it { is_expected.to redirect_to(root_path) } + end + context 'when the expert hasn’t signed up yet' do before { expert.user.update(last_sign_in_at: nil) }