From c9ca3c7e930bd59556b8e3a04c8e94a3bc412d6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Meny?= Date: Thu, 3 Feb 2022 11:25:11 +0000 Subject: [PATCH] Final changes to import organisations --- .github/workflows/pipeline.yml | 2 ++ app/services/import_service.rb | 4 +-- ...4800_add_unique_index_to_old_visible_id.rb | 3 +++ db/schema.rb | 3 ++- .../tasks/data_import/organisations_spec.rb | 21 ++++++++++----- spec/services/import_service_spec.rb | 27 ++++++++++++++++++- 6 files changed, 50 insertions(+), 10 deletions(-) create mode 100644 db/migrate/20220203104800_add_unique_index_to_old_visible_id.rb diff --git a/.github/workflows/pipeline.yml b/.github/workflows/pipeline.yml index 64dd170fe..9177f349e 100644 --- a/.github/workflows/pipeline.yml +++ b/.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 diff --git a/app/services/import_service.rb b/app/services/import_service.rb index 936b78f07..7bd32ed77 100644 --- a/app/services/import_service.rb +++ b/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) diff --git a/db/migrate/20220203104800_add_unique_index_to_old_visible_id.rb b/db/migrate/20220203104800_add_unique_index_to_old_visible_id.rb new file mode 100644 index 000000000..1805abbbd --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index a56d45399..a65b175d2 100644 --- a/db/schema.rb +++ b/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| diff --git a/spec/lib/tasks/data_import/organisations_spec.rb b/spec/lib/tasks/data_import/organisations_spec.rb index 2e1b93dc8..164b64218 100644 --- a/spec/lib/tasks/data_import/organisations_spec.rb +++ b/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 diff --git a/spec/services/import_service_spec.rb b/spec/services/import_service_spec.rb index bf154ab4d..cfc034d04 100644 --- a/spec/services/import_service_spec.rb +++ b/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