Handles visible IDs not being system unique

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

@@ -5,7 +5,7 @@ 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?
+    @derived = true
     @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 = Location.find_by(old_visible_id:)
-        scheme = location.scheme
-        # Set the scheme via location, because the scheme old visible ID is not unique
+        location_old_visible_id = safe_string_as_integer(xml_doc, "_1cschemecode")
+        scheme_old_visible_id = safe_string_as_integer(xml_doc, "_1cmangroupcode")
+
+        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/setup/questions/location_id_spec.rb

@@ -28,7 +28,7 @@ RSpec.describe Form::Setup::Questions::LocationId, type: :model do
   end
 
   it "is marked as derived" do
-    expect(question).not_to be_derived
+    expect(question).to be_derived
   end
 
   context "when there are no locations" do

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,14 +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
-    scheme = FactoryBot.create(:scheme, old_visible_id: 123)
-    locations =  FactoryBot.create_list(:location, 3, scheme_id: scheme.id)
-    locations[0].update!(
-      old_visible_id: 10,
-      wheelchair_adaptation: 1,
-      postcode: "LS166FT"
-    )
+    # Location setup
+    FactoryBot.create(:location, old_visible_id: 10, wheelchair_adaptation: 1, postcode: "LS166FT", scheme_id: scheme1.id)
+    FactoryBot.create(:location, scheme_id: scheme1.id)
+    FactoryBot.create(:location, old_visible_id: 10, wheelchair_adaptation: 1, postcode: "LS166FT", scheme_id: scheme2.id)
 
     # 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)
@@ -217,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
@@ -230,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