From 9243ae69dfdd7ed2fd1c1c552fb11c17167b514d Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 19 Mar 2020 11:51:47 +0100 Subject: [PATCH 1/2] Add reason to DeletedDossier # Conflicts: # db/schema.rb --- app/models/deleted_dossier.rb | 17 +++++++++++++++-- ...0319103836_add_reason_to_deleted_dossiers.rb | 5 +++++ db/schema.rb | 3 ++- 3 files changed, 22 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20200319103836_add_reason_to_deleted_dossiers.rb diff --git a/app/models/deleted_dossier.rb b/app/models/deleted_dossier.rb index 25338f6b4..bce5518ee 100644 --- a/app/models/deleted_dossier.rb +++ b/app/models/deleted_dossier.rb @@ -1,7 +1,20 @@ class DeletedDossier < ApplicationRecord belongs_to :procedure - def self.create_from_dossier(dossier) - DeletedDossier.create!(dossier_id: dossier.id, procedure: dossier.procedure, state: dossier.state, deleted_at: Time.zone.now) + enum reason: { + user_request: 'user_request', + manager_request: 'manager_request', + user_removed: 'user_removed', + expired: 'expired' + } + + def self.create_from_dossier(dossier, reason) + create!( + reason: reasons.fetch(reason), + dossier_id: dossier.id, + procedure: dossier.procedure, + state: dossier.state, + deleted_at: Time.zone.now + ) end end diff --git a/db/migrate/20200319103836_add_reason_to_deleted_dossiers.rb b/db/migrate/20200319103836_add_reason_to_deleted_dossiers.rb new file mode 100644 index 000000000..4b882b537 --- /dev/null +++ b/db/migrate/20200319103836_add_reason_to_deleted_dossiers.rb @@ -0,0 +1,5 @@ +class AddReasonToDeletedDossiers < ActiveRecord::Migration[5.2] + def change + add_column :deleted_dossiers, :reason, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index a7a596376..c6ed4c550 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_03_04_155418) do +ActiveRecord::Schema.define(version: 2020_03_19_103836) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -213,6 +213,7 @@ ActiveRecord::Schema.define(version: 2020_03_04_155418) do t.string "state" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.string "reason" t.index ["procedure_id"], name: "index_deleted_dossiers_on_procedure_id" end From 5c2eba1dd18a775ca128ff9c53fcc3dc41252eb6 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 19 Mar 2020 11:53:25 +0100 Subject: [PATCH 2/2] Always add a reason to dossier deletion --- .../manager/dossiers_controller.rb | 2 +- app/controllers/users/dossiers_controller.rb | 2 +- app/models/dossier.rb | 18 ++- app/models/user.rb | 2 +- .../expired_dossiers_deletion_service.rb | 10 +- spec/mailers/dossier_mailer_spec.rb | 4 +- spec/models/dossier_spec.rb | 144 ++++++++++++------ spec/models/user_spec.rb | 8 +- .../expired_dossiers_deletion_service_spec.rb | 2 +- 9 files changed, 119 insertions(+), 73 deletions(-) diff --git a/app/controllers/manager/dossiers_controller.rb b/app/controllers/manager/dossiers_controller.rb index 8fd264a62..c4c076cd8 100644 --- a/app/controllers/manager/dossiers_controller.rb +++ b/app/controllers/manager/dossiers_controller.rb @@ -22,7 +22,7 @@ module Manager def hide dossier = Dossier.find(params[:id]) - dossier.delete_and_keep_track(current_administration) + dossier.delete_and_keep_track!(current_administration, :manager_request) logger.info("Le dossier #{dossier.id} est supprimé par #{current_administration.email}") flash[:notice] = "Le dossier #{dossier.id} a été supprimé." diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index 8d4ffd26a..7a7d0590b 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/app/controllers/users/dossiers_controller.rb @@ -203,7 +203,7 @@ module Users dossier = current_user.dossiers.includes(:user, procedure: :administrateurs).find(params[:id]) if dossier.can_be_deleted_by_user? - dossier.delete_and_keep_track(current_user) + dossier.delete_and_keep_track!(current_user, :user_request) flash.notice = 'Votre dossier a bien été supprimé.' redirect_to dossiers_path else diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 0dca1c033..23fc39016 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -404,19 +404,25 @@ class Dossier < ApplicationRecord end end - def delete_and_keep_track(author) - deleted_dossier = DeletedDossier.create_from_dossier(self) - discard! + def expired_keep_track! + DeletedDossier.create_from_dossier(self, :expired) + log_automatic_dossier_operation(:supprimer, self) + end + def delete_and_keep_track!(author, reason) if en_construction? + deleted_dossier = DeletedDossier.create_from_dossier(self, reason) + administration_emails = followers_instructeurs.present? ? followers_instructeurs.map(&:email) : procedure.administrateurs.map(&:email) administration_emails.each do |email| DossierMailer.notify_deletion_to_administration(deleted_dossier, email).deliver_later end - end - DossierMailer.notify_deletion_to_user(deleted_dossier, user.email).deliver_later + DossierMailer.notify_deletion_to_user(deleted_dossier, user.email).deliver_later - log_dossier_operation(author, :supprimer, self) + log_dossier_operation(author, :supprimer, self) + end + + discard! end def after_passer_en_instruction(instructeur) diff --git a/app/models/user.rb b/app/models/user.rb index 72dc84cc4..8a5793fc4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -120,7 +120,7 @@ class User < ApplicationRecord end dossiers.each do |dossier| - dossier.delete_and_keep_track(administration) + dossier.delete_and_keep_track!(administration, :user_removed) end dossiers.with_discarded.destroy_all destroy! diff --git a/app/services/expired_dossiers_deletion_service.rb b/app/services/expired_dossiers_deletion_service.rb index 6129b5331..2029f08f6 100644 --- a/app/services/expired_dossiers_deletion_service.rb +++ b/app/services/expired_dossiers_deletion_service.rb @@ -65,18 +65,12 @@ class ExpiredDossiersDeletionService ).deliver_later end - dossiers_to_remove.each do |dossier| - DeletedDossier.create_from_dossier(dossier) - dossier.destroy - end + dossiers_to_remove.destroy_all end def self.delete_expired_en_construction_and_notify dossiers_to_remove = Dossier.en_construction_expired - - dossiers_to_remove.each do |dossier| - DeletedDossier.create_from_dossier(dossier) - end + dossiers_to_remove.each(&:expired_keep_track!) dossiers_to_remove .includes(:user) diff --git a/spec/mailers/dossier_mailer_spec.rb b/spec/mailers/dossier_mailer_spec.rb index c277461c8..b9016ac3c 100644 --- a/spec/mailers/dossier_mailer_spec.rb +++ b/spec/mailers/dossier_mailer_spec.rb @@ -98,7 +98,7 @@ RSpec.describe DossierMailer, type: :mailer do describe '.notify_automatic_deletion_to_user' do let(:dossier) { create(:dossier) } - let(:deleted_dossier) { DeletedDossier.create_from_dossier(dossier) } + let(:deleted_dossier) { DeletedDossier.create_from_dossier(dossier, :expired) } before do duree = dossier.procedure.duree_conservation_dossiers_dans_ds @@ -116,7 +116,7 @@ RSpec.describe DossierMailer, type: :mailer do describe '.notify_automatic_deletion_to_administration' do let(:dossier) { create(:dossier) } - let(:deleted_dossier) { DeletedDossier.create_from_dossier(dossier) } + let(:deleted_dossier) { DeletedDossier.create_from_dossier(dossier, :expired) } subject { described_class.notify_automatic_deletion_to_administration([deleted_dossier], dossier.user.email) } diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index da7c114fd..666bbad90 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -661,64 +661,110 @@ describe Dossier do end end - describe "#delete_and_keep_track" do - let(:dossier) { create(:dossier) } - let(:deleted_dossier) { DeletedDossier.find_by!(dossier_id: dossier.id) } + describe "#delete_and_keep_track!" do + let(:dossier) { create(:dossier, :en_construction) } + let(:deleted_dossier) { DeletedDossier.find_by(dossier_id: dossier.id) } let(:last_operation) { dossier.dossier_operation_logs.last } + let(:reason) { :user_request } before do allow(DossierMailer).to receive(:notify_deletion_to_user).and_return(double(deliver_later: nil)) allow(DossierMailer).to receive(:notify_deletion_to_administration).and_return(double(deliver_later: nil)) end - subject! { dossier.delete_and_keep_track(dossier.user) } + subject! { dossier.delete_and_keep_track!(dossier.user, reason) } - it 'hides the dossier' do - expect(dossier.hidden_at).to be_present - end - - it 'creates a DeletedDossier record' do - expect(deleted_dossier.dossier_id).to eq dossier.id - expect(deleted_dossier.procedure).to eq dossier.procedure - expect(deleted_dossier.state).to eq dossier.state - expect(deleted_dossier.deleted_at).to be_present - end - - it 'notifies the user' do - expect(DossierMailer).to have_received(:notify_deletion_to_user).with(deleted_dossier, dossier.user.email) - end - - it 'records the operation in the log' do - expect(last_operation.operation).to eq("supprimer") - expect(last_operation.automatic_operation?).to be_falsey - end - - context 'where instructeurs are following the dossier' do - let(:dossier) { create(:dossier, :en_construction, :followed) } - let!(:non_following_instructeur) do - non_following_instructeur = create(:instructeur) - non_following_instructeur.groupe_instructeurs << dossier.procedure.defaut_groupe_instructeur - non_following_instructeur - end - - it 'notifies the following instructeurs' do - expect(DossierMailer).to have_received(:notify_deletion_to_administration).once - expect(DossierMailer).to have_received(:notify_deletion_to_administration).with(deleted_dossier, dossier.followers_instructeurs.first.email) - end - end - - context 'when there are no following instructeurs' do - let(:dossier) { create(:dossier, :en_construction) } - it 'notifies the procedure administrateur' do - expect(DossierMailer).to have_received(:notify_deletion_to_administration).once - expect(DossierMailer).to have_received(:notify_deletion_to_administration).with(deleted_dossier, dossier.procedure.administrateurs.first.email) - end - end - - context 'when dossier is brouillon' do + context 'brouillon' do let(:dossier) { create(:dossier) } - it 'do not notifies the procedure administrateur' do - expect(DossierMailer).not_to have_received(:notify_deletion_to_administration) + + it 'hides the dossier' do + expect(dossier.discarded?).to be_truthy + end + + it 'do not creates a DeletedDossier record' do + expect(deleted_dossier).to be_nil + end + + it 'do not records the operation in the log' do + expect(last_operation).to be_nil + end + end + + context 'en_construction' do + it 'hides the dossier' do + expect(dossier.hidden_at).to be_present + end + + it 'creates a DeletedDossier record' do + expect(deleted_dossier.reason).to eq DeletedDossier.reasons.fetch(reason) + expect(deleted_dossier.dossier_id).to eq dossier.id + expect(deleted_dossier.procedure).to eq dossier.procedure + expect(deleted_dossier.state).to eq dossier.state + expect(deleted_dossier.deleted_at).to be_present + end + + it 'notifies the user' do + expect(DossierMailer).to have_received(:notify_deletion_to_user).with(deleted_dossier, dossier.user.email) + end + + it 'records the operation in the log' do + expect(last_operation.operation).to eq("supprimer") + expect(last_operation.automatic_operation?).to be_falsey + end + + context 'where instructeurs are following the dossier' do + let(:dossier) { create(:dossier, :en_construction, :followed) } + let!(:non_following_instructeur) do + non_following_instructeur = create(:instructeur) + non_following_instructeur.groupe_instructeurs << dossier.procedure.defaut_groupe_instructeur + non_following_instructeur + end + + it 'notifies the following instructeurs' do + expect(DossierMailer).to have_received(:notify_deletion_to_administration).once + expect(DossierMailer).to have_received(:notify_deletion_to_administration).with(deleted_dossier, dossier.followers_instructeurs.first.email) + end + end + + context 'when there are no following instructeurs' do + let(:dossier) { create(:dossier, :en_construction) } + it 'notifies the procedure administrateur' do + expect(DossierMailer).to have_received(:notify_deletion_to_administration).once + expect(DossierMailer).to have_received(:notify_deletion_to_administration).with(deleted_dossier, dossier.procedure.administrateurs.first.email) + end + end + + context 'when dossier is brouillon' do + let(:dossier) { create(:dossier) } + it 'do not notifies the procedure administrateur' do + expect(DossierMailer).not_to have_received(:notify_deletion_to_administration) + end + end + + context 'with reason: manager_request' do + let(:reason) { :manager_request } + + it 'hides the dossier' do + expect(dossier.discarded?).to be_truthy + end + + it 'records the operation in the log' do + expect(last_operation.operation).to eq("supprimer") + expect(last_operation.automatic_operation?).to be_falsey + end + end + + context 'with reason: user_removed' do + let(:reason) { :user_removed } + + it 'hides the dossier' do + expect(dossier.discarded?).to be_truthy + end + + it 'records the operation in the log' do + expect(last_operation.operation).to eq("supprimer") + expect(last_operation.automatic_operation?).to be_falsey + end end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index c96b769a9..7c6af4d69 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -264,7 +264,7 @@ describe User, type: :model do user.delete_and_keep_track_dossiers(administration) expect(DeletedDossier.find_by(dossier_id: dossier_en_construction)).to be_present - expect(DeletedDossier.find_by(dossier_id: dossier_brouillon)).to be_present + expect(DeletedDossier.find_by(dossier_id: dossier_brouillon)).to be_nil expect(User.find_by(id: user.id)).to be_nil end end @@ -278,16 +278,16 @@ describe User, type: :model do end it "keep track of dossiers and delete user" do - dossier_cache.delete_and_keep_track(administration) + dossier_cache.delete_and_keep_track!(administration, :user_request) user.delete_and_keep_track_dossiers(administration) expect(DeletedDossier.find_by(dossier_id: dossier_en_construction)).to be_present - expect(DeletedDossier.find_by(dossier_id: dossier_brouillon)).to be_present + expect(DeletedDossier.find_by(dossier_id: dossier_brouillon)).to be_nil expect(User.find_by(id: user.id)).to be_nil end it "doesn't destroy dossiers of another user" do - dossier_cache.delete_and_keep_track(administration) + dossier_cache.delete_and_keep_track!(administration, :user_request) user.delete_and_keep_track_dossiers(administration) expect(Dossier.find_by(id: dossier_from_another_user.id)).to be_present diff --git a/spec/services/expired_dossiers_deletion_service_spec.rb b/spec/services/expired_dossiers_deletion_service_spec.rb index 1b7c1088f..b6080a9ae 100644 --- a/spec/services/expired_dossiers_deletion_service_spec.rb +++ b/spec/services/expired_dossiers_deletion_service_spec.rb @@ -34,7 +34,7 @@ describe ExpiredDossiersDeletionService do it 'deletes and notify expired brouillon' do expect(DossierMailer).to have_received(:notify_brouillon_deletion).once expect(DossierMailer).to have_received(:notify_brouillon_deletion).with([expired_brouillon.hash_for_deletion_mail], expired_brouillon.user.email) - expect(DeletedDossier.find_by(dossier_id: expired_brouillon.id)).to be_present + expect(DeletedDossier.find_by(dossier_id: expired_brouillon.id)).not_to be_present expect { expired_brouillon.reload }.to raise_error(ActiveRecord::RecordNotFound) end end