From 60cb9bcf09b00cd40709e324ed1fe7f0bad0ea5b Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 31 Mar 2020 14:33:30 +0200 Subject: [PATCH 01/14] Remove brouillons deleted dossiers --- app/controllers/instructeurs/procedures_controller.rb | 2 +- .../20200326133630_cleanup_deleted_dossiers.rake | 10 ++++++++++ .../instructeurs/procedures_controller_spec.rb | 1 - 3 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 lib/tasks/deployment/20200326133630_cleanup_deleted_dossiers.rake diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index 4fce78b3f..7d02e0069 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -116,7 +116,7 @@ module Instructeurs def deleted_dossiers @procedure = procedure @deleted_dossiers = @procedure - .deleted_dossiers.where.not(state: :brouillon) + .deleted_dossiers .order(:dossier_id) .page params[:page] end diff --git a/lib/tasks/deployment/20200326133630_cleanup_deleted_dossiers.rake b/lib/tasks/deployment/20200326133630_cleanup_deleted_dossiers.rake new file mode 100644 index 000000000..64bdb9bdb --- /dev/null +++ b/lib/tasks/deployment/20200326133630_cleanup_deleted_dossiers.rake @@ -0,0 +1,10 @@ +namespace :after_party do + desc 'Deployment task: cleanup_deleted_dossiers' + task cleanup_deleted_dossiers: :environment do + puts "Running deploy task 'cleanup_deleted_dossiers'" + + DeletedDossier.where(state: :brouillon).destroy_all + + AfterParty::TaskRecord.create version: '20200326133630' + end +end diff --git a/spec/controllers/instructeurs/procedures_controller_spec.rb b/spec/controllers/instructeurs/procedures_controller_spec.rb index a3965bca6..75c75edae 100644 --- a/spec/controllers/instructeurs/procedures_controller_spec.rb +++ b/spec/controllers/instructeurs/procedures_controller_spec.rb @@ -413,7 +413,6 @@ describe Instructeurs::ProceduresController, type: :controller do let(:instructeur) { create(:instructeur) } let(:procedure) { create(:procedure, instructeurs: [instructeur]) } let(:deleted_dossier) { create(:deleted_dossier, procedure: procedure, state: :en_construction) } - let!(:deleted_dossier_brouillon) { create(:deleted_dossier, procedure: procedure, state: :brouillon) } before do sign_in(instructeur.user) From aa066bc20a63f08da02a9938f67fcf349c376902 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 31 Mar 2020 14:33:54 +0200 Subject: [PATCH 02/14] Validate deleted dossiers dossier_id uniqueness --- app/models/deleted_dossier.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/models/deleted_dossier.rb b/app/models/deleted_dossier.rb index bce5518ee..0538f09f1 100644 --- a/app/models/deleted_dossier.rb +++ b/app/models/deleted_dossier.rb @@ -1,6 +1,8 @@ class DeletedDossier < ApplicationRecord belongs_to :procedure + validates :dossier_id, uniqueness: true + enum reason: { user_request: 'user_request', manager_request: 'manager_request', From 876e05aed38f0da38d1d11b11efe97e9c94ef664 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 26 Mar 2020 09:08:52 +0100 Subject: [PATCH 03/14] Discard demarches from manager --- .../manager/procedures_controller.rb | 15 +++++++++------ app/models/deleted_dossier.rb | 9 +++++---- app/models/procedure.rb | 11 +++++++++-- app/views/manager/procedures/show.html.erb | 7 +++++-- config/locales/models/deleted_dossier/fr.yml | 2 +- config/routes.rb | 2 +- .../manager/procedures_controller_spec.rb | 17 +++++++++++++++-- spec/factories/procedure.rb | 6 +----- spec/models/dossier_spec.rb | 6 ++++-- spec/models/procedure_spec.rb | 7 ++++--- 10 files changed, 54 insertions(+), 28 deletions(-) diff --git a/app/controllers/manager/procedures_controller.rb b/app/controllers/manager/procedures_controller.rb index 0d05bed76..810d0a1a9 100644 --- a/app/controllers/manager/procedures_controller.rb +++ b/app/controllers/manager/procedures_controller.rb @@ -8,7 +8,7 @@ module Manager # this will be used to set the records shown on the `index` action. def scoped_resource if unfiltered_list? - # Don't display deleted dossiers in the unfiltered list… + # Don't display discarded demarches in the unfiltered list… Procedure.kept else # … but allow them to be searched and displayed. @@ -22,10 +22,13 @@ module Manager redirect_to manager_procedure_path(procedure) end - def hide - procedure.hide! - flash[:notice] = "La démarche a bien été supprimée, en cas d'erreur contactez un développeur." - redirect_to manager_procedures_path + def discard + procedure.discard_and_keep_track!(current_administration) + + logger.info("La démarche #{procedure.id} est supprimée par #{current_administration.email}") + flash[:notice] = "La démarche #{procedure.id} a été supprimée." + + redirect_to manager_procedure_path(procedure) end def add_administrateur @@ -51,7 +54,7 @@ module Manager private def procedure - Procedure.find(params[:id]) + @procedure ||= Procedure.with_discarded.find(params[:id]) end def type_de_champ diff --git a/app/models/deleted_dossier.rb b/app/models/deleted_dossier.rb index 0538f09f1..0a0ebc9a8 100644 --- a/app/models/deleted_dossier.rb +++ b/app/models/deleted_dossier.rb @@ -4,10 +4,11 @@ class DeletedDossier < ApplicationRecord validates :dossier_id, uniqueness: true enum reason: { - user_request: 'user_request', - manager_request: 'manager_request', - user_removed: 'user_removed', - expired: 'expired' + user_request: 'user_request', + manager_request: 'manager_request', + user_removed: 'user_removed', + procedure_removed: 'procedure_removed', + expired: 'expired' } def self.create_from_dossier(dossier, reason) diff --git a/app/models/procedure.rb b/app/models/procedure.rb index 3c3a72fca..b212b7ca9 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -511,9 +511,16 @@ class Procedure < ApplicationRecord groupe_instructeurs.count > 1 end - def hide! + def can_be_deleted_by_manager? + kept? && dossiers.state_instruction_commencee.empty? + end + + def discard_and_keep_track!(author) + dossiers.each do |dossier| + dossier.discard_and_keep_track!(author, :procedure_removed) + end + discard! - dossiers.discard_all end def flipper_id diff --git a/app/views/manager/procedures/show.html.erb b/app/views/manager/procedures/show.html.erb index dd020bb5f..ede594883 100644 --- a/app/views/manager/procedures/show.html.erb +++ b/app/views/manager/procedures/show.html.erb @@ -22,6 +22,9 @@ as well as a link to its edit page. diff --git a/app/views/manager/procedures/show.html.erb b/app/views/manager/procedures/show.html.erb index ede594883..faa058874 100644 --- a/app/views/manager/procedures/show.html.erb +++ b/app/views/manager/procedures/show.html.erb @@ -44,6 +44,8 @@ as well as a link to its edit page. <% if procedure.can_be_deleted_by_manager? %> <%= link_to 'Supprimer la démarche', discard_manager_procedure_path(procedure), method: :post, class: 'button', data: { confirm: "Confirmez-vous la suppression de la démarche ?" } %> + <% elsif procedure.discarded? %> + <%= link_to 'Restaurer la démarche', restore_manager_procedure_path(procedure), method: :post, class: 'button', data: { confirm: "Confirmez-vous la restauration de la démarche ?" } %> <% end %>
diff --git a/config/routes.rb b/config/routes.rb index 3e5412e6c..273880d2f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -10,12 +10,14 @@ Rails.application.routes.draw do post 'whitelist', on: :member post 'draft', on: :member post 'discard', on: :member + post 'restore', on: :member post 'add_administrateur', on: :member post 'change_piece_justificative_template', on: :member end resources :dossiers, only: [:index, :show] do post 'discard', on: :member + post 'restore', on: :member post 'repasser_en_instruction', on: :member end diff --git a/spec/controllers/manager/dossiers_controller_spec.rb b/spec/controllers/manager/dossiers_controller_spec.rb index c64100a22..3b1969fc5 100644 --- a/spec/controllers/manager/dossiers_controller_spec.rb +++ b/spec/controllers/manager/dossiers_controller_spec.rb @@ -1,23 +1,43 @@ describe Manager::DossiersController, type: :controller do + let(:administration) { create :administration } + let(:deleted_dossier) { DeletedDossier.find_by(dossier_id: dossier) } + let(:operations) { dossier.dossier_operation_logs.map(&:operation).map(&:to_sym) } + + before { sign_in administration } + describe '#discard' do - let(:administration) { create :administration } - let(:dossier) { create(:dossier) } + let(:dossier) { create(:dossier, :en_construction) } before do - sign_in administration post :discard, params: { id: dossier.id } dossier.reload end it { expect(dossier.discarded?).to be_truthy } + it { expect(deleted_dossier).not_to be_nil } + it { expect(deleted_dossier.reason).to eq("manager_request") } + it { expect(operations).to eq([:supprimer]) } + end + + describe '#restore' do + let(:dossier) { create(:dossier, :en_construction) } + + before do + dossier.discard_and_keep_track!(administration, :manager_request) + + post :restore, params: { id: dossier.id } + dossier.reload + end + + it { expect(dossier.kept?).to be_truthy } + it { expect(deleted_dossier).to be_nil } + it { expect(operations).to eq([:supprimer, :restaurer]) } end describe '#repasser_en_instruction' do - let(:administration) { create :administration } let(:dossier) { create(:dossier, :accepte) } before do - sign_in administration post :repasser_en_instruction, params: { id: dossier.id } dossier.reload end diff --git a/spec/controllers/manager/procedures_controller_spec.rb b/spec/controllers/manager/procedures_controller_spec.rb index 0e98c2660..b8bebd461 100644 --- a/spec/controllers/manager/procedures_controller_spec.rb +++ b/spec/controllers/manager/procedures_controller_spec.rb @@ -1,25 +1,25 @@ describe Manager::ProceduresController, type: :controller do + let(:administration) { create :administration } + + before { sign_in administration } + describe '#whitelist' do - let(:administration) { create :administration } let(:procedure) { create(:procedure) } before do - sign_in administration post :whitelist, params: { id: procedure.id } procedure.reload end - it { expect(procedure.whitelisted_at).not_to be_nil } + it { expect(procedure.whitelisted?).to be_truthy } end describe '#show' do render_views - let(:administration) { create(:administration) } let(:procedure) { create(:procedure, :with_repetition) } before do - sign_in(administration) get :show, params: { id: procedure.id } end @@ -27,27 +27,50 @@ describe Manager::ProceduresController, type: :controller do end describe '#discard' do - let(:administration) { create :administration } - let(:procedure) { create(:procedure) } + let(:dossier) { create(:dossier, :en_construction) } + let(:procedure) { dossier.procedure } + let(:deleted_dossier) { DeletedDossier.find_by(dossier_id: dossier.id) } + let(:operations) { dossier.dossier_operation_logs.map(&:operation).map(&:to_sym) } before do - sign_in administration post :discard, params: { id: procedure.id } procedure.reload + dossier.reload end it { expect(procedure.discarded?).to be_truthy } + it { expect(dossier.discarded?).to be_truthy } + it { expect(deleted_dossier).not_to be_nil } + it { expect(deleted_dossier.reason).to eq("procedure_removed") } + it { expect(operations).to eq([:supprimer]) } + end + + describe '#restore' do + let(:dossier) { create(:dossier, :en_construction) } + let(:procedure) { dossier.procedure } + let(:deleted_dossier) { DeletedDossier.find_by(dossier_id: dossier.id) } + let(:operations) { dossier.dossier_operation_logs.map(&:operation).map(&:to_sym) } + + before do + procedure.discard_and_keep_track!(administration) + + post :restore, params: { id: procedure.id } + procedure.reload + end + + it { expect(procedure.kept?).to be_truthy } + it { expect(dossier.kept?).to be_truthy } + it { expect(deleted_dossier).to be_nil } + it { expect(operations).to eq([:supprimer, :restaurer]) } end describe '#index' do render_views - let(:administration) { create(:administration) } - let!(:dossier) { create(:dossier) } - context 'sort by dossiers' do + let!(:dossier) { create(:dossier) } + before do - sign_in(administration) get :index, params: { procedure: { direction: 'asc', order: 'dossiers' } } end From 58c126308c83e905ddde4decbd80f74967b79227 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 26 Mar 2020 17:37:55 +0100 Subject: [PATCH 05/14] Add DiscardedProceduresDeletionJob job --- app/jobs/discarded_procedures_deletion_job.rb | 10 ++++++++++ app/models/dossier.rb | 2 -- app/models/procedure.rb | 6 ++++++ spec/models/dossier_spec.rb | 2 +- 4 files changed, 17 insertions(+), 3 deletions(-) create mode 100644 app/jobs/discarded_procedures_deletion_job.rb diff --git a/app/jobs/discarded_procedures_deletion_job.rb b/app/jobs/discarded_procedures_deletion_job.rb new file mode 100644 index 000000000..8ea745e89 --- /dev/null +++ b/app/jobs/discarded_procedures_deletion_job.rb @@ -0,0 +1,10 @@ +class DiscardedProceduresDeletionJob < CronJob + self.cron_expression = "0 7 * * *" + + def perform(*args) + Procedure.discarded_expired.find_each do |procedure| + procedure.dossiers.with_discarded.destroy_all + procedure.destroy + end + end +end diff --git a/app/models/dossier.rb b/app/models/dossier.rb index ad2f8487b..81e871f61 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -212,9 +212,7 @@ class Dossier < ApplicationRecord with_discarded .discarded .state_en_construction - .joins(:procedure) .where('dossiers.hidden_at < ?', 1.month.ago) - .where(procedures: { hidden_at: nil }) end scope :brouillon_near_procedure_closing_date, -> do diff --git a/app/models/procedure.rb b/app/models/procedure.rb index 32e6067f9..dbf49949d 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -53,6 +53,12 @@ class Procedure < ApplicationRecord scope :cloned_from_library, -> { where(cloned_from_library: true) } scope :declarative, -> { where.not(declarative_with_state: nil) } + scope :discarded_expired, -> do + with_discarded + .discarded + .where('hidden_at < ?', 1.month.ago) + end + scope :for_api, -> { includes( :administrateurs, diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index 3d9bc0b2a..52ca110ae 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -1248,6 +1248,6 @@ describe Dossier do end it { expect(Dossier.discarded_brouillon_expired.count).to eq(2) } - it { expect(Dossier.discarded_en_construction_expired.count).to eq(1) } + it { expect(Dossier.discarded_en_construction_expired.count).to eq(2) } end end From e4ab2574ce64aedcde8f63b943d3c46f7fc74997 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 26 Mar 2020 18:49:26 +0100 Subject: [PATCH 06/14] =?UTF-8?q?D=C3=A9marches=20discarded=20by=20adminis?= =?UTF-8?q?trateur=20can=20be=20reactivated=20in=20manager?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../admin/procedures_controller.rb | 15 +++-- app/models/procedure.rb | 12 +++- app/views/admin/procedures/_list.html.haml | 2 +- .../admin/procedures_controller_spec.rb | 58 ++++++++++++------- 4 files changed, 55 insertions(+), 32 deletions(-) diff --git a/app/controllers/admin/procedures_controller.rb b/app/controllers/admin/procedures_controller.rb index cd6259131..a9cde8461 100644 --- a/app/controllers/admin/procedures_controller.rb +++ b/app/controllers/admin/procedures_controller.rb @@ -52,15 +52,14 @@ class Admin::ProceduresController < AdminController def destroy procedure = current_administrateur.procedures.find(params[:id]) - if procedure.locked? - return render json: {}, status: 401 + if procedure.can_be_deleted_by_administrateur? + procedure.discard_and_keep_track!(current_administrateur) + + flash.notice = 'Démarche supprimée' + redirect_to admin_procedures_draft_path + else + render json: {}, status: 403 end - - procedure.reset! - procedure.destroy - - flash.notice = 'Démarche supprimée' - redirect_to admin_procedures_draft_path end def publish_validate diff --git a/app/models/procedure.rb b/app/models/procedure.rb index dbf49949d..f21b5fbb8 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -517,11 +517,21 @@ class Procedure < ApplicationRecord groupe_instructeurs.count > 1 end + def can_be_deleted_by_administrateur? + brouillon? || dossiers.state_instruction_commencee.empty? + end + def can_be_deleted_by_manager? - kept? && dossiers.state_instruction_commencee.empty? + kept? && can_be_deleted_by_administrateur? end def discard_and_keep_track!(author) + if brouillon? + reset! + elsif publiee? + close! + end + dossiers.each do |dossier| dossier.discard_and_keep_track!(author, :procedure_removed) end diff --git a/app/views/admin/procedures/_list.html.haml b/app/views/admin/procedures/_list.html.haml index 1828c02ff..33165d4ec 100644 --- a/app/views/admin/procedures/_list.html.haml +++ b/app/views/admin/procedures/_list.html.haml @@ -25,7 +25,7 @@ %td= link_to(try_format_datetime(procedure.created_at), admin_procedure_href) %td = link_to('Cloner', admin_procedure_clone_path(procedure.id), data: { method: :put }, class: 'btn-sm btn-primary clone-btn') - - if !procedure.locked? + - if !procedure.can_be_deleted_by_administrateur? = link_to('X', url_for(controller: 'admin/procedures', action: :destroy, id: procedure.id), data: { method: :delete, confirm: "Confirmez-vous la suppression de la démarche ? \n\n Attention : toute suppression est définitive et s’appliquera aux éventuels autres administrateurs de cette démarche !" }, class: 'btn-sm btn-danger') = smart_listing.paginate diff --git a/spec/controllers/admin/procedures_controller_spec.rb b/spec/controllers/admin/procedures_controller_spec.rb index 52f9b248b..e21a3e6f4 100644 --- a/spec/controllers/admin/procedures_controller_spec.rb +++ b/spec/controllers/admin/procedures_controller_spec.rb @@ -95,51 +95,65 @@ describe Admin::ProceduresController, type: :controller do end describe 'DELETE #destroy' do - let(:procedure_draft) { create :procedure_with_dossiers, administrateur: admin, instructeurs: [admin.instructeur], published_at: nil, closed_at: nil } - let(:procedure_published) { create :procedure_with_dossiers, administrateur: admin, instructeurs: [admin.instructeur], aasm_state: :publiee, published_at: Time.zone.now, closed_at: nil } - let(:procedure_closed) { create :procedure_with_dossiers, administrateur: admin, instructeurs: [admin.instructeur], aasm_state: :close, published_at: nil, closed_at: Time.zone.now } + let(:procedure_draft) { create(:procedure, administrateurs: [admin]) } + let(:procedure_published) { create(:procedure, :published, administrateurs: [admin]) } + let(:procedure_closed) { create(:procedure, :closed, administrateurs: [admin]) } + let(:procedure) { dossier.procedure } - subject { delete :destroy, params: { id: procedure.id } } + subject { delete :destroy, params: { id: procedure } } - context 'when the procedure is a draft' do - let!(:procedure) { procedure_draft } + context 'when the procedure is a brouillon' do + let(:dossier) { create(:dossier, :en_instruction, procedure: procedure_draft) } - it 'destroys the procedure' do - expect { subject }.to change { Procedure.count }.by(-1) + before { subject } + + it 'discard the procedure' do + expect(procedure.reload.discarded?).to be_truthy end it 'deletes associated dossiers' do - subject - expect(Dossier.find_by(procedure_id: procedure.id)).to be_blank + expect(procedure.dossiers.with_discarded.count).to eq(0) end it 'redirects to the procedure drafts page' do - subject expect(response).to redirect_to admin_procedures_draft_path expect(flash[:notice]).to be_present end end context 'when procedure is published' do - let!(:procedure) { procedure_published } + let(:dossier) { create(:dossier, :en_instruction, procedure: procedure_published) } - it { expect { subject }.not_to change { Procedure.count } } - it { expect { subject }.not_to change { Dossier.count } } - it { expect(subject.status).to eq 401 } + before { subject } + + it { expect(response.status).to eq 403 } + + context 'when dossier is en_construction' do + let(:dossier) { create(:dossier, :en_construction, procedure: procedure_published) } + + it { expect(procedure.reload.close?).to be_truthy } + it { expect(procedure.reload.discarded?).to be_truthy } + it { expect(dossier.reload.discarded?).to be_truthy } + end end context 'when procedure is closed' do - let!(:procedure) { procedure_closed } + let(:dossier) { create(:dossier, :en_instruction, procedure: procedure_closed) } - it { expect { subject }.not_to change { Procedure.count } } - it { expect { subject }.not_to change { Dossier.count } } - it { expect(subject.status).to eq 401 } + before { subject } + + it { expect(response.status).to eq 403 } + + context 'when dossier is en_construction' do + let(:dossier) { create(:dossier, :en_construction, procedure: procedure_published) } + + it { expect(procedure.reload.discarded?).to be_truthy } + it { expect(dossier.reload.discarded?).to be_truthy } + end end context "when administrateur does not own the procedure" do - let(:procedure_not_owned) { create :procedure, administrateur: create(:administrateur), published_at: nil, closed_at: nil } - - subject { delete :destroy, params: { id: procedure_not_owned.id } } + let(:dossier) { create(:dossier) } it { expect { subject }.to raise_error(ActiveRecord::RecordNotFound) } end From bd81970f6785d76b63b5f35ffe2dc863651a9a8a Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 26 Mar 2020 20:25:19 +0100 Subject: [PATCH 07/14] DeletedDossier should have access to discarded demarche --- app/models/deleted_dossier.rb | 2 +- spec/factories/deleted_dossier.rb | 1 + spec/models/deleted_dossier_spec.rb | 14 ++++++++++++++ 3 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 spec/models/deleted_dossier_spec.rb diff --git a/app/models/deleted_dossier.rb b/app/models/deleted_dossier.rb index 6a7845b08..30317bbf8 100644 --- a/app/models/deleted_dossier.rb +++ b/app/models/deleted_dossier.rb @@ -1,5 +1,5 @@ class DeletedDossier < ApplicationRecord - belongs_to :procedure + belongs_to :procedure, -> { with_discarded }, inverse_of: :deleted_dossiers validates :dossier_id, uniqueness: true diff --git a/spec/factories/deleted_dossier.rb b/spec/factories/deleted_dossier.rb index f511742ae..292c9abb8 100644 --- a/spec/factories/deleted_dossier.rb +++ b/spec/factories/deleted_dossier.rb @@ -2,6 +2,7 @@ FactoryBot.define do factory :deleted_dossier do dossier_id { 1111 } state { Dossier.states.fetch(:en_construction) } + reason { DeletedDossier.reasons.fetch(:user_request) } deleted_at { Time.zone.now } association :procedure, :published diff --git a/spec/models/deleted_dossier_spec.rb b/spec/models/deleted_dossier_spec.rb new file mode 100644 index 000000000..eed7407b4 --- /dev/null +++ b/spec/models/deleted_dossier_spec.rb @@ -0,0 +1,14 @@ +require 'spec_helper' + +describe DeletedDossier do + let(:deleted_dossier) { create(:deleted_dossier) } + + describe 'with discarded procedure' do + before do + deleted_dossier.procedure.discard! + deleted_dossier.reload + end + + it { expect(deleted_dossier.procedure).not_to be_nil } + end +end From 288ace2f348d556dd4d1d5e5cf39447793dde2e4 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 31 Mar 2020 14:51:07 +0200 Subject: [PATCH 08/14] Dossier on discarded procedure should have access to discarded procedure --- app/models/groupe_instructeur.rb | 2 +- spec/models/dossier_spec.rb | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/app/models/groupe_instructeur.rb b/app/models/groupe_instructeur.rb index ccb84e5f1..ca883e2a5 100644 --- a/app/models/groupe_instructeur.rb +++ b/app/models/groupe_instructeur.rb @@ -1,6 +1,6 @@ class GroupeInstructeur < ApplicationRecord DEFAULT_LABEL = 'défaut' - belongs_to :procedure + belongs_to :procedure, -> { with_discarded }, inverse_of: :groupe_instructeurs has_many :assign_tos has_many :instructeurs, through: :assign_tos, dependent: :destroy has_many :dossiers diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index 52ca110ae..6f18e8919 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -1250,4 +1250,14 @@ describe Dossier do it { expect(Dossier.discarded_brouillon_expired.count).to eq(2) } it { expect(Dossier.discarded_en_construction_expired.count).to eq(2) } end + + describe "discarded procedure dossier should be able to access it's procedure" do + let(:dossier) { create(:dossier) } + let(:procedure) { dossier.reload.procedure } + + before { dossier.procedure.discard! } + + it { expect(procedure).not_to be_nil } + it { expect(procedure.discarded?).to be_truthy } + end end From 7ba4c513e6609832bc02a18b7089d119055f74ca Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 1 Apr 2020 17:08:50 +0200 Subject: [PATCH 09/14] Refactor notify near deletion mailers --- app/mailers/dossier_mailer.rb | 24 +++++- .../expired_dossiers_deletion_service.rb | 47 +++++++----- ...otify_automatic_deletion_to_user.html.haml | 3 +- ...near_deletion_to_administration.html.haml} | 5 +- ...=> notify_near_deletion_to_user.html.haml} | 2 +- .../notify_automatic_deletion_to_user/fr.yml | 2 +- .../fr.yml | 8 +- .../fr.yml | 6 +- spec/mailers/dossier_mailer_spec.rb | 76 ++++++++----------- .../previews/dossier_mailer_preview.rb | 8 +- .../expired_dossiers_deletion_service_spec.rb | 36 ++++----- 11 files changed, 116 insertions(+), 101 deletions(-) rename app/views/dossier_mailer/{notify_en_construction_near_deletion_to_administration.html.haml => notify_near_deletion_to_administration.html.haml} (64%) rename app/views/dossier_mailer/{notify_en_construction_near_deletion_to_user.html.haml => notify_near_deletion_to_user.html.haml} (86%) rename config/locales/views/dossier_mailer/{notify_en_construction_near_deletion_to_administration => notify_near_deletion_to_administration}/fr.yml (79%) rename config/locales/views/dossier_mailer/{notify_en_construction_near_deletion_to_user => notify_near_deletion_to_user}/fr.yml (88%) diff --git a/app/mailers/dossier_mailer.rb b/app/mailers/dossier_mailer.rb index 6bc61df26..5c7dde111 100644 --- a/app/mailers/dossier_mailer.rb +++ b/app/mailers/dossier_mailer.rb @@ -70,6 +70,7 @@ class DossierMailer < ApplicationMailer end def notify_automatic_deletion_to_user(deleted_dossiers, to_email) + @state = deleted_dossiers.first.state @subject = default_i18n_subject(count: deleted_dossiers.count) @deleted_dossiers = deleted_dossiers @@ -83,15 +84,17 @@ class DossierMailer < ApplicationMailer mail(to: to_email, subject: @subject) end - def notify_en_construction_near_deletion_to_user(dossiers, to_email) - @subject = default_i18n_subject(count: dossiers.count) + def notify_near_deletion_to_user(dossiers, to_email) + @state = dossiers.first.state + @subject = default_i18n_subject(count: dossiers.count, state: @state) @dossiers = dossiers mail(to: to_email, subject: @subject) end - def notify_en_construction_near_deletion_to_administration(dossiers, to_email) - @subject = default_i18n_subject(count: dossiers.count) + def notify_near_deletion_to_administration(dossiers, to_email) + @state = dossiers.first.state + @subject = default_i18n_subject(count: dossiers.count, state: @state) @dossiers = dossiers mail(to: to_email, subject: @subject) @@ -111,4 +114,17 @@ class DossierMailer < ApplicationMailer mail(to: dossier.user.email, subject: @subject) end + + protected + + # 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 + def default_i18n_subject(interpolations = {}) + if interpolations[:state] + mailer_scope = self.class.mailer_name.tr('/', '.') + I18n.t("subject_#{interpolations[:state]}", interpolations.merge(scope: [mailer_scope, action_name])) + else + super + end + end end diff --git a/app/services/expired_dossiers_deletion_service.rb b/app/services/expired_dossiers_deletion_service.rb index e47fff4e2..9b8efaa29 100644 --- a/app/services/expired_dossiers_deletion_service.rb +++ b/app/services/expired_dossiers_deletion_service.rb @@ -33,23 +33,7 @@ class ExpiredDossiersDeletionService .en_construction_close_to_expiration .without_en_construction_expiration_notice_sent - dossiers_close_to_expiration - .with_notifiable_procedure - .includes(:user) - .group_by(&:user) - .each do |(user, dossiers)| - DossierMailer.notify_en_construction_near_deletion_to_user( - dossiers, - user.email - ).deliver_later - end - - group_by_fonctionnaire_email(dossiers_close_to_expiration).each do |(email, dossiers)| - DossierMailer.notify_en_construction_near_deletion_to_administration( - dossiers, - email - ).deliver_later - end + send_expiration_notices(dossiers_close_to_expiration) dossiers_close_to_expiration.update_all(en_construction_close_to_expiration_notice_sent_at: Time.zone.now) end @@ -72,7 +56,32 @@ class ExpiredDossiersDeletionService end def self.delete_expired_en_construction_and_notify - dossiers_to_remove = Dossier.en_construction_expired + delete_expired_and_notify(Dossier.en_construction_expired) + end + + private + + def self.send_expiration_notices(dossiers_close_to_expiration) + dossiers_close_to_expiration + .with_notifiable_procedure + .includes(:user) + .group_by(&:user) + .each do |(user, dossiers)| + DossierMailer.notify_near_deletion_to_user( + dossiers, + user.email + ).deliver_later + end + + group_by_fonctionnaire_email(dossiers_close_to_expiration).each do |(email, dossiers)| + DossierMailer.notify_near_deletion_to_administration( + dossiers.to_a, + email + ).deliver_later + end + end + + def self.delete_expired_and_notify(dossiers_to_remove) dossiers_to_remove.each(&:expired_keep_track!) dossiers_to_remove @@ -96,8 +105,6 @@ class ExpiredDossiersDeletionService dossiers_to_remove.destroy_all end - private - def self.group_by_fonctionnaire_email(dossiers) dossiers .with_notifiable_procedure diff --git a/app/views/dossier_mailer/notify_automatic_deletion_to_user.html.haml b/app/views/dossier_mailer/notify_automatic_deletion_to_user.html.haml index 8a824675c..bf35cabe2 100644 --- a/app/views/dossier_mailer/notify_automatic_deletion_to_user.html.haml +++ b/app/views/dossier_mailer/notify_automatic_deletion_to_user.html.haml @@ -9,6 +9,7 @@ - @deleted_dossiers.each do |d| %li n° #{d.dossier_id} (#{d.procedure.libelle}) -%p= t('.footer', count: @deleted_dossiers.count) +- if @state == Dossier.states.fetch(:en_construction) + %p= t('.footer_en_construction', count: @deleted_dossiers.count) = render partial: "layouts/mailers/signature" diff --git a/app/views/dossier_mailer/notify_en_construction_near_deletion_to_administration.html.haml b/app/views/dossier_mailer/notify_near_deletion_to_administration.html.haml similarity index 64% rename from app/views/dossier_mailer/notify_en_construction_near_deletion_to_administration.html.haml rename to app/views/dossier_mailer/notify_near_deletion_to_administration.html.haml index b89f885f9..62b0156ff 100644 --- a/app/views/dossier_mailer/notify_en_construction_near_deletion_to_administration.html.haml +++ b/app/views/dossier_mailer/notify_near_deletion_to_administration.html.haml @@ -4,13 +4,14 @@ Bonjour, %p - = t('.header', count: @dossiers.count) + = t('.header_en_construction', count: @dossiers.count) %ul - @dossiers.each do |d| %li #{link_to("n° #{d.id} (#{d.procedure.libelle})", dossier_url(d))}. Retrouvez le dossier au format #{link_to("PDF", instructeur_dossier_url(d.procedure, d, format: :pdf))} %p - = sanitize(t('.footer', count: @dossiers.count)) + - if @state == Dossier.states.fetch(:en_construction) + = sanitize(t('.footer_en_construction', count: @dossiers.count)) = render partial: "layouts/mailers/signature" diff --git a/app/views/dossier_mailer/notify_en_construction_near_deletion_to_user.html.haml b/app/views/dossier_mailer/notify_near_deletion_to_user.html.haml similarity index 86% rename from app/views/dossier_mailer/notify_en_construction_near_deletion_to_user.html.haml rename to app/views/dossier_mailer/notify_near_deletion_to_user.html.haml index 30dd62ad6..efe0c668d 100644 --- a/app/views/dossier_mailer/notify_en_construction_near_deletion_to_user.html.haml +++ b/app/views/dossier_mailer/notify_near_deletion_to_user.html.haml @@ -4,7 +4,7 @@ Bonjour, %p - = t('.header', count: @dossiers.count) + = t('.header_en_construction', count: @dossiers.count) %ul - @dossiers.each do |d| %li diff --git a/config/locales/views/dossier_mailer/notify_automatic_deletion_to_user/fr.yml b/config/locales/views/dossier_mailer/notify_automatic_deletion_to_user/fr.yml index 0f082b5ab..8650d30f4 100644 --- a/config/locales/views/dossier_mailer/notify_automatic_deletion_to_user/fr.yml +++ b/config/locales/views/dossier_mailer/notify_automatic_deletion_to_user/fr.yml @@ -7,6 +7,6 @@ fr: header: one: "Le délai maximum de conservation du dossier suivant a été atteint, il a donc été supprimé :" other: "Le délai maximum de conservation des dossiers suivants a été atteint, ils ont donc été supprimés :" - footer: + footer_en_construction: one: "Le dossier ne sera pas traité, nous nous excusons de la gène occasionnée." other: "Les dossiers ne seront pas traités, nous nous excusons de la gène occasionnée." diff --git a/config/locales/views/dossier_mailer/notify_en_construction_near_deletion_to_administration/fr.yml b/config/locales/views/dossier_mailer/notify_near_deletion_to_administration/fr.yml similarity index 79% rename from config/locales/views/dossier_mailer/notify_en_construction_near_deletion_to_administration/fr.yml rename to config/locales/views/dossier_mailer/notify_near_deletion_to_administration/fr.yml index a49026aab..e3d24481c 100644 --- a/config/locales/views/dossier_mailer/notify_en_construction_near_deletion_to_administration/fr.yml +++ b/config/locales/views/dossier_mailer/notify_near_deletion_to_administration/fr.yml @@ -1,12 +1,12 @@ fr: dossier_mailer: - notify_en_construction_near_deletion_to_administration: - subject: + notify_near_deletion_to_administration: + subject_en_construction: one: Un dossier en construction va bientôt être supprimé other: Des dossiers en construction vont bientôt être supprimés - header: + header_en_construction: one: "Le dossier en construction suivant sera bientôt automatiquement supprimé :" other: "Les dossiers en construction suivant seront bientôt automatiquement supprimés :" - footer: + footer_en_construction: one: "Vous avez un mois pour commencer l’instruction du dossier." other: "Vous avez un mois pour commencer l’instruction des dossiers." diff --git a/config/locales/views/dossier_mailer/notify_en_construction_near_deletion_to_user/fr.yml b/config/locales/views/dossier_mailer/notify_near_deletion_to_user/fr.yml similarity index 88% rename from config/locales/views/dossier_mailer/notify_en_construction_near_deletion_to_user/fr.yml rename to config/locales/views/dossier_mailer/notify_near_deletion_to_user/fr.yml index d9c58d97b..e81cad44d 100644 --- a/config/locales/views/dossier_mailer/notify_en_construction_near_deletion_to_user/fr.yml +++ b/config/locales/views/dossier_mailer/notify_near_deletion_to_user/fr.yml @@ -1,10 +1,10 @@ fr: dossier_mailer: - notify_en_construction_near_deletion_to_user: - subject: + notify_near_deletion_to_user: + subject_en_construction: one: Un dossier en construction va bientôt être supprimé other: Des dossiers en construction vont bientôt être supprimés - header: + header_en_construction: one: "Afin de limiter la conservation de vos données personnelles, le dossier en construction suivant sera bientôt automatiquement supprimé :" other: "Afin de limiter la conservation de vos données personnelles, les dossiers en construction suivant seront bientôt automatiquement supprimés :" footer: diff --git a/spec/mailers/dossier_mailer_spec.rb b/spec/mailers/dossier_mailer_spec.rb index a7578880b..7ba807f9a 100644 --- a/spec/mailers/dossier_mailer_spec.rb +++ b/spec/mailers/dossier_mailer_spec.rb @@ -74,11 +74,6 @@ RSpec.describe DossierMailer, type: :mailer do describe '.notify_brouillon_near_deletion' do let(:dossier) { create(:dossier) } - before do - duree = dossier.procedure.duree_conservation_dossiers_dans_ds - @date_suppression = dossier.created_at + duree.months - end - subject { described_class.notify_brouillon_near_deletion([dossier], dossier.user.email) } it { expect(subject.body).to include("n° #{dossier.id} ") } @@ -95,21 +90,18 @@ RSpec.describe DossierMailer, type: :mailer do end describe '.notify_automatic_deletion_to_user' do - let(:dossier) { create(:dossier) } - let(:deleted_dossier) { DeletedDossier.create_from_dossier(dossier, :expired) } + describe 'en_construction' do + let(:dossier) { create(:dossier, :en_construction) } + let(:deleted_dossier) { DeletedDossier.create_from_dossier(dossier, :expired) } - before do - duree = dossier.procedure.duree_conservation_dossiers_dans_ds - @date_suppression = dossier.created_at + duree.months + subject { described_class.notify_automatic_deletion_to_user([deleted_dossier], dossier.user.email) } + + it { expect(subject.to).to eq([dossier.user.email]) } + it { expect(subject.subject).to eq("Un dossier a été supprimé automatiquement") } + it { expect(subject.body).to include("n° #{dossier.id} ") } + it { expect(subject.body).to include(dossier.procedure.libelle) } + it { expect(subject.body).to include("nous nous excusons de la gène occasionnée") } end - - subject { described_class.notify_automatic_deletion_to_user([deleted_dossier], dossier.user.email) } - - it { expect(subject.to).to eq([dossier.user.email]) } - it { expect(subject.subject).to eq("Un dossier a été supprimé automatiquement") } - it { expect(subject.body).to include("n° #{dossier.id} ") } - it { expect(subject.body).to include(dossier.procedure.libelle) } - it { expect(subject.body).to include("nous nous excusons de la gène occasionnée") } end describe '.notify_automatic_deletion_to_administration' do @@ -122,39 +114,33 @@ RSpec.describe DossierMailer, type: :mailer do it { expect(subject.body).to include("n° #{dossier.id} (#{dossier.procedure.libelle})") } end - describe '.notify_en_construction_near_deletion_to_administration' do - let(:dossier) { create(:dossier) } + describe '.notify_near_deletion_to_administration' do + describe 'en_construction' do + let(:dossier) { create(:dossier, :en_construction) } - before do - duree = dossier.procedure.duree_conservation_dossiers_dans_ds - @date_suppression = dossier.created_at + duree.months + subject { described_class.notify_near_deletion_to_administration([dossier], dossier.user.email) } + + it { expect(subject.subject).to eq("Un dossier en construction va bientôt être supprimé") } + it { expect(subject.body).to include("n° #{dossier.id} ") } + it { expect(subject.body).to include(dossier.procedure.libelle) } + it { expect(subject.body).to include("PDF") } + it { expect(subject.body).to include("Vous avez un mois pour commencer l’instruction du dossier.") } end - - subject { described_class.notify_en_construction_near_deletion_to_administration([dossier], dossier.user.email) } - - it { expect(subject.subject).to eq("Un dossier en construction va bientôt être supprimé") } - it { expect(subject.body).to include("n° #{dossier.id} ") } - it { expect(subject.body).to include(dossier.procedure.libelle) } - it { expect(subject.body).to include("PDF") } - it { expect(subject.body).to include("Vous avez un mois pour commencer l’instruction du dossier.") } end - describe '.notify_en_construction_near_deletion_to_user' do - let(:dossier) { create(:dossier) } + describe '.notify_near_deletion_to_user' do + describe 'en_construction' do + let(:dossier) { create(:dossier, :en_construction) } - before do - duree = dossier.procedure.duree_conservation_dossiers_dans_ds - @date_suppression = dossier.created_at + duree.months + subject { described_class.notify_near_deletion_to_user([dossier], dossier.user.email) } + + it { expect(subject.to).to eq([dossier.user.email]) } + it { expect(subject.subject).to eq("Un dossier en construction va bientôt être supprimé") } + it { expect(subject.body).to include("n° #{dossier.id} ") } + it { expect(subject.body).to include(dossier.procedure.libelle) } + it { expect(subject.body).to include("PDF") } + it { expect(subject.body).to include("Vous pouvez retrouver votre dossier pendant encore un mois. Vous n’avez rien à faire.") } end - - subject { described_class.notify_en_construction_near_deletion_to_user([dossier], dossier.user.email) } - - it { expect(subject.to).to eq([dossier.user.email]) } - it { expect(subject.subject).to eq("Un dossier en construction va bientôt être supprimé") } - it { expect(subject.body).to include("n° #{dossier.id} ") } - it { expect(subject.body).to include(dossier.procedure.libelle) } - it { expect(subject.body).to include("PDF") } - it { expect(subject.body).to include("Vous pouvez retrouver votre dossier pendant encore un mois. Vous n’avez rien à faire.") } end describe '.notify_groupe_instructeur_changed_to_instructeur' do diff --git a/spec/mailers/previews/dossier_mailer_preview.rb b/spec/mailers/previews/dossier_mailer_preview.rb index c959003fd..fdda28d6c 100644 --- a/spec/mailers/previews/dossier_mailer_preview.rb +++ b/spec/mailers/previews/dossier_mailer_preview.rb @@ -21,11 +21,11 @@ class DossierMailerPreview < ActionMailer::Preview end def notify_en_construction_near_deletion_to_user - DossierMailer.notify_en_construction_near_deletion_to_user([dossier], usager_email) + DossierMailer.notify_near_deletion_to_user([dossier_en_construction], usager_email) end def notify_en_construction_near_deletion_to_administration - DossierMailer.notify_en_construction_near_deletion_to_administration([dossier, dossier], administration_email) + DossierMailer.notify_near_deletion_to_administration([dossier_en_construction, dossier_en_construction], administration_email) end def notify_brouillon_deletion @@ -79,6 +79,10 @@ class DossierMailerPreview < ActionMailer::Preview Dossier.new(id: 47882, state: :en_instruction, procedure: procedure, user: User.new(email: "usager@example.com")) end + def dossier_en_construction + Dossier.new(id: 47882, state: :en_construction, procedure: procedure, user: User.new(email: "usager@example.com")) + end + def procedure Procedure.new(id: 1234, libelle: 'Dotation d’Équipement des Territoires Ruraux - Exercice 2019', service: service, logo: Rack::Test::UploadedFile.new("./spec/fixtures/files/logo_test_procedure.png", 'image/png'), auto_archive_on: Time.zone.today + Dossier::REMAINING_DAYS_BEFORE_CLOSING.days) end diff --git a/spec/services/expired_dossiers_deletion_service_spec.rb b/spec/services/expired_dossiers_deletion_service_spec.rb index 38e241812..aecca6f64 100644 --- a/spec/services/expired_dossiers_deletion_service_spec.rb +++ b/spec/services/expired_dossiers_deletion_service_spec.rb @@ -139,8 +139,8 @@ describe ExpiredDossiersDeletionService do after { Timecop.return } before do - allow(DossierMailer).to receive(:notify_en_construction_near_deletion_to_user).and_return(double(deliver_later: nil)) - allow(DossierMailer).to receive(:notify_en_construction_near_deletion_to_administration).and_return(double(deliver_later: nil)) + allow(DossierMailer).to receive(:notify_near_deletion_to_user).and_return(double(deliver_later: nil)) + allow(DossierMailer).to receive(:notify_near_deletion_to_administration).and_return(double(deliver_later: nil)) end context 'with a single dossier' do @@ -152,8 +152,8 @@ describe ExpiredDossiersDeletionService do let(:en_construction_at) { (conservation_par_defaut - 1.month - 1.day).ago } it { expect(dossier.reload.en_construction_close_to_expiration_notice_sent_at).to be_nil } - it { expect(DossierMailer).not_to have_received(:notify_en_construction_near_deletion_to_user) } - it { expect(DossierMailer).not_to have_received(:notify_en_construction_near_deletion_to_administration) } + it { expect(DossierMailer).not_to have_received(:notify_near_deletion_to_user) } + it { expect(DossierMailer).not_to have_received(:notify_near_deletion_to_administration) } end context 'when the dossier is near deletion' do @@ -161,11 +161,11 @@ describe ExpiredDossiersDeletionService do it { expect(dossier.reload.en_construction_close_to_expiration_notice_sent_at).not_to be_nil } - it { expect(DossierMailer).to have_received(:notify_en_construction_near_deletion_to_user).once } - it { expect(DossierMailer).to have_received(:notify_en_construction_near_deletion_to_administration).twice } - it { expect(DossierMailer).to have_received(:notify_en_construction_near_deletion_to_user).with([dossier], dossier.user.email) } - it { expect(DossierMailer).to have_received(:notify_en_construction_near_deletion_to_administration).with([dossier], dossier.procedure.administrateurs.first.email) } - it { expect(DossierMailer).to have_received(:notify_en_construction_near_deletion_to_administration).with([dossier], dossier.followers_instructeurs.first.email) } + it { expect(DossierMailer).to have_received(:notify_near_deletion_to_user).once } + it { expect(DossierMailer).to have_received(:notify_near_deletion_to_administration).twice } + it { expect(DossierMailer).to have_received(:notify_near_deletion_to_user).with([dossier], dossier.user.email) } + it { expect(DossierMailer).to have_received(:notify_near_deletion_to_administration).with([dossier], dossier.procedure.administrateurs.first.email) } + it { expect(DossierMailer).to have_received(:notify_near_deletion_to_administration).with([dossier], dossier.followers_instructeurs.first.email) } end end @@ -181,12 +181,12 @@ describe ExpiredDossiersDeletionService do ExpiredDossiersDeletionService.send_en_construction_expiration_notices end - it { expect(DossierMailer).to have_received(:notify_en_construction_near_deletion_to_user).once } - it { expect(DossierMailer).to have_received(:notify_en_construction_near_deletion_to_administration).exactly(3).times } - it { expect(DossierMailer).to have_received(:notify_en_construction_near_deletion_to_user).with(match_array([dossier_1, dossier_2]), user.email) } - it { expect(DossierMailer).to have_received(:notify_en_construction_near_deletion_to_administration).with(match_array([dossier_1, dossier_2]), instructeur.email) } - it { expect(DossierMailer).to have_received(:notify_en_construction_near_deletion_to_administration).with([dossier_1], dossier_1.procedure.administrateurs.first.email) } - it { expect(DossierMailer).to have_received(:notify_en_construction_near_deletion_to_administration).with([dossier_2], dossier_2.procedure.administrateurs.first.email) } + it { expect(DossierMailer).to have_received(:notify_near_deletion_to_user).once } + it { expect(DossierMailer).to have_received(:notify_near_deletion_to_administration).exactly(3).times } + it { expect(DossierMailer).to have_received(:notify_near_deletion_to_user).with(match_array([dossier_1, dossier_2]), user.email) } + it { expect(DossierMailer).to have_received(:notify_near_deletion_to_administration).with(match_array([dossier_1, dossier_2]), instructeur.email) } + it { expect(DossierMailer).to have_received(:notify_near_deletion_to_administration).with([dossier_1], dossier_1.procedure.administrateurs.first.email) } + it { expect(DossierMailer).to have_received(:notify_near_deletion_to_administration).with([dossier_2], dossier_2.procedure.administrateurs.first.email) } end context 'when an instructeur is also administrateur' do @@ -199,9 +199,9 @@ describe ExpiredDossiersDeletionService do ExpiredDossiersDeletionService.send_en_construction_expiration_notices end - it { expect(DossierMailer).to have_received(:notify_en_construction_near_deletion_to_user).once } - it { expect(DossierMailer).to have_received(:notify_en_construction_near_deletion_to_user).with([dossier], dossier.user.email) } - it { expect(DossierMailer).to have_received(:notify_en_construction_near_deletion_to_administration).with([dossier], administrateur.email) } + it { expect(DossierMailer).to have_received(:notify_near_deletion_to_user).once } + it { expect(DossierMailer).to have_received(:notify_near_deletion_to_user).with([dossier], dossier.user.email) } + it { expect(DossierMailer).to have_received(:notify_near_deletion_to_administration).with([dossier], administrateur.email) } end end From c0bbfa42b7817e04dcfbfa064151fae3fb07dcff Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 1 Apr 2020 11:10:25 +0200 Subject: [PATCH 10/14] Reassign discarded dossiers on group_instructeur fix #4851 --- .../groupe_instructeurs_controller.rb | 2 +- app/models/dossier.rb | 4 +++- .../groupe_instructeurs_controller_spec.rb | 11 ++++++++--- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/app/controllers/new_administrateur/groupe_instructeurs_controller.rb b/app/controllers/new_administrateur/groupe_instructeurs_controller.rb index 8732b2bb1..b07b844b6 100644 --- a/app/controllers/new_administrateur/groupe_instructeurs_controller.rb +++ b/app/controllers/new_administrateur/groupe_instructeurs_controller.rb @@ -71,7 +71,7 @@ module NewAdministrateur def reaffecter target_group = procedure.groupe_instructeurs.find(params[:target_group]) - groupe_instructeur.dossiers.find_each do |dossier| + groupe_instructeur.dossiers.with_discarded.find_each do |dossier| dossier.assign_to_groupe_instructeur(target_group, current_administrateur) end diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 81e871f61..c8e13c658 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -759,7 +759,9 @@ class Dossier < ApplicationRecord followers_instructeurs.each do |instructeur| if instructeur.groupe_instructeurs.exclude?(groupe_instructeur) instructeur.unfollow(self) - DossierMailer.notify_groupe_instructeur_changed(instructeur, self).deliver_later + if kept? + DossierMailer.notify_groupe_instructeur_changed(instructeur, self).deliver_later + end end end end diff --git a/spec/controllers/new_administrateur/groupe_instructeurs_controller_spec.rb b/spec/controllers/new_administrateur/groupe_instructeurs_controller_spec.rb index 93d02b5fe..86b760787 100644 --- a/spec/controllers/new_administrateur/groupe_instructeurs_controller_spec.rb +++ b/spec/controllers/new_administrateur/groupe_instructeurs_controller_spec.rb @@ -121,7 +121,12 @@ describe NewAdministrateur::GroupeInstructeursController, type: :controller do describe '#reaffecter' do let!(:gi_1_2) { procedure.groupe_instructeurs.create(label: 'groupe instructeur 2') } - let!(:dossier12) { create(:dossier, procedure: procedure, state: Dossier.states.fetch(:en_construction), groupe_instructeur: gi_1_1) } + let!(:dossier12) { create(:dossier, :en_construction, procedure: procedure, groupe_instructeur: gi_1_1) } + let!(:dossier_discarded) do + dossier = create(:dossier, :en_construction, procedure: procedure, groupe_instructeur: gi_1_1) + dossier.discard! + dossier + end describe 'when the new group is a group of the procedure' do before do @@ -135,8 +140,8 @@ describe NewAdministrateur::GroupeInstructeursController, type: :controller do end it { expect(response).to redirect_to(procedure_groupe_instructeurs_path(procedure)) } - it { expect(gi_1_1.dossiers.count).to be(0) } - it { expect(gi_1_2.dossiers.count).to be(1) } + it { expect(gi_1_1.dossiers.with_discarded.count).to be(0) } + it { expect(gi_1_2.dossiers.with_discarded.count).to be(2) } it { expect(gi_1_2.dossiers.last.id).to be(dossier12.id) } it { expect(dossier12.groupe_instructeur.id).to be(gi_1_2.id) } end From ac922c456e7194acfa21e9373f1d032f15855c60 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 1 Apr 2020 15:55:40 +0200 Subject: [PATCH 11/14] policies: refactor champ_policy_spec Creating champs without attaching them to a procedure works only by chance. --- spec/policies/champ_policy_spec.rb | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/spec/policies/champ_policy_spec.rb b/spec/policies/champ_policy_spec.rb index b8e890577..a74e813a0 100644 --- a/spec/policies/champ_policy_spec.rb +++ b/spec/policies/champ_policy_spec.rb @@ -1,6 +1,6 @@ describe ChampPolicy do - let(:champ) { create(:champ_text, private: private, dossier: dossier) } - let(:dossier) { create(:dossier, user: dossier_owner) } + let(:procedure) { create(:procedure, :with_type_de_champ, :with_type_de_champ_private) } + let(:dossier) { create(:dossier, procedure: procedure, user: dossier_owner) } let(:dossier_owner) { create(:user) } let(:signed_in_user) { create(:user) } @@ -8,24 +8,23 @@ describe ChampPolicy do subject { Pundit.policy_scope(account, Champ) } + let(:champ) { dossier.champs.first } + let(:champ_private) { dossier.champs_private.first } + shared_examples_for 'they can access a public champ' do - let(:private) { false } it { expect(subject.find_by(id: champ.id)).to eq(champ) } end shared_examples_for 'they can’t access a public champ' do - let(:private) { false } it { expect(subject.find_by(id: champ.id)).to eq(nil) } end shared_examples_for 'they can access a private champ' do - let(:private) { true } - it { expect(subject.find_by(id: champ.id)).to eq(champ) } + it { expect(subject.find_by(id: champ_private.id)).to eq(champ_private) } end shared_examples_for 'they can’t access a private champ' do - let(:private) { true } - it { expect(subject.find_by(id: champ.id)).to eq(nil) } + it { expect(subject.find_by(id: champ_private.id)).to eq(nil) } end context 'when an user only has user rights' do From c17ceb44401bd6ee40d978eaac407d8ccbed2221 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 1 Apr 2020 14:05:54 +0000 Subject: [PATCH 12/14] policies: DRY the scope --- app/policies/champ_policy.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/app/policies/champ_policy.rb b/app/policies/champ_policy.rb index 81d2eb11b..2df555cbd 100644 --- a/app/policies/champ_policy.rb +++ b/app/policies/champ_policy.rb @@ -5,16 +5,18 @@ class ChampPolicy < ApplicationPolicy return scope.none end - # Users can access public champs on their own dossiers. - resolved_scope = scope + # The join must be the same for all elements of the WHERE clause. + joined_scope = scope .left_outer_joins(dossier: { groupe_instructeur: [:instructeurs] }) + + # Users can access public champs on their own dossiers. + resolved_scope = joined_scope .where('dossiers.user_id': user.id, private: false) if instructeur.present? # Additionnaly, instructeurs can access private champs # on dossiers they are allowed to instruct. - instructeur_clause = scope - .left_outer_joins(dossier: { groupe_instructeur: [:instructeurs] }) + instructeur_clause = joined_scope .where('instructeurs.id': instructeur.id, private: true) resolved_scope = resolved_scope.or(instructeur_clause) end From 059e11601bb6d3c95b94691237e81e711780eb3e Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 1 Apr 2020 16:10:45 +0200 Subject: [PATCH 13/14] policies: fix champ policy for guest users --- app/policies/champ_policy.rb | 14 +++++++++++++- spec/policies/champ_policy_spec.rb | 8 ++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/app/policies/champ_policy.rb b/app/policies/champ_policy.rb index 2df555cbd..a55efe1a0 100644 --- a/app/policies/champ_policy.rb +++ b/app/policies/champ_policy.rb @@ -6,13 +6,25 @@ class ChampPolicy < ApplicationPolicy end # The join must be the same for all elements of the WHERE clause. + # + # NB: here we want to do `.left_outer_joins(dossier: [:invites, { :groupe_instructeur: :instructeurs }]))`, + # but for some reasons ActiveRecord <= 5.2 generates bogus SQL. Hence the manual version of it below. joined_scope = scope - .left_outer_joins(dossier: { groupe_instructeur: [:instructeurs] }) + .joins('LEFT OUTER JOIN dossiers ON dossiers.id = champs.dossier_id') + .joins('LEFT OUTER JOIN invites ON invites.dossier_id = dossiers.id') + .joins('LEFT OUTER JOIN groupe_instructeurs ON groupe_instructeurs.id = dossiers.groupe_instructeur_id') + .joins('LEFT OUTER JOIN assign_tos ON assign_tos.groupe_instructeur_id = groupe_instructeurs.id') + .joins('LEFT OUTER JOIN instructeurs ON instructeurs.id = assign_tos.instructeur_id') # Users can access public champs on their own dossiers. resolved_scope = joined_scope .where('dossiers.user_id': user.id, private: false) + # Invited users can access public champs on dossiers they are invited to + invite_clause = joined_scope + .where('invites.user_id': user.id, private: false) + resolved_scope = resolved_scope.or(invite_clause) + if instructeur.present? # Additionnaly, instructeurs can access private champs # on dossiers they are allowed to instruct. diff --git a/spec/policies/champ_policy_spec.rb b/spec/policies/champ_policy_spec.rb index a74e813a0..3a49c5b08 100644 --- a/spec/policies/champ_policy_spec.rb +++ b/spec/policies/champ_policy_spec.rb @@ -35,6 +35,14 @@ describe ChampPolicy do it_behaves_like 'they can’t access a private champ' end + context 'as a person invited on the dossier' do + let(:invite) { create(:invite, :with_user, dossier: dossier) } + let(:signed_in_user) { invite.user } + + it_behaves_like 'they can access a public champ' + it_behaves_like 'they can’t access a private champ' + end + context 'as another user' do let(:signed_in_user) { create(:user) } From c4f2e6cab0d00ed87d21babcc7e988bcad142d6e Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 31 Mar 2020 16:58:36 +0200 Subject: [PATCH 14/14] credentials: add autocomplete attributes Allow browsers to autofill more informations. References: - MDN: Autofill (https://developer.mozilla.org/fr/docs/Web/HTML/Attributs/autocomplete) - Safari Password Autofill (https://developer.apple.com/documentation/security/password_autofill/enabling_password_autofill_on_an_html_input_element) --- app/views/users/dossiers/identite.html.haml | 4 ++-- app/views/users/registrations/new.html.haml | 4 ++-- app/views/users/sessions/new.html.haml | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/views/users/dossiers/identite.html.haml b/app/views/users/dossiers/identite.html.haml index 1e65a5123..6e5d4cc16 100644 --- a/app/views/users/dossiers/identite.html.haml +++ b/app/views/users/dossiers/identite.html.haml @@ -26,10 +26,10 @@ .flex .inline-champ = f.label :prenom, class: "required" - = f.text_field :prenom, class: "small", required: true + = f.text_field :prenom, class: "small", required: true, autocomplete: 'given-name' .inline-champ = f.label :nom, class: "required" - = f.text_field :nom, class: "small", required: true + = f.text_field :nom, class: "small", required: true, autocomplete: 'family-name' - if @dossier.procedure.ask_birthday? = f.label :birthdate, class: "required" diff --git a/app/views/users/registrations/new.html.haml b/app/views/users/registrations/new.html.haml index e6602c9b5..356afb8d9 100644 --- a/app/views/users/registrations/new.html.haml +++ b/app/views/users/registrations/new.html.haml @@ -6,7 +6,7 @@ %h1 Créez-vous un compte demarches-simplifiees.fr = f.label :email, "Email" - = f.text_field :email, autofocus: true, placeholder: "Votre adresse email" + = f.text_field :email, type: :email, autocomplete: 'username', autofocus: true, placeholder: "Votre adresse email" .suspect-email.hidden .email-suggestion-title @@ -20,7 +20,7 @@ Non = f.label :password, "Mot de passe" - = f.password_field :password, value: @user.password, placeholder: "8 caractères minimum", 'aria-describedby':'8 caractères minimum' + = f.password_field :password, autocomplete: 'new-password', value: @user.password, placeholder: "8 caractères minimum", 'aria-describedby':'8 caractères minimum' = f.submit "Créer un compte", class: "button large primary expand" diff --git a/app/views/users/sessions/new.html.haml b/app/views/users/sessions/new.html.haml index ecb783bfe..992008b30 100644 --- a/app/views/users/sessions/new.html.haml +++ b/app/views/users/sessions/new.html.haml @@ -6,10 +6,10 @@ %h2.huge-title Connectez-vous = f.label :email, "Email" - = f.text_field :email, autofocus: true + = f.text_field :email, type: :email, autocomplete: 'username', autofocus: true = f.label :password, "Mot de passe" - = f.password_field :password + = f.password_field :password, autocomplete: 'current-password' .auth-options %div