From 719321945b3e9e0eef7643a991a0ac079545374f Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Thu, 7 Sep 2023 11:48:41 +0100 Subject: [PATCH] Set unassigned user if legacy user belongs to a different organisation (#1894) * Set user to unassigned in lettings logs if the legacy user exists but belongs to a different organisation * Set user to unassigned in sales logs if the legacy user exists but belongs to a different organisation * Lint --- .../imports/lettings_logs_import_service.rb | 8 +++- .../imports/sales_logs_import_service.rb | 8 +++- .../lettings_logs_import_service_spec.rb | 40 ++++++++++++++++++ .../imports/sales_logs_import_service_spec.rb | 42 +++++++++++++++++++ 4 files changed, 94 insertions(+), 4 deletions(-) diff --git a/app/services/imports/lettings_logs_import_service.rb b/app/services/imports/lettings_logs_import_service.rb index 138126cbb..a53048d83 100644 --- a/app/services/imports/lettings_logs_import_service.rb +++ b/app/services/imports/lettings_logs_import_service.rb @@ -251,8 +251,12 @@ module Imports owner_id = meta_field_value(xml_doc, "owner-user-id").strip if owner_id.present? user = LegacyUser.find_by(old_user_id: owner_id)&.user - if user.blank? - @logger.error("Lettings log '#{attributes['old_id']}' belongs to legacy user with owner-user-id: '#{owner_id}' which cannot be found. Assigning log to 'Unassigned' user.") + if user.blank? || (user.organisation_id != attributes["managing_organisation_id"] && user.organisation_id != attributes["owning_organisation_id"]) + if user.blank? + @logger.error("Lettings log '#{attributes['old_id']}' belongs to legacy user with owner-user-id: '#{owner_id}' which cannot be found. Assigning log to 'Unassigned' user.") + else + @logger.error("Lettings log '#{attributes['old_id']}' belongs to legacy user with owner-user-id: '#{owner_id}' which belongs to a different organisation. Assigning log to 'Unassigned' user.") + end if User.find_by(name: "Unassigned", organisation_id: attributes["managing_organisation_id"]) user = User.find_by(name: "Unassigned", organisation_id: attributes["managing_organisation_id"]) else diff --git a/app/services/imports/sales_logs_import_service.rb b/app/services/imports/sales_logs_import_service.rb index ecee55c22..157e797b4 100644 --- a/app/services/imports/sales_logs_import_service.rb +++ b/app/services/imports/sales_logs_import_service.rb @@ -179,8 +179,12 @@ module Imports if owner_id.present? user = LegacyUser.find_by(old_user_id: owner_id)&.user - if user.blank? - @logger.error("Sales log '#{attributes['old_id']}' belongs to legacy user with owner-user-id: '#{owner_id}' which cannot be found. Assigning log to 'Unassigned' user.") + if user.blank? || user.organisation_id != attributes["owning_organisation_id"] + if user.blank? + @logger.error("Sales log '#{attributes['old_id']}' belongs to legacy user with owner-user-id: '#{owner_id}' which cannot be found. Assigning log to 'Unassigned' user.") + else + @logger.error("Sales log '#{attributes['old_id']}' belongs to legacy user with owner-user-id: '#{owner_id}' which belongs to a different organisation. Assigning log to 'Unassigned' user.") + end if User.find_by(name: "Unassigned", organisation_id: attributes["owning_organisation_id"]) user = User.find_by(name: "Unassigned", organisation_id: attributes["owning_organisation_id"]) else diff --git a/spec/services/imports/lettings_logs_import_service_spec.rb b/spec/services/imports/lettings_logs_import_service_spec.rb index 1025fcd3b..7dc3d578d 100644 --- a/spec/services/imports/lettings_logs_import_service_spec.rb +++ b/spec/services/imports/lettings_logs_import_service_spec.rb @@ -172,6 +172,46 @@ RSpec.describe Imports::LettingsLogsImportService do end end + context "and the user exists on a different organisation" do + before do + create(:legacy_user, old_user_id: "fake_id") + lettings_log_xml.at_xpath("//meta:owner-user-id").content = "fake_id" + end + + it "creates a new unassigned user" do + expect(logger).to receive(:error).with("Lettings log '0ead17cb-1668-442d-898c-0d52879ff592' belongs to legacy user with owner-user-id: 'fake_id' which belongs to a different organisation. Assigning log to 'Unassigned' user.") + lettings_log_service.send(:create_log, lettings_log_xml) + + lettings_log = LettingsLog.where(old_id: lettings_log_id).first + expect(lettings_log&.created_by&.name).to eq("Unassigned") + end + + it "only creates one unassigned user" do + expect(logger).to receive(:error).with("Lettings log '0ead17cb-1668-442d-898c-0d52879ff592' belongs to legacy user with owner-user-id: 'fake_id' which belongs to a different organisation. Assigning log to 'Unassigned' user.") + expect(logger).to receive(:error).with("Lettings log 'fake_id' belongs to legacy user with owner-user-id: 'fake_id' which belongs to a different organisation. Assigning log to 'Unassigned' user.") + lettings_log_service.send(:create_log, lettings_log_xml) + lettings_log_xml.at_xpath("//meta:document-id").content = "fake_id" + lettings_log_service.send(:create_log, lettings_log_xml) + + lettings_log = LettingsLog.where(old_id: lettings_log_id).first + second_lettings_log = LettingsLog.where(old_id: "fake_id").first + expect(lettings_log&.created_by).to eq(second_lettings_log&.created_by) + end + + context "when unassigned user exist for a different organisation" do + let!(:other_unassigned_user) { create(:user, name: "Unassigned") } + + it "creates a new unassigned user for current organisation" do + expect(logger).to receive(:error).with("Lettings log '0ead17cb-1668-442d-898c-0d52879ff592' belongs to legacy user with owner-user-id: 'fake_id' which belongs to a different organisation. Assigning log to 'Unassigned' user.") + lettings_log_service.send(:create_log, lettings_log_xml) + + lettings_log = LettingsLog.where(old_id: lettings_log_id).first + expect(lettings_log&.created_by&.name).to eq("Unassigned") + expect(lettings_log&.created_by).not_to eq(other_unassigned_user) + end + end + end + it "correctly sets imported at date" do lettings_log_service.send(:create_log, lettings_log_xml) diff --git a/spec/services/imports/sales_logs_import_service_spec.rb b/spec/services/imports/sales_logs_import_service_spec.rb index bbabc5c1a..df279e5d5 100644 --- a/spec/services/imports/sales_logs_import_service_spec.rb +++ b/spec/services/imports/sales_logs_import_service_spec.rb @@ -163,6 +163,48 @@ RSpec.describe Imports::SalesLogsImportService do end end + context "and the user exists on a different organisation" do + let(:sales_log_id) { "shared_ownership_sales_log" } + + before do + create(:legacy_user, old_user_id: "fake_id") + sales_log_xml.at_xpath("//meta:owner-user-id").content = "fake_id" + end + + it "creates a new unassigned user" do + expect(logger).to receive(:error).with("Sales log 'shared_ownership_sales_log' belongs to legacy user with owner-user-id: 'fake_id' which belongs to a different organisation. Assigning log to 'Unassigned' user.") + sales_log_service.send(:create_log, sales_log_xml) + + sales_log = SalesLog.where(old_id: sales_log_id).first + expect(sales_log&.created_by&.name).to eq("Unassigned") + end + + it "only creates one unassigned user" do + expect(logger).to receive(:error).with("Sales log 'shared_ownership_sales_log' belongs to legacy user with owner-user-id: 'fake_id' which belongs to a different organisation. Assigning log to 'Unassigned' user.") + expect(logger).to receive(:error).with("Sales log 'fake_id' belongs to legacy user with owner-user-id: 'fake_id' which belongs to a different organisation. Assigning log to 'Unassigned' user.") + sales_log_service.send(:create_log, sales_log_xml) + sales_log_xml.at_xpath("//meta:document-id").content = "fake_id" + sales_log_service.send(:create_log, sales_log_xml) + + sales_log = SalesLog.where(old_id: sales_log_id).first + second_sales_log = SalesLog.where(old_id: "fake_id").first + expect(sales_log&.created_by).to eq(second_sales_log&.created_by) + end + + context "when unassigned user exist for a different organisation" do + let!(:other_unassigned_user) { create(:user, name: "Unassigned") } + + it "creates a new unassigned user for current organisation" do + expect(logger).to receive(:error).with("Sales log 'shared_ownership_sales_log' belongs to legacy user with owner-user-id: 'fake_id' which belongs to a different organisation. Assigning log to 'Unassigned' user.") + sales_log_service.send(:create_log, sales_log_xml) + + sales_log = SalesLog.where(old_id: sales_log_id).first + expect(sales_log&.created_by&.name).to eq("Unassigned") + expect(sales_log&.created_by).not_to eq(other_unassigned_user) + end + end + end + context "and the log startdate is before 22/23 collection period" do let(:sales_log_id) { "shared_ownership_sales_log" }