diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 627ea0eba..c8922029a 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[:provider_type]}" if renttype.present? && needstype.present? && owning_organisation[:provider_type].present? self.totchild = get_totchild self.totelder = get_totelder self.totadult = get_totadult diff --git a/app/models/constants/organisation.rb b/app/models/constants/organisation.rb index 2a6501383..017442d27 100644 --- a/app/models/constants/organisation.rb +++ b/app/models/constants/organisation.rb @@ -1,6 +1,6 @@ module Constants::Organisation - ORG_TYPE = { - "LA" => 1, - "PRP" => 2, + PROVIDER_TYPE = { + LA: 1, + PRP: 2, }.freeze end diff --git a/app/models/constants/user.rb b/app/models/constants/user.rb index d0a24cbeb..5b2292385 100644 --- a/app/models/constants/user.rb +++ b/app/models/constants/user.rb @@ -1,7 +1,7 @@ module Constants::User ROLES = { - "data_accessor" => 0, - "data_provider" => 1, - "data_coordinator" => 2, + data_accessor: 0, + data_provider: 1, + data_coordinator: 2, }.freeze end diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 5b082f292..6d31f8a94 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 + enum provider_type: PROVIDER_TYPE def case_logs CaseLog.for_organisation(self) diff --git a/app/services/imports/organisation_import_service.rb b/app/services/imports/organisation_import_service.rb index 358d51c4d..77f00ed0d 100644 --- a/app/services/imports/organisation_import_service.rb +++ b/app/services/imports/organisation_import_service.rb @@ -7,13 +7,13 @@ module Imports private PROVIDER_TYPE = { - "HOUSING-ASSOCIATION" => Organisation.org_types[:PRP], + "HOUSING-ASSOCIATION" => Organisation.provider_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")), + provider_type: 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")), @@ -39,14 +39,6 @@ module Imports @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 diff --git a/app/services/imports/user_import_service.rb b/app/services/imports/user_import_service.rb index bfeaad515..b4a39db6f 100644 --- a/app/services/imports/user_import_service.rb +++ b/app/services/imports/user_import_service.rb @@ -6,12 +6,23 @@ module Imports private + PROVIDER_TYPE = { + "Data Provider" => User.roles[:data_provider], + }.freeze + def create_user(xml_document) - Organisation.create!( + organisation = Organisation.find_by(old_org_id: user_field_value(xml_document, "institution")) + User.create!( + email: user_field_value(xml_document, "user-name"), + name: user_field_value(xml_document, "full-name"), + password: Devise.friendly_token, + phone: user_field_value(xml_document, "telephone-no"), old_user_id: user_field_value(xml_document, "id"), + organisation: organisation, + role: PROVIDER_TYPE[user_field_value(xml_document, "user-type")], ) rescue ActiveRecord::RecordNotUnique - @logger.warn("Organisation #{name} is already present with old visible ID #{old_visible_id}, skipping.") + @logger.warn("User #{name} with old user id #{old_user_id} is already present, skipping.") end def user_field_value(xml_document, field) diff --git a/app/views/users/new.html.erb b/app/views/users/new.html.erb index 3ce6c9e06..53e64eafa 100644 --- a/app/views/users/new.html.erb +++ b/app/views/users/new.html.erb @@ -27,7 +27,7 @@ value: @resource.email %> - <%= roles = User::ROLES.map { |key, value| OpenStruct.new(id:key, name: key.humanize) } + <%= roles = User::ROLES.map { |key, _| OpenStruct.new(id: key, name: key.to_s.humanize) } f.govuk_collection_radio_buttons :role, roles, :id, :name, legend: { text: "Role", size: "m" } %> diff --git a/db/migrate/202202071123100_additional_user_fields2.rb b/db/migrate/202202071123100_additional_user_fields2.rb new file mode 100644 index 000000000..95d2da99b --- /dev/null +++ b/db/migrate/202202071123100_additional_user_fields2.rb @@ -0,0 +1,13 @@ +class AdditionalUserFields2 < ActiveRecord::Migration[7.0] + def up + change_table :users, bulk: true do |t| + t.column :phone, :string + end + end + + def down + change_table :users, bulk: true do |t| + t.remove :phone + end + end +end diff --git a/db/migrate/20220207120200_rename_provider_type.rb b/db/migrate/20220207120200_rename_provider_type.rb new file mode 100644 index 000000000..5f486a35b --- /dev/null +++ b/db/migrate/20220207120200_rename_provider_type.rb @@ -0,0 +1,9 @@ +class RenameProviderType < ActiveRecord::Migration[7.0] + def up + rename_column :organisations, :providertype, :provider_type + end + + def down + rename_column :organisations, :provider_type, :providertype + end +end diff --git a/db/schema.rb b/db/schema.rb index 94c8453f3..d493975a8 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_04_134000) do +ActiveRecord::Schema.define(version: 202202071123100) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -202,7 +202,7 @@ ActiveRecord::Schema.define(version: 2022_02_04_134000) do create_table "organisations", force: :cascade do |t| t.string "name" t.string "phone" - t.integer "providertype" + t.integer "provider_type" t.string "address_line1" t.string "address_line2" t.string "postcode" @@ -248,6 +248,7 @@ ActiveRecord::Schema.define(version: 2022_02_04_134000) do t.string "last_sign_in_ip" t.integer "role" t.string "old_user_id" + t.string "phone" 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/spec/controllers/admin/organisations_controller_spec.rb b/spec/controllers/admin/organisations_controller_spec.rb index f13be1259..16dedcb69 100644 --- a/spec/controllers/admin/organisations_controller_spec.rb +++ b/spec/controllers/admin/organisations_controller_spec.rb @@ -37,7 +37,7 @@ describe Admin::OrganisationsController, type: :controller do it "creates a new admin users" do expect(page).to have_field("organisation_name") - expect(page).to have_field("organisation_providertype") + expect(page).to have_field("organisation_provider_type") expect(page).to have_field("organisation_phone") end end diff --git a/spec/fixtures/softwire_imports/users/fc7625a02b24ae16162aa63ae7cb33feeec0c373.xml b/spec/fixtures/softwire_imports/users/fc7625a02b24ae16162aa63ae7cb33feeec0c373.xml new file mode 100644 index 000000000..535756aca --- /dev/null +++ b/spec/fixtures/softwire_imports/users/fc7625a02b24ae16162aa63ae7cb33feeec0c373.xml @@ -0,0 +1,13 @@ + + fc7625a02b24ae16162aa63ae7cb33feeec0c373 + xxx + John Doe + john.doe@gov.uk + 7c5bd5fb549c09a2c55d7cb90d7ba84927e64618 + john.doe@gov.uk + Data Provider + true + false + None + 02012345678 + diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index 71ebe35d5..82744db29 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, provider_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 bf0cda2ef..a1b4f011a 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", "provider_type") end it "has users" do diff --git a/spec/services/imports/organisation_import_service_spec.rb b/spec/services/imports/organisation_import_service_spec.rb index 94dc86d66..c2f23e199 100644 --- a/spec/services/imports/organisation_import_service_spec.rb +++ b/spec/services/imports/organisation_import_service_spec.rb @@ -34,21 +34,19 @@ RSpec.describe Imports::OrganisationImportService do organisation = Organisation.find_by(old_visible_id: 1) expect(organisation.name).to eq("HA Ltd") - expect(organisation.providertype).to eq(2) + expect(organisation.provider_type).to eq("PRP") 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.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.owns_affordable_logs).to be_truthy # owns_affordable_rent expect(organisation.housing_registration_no).to eq("LH9999") expect(organisation.general_needs_units).to eq(1104) expect(organisation.supported_housing_units).to eq(217) diff --git a/spec/services/imports/user_import_service_spec.rb b/spec/services/imports/user_import_service_spec.rb new file mode 100644 index 000000000..0e0c4d8b0 --- /dev/null +++ b/spec/services/imports/user_import_service_spec.rb @@ -0,0 +1,37 @@ +require "rails_helper" + +RSpec.describe Imports::UserImportService do + let(:fixture_directory) { "spec/fixtures/softwire_imports/users" } + let(:user_file) { File.open("#{fixture_directory}/fc7625a02b24ae16162aa63ae7cb33feeec0c373.xml") } + let(:storage_service) { instance_double(StorageService) } + + context "when importing users" do + subject(:import_service) { described_class.new(storage_service) } + + before do + allow(storage_service).to receive(:list_files) + .and_return(["user_directory/fc7625a02b24ae16162aa63ae7cb33feeec0c373.xml"]) + allow(storage_service).to receive(:get_file_io) + .with("user_directory/fc7625a02b24ae16162aa63ae7cb33feeec0c373.xml") + .and_return(user_file) + end + + it "successfully create a user with the expected data" do + FactoryBot.create(:organisation, old_org_id: "7c5bd5fb549c09a2c55d7cb90d7ba84927e64618") + import_service.create_users("user_directory") + + user = User.find_by(old_user_id: "fc7625a02b24ae16162aa63ae7cb33feeec0c373") + expect(user.name).to eq("John Doe") + expect(user.email).to eq("john.doe@gov.uk") + expect(user.encrypted_password).not_to be_nil + expect(user.phone).to eq("02012345678") + expect(user).to be_data_provider + expect(user.organisation.old_org_id).to eq("7c5bd5fb549c09a2c55d7cb90d7ba84927e64618") + end + + it "refuses to create a user belonging to a non existing organisation" do + expect { import_service.create_users("user_directory") } + .to raise_error(ActiveRecord::RecordInvalid, /Organisation must exist/) + end + end +end