From 9efe368dfd38de4306a6191211f90f4be333f57b Mon Sep 17 00:00:00 2001 From: Rachael Booth Date: Fri, 16 Jun 2023 18:25:23 +0100 Subject: [PATCH 1/9] Add method for importing the 'offered' field into already imported lettings logs --- .../lettings_logs_field_import_service.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/app/services/imports/lettings_logs_field_import_service.rb b/app/services/imports/lettings_logs_field_import_service.rb index e59e178f2..694eb943d 100644 --- a/app/services/imports/lettings_logs_field_import_service.rb +++ b/app/services/imports/lettings_logs_field_import_service.rb @@ -8,6 +8,8 @@ module Imports import_from(folder, :update_major_repairs) when "lettings_allocation" import_from(folder, :update_lettings_allocation) + when "offered" + import_from(folder, :update_offered) else raise "Updating #{field} is not supported by the field import service" end @@ -15,6 +17,23 @@ module Imports private + def update_offered(xml_doc) + old_id = meta_field_value(xml_doc, "document-id") + record = LettingsLog.find_by(old_id:) + + if record.present? + if record.offered.present? + @logger.info("lettings log #{record.id} has a value for offered, skipping update") + else + offered = safe_string_as_integer(xml_doc, "Q20") + record.update!(offered:) + @logger.info("lettings log #{record.id}'s offered value has been set to #{offered}'") + end + else + @logger.warn("lettings log with old id #{old_id} not found") + end + end + def update_lettings_allocation(xml_doc) old_id = meta_field_value(xml_doc, "document-id") previous_status = meta_field_value(xml_doc, "status") From 47f02e6f9b24032b11865270f0095a38c319c0ed Mon Sep 17 00:00:00 2001 From: Rachael Booth Date: Mon, 19 Jun 2023 18:43:18 +0100 Subject: [PATCH 2/9] Add minimal tests --- .../lettings_logs_field_import_service.rb | 2 +- .../0ead17cb-1668-442d-898c-0d52879ff592.xml | 2 +- ...lettings_logs_field_import_service_spec.rb | 36 +++++++++++++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/app/services/imports/lettings_logs_field_import_service.rb b/app/services/imports/lettings_logs_field_import_service.rb index 694eb943d..8e33da0c2 100644 --- a/app/services/imports/lettings_logs_field_import_service.rb +++ b/app/services/imports/lettings_logs_field_import_service.rb @@ -1,5 +1,5 @@ module Imports - class LettingsLogsFieldImportService < ImportService + class LettingsLogsFieldImportService < LogsImportService def update_field(field, folder) case field when "tenancycode" diff --git a/spec/fixtures/imports/logs/0ead17cb-1668-442d-898c-0d52879ff592.xml b/spec/fixtures/imports/logs/0ead17cb-1668-442d-898c-0d52879ff592.xml index 15f97ce5b..87797e242 100644 --- a/spec/fixtures/imports/logs/0ead17cb-1668-442d-898c-0d52879ff592.xml +++ b/spec/fixtures/imports/logs/0ead17cb-1668-442d-898c-0d52879ff592.xml @@ -170,7 +170,7 @@ 2021-09-30 - 0 + 21 MCB003 diff --git a/spec/services/imports/lettings_logs_field_import_service_spec.rb b/spec/services/imports/lettings_logs_field_import_service_spec.rb index 0c55139e8..3cd4e930d 100644 --- a/spec/services/imports/lettings_logs_field_import_service_spec.rb +++ b/spec/services/imports/lettings_logs_field_import_service_spec.rb @@ -249,4 +249,40 @@ RSpec.describe Imports::LettingsLogsFieldImportService do end end end + + context "when updating offered" do + let(:field) { "offered" } + + context "when the lettings log has no offered value" do + let(:lettings_log) { LettingsLog.find_by(old_id: lettings_log_id) } + + before do + Imports::LettingsLogsImportService.new(storage_service, logger).create_logs(fixture_directory) + lettings_log_file.rewind + lettings_log.update!(offered: nil) + end + + it "updates the lettings_log offered value" do + expect(logger).to receive(:info).with(/lettings log \d+'s offered value has been set to 21/) + expect { import_service.send(:update_field, field, remote_folder) } + .to(change { lettings_log.reload.offered }.from(nil).to(21)) + end + end + + context "when the lettings log has a different offered value" do + let(:lettings_log) { LettingsLog.find_by(old_id: lettings_log_id) } + + before do + Imports::LettingsLogsImportService.new(storage_service, logger).create_logs(fixture_directory) + lettings_log_file.rewind + lettings_log.update!(offered: 18) + end + + it "does not update the lettings_log offered value" do + expect(logger).to receive(:info).with(/lettings log \d+ has a value for offered, skipping update/) + expect { import_service.send(:update_field, field, remote_folder) } + .not_to(change { lettings_log.reload.offered }) + end + end + end end From 7f3cc8daf1795263bf2ad2b96eceeb428e8eae1b Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Mon, 26 Jun 2023 12:03:30 +0100 Subject: [PATCH 3/9] feat: update tests --- spec/services/imports/lettings_logs_import_service_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/services/imports/lettings_logs_import_service_spec.rb b/spec/services/imports/lettings_logs_import_service_spec.rb index 00db327d5..869bd36fc 100644 --- a/spec/services/imports/lettings_logs_import_service_spec.rb +++ b/spec/services/imports/lettings_logs_import_service_spec.rb @@ -456,7 +456,7 @@ RSpec.describe Imports::LettingsLogsImportService do end end - context "and the number the property was relet is over 150" do + context "and the number of times the property was relet is over 150" do before do lettings_log_xml.at_xpath("//xmlns:Q20").content = "155" end @@ -1360,6 +1360,7 @@ RSpec.describe Imports::LettingsLogsImportService do lettings_log_xml.at_xpath("//xmlns:Q9a").content = "" lettings_log_xml.at_xpath("//xmlns:Q11").content = "32" lettings_log_xml.at_xpath("//xmlns:Q16").content = "1" + lettings_log_xml.at_xpath("//xmlns:Q20").content = "0" end it "intercepts the relevant validation error" do From 02a1fbba3b2d2eeab3a6ce340e335219bb1c13ad Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Wed, 28 Jun 2023 10:17:57 +0100 Subject: [PATCH 4/9] feat: update take task --- app/services/imports/lettings_logs_field_import_service.rb | 2 +- lib/tasks/data_import_field.rake | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/imports/lettings_logs_field_import_service.rb b/app/services/imports/lettings_logs_field_import_service.rb index 8e33da0c2..0d11610d5 100644 --- a/app/services/imports/lettings_logs_field_import_service.rb +++ b/app/services/imports/lettings_logs_field_import_service.rb @@ -2,7 +2,7 @@ module Imports class LettingsLogsFieldImportService < LogsImportService def update_field(field, folder) case field - when "tenancycode" + when "tenant_code" import_from(folder, :update_tenant_code) when "major_repairs" import_from(folder, :update_major_repairs) diff --git a/lib/tasks/data_import_field.rake b/lib/tasks/data_import_field.rake index ba389d317..22289dd36 100644 --- a/lib/tasks/data_import_field.rake +++ b/lib/tasks/data_import_field.rake @@ -9,7 +9,7 @@ namespace :core do # We only allow a reduced list of known fields to be updatable case field - when "tenant_code", "major_repairs", "lettings_allocation" + when "tenant_code", "major_repairs", "lettings_allocation", "offered" Imports::LettingsLogsFieldImportService.new(storage_service).update_field(field, path) else raise "Field #{field} cannot be updated by data_import_field" From 7cfc923457c39bdf53b1a501600d3d7bcacfe5ce Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Wed, 28 Jun 2023 10:17:57 +0100 Subject: [PATCH 5/9] feat: update rake task --- app/services/imports/lettings_logs_field_import_service.rb | 2 +- lib/tasks/data_import_field.rake | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/imports/lettings_logs_field_import_service.rb b/app/services/imports/lettings_logs_field_import_service.rb index 8e33da0c2..0d11610d5 100644 --- a/app/services/imports/lettings_logs_field_import_service.rb +++ b/app/services/imports/lettings_logs_field_import_service.rb @@ -2,7 +2,7 @@ module Imports class LettingsLogsFieldImportService < LogsImportService def update_field(field, folder) case field - when "tenancycode" + when "tenant_code" import_from(folder, :update_tenant_code) when "major_repairs" import_from(folder, :update_major_repairs) diff --git a/lib/tasks/data_import_field.rake b/lib/tasks/data_import_field.rake index ba389d317..22289dd36 100644 --- a/lib/tasks/data_import_field.rake +++ b/lib/tasks/data_import_field.rake @@ -9,7 +9,7 @@ namespace :core do # We only allow a reduced list of known fields to be updatable case field - when "tenant_code", "major_repairs", "lettings_allocation" + when "tenant_code", "major_repairs", "lettings_allocation", "offered" Imports::LettingsLogsFieldImportService.new(storage_service).update_field(field, path) else raise "Field #{field} cannot be updated by data_import_field" From ed1fd85d806556c3d0274167d401a556554bac3a Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Wed, 28 Jun 2023 10:27:13 +0100 Subject: [PATCH 6/9] feat: update rake task --- app/services/imports/lettings_logs_field_import_service.rb | 2 +- lib/tasks/data_import_field.rake | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/imports/lettings_logs_field_import_service.rb b/app/services/imports/lettings_logs_field_import_service.rb index 0d11610d5..8e33da0c2 100644 --- a/app/services/imports/lettings_logs_field_import_service.rb +++ b/app/services/imports/lettings_logs_field_import_service.rb @@ -2,7 +2,7 @@ module Imports class LettingsLogsFieldImportService < LogsImportService def update_field(field, folder) case field - when "tenant_code" + when "tenancycode" import_from(folder, :update_tenant_code) when "major_repairs" import_from(folder, :update_major_repairs) diff --git a/lib/tasks/data_import_field.rake b/lib/tasks/data_import_field.rake index 22289dd36..29512761d 100644 --- a/lib/tasks/data_import_field.rake +++ b/lib/tasks/data_import_field.rake @@ -9,7 +9,7 @@ namespace :core do # We only allow a reduced list of known fields to be updatable case field - when "tenant_code", "major_repairs", "lettings_allocation", "offered" + when "tenancycode", "major_repairs", "lettings_allocation", "offered" Imports::LettingsLogsFieldImportService.new(storage_service).update_field(field, path) else raise "Field #{field} cannot be updated by data_import_field" From bf5c40122563c99eb4af29adf347d38172f921b6 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Wed, 28 Jun 2023 10:43:50 +0100 Subject: [PATCH 7/9] feat: update rake task spec --- spec/lib/tasks/date_import_field_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/lib/tasks/date_import_field_spec.rb b/spec/lib/tasks/date_import_field_spec.rb index 6c7cc9943..1ed2d0073 100644 --- a/spec/lib/tasks/date_import_field_spec.rb +++ b/spec/lib/tasks/date_import_field_spec.rb @@ -28,8 +28,8 @@ describe "rake core:data_import_field", type: :task do allow(import_service).to receive(:update_field) end - context "and we update the tenant_code field" do - let(:field) { "tenant_code" } + context "and we update the tenancycode field" do + let(:field) { "tenancycode" } it "properly configures the storage service" do expect(Storage::S3Service).to receive(:new).with(paas_config_service, instance_name) From 2fd46a382d1cc04c1ad0bad2a72ec134ff19838d Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Thu, 29 Jun 2023 13:07:54 +0100 Subject: [PATCH 8/9] feat: update rake task and tests --- lib/tasks/data_import_field.rake | 11 ++++-- spec/lib/tasks/date_import_field_spec.rb | 43 ++++++++++++++---------- 2 files changed, 34 insertions(+), 20 deletions(-) diff --git a/lib/tasks/data_import_field.rake b/lib/tasks/data_import_field.rake index 29512761d..54d25517b 100644 --- a/lib/tasks/data_import_field.rake +++ b/lib/tasks/data_import_field.rake @@ -5,12 +5,17 @@ namespace :core do path = args[:path] raise "Usage: rake core:data_import_field['field','path/to/xml_files']" if path.blank? || field.blank? - storage_service = Storage::S3Service.new(Configuration::PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"]) - # We only allow a reduced list of known fields to be updatable case field when "tenancycode", "major_repairs", "lettings_allocation", "offered" - Imports::LettingsLogsFieldImportService.new(storage_service).update_field(field, path) + s3_service = Storage::S3Service.new(Configuration::PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"]) + archive_io = s3_service.get_file_io(path) + archive_service = Storage::ArchiveService.new(archive_io) + if archive_service.folder_present?("logs") + Rails.logger.info("Start importing field from folder logs") + Imports::LettingsLogsFieldImportService.new(archive_service).update_field(field, "logs") + Rails.logger.info("Imported") + end else raise "Field #{field} cannot be updated by data_import_field" end diff --git a/spec/lib/tasks/date_import_field_spec.rb b/spec/lib/tasks/date_import_field_spec.rb index 1ed2d0073..a30920694 100644 --- a/spec/lib/tasks/date_import_field_spec.rb +++ b/spec/lib/tasks/date_import_field_spec.rb @@ -17,27 +17,28 @@ describe "rake core:data_import_field", type: :task do allow(Configuration::PaasConfigurationService).to receive(:new).and_return(paas_config_service) allow(ENV).to receive(:[]) allow(ENV).to receive(:[]).with("IMPORT_PAAS_INSTANCE").and_return(instance_name) + allow(Imports::LettingsLogsFieldImportService).to receive(:new).and_return(import_service) end context "when importing a lettings log field" do let(:import_service) { instance_double(Imports::LettingsLogsFieldImportService) } - let(:fixture_path) { "spec/fixtures/imports/lettings_logs" } + let(:fixture_path) { "spec/fixtures/imports/logs" } + let(:archive_service) { instance_double(Storage::ArchiveService) } before do - allow(Imports::LettingsLogsFieldImportService).to receive(:new).and_return(import_service) allow(import_service).to receive(:update_field) + allow(Storage::ArchiveService).to receive(:new).and_return(archive_service) + allow(archive_service).to receive(:folder_present?).with("logs").and_return(true) end context "and we update the tenancycode field" do let(:field) { "tenancycode" } - it "properly configures the storage service" do + it "updates the logs from the given XML file" do expect(Storage::S3Service).to receive(:new).with(paas_config_service, instance_name) - task.invoke(field, fixture_path) - end - - it "calls the expected update method with parameters" do - expect(import_service).to receive(:update_field).with(field, fixture_path) + expect(storage_service).to receive(:get_file_io).with("spec/fixtures/imports/logs") + expect(Imports::LettingsLogsFieldImportService).to receive(:new).with(archive_service) + expect(import_service).to receive(:update_field).with(field, "logs") task.invoke(field, fixture_path) end end @@ -45,13 +46,11 @@ describe "rake core:data_import_field", type: :task do context "and we update the lettings_allocation fields" do let(:field) { "lettings_allocation" } - it "properly configures the storage service" do + it "updates the logs from the given XML file" do expect(Storage::S3Service).to receive(:new).with(paas_config_service, instance_name) - task.invoke(field, fixture_path) - end - - it "calls the expected update method with parameters" do - expect(import_service).to receive(:update_field).with(field, fixture_path) + expect(storage_service).to receive(:get_file_io).with("spec/fixtures/imports/logs") + expect(Imports::LettingsLogsFieldImportService).to receive(:new).with(archive_service) + expect(import_service).to receive(:update_field).with(field, "logs") task.invoke(field, fixture_path) end end @@ -59,13 +58,23 @@ describe "rake core:data_import_field", type: :task do context "and we update the major repairs fields" do let(:field) { "major_repairs" } - it "properly configures the storage service" do + it "updates the logs from the given XML file" do expect(Storage::S3Service).to receive(:new).with(paas_config_service, instance_name) + expect(storage_service).to receive(:get_file_io).with("spec/fixtures/imports/logs") + expect(Imports::LettingsLogsFieldImportService).to receive(:new).with(archive_service) + expect(import_service).to receive(:update_field).with(field, "logs") task.invoke(field, fixture_path) end + end - it "calls the expected update method with parameters" do - expect(import_service).to receive(:update_field).with(field, fixture_path) + context "and we update the offered fields" do + let(:field) { "offered" } + + it "updates the logs from the given XML file" do + expect(Storage::S3Service).to receive(:new).with(paas_config_service, instance_name) + expect(storage_service).to receive(:get_file_io).with("spec/fixtures/imports/logs") + expect(Imports::LettingsLogsFieldImportService).to receive(:new).with(archive_service) + expect(import_service).to receive(:update_field).with(field, "logs") task.invoke(field, fixture_path) end end From 520fee565f561fca1ef76ba4dfbfee7a574ae206 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Thu, 29 Jun 2023 15:07:40 +0100 Subject: [PATCH 9/9] feat: typo fix and return for sales logs --- .../imports/lettings_logs_field_import_service.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/app/services/imports/lettings_logs_field_import_service.rb b/app/services/imports/lettings_logs_field_import_service.rb index 8e33da0c2..a976e15b2 100644 --- a/app/services/imports/lettings_logs_field_import_service.rb +++ b/app/services/imports/lettings_logs_field_import_service.rb @@ -18,6 +18,8 @@ module Imports private def update_offered(xml_doc) + return if meta_field_value(xml_doc, "form-name").include?("Sales") + old_id = meta_field_value(xml_doc, "document-id") record = LettingsLog.find_by(old_id:) @@ -27,7 +29,7 @@ module Imports else offered = safe_string_as_integer(xml_doc, "Q20") record.update!(offered:) - @logger.info("lettings log #{record.id}'s offered value has been set to #{offered}'") + @logger.info("lettings log #{record.id}'s offered value has been set to #{offered}") end else @logger.warn("lettings log with old id #{old_id} not found") @@ -35,6 +37,8 @@ module Imports end def update_lettings_allocation(xml_doc) + return if meta_field_value(xml_doc, "form-name").include?("Sales") + old_id = meta_field_value(xml_doc, "document-id") previous_status = meta_field_value(xml_doc, "status") record = LettingsLog.find_by(old_id:) @@ -65,6 +69,8 @@ module Imports end def update_major_repairs(xml_doc) + return if meta_field_value(xml_doc, "form-name").include?("Sales") + old_id = meta_field_value(xml_doc, "document-id") record = LettingsLog.find_by(old_id:) @@ -88,6 +94,8 @@ module Imports end def update_tenant_code(xml_doc) + return if meta_field_value(xml_doc, "form-name").include?("Sales") + old_id = meta_field_value(xml_doc, "document-id") record = LettingsLog.find_by(old_id:)