From 820a7fe26ea465d60bd7bc92a076bc5d25520be6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Meny?= Date: Fri, 4 Feb 2022 17:04:46 +0000 Subject: [PATCH] Introduce user import --- app/models/case_log.rb | 2 +- app/models/organisation.rb | 2 +- app/services/import_service.rb | 70 ------------------- app/services/imports/import_service.rb | 27 +++++++ .../imports/organisation_import_service.rb | 54 ++++++++++++++ app/services/imports/user_import_service.rb | 21 ++++++ .../20220204134000_additional_user_fields.rb | 13 ++++ db/schema.rb | 3 +- lib/tasks/data_import.rake | 5 +- spec/lib/tasks/data_import_spec.rb | 8 +-- spec/models/case_log_spec.rb | 2 +- spec/models/organisation_spec.rb | 2 +- .../organisation_import_service_spec.rb} | 47 ++++++++++--- 13 files changed, 166 insertions(+), 90 deletions(-) delete mode 100644 app/services/import_service.rb create mode 100644 app/services/imports/import_service.rb create mode 100644 app/services/imports/organisation_import_service.rb create mode 100644 app/services/imports/user_import_service.rb create mode 100644 db/migrate/20220204134000_additional_user_fields.rb rename spec/services/{import_service_spec.rb => imports/organisation_import_service_spec.rb} (55%) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index e68190e10..627ea0eba 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -259,7 +259,7 @@ private end self.hhmemb = other_hhmemb + 1 if other_hhmemb.present? self.renttype = RENT_TYPE_MAPPING[rent_type] - self.lettype = "#{renttype} #{needstype} #{owning_organisation['Org type']}" if renttype.present? && needstype.present? && owning_organisation["Org type"].present? + self.lettype = "#{renttype} #{needstype} #{owning_organisation[:org_type]}" if renttype.present? && needstype.present? && owning_organisation[:org_type].present? self.totchild = get_totchild self.totelder = get_totelder self.totadult = get_totadult diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 6b63b612b..5b082f292 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -4,7 +4,7 @@ class Organisation < ApplicationRecord has_many :managed_case_logs, class_name: "CaseLog", foreign_key: "managing_organisation_id" include Constants::Organisation - enum "Org type": ORG_TYPE, _suffix: true + enum org_type: ORG_TYPE def case_logs CaseLog.for_organisation(self) diff --git a/app/services/import_service.rb b/app/services/import_service.rb deleted file mode 100644 index 081e8b00b..000000000 --- a/app/services/import_service.rb +++ /dev/null @@ -1,70 +0,0 @@ -class ImportService - PROVIDER_TYPE = { - "HOUSING-ASSOCIATION" => "PRP", - }.freeze - - def initialize(storage_service, logger = Rails.logger) - @storage_service = storage_service - @logger = logger - end - - def update_organisations(folder) - filenames = @storage_service.list_files(folder) - filenames.each do |filename| - file_io = @storage_service.get_file_io(filename) - xml_document = Nokogiri::XML(file_io) - create_organisation(xml_document) - end - end - -private - - 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: 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 - @logger.warn("Organisation #{name} is already present with old visible ID #{old_visible_id}, skipping.") - end - end - - 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) - input_string == "true" - end -end diff --git a/app/services/imports/import_service.rb b/app/services/imports/import_service.rb new file mode 100644 index 000000000..8ac8a035d --- /dev/null +++ b/app/services/imports/import_service.rb @@ -0,0 +1,27 @@ +module Imports + class ImportService + private + + def initialize(storage_service, logger = Rails.logger) + @storage_service = storage_service + @logger = logger + end + + def import_from(folder, create_method) + filenames = @storage_service.list_files(folder) + filenames.each do |filename| + file_io = @storage_service.get_file_io(filename) + xml_document = Nokogiri::XML(file_io) + send(create_method, xml_document) + end + end + + def field_value(xml_document, namespace, field) + xml_document.at_xpath("//#{namespace}:#{field}")&.text + end + + def to_boolean(input_string) + input_string == "true" + end + end +end diff --git a/app/services/imports/organisation_import_service.rb b/app/services/imports/organisation_import_service.rb new file mode 100644 index 000000000..358d51c4d --- /dev/null +++ b/app/services/imports/organisation_import_service.rb @@ -0,0 +1,54 @@ +module Imports + class OrganisationImportService < ImportService + def create_organisations(folder) + import_from(folder, :create_organisation) + end + + private + + PROVIDER_TYPE = { + "HOUSING-ASSOCIATION" => Organisation.org_types[:PRP], + }.freeze + + def create_organisation(xml_document) + Organisation.create!( + name: organisation_field_value(xml_document, "name"), + providertype: map_provider_type(organisation_field_value(xml_document, "institution-type")), + phone: organisation_field_value(xml_document, "telephone-number"), + holds_own_stock: to_boolean(organisation_field_value(xml_document, "holds-stock")), + active: to_boolean(organisation_field_value(xml_document, "active")), + old_association_type: organisation_field_value(xml_document, "old-association-type"), + software_supplier_id: organisation_field_value(xml_document, "software-supplier-id"), + housing_management_system: organisation_field_value(xml_document, "housing-management-system"), + choice_based_lettings: to_boolean(organisation_field_value(xml_document, "choice-based-lettings")), + common_housing_register: to_boolean(organisation_field_value(xml_document, "common-housing-register")), + choice_allocation_policy: to_boolean(organisation_field_value(xml_document, "choice-allocation-policy")), + cbl_proportion_percentage: organisation_field_value(xml_document, "cbl-proportion-percentage"), + enter_affordable_logs: to_boolean(organisation_field_value(xml_document, "enter-affordable-logs")), + owns_affordable_logs: to_boolean(organisation_field_value(xml_document, "owns-affordable-rent")), + housing_registration_no: organisation_field_value(xml_document, "housing-registration-no"), + general_needs_units: organisation_field_value(xml_document, "general-needs-units"), + supported_housing_units: organisation_field_value(xml_document, "supported-housing-units"), + unspecified_units: organisation_field_value(xml_document, "unspecified-units"), + old_org_id: organisation_field_value(xml_document, "id"), + old_visible_id: organisation_field_value(xml_document, "visible-id"), + ) + rescue ActiveRecord::RecordNotUnique + name = organisation_field_value(xml_document, "name") + old_visible_id = organisation_field_value(xml_document, "visible-id") + @logger.warn("Organisation #{name} is already present with old visible ID #{old_visible_id}, skipping.") + end + + def map_provider_type(institution_type) + if PROVIDER_TYPE.key?(institution_type) + PROVIDER_TYPE[institution_type] + else + institution_type + end + end + + def organisation_field_value(xml_document, field) + field_value(xml_document, "institution", field) + end + end +end diff --git a/app/services/imports/user_import_service.rb b/app/services/imports/user_import_service.rb new file mode 100644 index 000000000..bfeaad515 --- /dev/null +++ b/app/services/imports/user_import_service.rb @@ -0,0 +1,21 @@ +module Imports + class UserImportService < ImportService + def create_users(folder) + import_from(folder, :create_user) + end + + private + + def create_user(xml_document) + Organisation.create!( + old_user_id: user_field_value(xml_document, "id"), + ) + rescue ActiveRecord::RecordNotUnique + @logger.warn("Organisation #{name} is already present with old visible ID #{old_visible_id}, skipping.") + end + + def user_field_value(xml_document, field) + field_value(xml_document, "user", field) + end + end +end diff --git a/db/migrate/20220204134000_additional_user_fields.rb b/db/migrate/20220204134000_additional_user_fields.rb new file mode 100644 index 000000000..71737f49e --- /dev/null +++ b/db/migrate/20220204134000_additional_user_fields.rb @@ -0,0 +1,13 @@ +class AdditionalUserFields < ActiveRecord::Migration[7.0] + def up + change_table :users, bulk: true do |t| + t.column :old_user_id, :string + end + end + + def down + change_table :users, bulk: true do |t| + t.remove :old_user_id + end + end +end diff --git a/db/schema.rb b/db/schema.rb index b3f0fca4d..94c8453f3 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_02_03_104800) do +ActiveRecord::Schema.define(version: 2022_02_04_134000) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -247,6 +247,7 @@ ActiveRecord::Schema.define(version: 2022_02_03_104800) do t.string "current_sign_in_ip" t.string "last_sign_in_ip" t.integer "role" + t.string "old_user_id" t.index ["email"], name: "index_users_on_email", unique: true t.index ["organisation_id"], name: "index_users_on_organisation_id" t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true diff --git a/lib/tasks/data_import.rake b/lib/tasks/data_import.rake index 0a49d21c6..963782b77 100644 --- a/lib/tasks/data_import.rake +++ b/lib/tasks/data_import.rake @@ -8,11 +8,12 @@ namespace :core do 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) + Imports::OrganisationImportService.new(storage_service).create_organisations(path) + when "user" + Imports::UserImportService.new(storage_service).create_users(path) else raise "Type #{type} is not supported by data_import" end diff --git a/spec/lib/tasks/data_import_spec.rb b/spec/lib/tasks/data_import_spec.rb index aa7aaca48..22dd7089c 100644 --- a/spec/lib/tasks/data_import_spec.rb +++ b/spec/lib/tasks/data_import_spec.rb @@ -10,7 +10,7 @@ describe "rake core:data_import", type: :task do let(:storage_service) { instance_double(StorageService) } let(:paas_config_service) { instance_double(PaasConfigurationService) } - let(:import_service) { instance_double(ImportService) } + let(:import_service) { instance_double(Imports::OrganisationImportService) } before do Rake.application.rake_require("tasks/data_import") @@ -19,7 +19,7 @@ describe "rake core:data_import", type: :task 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(Imports::OrganisationImportService).to receive(:new).and_return(import_service) allow(ENV).to receive(:[]) allow(ENV).to receive(:[]).with("IMPORT_PAAS_INSTANCE").and_return(instance_name) end @@ -27,8 +27,8 @@ describe "rake core:data_import", type: :task do 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) + expect(Imports::OrganisationImportService).to receive(:new).with(storage_service) + expect(import_service).to receive(:create_organisations).with(fixture_path) task.invoke(organisation_type, fixture_path) end diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index 65f9c84ba..71ebe35d5 100644 --- a/spec/models/case_log_spec.rb +++ b/spec/models/case_log_spec.rb @@ -962,7 +962,7 @@ RSpec.describe CaseLog do describe "derived variables" do require "date" - let(:organisation) { FactoryBot.create(:organisation, "Org type": "PRP") } + let(:organisation) { FactoryBot.create(:organisation, org_type: "PRP") } let!(:case_log) do described_class.create({ managing_organisation: organisation, diff --git a/spec/models/organisation_spec.rb b/spec/models/organisation_spec.rb index c944aabcb..bf0cda2ef 100644 --- a/spec/models/organisation_spec.rb +++ b/spec/models/organisation_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Organisation, type: :model do let(:organisation) { user.organisation } it "has expected fields" do - expect(organisation.attribute_names).to include("name", "phone", "Org type") + expect(organisation.attribute_names).to include("name", "phone", "org_type") end it "has users" do diff --git a/spec/services/import_service_spec.rb b/spec/services/imports/organisation_import_service_spec.rb similarity index 55% rename from spec/services/import_service_spec.rb rename to spec/services/imports/organisation_import_service_spec.rb index a72f77326..94dc86d66 100644 --- a/spec/services/import_service_spec.rb +++ b/spec/services/imports/organisation_import_service_spec.rb @@ -1,17 +1,17 @@ require "rails_helper" -RSpec.describe ImportService do +RSpec.describe Imports::OrganisationImportService 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" } - def create_organisation_file(fixture_directory, visible_id, name = "my_organisation") + def create_organisation_file(fixture_directory, visible_id, name = nil) 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 + doc.at_xpath("//institution:visible-id").content = visible_id if visible_id + doc.at_xpath("//institution:name").content = name if name StringIO.new(doc.to_xml) end @@ -29,12 +29,41 @@ RSpec.describe ImportService do .and_return(create_organisation_file(fixture_directory, 2)) end - it "successfully create a new organisation if it does not exist" do + it "successfully create an organisation with the expected data" do + import_service.create_organisations(folder_name) + + organisation = Organisation.find_by(old_visible_id: 1) + expect(organisation.name).to eq("HA Ltd") + expect(organisation.providertype).to eq(2) + expect(organisation.phone).to eq("xxxxxxxx") + expect(organisation.holds_own_stock).to be_truthy + expect(organisation.active).to be_truthy + # expect(organisation.old_association_type).to eq() + # string VS integer + # expect(organisation.software_supplier_id).to eq() + # boolean VS string + expect(organisation.housing_management_system).to eq("") # Need examples + expect(organisation.choice_based_lettings).to be_falsey + expect(organisation.common_housing_register).to be_falsey + expect(organisation.choice_allocation_policy).to be_falsey + expect(organisation.cbl_proportion_percentage).to be_nil # Need example + expect(organisation.enter_affordable_logs).to be_truthy + expect(organisation.owns_affordable_logs).to be_truthy # owns_affordable_rent (difference rents and logs) + expect(organisation.housing_registration_no).to eq("LH9999") + expect(organisation.general_needs_units).to eq(1104) + expect(organisation.supported_housing_units).to eq(217) + expect(organisation.unspecified_units).to eq(0) + expect(organisation.unspecified_units).to eq(0) + expect(organisation.old_org_id).to eq("7c5bd5fb549c09z2c55d9cb90d7ba84927e64618") + expect(organisation.old_visible_id).to eq(1) + end + + it "successfully create multiple organisations" do expect(storage_service).to receive(:list_files).with(folder_name) expect(storage_service).to receive(:get_file_io).with(filenames[0]).ordered expect(storage_service).to receive(:get_file_io).with(filenames[1]).ordered - expect { import_service.update_organisations(folder_name) }.to change(Organisation, :count).by(2) + expect { import_service.create_organisations(folder_name) }.to change(Organisation, :count).by(2) expect(Organisation).to exist(old_visible_id: 1) expect(Organisation).to exist(old_visible_id: 2) end @@ -56,10 +85,10 @@ RSpec.describe ImportService do 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) + expect { import_service.create_organisations(folder_name) }.to change(Organisation, :count).by(1) + expect { import_service.create_organisations(folder_name) }.to change(Organisation, :count).by(0) - expect(Organisation).to exist(old_visible_id: 1, name: "my_organisation") + expect(Organisation).to exist(old_visible_id: 1, name: "HA Ltd") end end end