From ef08519de290e85d23620841d39a7a0449f76f89 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com> Date: Wed, 30 Aug 2023 16:41:48 +0100 Subject: [PATCH] feat: update org importer to handle xmls without institution namespace (#1879) * feat: update org importer to handle xmls without institution namespace * refactor: lint * refactor: lint * feat: remove erroneously added file --- app/services/imports/import_service.rb | 6 +- .../imports/organisation_import_service.rb | 2 + ...5bd5fb549c09a2c55d7cb90d7ba84927e64618.xml | 23 ++++++ .../organisation_import_service_spec.rb | 74 +++++++++++++++++-- 4 files changed, 96 insertions(+), 9 deletions(-) create mode 100644 spec/fixtures/imports/institution/8c5bd5fb549c09a2c55d7cb90d7ba84927e64618.xml diff --git a/app/services/imports/import_service.rb b/app/services/imports/import_service.rb index 2a2e0f3a6..d631c7fe4 100644 --- a/app/services/imports/import_service.rb +++ b/app/services/imports/import_service.rb @@ -23,7 +23,11 @@ module Imports end def field_value(xml_document, namespace, field, *args) - xml_document.at_xpath("//#{namespace}:#{field}", *args)&.text + if namespace.present? + xml_document.at_xpath("//#{namespace}:#{field}", *args)&.text + else + xml_document.at_xpath("//dclg:#{field}", *args + [{ "dclg" => "dclg:institution" }])&.text + end end def meta_field_value(xml_document, field) diff --git a/app/services/imports/organisation_import_service.rb b/app/services/imports/organisation_import_service.rb index 1ec1575bc..d48038bfd 100644 --- a/app/services/imports/organisation_import_service.rb +++ b/app/services/imports/organisation_import_service.rb @@ -46,6 +46,8 @@ module Imports def organisation_field_value(xml_document, field) field_value(xml_document, "institution", field) + rescue Nokogiri::SyntaxError + field_value(xml_document, nil, field) end end end diff --git a/spec/fixtures/imports/institution/8c5bd5fb549c09a2c55d7cb90d7ba84927e64618.xml b/spec/fixtures/imports/institution/8c5bd5fb549c09a2c55d7cb90d7ba84927e64618.xml new file mode 100644 index 000000000..bd0bc58e2 --- /dev/null +++ b/spec/fixtures/imports/institution/8c5bd5fb549c09a2c55d7cb90d7ba84927e64618.xml @@ -0,0 +1,23 @@ + + 8c5bd5fb549c09z2c55d9cb90d7ba84927e64618 + HA Ltd + HOUSING-ASSOCIATION + true + false + CHHA + xxxxxxxx + false + + false + false + false + + true + true + LH9999 + 1034 + true + 1104 + 217 + 0 + diff --git a/spec/services/imports/organisation_import_service_spec.rb b/spec/services/imports/organisation_import_service_spec.rb index 41e843e3c..fd02d8eda 100644 --- a/spec/services/imports/organisation_import_service_spec.rb +++ b/spec/services/imports/organisation_import_service_spec.rb @@ -8,11 +8,17 @@ RSpec.describe Imports::OrganisationImportService do let(:filenames) { %w[my_folder/my_file1.xml my_folder/my_file2.xml] } let(:fixture_directory) { "spec/fixtures/imports/institution" } - def create_organisation_file(fixture_directory, visible_id, name = nil) - file = File.open("#{fixture_directory}/7c5bd5fb549c09a2c55d7cb90d7ba84927e64618.xml") + def create_organisation_file(fixture_directory, visible_id, filename, namespace_given, name = nil) + file = File.open("#{fixture_directory}/#{filename}.xml") doc = Nokogiri::XML(file) - doc.at_xpath("//institution:visible-id").content = visible_id if visible_id - doc.at_xpath("//institution:name").content = name if name + if namespace_given + doc.at_xpath("//institution:visible-id").content = visible_id if visible_id + doc.at_xpath("//institution:name").content = name if name + else + doc.at_xpath("//dclg:visible-id", { "dclg" => "dclg:institution" }).content = visible_id if visible_id + doc.at_xpath("//dclg:name", { "dclg" => "dclg:institution" }).content = name if name + end + StringIO.new(doc.to_xml) end @@ -24,10 +30,10 @@ RSpec.describe Imports::OrganisationImportService do .and_return(filenames) allow(storage_service).to receive(:get_file_io) .with(filenames[0]) - .and_return(create_organisation_file(fixture_directory, 1)) + .and_return(create_organisation_file(fixture_directory, 1, "7c5bd5fb549c09a2c55d7cb90d7ba84927e64618", true)) allow(storage_service).to receive(:get_file_io) .with(filenames[1]) - .and_return(create_organisation_file(fixture_directory, 2)) + .and_return(create_organisation_file(fixture_directory, 2, "7c5bd5fb549c09a2c55d7cb90d7ba84927e64618", true)) end it "successfully create an organisation with the expected data" do @@ -74,8 +80,8 @@ RSpec.describe Imports::OrganisationImportService do before do allow(storage_service).to receive(:list_files).and_return([filenames[0]]) allow(storage_service).to receive(:get_file_io).and_return( - create_organisation_file(fixture_directory, 1), - create_organisation_file(fixture_directory, 1, "my_new_organisation"), + create_organisation_file(fixture_directory, 1, "7c5bd5fb549c09a2c55d7cb90d7ba84927e64618", true), + create_organisation_file(fixture_directory, 1, "7c5bd5fb549c09a2c55d7cb90d7ba84927e64618", true, "my_new_organisation"), ) end @@ -92,4 +98,56 @@ RSpec.describe Imports::OrganisationImportService do expect(Organisation).to exist(old_visible_id: "1", name: "HA Ltd") end end + + context "when importing organisations with no namespace" do + subject(:import_service) { described_class.new(storage_service) } + + before do + allow(storage_service).to receive(:list_files) + .and_return(filenames) + allow(storage_service).to receive(:get_file_io) + .with(filenames[0]) + .and_return(create_organisation_file(fixture_directory, 1, "8c5bd5fb549c09a2c55d7cb90d7ba84927e64618", false)) + allow(storage_service).to receive(:get_file_io) + .with(filenames[1]) + .and_return(create_organisation_file(fixture_directory, 2, "8c5bd5fb549c09a2c55d7cb90d7ba84927e64618", false)) + end + + it "successfully create an organisation with the expected data" do + import_service.create_organisations(folder_name) + + organisation = Organisation.find_by(old_visible_id: "1") + expect(organisation.name).to eq("HA Ltd") + expect(organisation.provider_type).to eq("PRP") + expect(organisation.phone).to eq("xxxxxxxx") + expect(organisation.holds_own_stock).to be_truthy + expect(organisation.active).to be_truthy + # expect(organisation.old_association_type).to eq() string VS integer + # expect(organisation.software_supplier_id).to eq() boolean VS string + expect(organisation.housing_management_system).to eq("") # Need examples + expect(organisation.choice_based_lettings).to be_falsey + expect(organisation.common_housing_register).to be_falsey + expect(organisation.choice_allocation_policy).to be_falsey + expect(organisation.cbl_proportion_percentage).to be_nil # Need example + expect(organisation.enter_affordable_logs).to be_truthy + expect(organisation.owns_affordable_logs).to be_truthy # owns_affordable_rent + expect(organisation.housing_registration_no).to eq("LH9999") + expect(organisation.general_needs_units).to eq(1104) + expect(organisation.supported_housing_units).to eq(217) + expect(organisation.unspecified_units).to eq(0) + expect(organisation.unspecified_units).to eq(0) + expect(organisation.old_org_id).to eq("8c5bd5fb549c09z2c55d9cb90d7ba84927e64618") + expect(organisation.old_visible_id).to eq("1") + end + + it "successfully create multiple organisations" do + expect(storage_service).to receive(:list_files).with(folder_name) + expect(storage_service).to receive(:get_file_io).with(filenames[0]).ordered + expect(storage_service).to receive(:get_file_io).with(filenames[1]).ordered + + expect { import_service.create_organisations(folder_name) }.to change(Organisation, :count).by(2) + expect(Organisation).to exist(old_visible_id: "1") + expect(Organisation).to exist(old_visible_id: "2") + end + end end