diff --git a/config/initializers/multi_logger.rb b/config/initializers/multi_logger.rb index d39d786c1..6c0b4eb27 100644 --- a/config/initializers/multi_logger.rb +++ b/config/initializers/multi_logger.rb @@ -1,17 +1,19 @@ class MultiLogger - def initialize(*targets) - @targets = targets + def initialize(file_logger) + @rails_logger = Rails.logger + @file_logger = file_logger end def info(data) - @targets.each { |t| t.info(data) } + @rails_logger.info(data) end def warn(data) - @targets.each { |t| t.warn(data) } + @rails_logger.warn(data) end def error(data) - @targets.each { |t| t.error(data) } + @rails_logger.error(data) + @file_logger.error(data) end end diff --git a/lib/tasks/full_import.rake b/lib/tasks/full_import.rake index c1fe146d8..3b74b458a 100644 --- a/lib/tasks/full_import.rake +++ b/lib/tasks/full_import.rake @@ -9,7 +9,7 @@ namespace :import do s3_service = Storage::S3Service.new(PlatformHelper.is_paas? ? Configuration::PaasConfigurationService.new : Configuration::EnvConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"]) csv = CSV.parse(s3_service.get_file_io(institutions_csv_name), headers: true) logs_string = StringIO.new - logger = MultiLogger.new(Rails.logger, Logger.new(logs_string)) + logger = MultiLogger.new(Logger.new(logs_string)) org_count = csv.length initial_import_list = [ @@ -36,7 +36,7 @@ namespace :import do end log_file = "#{File.basename(institutions_csv_name, File.extname(institutions_csv_name))}_#{File.basename(archive_path, File.extname(archive_path))}_initial.log" - s3_service.write_file(log_file, logs_string.string) + s3_service.write_file(log_file, logs_string.string) if logs_string.string.present? logs_string.rewind logs_string.truncate(0) end @@ -53,7 +53,7 @@ namespace :import do csv = CSV.parse(s3_service.get_file_io(institutions_csv_name), headers: true) org_count = csv.length logs_string = StringIO.new - logger = MultiLogger.new(Rails.logger, Logger.new(logs_string)) + logger = MultiLogger.new(Logger.new(logs_string)) logs_import_list = [ Import.new(Imports::LettingsLogsImportService, :create_logs, "logs", logger), @@ -77,7 +77,7 @@ namespace :import do end log_file = "#{File.basename(institutions_csv_name, File.extname(institutions_csv_name))}_#{File.basename(archive_path, File.extname(archive_path))}_logs.log" - s3_service.write_file(log_file, logs_string.string) + s3_service.write_file(log_file, logs_string.string) if logs_string.string.present? logs_string.rewind logs_string.truncate(0) end diff --git a/spec/lib/tasks/full_import_spec.rb b/spec/lib/tasks/full_import_spec.rb index 0b8d3bd9a..b95b370cd 100644 --- a/spec/lib/tasks/full_import_spec.rb +++ b/spec/lib/tasks/full_import_spec.rb @@ -77,10 +77,10 @@ describe "full import", type: :task do allow(Imports::OrganisationRentPeriodImportService).to receive(:new).and_return(instance_double(Imports::OrganisationRentPeriodImportService)) end - it "creates a report using given organisation csv" do + it "does not write a report if there were no errors" do expect(Storage::S3Service).to receive(:new).with(env_config_service, instance_name) - expect(storage_service).to receive(:write_file).with("some_name_1_initial.log", / INFO -- : Performing initial imports for organisation org1/) - expect(storage_service).to receive(:write_file).with("some_name_2_initial.log", / INFO -- : Performing initial imports for organisation org2/) + expect(storage_service).not_to receive(:write_file).with("some_name_1_initial.log", "") + expect(storage_service).not_to receive(:write_file).with("some_name_2_initial.log", "") task.invoke("some_name.csv") end @@ -106,10 +106,10 @@ describe "full import", type: :task do allow(Imports::SalesLogsImportService).to receive(:new).and_return(instance_double(Imports::SalesLogsImportService)) end - it "creates a report using given organisation csv" do + it "does not write a report if there were no errors" do expect(Storage::S3Service).to receive(:new).with(env_config_service, instance_name) - expect(storage_service).to receive(:write_file).with("some_name_1_logs.log", / INFO -- : Importing logs for organisation org1, expecting 0 logs/) - expect(storage_service).to receive(:write_file).with("some_name_2_logs.log", / INFO -- : Importing logs for organisation org2, expecting 0 logs/) + expect(storage_service).not_to receive(:write_file).with("some_name_1_logs.log", "") + expect(storage_service).not_to receive(:write_file).with("some_name_2_logs.log", "") task.invoke("some_name.csv") end diff --git a/spec/services/imports/lettings_logs_import_service_spec.rb b/spec/services/imports/lettings_logs_import_service_spec.rb index 738d2ed39..8986710d9 100644 --- a/spec/services/imports/lettings_logs_import_service_spec.rb +++ b/spec/services/imports/lettings_logs_import_service_spec.rb @@ -14,7 +14,9 @@ RSpec.describe Imports::LettingsLogsImportService do end let(:storage_service) { instance_double(Storage::S3Service) } - let(:logger) { instance_double(ActiveSupport::Logger) } + let(:logs_string) { StringIO.new } + let(:file_logger) { Logger.new(logs_string) } + let(:logger) { MultiLogger.new(file_logger) } let(:real_2021_2022_form) { Form.new("config/forms/2021_2022.json") } let(:real_2022_2023_form) { Form.new("config/forms/2022_2023.json") } @@ -328,6 +330,24 @@ RSpec.describe Imports::LettingsLogsImportService do end end + context "and submitted log has errors" do + let(:lettings_log_id) { "0ead17cb-1668-442d-898c-0d52879ff592" } + + before do + lettings_log_xml.at_xpath("//meta:status").content = "submitted" + lettings_log_xml.at_xpath("//xmlns:HHMEMB").content = "32" + end + + it "logs the error" do + expect { lettings_log_service.send(:create_log, lettings_log_xml) } + .to raise_error(ActiveRecord::RecordInvalid) + + expect(logs_string.string).to include("Failed to import") + expect(logs_string.string).to include("ERROR -- : Validation error: Field hhmemb") + expect(logs_string.string).to include("Error message: outside_the_range") + end + end + context "and this is an internal transfer from a non social housing" do before do lettings_log_xml.at_xpath("//xmlns:Q11").content = "9 Residential care home" @@ -1048,7 +1068,9 @@ RSpec.describe Imports::LettingsLogsImportService do end let(:storage_service) { instance_double(Storage::S3Service) } - let(:logger) { instance_double(ActiveSupport::Logger) } + let(:logs_string) { StringIO.new } + let(:file_logger) { Logger.new(logs_string) } + let(:logger) { MultiLogger.new(file_logger) } let(:real_2021_2022_form) { Form.new("config/forms/2021_2022.json") } let(:real_2022_2023_form) { Form.new("config/forms/2022_2023.json") } diff --git a/spec/services/imports/organisation_import_service_spec.rb b/spec/services/imports/organisation_import_service_spec.rb index 8496c046f..41e843e3c 100644 --- a/spec/services/imports/organisation_import_service_spec.rb +++ b/spec/services/imports/organisation_import_service_spec.rb @@ -3,7 +3,7 @@ require "rails_helper" RSpec.describe Imports::OrganisationImportService do let(:storage_service) { instance_double(Storage::S3Service) } let(:logs_string) { StringIO.new } - let(:logger) { MultiLogger.new(Rails.logger, Logger.new(logs_string)) } + let(:logger) { MultiLogger.new(logs_string) } let(:folder_name) { "organisations" } let(:filenames) { %w[my_folder/my_file1.xml my_folder/my_file2.xml] } let(:fixture_directory) { "spec/fixtures/imports/institution" } @@ -87,7 +87,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(logs_string.string).to include("Organisation my_new_organisation is already present with old visible ID 1, skipping.") + expect(logs_string.string).not_to include("Organisation my_new_organisation is already present with old visible ID 1, skipping.") expect(Organisation).to exist(old_visible_id: "1", name: "HA Ltd") end