From 76b3ddde5a1c2661fa4a7d8beaede86510954f2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Meny?= Date: Tue, 24 May 2022 14:34:49 +0100 Subject: [PATCH] CLDC-1258: Handle invalid void date during import (#604) * Now we raise an exception if a non-existing organisation is referenced by a case log * The void date is not imported if it is after the tenancy start date --- .../imports/case_logs_import_service.rb | 31 +++---- .../00d2343e-d5fa-4c89-8400-ec3854b0f2b4.xml | 16 ++-- .../0ead17cb-1668-442d-898c-0d52879ff592.xml | 12 +-- .../166fc004-392e-47a8-acb8-1c018734882b.xml | 12 +-- .../imports/case_logs_import_service_spec.rb | 84 ++++++++++++++----- 5 files changed, 97 insertions(+), 58 deletions(-) diff --git a/app/services/imports/case_logs_import_service.rb b/app/services/imports/case_logs_import_service.rb index 123cacc93..e2297e760 100644 --- a/app/services/imports/case_logs_import_service.rb +++ b/app/services/imports/case_logs_import_service.rb @@ -47,8 +47,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", "OWNINGORGNAME", "HCNUM") - attributes["managing_organisation_id"] = find_organisation_id(xml_doc, "MANINGORGID", "MANINGORGNAME", "MANHCNUM") + attributes["owning_organisation_id"] = find_organisation_id(xml_doc, "OWNINGORGID") + attributes["managing_organisation_id"] = find_organisation_id(xml_doc, "MANINGORGID") attributes["joint"] = unsafe_string_as_integer(xml_doc, "joint") attributes["startertenancy"] = unsafe_string_as_integer(xml_doc, "_2a") attributes["tenancy"] = unsafe_string_as_integer(xml_doc, "Q2b") @@ -138,6 +138,8 @@ module Imports 0 end + apply_date_consistency!(attributes) + attributes["offered"] = safe_string_as_integer(xml_doc, "Q20") attributes["propcode"] = string_or_nil(xml_doc, "Q21a") attributes["beds"] = safe_string_as_integer(xml_doc, "Q22") @@ -306,24 +308,11 @@ module Imports end end - def find_organisation_id(xml_doc, id_field, name_field, reg_field) + def find_organisation_id(xml_doc, id_field) old_visible_id = unsafe_string_as_integer(xml_doc, id_field) organisation = Organisation.find_by(old_visible_id:) - # Quick hack: should be removed when all organisations are imported - # Will fail in the future if the organisation is missing - if organisation.nil? - organisation = Organisation.new - organisation.old_visible_id = old_visible_id - let_type = unsafe_string_as_integer(xml_doc, "landlord") - organisation.provider_type = if let_type == PRP_LA[:local_authority] - 1 - else - 2 - end - organisation.name = string_or_nil(xml_doc, name_field) - organisation.housing_registration_no = string_or_nil(xml_doc, reg_field) - organisation.save! - end + raise "Organisation not found with legacy ID #{old_visible_id}" if organisation.nil? + organisation.id end @@ -529,5 +518,11 @@ module Imports 0 end end + + def apply_date_consistency!(attributes) + if attributes["voiddate"] > attributes["startdate"] + attributes.delete("voiddate") + end + end end end diff --git a/spec/fixtures/softwire_imports/case_logs/00d2343e-d5fa-4c89-8400-ec3854b0f2b4.xml b/spec/fixtures/softwire_imports/case_logs/00d2343e-d5fa-4c89-8400-ec3854b0f2b4.xml index f5a5c87e4..e9835d1e1 100644 --- a/spec/fixtures/softwire_imports/case_logs/00d2343e-d5fa-4c89-8400-ec3854b0f2b4.xml +++ b/spec/fixtures/softwire_imports/case_logs/00d2343e-d5fa-4c89-8400-ec3854b0f2b4.xml @@ -3,8 +3,8 @@ 2022-CORE-SR-GN 00d2343e-d5fa-4c89-8400-ec3854b0f2b4 c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa - 35459cb12d312a472d8585b97b865917e8f2144e - 35459cb12d312a472d8585b97b865917e8f2144e + 7c5bd5fb549c09a2c55d7cb90d7ba84927e64618 + 7c5bd5fb549c09a2c55d7cb90d7ba84927e64618 2022-04-14T16:01:30.369241Z 2022-04-14T16:01:30.369241Z submitted-valid @@ -407,13 +407,13 @@ E12000001 - 107178 - Believe Housing Ltd. - Believe Housing Ltd. - 8076 - 8076 + 1 + DLUHC + DLUHC + 655 + 655 - 107178 + 1 3 diff --git a/spec/fixtures/softwire_imports/case_logs/0ead17cb-1668-442d-898c-0d52879ff592.xml b/spec/fixtures/softwire_imports/case_logs/0ead17cb-1668-442d-898c-0d52879ff592.xml index d34952c31..8413abb7e 100644 --- a/spec/fixtures/softwire_imports/case_logs/0ead17cb-1668-442d-898c-0d52879ff592.xml +++ b/spec/fixtures/softwire_imports/case_logs/0ead17cb-1668-442d-898c-0d52879ff592.xml @@ -431,13 +431,13 @@ E12000003 - 1034 - LEEDS & YORKSHIRE HA Ltd - LEEDS & YORKSHIRE HA Ltd - LH0704 - LH0704 + 1 + DLUHC + DLUHC + 655 + 655 - 1034 + 1 diff --git a/spec/fixtures/softwire_imports/case_logs/166fc004-392e-47a8-acb8-1c018734882b.xml b/spec/fixtures/softwire_imports/case_logs/166fc004-392e-47a8-acb8-1c018734882b.xml index bb351d7e7..631f41cae 100644 --- a/spec/fixtures/softwire_imports/case_logs/166fc004-392e-47a8-acb8-1c018734882b.xml +++ b/spec/fixtures/softwire_imports/case_logs/166fc004-392e-47a8-acb8-1c018734882b.xml @@ -3,8 +3,8 @@ 2021-CORE-IR-GN 166fc004-392e-47a8-acb8-1c018734882b e29c492473446dca4d50224f2bb7cf965a261d6f - 26139d1be7a44ebb1af23d3f8882c14025e9903c - 26139d1be7a44ebb1af23d3f8882c14025e9903c + 7c5bd5fb549c09a2c55d7cb90d7ba84927e64618 + 7c5bd5fb549c09a2c55d7cb90d7ba84927e64618 2022-04-12T14:10:59.953121Z 2022-04-12T14:10:59.953121Z submitted-valid @@ -411,13 +411,13 @@ E12000008 - 107138 - 1 Test - 1 Test + 1 + DLUHC + DLUHC 655 655 - 107138 + 1 diff --git a/spec/services/imports/case_logs_import_service_spec.rb b/spec/services/imports/case_logs_import_service_spec.rb index 82f05472c..92da82907 100644 --- a/spec/services/imports/case_logs_import_service_spec.rb +++ b/spec/services/imports/case_logs_import_service_spec.rb @@ -1,22 +1,40 @@ require "rails_helper" RSpec.describe Imports::CaseLogsImportService do - let(:remote_folder) { "case_logs" } - let(:fixture_directory) { "spec/fixtures/softwire_imports/case_logs" } - let(:case_log_id) { "0ead17cb-1668-442d-898c-0d52879ff592" } - let(:case_log_id2) { "166fc004-392e-47a8-acb8-1c018734882b" } - let(:case_log_id3) { "00d2343e-d5fa-4c89-8400-ec3854b0f2b4" } + subject(:case_log_service) { described_class.new(storage_service, logger) } + let(:storage_service) { instance_double(StorageService) } + let(:logger) { instance_double(ActiveSupport::Logger) } + 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(:logger) { instance_double(ActiveSupport::Logger) } + let(:fixture_directory) { "spec/fixtures/softwire_imports/case_logs" } - context "when importing users" do - subject(:case_log_service) { described_class.new(storage_service, logger) } + def open_file(directory, filename) + File.open("#{directory}/#{filename}.xml") + end - def open_file(directory, filename) - File.open("#{directory}/#{filename}.xml") - end + before do + # Owning and Managing organisations + FactoryBot.create(:organisation, old_visible_id: "1", provider_type: "PRP") + + # Created by users + FactoryBot.create(:user, old_user_id: "c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa") + FactoryBot.create(:user, old_user_id: "e29c492473446dca4d50224f2bb7cf965a261d6f") + + # 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) + allow(FormHandler.instance).to receive(:get_form).with("2022_2023").and_return(real_2022_2023_form) + + WebMock.stub_request(:get, /api.postcodes.io\/postcodes\/LS166FT/) + .to_return(status: 200, body: '{"status":200,"result":{"codes":{"admin_district":"E08000035"}}}', headers: {}) + end + + context "when importing users" do + let(:remote_folder) { "case_logs" } + let(:case_log_id) { "0ead17cb-1668-442d-898c-0d52879ff592" } + let(:case_log_id2) { "166fc004-392e-47a8-acb8-1c018734882b" } + let(:case_log_id3) { "00d2343e-d5fa-4c89-8400-ec3854b0f2b4" } before do # Stub the S3 file listing and download @@ -31,15 +49,6 @@ RSpec.describe Imports::CaseLogsImportService do allow(storage_service).to receive(:get_file_io) .with("#{remote_folder}/#{case_log_id3}.xml") .and_return(open_file(fixture_directory, case_log_id3), open_file(fixture_directory, case_log_id3)) - # 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) - allow(FormHandler.instance).to receive(:get_form).with("2022_2023").and_return(real_2022_2023_form) - - WebMock.stub_request(:get, /api.postcodes.io\/postcodes\/LS166FT/) - .to_return(status: 200, body: '{"status":200,"result":{"codes":{"admin_district":"E08000035"}}}', headers: {}) - - FactoryBot.create(:user, old_user_id: "c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa") - FactoryBot.create(:user, old_user_id: "e29c492473446dca4d50224f2bb7cf965a261d6f") end it "successfully create all case logs" do @@ -58,4 +67,39 @@ RSpec.describe Imports::CaseLogsImportService do .to change(CaseLog, :count).by(3) end end + + context "when importing a specific log" do + let(:case_log_id) { "0ead17cb-1668-442d-898c-0d52879ff592" } + let(:case_log_file) { open_file(fixture_directory, case_log_id) } + + context "and the void date is after the start date" do + let(:case_log_xml) do + xml_doc = Nokogiri::XML(case_log_file) + xml_doc.at_xpath("//xmlns:VYEAR").content = 2023 + xml_doc + end + + it "does not import the voiddate" do + allow(logger).to receive(:warn).with(/is not completed/) + case_log_service.send(:create_log, case_log_xml) + + case_log = CaseLog.where(old_id: case_log_id).first + expect(case_log).not_to be_nil + expect(case_log.voiddate).to be_nil + end + end + + context "and the organisation legacy ID does not exist" do + let(:case_log_xml) do + xml_doc = Nokogiri::XML(case_log_file) + xml_doc.at_xpath("//xmlns:OWNINGORGID").content = 99_999 + xml_doc + end + + it "raises an exception" do + expect { case_log_service.send(:create_log, case_log_xml) } + .to raise_error(RuntimeError, "Organisation not found with legacy ID 99999") + end + end + end end