Data import refactor

app/services/import_service.rb

@@ -1,4 +1,8 @@
 class ImportService
+  PROVIDER_TYPE = {
+    "HOUSING-ASSOCIATION" => "PRP",
+  }.freeze
+
   def initialize(storage_service, logger = Rails.logger)
     @storage_service = storage_service
     @logger = logger
@@ -8,37 +12,39 @@ class ImportService
     filenames = @storage_service.list_files(folder)
     filenames.each do |filename|
       file_io = @storage_service.get_file_io(filename)
-      create_organisation(file_io)
+      xml_document = Nokogiri::XML(file_io)
+      create_organisation(xml_document)
     end
   end
 
 private
 
-  def create_organisation(file_io)
-    doc = Nokogiri::XML(file_io)
-    name = field_value(doc, "name")
-    old_visible_id = field_value(doc, "visible-id")
+  def create_organisation(xml_document)
+    namespace = "institution"
+    name = field_value(xml_document, namespace, "name")
+    old_visible_id = field_value(xml_document, namespace, "visible-id")
+
     begin
       Organisation.create!(
         name: name,
-        providertype: field_value(doc, "institution-type"),
-        phone: field_value(doc, "telephone-number"),
-        holds_own_stock: to_boolean(field_value(doc, "holds-stock")),
-        active: to_boolean(field_value(doc, "active")),
-        old_association_type: field_value(doc, "old-association-type"),
-        software_supplier_id: field_value(doc, "software-supplier-id"),
-        housing_management_system: field_value(doc, "housing-management-system"),
-        choice_based_lettings: to_boolean(field_value(doc, "choice-based-lettings")),
-        common_housing_register: to_boolean(field_value(doc, "common-housing-register")),
-        choice_allocation_policy: to_boolean(field_value(doc, "choice-allocation-policy")),
-        cbl_proportion_percentage: field_value(doc, "cbl-proportion-percentage"),
-        enter_affordable_logs: to_boolean(field_value(doc, "enter-affordable-logs")),
-        owns_affordable_logs: to_boolean(field_value(doc, "owns-affordable-rent")),
-        housing_registration_no: field_value(doc, "housing-registration-no"),
-        general_needs_units: field_value(doc, "general-needs-units"),
-        supported_housing_units: field_value(doc, "supported-housing-units"),
-        unspecified_units: field_value(doc, "unspecified-units"),
-        old_org_id: field_value(doc, "id"),
+        providertype: map_provider_type(field_value(xml_document, namespace, "institution-type")),
+        phone: field_value(xml_document, namespace, "telephone-number"),
+        holds_own_stock: to_boolean(field_value(xml_document, namespace, "holds-stock")),
+        active: to_boolean(field_value(xml_document, namespace, "active")),
+        old_association_type: field_value(xml_document, namespace, "old-association-type"),
+        software_supplier_id: field_value(xml_document, namespace, "software-supplier-id"),
+        housing_management_system: field_value(xml_document, namespace, "housing-management-system"),
+        choice_based_lettings: to_boolean(field_value(xml_document, namespace, "choice-based-lettings")),
+        common_housing_register: to_boolean(field_value(xml_document, namespace, "common-housing-register")),
+        choice_allocation_policy: to_boolean(field_value(xml_document, namespace, "choice-allocation-policy")),
+        cbl_proportion_percentage: field_value(xml_document, namespace, "cbl-proportion-percentage"),
+        enter_affordable_logs: to_boolean(field_value(xml_document, namespace, "enter-affordable-logs")),
+        owns_affordable_logs: to_boolean(field_value(xml_document, namespace, "owns-affordable-rent")),
+        housing_registration_no: field_value(xml_document, namespace, "housing-registration-no"),
+        general_needs_units: field_value(xml_document, namespace, "general-needs-units"),
+        supported_housing_units: field_value(xml_document, namespace, "supported-housing-units"),
+        unspecified_units: field_value(xml_document, namespace, "unspecified-units"),
+        old_org_id: field_value(xml_document, namespace, "id"),
         old_visible_id: old_visible_id,
       )
     rescue ActiveRecord::RecordNotUnique
@@ -46,8 +52,16 @@ private
     end
   end
 
-  def field_value(doc, field)
-    doc.at_xpath("//institution:#{field}")&.text
+  def map_provider_type(institution_type)
+    if PROVIDER_TYPE.key?(institution_type)
+      PROVIDER_TYPE[institution_type]
+    else
+      institution_type
+    end
+  end
+
+  def field_value(xml_document, namespace, field)
+    xml_document.at_xpath("//#{namespace}:#{field}")&.text
   end
 
   def to_boolean(input_string)

app/services/storage_service.rb

@@ -3,7 +3,7 @@ class StorageService
 
   def initialize(paas_config_service, paas_instance_name)
     @paas_config_service = paas_config_service
-    @paas_instance_name = paas_instance_name.to_sym
+    @paas_instance_name = (paas_instance_name || "").to_sym
     @configuration = create_configuration
     @client = create_client
   end

lib/tasks/data_import.rake

@@ -0,0 +1,20 @@
+require "nokogiri"
+
+namespace :core do
+  desc "Import data XMLs from Softwire system"
+  task :data_import, %i[type path] => :environment do |_task, args|
+    type = args[:type]
+    path = args[:path]
+    raise "Usage: rake core:data_import['data_type', 'path/to/xml_files']" if path.blank? || type.blank?
+
+    storage_service = StorageService.new(PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
+    import_service = ImportService.new(storage_service)
+
+    case type
+    when "organisation"
+      import_service.update_organisations(path)
+    else
+      raise "Type #{type} is not supported by data_import"
+    end
+  end
+end

lib/tasks/data_import/organisations.rake

@@ -1,13 +0,0 @@
-require "nokogiri"
-
-namespace :data_import do
-  desc "Import Organisation XMLs from Softwire system"
-
-  # rake data_import:organisations['path/to/xml_files']
-  task :organisations, %i[path] => :environment do |_task, args|
-    directory = args.path
-    storage_service = StorageService.new(PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
-    import_service = ImportService.new(storage_service)
-    import_service.update_organisations(directory)
-  end
-end

spec/lib/tasks/data_import/organisations_spec.rb

@@ -1,32 +0,0 @@
-require "rails_helper"
-require "rake"
-
-describe "rake data_import:organisations", type: :task do
-  subject(:task) { Rake::Task["data_import:organisations"] }
-
-  let(:fixture_path) { "spec/fixtures/softwire_imports/organisations" }
-  let(:instance_name) { "my_instance" }
-  let(:storage_service) { instance_double(StorageService) }
-  let(:paas_config_service) { instance_double(PaasConfigurationService) }
-  let(:import_service) { instance_double(ImportService) }
-
-  before do
-    allow(StorageService).to receive(:new).and_return(storage_service)
-    allow(PaasConfigurationService).to receive(:new).and_return(paas_config_service)
-    allow(ImportService).to receive(:new).and_return(import_service)
-    allow(ENV).to receive(:[])
-    allow(ENV).to receive(:[]).with("IMPORT_PAAS_INSTANCE").and_return(instance_name)
-
-    Rake.application.rake_require("tasks/data_import/organisations")
-    Rake::Task.define_task(:environment)
-    task.reenable
-  end
-
-  it "creates an organisation from the given XML file" do
-    expect(StorageService).to receive(:new).with(paas_config_service, instance_name)
-    expect(ImportService).to receive(:new).with(storage_service)
-    expect(import_service).to receive(:update_organisations).with(fixture_path)
-
-    task.invoke(fixture_path)
-  end
-end

spec/lib/tasks/data_import_spec.rb

@@ -0,0 +1,48 @@
+require "rails_helper"
+require "rake"
+
+describe "rake core:data_import", type: :task do
+  subject(:task) { Rake::Task["core:data_import"] }
+
+  let(:fixture_path) { "spec/fixtures/softwire_imports/organisations" }
+  let(:instance_name) { "my_instance" }
+  let(:organisation_type) { "organisation" }
+
+  let(:storage_service) { instance_double(StorageService) }
+  let(:paas_config_service) { instance_double(PaasConfigurationService) }
+  let(:import_service) { instance_double(ImportService) }
+
+  before do
+    Rake.application.rake_require("tasks/data_import")
+    Rake::Task.define_task(:environment)
+    task.reenable
+
+    allow(StorageService).to receive(:new).and_return(storage_service)
+    allow(PaasConfigurationService).to receive(:new).and_return(paas_config_service)
+    allow(ImportService).to receive(:new).and_return(import_service)
+    allow(ENV).to receive(:[])
+    allow(ENV).to receive(:[]).with("IMPORT_PAAS_INSTANCE").and_return(instance_name)
+  end
+
+  context "when importing organisation data" do
+    it "creates an organisation from the given XML file" do
+      expect(StorageService).to receive(:new).with(paas_config_service, instance_name)
+      expect(ImportService).to receive(:new).with(storage_service)
+      expect(import_service).to receive(:update_organisations).with(fixture_path)
+
+      task.invoke(organisation_type, fixture_path)
+    end
+  end
+
+  it "raises an exception if no parameters are provided" do
+    expect { task.invoke }.to raise_error(/Usage/)
+  end
+
+  it "raises an exception if a single parameter is provided" do
+    expect { task.invoke("one_parameter") }.to raise_error(/Usage/)
+  end
+
+  it "raises an exception if the type is not supported" do
+    expect { task.invoke("unknown_type", "my_path") }.to raise_error(/Type unknown_type is not supported/)
+  end
+end