[CPDLP-3484] Check for multiple users and tests

app/services/migration/migrators/user.rb

@@ -55,14 +55,23 @@ def call
   private
 
     def find_or_initialize_user(ecf_user)
-      # Find User with `ecf_user.id`
-      user_with_ecf_id = ::User.find_by(ecf_id: ecf_user.id)
+      # Find Users with `ecf_user.id`
+      users_with_ecf_id = ::User.where(ecf_id: ecf_user.id)
 
       # Find User with `ecf_user.get_an_identity_id`
       if ecf_user.get_an_identity_id.present?
         user_with_gai_id = ::User.find_by(uid: ecf_user.get_an_identity_id)
       end
 
+      # user.ecf_id is not validated as unique, check for duplicates
+      if users_with_ecf_id && users_with_ecf_id.size > 1
+        ecf_user.errors.add(:base, "ecf_user.id has multiple users in NPQ")
+        raise ActiveRecord::RecordInvalid, ecf_user
+      end
+
+      # User with `ecf_user.id`
+      user_with_ecf_id = users_with_ecf_id.first
+
       # User does not exist, initialize new user
       if user_with_ecf_id.nil? && user_with_gai_id.nil?
         return ::User.new(ecf_id: ecf_user.id)
@@ -80,7 +89,8 @@ def find_or_initialize_user(ecf_user)
 
       # We have User 2 records, but they are different
       if user_with_ecf_id && user_with_gai_id && user_with_ecf_id != user_with_gai_id
-        raise "[#{ecf_user.id}] ecf_user.id and ecf_user.get_an_identity_id both return User records, but they are different"
+        ecf_user.errors.add(:base, "ecf_user.id and ecf_user.get_an_identity_id both return User records, but they are different")
+        raise ActiveRecord::RecordInvalid, ecf_user
       end
 
       # Found User with `ecf_user.get_an_identity_id` only (not found with `ecf_user.id`)
@@ -102,10 +112,12 @@ def find_or_initialize_user(ecf_user)
           return user_with_gai_id
         end
 
-        raise "[#{ecf_user.id}] User found with ecf_user.get_an_identity_id, but its user.uid linked to another ecf_user that is not an orphan"
+        ecf_user.errors.add(:base, "User found with ecf_user.get_an_identity_id, but its user.ecf_id linked to another ecf_user that is not an orphan")
+        raise ActiveRecord::RecordInvalid, ecf_user
       end
 
-      raise "[#{ecf_user.id}] End of find_or_initialize_user logic, something weird happened"
+      ecf_user.errors.add(:base, "End of find_or_initialize_user logic, something weird happened")
+      raise ActiveRecord::RecordInvalid, ecf_user
     end
 
     def unique_validated_trns(ecf_user)

spec/services/migration/migrators/user_spec.rb

@@ -79,6 +79,114 @@ def setup_failure_state
           trn_verified: true,
         )
       end
+
+      context "when there are multiple users with the same ecf_id" do
+        it "raises error" do
+          ecf_user = create(:ecf_migration_user, :npq, email: "[email protected]")
+          create(:user, ecf_id: ecf_user.id, email: "[email protected]")
+          create(:user, ecf_id: ecf_user.id, email: "[email protected]")
+
+          instance.call
+          expect(failure_manager).to have_received(:record_failure).with(ecf_user, /ecf_user.id has multiple users in NPQ/)
+        end
+      end
+
+      context "when no user exists with ecf_id or get_an_identity_id" do
+        it "creates a new user" do
+          ecf_user = create(:ecf_migration_user, :npq, get_an_identity_id: SecureRandom.uuid)
+
+          expect(User.find_by(ecf_id: ecf_user.id)).to be_nil
+          expect(User.find_by(uid: ecf_user.get_an_identity_id)).to be_nil
+
+          instance.call
+
+          expect(User.where(ecf_id: ecf_user.id).count).to eq(1)
+          user = User.find_by(uid: ecf_user.get_an_identity_id)
+          expect(user.ecf_id).to eq(ecf_user.id)
+        end
+      end
+
+      context "when only one user exists with both ecf_id and get_an_identity_id" do
+        it "updates the user" do
+          ecf_user = create(:ecf_migration_user, :npq, get_an_identity_id: SecureRandom.uuid, full_name: "New Name")
+          create(:user, email: ecf_user.email, ecf_id: ecf_user.id, uid: ecf_user.get_an_identity_id, full_name: "Old Name")
+
+          instance.call
+
+          expect(User.where(ecf_id: ecf_user.id).count).to eq(1)
+          user = User.find_by(uid: ecf_user.get_an_identity_id)
+          expect(user.ecf_id).to eq(ecf_user.id)
+          expect(user.full_name).to eq("New Name")
+        end
+      end
+
+      context "when one user exists with ecf_id only" do
+        it "updates the user" do
+          ecf_user = create(:ecf_migration_user, :npq, get_an_identity_id: SecureRandom.uuid, full_name: "New Name")
+          create(:user, email: ecf_user.email, ecf_id: ecf_user.id, uid: SecureRandom.uuid, full_name: "Old Name")
+
+          expect(User.where(ecf_id: ecf_user.id).count).to eq(1)
+          expect(User.where(uid: ecf_user.get_an_identity_id).count).to eq(0)
+
+          instance.call
+
+          expect(User.where(ecf_id: ecf_user.id).count).to eq(1)
+          user = User.find_by(uid: ecf_user.get_an_identity_id)
+          expect(user.ecf_id).to eq(ecf_user.id)
+          expect(user.full_name).to eq("New Name")
+        end
+      end
+
+      context "when ecf_id and get_an_identity_id both return users and are different" do
+        it "raises error" do
+          ecf_user = create(:ecf_migration_user, :npq, email: "[email protected]", get_an_identity_id: SecureRandom.uuid)
+          create(:user, ecf_id: ecf_user.id, email: "[email protected]")
+          create(:user, uid: ecf_user.get_an_identity_id, email: "[email protected]")
+
+          instance.call
+          expect(failure_manager).to have_received(:record_failure).with(ecf_user, /ecf_user.id and ecf_user.get_an_identity_id both return User records, but they are different/)
+        end
+      end
+
+      context "when NPQ user found with ecf_user.get_an_identity_id only" do
+        let(:ecf_user) { create(:ecf_migration_user, :npq, get_an_identity_id: SecureRandom.uuid, full_name: "New Name") }
+        let(:user) do
+          create(:user, email: ecf_user.email, ecf_id: SecureRandom.uuid, uid: ecf_user.get_an_identity_id, full_name: "Old Name")
+        end
+
+        context "when NPQ user.ecf_id is set but no ecf user found" do
+          it "updates user.ecf_id with ecf_user.id" do
+            expect(Migration::Ecf::User.find_by(id: user.ecf_id)).to be_nil
+
+            instance.call
+            expect(user.reload.ecf_id).to eq(ecf_user.id)
+            expect(user.full_name).to eq("New Name")
+          end
+        end
+
+        context "when NPQ user.ecf_id links to a different ecf_user" do
+          context "when linked ecf_user is an orphan" do
+            it "updates user.ecf_id with ecf_user.id" do
+              orphaned_ecf_user = create(:ecf_migration_user, id: user.ecf_id)
+              expect(orphaned_ecf_user.npq_applications).to be_empty
+
+              instance.call
+              expect(user.reload.ecf_id).to eq(ecf_user.id)
+              expect(user.full_name).to eq("New Name")
+            end
+          end
+
+          context "when linked ecf_user is not an orphan" do
+            it "raises error" do
+              non_orphaned_ecf_user = create(:ecf_migration_user, :npq, id: user.ecf_id)
+              expect(non_orphaned_ecf_user.npq_applications).to be_present
+
+              instance.call
+              expect(failure_manager).to have_received(:record_failure).with(ecf_user, /User found with ecf_user.get_an_identity_id, but its user.ecf_id linked to another ecf_user that is not an orphan/)
+            end
+          end
+        end
+      end
     end
   end
 end