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

@@ -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)

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

spec/fixtures/imports/institution/8c5bd5fb549c09a2c55d7cb90d7ba84927e64618.xml

@@ -0,0 +1,23 @@
+<institution xmlns="dclg:institution">
+  <id>8c5bd5fb549c09z2c55d9cb90d7ba84927e64618</id>
+  <name>HA Ltd</name>
+  <institution-type>HOUSING-ASSOCIATION</institution-type>
+  <active>true</active>
+  <almsyn>false</almsyn>
+  <old-association-type>CHHA</old-association-type>
+  <telephone-number>xxxxxxxx</telephone-number>
+  <software-supplier-id>false</software-supplier-id>
+  <housing-management-system/>
+  <choice-based-letting>false</choice-based-letting>
+  <common-housing-register>false</common-housing-register>
+  <choice-allocation-policy>false</choice-allocation-policy>
+  <cbl-proportion-percentage/>
+  <enter-affordable-logs>true</enter-affordable-logs>
+  <owns-affordable-rent>true</owns-affordable-rent>
+  <housing-registration-no>LH9999</housing-registration-no>
+  <visible-id>1034</visible-id>
+  <holds-stock>true</holds-stock>
+  <general-needs-units>1104</general-needs-units>
+  <supported-housing-units>217</supported-housing-units>
+  <unspecified-units>0</unspecified-units>
+</institution>

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)
+  def create_organisation_file(fixture_directory, visible_id, filename, namespace_given, name = nil)
-    file = File.open("#{fixture_directory}/7c5bd5fb549c09a2c55d7cb90d7ba84927e64618.xml")
+    file = File.open("#{fixture_directory}/#{filename}.xml")
     doc =  Nokogiri::XML(file)
-    doc.at_xpath("//institution:visible-id").content = visible_id if visible_id
+    if namespace_given
-    doc.at_xpath("//institution:name").content = name if name
+      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, "7c5bd5fb549c09a2c55d7cb90d7ba84927e64618", true),
-        create_organisation_file(fixture_directory, 1, "my_new_organisation"),
+        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