CLDC-2969 Move dpo user when merging (#2056)

* wip

* skip_dpo_validation when merging orgs

app/models/log.rb

@@ -50,7 +50,7 @@ class Log < ApplicationRecord
   scope :imported_2023_with_old_form_id, -> { imported.filter_by_year(2023).has_old_form_id }
   scope :imported_2023, -> { imported.filter_by_year(2023) }
 
-  attr_accessor :skip_update_status, :skip_update_uprn_confirmed
+  attr_accessor :skip_update_status, :skip_update_uprn_confirmed, :skip_dpo_validation
 
   def process_uprn_change!
     if uprn.present?

app/models/validations/setup_validations.rb

@@ -82,6 +82,8 @@ module Validations::SetupValidations
   end
 
   def validate_managing_organisation_data_sharing_agremeent_signed(record)
+    return if record.skip_dpo_validation
+
     if record.managing_organisation_id_changed? && record.managing_organisation.present? && !record.managing_organisation.data_protection_confirmed?
       record.errors.add :managing_organisation_id, I18n.t("validations.setup.managing_organisation.data_sharing_agreement_not_signed")
     end

app/models/validations/shared_validations.rb

@@ -121,6 +121,8 @@ module Validations::SharedValidations
   end
 
   def validate_owning_organisation_data_sharing_agremeent_signed(record)
+    return if record.skip_dpo_validation
+
     if record.owning_organisation_id_changed? && record.owning_organisation.present? && !record.owning_organisation.data_protection_confirmed?
       record.errors.add :owning_organisation_id, I18n.t("validations.setup.owning_organisation.data_sharing_agreement_not_signed")
     end

app/services/merge/merge_organisations_service.rb

@@ -59,8 +59,9 @@ private
   end
 
   def merge_users(merging_organisation)
-    @merged_users[merging_organisation.name] = merging_organisation.users.map { |user| { name: user.name, email: user.email } }
+    users_to_merge = users_to_merge(merging_organisation)
-    merging_organisation.users.update_all(organisation_id: @absorbing_organisation.id)
+    @merged_users[merging_organisation.name] = users_to_merge.map { |user| { name: user.name, email: user.email } }
+    users_to_merge.update_all(organisation_id: @absorbing_organisation.id)
   end
 
   def merge_schemes_and_locations(merging_organisation)
@@ -87,18 +88,21 @@ private
         lettings_log.location = location_to_set if location_to_set.present?
       end
       lettings_log.owning_organisation = @absorbing_organisation
-      lettings_log.save!(validate: false)
+      lettings_log.skip_dpo_validation = true
+      lettings_log.save!
     end
     merging_organisation.managed_lettings_logs.after_date(@merge_date.to_time).each do |lettings_log|
       lettings_log.managing_organisation = @absorbing_organisation
-      lettings_log.save!(validate: false)
+      lettings_log.skip_dpo_validation = true
+      lettings_log.save!
     end
   end
 
   def merge_sales_logs(merging_organisation)
     merging_organisation.sales_logs.after_date(@merge_date.to_time).each do |sales_log|
       sales_log.owning_organisation = @absorbing_organisation
-      sales_log.save!(validate: false)
+      sales_log.skip_dpo_validation = true
+      sales_log.save!
     end
   end
 
@@ -132,4 +136,25 @@ private
   def child_relationship_exists_on_absorbing_organisation?(child_organisation_relationship)
     child_organisation_relationship.child_organisation == @absorbing_organisation || @merging_organisations.include?(child_organisation_relationship.child_organisation) || @absorbing_organisation.child_organisation_relationships.where(child_organisation: child_organisation_relationship.child_organisation).exists?
   end
+
+  def users_to_merge(merging_organisation)
+    return merging_organisation.users if merging_organisation.data_protection_confirmation.blank?
+    if merging_organisation.data_protection_confirmation.data_protection_officer.email.exclude?("@")
+      return merging_organisation.users.where.not(id: merging_organisation.data_protection_confirmation.data_protection_officer.id)
+    end
+
+    new_dpo = User.new(
+      name: merging_organisation.data_protection_confirmation.data_protection_officer.name,
+      organisation: merging_organisation,
+      is_dpo: true,
+      encrypted_password: SecureRandom.hex(10),
+      email: SecureRandom.uuid,
+      confirmed_at: Time.zone.now,
+      active: false,
+    )
+    new_dpo.save!(validate: false)
+    merging_organisation.data_protection_confirmation.update!(data_protection_officer: new_dpo)
+
+    merging_organisation.users.where.not(id: new_dpo.id)
+  end
 end

spec/services/merge/merge_organisations_service_spec.rb

@@ -160,11 +160,11 @@ RSpec.describe Merge::MergeOrganisationsService do
           expect(absorbing_organisation.owned_schemes.count).to eq(1)
           expect(absorbing_organisation.owned_schemes.first.service_name).to eq(scheme.service_name)
           expect(absorbing_organisation.owned_schemes.first.old_id).to be_nil
-          expect(absorbing_organisation.owned_schemes.first.old_visible_id).to be_nil
+          expect(absorbing_organisation.owned_schemes.first.old_visible_id).to eq(nil)
           expect(absorbing_organisation.owned_schemes.first.locations.count).to eq(1)
           expect(absorbing_organisation.owned_schemes.first.locations.first.postcode).to eq(location.postcode)
           expect(absorbing_organisation.owned_schemes.first.locations.first.old_id).to be_nil
-          expect(absorbing_organisation.owned_schemes.first.locations.first.old_visible_id).to be_nil
+          expect(absorbing_organisation.owned_schemes.first.locations.first.old_visible_id).to eq(nil)
           expect(scheme.scheme_deactivation_periods.count).to eq(1)
           expect(scheme.scheme_deactivation_periods.first.deactivation_date.to_date).to eq(Time.zone.today)
         end
@@ -179,7 +179,7 @@ RSpec.describe Merge::MergeOrganisationsService do
           expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log.id).scheme).to eq(absorbing_organisation.owned_schemes.first)
           expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log.id).location).to eq(absorbing_organisation.owned_schemes.first.locations.first)
           expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).scheme).to eq(absorbing_organisation.owned_schemes.first)
-          expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).location).to eq(nil)
+          expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).location).to eq(absorbing_organisation.owned_schemes.first.locations.first)
         end
 
         it "rolls back if there's an error" do
@@ -345,7 +345,7 @@ RSpec.describe Merge::MergeOrganisationsService do
             expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log.id).scheme).to eq(absorbing_organisation.owned_schemes.first)
             expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log.id).location).to eq(absorbing_organisation.owned_schemes.first.locations.first)
             expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).scheme).to eq(absorbing_organisation.owned_schemes.first)
-            expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).location).to eq(nil)
+            expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).location).to eq(absorbing_organisation.owned_schemes.first.locations.first)
           end
 
           it "rolls back if there's an error" do
@@ -404,20 +404,6 @@ RSpec.describe Merge::MergeOrganisationsService do
         create_list(:user, 5, organisation: merging_organisation_too)
       end
 
-      it "moves the users from merging organisations to absorbing organisation" do
-        expect(Rails.logger).to receive(:info).with("Merged users from fake org:")
-        expect(Rails.logger).to receive(:info).with("\tDanny Rojas (#{merging_organisation.data_protection_officers.first.email})")
-        expect(Rails.logger).to receive(:info).with("\tfake name (fake@email.com)")
-        expect(Rails.logger).to receive(:info).with("Merged users from second org:")
-        expect(Rails.logger).to receive(:info).with(/\tDanny Rojas/).exactly(6).times
-        expect(Rails.logger).to receive(:info).with("New schemes from fake org:")
-        expect(Rails.logger).to receive(:info).with("New schemes from second org:")
-        merge_organisations_service.call
-
-        merging_organisation_user.reload
-        expect(merging_organisation_user.organisation).to eq(absorbing_organisation)
-      end
-
       it "sets merge date and absorbing organisation on merged organisations" do
         merge_organisations_service.call
 
@@ -451,6 +437,60 @@ RSpec.describe Merge::MergeOrganisationsService do
         expect(merging_organisation_user.organisation).to eq(merging_organisation)
       end
 
+      context "and merging users" do
+        it "moves the users from merging organisations to absorbing organisation" do
+          expect(Rails.logger).to receive(:info).with("Merged users from fake org:")
+          expect(Rails.logger).to receive(:info).with("\tDanny Rojas (#{merging_organisation.data_protection_officers.first.email})")
+          expect(Rails.logger).to receive(:info).with("\tfake name (fake@email.com)")
+          expect(Rails.logger).to receive(:info).with("Merged users from second org:")
+          expect(Rails.logger).to receive(:info).with(/\tDanny Rojas/).exactly(6).times
+          expect(Rails.logger).to receive(:info).with("New schemes from fake org:")
+          expect(Rails.logger).to receive(:info).with("New schemes from second org:")
+          merge_organisations_service.call
+
+          merging_organisation_user.reload
+          expect(merging_organisation_user.organisation).to eq(absorbing_organisation)
+        end
+
+        it "replaces dpo users with fake users if they have signed the data sharing agreement" do
+          merging_organisation_user.update!(is_dpo: true)
+          merging_organisation.data_protection_confirmation.update!(data_protection_officer: merging_organisation_user)
+
+          merge_organisations_service.call
+
+          merging_organisation_user.reload
+          merging_organisation.reload
+          expect(merging_organisation_user.organisation).to eq(absorbing_organisation)
+          expect(merging_organisation.users.count).to eq(1)
+          expect(merging_organisation.users.first.name).to eq(merging_organisation_user.name)
+          expect(merging_organisation.users.first.email).not_to eq(merging_organisation_user.email)
+          expect(merging_organisation.data_protection_confirmation.data_protection_officer).to eq(merging_organisation.users.first)
+        end
+
+        it "does not move dpo users who have signed data sharing agreement if they have a fake email address" do
+          dpo = User.new(
+            name: merging_organisation.data_protection_confirmation.data_protection_officer.name,
+            organisation: merging_organisation,
+            is_dpo: true,
+            encrypted_password: SecureRandom.hex(10),
+            email: SecureRandom.uuid,
+            confirmed_at: Time.zone.now,
+            active: false,
+          )
+          dpo.save!(validate: false)
+          merging_organisation.data_protection_confirmation.update!(data_protection_officer: dpo)
+
+          merge_organisations_service.call
+
+          dpo.reload
+          merging_organisation.reload
+          expect(dpo.organisation).to eq(merging_organisation)
+          expect(merging_organisation.users.count).to eq(1)
+          expect(merging_organisation.users.first).to eq(dpo)
+          expect(merging_organisation.data_protection_confirmation.data_protection_officer).to eq(dpo)
+        end
+      end
+
       context "and merging organisation relationships" do
         let(:other_organisation) { create(:organisation) }
         let!(:merging_organisation_relationship) { create(:organisation_relationship, parent_organisation: merging_organisation) }
@@ -688,7 +728,7 @@ RSpec.describe Merge::MergeOrganisationsService do
           expect(new_absorbing_organisation.owned_lettings_logs.find(owned_lettings_log.id).scheme).to eq(new_absorbing_organisation.owned_schemes.first)
           expect(new_absorbing_organisation.owned_lettings_logs.find(owned_lettings_log.id).location).to eq(new_absorbing_organisation.owned_schemes.first.locations.first)
           expect(new_absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).scheme).to eq(new_absorbing_organisation.owned_schemes.first)
-          expect(new_absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).location).to eq(nil)
+          expect(new_absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).location).to eq(new_absorbing_organisation.owned_schemes.first.locations.first)
         end
 
         it "rolls back if there's an error" do
@@ -849,12 +889,13 @@ RSpec.describe Merge::MergeOrganisationsService do
 
             new_absorbing_organisation.reload
             merging_organisation.reload
+            owned_lettings_log_no_location.reload
             expect(new_absorbing_organisation.owned_lettings_logs.count).to eq(2)
             expect(new_absorbing_organisation.managed_lettings_logs.count).to eq(1)
             expect(new_absorbing_organisation.owned_lettings_logs.find(owned_lettings_log.id).scheme).to eq(new_absorbing_organisation.owned_schemes.first)
             expect(new_absorbing_organisation.owned_lettings_logs.find(owned_lettings_log.id).location).to eq(new_absorbing_organisation.owned_schemes.first.locations.first)
             expect(new_absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).scheme).to eq(new_absorbing_organisation.owned_schemes.first)
-            expect(new_absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).location).to eq(nil)
+            expect(new_absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).location).to eq(new_absorbing_organisation.owned_schemes.first.locations.first)
           end
 
           it "rolls back if there's an error" do