From 0b7d4452fda074fac67e9afe3c12d0704902b7ae Mon Sep 17 00:00:00 2001 From: Christophe Robillard Date: Thu, 5 Oct 2023 11:23:46 +0200 Subject: [PATCH 1/2] remove admin from manager --- .../manager/administrateurs_controller.rb | 11 +++- app/models/administrateur.rb | 2 +- .../administrateur_deletion_service.rb | 65 +++++++++++++++++++ config/locales/views/manager/fr.yml | 10 +++ spec/models/administrateur_spec.rb | 19 +++++- .../administrateur_deletion_service_spec.rb | 63 ++++++++++++++++++ 6 files changed, 164 insertions(+), 6 deletions(-) create mode 100644 app/services/administrateur_deletion_service.rb create mode 100644 spec/services/administrateur_deletion_service_spec.rb diff --git a/app/controllers/manager/administrateurs_controller.rb b/app/controllers/manager/administrateurs_controller.rb index 53f436f30..befabd759 100644 --- a/app/controllers/manager/administrateurs_controller.rb +++ b/app/controllers/manager/administrateurs_controller.rb @@ -22,10 +22,15 @@ module Manager def delete administrateur = Administrateur.find(params[:id]) - administrateur.delete_and_transfer_services + result = AdministrateurDeletionService.new(current_super_admin, administrateur).call - logger.info("L'administrateur #{administrateur.id} est supprimé par #{current_super_admin.id}") - flash[:notice] = "L'administrateur #{administrateur.id} est supprimé" + case result + in Dry::Monads::Result::Success + logger.info("L'administrateur #{administrateur.id} est supprimé par #{current_super_admin.id}") + flash[:notice] = "L'administrateur #{administrateur.id} est supprimé" + in Dry::Monads::Result::Failure(reason) + flash[:alert] = I18n.t(reason, scope: "manager.administrateurs.delete") + end redirect_to manager_administrateurs_path end diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb index 0f6e1e6de..e0af22bbc 100644 --- a/app/models/administrateur.rb +++ b/app/models/administrateur.rb @@ -70,7 +70,7 @@ class Administrateur < ApplicationRecord end def can_be_deleted? - procedures.all? { |p| p.administrateurs.count > 1 } + procedures.all? { |p| p.administrateurs.count > 1 || p.dossiers.empty? } end def delete_and_transfer_services diff --git a/app/services/administrateur_deletion_service.rb b/app/services/administrateur_deletion_service.rb new file mode 100644 index 000000000..0b4809384 --- /dev/null +++ b/app/services/administrateur_deletion_service.rb @@ -0,0 +1,65 @@ +class AdministrateurDeletionService + include Dry::Monads[:result] + + attr_reader :super_admin, :admin, :owned_procedures, :shared_procedures + + def initialize(super_admin, admin) + @super_admin = super_admin + @admin = admin + @owned_procedures, @shared_procedures = admin + .procedures + .with_discarded + .partition { _1.administrateurs.one? } + end + + def call + return Failure(:cannot_be_deleted) unless admin.can_be_deleted? + + result = nil + + ApplicationRecord.transaction do + delete_admin_from_shared_procedures + delete_procedures_without_dossier + + if admin.procedures.with_discarded.count.positive? + result = Failure(:still_procedures) + raise ActiveRecord::Rollback + end + + delete_services_without_procedures + transfer_services + if admin.services.count.positive? + result = Failure(:still_services) + raise ActiveRecord::Rollback + end + + admin.destroy! + result = Success(:ok) + end + result + end + + private + + def delete_admin_from_shared_procedures + @shared_procedures.each { _1.administrateurs.delete(admin) } + end + + def delete_procedures_without_dossier + procedures_without_dossier = owned_procedures.filter { _1.dossiers.empty? } + procedures_without_dossier.each { _1.discard_and_keep_track!(super_admin) } + procedures_without_dossier.each(&:purge_discarded) + end + + def delete_services_without_procedures + admin.services.filter { _1.procedures.with_discarded.count.zero? }.each(&:destroy) + end + + def transfer_services + shared_procedures.each do |procedure| + next if procedure.service.nil? + next_admin = procedure.administrateurs.where.not(id: admin.id).first + procedure.service.update(administrateur: next_admin) + end + end +end diff --git a/config/locales/views/manager/fr.yml b/config/locales/views/manager/fr.yml index 5898e2120..378148b02 100644 --- a/config/locales/views/manager/fr.yml +++ b/config/locales/views/manager/fr.yml @@ -2,3 +2,13 @@ fr: manager: gestionnaires: manage_root_groupe_gestionnaire: Gestion du groupe racine + groupe_gestionnaires: + add_gestionnaire: + wrong_address: + one: "%{emails} n’est pas une adresse email valide" + other: "%{emails} ne sont pas des adresses emails valides" + administrateurs: + delete: + cannot_be_deleted: "L'administrateur ne peut pas être supprimé" + still_procedures: "L'administrateur ne peut pas être supprimé car il a encore des démarches" + still_services: "L'administrateur ne peut pas être supprimé car il a encore des services" diff --git a/spec/models/administrateur_spec.rb b/spec/models/administrateur_spec.rb index 958d9016e..7750b59a1 100644 --- a/spec/models/administrateur_spec.rb +++ b/spec/models/administrateur_spec.rb @@ -17,11 +17,26 @@ describe Administrateur, type: :model do it { is_expected.to be true } end - context "when the administrateur has a procedure where they is the only admin" do + context "when the administrateur has a procedure with dossiers where they is the only admin" do + let!(:administrateur) { create(:administrateur) } + let!(:procedure) { create(:procedure_with_dossiers, administrateurs: [administrateur]) } + + it { is_expected.to be false } + end + + context "when the administrateur has a procedure with dossiers and with other admins" do + let!(:administrateur) { create(:administrateur) } + let!(:administrateur2) { create(:administrateur) } + let!(:procedure) { create(:procedure_with_dossiers, administrateurs: [administrateur, administrateur2]) } + + it { is_expected.to be true } + end + + context "when the administrateur has a procedure without dossiers" do let!(:administrateur) { create(:administrateur) } let!(:procedure) { create(:procedure, administrateurs: [administrateur]) } - it { is_expected.to be false } + it { is_expected.to be true } end context "when the administrateur has no procedure" do diff --git a/spec/services/administrateur_deletion_service_spec.rb b/spec/services/administrateur_deletion_service_spec.rb new file mode 100644 index 000000000..2a64392cb --- /dev/null +++ b/spec/services/administrateur_deletion_service_spec.rb @@ -0,0 +1,63 @@ +describe AdministrateurDeletionService do + let(:super_admin) { create(:super_admin) } + let(:admin) { create(:administrateur) } + let(:service) { create(:service, administrateur: admin) } + let(:other_admin) { create(:administrateur) } + let(:procedure) { create(:procedure, service: service, administrateurs: [admin, other_admin]) } + let(:owned_procedure_service) { create(:service, administrateur: admin) } + let(:owned_procedure) { create(:procedure, service: owned_procedure_service, administrateurs: [admin]) } + + describe '#call' do + subject { AdministrateurDeletionService.new(super_admin, admin).call } + + context 'when admin can be deleted' do + it 'removes admin procedures without dossiers' do + owned_procedure + subject + expect(Procedure.find_by(id: owned_procedure.id)).to be_nil + end + + it 'removes service admins without procedure' do + owned_procedure + subject + expect(Service.find_by(id: owned_procedure_service.id)).to be_nil + end + + it 'transfer services to other admin' do + procedure + subject + expect(procedure.service.administrateur).to eq other_admin + end + + it 'deletes admin' do + procedure + owned_procedure + subject + expect(Administrateur.find_by(id: admin.id)).to be_nil + end + end + + context 'when admin has some procedures with dossiers and only one admin' do + let(:owned_procedure_with_dossier) { create(:procedure_with_dossiers, service: owned_procedure_service, administrateurs: [admin]) } + + it "doesn't destroy admin" do + owned_procedure_with_dossier + subject + expect(Administrateur.find_by(id: admin.id)).to eq admin + expect(subject.failure).to eq :cannot_be_deleted + end + end + + context "when there is a failure" do + it 'rollbacks' do + allow_any_instance_of(Service).to receive(:update).and_return(false) + procedure + owned_procedure + subject + expect(subject.failure).to eq :still_services + expect(admin.procedures.count).to eq 2 + expect(procedure.administrateurs).to include(admin) + end + end + end +end From 1869d6b9102b4b21777a254dff3b7baeb122ad3b Mon Sep 17 00:00:00 2001 From: Christophe Robillard Date: Wed, 18 Oct 2023 11:43:21 +0200 Subject: [PATCH 2/2] bugfix: have to repeatedly press confirm button when removing accounts --- app/views/manager/administrateurs/show.html.erb | 2 +- app/views/manager/instructeurs/show.html.erb | 2 +- app/views/manager/users/show.html.erb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/manager/administrateurs/show.html.erb b/app/views/manager/administrateurs/show.html.erb index b0d47c512..ab68a1f80 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 démarches 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: { turbo_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/app/views/manager/instructeurs/show.html.erb b/app/views/manager/instructeurs/show.html.erb index 0a043f377..ba6af8ea7 100644 --- a/app/views/manager/instructeurs/show.html.erb +++ b/app/views/manager/instructeurs/show.html.erb @@ -36,7 +36,7 @@ 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é" %> + <%= button_to "Supprimer", delete_manager_instructeur_path(page.resource), method: :delete, disabled: !page.resource.can_be_deleted?, class: "button", data: { turbo_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 03e68bc26..9a0fa57ee 100644 --- a/app/views/manager/users/show.html.erb +++ b/app/views/manager/users/show.html.erb @@ -29,7 +29,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 ne peut être supprimé. Il a des dossiers dont l'instruction a commencé ou il est administrateur ou instructeur" %> + <%= button_to "supprimer", delete_manager_user_path(page.resource), method: :delete, disabled: !page.resource.can_be_deleted?, class: "button", data: { turbo_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? %>