Complete user import and corrects enums

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

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

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

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)

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

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)

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" }
       %>
 

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

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

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

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

spec/fixtures/softwire_imports/users/fc7625a02b24ae16162aa63ae7cb33feeec0c373.xml

@@ -0,0 +1,13 @@
+<user:user xmlns:user="dclg:user">
+  <user:id>fc7625a02b24ae16162aa63ae7cb33feeec0c373</user:id>
+  <user:password>xxx</user:password>
+  <user:full-name>John Doe</user:full-name>
+  <user:user-name>john.doe@gov.uk</user:user-name>
+  <user:institution>7c5bd5fb549c09a2c55d7cb90d7ba84927e64618</user:institution>
+  <user:email>john.doe@gov.uk</user:email>
+  <user:user-type>Data Provider</user:user-type>
+  <user:active>true</user:active>
+  <user:deleted>false</user:deleted>
+  <user:contact-priority-id>None</user:contact-priority-id>
+  <user:telephone-no>02012345678</user:telephone-no>
+</user:user>

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,

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

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)

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