diff --git a/app/services/imports/import_service.rb b/app/services/imports/import_service.rb index 4e5352db7..5228d753d 100644 --- a/app/services/imports/import_service.rb +++ b/app/services/imports/import_service.rb @@ -2,8 +2,6 @@ module Imports class ImportService include Imports::ImportUtils - private - def initialize(storage_service, logger = Rails.logger) @storage_service = storage_service @logger = logger diff --git a/app/services/imports/lettings_logs_import_processor.rb b/app/services/imports/lettings_logs_import_processor.rb index 2d41a7fd9..977d99656 100644 --- a/app/services/imports/lettings_logs_import_processor.rb +++ b/app/services/imports/lettings_logs_import_processor.rb @@ -320,14 +320,14 @@ module Imports end def compute_differences(lettings_log, attributes) - differences = attributes.map do |key, value| + differences = attributes.map { |key, value| lettings_log_value = lettings_log.send(key.to_sym) next if FIELDS_NOT_PRESENT_IN_SOFTWIRE_DATA.include?(key) next if value == lettings_log_value "#{key} #{value.inspect} #{lettings_log_value.inspect}" - end.compact + }.compact if differences.any? @logger.warn "Differences found when saving log #{lettings_log.old_id}: #{differences}" diff --git a/spec/jobs/lettings_log_import_job_spec.rb b/spec/jobs/lettings_log_import_job_spec.rb index c14219219..89c6e6dfb 100644 --- a/spec/jobs/lettings_log_import_job_spec.rb +++ b/spec/jobs/lettings_log_import_job_spec.rb @@ -7,10 +7,6 @@ RSpec.describe LettingsLogImportJob do describe "#perform" do context "with valid params" do - before do - - end - it "executes LettingsLogsImportProcessor" do expect(Imports::LettingsLogsImportProcessor).to receive(:new) diff --git a/spec/services/imports/lettings_logs_import_processor_spec.rb b/spec/services/imports/lettings_logs_import_processor_spec.rb index 0e82e9020..9625c4b0b 100644 --- a/spec/services/imports/lettings_logs_import_processor_spec.rb +++ b/spec/services/imports/lettings_logs_import_processor_spec.rb @@ -11,10 +11,9 @@ RSpec.describe Imports::LettingsLogsImportProcessor do 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) } - - def open_file(directory, filename) - File.open("#{directory}/#{filename}.xml") - end + let(:lettings_log_id) { "0ead17cb-1668-442d-898c-0d52879ff592" } + let(:lettings_log_file) { open_file(fixture_directory, lettings_log_id) } + let(:lettings_log_xml) { Nokogiri::XML(lettings_log_file) } before do WebMock.stub_request(:get, /api.postcodes.io\/postcodes\/LS166FT/) @@ -37,14 +36,10 @@ RSpec.describe Imports::LettingsLogsImportProcessor do allow(FormHandler.instance).to receive(:get_form).with("current_lettings").and_return(real_2022_2023_form) end - let(:lettings_log_id) { "0ead17cb-1668-442d-898c-0d52879ff592" } - let(:lettings_log_file) { open_file(fixture_directory, lettings_log_id) } - let(:lettings_log_xml) { Nokogiri::XML(lettings_log_file) } - - describe '#initialize' do + describe "#initialize" do context "with valid params" do it "sets document-id as old_id" do - import = Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger) + import = described_class.new(lettings_log_xml.to_s, logger) expect(import.old_id).to eq "0ead17cb-1668-442d-898c-0d52879ff592" end @@ -57,7 +52,7 @@ RSpec.describe Imports::LettingsLogsImportProcessor do expect(logger).to receive(:warn).with(/is not completed/).once expect(logger).to receive(:warn).with(/lettings log with old id:#{lettings_log_id} is incomplete but status should be complete/).once - import = Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger) + import = described_class.new(lettings_log_xml.to_s, logger) lettings_log = LettingsLog.where(old_id: lettings_log_id).first expect(lettings_log&.voiddate).to be_nil @@ -69,7 +64,7 @@ RSpec.describe Imports::LettingsLogsImportProcessor do before { lettings_log_xml.at_xpath("//xmlns:OWNINGORGID").content = 99_999 } it "raises an exception" do - expect { Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger) } + expect { described_class.new(lettings_log_xml.to_s, logger) } .to raise_error(RuntimeError, "Organisation not found with legacy ID 99999") end end @@ -83,7 +78,7 @@ RSpec.describe Imports::LettingsLogsImportProcessor do it "sets the economic status to child under 16" do # The update is done when calculating derived variables expect(logger).to receive(:warn).with(/Differences found when saving log/) - Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger) + described_class.new(lettings_log_xml.to_s, logger) lettings_log = LettingsLog.where(old_id: lettings_log_id).first expect(lettings_log&.ecstat2).to be(9) @@ -94,7 +89,7 @@ RSpec.describe Imports::LettingsLogsImportProcessor do before { lettings_log_xml.at_xpath("//xmlns:P2Rel").content = "Refused" } it "sets the relationship to lead tenant to child" do - Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger) + described_class.new(lettings_log_xml.to_s, logger) lettings_log = LettingsLog.where(old_id: lettings_log_id).first expect(lettings_log&.relat2).to eq("C") @@ -110,14 +105,14 @@ RSpec.describe Imports::LettingsLogsImportProcessor do it "intercepts the relevant validation error" do expect(logger).to receive(:warn).with(/Removing internal transfer referral since previous tenancy is a non social housing/) - expect { Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger) } + expect { described_class.new(lettings_log_xml.to_s, logger) } .not_to raise_error end it "clears out the referral answer" do allow(logger).to receive(:warn) - Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger) + described_class.new(lettings_log_xml.to_s, logger) lettings_log = LettingsLog.find_by(old_id: lettings_log_id) expect(lettings_log).not_to be_nil @@ -132,14 +127,14 @@ RSpec.describe Imports::LettingsLogsImportProcessor do it "intercepts the relevant validation error" do expect(logger).to receive(:warn).with(/Removing internal transfer referral since previous tenancy is fixed terms or lifetime/) - expect { Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger) } + expect { described_class.new(lettings_log_xml.to_s, logger) } .not_to raise_error end it "clears out the referral answer" do allow(logger).to receive(:warn) - Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger) + described_class.new(lettings_log_xml.to_s, logger) lettings_log = LettingsLog.find_by(old_id: lettings_log_id) expect(lettings_log).not_to be_nil @@ -155,7 +150,7 @@ RSpec.describe Imports::LettingsLogsImportProcessor do end it "completes the log" do - Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger) + described_class.new(lettings_log_xml.to_s, logger) lettings_log = LettingsLog.find_by(old_id: lettings_log_id) expect(lettings_log.status).to eq("completed") end @@ -179,7 +174,7 @@ RSpec.describe Imports::LettingsLogsImportProcessor do end it "completes the log" do - Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger) + described_class.new(lettings_log_xml.to_s, logger) lettings_log = LettingsLog.find_by(old_id: lettings_log_id) expect(lettings_log.status).to eq("completed") end @@ -192,7 +187,7 @@ RSpec.describe Imports::LettingsLogsImportProcessor do end it "completes the log" do - Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger) + described_class.new(lettings_log_xml.to_s, logger) lettings_log = LettingsLog.find_by(old_id: lettings_log_id) expect(lettings_log.status).to eq("completed") end @@ -203,7 +198,7 @@ RSpec.describe Imports::LettingsLogsImportProcessor do it "sets the scheme and location values" do expect(logger).not_to receive(:warn) - Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger) + described_class.new(lettings_log_xml.to_s, logger) lettings_log = LettingsLog.find_by(old_id: lettings_log_id) expect(lettings_log.scheme_id).not_to be_nil @@ -219,7 +214,7 @@ RSpec.describe Imports::LettingsLogsImportProcessor do it "sets the scheme and location values" do expect(logger).not_to receive(:warn) - Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger) + described_class.new(lettings_log_xml.to_s, logger) lettings_log = LettingsLog.find_by(old_id: lettings_log_id) expect(lettings_log.scheme_id).not_to be_nil @@ -228,4 +223,8 @@ RSpec.describe Imports::LettingsLogsImportProcessor do end end end + + def open_file(directory, filename) + File.open("#{directory}/#{filename}.xml") + end end diff --git a/spec/services/imports/lettings_logs_import_service_spec.rb b/spec/services/imports/lettings_logs_import_service_spec.rb index 6a9099ef5..5aa57dce6 100644 --- a/spec/services/imports/lettings_logs_import_service_spec.rb +++ b/spec/services/imports/lettings_logs_import_service_spec.rb @@ -14,9 +14,15 @@ RSpec.describe Imports::LettingsLogsImportService do let(:scheme1) { FactoryBot.create(:scheme, old_visible_id: 123, 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") - end + let(:remote_folder) { "lettings_logs" } + let(:lettings_log_id) { "0ead17cb-1668-442d-898c-0d52879ff592" } + let(:lettings_log_id2) { "166fc004-392e-47a8-acb8-1c018734882b" } + let(:lettings_log_id3) { "00d2343e-d5fa-4c89-8400-ec3854b0f2b4" } + let(:lettings_log_id4) { "0b4a68df-30cc-474a-93c0-a56ce8fdad3b" } + let(:lettings_log_id5) { "893ufj2s-lq77-42m4-rty6-ej09gh585uy1" } + let(:lettings_log_id6) { "5ybz29dj-l33t-k1l0-hj86-n4k4ma77xkcd" } + let(:lettings_log_file) { open_file(fixture_directory, lettings_log_id5) } + let(:lettings_log_xml) { Nokogiri::XML(lettings_log_file) } before do WebMock.stub_request(:get, /api.postcodes.io\/postcodes\/LS166FT/) @@ -37,19 +43,7 @@ RSpec.describe Imports::LettingsLogsImportService do # 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) allow(FormHandler.instance).to receive(:get_form).with("current_lettings").and_return(real_2022_2023_form) - end - - let(:remote_folder) { "lettings_logs" } - let(:lettings_log_id) { "0ead17cb-1668-442d-898c-0d52879ff592" } - let(:lettings_log_id2) { "166fc004-392e-47a8-acb8-1c018734882b" } - let(:lettings_log_id3) { "00d2343e-d5fa-4c89-8400-ec3854b0f2b4" } - let(:lettings_log_id4) { "0b4a68df-30cc-474a-93c0-a56ce8fdad3b" } - let(:lettings_log_id5) { "893ufj2s-lq77-42m4-rty6-ej09gh585uy1" } - let(:lettings_log_id6) { "5ybz29dj-l33t-k1l0-hj86-n4k4ma77xkcd" } - let(:lettings_log_file) { open_file(fixture_directory, lettings_log_id5) } - let(:lettings_log_xml) { Nokogiri::XML(lettings_log_file) } - before do # Stub the S3 file listing and download allow(storage_service).to receive(:list_files) .and_return(%W[#{remote_folder}/#{lettings_log_id}.xml #{remote_folder}/#{lettings_log_id2}.xml #{remote_folder}/#{lettings_log_id3}.xml #{remote_folder}/#{lettings_log_id4}.xml]) @@ -86,12 +80,12 @@ RSpec.describe Imports::LettingsLogsImportService do expect(logger).not_to receive(:error) expect(logger).not_to receive(:warn) - expect do + expect { perform_enqueued_jobs do lettings_log_service.create_logs(remote_folder) lettings_log_service.create_logs(remote_folder) end - end.to change(LettingsLog, :count).by(4) # Rather than 8 + }.to change(LettingsLog, :count).by(4) # Rather than 8 end end @@ -135,4 +129,8 @@ RSpec.describe Imports::LettingsLogsImportService do end end end + + def open_file(directory, filename) + File.open("#{directory}/#{filename}.xml") + end end