From cee4c5b8fb4ba73bd139d5d969957f45dd8141fe Mon Sep 17 00:00:00 2001 From: Christophe Robillard Date: Thu, 30 Jan 2020 10:48:28 +0100 Subject: [PATCH 1/6] Revert "Revert "4127 fix superadmin supprime compte usager"" This reverts commit 751f24f7bb79c7d9d0af4ef1b7afe7aeabb4cdbc. --- .../manager/administrateurs_controller.rb | 6 +-- .../manager/instructeurs_controller.rb | 14 ++++++ app/controllers/manager/users_controller.rb | 2 +- app/models/administrateur.rb | 13 ++++++ app/models/instructeur.rb | 8 +++- app/models/user.rb | 11 +++-- app/views/manager/instructeurs/show.html.erb | 3 ++ app/views/manager/users/show.html.erb | 2 +- config/routes.rb | 1 + .../manager/instructeurs_controller_spec.rb | 17 +++++++ spec/models/administrateur_spec.rb | 15 +++++++ spec/models/instructeur_spec.rb | 35 ++++++++++++++- spec/models/user_spec.rb | 45 ++++++++++++++++--- 13 files changed, 150 insertions(+), 22 deletions(-) create mode 100644 spec/controllers/manager/instructeurs_controller_spec.rb diff --git a/app/controllers/manager/administrateurs_controller.rb b/app/controllers/manager/administrateurs_controller.rb index f976f3cf6..10dec19d5 100644 --- a/app/controllers/manager/administrateurs_controller.rb +++ b/app/controllers/manager/administrateurs_controller.rb @@ -22,11 +22,7 @@ module Manager def delete administrateur = Administrateur.find(params[:id]) - if !administrateur.can_be_deleted? - fail "Impossible de supprimer cet administrateur car il a des dossiers ou des procédures" - end - administrateur.dossiers.each(&:delete_and_keep_track) - administrateur.destroy + administrateur.delete_and_transfer_services logger.info("L'administrateur #{administrateur.id} est supprimé par #{current_administration.id}") flash[:notice] = "L'administrateur #{administrateur.id} est supprimé" diff --git a/app/controllers/manager/instructeurs_controller.rb b/app/controllers/manager/instructeurs_controller.rb index 82b1e11eb..ed2107c45 100644 --- a/app/controllers/manager/instructeurs_controller.rb +++ b/app/controllers/manager/instructeurs_controller.rb @@ -6,5 +6,19 @@ module Manager flash[:notice] = "Instructeur réinvité." redirect_to manager_instructeur_path(instructeur) end + + def delete + instructeur = Instructeur.find(params[:id]) + + if !instructeur.can_be_deleted? + fail "Impossible de supprimer cet instructeur car il est administrateur ou il est le seul instructeur sur une démarche" + end + instructeur.destroy! + + logger.info("L'instructeur #{instructeur.id} est supprimé par #{current_administration.id}") + flash[:notice] = "L'instructeur #{instructeur.id} est supprimé" + + redirect_to manager_instructeurs_path + end end end diff --git a/app/controllers/manager/users_controller.rb b/app/controllers/manager/users_controller.rb index 9bdbb0449..d9e33fcb9 100644 --- a/app/controllers/manager/users_controller.rb +++ b/app/controllers/manager/users_controller.rb @@ -37,7 +37,7 @@ module Manager def delete user = User.find(params[:id]) if !user.can_be_deleted? - fail "Impossible de supprimer cet utilisateur car il a des dossiers en instruction" + fail "Impossible de supprimer cet utilisateur. Il a des dossiers en instruction ou il est administrateur." end user.delete_and_keep_track_dossiers(current_administration) diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb index 4d4b84d4a..1174d4b60 100644 --- a/app/models/administrateur.rb +++ b/app/models/administrateur.rb @@ -70,4 +70,17 @@ class Administrateur < ApplicationRecord def can_be_deleted? dossiers.state_instruction_commencee.none? && procedures.all? { |p| p.administrateurs.count > 1 } end + + def delete_and_transfer_services + if !can_be_deleted? + fail "Impossible de supprimer cet administrateur car il a des dossiers ou des procédures" + end + dossiers.each(&:delete_and_keep_track) + + procedures.each do |procedure| + next_administrateur = procedure.administrateurs.where.not(id: self.id).first + procedure.service.update(administrateur: next_administrateur) + end + destroy + end end diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb index 4c142b870..97b2b0a49 100644 --- a/app/models/instructeur.rb +++ b/app/models/instructeur.rb @@ -17,9 +17,9 @@ class Instructeur < ApplicationRecord has_many :previously_followed_dossiers, -> { distinct }, through: :previous_follows, source: :dossier has_many :avis has_many :dossiers_from_avis, through: :avis, source: :dossier - has_many :trusted_device_tokens + has_many :trusted_device_tokens, dependent: :destroy - has_one :user + has_one :user, dependent: :nullify default_scope { eager_load(:user) } @@ -176,6 +176,10 @@ class Instructeur < ApplicationRecord trusted_device_token&.token_young? end + def can_be_deleted? + user.administrateur.nil? && procedures.all? { |p| p.defaut_groupe_instructeur.instructeurs.count > 1 } + end + private def annotations_hash(demande, annotations_privees, avis, messagerie) diff --git a/app/models/user.rb b/app/models/user.rb index 4838da20d..35c0edd5e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -97,7 +97,7 @@ class User < ApplicationRecord end def can_be_deleted? - dossiers.state_instruction_commencee.empty? + administrateur.nil? && instructeur.nil? && dossiers.state_instruction_commencee.empty? end def delete_and_keep_track_dossiers(administration) @@ -105,12 +105,11 @@ class User < ApplicationRecord raise "Cannot delete this user because instruction has started for some dossiers" end - if can_be_deleted? - dossiers.each do |dossier| - dossier.delete_and_keep_track(administration) - end - destroy + dossiers.each do |dossier| + dossier.delete_and_keep_track(administration) end + dossiers.unscoped.destroy_all + destroy! end private diff --git a/app/views/manager/instructeurs/show.html.erb b/app/views/manager/instructeurs/show.html.erb index e423414a9..983e171e9 100644 --- a/app/views/manager/instructeurs/show.html.erb +++ b/app/views/manager/instructeurs/show.html.erb @@ -34,7 +34,10 @@ as well as a link to its edit page.
<%= link_to 'Réinviter', reinvite_manager_instructeur_path(instructeur), method: :post, class: 'button' %> +
+ <%= button_to "Supprimer", delete_manager_instructeur_path(page.resource), method: :delete, disabled: !page.resource.can_be_deleted?, class: "button", data: { confirm: "Confirmez-vous la suppression de l'instructeur ?" }, title: page.resource.can_be_deleted? ? "Supprimer" : "Cet instructeur est administrateur ou a des démarches dont il est le seul instructeur et ne peut être supprimé" %> +
diff --git a/app/views/manager/users/show.html.erb b/app/views/manager/users/show.html.erb index bf49d4076..a14acc228 100644 --- a/app/views/manager/users/show.html.erb +++ b/app/views/manager/users/show.html.erb @@ -28,7 +28,7 @@ as well as a link to its edit page. <%= button_to "modifier", edit_manager_user_path(page.resource), method: :get, class: "button" %>
- <%= button_to "supprimer", delete_manager_user_path(page.resource), method: :delete, disabled: !page.resource.can_be_deleted?, class: "button", data: { confirm: "Confirmez-vous la suppression de l'utilisateur ?" }, title: page.resource.can_be_deleted? ? "Supprimer" : "Cet utilisateur a des dossiers dont l'instruction a commencé et ne peut être supprimé" %> + <%= button_to "supprimer", delete_manager_user_path(page.resource), method: :delete, disabled: !page.resource.can_be_deleted?, class: "button", data: { confirm: "Confirmez-vous la suppression de l'utilisateur ?" }, title: page.resource.can_be_deleted? ? "Supprimer" : "Cet utilisateur ne peut être supprimé. Il a des dossiers dont l'instruction a commencé ou il est administrateur ou instructeur" %>
<% if !user.confirmed? %> diff --git a/config/routes.rb b/config/routes.rb index 7feeaadb4..30d4a3402 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -32,6 +32,7 @@ Rails.application.routes.draw do resources :instructeurs, only: [:index, :show] do post 'reinvite', on: :member + delete 'delete', on: :member end resources :dossiers, only: [:show] diff --git a/spec/controllers/manager/instructeurs_controller_spec.rb b/spec/controllers/manager/instructeurs_controller_spec.rb new file mode 100644 index 000000000..3a0164d4b --- /dev/null +++ b/spec/controllers/manager/instructeurs_controller_spec.rb @@ -0,0 +1,17 @@ +describe Manager::InstructeursController, type: :controller do + let(:administration) { create(:administration) } + + describe '#delete' do + let!(:instructeur) { create(:instructeur) } + + before { sign_in administration } + + subject { delete :delete, params: { id: instructeur.id } } + + it 'deletes the instructeur' do + subject + + expect(Instructeur.find_by(id: instructeur.id)).to be_nil + end + end +end diff --git a/spec/models/administrateur_spec.rb b/spec/models/administrateur_spec.rb index 9f0c30923..3e6864466 100644 --- a/spec/models/administrateur_spec.rb +++ b/spec/models/administrateur_spec.rb @@ -46,6 +46,21 @@ describe Administrateur, type: :model do end end + describe '#delete_and_transfer_services' do + let!(:administrateur) { create(:administrateur) } + let!(:autre_administrateur) { create(:administrateur) } + let!(:procedure) { create(:procedure, :with_service, administrateurs: [administrateur, autre_administrateur]) } + let(:service) { procedure.service } + + it "delete and transfer services to other admin" do + service.update(administrateur: administrateur) + administrateur.delete_and_transfer_services + + expect(Administrateur.find_by(id: administrateur.id)).to be_nil + expect(service.reload.administrateur).to eq(autre_administrateur) + end + end + # describe '#password_complexity' do # let(:email) { 'mail@beta.gouv.fr' } # let(:passwords) { ['pass', '12pass23', 'démarches ', 'démarches-simple', 'démarches-simplifiées-pwd'] } diff --git a/spec/models/instructeur_spec.rb b/spec/models/instructeur_spec.rb index 37917bf18..78737bf89 100644 --- a/spec/models/instructeur_spec.rb +++ b/spec/models/instructeur_spec.rb @@ -424,9 +424,40 @@ describe Instructeur, type: :model do it { expect(instructeur_a.procedures.all.to_ary).to eq([procedure_a]) } end + describe "#can_be_deleted?" do + subject { instructeur.can_be_deleted? } + + context 'when the instructeur is an administrateur' do + let!(:administrateur) { create(:administrateur) } + let(:instructeur) { administrateur.instructeur } + + it { is_expected.to be false } + end + + context "when the instructeur's procedures have other instructeurs" do + let(:instructeur_not_admin) { create(:instructeur) } + let(:autre_instructeur) { create(:instructeur) } + + it "can be deleted" do + assign(procedure, instructeur_assigne: instructeur_not_admin) + assign(procedure, instructeur_assigne: autre_instructeur) + expect(autre_instructeur.can_be_deleted?).to be_truthy + end + end + + context "when the instructeur's procedures is the only one" do + let(:instructeur_not_admin) { create :instructeur } + let(:autre_procedure) { create :procedure } + it "can be deleted" do + assign(autre_procedure, instructeur_assigne: instructeur_not_admin) + expect(instructeur_not_admin.can_be_deleted?).to be_falsy + end + end + end + private - def assign(procedure_to_assign) - create :assign_to, instructeur: instructeur, procedure: procedure_to_assign, groupe_instructeur: procedure_to_assign.defaut_groupe_instructeur + def assign(procedure_to_assign, instructeur_assigne: instructeur) + create :assign_to, instructeur: instructeur_assigne, procedure: procedure_to_assign, groupe_instructeur: procedure_to_assign.defaut_groupe_instructeur end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index b127d35c6..19b317f40 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -224,6 +224,24 @@ describe User, type: :model do context 'when the user has no dossier in instruction' do it { is_expected.to be true } end + + context 'when the user is an administrateur' do + it 'cannot be deleted' do + administrateur = create(:administrateur) + user = administrateur.user + + expect(user.can_be_deleted?).to be_falsy + end + end + + context 'when the user is an instructeur' do + it 'cannot be deleted' do + instructeur = create(:instructeur) + user = instructeur.user + + expect(user.can_be_deleted?).to be_falsy + end + end end describe '#delete_and_keep_track_dossiers' do @@ -241,12 +259,29 @@ describe User, type: :model do let!(:dossier_en_construction) { create(:dossier, :en_construction, user: user) } let!(:dossier_brouillon) { create(:dossier, user: user) } - it "keep track of dossiers and delete user" do - user.delete_and_keep_track_dossiers(administration) + context 'without a hidden dossier' do + it "keep track of dossiers and delete user" 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(User.find_by(id: user.id)).to be_nil + expect(DeletedDossier.find_by(dossier_id: dossier_en_construction)).to be_present + expect(DeletedDossier.find_by(dossier_id: dossier_brouillon)).to be_present + expect(User.find_by(id: user.id)).to be_nil + end + end + + context 'with a hidden dossier' do + let!(:dossier_cache) do + create(:dossier, :en_construction, user: user) + end + + it "keep track of dossiers and delete user" do + dossier_cache.delete_and_keep_track(administration) + 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(User.find_by(id: user.id)).to be_nil + end end end end From 6fc8a27bd74c5952b1552895254a3326444cfa76 Mon Sep 17 00:00:00 2001 From: Christophe Robillard Date: Thu, 30 Jan 2020 11:18:01 +0100 Subject: [PATCH 2/6] destroys not all dossiers but only dossiers for a specific user --- app/models/user.rb | 2 +- spec/models/user_spec.rb | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index 35c0edd5e..6d2c6db70 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -108,7 +108,7 @@ class User < ApplicationRecord dossiers.each do |dossier| dossier.delete_and_keep_track(administration) end - dossiers.unscoped.destroy_all + dossiers.with_hidden.destroy_all destroy! end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 19b317f40..2de7cfccd 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -273,6 +273,9 @@ describe User, type: :model do let!(:dossier_cache) do create(:dossier, :en_construction, user: user) end + let!(:dossier_from_another_user) do + create(:dossier, :en_construction, user: create(:user)) + end it "keep track of dossiers and delete user" do dossier_cache.delete_and_keep_track(administration) @@ -282,6 +285,13 @@ describe User, type: :model do expect(DeletedDossier.find_by(dossier_id: dossier_brouillon)).to be_present 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) + user.delete_and_keep_track_dossiers(administration) + + expect(Dossier.find_by(id: dossier_from_another_user.id)).not_to be_nil + end end end end From 4bf020cb9602dc330bf2ee5978d66e849995cc4f Mon Sep 17 00:00:00 2001 From: Christophe Robillard Date: Mon, 3 Feb 2020 16:14:35 +0100 Subject: [PATCH 3/6] =?UTF-8?q?rend=20le=20test=20plus=20=C3=A9l=C3=A9gant?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- spec/models/user_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 2de7cfccd..1af6ae573 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -290,7 +290,7 @@ describe User, type: :model do dossier_cache.delete_and_keep_track(administration) user.delete_and_keep_track_dossiers(administration) - expect(Dossier.find_by(id: dossier_from_another_user.id)).not_to be_nil + expect(Dossier.find_by(id: dossier_from_another_user.id)).to be_present end end end From 5a46effcbc8d714a31825a9dbb896dfb3c69cec6 Mon Sep 17 00:00:00 2001 From: Christophe Robillard Date: Mon, 3 Feb 2020 16:33:47 +0100 Subject: [PATCH 4/6] remove useless condition to admin that can be deleted administrateur can be deleted only if he/she has a procedure where he/she is the only admin --- app/models/administrateur.rb | 5 ++--- app/views/manager/administrateurs/show.html.erb | 2 +- spec/models/administrateur_spec.rb | 14 +++++++------- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb index 1174d4b60..195352b6f 100644 --- a/app/models/administrateur.rb +++ b/app/models/administrateur.rb @@ -68,14 +68,13 @@ class Administrateur < ApplicationRecord end def can_be_deleted? - dossiers.state_instruction_commencee.none? && procedures.all? { |p| p.administrateurs.count > 1 } + procedures.all? { |p| p.administrateurs.count > 1 } end def delete_and_transfer_services if !can_be_deleted? - fail "Impossible de supprimer cet administrateur car il a des dossiers ou des procédures" + fail "Impossible de supprimer cet administrateur car il a des procédures où il est le seul administrateur" end - dossiers.each(&:delete_and_keep_track) procedures.each do |procedure| next_administrateur = procedure.administrateurs.where.not(id: self.id).first diff --git a/app/views/manager/administrateurs/show.html.erb b/app/views/manager/administrateurs/show.html.erb index 73019d37f..650f6ab60 100644 --- a/app/views/manager/administrateurs/show.html.erb +++ b/app/views/manager/administrateurs/show.html.erb @@ -36,7 +36,7 @@ as well as a link to its edit page. <% if page.resource.invitation_expired? %> <%= link_to "renvoyer l'invitation", reinvite_manager_administrateur_path(page.resource), method: :post, class: "button" %> <% end %> - <%= button_to "supprimer", delete_manager_administrateur_path(page.resource), method: :delete, disabled: !page.resource.can_be_deleted?, class: "button", data: { confirm: "Confirmez-vous la suppression de l'administrateur ?" }, title: page.resource.can_be_deleted? ? "Supprimer" : "Cet administrateur a des dossiers ou des procédures dont il est le seul admin et ne peut être supprimé" %> + <%= button_to "supprimer", delete_manager_administrateur_path(page.resource), method: :delete, disabled: !page.resource.can_be_deleted?, class: "button", data: { confirm: "Confirmez-vous la suppression de l'administrateur ?" }, title: page.resource.can_be_deleted? ? "Supprimer" : "Cet administrateur a des procédures dont il est le seul admin et ne peut être supprimé" %>
diff --git a/spec/models/administrateur_spec.rb b/spec/models/administrateur_spec.rb index 3e6864466..b252dc42b 100644 --- a/spec/models/administrateur_spec.rb +++ b/spec/models/administrateur_spec.rb @@ -24,13 +24,6 @@ describe Administrateur, type: :model do describe "#can_be_deleted?" do subject { administrateur.can_be_deleted? } - context 'when the administrateur has a dossier in instruction' do - let!(:dossier) { create(:dossier, :en_instruction) } - let(:administrateur) { dossier.procedure.administrateurs.first } - - it { is_expected.to be false } - end - context "when the administrateur's procedures have other administrateurs" do let!(:administrateur) { create(:administrateur) } let!(:autre_administrateur) { create(:administrateur) } @@ -39,6 +32,13 @@ describe Administrateur, type: :model do it { is_expected.to be true } end + context "when the administrateur has a procedure where he/she is the only admin" do + let!(:administrateur) { create(:administrateur) } + let!(:procedure) { create(:procedure, administrateurs: [administrateur]) } + + it { is_expected.to be false } + end + context "when the administrateur has no procedure" do let!(:administrateur) { create(:administrateur) } From deb11f281f415e36a3165ff17a2fcbd7f066c30c Mon Sep 17 00:00:00 2001 From: Christophe Robillard Date: Mon, 3 Feb 2020 16:37:18 +0100 Subject: [PATCH 5/6] remove useless relation between administrateur and dossiers --- app/models/administrateur.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb index 195352b6f..04323eb97 100644 --- a/app/models/administrateur.rb +++ b/app/models/administrateur.rb @@ -7,7 +7,6 @@ class Administrateur < ApplicationRecord has_many :administrateurs_procedures has_many :procedures, through: :administrateurs_procedures has_many :services - has_many :dossiers, -> { state_not_brouillon }, through: :procedures has_one :user, dependent: :nullify From c137917396c8e42cd0f53f23561ee598e3515bc1 Mon Sep 17 00:00:00 2001 From: Christophe Robillard Date: Mon, 3 Feb 2020 17:46:24 +0100 Subject: [PATCH 6/6] fix typo --- app/models/administrateur.rb | 2 +- app/views/manager/administrateurs/show.html.erb | 2 +- spec/models/administrateur_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb index 04323eb97..90bf8e8f7 100644 --- a/app/models/administrateur.rb +++ b/app/models/administrateur.rb @@ -72,7 +72,7 @@ class Administrateur < ApplicationRecord def delete_and_transfer_services if !can_be_deleted? - fail "Impossible de supprimer cet administrateur car il a des procédures où il est le seul administrateur" + fail "Impossible de supprimer cet administrateur car il a des démarches où il est le seul administrateur" end procedures.each do |procedure| diff --git a/app/views/manager/administrateurs/show.html.erb b/app/views/manager/administrateurs/show.html.erb index 650f6ab60..b33304288 100644 --- a/app/views/manager/administrateurs/show.html.erb +++ b/app/views/manager/administrateurs/show.html.erb @@ -36,7 +36,7 @@ as well as a link to its edit page. <% if page.resource.invitation_expired? %> <%= link_to "renvoyer l'invitation", reinvite_manager_administrateur_path(page.resource), method: :post, class: "button" %> <% end %> - <%= button_to "supprimer", delete_manager_administrateur_path(page.resource), method: :delete, disabled: !page.resource.can_be_deleted?, class: "button", data: { confirm: "Confirmez-vous la suppression de l'administrateur ?" }, title: page.resource.can_be_deleted? ? "Supprimer" : "Cet administrateur a des procédures dont il est le seul admin et ne peut être supprimé" %> + <%= button_to "supprimer", delete_manager_administrateur_path(page.resource), method: :delete, disabled: !page.resource.can_be_deleted?, class: "button", data: { confirm: "Confirmez-vous la suppression de l'administrateur ?" }, title: page.resource.can_be_deleted? ? "Supprimer" : "Cet administrateur a des démarches dont il est le seul admin et ne peut être supprimé" %> diff --git a/spec/models/administrateur_spec.rb b/spec/models/administrateur_spec.rb index b252dc42b..3b1a023cd 100644 --- a/spec/models/administrateur_spec.rb +++ b/spec/models/administrateur_spec.rb @@ -32,7 +32,7 @@ describe Administrateur, type: :model do it { is_expected.to be true } end - context "when the administrateur has a procedure where he/she is the only admin" do + context "when the administrateur has a procedure where they is the only admin" do let!(:administrateur) { create(:administrateur) } let!(:procedure) { create(:procedure, administrateurs: [administrateur]) }