From f447939336b4a0219bb180a0b2f45dafc285019e Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Tue, 29 Aug 2023 17:06:43 +0200 Subject: [PATCH] CLDC-2663 Import sales form id (#1851) * Add old form id column * Import old for id * Export old_form_id instead of old_id * Add old_form_id import task and service * rename a spec file --- app/services/csv/sales_log_csv_service.rb | 2 +- .../sales_logs_field_import_service.rb | 21 +++++++++++ .../imports/sales_logs_import_service.rb | 1 + ...20230816083950_add_old_form_id_to_sales.rb | 7 ++++ db/schema.rb | 3 +- lib/tasks/data_import_field.rake | 2 +- .../files/sales_logs_csv_export_codes.csv | 2 +- .../files/sales_logs_csv_export_labels.csv | 2 +- ...ield_spec.rb => data_import_field_spec.rb} | 21 +++++++++++ .../csv/sales_log_csv_service_spec.rb | 2 +- .../sales_logs_field_import_service_spec.rb | 37 +++++++++++++++++++ .../imports/sales_logs_import_service_spec.rb | 11 ++++++ 12 files changed, 105 insertions(+), 6 deletions(-) create mode 100644 db/migrate/20230816083950_add_old_form_id_to_sales.rb rename spec/lib/tasks/{date_import_field_spec.rb => data_import_field_spec.rb} (87%) diff --git a/app/services/csv/sales_log_csv_service.rb b/app/services/csv/sales_log_csv_service.rb index 895b544cf..57f48c126 100644 --- a/app/services/csv/sales_log_csv_service.rb +++ b/app/services/csv/sales_log_csv_service.rb @@ -122,7 +122,7 @@ module Csv question.id end end - non_question_fields = %w[id status created_at updated_at old_id collection_start_year creation_method is_dpo] + non_question_fields = %w[id status created_at updated_at old_form_id collection_start_year creation_method is_dpo] non_question_fields + attributes end end diff --git a/app/services/imports/sales_logs_field_import_service.rb b/app/services/imports/sales_logs_field_import_service.rb index e677cc8a0..2a08a7ccf 100644 --- a/app/services/imports/sales_logs_field_import_service.rb +++ b/app/services/imports/sales_logs_field_import_service.rb @@ -6,6 +6,8 @@ module Imports import_from(folder, :update_creation_method) when "owning_organisation_id" import_from(folder, :update_owning_organisation_id) + when "old_form_id" + import_from(folder, :update_old_form_id) else raise "Updating #{field} is not supported by the field import service" end @@ -51,5 +53,24 @@ module Imports @logger.warn("sales log with old id #{old_id} not found") end end + + def update_old_form_id(xml_doc) + return unless meta_field_value(xml_doc, "form-name").include?("Sales") + + old_id = meta_field_value(xml_doc, "document-id") + record = SalesLog.find_by(old_id:) + + if record.present? + if record.old_form_id.present? + @logger.info("sales log #{record.id} has a value for old_form_id, skipping update") + else + old_form_id = safe_string_as_integer(xml_doc, "Form") + record.update!(old_form_id:) + @logger.info("sales log #{record.id}'s old_form_id value has been set to #{old_form_id}") + end + else + @logger.warn("sales log with old id #{old_id} not found") + end + end end end diff --git a/app/services/imports/sales_logs_import_service.rb b/app/services/imports/sales_logs_import_service.rb index 2c0bd4972..0d780c87d 100644 --- a/app/services/imports/sales_logs_import_service.rb +++ b/app/services/imports/sales_logs_import_service.rb @@ -28,6 +28,7 @@ module Imports attributes["owning_organisation_id"] = find_organisation_id(xml_doc, "OWNINGORGID") attributes["type"] = unsafe_string_as_integer(xml_doc, "DerSaleType") attributes["old_id"] = meta_field_value(xml_doc, "document-id") + attributes["old_form_id"] = safe_string_as_integer(xml_doc, "Form") attributes["creation_method"] = creation_method(xml_doc) attributes["created_at"] = Time.zone.parse(meta_field_value(xml_doc, "created-date")) attributes["updated_at"] = Time.zone.parse(meta_field_value(xml_doc, "modified-date")) diff --git a/db/migrate/20230816083950_add_old_form_id_to_sales.rb b/db/migrate/20230816083950_add_old_form_id_to_sales.rb new file mode 100644 index 000000000..f5e02f1ce --- /dev/null +++ b/db/migrate/20230816083950_add_old_form_id_to_sales.rb @@ -0,0 +1,7 @@ +class AddOldFormIdToSales < ActiveRecord::Migration[7.0] + def change + change_table :sales_logs, bulk: true do |t| + t.integer :old_form_id + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 7b5167a24..fe5b6cde4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_07_19_150610) do +ActiveRecord::Schema[7.0].define(version: 2023_08_16_083950) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -616,6 +616,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_07_19_150610) do t.datetime "discarded_at" t.integer "stairowned_value_check" t.integer "creation_method", default: 1 + t.integer "old_form_id" t.index ["bulk_upload_id"], name: "index_sales_logs_on_bulk_upload_id" t.index ["created_by_id"], name: "index_sales_logs_on_created_by_id" t.index ["old_id"], name: "index_sales_logs_on_old_id", unique: true diff --git a/lib/tasks/data_import_field.rake b/lib/tasks/data_import_field.rake index d16ae752e..80e42399e 100644 --- a/lib/tasks/data_import_field.rake +++ b/lib/tasks/data_import_field.rake @@ -29,7 +29,7 @@ namespace :core do # We only allow a reduced list of known fields to be updatable case field - when "owning_organisation_id" + when "owning_organisation_id", "old_form_id" s3_service = Storage::S3Service.new(PlatformHelper.is_paas? ? Configuration::PaasConfigurationService.new : Configuration::EnvConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"]) archive_io = s3_service.get_file_io(path) archive_service = Storage::ArchiveService.new(archive_io) diff --git a/spec/fixtures/files/sales_logs_csv_export_codes.csv b/spec/fixtures/files/sales_logs_csv_export_codes.csv index 3467917d6..bd7b8976a 100644 --- a/spec/fixtures/files/sales_logs_csv_export_codes.csv +++ b/spec/fixtures/files/sales_logs_csv_export_codes.csv @@ -1,2 +1,2 @@ -id,status,created_at,updated_at,old_id,collection_start_year,creation_method,is_dpo,owning_organisation_name,created_by,day,month,year,purchid,ownershipsch,type,othtype,companybuy,buylivein,jointpur,jointmore,beds,proptype,builtype,pcodenk,uprn,uprn_confirmed,address_line1,address_line2,town_or_city,county,pcode1,pcode2,la_known,la,la_label,wchair,noint,privacynotice,age1,sex1,ethnic_group,ethnic,national,ecstat1,buy1livein,relat2,age2,sex2,ethnic_group2,ethnicbuy2,nationalbuy2,ecstat2,buy2livein,hholdcount,relat3,age3,sex3,ecstat3,relat4,age4,sex4,ecstat4,relat5,age5,sex5,ecstat5,relat6,age6,sex6,ecstat6,prevten,ppcodenk,ppostc1,ppostc2,previous_la_known,prevloc,prevloc_label,pregyrha,pregother,pregla,pregghb,pregblank,buy2living,prevtenbuy2,hhregres,hhregresstill,armedforcesspouse,disabled,wheel,income1nk,income1,inc1mort,income2nk,income2,inc2mort,hb,savingsnk,savings,prevown,prevshared,proplen,staircase,stairbought,stairowned,staircasesale,resale,exday,exmonth,exyear,hoday,homonth,hoyear,lanomagr,soctenant,frombeds,fromprop,socprevten,value,equity,mortgageused,mortgage,mortgagelender,mortgagelenderother,mortlen,extrabor,deposit,cashdis,mrent,has_mscharge,mscharge,discount,grant +id,status,created_at,updated_at,old_form_id,collection_start_year,creation_method,is_dpo,owning_organisation_name,created_by,day,month,year,purchid,ownershipsch,type,othtype,companybuy,buylivein,jointpur,jointmore,beds,proptype,builtype,pcodenk,uprn,uprn_confirmed,address_line1,address_line2,town_or_city,county,pcode1,pcode2,la_known,la,la_label,wchair,noint,privacynotice,age1,sex1,ethnic_group,ethnic,national,ecstat1,buy1livein,relat2,age2,sex2,ethnic_group2,ethnicbuy2,nationalbuy2,ecstat2,buy2livein,hholdcount,relat3,age3,sex3,ecstat3,relat4,age4,sex4,ecstat4,relat5,age5,sex5,ecstat5,relat6,age6,sex6,ecstat6,prevten,ppcodenk,ppostc1,ppostc2,previous_la_known,prevloc,prevloc_label,pregyrha,pregother,pregla,pregghb,pregblank,buy2living,prevtenbuy2,hhregres,hhregresstill,armedforcesspouse,disabled,wheel,income1nk,income1,inc1mort,income2nk,income2,inc2mort,hb,savingsnk,savings,prevown,prevshared,proplen,staircase,stairbought,stairowned,staircasesale,resale,exday,exmonth,exyear,hoday,homonth,hoyear,lanomagr,soctenant,frombeds,fromprop,socprevten,value,equity,mortgageused,mortgage,mortgagelender,mortgagelenderother,mortlen,extrabor,deposit,cashdis,mrent,has_mscharge,mscharge,discount,grant ,completed,2023-02-08T00:00:00+00:00,2023-02-08T00:00:00+00:00,,2022,1,false,DLUHC,billyboy@eyeklaud.com,8,2,2023,,2,8,,,,1,1,2,1,1,0,,,Address line 1,,Town or city,,SW1A,1AA,1,E09000003,Barnet,1,2,1,30,X,17,17,18,1,1,P,35,X,17,,13,1,1,6,C,14,X,9,X,18,X,3,R,40,X,2,R,40,X,1,1,1,,,0,,,1,1,1,1,,3,,1,4,5,1,1,0,10000,1,0,10000,1,4,1,,1,2,10,,,,,,,,,,,,,,,,,110000.0,,1,20000.0,5,,10,1,80000.0,,,1,100.0,,10000.0 diff --git a/spec/fixtures/files/sales_logs_csv_export_labels.csv b/spec/fixtures/files/sales_logs_csv_export_labels.csv index dc0acf9d2..7060defbc 100644 --- a/spec/fixtures/files/sales_logs_csv_export_labels.csv +++ b/spec/fixtures/files/sales_logs_csv_export_labels.csv @@ -1,2 +1,2 @@ -id,status,created_at,updated_at,old_id,collection_start_year,creation_method,is_dpo,owning_organisation_name,created_by,day,month,year,purchid,ownershipsch,type,othtype,companybuy,buylivein,jointpur,jointmore,beds,proptype,builtype,pcodenk,uprn,uprn_confirmed,address_line1,address_line2,town_or_city,county,pcode1,pcode2,la_known,la,la_label,wchair,noint,privacynotice,age1,sex1,ethnic_group,ethnic,national,ecstat1,buy1livein,relat2,age2,sex2,ethnic_group2,ethnicbuy2,nationalbuy2,ecstat2,buy2livein,hholdcount,relat3,age3,sex3,ecstat3,relat4,age4,sex4,ecstat4,relat5,age5,sex5,ecstat5,relat6,age6,sex6,ecstat6,prevten,ppcodenk,ppostc1,ppostc2,previous_la_known,prevloc,prevloc_label,pregyrha,pregother,pregla,pregghb,pregblank,buy2living,prevtenbuy2,hhregres,hhregresstill,armedforcesspouse,disabled,wheel,income1nk,income1,inc1mort,income2nk,income2,inc2mort,hb,savingsnk,savings,prevown,prevshared,proplen,staircase,stairbought,stairowned,staircasesale,resale,exday,exmonth,exyear,hoday,homonth,hoyear,lanomagr,soctenant,frombeds,fromprop,socprevten,value,equity,mortgageused,mortgage,mortgagelender,mortgagelenderother,mortlen,extrabor,deposit,cashdis,mrent,has_mscharge,mscharge,discount,grant +id,status,created_at,updated_at,old_form_id,collection_start_year,creation_method,is_dpo,owning_organisation_name,created_by,day,month,year,purchid,ownershipsch,type,othtype,companybuy,buylivein,jointpur,jointmore,beds,proptype,builtype,pcodenk,uprn,uprn_confirmed,address_line1,address_line2,town_or_city,county,pcode1,pcode2,la_known,la,la_label,wchair,noint,privacynotice,age1,sex1,ethnic_group,ethnic,national,ecstat1,buy1livein,relat2,age2,sex2,ethnic_group2,ethnicbuy2,nationalbuy2,ecstat2,buy2livein,hholdcount,relat3,age3,sex3,ecstat3,relat4,age4,sex4,ecstat4,relat5,age5,sex5,ecstat5,relat6,age6,sex6,ecstat6,prevten,ppcodenk,ppostc1,ppostc2,previous_la_known,prevloc,prevloc_label,pregyrha,pregother,pregla,pregghb,pregblank,buy2living,prevtenbuy2,hhregres,hhregresstill,armedforcesspouse,disabled,wheel,income1nk,income1,inc1mort,income2nk,income2,inc2mort,hb,savingsnk,savings,prevown,prevshared,proplen,staircase,stairbought,stairowned,staircasesale,resale,exday,exmonth,exyear,hoday,homonth,hoyear,lanomagr,soctenant,frombeds,fromprop,socprevten,value,equity,mortgageused,mortgage,mortgagelender,mortgagelenderother,mortlen,extrabor,deposit,cashdis,mrent,has_mscharge,mscharge,discount,grant ,completed,2023-02-08T00:00:00+00:00,2023-02-08T00:00:00+00:00,,2022,single log,false,DLUHC,billyboy@eyeklaud.com,8,2,2023,,Yes - a discounted ownership scheme,Right to Acquire (RTA),,,,Yes,Yes,2,Flat or maisonette,Purpose built,Yes,,,Address line 1,,Town or city,,SW1A,1AA,Yes,E09000003,Barnet,Yes,Yes,1,30,Non-binary,Buyer 1 prefers not to say,17,United Kingdom,Full-time - 30 hours or more,Yes,Partner,35,Non-binary,17,,13,Full-time - 30 hours or more,Yes,6,Child,14,Non-binary,Child under 16,Other,18,Non-binary,"In government training into work, such as New Deal",Person prefers not to say,40,Non-binary,Part-time - Less than 30 hours,Person prefers not to say,40,Non-binary,Full-time - 30 hours or more,Local authority tenant,No,,,No,,,1,1,1,1,,3,,Yes,Yes,No,Yes,Yes,Yes,10000,Yes,Yes,10000,Yes,"Don’t know ",No,,Yes,2,10,,,,,,,,,,,,,,,,,110000.0,,Yes,20000.0,Cambridge Building Society,,10,Yes,80000.0,,,Yes,100.0,,10000.0 diff --git a/spec/lib/tasks/date_import_field_spec.rb b/spec/lib/tasks/data_import_field_spec.rb similarity index 87% rename from spec/lib/tasks/date_import_field_spec.rb rename to spec/lib/tasks/data_import_field_spec.rb index cac433449..84de7cf87 100644 --- a/spec/lib/tasks/date_import_field_spec.rb +++ b/spec/lib/tasks/data_import_field_spec.rb @@ -160,6 +160,27 @@ describe "data_import_field imports" do end end + context "and we update the old_form_id field" do + let(:field) { "old_form_id" } + + it "updates the logs from the given XML file when the VCAP_SERVICES environment variable exists" 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/sales_logs") + expect(Imports::SalesLogsFieldImportService).to receive(:new).with(archive_service) + expect(import_service).to receive(:update_field).with(field, "logs") + task.invoke(field, fixture_path) + end + + it "updates the logs from the given XML file when the VCAP_SERVICES environment variable does not exist" do + allow(ENV).to receive(:[]).with("VCAP_SERVICES") + expect(Storage::S3Service).to receive(:new).with(env_config_service, instance_name) + expect(storage_service).to receive(:get_file_io).with("spec/fixtures/imports/sales_logs") + expect(Imports::SalesLogsFieldImportService).to receive(:new).with(archive_service) + expect(import_service).to receive(:update_field).with(field, "logs") + task.invoke(field, fixture_path) + end + end + it "raises an exception if no parameters are provided" do expect { task.invoke }.to raise_error(/Usage/) end diff --git a/spec/services/csv/sales_log_csv_service_spec.rb b/spec/services/csv/sales_log_csv_service_spec.rb index 477b20709..62d3f406a 100644 --- a/spec/services/csv/sales_log_csv_service_spec.rb +++ b/spec/services/csv/sales_log_csv_service_spec.rb @@ -101,7 +101,7 @@ RSpec.describe Csv::SalesLogCsvService do it "includes attributes not related to questions to the headers" do headers = csv.first - expect(headers).to include(*%w[id status created_at updated_at old_id]) + expect(headers).to include(*%w[id status created_at updated_at old_form_id]) end it "returns a csv with the correct number of logs" do diff --git a/spec/services/imports/sales_logs_field_import_service_spec.rb b/spec/services/imports/sales_logs_field_import_service_spec.rb index 238e91359..f1ae6680c 100644 --- a/spec/services/imports/sales_logs_field_import_service_spec.rb +++ b/spec/services/imports/sales_logs_field_import_service_spec.rb @@ -110,4 +110,41 @@ RSpec.describe Imports::SalesLogsFieldImportService do end end end + + context "when updating old_form_id" do + let(:field) { "old_form_id" } + let(:sales_log_filename) { "shared_ownership_sales_log" } + + context "when the sales log has no offered value" do + let(:sales_log) { SalesLog.find_by(old_id: sales_log_filename) } + + before do + Imports::SalesLogsImportService.new(storage_service, logger).create_logs(fixture_directory) + sales_log_file.rewind + sales_log.update!(old_form_id: nil) + end + + it "updates the sales_log old_form_id value" do + expect(logger).to receive(:info).with("sales log #{sales_log.id}'s old_form_id value has been set to 300204") + expect { import_service.send(:update_field, field, remote_folder) } + .to(change { sales_log.reload.old_form_id }.from(nil).to(300_204)) + end + end + + context "when the sales log has a different offered value" do + let(:sales_log) { SalesLog.find_by(old_id: sales_log_filename) } + + before do + Imports::SalesLogsImportService.new(storage_service, logger).create_logs(fixture_directory) + sales_log_file.rewind + sales_log.update!(old_form_id: 123) + end + + it "does not update the sales_log old_form_id value" do + expect(logger).to receive(:info).with(/sales log \d+ has a value for old_form_id, skipping update/) + expect { import_service.send(:update_field, field, remote_folder) } + .not_to(change { sales_log.reload.old_form_id }) + end + end + end end diff --git a/spec/services/imports/sales_logs_import_service_spec.rb b/spec/services/imports/sales_logs_import_service_spec.rb index 3a7502925..740614775 100644 --- a/spec/services/imports/sales_logs_import_service_spec.rb +++ b/spec/services/imports/sales_logs_import_service_spec.rb @@ -139,6 +139,17 @@ RSpec.describe Imports::SalesLogsImportService do end end + context "when the log is valid" do + let(:sales_log_id) { "shared_ownership_sales_log" } + + it "correctly sets old form id" do + sales_log_service.send(:create_log, sales_log_xml) + + sales_log = SalesLog.find_by(old_id: sales_log_id) + expect(sales_log&.old_form_id).to eq(300_204) + end + end + context "when the mortgage lender is set to an existing option" do let(:sales_log_id) { "discounted_ownership_sales_log" }