Migration use kebab org fields (#1913)

* feat: use kebab case org fields

* feat: update tests

app/services/imports/lettings_logs_import_service.rb

@@ -61,8 +61,8 @@ module Imports
       # Required fields for status complete or logic to work
       # Note: order matters when we derive from previous values (attributes parameter)
       attributes["startdate"] = compose_date(xml_doc, "DAY", "MONTH", "YEAR")
-      attributes["owning_organisation_id"] = find_organisation_id(xml_doc, "OWNINGORGID")
-      attributes["managing_organisation_id"] = find_organisation_id(xml_doc, "MANINGORGID")
+      attributes["owning_organisation_id"] = find_organisation_id(xml_doc, "owner-institution-id")
+      attributes["managing_organisation_id"] = find_organisation_id(xml_doc, "managing-institution-id")
       attributes["creation_method"] = creation_method(xml_doc)
       attributes["joint"] = unsafe_string_as_integer(xml_doc, "joint")
       attributes["startertenancy"] = unsafe_string_as_integer(xml_doc, "_2a")

app/services/imports/logs_import_service.rb

@@ -31,9 +31,9 @@ module Imports
     end
 
     def find_organisation_id(xml_doc, id_field)
-      old_visible_id = string_or_nil(xml_doc, id_field)
-      organisation = Organisation.find_by(old_visible_id:)
-      raise "Organisation not found with legacy ID #{old_visible_id}" if organisation.nil?
+      old_org_id = meta_field_value(xml_doc, id_field)&.strip.presence
+      organisation = Organisation.find_by(old_org_id:)
+      raise "Organisation not found with old org ID #{old_org_id}" if organisation.nil?
 
       organisation.id
     end

app/services/imports/sales_logs_field_import_service.rb

@@ -45,7 +45,7 @@ module Imports
         if record.owning_organisation_id.present?
           @logger.info("sales log #{record.id} has a value for owning_organisation_id, skipping update")
         else
-          owning_organisation_id = find_organisation_id(xml_doc, "OWNINGORGID")
+          owning_organisation_id = find_organisation_id(xml_doc, "owner-institution-id")
           record.update!(owning_organisation_id:)
           @logger.info("sales log #{record.id}'s owning_organisation_id value has been set to #{owning_organisation_id}")
         end

app/services/imports/sales_logs_import_service.rb

@@ -27,7 +27,7 @@ module Imports
       # Note: order matters when we derive from previous values (attributes parameter)
 
       attributes["saledate"] = compose_date(xml_doc, "DAY", "MONTH", "YEAR") || Time.zone.parse(field_value(xml_doc, "xmlns", "CompletionDate"))
-      attributes["owning_organisation_id"] = find_organisation_id(xml_doc, "OWNINGORGID")
+      attributes["owning_organisation_id"] = find_organisation_id(xml_doc, "owner-institution-id")
       attributes["type"] = unsafe_string_as_integer(xml_doc, "DerSaleType")
       attributes["old_id"] = meta_field_value(xml_doc, "document-id")
       attributes["old_form_id"] = safe_string_as_integer(xml_doc, "Form")

app/services/imports/scheme_import_service.rb

@@ -54,7 +54,7 @@ module Imports
 
     def find_owning_organisation_id(old_org_id)
       organisation = Organisation.find_by(old_org_id:)
-      raise "Organisation not found with legacy ID #{old_org_id}" if organisation.nil?
+      raise "Organisation not found with old org ID #{old_org_id}" if organisation.nil?
 
       organisation.id
     end

spec/fixtures/imports/logs/00d2343e-d5fa-4c89-8400-ec3854b0f2b4.xml

@@ -4,7 +4,7 @@
     <meta:document-id>00d2343e-d5fa-4c89-8400-ec3854b0f2b4</meta:document-id>
     <meta:owner-user-id>c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa</meta:owner-user-id>
     <meta:owner-institution-id>7c5bd5fb549c09a2c55d7cb90d7ba84927e64618</meta:owner-institution-id>
-    <meta:managing-institution-id>7c5bd5fb549c09a2c55d7cb90d7ba84927e64618</meta:managing-institution-id>
+    <meta:managing-institution-id>7c5bd5fb549c09z2c55d9cb90d7ba84927e64618</meta:managing-institution-id>
     <meta:created-date>2022-04-14T16:01:30.369241Z</meta:created-date>
     <meta:modified-date>2022-04-14T16:01:30.369241Z</meta:modified-date>
     <meta:status>submitted-valid</meta:status>

spec/fixtures/imports/logs/0b4a68df-30cc-474a-93c0-a56ce8fdad3b.xml

@@ -3,7 +3,7 @@
     <meta:form-name>2021-CORE-SR-SH</meta:form-name>
     <meta:document-id>0b4a68df-30cc-474a-93c0-a56ce8fdad3b</meta:document-id>
     <meta:owner-user-id>c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa</meta:owner-user-id>
-    <meta:owner-institution-id>7c5bd5fb549c09z2c55d9cb90d7ba84927e64618</meta:owner-institution-id>
+    <meta:owner-institution-id>7c5bd5fb549c09a2c55d7cb90d7ba84927e64618</meta:owner-institution-id>
     <meta:managing-institution-id>7c5bd5fb549c09z2c55d9cb90d7ba84927e64618</meta:managing-institution-id>
     <meta:created-date>2022-01-05T12:50:20.39153Z</meta:created-date>
     <meta:modified-date>2022-01-05T12:50:20.39153Z</meta:modified-date>

spec/fixtures/imports/logs/0ead17cb-1668-442d-898c-0d52879ff592.xml

@@ -19,7 +19,7 @@
     </meta:owner-user-id>
     <meta:owner-institution-id>7c5bd5fb549c09a2c55d7cb90d7ba84927e64618
     </meta:owner-institution-id>
-    <meta:managing-institution-id>7c5bd5fb549c09a2c55d7cb90d7ba84927e64618
+    <meta:managing-institution-id>7c5bd5fb549c09z2c55d9cb90d7ba84927e64618
     </meta:managing-institution-id>
     <meta:created-date>2021-10-08T14:48:17.096123Z</meta:created-date>
     <meta:modified-date>2021-10-08T14:48:17.096123Z</meta:modified-date>

spec/fixtures/imports/sales_logs/discounted_ownership_sales_log.xml

@@ -3,8 +3,8 @@
     <meta:form-name>2022-CORE-Sales</meta:form-name>
     <meta:document-id>discounted_ownership_sales_log</meta:document-id>
     <meta:owner-user-id>c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa</meta:owner-user-id>
-    <meta:owner-institution-id>7c5bd5fb549c09z2c55d9cb90d7ba84927e64618</meta:owner-institution-id>
-    <meta:managing-institution-id>7c5bd5fb549c09z2c55d9cb90d7ba84927e64618</meta:managing-institution-id>
+    <meta:owner-institution-id>7c5bd5fb549c09a2c55d7cb90d7ba84927e64618</meta:owner-institution-id>
+    <meta:managing-institution-id>7c5bd5fb549c09a2c55d7cb90d7ba84927e64618</meta:managing-institution-id>
     <meta:created-date>2023-02-21T11:54:51.786722Z</meta:created-date>
     <meta:modified-date>2023-02-22T10:59:45.88188Z</meta:modified-date>
     <meta:status>submitted-valid</meta:status>

spec/fixtures/imports/sales_logs/lettings_log.xml

@@ -3,8 +3,8 @@
     <meta:form-name>2021-CORE-SR-SH</meta:form-name>
     <meta:document-id>lettings_log</meta:document-id>
     <meta:owner-user-id>c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa</meta:owner-user-id>
-    <meta:owner-institution-id>7c5bd5fb549c09z2c55d9cb90d7ba84927e64618</meta:owner-institution-id>
-    <meta:managing-institution-id>7c5bd5fb549c09z2c55d9cb90d7ba84927e64618</meta:managing-institution-id>
+    <meta:owner-institution-id>7c5bd5fb549c09a2c55d7cb90d7ba84927e64618</meta:owner-institution-id>
+    <meta:managing-institution-id>7c5bd5fb549c09a2c55d7cb90d7ba84927e64618</meta:managing-institution-id>
     <meta:created-date>2022-01-05T12:50:20.39153Z</meta:created-date>
     <meta:modified-date>2022-01-05T12:50:20.39153Z</meta:modified-date>
     <meta:status>submitted-valid</meta:status>

spec/services/imports/lettings_logs_import_service_spec.rb

@@ -22,8 +22,8 @@ RSpec.describe Imports::LettingsLogsImportService do
     let(:real_2022_2023_form) { Form.new("config/forms/2022_2023.json") }
     let(:fixture_directory) { "spec/fixtures/imports/logs" }
 
-    let(:organisation) { FactoryBot.create(:organisation, old_visible_id: "1", provider_type: "PRP") }
-    let(:managing_organisation) { FactoryBot.create(:organisation, old_visible_id: "2", provider_type: "PRP") }
+    let(:organisation) { FactoryBot.create(:organisation, old_org_id: "7c5bd5fb549c09a2c55d7cb90d7ba84927e64618", provider_type: "PRP") }
+    let(:managing_organisation) { FactoryBot.create(:organisation, old_org_id: "7c5bd5fb549c09z2c55d9cb90d7ba84927e64618", provider_type: "PRP") }
     let(:scheme1) { FactoryBot.create(:scheme, old_visible_id: "0123", owning_organisation: organisation) }
     let(:scheme2) { FactoryBot.create(:scheme, old_visible_id: "456", owning_organisation: organisation) }
 
@@ -36,8 +36,8 @@ RSpec.describe Imports::LettingsLogsImportService do
              .to_return(status: 200, body: '{"status":200,"result":{"admin_district":"Westminster","codes":{"admin_district":"E08000035"}}}', headers: {})
 
       allow(Organisation).to receive(:find_by).and_return(nil)
-      allow(Organisation).to receive(:find_by).with(old_visible_id: organisation.old_visible_id).and_return(organisation)
-      allow(Organisation).to receive(:find_by).with(old_visible_id: managing_organisation.old_visible_id).and_return(managing_organisation)
+      allow(Organisation).to receive(:find_by).with(old_org_id: organisation.old_org_id).and_return(organisation)
+      allow(Organisation).to receive(:find_by).with(old_org_id: managing_organisation.old_org_id).and_return(managing_organisation)
 
       # Created by users
       FactoryBot.create(:user, old_user_id: "c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa", organisation:)
@@ -231,11 +231,11 @@ RSpec.describe Imports::LettingsLogsImportService do
       end
 
       context "and the organisation legacy ID does not exist" do
-        before { lettings_log_xml.at_xpath("//xmlns:OWNINGORGID").content = 99_999 }
+        before { lettings_log_xml.at_xpath("//meta:owner-institution-id").content = 99_999 }
 
         it "raises an exception" do
           expect { lettings_log_service.send(:create_log, lettings_log_xml) }
-            .to raise_error(RuntimeError, "Organisation not found with legacy ID 99999")
+            .to raise_error(RuntimeError, "Organisation not found with old org ID 99999")
         end
       end
 
@@ -1547,8 +1547,8 @@ RSpec.describe Imports::LettingsLogsImportService do
     let(:real_2022_2023_form) { Form.new("config/forms/2022_2023.json") }
     let(:fixture_directory) { "spec/fixtures/imports/logs" }
 
-    let(:organisation) { FactoryBot.create(:organisation, old_visible_id: "1", provider_type: "PRP") }
-    let(:managing_organisation) { FactoryBot.create(:organisation, old_visible_id: "2", provider_type: "PRP") }
+    let(:organisation) { FactoryBot.create(:organisation, old_org_id: "7c5bd5fb549c09a2c55d7cb90d7ba84927e64618", provider_type: "PRP") }
+    let(:managing_organisation) { FactoryBot.create(:organisation, old_org_id: "7c5bd5fb549c09z2c55d9cb90d7ba84927e64618", provider_type: "PRP") }
     let(:scheme1) { FactoryBot.create(:scheme, old_visible_id: "0123", owning_organisation: organisation) }
     let(:scheme2) { FactoryBot.create(:scheme, old_visible_id: "456", owning_organisation: organisation) }
 
@@ -1561,8 +1561,8 @@ RSpec.describe Imports::LettingsLogsImportService do
              .to_return(status: 200, body: '{"status":200,"result":{"admin_district":"Westminster","codes":{"admin_district":"E08000035"}}}', headers: {})
 
       allow(Organisation).to receive(:find_by).and_return(nil)
-      allow(Organisation).to receive(:find_by).with(old_visible_id: organisation.old_visible_id).and_return(organisation)
-      allow(Organisation).to receive(:find_by).with(old_visible_id: managing_organisation.old_visible_id).and_return(managing_organisation)
+      allow(Organisation).to receive(:find_by).with(old_org_id: organisation.old_org_id).and_return(organisation)
+      allow(Organisation).to receive(:find_by).with(old_org_id: managing_organisation.old_org_id).and_return(managing_organisation)
 
       # Created by users
       FactoryBot.create(:user, old_user_id: "c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa", organisation:)
@@ -1792,7 +1792,7 @@ RSpec.describe Imports::LettingsLogsImportService do
         .not_to raise_error
       end
 
-      it "clears out the referral answer" do
+      it "clears out the period answer" do
         allow(logger).to receive(:warn)
         lettings_log_service.send(:create_log, lettings_log_xml)
         lettings_log = LettingsLog.find_by(old_id: lettings_log_id)

spec/services/imports/sales_logs_field_import_service_spec.rb

@@ -9,7 +9,7 @@ RSpec.describe Imports::SalesLogsFieldImportService do
   let(:fixture_directory) { "spec/fixtures/imports/sales_logs" }
   let(:sales_log_filename) { "shared_ownership_sales_log" }
   let(:sales_log_file) { File.open("#{fixture_directory}/#{sales_log_filename}.xml") }
-  let(:organisation) { create(:organisation, old_visible_id: "1") }
+  let(:organisation) { create(:organisation, old_visible_id: "1", old_org_id: "7c5bd5fb549c09a2c55d7cb90d7ba84927e64618") }
   let(:old_user_id) { "c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa" }
 
   let(:remote_folder) { "sales_logs" }

spec/services/imports/sales_logs_import_service_spec.rb

@@ -8,8 +8,8 @@ RSpec.describe Imports::SalesLogsImportService do
 
   let(:fixture_directory) { "spec/fixtures/imports/sales_logs" }
 
-  let(:organisation) { FactoryBot.create(:organisation, old_visible_id: "1", provider_type: "PRP") }
-  let(:managing_organisation) { FactoryBot.create(:organisation, old_visible_id: "2", provider_type: "PRP") }
+  let(:organisation) { FactoryBot.create(:organisation, old_org_id: "7c5bd5fb549c09a2c55d7cb90d7ba84927e64618", provider_type: "PRP") }
+  let(:managing_organisation) { FactoryBot.create(:organisation, old_org_id: "7c5bd5fb549c09z2c55d9cb90d7ba84927e64618", provider_type: "PRP") }
   let(:remote_folder) { "sales_logs" }
 
   def open_file(directory, filename)
@@ -27,8 +27,8 @@ RSpec.describe Imports::SalesLogsImportService do
 
   before do
     allow(Organisation).to receive(:find_by).and_return(nil)
-    allow(Organisation).to receive(:find_by).with(old_visible_id: organisation.old_visible_id).and_return(organisation)
-    allow(Organisation).to receive(:find_by).with(old_visible_id: managing_organisation.old_visible_id).and_return(managing_organisation)
+    allow(Organisation).to receive(:find_by).with(old_org_id: organisation.old_org_id).and_return(organisation)
+    allow(Organisation).to receive(:find_by).with(old_org_id: managing_organisation.old_org_id).and_return(managing_organisation)
 
     # Created by users
     FactoryBot.create(:user, old_user_id: "c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa", organisation:)
@@ -116,11 +116,11 @@ RSpec.describe Imports::SalesLogsImportService do
     context "and the organisation legacy ID does not exist" do
       let(:sales_log_id) { "shared_ownership_sales_log" }
 
-      before { sales_log_xml.at_xpath("//xmlns:OWNINGORGID").content = 99_999 }
+      before { sales_log_xml.at_xpath("//meta:owner-institution-id").content = 99_999 }
 
       it "raises an exception" do
         expect { sales_log_service.send(:create_log, sales_log_xml) }
-          .to raise_error(RuntimeError, "Organisation not found with legacy ID 99999")
+          .to raise_error(RuntimeError, "Organisation not found with old org ID 99999")
       end
     end