CLDC-4094: Fix associating log locations after merge (#3112)

* CLDC-4094: Use IDs to reassociate logs after a merge

using names was unsafe as these are not guaranteed unique in an organisation or scheme

build a list of old to new log IDs as merging schemes and locations, then use this when merging lettings logs

* CLDC-4094: Add a verifying test

app/services/merge/merge_organisations_service.rb

@@ -4,6 +4,8 @@ class Merge::MergeOrganisationsService
     @merging_organisations = Organisation.find(merging_organisation_ids)
     @merge_date = merge_date || Time.zone.today
     @absorbing_organisation_active_from_merge_date = absorbing_organisation_active_from_merge_date
+    @pre_to_post_merge_scheme_ids = {}
+    @pre_to_post_merge_location_ids = {}
   end
 
   def call
@@ -70,12 +72,14 @@ private
     merging_organisation.owned_schemes.each do |scheme|
       new_scheme = Scheme.new(scheme.attributes.except("id", "owning_organisation_id", "old_id", "old_visible_id").merge(owning_organisation: @absorbing_organisation, startdate: [scheme&.startdate, @merge_date].compact.max))
       new_scheme.save!(validate: false)
+      @pre_to_post_merge_scheme_ids[scheme.id] = new_scheme.id
       scheme.scheme_deactivation_periods.each do |deactivation_period|
         split_scheme_deactivation_period_between_organisations(deactivation_period, new_scheme)
       end
       scheme.locations.each do |location|
         new_location = Location.new(location.attributes.except("id", "scheme_id", "old_id", "old_visible_id").merge(scheme: new_scheme, startdate: [location&.startdate, @merge_date].compact.max))
         new_location.save!(validate: false)
+        @pre_to_post_merge_location_ids[location.id] = new_location.id
         location.location_deactivation_periods.each do |deactivation_period|
           split_location_deactivation_period_between_organisations(deactivation_period, new_location)
         end
@@ -98,8 +102,8 @@ private
         lettings_log.managing_organisation = @absorbing_organisation
       end
       if lettings_log.scheme.present?
-        scheme_to_set = @absorbing_organisation.owned_schemes.find_by(service_name: lettings_log.scheme.service_name)
-        location_to_set = scheme_to_set.locations.find_by(name: lettings_log.location&.name, postcode: lettings_log.location&.postcode)
+        scheme_to_set = @absorbing_organisation.owned_schemes.find_by(id: @pre_to_post_merge_scheme_ids[lettings_log.scheme.id])
+        location_to_set = scheme_to_set.locations.find_by(id: @pre_to_post_merge_location_ids[lettings_log.location&.id])
 
         lettings_log.scheme = scheme_to_set if scheme_to_set.present?
         # in some cases the lettings_log location is nil even if scheme is present (they're two different questions).

spec/services/merge/merge_organisations_service_spec.rb

@@ -728,6 +728,31 @@ RSpec.describe Merge::MergeOrganisationsService do
             expect(incomplete_lettings_log.location).to be_nil
           end
 
+          it "associates correct lettings logs with correct locations which share a name and postcode" do
+            scheme = create(:scheme, owning_organisation: merging_organisation)
+            location_0 = create(:location, scheme:, name: "duplicate name", postcode: "EE1 1EE", location_admin_district: "dist1", startdate: Time.zone.today)
+            location_1 = create(:location, scheme:, name: "duplicate name", postcode: "EE1 1EE", location_admin_district: "dist2", startdate: Time.zone.today)
+            location_2 = create(:location, scheme:, name: "duplicate name", postcode: "EE1 1EE", location_admin_district: "dist3", startdate: Time.zone.today)
+
+            lettings_log_0 = build(:lettings_log, scheme:, owning_organisation: merging_organisation, startdate: Time.zone.today, location: location_0)
+            lettings_log_0.save!(validate: false)
+            lettings_log_1 = build(:lettings_log, scheme:, owning_organisation: merging_organisation, startdate: Time.zone.today, location: location_1)
+            lettings_log_1.save!(validate: false)
+            lettings_log_2 = build(:lettings_log, scheme:, owning_organisation: merging_organisation, startdate: Time.zone.today, location: location_2)
+            lettings_log_2.save!(validate: false)
+
+            expect(Rails.logger).not_to receive(:error)
+            merge_organisations_service.call
+
+            lettings_log_0.reload
+            lettings_log_1.reload
+            lettings_log_2.reload
+
+            expect(lettings_log_0.location.location_admin_district).to eq("dist1")
+            expect(lettings_log_1.location.location_admin_district).to eq("dist2")
+            expect(lettings_log_2.location.location_admin_district).to eq("dist3")
+          end
+
           context "with merge date in closed collection year" do
             subject(:merge_organisations_service) { described_class.new(absorbing_organisation_id: absorbing_organisation.id, merging_organisation_ids:, merge_date: Time.zone.local(2021, 3, 3)) }