Final changes to import organisations

.github/workflows/pipeline.yml

@@ -122,6 +122,7 @@ jobs:
         GOVUK_NOTIFY_API_KEY: ${{ secrets.GOVUK_NOTIFY_API_KEY }}
         APP_HOST: ${{ secrets.APP_HOST }}
         RAILS_MASTER_KEY: ${{ secrets.RAILS_MASTER_KEY }}
+        IMPORT_PAAS_INSTANCE: ${{ secrets.IMPORT_PAAS_INSTANCE }}
       run: |
         cf7 api $CF_API_ENDPOINT
         cf7 auth
@@ -131,4 +132,5 @@ jobs:
         cf7 set-env $APP_NAME GOVUK_NOTIFY_API_KEY $GOVUK_NOTIFY_API_KEY
         cf7 set-env $APP_NAME APP_HOST $APP_HOST
         cf7 set-env $APP_NAME RAILS_MASTER_KEY $RAILS_MASTER_KEY
+        cf7 set-env $APP_NAME IMPORT_PAAS_INSTANCE $IMPORT_PAAS_INSTANCE
         cf7 push $APP_NAME --strategy rolling

app/services/import_service.rb

@@ -15,7 +15,7 @@ private
 
   def create_organisation(file_io)
     doc = Nokogiri::XML(file_io)
-    Organisation.create!(
+    Organisation.upsert({
       name: field_value(doc, "name"),
       providertype: field_value(doc, "institution-type"),
       phone: field_value(doc, "telephone-number"),
@@ -36,7 +36,7 @@ private
       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")
   end
 
   def field_value(doc, field)

db/migrate/20220203104800_add_unique_index_to_old_visible_id.rb

@@ -0,0 +1,3 @@
+class AddUniqueIndexToOldVisibleId < ActiveRecord::Migration[7.0]
+  add_index :organisations, :old_visible_id, unique: true
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2022_01_31_123638) do
+ActiveRecord::Schema.define(version: 2022_02_03_104800) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -219,6 +219,7 @@ ActiveRecord::Schema.define(version: 2022_01_31_123638) do
     t.integer "unspecified_units"
     t.string "old_org_id"
     t.integer "old_visible_id"
+    t.index ["old_visible_id"], name: "index_organisations_on_old_visible_id", unique: true
   end
 
   create_table "users", force: :cascade do |t|

spec/lib/tasks/data_import/organisations_spec.rb

@@ -5,19 +5,28 @@ 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
-
-    allow(ENV).to receive(:[]).with("VCAP_SERVICE").and_return(vcap_service)
-    allow(ENV).to receive(:[]).with("IMPORT_PAAS_INSTANCE").and_return("my_instance")
-
   end
 
   it "creates an organisation from the given XML file" do
-    expect { task.invoke(fixture_path) }.to change(Organisation, :count).by(1)
-    expect(Organisation.find_by(old_visible_id: 1034).name).to eq("HA Ltd")
+    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/services/import_service_spec.rb

@@ -6,10 +6,11 @@ RSpec.describe ImportService do
   let(:filenames) { %w[my_folder/my_file1.xml my_folder/my_file2.xml] }
   let(:fixture_directory) { "spec/fixtures/softwire_imports/organisations" }
 
-  def create_organisation_file(fixture_directory, visible_id)
+  def create_organisation_file(fixture_directory, visible_id, name = "my_organisation")
     file = File.open("#{fixture_directory}/7c5bd5fb549c09a2c55d7cb90d7ba84927e64618.xml")
     doc =  Nokogiri::XML(file)
     doc.at_xpath("//institution:visible-id").content = visible_id
+    doc.at_xpath("//institution:name").content = name
     StringIO.new(doc.to_xml)
   end
 
@@ -37,4 +38,28 @@ RSpec.describe ImportService do
       expect(Organisation).to exist(old_visible_id: 2)
     end
   end
+
+  context "when importing organisations with duplicate old visible ID" do
+    subject(:import_service) { described_class.new(storage_service) }
+
+    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"),
+      )
+    end
+
+    it "successfully create and update an organisation" 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 { 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_not be_nil
+      expect(organisation.name).to eq("my_new_organisation")
+    end
+  end
 end