Merge pull request #6552 from betagouv/a_super_admin_can_merge_admin

Un super administrateur peut changer l'email d'un administrateur

app/controllers/manager/users_controller.rb

@@ -2,11 +2,9 @@ module Manager
   class UsersController < Manager::ApplicationController
     def update
       user = User.find(params[:id])
-      preexisting_user = User.find_by(email: targeted_email)
+      targeted_user = User.find_by(email: targeted_email)
 
-      if user.administrateur.present?
-        flash[:error] = "« #{targeted_email} » est un administrateur. On ne sait pas encore faire."
-      elsif preexisting_user.nil?
+      if targeted_user.nil?
         user.skip_reconfirmation!
         user.update(email: targeted_email)
 
@@ -16,18 +14,18 @@ module Manager
           flash[:error] = user.errors.full_messages.to_sentence
         end
       else
-        user.dossiers.update_all(user_id: preexisting_user.id)
+        user.dossiers.update_all(user_id: targeted_user.id)
 
-        if preexisting_user.instructeur.nil?
-          user.instructeur&.update(user: preexisting_user)
-        else
-          preexisting_user.instructeur.merge(user.instructeur)
-        end
-
-        if preexisting_user.expert.nil?
-          user.expert&.update(user: preexisting_user)
-        else
-          preexisting_user.expert.merge(user.expert)
+        [
+          [user.instructeur, targeted_user.instructeur],
+          [user.expert, targeted_user.expert],
+          [user.administrateur, targeted_user.administrateur]
+        ].each do |old_role, targeted_role|
+          if targeted_role.nil?
+            old_role&.update(user: targeted_user)
+          else
+            targeted_role.merge(old_role)
+          end
         end
 
         flash[:notice] = "Le compte « #{targeted_email} » a absorbé le compte « #{user.email} »."

app/models/administrateur.rb

@@ -96,6 +96,36 @@ class Administrateur < ApplicationRecord
     destroy
   end
 
+  def merge(old_admin)
+    return if old_admin.nil?
+
+    procedures_with_new_admin, procedures_without_new_admin = old_admin.procedures
+      .partition { |p| p.administrateurs.exists?(id) }
+
+    procedures_with_new_admin.each do |p|
+      p.administrateurs.delete(old_admin)
+    end
+
+    procedures_without_new_admin.each do |p|
+      p.administrateurs << self
+      p.administrateurs.delete(old_admin)
+    end
+
+    old_admin.services.update_all(administrateur_id: id)
+
+    instructeurs_with_new_admin, instructeurs_without_new_admin = old_admin.instructeurs
+      .partition { |i| i.administrateurs.exists?(id) }
+
+    instructeurs_with_new_admin.each do |i|
+      i.administrateurs.delete(old_admin)
+    end
+
+    instructeurs_without_new_admin.each do |i|
+      i.administrateurs << self
+      i.administrateurs.delete(old_admin)
+    end
+  end
+
   # required to display feature flags field in manager
   def features
   end

spec/controllers/manager/users_controller_spec.rb

@@ -17,7 +17,7 @@ describe Manager::UsersController, type: :controller do
   end
 
   describe '#update' do
-    let(:user) { create(:user, email: 'ancien.email@domaine.fr') }
+    let(:user) { create(:user, email: 'ancien.email@domaine.fr', password: '{My-$3cure-p4ssWord}') }
 
     subject { patch :update, params: { id: user.id, user: { email: nouvel_email } } }
 
@@ -30,17 +30,6 @@ describe Manager::UsersController, type: :controller do
 
           expect(User.find_by(id: user.id).email).to eq(nouvel_email)
         end
-
-        context 'and the user is an administrateur' do
-          let(:user) { create(:administrateur).user }
-
-          it 'rejects the modification' do
-            subject
-
-            expect(flash[:error]).to match("« nouvel.email@domaine.fr » est un administrateur. On ne sait pas encore faire.")
-            expect(user.reload.email).not_to eq(nouvel_email)
-          end
-        end
       end
 
       describe 'with an invalid email' do
@@ -56,8 +45,8 @@ describe Manager::UsersController, type: :controller do
     end
 
     context 'when the targeted email exists' do
-      let(:preexisting_user) { create(:user, email: 'email.existant@domaine.fr') }
-      let(:nouvel_email) { preexisting_user.email }
+      let(:targeted_user) { create(:user, email: 'email.existant@domaine.fr', password: '{My-$3cure-p4ssWord}') }
+      let(:nouvel_email) { targeted_user.email }
 
       context 'and the old account has a dossier' do
         let!(:dossier) { create(:dossier, user: user) }
@@ -65,63 +54,35 @@ describe Manager::UsersController, type: :controller do
         it 'transfers the dossier' do
           subject
 
-          expect(preexisting_user.dossiers).to match([dossier])
+          expect(targeted_user.dossiers).to match([dossier])
         end
       end
 
-      context 'and the old account belongs to an instructeur and expert' do
+      context 'and the old account belongs to an instructeur, expert and administrateur' do
         let!(:instructeur) { create(:instructeur, user: user) }
         let!(:expert) { create(:expert, user: user) }
+        let!(:administrateur) { create(:administrateur, user: user) }
 
         it 'transfers instructeur account' do
           subject
-          preexisting_user.reload
+          targeted_user.reload
 
-          expect(preexisting_user.instructeur).to match(instructeur)
-          expect(preexisting_user.expert).to match(expert)
+          expect(targeted_user.instructeur).to match(instructeur)
+          expect(targeted_user.expert).to match(expert)
+          expect(targeted_user.administrateur).to match(administrateur)
           expect(flash[:notice]).to match("Le compte « email.existant@domaine.fr » a absorbé le compte « ancien.email@domaine.fr ».")
         end
 
-        context 'and the preexisting account owns an instructeur and expert as well' do
-          let!(:preexisting_instructeur) { create(:instructeur, user: preexisting_user) }
-          let!(:preexisting_expert) { create(:expert, user: preexisting_user) }
+        context 'and the targeted account owns an instructeur and expert as well' do
+          let!(:targeted_instructeur) { create(:instructeur, user: targeted_user) }
+          let!(:targeted_expert) { create(:expert, user: targeted_user) }
+          let!(:targeted_administrateur) { create(:administrateur, user: targeted_user) }
 
-          context 'and the source instructeur has some procedures and dossiers' do
-            let!(:procedure) { create(:procedure, instructeurs: [instructeur]) }
-            let(:dossier) { create(:dossier) }
-            let(:administrateur) { create(:administrateur) }
-            let!(:commentaire) { create(:commentaire, instructeur: instructeur, dossier: dossier) }
-            let!(:bulk_message) { BulkMessage.create!(instructeur: instructeur, body: 'body', sent_at: Time.zone.now) }
-
-            before do
-              user.instructeur.followed_dossiers << dossier
-              user.instructeur.administrateurs << administrateur
-            end
-
-            it 'transferts all the stuff' do
-              subject
-              preexisting_user.reload
-
-              expect(procedure.instructeurs).to match([preexisting_user.instructeur])
-              expect(preexisting_user.instructeur.followed_dossiers).to match([dossier])
-              expect(preexisting_user.instructeur.administrateurs).to match([administrateur])
-              expect(preexisting_user.instructeur.commentaires).to match([commentaire])
-              expect(preexisting_user.instructeur.bulk_messages).to match([bulk_message])
-            end
-          end
-
-          context 'and the source expert has some avis and commentaires' do
-            let(:dossier) { create(:dossier) }
-            let(:experts_procedure) { create(:experts_procedure, expert: user.expert, procedure: dossier.procedure) }
-            let!(:avis) { create(:avis, dossier: dossier, claimant: create(:instructeur), experts_procedure: experts_procedure) }
-            let!(:commentaire) { create(:commentaire, expert: expert, dossier: dossier) }
-
-            it 'transfers the avis' do
-              subject
-
-              expect(preexisting_user.expert.avis).to match([avis])
-              expect(preexisting_user.expert.commentaires).to match([commentaire])
-            end
+          it 'merge the account' do
+            expect_any_instance_of(Instructeur).to receive(:merge)
+            expect_any_instance_of(Expert).to receive(:merge)
+            expect_any_instance_of(Administrateur).to receive(:merge)
+            subject
           end
         end
       end

spec/models/administrateur_spec.rb

@@ -75,4 +75,92 @@ describe Administrateur, type: :model do
       expect(Administrateur.find_by(id: administrateur.id)).to be_nil
     end
   end
+
+  describe '#merge' do
+    let(:new_admin) { create(:administrateur) }
+    let(:old_admin) { create(:administrateur) }
+
+    subject { new_admin.merge(old_admin) }
+
+    context 'when the old admin does not exist' do
+      let(:old_admin) { nil }
+
+      it { expect { subject }.not_to raise_error }
+    end
+
+    context 'when the old admin has a procedure' do
+      let(:procedure) { create(:procedure) }
+
+      before do
+        old_admin.procedures << procedure
+        subject
+        [new_admin, old_admin].map(&:reload)
+      end
+
+      it 'transfers the procedure' do
+        expect(new_admin.procedures).to match_array(procedure)
+        expect(old_admin.procedures).to be_empty
+      end
+    end
+
+    context 'when both admins share a procedure' do
+      let(:procedure) { create(:procedure) }
+
+      before do
+        new_admin.procedures << procedure
+        old_admin.procedures << procedure
+        subject
+        [new_admin, old_admin].map(&:reload)
+      end
+
+      it 'removes the procedure from the old one' do
+        expect(old_admin.procedures).to be_empty
+      end
+    end
+
+    context 'when the old admin has a service' do
+      let(:service) { create(:service, administrateur: old_admin) }
+
+      before do
+        service
+        subject
+        [new_admin, old_admin].map(&:reload)
+      end
+
+      it 'transfers the service' do
+        expect(new_admin.services).to match_array(service)
+      end
+    end
+
+    context 'when the old admin has an instructeur' do
+      let(:instructeur) { create(:instructeur) }
+
+      before do
+        old_admin.instructeurs << instructeur
+        subject
+        [new_admin, old_admin].map(&:reload)
+      end
+
+      it 'transfers the instructeur' do
+        expect(new_admin.instructeurs).to match_array(instructeur)
+        expect(old_admin.instructeurs).to be_empty
+      end
+    end
+
+    context 'when both admins share an instructeur' do
+      let(:instructeur) { create(:instructeur) }
+
+      before do
+        old_admin.instructeurs << instructeur
+        new_admin.instructeurs << instructeur
+        subject
+        [new_admin, old_admin].map(&:reload)
+      end
+
+      it 'transfers the instructeur' do
+        expect(new_admin.instructeurs).to match_array(instructeur)
+        expect(old_admin.instructeurs).to be_empty
+      end
+    end
+  end
 end