From 20337fde59481e6d487612284024d2d274ab5cdd Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com> Date: Wed, 13 Sep 2023 16:07:04 +0100 Subject: [PATCH] Migration use kebab org fields (#1913) * feat: use kebab case org fields * feat: update tests --- .../imports/lettings_logs_import_service.rb | 4 ++-- app/services/imports/logs_import_service.rb | 6 ++--- .../sales_logs_field_import_service.rb | 2 +- .../imports/sales_logs_import_service.rb | 2 +- app/services/imports/scheme_import_service.rb | 2 +- .../00d2343e-d5fa-4c89-8400-ec3854b0f2b4.xml | 2 +- .../0b4a68df-30cc-474a-93c0-a56ce8fdad3b.xml | 2 +- .../0ead17cb-1668-442d-898c-0d52879ff592.xml | 2 +- .../discounted_ownership_sales_log.xml | 4 ++-- .../imports/sales_logs/lettings_log.xml | 4 ++-- .../lettings_logs_import_service_spec.rb | 22 +++++++++---------- .../sales_logs_field_import_service_spec.rb | 2 +- .../imports/sales_logs_import_service_spec.rb | 12 +++++----- 13 files changed, 33 insertions(+), 33 deletions(-) diff --git a/app/services/imports/lettings_logs_import_service.rb b/app/services/imports/lettings_logs_import_service.rb index d40df0fc1..1d3a3c3c7 100644 --- a/app/services/imports/lettings_logs_import_service.rb +++ b/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") diff --git a/app/services/imports/logs_import_service.rb b/app/services/imports/logs_import_service.rb index f7456a3fb..3b08f57ed 100644 --- a/app/services/imports/logs_import_service.rb +++ b/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 diff --git a/app/services/imports/sales_logs_field_import_service.rb b/app/services/imports/sales_logs_field_import_service.rb index 2a08a7ccf..4cda93d9d 100644 --- a/app/services/imports/sales_logs_field_import_service.rb +++ b/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 diff --git a/app/services/imports/sales_logs_import_service.rb b/app/services/imports/sales_logs_import_service.rb index c57f2d16a..b406c1c8b 100644 --- a/app/services/imports/sales_logs_import_service.rb +++ b/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") diff --git a/app/services/imports/scheme_import_service.rb b/app/services/imports/scheme_import_service.rb index b0b8f3ceb..7000c391f 100644 --- a/app/services/imports/scheme_import_service.rb +++ b/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 diff --git a/spec/fixtures/imports/logs/00d2343e-d5fa-4c89-8400-ec3854b0f2b4.xml b/spec/fixtures/imports/logs/00d2343e-d5fa-4c89-8400-ec3854b0f2b4.xml index cd0e03d74..50f4a4d16 100644 --- a/spec/fixtures/imports/logs/00d2343e-d5fa-4c89-8400-ec3854b0f2b4.xml +++ b/spec/fixtures/imports/logs/00d2343e-d5fa-4c89-8400-ec3854b0f2b4.xml @@ -4,7 +4,7 @@ 00d2343e-d5fa-4c89-8400-ec3854b0f2b4 c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa 7c5bd5fb549c09a2c55d7cb90d7ba84927e64618 - 7c5bd5fb549c09a2c55d7cb90d7ba84927e64618 + 7c5bd5fb549c09z2c55d9cb90d7ba84927e64618 2022-04-14T16:01:30.369241Z 2022-04-14T16:01:30.369241Z submitted-valid diff --git a/spec/fixtures/imports/logs/0b4a68df-30cc-474a-93c0-a56ce8fdad3b.xml b/spec/fixtures/imports/logs/0b4a68df-30cc-474a-93c0-a56ce8fdad3b.xml index 38cef1339..8d9465305 100644 --- a/spec/fixtures/imports/logs/0b4a68df-30cc-474a-93c0-a56ce8fdad3b.xml +++ b/spec/fixtures/imports/logs/0b4a68df-30cc-474a-93c0-a56ce8fdad3b.xml @@ -3,7 +3,7 @@ 2021-CORE-SR-SH 0b4a68df-30cc-474a-93c0-a56ce8fdad3b c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa - 7c5bd5fb549c09z2c55d9cb90d7ba84927e64618 + 7c5bd5fb549c09a2c55d7cb90d7ba84927e64618 7c5bd5fb549c09z2c55d9cb90d7ba84927e64618 2022-01-05T12:50:20.39153Z 2022-01-05T12:50:20.39153Z diff --git a/spec/fixtures/imports/logs/0ead17cb-1668-442d-898c-0d52879ff592.xml b/spec/fixtures/imports/logs/0ead17cb-1668-442d-898c-0d52879ff592.xml index 87797e242..aba2b2256 100644 --- a/spec/fixtures/imports/logs/0ead17cb-1668-442d-898c-0d52879ff592.xml +++ b/spec/fixtures/imports/logs/0ead17cb-1668-442d-898c-0d52879ff592.xml @@ -19,7 +19,7 @@ 7c5bd5fb549c09a2c55d7cb90d7ba84927e64618 - 7c5bd5fb549c09a2c55d7cb90d7ba84927e64618 + 7c5bd5fb549c09z2c55d9cb90d7ba84927e64618 2021-10-08T14:48:17.096123Z 2021-10-08T14:48:17.096123Z diff --git a/spec/fixtures/imports/sales_logs/discounted_ownership_sales_log.xml b/spec/fixtures/imports/sales_logs/discounted_ownership_sales_log.xml index 0c912dd3c..ec39157c3 100644 --- a/spec/fixtures/imports/sales_logs/discounted_ownership_sales_log.xml +++ b/spec/fixtures/imports/sales_logs/discounted_ownership_sales_log.xml @@ -3,8 +3,8 @@ 2022-CORE-Sales discounted_ownership_sales_log c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa - 7c5bd5fb549c09z2c55d9cb90d7ba84927e64618 - 7c5bd5fb549c09z2c55d9cb90d7ba84927e64618 + 7c5bd5fb549c09a2c55d7cb90d7ba84927e64618 + 7c5bd5fb549c09a2c55d7cb90d7ba84927e64618 2023-02-21T11:54:51.786722Z 2023-02-22T10:59:45.88188Z submitted-valid diff --git a/spec/fixtures/imports/sales_logs/lettings_log.xml b/spec/fixtures/imports/sales_logs/lettings_log.xml index 1f7f794f6..d02fd6052 100644 --- a/spec/fixtures/imports/sales_logs/lettings_log.xml +++ b/spec/fixtures/imports/sales_logs/lettings_log.xml @@ -3,8 +3,8 @@ 2021-CORE-SR-SH lettings_log c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa - 7c5bd5fb549c09z2c55d9cb90d7ba84927e64618 - 7c5bd5fb549c09z2c55d9cb90d7ba84927e64618 + 7c5bd5fb549c09a2c55d7cb90d7ba84927e64618 + 7c5bd5fb549c09a2c55d7cb90d7ba84927e64618 2022-01-05T12:50:20.39153Z 2022-01-05T12:50:20.39153Z submitted-valid diff --git a/spec/services/imports/lettings_logs_import_service_spec.rb b/spec/services/imports/lettings_logs_import_service_spec.rb index efef0997a..ac8dddccd 100644 --- a/spec/services/imports/lettings_logs_import_service_spec.rb +++ b/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) diff --git a/spec/services/imports/sales_logs_field_import_service_spec.rb b/spec/services/imports/sales_logs_field_import_service_spec.rb index f1ae6680c..6b39b3acb 100644 --- a/spec/services/imports/sales_logs_field_import_service_spec.rb +++ b/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" } diff --git a/spec/services/imports/sales_logs_import_service_spec.rb b/spec/services/imports/sales_logs_import_service_spec.rb index 615201f3a..2306448b8 100644 --- a/spec/services/imports/sales_logs_import_service_spec.rb +++ b/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