diff --git a/app/services/import_service.rb b/app/services/import_service.rb index 7bd32ed77..6d570e1c3 100644 --- a/app/services/import_service.rb +++ b/app/services/import_service.rb @@ -1,6 +1,7 @@ class ImportService - def initialize(storage_service) + def initialize(storage_service, logger = Rails.logger) @storage_service = storage_service + @logger = logger end def update_organisations(folder) @@ -15,28 +16,34 @@ private def create_organisation(file_io) doc = Nokogiri::XML(file_io) - Organisation.upsert({ - name: field_value(doc, "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"), - old_visible_id: field_value(doc, "visible-id"), - }, unique_by: "old_visible_id") + name = field_value(doc, "name") + old_visible_id = field_value(doc, "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"), + old_visible_id: old_visible_id, + ) + rescue ActiveRecord::RecordNotUnique + @logger.warn("Organisation #{name} is already present with old visible ID #{old_visible_id}, skipping.") + end end def field_value(doc, field) diff --git a/spec/services/import_service_spec.rb b/spec/services/import_service_spec.rb index 5c8be8d7b..a72f77326 100644 --- a/spec/services/import_service_spec.rb +++ b/spec/services/import_service_spec.rb @@ -2,6 +2,7 @@ require "rails_helper" RSpec.describe ImportService do let(:storage_service) { instance_double(StorageService) } + let(:logger) { instance_double(Rails::Rack::Logger) } let(:folder_name) { "organisations" } let(:filenames) { %w[my_folder/my_file1.xml my_folder/my_file2.xml] } let(:fixture_directory) { "spec/fixtures/softwire_imports/organisations" } @@ -39,8 +40,8 @@ RSpec.describe ImportService do end end - context "when importing organisations with duplicate old visible ID" do - subject(:import_service) { described_class.new(storage_service) } + context "when importing organisations twice" do + subject(:import_service) { described_class.new(storage_service, logger) } before do allow(storage_service).to receive(:list_files).and_return([filenames[0]]) @@ -50,16 +51,15 @@ RSpec.describe ImportService do ) end - it "successfully create and update an organisation" do + it "successfully create an organisation the first time, and does not update it" do expect(storage_service).to receive(:list_files).with(folder_name).twice expect(storage_service).to receive(:get_file_io).with(filenames[0]).twice + expect(logger).to receive(:warn).once expect { import_service.update_organisations(folder_name) }.to change(Organisation, :count).by(1) expect { import_service.update_organisations(folder_name) }.to change(Organisation, :count).by(0) - organisation = Organisation.find_by(old_visible_id: 1) - expect(organisation).not_to be_nil - expect(organisation.name).to eq("my_new_organisation") + expect(Organisation).to exist(old_visible_id: 1, name: "my_organisation") end end end