CLDC-3487: Ensure locations and previous_la_known are cleared when necessary (#2452)

* CLDC-3487: Clear previous_la_known when necessary for renewal logs

* CLDC-3487: Clear location when clearing scheme due to org change

* CLDC-3487: Use nil instead of 0

* Specify orgs on sh logs when needed for tests

* Fix flaky tests

app/models/derived_variables/lettings_log_variables.rb

@@ -123,7 +123,7 @@ module DerivedVariables::LettingsLogVariables
                         0
                       end
       self.is_previous_la_inferred = is_la_inferred
-      self.previous_la_known = 1 if la.present?
+      self.previous_la_known = la.present? ? 1 : nil
       self.prevloc = la
     end
 

app/models/lettings_log.rb

@@ -721,8 +721,10 @@ private
 
   def reset_scheme
     return unless scheme && owning_organisation
+    return unless scheme.owning_organisation != owning_organisation
 
-    self.scheme = nil if scheme.owning_organisation != owning_organisation
+    self.scheme = nil
+    self.location = nil
   end
 
   def reset_invalidated_dependent_fields!

spec/lib/tasks/update_schemes_and_locations_from_csv_spec.rb

@@ -336,11 +336,11 @@ RSpec.describe "bulk_update" do
                     updated_at: Time.zone.local(2022, 3, 1),
                     scheme:)
       end
-      let!(:lettings_log) { FactoryBot.create(:lettings_log, :sh, location: locations[0], scheme:, values_updated_at: nil, startdate: Time.zone.local(2023, 4, 4)) }
+      let!(:lettings_log) { FactoryBot.create(:lettings_log, :sh, owning_organisation: scheme.owning_organisation, location: locations[0], scheme:, values_updated_at: nil, startdate: Time.zone.local(2023, 4, 4)) }
-      let!(:lettings_log_2) { FactoryBot.create(:lettings_log, :sh, location: locations[1], scheme:, values_updated_at: nil, startdate: Time.zone.local(2023, 4, 4)) }
+      let!(:lettings_log_2) { FactoryBot.create(:lettings_log, :sh, owning_organisation: scheme.owning_organisation, location: locations[1], scheme:, values_updated_at: nil, startdate: Time.zone.local(2023, 4, 4)) }
-      let!(:lettings_log_3) { FactoryBot.create(:lettings_log, :sh, location: locations[2], scheme:, values_updated_at: nil, startdate: Time.zone.local(2023, 4, 4)) }
+      let!(:lettings_log_3) { FactoryBot.create(:lettings_log, :sh, owning_organisation: scheme.owning_organisation, location: locations[2], scheme:, values_updated_at: nil, startdate: Time.zone.local(2023, 4, 4)) }
-      let!(:lettings_log_4) { FactoryBot.create(:lettings_log, :sh, location: locations[0], scheme:, values_updated_at: nil, startdate: Time.zone.local(2023, 4, 4)) }
+      let!(:lettings_log_4) { FactoryBot.create(:lettings_log, :sh, owning_organisation: scheme.owning_organisation, location: locations[0], scheme:, values_updated_at: nil, startdate: Time.zone.local(2023, 4, 4)) }
-      let!(:lettings_log_5) { FactoryBot.create(:lettings_log, :sh, location: locations[0], scheme:, values_updated_at: nil, startdate: Time.zone.local(2023, 4, 4)) }
+      let!(:lettings_log_5) { FactoryBot.create(:lettings_log, :sh, owning_organisation: scheme.owning_organisation, location: locations[0], scheme:, values_updated_at: nil, startdate: Time.zone.local(2023, 4, 4)) }
 
       before do
         allow(storage_service).to receive(:get_file_io)

spec/models/lettings_log_derived_fields_spec.rb

@@ -1006,6 +1006,16 @@ RSpec.describe LettingsLog, type: :model do
         .and change(log, :prevloc).to(expected_la)
     end
 
+    it "clears values for previous location and related fields when log is a renewal and current values are cleared" do
+      log.assign_attributes(postcode_known: 0, postcode_full: nil, la: nil, renewal: 1, previous_la_known: 1, prevloc: "E09000033", ppostcode_full: "SW1A 1AA", ppcodenk: 0)
+
+      expect { log.set_derived_fields! }
+        .to change(log, :previous_la_known).to(nil)
+        .and change(log, :prevloc).to(nil)
+        .and change(log, :ppcodenk).to(1)
+        .and change(log, :ppostcode_full).to(nil)
+    end
+
     context "when the log is general needs" do
       context "and the managing organisation is a private registered provider" do
         before do

spec/models/lettings_log_spec.rb

@@ -501,7 +501,7 @@ RSpec.describe LettingsLog do
       end
 
       context "and a scheme with a single log is selected" do
-        let(:scheme) { create(:scheme) }
+        let(:scheme) { create(:scheme, owning_organisation:) }
         let!(:location) { create(:location, scheme:) }
 
         before do
@@ -574,7 +574,7 @@ RSpec.describe LettingsLog do
       end
 
       context "and not renewal" do
-        let(:scheme) { create(:scheme) }
+        let(:scheme) { create(:scheme, owning_organisation:) }
         let(:location) { create(:location, scheme:, postcode: "M11AE", type_of_unit: 1, mobility_type: "W") }
 
         let(:supported_housing_lettings_log) do
@@ -956,23 +956,27 @@ RSpec.describe LettingsLog do
 
       context "when the organisation selected doesn't match the scheme set" do
         let(:scheme) { create(:scheme, owning_organisation: assigned_to_user.organisation) }
-        let(:location) { create(:location, scheme:) }
+        let(:location) { create_list(:location, 2, scheme:).first }
-        let(:lettings_log) { create(:lettings_log, owning_organisation: nil, needstype: 2, scheme_id: scheme.id) }
+        let(:lettings_log) { create(:lettings_log, owning_organisation: nil, needstype: 2, scheme_id: scheme.id, location_id: location.id) }
 
-        it "clears the scheme value" do
+        it "clears the scheme and location values" do
           lettings_log.update!(owning_organisation: organisation_2)
-          expect(lettings_log.reload.scheme).to be nil
+          lettings_log.reload
+          expect(lettings_log.scheme).to be nil
+          expect(lettings_log.location).to be nil
         end
       end
 
       context "when the organisation selected still matches the scheme set" do
         let(:scheme) { create(:scheme, owning_organisation: organisation_2) }
-        let(:location) { create(:location, scheme:) }
+        let(:location) { create_list(:location, 2, scheme:).first }
-        let(:lettings_log) { create(:lettings_log, owning_organisation: nil, needstype: 2, scheme_id: scheme.id) }
+        let(:lettings_log) { create(:lettings_log, owning_organisation: nil, needstype: 2, scheme_id: scheme.id, location_id: location.id) }
 
-        it "does not clear the scheme value" do
+        it "does not clear the scheme or location value" do
           lettings_log.update!(owning_organisation: organisation_2)
-          expect(lettings_log.reload.scheme_id).to eq(scheme.id)
+          lettings_log.reload
+          expect(lettings_log.scheme_id).to eq(scheme.id)
+          expect(lettings_log.location_id).to eq(location.id)
         end
       end
     end
@@ -1746,7 +1750,7 @@ RSpec.describe LettingsLog do
       end
 
       context "when there is a duplicate supported housing log" do
-        let(:scheme) { create(:scheme) }
+        let(:scheme) { create(:scheme, owning_organisation: organisation) }
         let(:location) { create(:location, scheme:) }
         let(:location_2) { create(:location, scheme:) }
         let!(:supported_housing_log) { create(:lettings_log, :duplicate, needstype: 2, location:, scheme:, owning_organisation: organisation) }
@@ -1773,9 +1777,8 @@ RSpec.describe LettingsLog do
         end
 
         it "does not return logs not associated with the user if user is given" do
-          user = create(:user)
+          user = create(:user, organisation:)
-          supported_housing_log.update!(assigned_to: user, owning_organisation: user.organisation)
+          supported_housing_log.update!(assigned_to: user)
-          duplicate_supported_housing_log.update!(owning_organisation: user.organisation)
           duplicate_sets = described_class.duplicate_sets(user.id)
           expect(duplicate_sets.count).to eq(1)
           expect(duplicate_sets.first).to contain_exactly(supported_housing_log.id, duplicate_supported_housing_log.id)

spec/models/sales_log_spec.rb

@@ -191,32 +191,32 @@ RSpec.describe SalesLog, type: :model do
 
     it "allows searching using ID" do
       result = described_class.search_by(sales_log_to_search.id.to_s)
-      expect(result.count).to eq(1)
+      expect(result.count).to be >= 1
-      expect(result.first.id).to eq sales_log_to_search.id
+      expect(result).to include(have_attributes(id: sales_log_to_search.id))
     end
 
     it "allows searching using purchaser code" do
       result = described_class.search_by(sales_log_to_search.purchaser_code)
-      expect(result.count).to eq(1)
+      expect(result.count).to be >= 1
-      expect(result.first.id).to eq sales_log_to_search.id
+      expect(result).to include(have_attributes(id: sales_log_to_search.id))
     end
 
     it "allows searching by a Property Postcode" do
       result = described_class.search_by(sales_log_to_search.postcode_full)
-      expect(result.count).to eq(1)
+      expect(result.count).to be >= 1
-      expect(result.first.id).to eq sales_log_to_search.id
+      expect(result).to include(have_attributes(id: sales_log_to_search.id))
     end
 
     it "allows searching by id including the word log" do
       result = described_class.search_by("log#{sales_log_to_search.id}")
-      expect(result.count).to eq(1)
+      expect(result.count).to be >= 1
-      expect(result.first.id).to eq sales_log_to_search.id
+      expect(result).to include(have_attributes(id: sales_log_to_search.id))
     end
 
     it "allows searching by id including the capitalised word Log" do
       result = described_class.search_by("Log#{sales_log_to_search.id}")
-      expect(result.count).to eq(1)
+      expect(result.count).to be >= 1
-      expect(result.first.id).to eq sales_log_to_search.id
+      expect(result).to include(have_attributes(id: sales_log_to_search.id))
     end
 
     context "when postcode has spaces and lower case letters" do
@@ -224,8 +224,8 @@ RSpec.describe SalesLog, type: :model do
 
       it "allows searching by a Property Postcode" do
         result = described_class.search_by(matching_postcode_lower_case_with_spaces)
-        expect(result.count).to eq(1)
+        expect(result.count).to be >= 1
-        expect(result.first.id).to eq sales_log_to_search.id
+        expect(result).to include(have_attributes(id: sales_log_to_search.id))
       end
     end
   end

spec/requests/locations_controller_spec.rb

@@ -1582,8 +1582,8 @@ RSpec.describe LocationsController, type: :request do
         before do
           allow(LocationOrSchemeDeactivationMailer).to receive(:send_deactivation_mail).and_call_original
 
-          create(:lettings_log, :sh, location:, scheme:, startdate:, assigned_to: user_a)
+          create(:lettings_log, :sh, owning_organisation: scheme.owning_organisation, location:, scheme:, startdate:, assigned_to: user_a)
-          create_list(:lettings_log, 3, :sh, location:, scheme:, startdate:, assigned_to: user_b)
+          create_list(:lettings_log, 3, :sh, owning_organisation: scheme.owning_organisation, location:, scheme:, startdate:, assigned_to: user_b)
 
           Timecop.freeze(Time.utc(2022, 10, 10))
           sign_in user
@@ -1915,7 +1915,7 @@ RSpec.describe LocationsController, type: :request do
 
     context "when signed in as a support user" do
       let(:user) { create(:user, :support) }
-      let(:scheme) { create(:scheme) }
+      let(:scheme) { create(:scheme, owning_organisation: user.organisation) }
       let(:location) { create(:location, scheme:) }
       let(:add_deactivations) { location.location_deactivation_periods << location_deactivation_period }