diff --git a/app/services/imports/lettings_logs_import_service.rb b/app/services/imports/lettings_logs_import_service.rb index 1aecc4f81..ab15e7f3d 100644 --- a/app/services/imports/lettings_logs_import_service.rb +++ b/app/services/imports/lettings_logs_import_service.rb @@ -205,8 +205,8 @@ module Imports # Supported Housing fields if attributes["needstype"] == GN_SH[:supported_housing] - location_old_visible_id = safe_string_as_integer(xml_doc, "_1cschemecode") - scheme_old_visible_id = safe_string_as_integer(xml_doc, "_1cmangroupcode") + location_old_visible_id = string_or_nil(xml_doc, "_1cschemecode") + scheme_old_visible_id = string_or_nil(xml_doc, "_1cmangroupcode") schemes = Scheme.where(old_visible_id: scheme_old_visible_id, owning_organisation_id: attributes["owning_organisation_id"]) location = Location.find_by(old_visible_id: location_old_visible_id, scheme: schemes) @@ -399,7 +399,7 @@ module Imports end def find_organisation_id(xml_doc, id_field) - old_visible_id = unsafe_string_as_integer(xml_doc, id_field) + old_visible_id = string_or_nil(xml_doc, id_field) organisation = Organisation.find_by(old_visible_id:) raise "Organisation not found with legacy ID #{old_visible_id}" if organisation.nil? diff --git a/app/services/imports/scheme_location_import_service.rb b/app/services/imports/scheme_location_import_service.rb index 4214555cc..837be0b49 100644 --- a/app/services/imports/scheme_location_import_service.rb +++ b/app/services/imports/scheme_location_import_service.rb @@ -91,7 +91,7 @@ module Imports attributes["units"] = safe_string_as_integer(xml_doc, "total-units") attributes["type_of_unit"] = safe_string_as_integer(xml_doc, "unit-type") attributes["location_old_id"] = string_or_nil(xml_doc, "id") - attributes["location_old_visible_id"] = safe_string_as_integer(xml_doc, "visible-id") + attributes["location_old_visible_id"] = string_or_nil(xml_doc, "visible-id") attributes["scheme_old_id"] = string_or_nil(xml_doc, "mgmtgroup") attributes end diff --git a/db/migrate/20221111102656_change_old_visible_id_type.rb b/db/migrate/20221111102656_change_old_visible_id_type.rb new file mode 100644 index 000000000..d4c568bb7 --- /dev/null +++ b/db/migrate/20221111102656_change_old_visible_id_type.rb @@ -0,0 +1,13 @@ +class ChangeOldVisibleIdType < ActiveRecord::Migration[7.0] + def up + change_column :organisations, :old_visible_id, :string + change_column :schemes, :old_visible_id, :string + change_column :locations, :old_visible_id, :string + end + + def down + change_column :organisations, :old_visible_id, :integer + change_column :schemes, :old_visible_id, :integer + change_column :locations, :old_visible_id, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index ea4b59670..5faff28ba 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[7.0].define(version: 2022_11_09_122033) do +ActiveRecord::Schema[7.0].define(version: 2022_11_11_102656) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -255,7 +255,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_11_09_122033) do t.integer "units" t.integer "type_of_unit" t.string "old_id" - t.integer "old_visible_id" + t.string "old_visible_id" t.string "mobility_type" t.datetime "startdate", precision: nil t.string "location_admin_district" @@ -316,7 +316,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_11_09_122033) do t.integer "supported_housing_units" t.integer "unspecified_units" t.string "old_org_id" - t.integer "old_visible_id" + t.string "old_visible_id" t.index ["old_visible_id"], name: "index_organisations_on_old_visible_id", unique: true end @@ -395,7 +395,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_11_09_122033) do t.bigint "managing_organisation_id" t.string "arrangement_type" t.string "old_id" - t.integer "old_visible_id" + t.string "old_visible_id" t.integer "total_units" t.boolean "confirmed" t.index ["managing_organisation_id"], name: "index_schemes_on_managing_organisation_id" diff --git a/spec/factories/location.rb b/spec/factories/location.rb index a8418f8ba..3359f64cd 100644 --- a/spec/factories/location.rb +++ b/spec/factories/location.rb @@ -17,7 +17,7 @@ FactoryBot.define do units { 20 } mobility_type { "A" } scheme { FactoryBot.create(:scheme, :export) } - old_visible_id { 111 } + old_visible_id { "111" } end end end diff --git a/spec/fixtures/imports/logs/0b4a68df-30cc-474a-93c0-a56ce8fdad3b.xml b/spec/fixtures/imports/logs/0b4a68df-30cc-474a-93c0-a56ce8fdad3b.xml index 59c1eada7..3faacf28b 100644 --- a/spec/fixtures/imports/logs/0b4a68df-30cc-474a-93c0-a56ce8fdad3b.xml +++ b/spec/fixtures/imports/logs/0b4a68df-30cc-474a-93c0-a56ce8fdad3b.xml @@ -19,7 +19,7 @@
123456
2 Local Authority - <_1cmangroupcode>123 + <_1cmangroupcode>0123 <_1cschemecode>10 3 No diff --git a/spec/fixtures/imports/mgmtgroups/6d6d7618b58affe2a150a5ef2e9f4765fa6cd05d.xml b/spec/fixtures/imports/mgmtgroups/6d6d7618b58affe2a150a5ef2e9f4765fa6cd05d.xml index 5a7b23b5b..712d3f6e5 100644 --- a/spec/fixtures/imports/mgmtgroups/6d6d7618b58affe2a150a5ef2e9f4765fa6cd05d.xml +++ b/spec/fixtures/imports/mgmtgroups/6d6d7618b58affe2a150a5ef2e9f4765fa6cd05d.xml @@ -5,5 +5,5 @@ 456 7c5bd5fb549c09z2c55d9cb90d7ba84927e64618 Approved - 123 + 0123 diff --git a/spec/services/imports/lettings_logs_import_service_spec.rb b/spec/services/imports/lettings_logs_import_service_spec.rb index 576249971..453ee7e30 100644 --- a/spec/services/imports/lettings_logs_import_service_spec.rb +++ b/spec/services/imports/lettings_logs_import_service_spec.rb @@ -11,8 +11,8 @@ RSpec.describe Imports::LettingsLogsImportService do let(:fixture_directory) { "spec/fixtures/imports/logs" } let(:organisation) { FactoryBot.create(:organisation, old_visible_id: "1", provider_type: "PRP") } - let(:scheme1) { FactoryBot.create(:scheme, old_visible_id: 123, owning_organisation: organisation) } - let(:scheme2) { FactoryBot.create(:scheme, old_visible_id: 456, owning_organisation: organisation) } + let(:scheme1) { FactoryBot.create(:scheme, old_visible_id: "0123", owning_organisation: organisation) } + let(:scheme2) { FactoryBot.create(:scheme, old_visible_id: "456", owning_organisation: organisation) } def open_file(directory, filename) File.open("#{directory}/#{filename}.xml") @@ -23,16 +23,16 @@ RSpec.describe Imports::LettingsLogsImportService do .to_return(status: 200, body: '{"status":200,"result":{"admin_district":"Westminster","codes":{"admin_district":"E08000035"}}}', headers: {}) allow(Organisation).to receive(:find_by).and_return(nil) - allow(Organisation).to receive(:find_by).with(old_visible_id: organisation.old_visible_id.to_i).and_return(organisation) + allow(Organisation).to receive(:find_by).with(old_visible_id: organisation.old_visible_id).and_return(organisation) # Created by users FactoryBot.create(:user, old_user_id: "c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa", organisation:) FactoryBot.create(:user, old_user_id: "e29c492473446dca4d50224f2bb7cf965a261d6f", organisation:) # Location setup - FactoryBot.create(:location, old_visible_id: 10, postcode: "LS166FT", scheme_id: scheme1.id, mobility_type: "W") + FactoryBot.create(:location, old_visible_id: "10", postcode: "LS166FT", scheme_id: scheme1.id, mobility_type: "W") FactoryBot.create(:location, scheme_id: scheme1.id) - FactoryBot.create(:location, old_visible_id: 10, postcode: "LS166FT", scheme_id: scheme2.id, mobility_type: "W") + FactoryBot.create(:location, old_visible_id: "10", postcode: "LS166FT", scheme_id: scheme2.id, mobility_type: "W") # Stub the form handler to use the real form allow(FormHandler.instance).to receive(:get_form).with("previous_lettings").and_return(real_2021_2022_form) diff --git a/spec/services/imports/organisation_import_service_spec.rb b/spec/services/imports/organisation_import_service_spec.rb index 48a0d72bb..c8fcd4829 100644 --- a/spec/services/imports/organisation_import_service_spec.rb +++ b/spec/services/imports/organisation_import_service_spec.rb @@ -32,7 +32,7 @@ RSpec.describe Imports::OrganisationImportService 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) + organisation = Organisation.find_by(old_visible_id: "1") expect(organisation.name).to eq("HA Ltd") expect(organisation.provider_type).to eq("PRP") expect(organisation.phone).to eq("xxxxxxxx") @@ -53,7 +53,7 @@ RSpec.describe Imports::OrganisationImportService do 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) + expect(organisation.old_visible_id).to eq("1") end it "successfully create multiple organisations" do @@ -62,8 +62,8 @@ RSpec.describe Imports::OrganisationImportService do expect(storage_service).to receive(:get_file_io).with(filenames[1]).ordered 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) + expect(Organisation).to exist(old_visible_id: "1") + expect(Organisation).to exist(old_visible_id: "2") end end @@ -86,7 +86,7 @@ RSpec.describe Imports::OrganisationImportService do 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: "HA Ltd") + expect(Organisation).to exist(old_visible_id: "1", name: "HA Ltd") end end end diff --git a/spec/services/imports/scheme_import_service_spec.rb b/spec/services/imports/scheme_import_service_spec.rb index efba81a1c..97e9359b5 100644 --- a/spec/services/imports/scheme_import_service_spec.rb +++ b/spec/services/imports/scheme_import_service_spec.rb @@ -10,7 +10,7 @@ RSpec.describe Imports::SchemeImportService do let(:scheme_id) { "6d6d7618b58affe2a150a5ef2e9f4765fa6cd05d" } let!(:owning_org) { FactoryBot.create(:organisation, old_org_id: "7c5bd5fb549c09z2c55d9cb90d7ba84927e64618") } - let!(:managing_org) { FactoryBot.create(:organisation, old_visible_id: 456) } + let!(:managing_org) { FactoryBot.create(:organisation, old_visible_id: "456") } def open_file(directory, filename) File.open("#{directory}/#{filename}.xml") @@ -46,7 +46,7 @@ RSpec.describe Imports::SchemeImportService do expect(scheme.owning_organisation).to eq(owning_org) expect(scheme.managing_organisation).to eq(managing_org) expect(scheme.old_id).to eq("6d6d7618b58affe2a150a5ef2e9f4765fa6cd05d") - expect(scheme.old_visible_id).to eq(123) + expect(scheme.old_visible_id).to eq("0123") expect(scheme.service_name).to eq("Management Group") expect(scheme.arrangement_type).to eq("Another organisation") end diff --git a/spec/services/imports/scheme_location_import_service_spec.rb b/spec/services/imports/scheme_location_import_service_spec.rb index 8b93d4969..c11c2ca2d 100644 --- a/spec/services/imports/scheme_location_import_service_spec.rb +++ b/spec/services/imports/scheme_location_import_service_spec.rb @@ -142,7 +142,7 @@ RSpec.describe Imports::SchemeLocationImportService do expect(location.mobility_type).to eq("Fitted with equipment and adaptations") expect(location.type_of_unit).to eq("Bungalow") expect(location.old_id).to eq(first_location_id) - expect(location.old_visible_id).to eq(10) + expect(location.old_visible_id).to eq("10") expect(location.startdate).to eq("1900-01-01") expect(location.scheme).to eq(scheme) end