CLDC-1228: Fixes 4 (#748)

* Prevents location from being cleared when it has been changed alongside the scheme
* Handles visible IDs not being system unique
* Hides the location correctly

app/models/case_log.rb

@@ -23,7 +23,7 @@ class CaseLog < ApplicationRecord
 
   validates_with CaseLogValidator
   before_validation :recalculate_start_year!, if: :startdate_changed?
-  before_validation :reset_scheme_location!, if: :scheme_changed?
+  before_validation :reset_scheme_location!, if: :scheme_changed?, unless: :location_changed?
   before_validation :process_postcode_changes!, if: :postcode_full_changed?
   before_validation :process_previous_postcode_changes!, if: :ppostcode_full_changed?
   before_validation :reset_invalidated_dependent_fields!
@@ -696,8 +696,4 @@ private
   def upcase_and_remove_whitespace(string)
     string.present? ? string.upcase.gsub(/\s+/, "") : string
   end
-
-  def reset_scheme_location!
-    self.location = nil
-  end
 end

app/models/derived_variables/case_log_variables.rb

@@ -68,10 +68,7 @@ module DerivedVariables::CaseLogVariables
     self.new_old = new_or_existing_tenant
     self.vacdays = property_vacant_days
 
-    if is_supported_housing? && scheme
+    if is_supported_housing?
-      if scheme.locations.size == 1
-        self.location = scheme.locations.first
-      end
       if location
         # TODO: Remove and replace with mobility type
         self.wchair = location.wheelchair_adaptation_before_type_cast
@@ -189,4 +186,11 @@ private
       (startdate - voiddate).to_i / 1.day
     end
   end
+
+  def reset_scheme_location!
+    self.location = nil
+    if scheme && scheme.locations.size == 1
+      self.location = scheme.locations.first
+    end
+  end
 end

app/models/form.rb

@@ -127,9 +127,9 @@ class Form
   end
 
   def invalidated_page_questions(case_log, current_user = nil)
-    # we're already treating address fields as a special case and reset their values upon saving a case_log
+    # we're already treating these fields as a special case and reset their values upon saving a case_log
-    address_questions = %w[postcode_known la ppcodenk previous_la_known prevloc postcode_full ppostcode_full]
+    callback_questions = %w[postcode_known la ppcodenk previous_la_known prevloc postcode_full ppostcode_full location_id]
-    questions.reject { |q| q.page.routed_to?(case_log, current_user) || q.derived? || address_questions.include?(q.id) } || []
+    questions.reject { |q| q.page.routed_to?(case_log, current_user) || q.derived? || callback_questions.include?(q.id) } || []
   end
 
   def enabled_page_questions(case_log)

app/models/form/setup/questions/location_id.rb

@@ -5,7 +5,6 @@ class Form::Setup::Questions::LocationId < ::Form::Question
     @header = "Which location is this log for?"
     @hint_text = ""
     @type = "radio"
-    @derived = true unless FeatureToggle.supported_housing_schemes_enabled?
     @answer_options = answer_options
   end
 

app/services/imports/case_logs_import_service.rb

@@ -185,16 +185,20 @@ module Imports
 
       # Supported Housing fields
       if attributes["needstype"] == GN_SH[:supported_housing]
-        old_visible_id = safe_string_as_integer(xml_doc, "_1cschemecode")
+        location_old_visible_id = safe_string_as_integer(xml_doc, "_1cschemecode")
-        location = Location.find_by(old_visible_id:)
+        scheme_old_visible_id = safe_string_as_integer(xml_doc, "_1cmangroupcode")
-        scheme = location.scheme
+
-        # Set the scheme via location, because the scheme old visible ID is not unique
+        schemes = Scheme.where(old_visible_id: scheme_old_visible_id, owning_organisation_id: attributes["owning_organisation_id"])
+        location = Location.find_by(old_visible_id: location_old_visible_id, scheme: schemes)
+        raise "No matching location for scheme #{scheme_old_visible_id} and location #{location_old_visible_id} (visible IDs)" if location.nil?
+
+        # Set the scheme via location, because the scheme old visible ID can be duplicated at import
         attributes["location_id"] = location.id
-        attributes["scheme_id"] = scheme.id
+        attributes["scheme_id"] = location.scheme.id
         attributes["sheltered"] = unsafe_string_as_integer(xml_doc, "Q1e")
         attributes["chcharge"] = safe_string_as_decimal(xml_doc, "Q18b")
         attributes["household_charge"] = household_charge(xml_doc)
-        attributes["is_carehome"] = is_carehome(scheme)
+        attributes["is_carehome"] = is_carehome(location.scheme)
       end
 
       # Handles confidential schemes

spec/models/form_spec.rb

@@ -178,18 +178,15 @@ RSpec.describe Form, type: :model do
 
   describe "invalidated_page_questions" do
     let(:case_log) { FactoryBot.create(:case_log, :in_progress, needstype: 1) }
+    let(:expected_invalid) { %w[scheme_id condition_effects cbl conditional_question_no_second_question net_income_value_check dependent_question offered layear declaration] }
 
     context "when dependencies are not met" do
-      let(:expected_invalid) { %w[scheme_id location_id condition_effects cbl conditional_question_no_second_question net_income_value_check dependent_question offered layear declaration] }
-
       it "returns an array of question keys whose pages conditions are not met" do
         expect(form.invalidated_page_questions(case_log).map(&:id).uniq).to eq(expected_invalid)
       end
     end
 
     context "with two pages having the same question and only one has dependencies met" do
-      let(:expected_invalid) { %w[scheme_id location_id condition_effects cbl conditional_question_no_second_question net_income_value_check dependent_question offered layear declaration] }
-
       it "returns an array of question keys whose pages conditions are not met" do
         case_log["preg_occ"] = "No"
         expect(form.invalidated_page_questions(case_log).map(&:id).uniq).to eq(expected_invalid)

spec/services/imports/case_logs_import_service_spec.rb

@@ -9,7 +9,10 @@ RSpec.describe Imports::CaseLogsImportService do
   let(:real_2021_2022_form) { Form.new("config/forms/2021_2022.json", "2021_2022") }
   let(:real_2022_2023_form) { Form.new("config/forms/2022_2023.json", "2022_2023") }
   let(:fixture_directory) { "spec/fixtures/imports/case_logs" }
+
   let(:organisation) { FactoryBot.create(:organisation, old_visible_id: "1", provider_type: "PRP") }
+  let(:scheme1) { FactoryBot.create(:scheme, old_visible_id: 123, owning_organisation: organisation) }
+  let(:scheme2) { FactoryBot.create(:scheme, old_visible_id: 456, owning_organisation: organisation) }
 
   def open_file(directory, filename)
     File.open("#{directory}/#{filename}.xml")
@@ -26,13 +29,10 @@ RSpec.describe Imports::CaseLogsImportService do
     FactoryBot.create(:user, old_user_id: "c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa", organisation:)
     FactoryBot.create(:user, old_user_id: "e29c492473446dca4d50224f2bb7cf965a261d6f", organisation:)
 
-    # Scheme and Location
+    # Location setup
-    scheme = FactoryBot.create(:scheme, old_visible_id: 123)
+    FactoryBot.create(:location, old_visible_id: 10, wheelchair_adaptation: 1, postcode: "LS166FT", scheme_id: scheme1.id)
-    FactoryBot.create(:location,
+    FactoryBot.create(:location, scheme_id: scheme1.id)
-                      old_visible_id: 10,
+    FactoryBot.create(:location, old_visible_id: 10, wheelchair_adaptation: 1, postcode: "LS166FT", scheme_id: scheme2.id)
-                      scheme_id: scheme.id,
-                      wheelchair_adaptation: 1,
-                      postcode: "LS166FT")
 
     # Stub the form handler to use the real form
     allow(FormHandler.instance).to receive(:get_form).with("2021_2022").and_return(real_2021_2022_form)
@@ -216,7 +216,7 @@ RSpec.describe Imports::CaseLogsImportService do
       end
     end
 
-    context "and this is a supported housing log" do
+    context "and this is a supported housing log with multiple locations under a scheme" do
       let(:case_log_id) { "0b4a68df-30cc-474a-93c0-a56ce8fdad3b" }
 
       it "sets the scheme and location values" do
@@ -229,5 +229,21 @@ RSpec.describe Imports::CaseLogsImportService do
         expect(case_log.status).to eq("completed")
       end
     end
+
+    context "and this is a supported housing log with a single location under a scheme" do
+      let(:case_log_id) { "0b4a68df-30cc-474a-93c0-a56ce8fdad3b" }
+
+      before { case_log_xml.at_xpath("//xmlns:_1cmangroupcode").content = scheme2.old_visible_id }
+
+      it "sets the scheme and location values" do
+        expect(logger).not_to receive(:warn)
+        case_log_service.send(:create_log, case_log_xml)
+        case_log = CaseLog.find_by(old_id: case_log_id)
+
+        expect(case_log.scheme_id).not_to be_nil
+        expect(case_log.location_id).not_to be_nil
+        expect(case_log.status).to eq("completed")
+      end
+    end
   end
 end