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

@@ -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

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

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"))

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

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

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)

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

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

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

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

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

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" }