Prevent organisations to be updated when they exist

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")
-      name: field_value(doc, "name"),
+    old_visible_id = field_value(doc, "visible-id")
-      providertype: field_value(doc, "institution-type"),
+    begin
-      phone: field_value(doc, "telephone-number"),
+      Organisation.create!(
-      holds_own_stock: to_boolean(field_value(doc, "holds-stock")),
+        name: name,
-      active: to_boolean(field_value(doc, "active")),
+        providertype: field_value(doc, "institution-type"),
-      old_association_type: field_value(doc, "old-association-type"),
+        phone: field_value(doc, "telephone-number"),
-      software_supplier_id: field_value(doc, "software-supplier-id"),
+        holds_own_stock: to_boolean(field_value(doc, "holds-stock")),
-      housing_management_system: field_value(doc, "housing-management-system"),
+        active: to_boolean(field_value(doc, "active")),
-      choice_based_lettings: to_boolean(field_value(doc, "choice-based-lettings")),
+        old_association_type: field_value(doc, "old-association-type"),
-      common_housing_register: to_boolean(field_value(doc, "common-housing-register")),
+        software_supplier_id: field_value(doc, "software-supplier-id"),
-      choice_allocation_policy: to_boolean(field_value(doc, "choice-allocation-policy")),
+        housing_management_system: field_value(doc, "housing-management-system"),
-      cbl_proportion_percentage: field_value(doc, "cbl-proportion-percentage"),
+        choice_based_lettings: to_boolean(field_value(doc, "choice-based-lettings")),
-      enter_affordable_logs: to_boolean(field_value(doc, "enter-affordable-logs")),
+        common_housing_register: to_boolean(field_value(doc, "common-housing-register")),
-      owns_affordable_logs: to_boolean(field_value(doc, "owns-affordable-rent")),
+        choice_allocation_policy: to_boolean(field_value(doc, "choice-allocation-policy")),
-      housing_registration_no: field_value(doc, "housing-registration-no"),
+        cbl_proportion_percentage: field_value(doc, "cbl-proportion-percentage"),
-      general_needs_units: field_value(doc, "general-needs-units"),
+        enter_affordable_logs: to_boolean(field_value(doc, "enter-affordable-logs")),
-      supported_housing_units: field_value(doc, "supported-housing-units"),
+        owns_affordable_logs: to_boolean(field_value(doc, "owns-affordable-rent")),
-      unspecified_units: field_value(doc, "unspecified-units"),
+        housing_registration_no: field_value(doc, "housing-registration-no"),
-      old_org_id: field_value(doc, "id"),
+        general_needs_units: field_value(doc, "general-needs-units"),
-      old_visible_id: field_value(doc, "visible-id"),
+        supported_housing_units: field_value(doc, "supported-housing-units"),
-    }, unique_by: "old_visible_id")
+        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)

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
+  context "when importing organisations twice" do
-    subject(:import_service) { described_class.new(storage_service) }
+    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).to exist(old_visible_id: 1, name: "my_organisation")
-      expect(organisation).not_to be_nil
-      expect(organisation.name).to eq("my_new_organisation")
     end
   end
 end