Merge pull request #6585 from betagouv/merge_and_delete

Supprime le vieux compte lors d'un merge

app/controllers/manager/users_controller.rb

@@ -14,19 +14,7 @@ module Manager
           flash[:error] = user.errors.full_messages.to_sentence
         end
       else
-        user.dossiers.update_all(user_id: targeted_user.id)
-
-        [
-          [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
+        targeted_user.merge(user)
 
         flash[:notice] = "Le compte « #{targeted_email} » a absorbé le compte « #{user.email} »."
       end

app/models/merge_log.rb

@@ -0,0 +1,14 @@
+# == Schema Information
+#
+# Table name: merge_logs
+#
+#  id              :bigint           not null, primary key
+#  from_user_email :string           not null
+#  created_at      :datetime         not null
+#  updated_at      :datetime         not null
+#  from_user_id    :bigint           not null
+#  user_id         :bigint           not null
+#
+class MergeLog < ApplicationRecord
+  belongs_to :user
+end

app/models/user.rb

@@ -47,10 +47,11 @@ class User < ApplicationRecord
   has_many :invites, dependent: :destroy
   has_many :dossiers_invites, through: :invites, source: :dossier
   has_many :deleted_dossiers
+  has_many :merge_logs, dependent: :destroy
   has_one :france_connect_information, dependent: :destroy
-  belongs_to :instructeur, optional: true
-  belongs_to :administrateur, optional: true
-  belongs_to :expert, optional: true
+  belongs_to :instructeur, optional: true, dependent: :destroy
+  belongs_to :administrateur, optional: true, dependent: :destroy
+  belongs_to :expert, optional: true, dependent: :destroy
 
   accepts_nested_attributes_for :france_connect_information
 
@@ -194,6 +195,29 @@ class User < ApplicationRecord
     end
   end
 
+  def merge(old_user)
+    transaction do
+      old_user.dossiers.update_all(user_id: id)
+      old_user.invites.update_all(user_id: id)
+      old_user.merge_logs.update_all(user_id: id)
+
+      [
+        [old_user.instructeur, instructeur],
+        [old_user.expert, expert],
+        [old_user.administrateur, administrateur]
+      ].each do |old_role, targeted_role|
+        if targeted_role.nil?
+          old_role&.update(user: self)
+        else
+          targeted_role.merge(old_role)
+        end
+      end
+
+      merge_logs.create(from_user_id: old_user.id, from_user_email: old_user.email)
+      old_user.destroy
+    end
+  end
+
   private
 
   def link_invites!

db/migrate/20211026131800_create_merge_logs.rb

@@ -0,0 +1,11 @@
+class CreateMergeLogs < ActiveRecord::Migration[6.1]
+  def change
+    create_table :merge_logs do |t|
+      t.bigint :from_user_id, null: false
+      t.string :from_user_email, null: false
+      t.references :user, null: false, foreign_key: true
+
+      t.timestamps
+    end
+  end
+end

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: 2021_10_20_114237) do
+ActiveRecord::Schema.define(version: 2021_10_26_131800) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -546,6 +546,15 @@ ActiveRecord::Schema.define(version: 2021_10_20_114237) do
     t.index ["email", "dossier_id"], name: "index_invites_on_email_and_dossier_id", unique: true
   end
 
+  create_table "merge_logs", force: :cascade do |t|
+    t.bigint "from_user_id", null: false
+    t.string "from_user_email", null: false
+    t.bigint "user_id", null: false
+    t.datetime "created_at", precision: 6, null: false
+    t.datetime "updated_at", precision: 6, null: false
+    t.index ["user_id"], name: "index_merge_logs_on_user_id"
+  end
+
   create_table "module_api_cartos", id: :serial, force: :cascade do |t|
     t.integer "procedure_id"
     t.boolean "use_api_carto", default: false
@@ -841,6 +850,7 @@ ActiveRecord::Schema.define(version: 2021_10_20_114237) do
   add_foreign_key "geo_areas", "champs"
   add_foreign_key "groupe_instructeurs", "procedures"
   add_foreign_key "initiated_mails", "procedures"
+  add_foreign_key "merge_logs", "users"
   add_foreign_key "procedure_presentations", "assign_tos"
   add_foreign_key "procedure_revision_types_de_champ", "procedure_revisions", column: "revision_id"
   add_foreign_key "procedure_revision_types_de_champ", "types_de_champ"

spec/controllers/manager/users_controller_spec.rb

@@ -48,43 +48,12 @@ describe Manager::UsersController, type: :controller do
       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) }
+      it 'launches the merge process' do
+        expect_any_instance_of(User).to receive(:merge).with(user)
 
-        it 'transfers the dossier' do
-          subject
+        subject
 
-          expect(targeted_user.dossiers).to match([dossier])
-        end
-      end
-
-      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, instructeur: instructeur) }
-
-        it 'transfers instructeur account' do
-          subject
-          targeted_user.reload
-
-          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 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) }
-
-          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
+        expect(flash[:notice]).to match("Le compte « email.existant@domaine.fr » a absorbé le compte « ancien.email@domaine.fr ».")
       end
     end
   end

spec/factories/user.rb

@@ -2,7 +2,7 @@ FactoryBot.define do
   sequence(:user_email) { |n| "user#{n}@user.com" }
   factory :user do
     email { generate(:user_email) }
-    password { 'my-s3cure-p4ssword' }
+    password { '{My-$3cure-p4ssWord}' }
     confirmed_at { Time.zone.now }
 
     trait :unconfirmed do

spec/models/user_spec.rb

@@ -417,4 +417,63 @@ describe User, type: :model do
       end
     end
   end
+
+  describe '#merge' do
+    let(:old_user) { create(:user) }
+    let(:targeted_user) { create(:user) }
+
+    subject { targeted_user.merge(old_user) }
+
+    context 'and the old account has some stuff' do
+      let!(:dossier) { create(:dossier, user: old_user) }
+      let!(:invite) { create(:invite, user: old_user) }
+      let!(:merge_log) { MergeLog.create(user: old_user, from_user_id: 1, from_user_email: 'a') }
+
+      it 'transfers the dossier' do
+        subject
+
+        expect(targeted_user.dossiers).to match([dossier])
+        expect(targeted_user.invites).to match([invite])
+        expect(targeted_user.merge_logs.first).to eq(merge_log)
+
+        added_merge_log = targeted_user.merge_logs.last
+        expect(added_merge_log.from_user_id).to eq(old_user.id)
+        expect(added_merge_log.from_user_email).to eq(old_user.email)
+      end
+    end
+
+    context 'and the old account belongs to an instructeur, expert and administrateur' do
+      let!(:expert) { create(:expert, user: old_user) }
+      let!(:administrateur) { create(:administrateur, user: old_user) }
+      let!(:instructeur) { old_user.instructeur }
+
+      it 'transfers instructeur account' do
+        subject
+        targeted_user.reload
+
+        expect(targeted_user.instructeur).to match(instructeur)
+        expect(targeted_user.expert).to match(expert)
+        expect(targeted_user.administrateur).to match(administrateur)
+      end
+
+      context 'and the targeted account owns an instructeur and expert as well' do
+        let!(:targeted_administrateur) { create(:administrateur, user: targeted_user) }
+        let!(:targeted_instructeur) { targeted_user.instructeur }
+        let!(:targeted_expert) { create(:expert, user: targeted_user) }
+
+        it 'merge the account' do
+          expect(targeted_instructeur).to receive(:merge).with(instructeur)
+          expect(targeted_expert).to receive(:merge).with(expert)
+          expect(targeted_administrateur).to receive(:merge).with(administrateur)
+
+          subject
+
+          expect { instructeur.reload }.to raise_error(ActiveRecord::RecordNotFound)
+          expect { expert.reload }.to raise_error(ActiveRecord::RecordNotFound)
+          expect { administrateur.reload }.to raise_error(ActiveRecord::RecordNotFound)
+          expect { old_user.reload }.to raise_error(ActiveRecord::RecordNotFound)
+        end
+      end
+    end
+  end
 end