CLDC-4039: Fix merge functionality not working (#3080)

* ensure nil locations are never overwritten

* add a verifying test

* fixup! add a verifying test

remove unneeded context

* fixup! add a verifying test

clarify test structure

app/services/merge/merge_organisations_service.rb

@@ -102,7 +102,11 @@ private
         location_to_set = scheme_to_set.locations.find_by(name: lettings_log.location&.name, postcode: lettings_log.location&.postcode)
 
         lettings_log.scheme = scheme_to_set if scheme_to_set.present?
-        lettings_log.location = location_to_set if location_to_set.present?
+        # in some cases the lettings_log location is nil even if scheme is present (they're two different questions).
+        # in certain cases the location_to_set query can find a location in the scheme with nil values for name and postcode,
+        # so we can end up setting the location to non nil.
+        # hence the extra check here
+        lettings_log.location = location_to_set if location_to_set.present? && lettings_log.location.present?
       end
       lettings_log.owning_organisation = @absorbing_organisation
       lettings_log.managing_organisation = @absorbing_organisation if lettings_log.managing_organisation == merging_organisation

spec/services/merge/merge_organisations_service_spec.rb

@@ -707,6 +707,27 @@ RSpec.describe Merge::MergeOrganisationsService do
             expect(owned_lettings_log.managing_organisation).to eq(absorbing_organisation)
           end
 
+          it "does not change the lettings log location" do
+            scheme = create(:scheme, owning_organisation: merging_organisation)
+            create(:location, scheme:, name: nil, postcode: nil)
+            # necessary to have a couple valid locations else the scheme will be invalid
+            create(:location, scheme:)
+            create(:location, scheme:)
+            incomplete_lettings_log = build(:lettings_log, scheme:, owning_organisation: merging_organisation, startdate: Time.zone.today)
+            incomplete_lettings_log.save!(validate: false)
+
+            # if the location is overwritten with the nil one above, it will fail validation
+            # since a rollback will occur incomplete_lettings_log will not change so there's nothing to verify later
+            # so instead we verify that no rollback occurs
+            expect(Rails.logger).not_to receive(:error)
+            merge_organisations_service.call
+
+            incomplete_lettings_log.reload
+
+            # also ensure it wasn't overwritten with a valid location
+            expect(incomplete_lettings_log.location).to be_nil
+          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)) }