diff --git a/app/models/log.rb b/app/models/log.rb index 5354b3ba5..c1778b299 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -45,6 +45,7 @@ class Log < ApplicationRecord } scope :created_by, ->(user) { where(created_by: user) } scope :imported, -> { where.not(old_id: nil) } + scope :not_imported, -> { where(old_id: nil) } attr_accessor :skip_update_status, :skip_update_uprn_confirmed diff --git a/app/services/imports/lettings_logs_import_service.rb b/app/services/imports/lettings_logs_import_service.rb index 4d837ad77..0083bda35 100644 --- a/app/services/imports/lettings_logs_import_service.rb +++ b/app/services/imports/lettings_logs_import_service.rb @@ -298,7 +298,7 @@ module Imports end record else - duplicate_logs = lettings_log.owning_organisation.owned_lettings_logs.duplicate_logs(lettings_log) + duplicate_logs = lettings_log.owning_organisation.owned_lettings_logs.not_imported.duplicate_logs(lettings_log) if duplicate_logs.count.positive? @logger.info("Duplicate log with id #{duplicate_logs.map(&:id).join(', ')} found for log #{lettings_log.old_id}, skipping log") else diff --git a/app/services/imports/sales_logs_import_service.rb b/app/services/imports/sales_logs_import_service.rb index f2d9d8966..d55ace377 100644 --- a/app/services/imports/sales_logs_import_service.rb +++ b/app/services/imports/sales_logs_import_service.rb @@ -232,7 +232,7 @@ module Imports end record else - duplicate_logs = sales_log.owning_organisation.owned_sales_logs.duplicate_logs(sales_log) + duplicate_logs = sales_log.owning_organisation.owned_sales_logs.not_imported.duplicate_logs(sales_log) if duplicate_logs.count.positive? @logger.info("Duplicate log with id #{duplicate_logs.map(&:id).join(', ')} found for log #{sales_log.old_id}, skipping log") else diff --git a/spec/services/imports/lettings_logs_import_service_spec.rb b/spec/services/imports/lettings_logs_import_service_spec.rb index 45bfd3331..704d39d14 100644 --- a/spec/services/imports/lettings_logs_import_service_spec.rb +++ b/spec/services/imports/lettings_logs_import_service_spec.rb @@ -119,6 +119,19 @@ RSpec.describe Imports::LettingsLogsImportService do expect(LettingsLog.count).to eq(3) end + it "imports the log if a duplicate imported log exists on the system (with different legacy ID)" do + expect(logger).not_to receive(:error) + expect(logger).not_to receive(:warn) + expect(logger).not_to receive(:info).with(/Updating lettings log/) + + lettings_log_service.create_logs(remote_folder) + expect(LettingsLog.count).to eq(3) + LettingsLog.all.each { |log| log.update(old_id: "old_id_#{rand(1..999_999)}") } + + lettings_log_service.create_logs(remote_folder) + expect(LettingsLog.count).to eq(6) + end + context "with updates allowed" do subject(:lettings_log_service) { described_class.new(storage_service, logger, allow_updates: true) } @@ -167,7 +180,6 @@ RSpec.describe Imports::LettingsLogsImportService do expect(logger).to receive(:error).with("Lettings log 'fake_id' belongs to legacy user with owner-user-id: 'fake_id' which cannot be found. 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_xml.at_xpath("//xmlns:_2bTenCode").content = "fake_code" lettings_log_service.send(:create_log, lettings_log_xml) lettings_log = LettingsLog.where(old_id: lettings_log_id).first @@ -205,7 +217,6 @@ RSpec.describe Imports::LettingsLogsImportService do expect(logger).to receive(:error).with("Lettings log 'fake_id' does not have the owner id. 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_xml.at_xpath("//xmlns:_2bTenCode").content = "fake_code" lettings_log_service.send(:create_log, lettings_log_xml) lettings_log = LettingsLog.where(old_id: lettings_log_id).first @@ -233,7 +244,6 @@ RSpec.describe Imports::LettingsLogsImportService do 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_xml.at_xpath("//xmlns:_2bTenCode").content = "fake_code" lettings_log_service.send(:create_log, lettings_log_xml) lettings_log = LettingsLog.where(old_id: lettings_log_id).first diff --git a/spec/services/imports/sales_logs_import_service_spec.rb b/spec/services/imports/sales_logs_import_service_spec.rb index a3ceffe80..6ce399a78 100644 --- a/spec/services/imports/sales_logs_import_service_spec.rb +++ b/spec/services/imports/sales_logs_import_service_spec.rb @@ -98,6 +98,19 @@ RSpec.describe Imports::SalesLogsImportService do expect(SalesLog.count).to eq(4) end + it "imports the log if a duplicate imported log exists on the system (with different legacy ID)" do + expect(logger).not_to receive(:error) + expect(logger).not_to receive(:warn) + expect(logger).not_to receive(:info).with(/Updating sales log/) + + sales_log_service.create_logs(remote_folder) + expect(SalesLog.count).to eq(4) + SalesLog.all.each { |log| log.update(old_id: "old_id_#{rand(1..999_999)}") } + + sales_log_service.create_logs(remote_folder) + expect(SalesLog.count).to eq(8) + end + context "with updates allowed" do subject(:sales_log_service) { described_class.new(storage_service, logger, allow_updates: true) } @@ -159,7 +172,6 @@ RSpec.describe Imports::SalesLogsImportService do expect(logger).to receive(:error).with("Sales log 'fake_id' belongs to legacy user with owner-user-id: 'fake_id' which cannot be found. 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_xml.at_xpath("//xmlns:PurchaserCode").content = "fake_code" sales_log_service.send(:create_log, sales_log_xml) sales_log = SalesLog.where(old_id: sales_log_id).first @@ -199,7 +211,6 @@ RSpec.describe Imports::SalesLogsImportService do expect(logger).to receive(:error).with("Sales log 'fake_id' does not have the owner id. 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_xml.at_xpath("//xmlns:PurchaserCode").content = "fake_code" sales_log_service.send(:create_log, sales_log_xml) sales_log = SalesLog.where(old_id: sales_log_id).first @@ -229,7 +240,6 @@ RSpec.describe Imports::SalesLogsImportService do 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_xml.at_xpath("//xmlns:PurchaserCode").content = "fake_code" sales_log_service.send(:create_log, sales_log_xml) sales_log = SalesLog.where(old_id: sales_log_id).first