From c4c93dd4cdd3cdf57c7a7d4daaac39afb522c173 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Wed, 6 Sep 2023 14:13:46 +0100
Subject: [PATCH 01/18] CLDC-2713 Add a rake task to reimport addresses from
 csv (#1890)

* Add a rake task to reimport addresses from csv

* Add empty line

* Update postcode known

* Update values updated at column
---
 lib/tasks/import_address_from_csv.rake        |  42 +++++
 spec/fixtures/files/addresses_reimport.csv    |   6 +
 .../tasks/correct_address_from_csv_spec.rb    | 154 ++++++++++++++++++
 3 files changed, 202 insertions(+)
 create mode 100644 lib/tasks/import_address_from_csv.rake
 create mode 100644 spec/fixtures/files/addresses_reimport.csv
 create mode 100644 spec/lib/tasks/correct_address_from_csv_spec.rb

diff --git a/lib/tasks/import_address_from_csv.rake b/lib/tasks/import_address_from_csv.rake
new file mode 100644
index 000000000..e910f3a77
--- /dev/null
+++ b/lib/tasks/import_address_from_csv.rake
@@ -0,0 +1,42 @@
+namespace :data_import do
+  desc "Import address data from a csv file"
+  task :import_address_from_csv, %i[file_name] => :environment do |_task, args|
+    file_name = args[:file_name]
+
+    raise "Usage: rake data_import:import_address_from_csv['csv_file_name']" if file_name.blank?
+
+    s3_service = Storage::S3Service.new(PlatformHelper.is_paas? ? Configuration::PaasConfigurationService.new : Configuration::EnvConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
+    addresses_csv = CSV.parse(s3_service.get_file_io(file_name), headers: true)
+
+    addresses_csv.each do |row|
+      lettings_log_id = row[0]
+
+      if lettings_log_id.blank?
+        Rails.logger.info("Lettings log ID not provided for address: #{[row[1], row[2], row[3], row[4]].join(', ')}")
+        next
+      end
+
+      lettings_log = LettingsLog.find_by(id: lettings_log_id)
+      if lettings_log.blank?
+        Rails.logger.info("Could not find a lettings log with id #{lettings_log_id}")
+        next
+      end
+
+      lettings_log.uprn_known = 0
+      lettings_log.uprn = nil
+      lettings_log.uprn_confirmed = nil
+      lettings_log.address_line1 = row[1]
+      lettings_log.address_line2 = row[2]
+      lettings_log.town_or_city = row[3]
+      lettings_log.postcode_full = row[4]
+      lettings_log.postcode_known = lettings_log.postcode_full.present? ? 1 : nil
+      lettings_log.county = nil
+      lettings_log.is_la_inferred = nil
+      lettings_log.la = nil
+      lettings_log.values_updated_at = Time.zone.now
+
+      lettings_log.save!
+      Rails.logger.info("Updated lettings log #{lettings_log_id}, with address: #{[lettings_log.address_line1, lettings_log.address_line2, lettings_log.town_or_city, lettings_log.postcode_full].join(', ')}")
+    end
+  end
+end
diff --git a/spec/fixtures/files/addresses_reimport.csv b/spec/fixtures/files/addresses_reimport.csv
new file mode 100644
index 000000000..998d2fbfa
--- /dev/null
+++ b/spec/fixtures/files/addresses_reimport.csv
@@ -0,0 +1,6 @@
+lettings_log_id,address_line1,address_line2,town_or_city,postcode_full
+{id},address 1,address 2,town,B1 1BB
+{id2},address 3,address 4,city,B1 1BB
+{id3},,,,C1 1CC
+,address 5,address 6,city,D1 1DD
+fake_id,address 7,address 8,city,D1 1DD
diff --git a/spec/lib/tasks/correct_address_from_csv_spec.rb b/spec/lib/tasks/correct_address_from_csv_spec.rb
new file mode 100644
index 000000000..0a3d1c563
--- /dev/null
+++ b/spec/lib/tasks/correct_address_from_csv_spec.rb
@@ -0,0 +1,154 @@
+require "rails_helper"
+require "rake"
+
+RSpec.describe "data_import" do
+  def replace_entity_ids(lettings_log, second_lettings_log, third_lettings_log, export_template)
+    export_template.sub!(/\{id\}/, lettings_log.id.to_s)
+    export_template.sub!(/\{id2\}/, second_lettings_log.id.to_s)
+    export_template.sub!(/\{id3\}/, third_lettings_log.id.to_s)
+  end
+
+  describe ":import_address_from_csv", type: :task do
+    subject(:task) { Rake::Task["data_import:import_address_from_csv"] }
+
+    let(:instance_name) { "paas_import_instance" }
+    let(:storage_service) { instance_double(Storage::S3Service) }
+    let(:env_config_service) { instance_double(Configuration::EnvConfigurationService) }
+    let(:paas_config_service) { instance_double(Configuration::PaasConfigurationService) }
+
+    before do
+      allow(Storage::S3Service).to receive(:new).and_return(storage_service)
+      allow(Configuration::EnvConfigurationService).to receive(:new).and_return(env_config_service)
+      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(ENV).to receive(:[]).with("VCAP_SERVICES").and_return("dummy")
+
+      Rake.application.rake_require("tasks/import_address_from_csv")
+      Rake::Task.define_task(:environment)
+      task.reenable
+
+      WebMock.stub_request(:get, /api.postcodes.io/)
+        .to_return(status: 200, body: "{\"status\":404,\"error\":\"Postcode not found\"}", headers: {})
+      WebMock.stub_request(:get, /api.postcodes.io\/postcodes\/B11BB/)
+        .to_return(status: 200, body: '{"status":200,"result":{"admin_district":"Westminster","codes":{"admin_district":"E08000035"}}}', headers: {})
+    end
+
+    context "when the rake task is run" do
+      let(:addresses_csv_path) { "addresses_reimport_123.csv" }
+      let(:wrong_file_path) { "/test/no_csv_here.csv" }
+      let!(:lettings_log) do
+        create(:lettings_log,
+               uprn_known: nil,
+               uprn: nil,
+               uprn_confirmed: nil,
+               address_line1: nil,
+               address_line2: nil,
+               town_or_city: nil,
+               county: nil,
+               postcode_known: 1,
+               postcode_full: "A1 1AA",
+               la: "E06000064",
+               is_la_inferred: true)
+      end
+
+      let!(:second_lettings_log) do
+        create(:lettings_log,
+               uprn_known: 1,
+               uprn: "1",
+               uprn_confirmed: nil,
+               address_line1: "wrong address line1",
+               address_line2: "wrong address 2",
+               town_or_city: "wrong town",
+               county: "wrong city",
+               postcode_known: 1,
+               postcode_full: "A1 1AA",
+               la: "E06000064",
+               is_la_inferred: true)
+      end
+
+      let!(:third_lettings_log) do
+        create(:lettings_log,
+               uprn_known: 1,
+               uprn: "1",
+               uprn_confirmed: nil,
+               address_line1: "wrong address line1",
+               address_line2: "wrong address 2",
+               town_or_city: "wrong town",
+               county: "wrong city",
+               postcode_known: 1,
+               postcode_full: "A1 1AA",
+               la: "E06000064",
+               is_la_inferred: true)
+      end
+
+      before do
+        allow(storage_service).to receive(:get_file_io)
+        .with("addresses_reimport_123.csv")
+        .and_return(replace_entity_ids(lettings_log, second_lettings_log, third_lettings_log, File.open("./spec/fixtures/files/addresses_reimport.csv").read))
+      end
+
+      it "updates the log address when old address was not given" do
+        task.invoke(addresses_csv_path)
+        lettings_log.reload
+        expect(lettings_log.uprn_known).to eq(0)
+        expect(lettings_log.uprn).to eq(nil)
+        expect(lettings_log.uprn_confirmed).to eq(nil)
+        expect(lettings_log.address_line1).to eq("address 1")
+        expect(lettings_log.address_line2).to eq("address 2")
+        expect(lettings_log.town_or_city).to eq("town")
+        expect(lettings_log.county).to eq(nil)
+        expect(lettings_log.postcode_known).to eq(1)
+        expect(lettings_log.postcode_full).to eq("B1 1BB")
+        expect(lettings_log.la).to eq("E08000035")
+        expect(lettings_log.is_la_inferred).to eq(true)
+      end
+
+      it "updates the log address when old address was given" do
+        task.invoke(addresses_csv_path)
+        second_lettings_log.reload
+        expect(second_lettings_log.uprn_known).to eq(0)
+        expect(second_lettings_log.uprn).to eq(nil)
+        expect(second_lettings_log.uprn_confirmed).to eq(nil)
+        expect(second_lettings_log.address_line1).to eq("address 3")
+        expect(second_lettings_log.address_line2).to eq("address 4")
+        expect(second_lettings_log.town_or_city).to eq("city")
+        expect(second_lettings_log.county).to eq(nil)
+        expect(second_lettings_log.postcode_known).to eq(1)
+        expect(second_lettings_log.postcode_full).to eq("B1 1BB")
+        expect(second_lettings_log.la).to eq("E08000035")
+        expect(second_lettings_log.is_la_inferred).to eq(true)
+      end
+
+      it "updates the log address when new address is not given" do
+        task.invoke(addresses_csv_path)
+        third_lettings_log.reload
+        expect(third_lettings_log.uprn_known).to eq(0)
+        expect(third_lettings_log.uprn).to eq(nil)
+        expect(third_lettings_log.uprn_confirmed).to eq(nil)
+        expect(third_lettings_log.address_line1).to eq(nil)
+        expect(third_lettings_log.address_line2).to eq(nil)
+        expect(third_lettings_log.town_or_city).to eq(nil)
+        expect(third_lettings_log.county).to eq(nil)
+        expect(third_lettings_log.postcode_known).to eq(1)
+        expect(third_lettings_log.postcode_full).to eq("C11CC")
+        expect(third_lettings_log.la).to eq(nil)
+        expect(third_lettings_log.is_la_inferred).to eq(false)
+      end
+
+      it "logs the progress of the update" do
+        expect(Rails.logger).to receive(:info).with("Updated lettings log #{lettings_log.id}, with address: address 1, address 2, town, B1 1BB")
+        expect(Rails.logger).to receive(:info).with("Updated lettings log #{second_lettings_log.id}, with address: address 3, address 4, city, B1 1BB")
+        expect(Rails.logger).to receive(:info).with("Updated lettings log #{third_lettings_log.id}, with address: , , , C11CC")
+        expect(Rails.logger).to receive(:info).with("Lettings log ID not provided for address: address 5, address 6, city, D1 1DD")
+        expect(Rails.logger).to receive(:info).with("Could not find a lettings log with id fake_id")
+
+        task.invoke(addresses_csv_path)
+      end
+
+      it "raises an error when no path is given" do
+        expect { task.invoke(nil) }.to raise_error(RuntimeError, "Usage: rake data_import:import_address_from_csv['csv_file_name']")
+      end
+    end
+  end
+end

From 719321945b3e9e0eef7643a991a0ac079545374f Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Thu, 7 Sep 2023 11:48:41 +0100
Subject: [PATCH 02/18] Set unassigned user if legacy user belongs to a
 different organisation (#1894)

* Set user to unassigned in lettings logs if the legacy user exists but belongs to a different organisation

* Set user to unassigned in sales logs if the legacy user exists but belongs to a different organisation

* Lint
---
 .../imports/lettings_logs_import_service.rb   |  8 +++-
 .../imports/sales_logs_import_service.rb      |  8 +++-
 .../lettings_logs_import_service_spec.rb      | 40 ++++++++++++++++++
 .../imports/sales_logs_import_service_spec.rb | 42 +++++++++++++++++++
 4 files changed, 94 insertions(+), 4 deletions(-)

diff --git a/app/services/imports/lettings_logs_import_service.rb b/app/services/imports/lettings_logs_import_service.rb
index 138126cbb..a53048d83 100644
--- a/app/services/imports/lettings_logs_import_service.rb
+++ b/app/services/imports/lettings_logs_import_service.rb
@@ -251,8 +251,12 @@ module Imports
       owner_id = meta_field_value(xml_doc, "owner-user-id").strip
       if owner_id.present?
         user = LegacyUser.find_by(old_user_id: owner_id)&.user
-        if user.blank?
-          @logger.error("Lettings log '#{attributes['old_id']}' belongs to legacy user with owner-user-id: '#{owner_id}' which cannot be found. Assigning log to 'Unassigned' user.")
+        if user.blank? || (user.organisation_id != attributes["managing_organisation_id"] && user.organisation_id != attributes["owning_organisation_id"])
+          if user.blank?
+            @logger.error("Lettings log '#{attributes['old_id']}' belongs to legacy user with owner-user-id: '#{owner_id}' which cannot be found. Assigning log to 'Unassigned' user.")
+          else
+            @logger.error("Lettings log '#{attributes['old_id']}' belongs to legacy user with owner-user-id: '#{owner_id}' which belongs to a different organisation. Assigning log to 'Unassigned' user.")
+          end
           if User.find_by(name: "Unassigned", organisation_id: attributes["managing_organisation_id"])
             user = User.find_by(name: "Unassigned", organisation_id: attributes["managing_organisation_id"])
           else
diff --git a/app/services/imports/sales_logs_import_service.rb b/app/services/imports/sales_logs_import_service.rb
index ecee55c22..157e797b4 100644
--- a/app/services/imports/sales_logs_import_service.rb
+++ b/app/services/imports/sales_logs_import_service.rb
@@ -179,8 +179,12 @@ module Imports
       if owner_id.present?
         user = LegacyUser.find_by(old_user_id: owner_id)&.user
 
-        if user.blank?
-          @logger.error("Sales log '#{attributes['old_id']}' belongs to legacy user with owner-user-id: '#{owner_id}' which cannot be found. Assigning log to 'Unassigned' user.")
+        if user.blank? || user.organisation_id != attributes["owning_organisation_id"]
+          if user.blank?
+            @logger.error("Sales log '#{attributes['old_id']}' belongs to legacy user with owner-user-id: '#{owner_id}' which cannot be found. Assigning log to 'Unassigned' user.")
+          else
+            @logger.error("Sales log '#{attributes['old_id']}' belongs to legacy user with owner-user-id: '#{owner_id}' which belongs to a different organisation. Assigning log to 'Unassigned' user.")
+          end
           if User.find_by(name: "Unassigned", organisation_id: attributes["owning_organisation_id"])
             user = User.find_by(name: "Unassigned", organisation_id: attributes["owning_organisation_id"])
           else
diff --git a/spec/services/imports/lettings_logs_import_service_spec.rb b/spec/services/imports/lettings_logs_import_service_spec.rb
index 1025fcd3b..7dc3d578d 100644
--- a/spec/services/imports/lettings_logs_import_service_spec.rb
+++ b/spec/services/imports/lettings_logs_import_service_spec.rb
@@ -172,6 +172,46 @@ RSpec.describe Imports::LettingsLogsImportService do
         end
       end
 
+      context "and the user exists on a different organisation" do
+        before do
+          create(:legacy_user, old_user_id: "fake_id")
+          lettings_log_xml.at_xpath("//meta:owner-user-id").content = "fake_id"
+        end
+
+        it "creates a new unassigned user" do
+          expect(logger).to receive(:error).with("Lettings log '0ead17cb-1668-442d-898c-0d52879ff592' belongs to legacy user with owner-user-id: 'fake_id' which belongs to a different organisation. Assigning log to 'Unassigned' user.")
+          lettings_log_service.send(:create_log, lettings_log_xml)
+
+          lettings_log = LettingsLog.where(old_id: lettings_log_id).first
+          expect(lettings_log&.created_by&.name).to eq("Unassigned")
+        end
+
+        it "only creates one unassigned user" do
+          expect(logger).to receive(:error).with("Lettings log '0ead17cb-1668-442d-898c-0d52879ff592' belongs to legacy user with owner-user-id: 'fake_id' which belongs to a different organisation. Assigning log to 'Unassigned' user.")
+          expect(logger).to receive(:error).with("Lettings log 'fake_id' belongs to legacy user with owner-user-id: 'fake_id' which belongs to a different organisation. Assigning log to 'Unassigned' user.")
+          lettings_log_service.send(:create_log, lettings_log_xml)
+          lettings_log_xml.at_xpath("//meta:document-id").content = "fake_id"
+          lettings_log_service.send(:create_log, lettings_log_xml)
+
+          lettings_log = LettingsLog.where(old_id: lettings_log_id).first
+          second_lettings_log = LettingsLog.where(old_id: "fake_id").first
+          expect(lettings_log&.created_by).to eq(second_lettings_log&.created_by)
+        end
+
+        context "when unassigned user exist for a different organisation" do
+          let!(:other_unassigned_user) { create(:user, name: "Unassigned") }
+
+          it "creates a new unassigned user for current organisation" do
+            expect(logger).to receive(:error).with("Lettings log '0ead17cb-1668-442d-898c-0d52879ff592' belongs to legacy user with owner-user-id: 'fake_id' which belongs to a different organisation. Assigning log to 'Unassigned' user.")
+            lettings_log_service.send(:create_log, lettings_log_xml)
+
+            lettings_log = LettingsLog.where(old_id: lettings_log_id).first
+            expect(lettings_log&.created_by&.name).to eq("Unassigned")
+            expect(lettings_log&.created_by).not_to eq(other_unassigned_user)
+          end
+        end
+      end
+
       it "correctly sets imported at date" do
         lettings_log_service.send(:create_log, lettings_log_xml)
 
diff --git a/spec/services/imports/sales_logs_import_service_spec.rb b/spec/services/imports/sales_logs_import_service_spec.rb
index bbabc5c1a..df279e5d5 100644
--- a/spec/services/imports/sales_logs_import_service_spec.rb
+++ b/spec/services/imports/sales_logs_import_service_spec.rb
@@ -163,6 +163,48 @@ RSpec.describe Imports::SalesLogsImportService do
       end
     end
 
+    context "and the user exists on a different organisation" do
+      let(:sales_log_id) { "shared_ownership_sales_log" }
+
+      before do
+        create(:legacy_user, old_user_id: "fake_id")
+        sales_log_xml.at_xpath("//meta:owner-user-id").content = "fake_id"
+      end
+
+      it "creates a new unassigned user" do
+        expect(logger).to receive(:error).with("Sales log 'shared_ownership_sales_log' belongs to legacy user with owner-user-id: 'fake_id' which belongs to a different organisation. Assigning log to 'Unassigned' user.")
+        sales_log_service.send(:create_log, sales_log_xml)
+
+        sales_log = SalesLog.where(old_id: sales_log_id).first
+        expect(sales_log&.created_by&.name).to eq("Unassigned")
+      end
+
+      it "only creates one unassigned user" do
+        expect(logger).to receive(:error).with("Sales log 'shared_ownership_sales_log' belongs to legacy user with owner-user-id: 'fake_id' which belongs to a different organisation. Assigning log to 'Unassigned' user.")
+        expect(logger).to receive(:error).with("Sales log 'fake_id' belongs to legacy user with owner-user-id: 'fake_id' which belongs to a different organisation. Assigning log to 'Unassigned' user.")
+        sales_log_service.send(:create_log, sales_log_xml)
+        sales_log_xml.at_xpath("//meta:document-id").content = "fake_id"
+        sales_log_service.send(:create_log, sales_log_xml)
+
+        sales_log = SalesLog.where(old_id: sales_log_id).first
+        second_sales_log = SalesLog.where(old_id: "fake_id").first
+        expect(sales_log&.created_by).to eq(second_sales_log&.created_by)
+      end
+
+      context "when unassigned user exist for a different organisation" do
+        let!(:other_unassigned_user) { create(:user, name: "Unassigned") }
+
+        it "creates a new unassigned user for current organisation" do
+          expect(logger).to receive(:error).with("Sales log 'shared_ownership_sales_log' belongs to legacy user with owner-user-id: 'fake_id' which belongs to a different organisation. Assigning log to 'Unassigned' user.")
+          sales_log_service.send(:create_log, sales_log_xml)
+
+          sales_log = SalesLog.where(old_id: sales_log_id).first
+          expect(sales_log&.created_by&.name).to eq("Unassigned")
+          expect(sales_log&.created_by).not_to eq(other_unassigned_user)
+        end
+      end
+    end
+
     context "and the log startdate is before 22/23 collection period" do
       let(:sales_log_id) { "shared_ownership_sales_log" }
 

From c3636c12864f6c3d5d04765e52f169266c99e349 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Thu, 7 Sep 2023 12:03:38 +0100
Subject: [PATCH 03/18] CLDC-2696 Add reimport addresses task (#1891)

* Add update address service for lettings

* Only process collections 2023 onwards

* Update app/services/imports/lettings_logs_field_import_service.rb

Co-authored-by: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com>

* Refactor postcode_known

* test

---------

Co-authored-by: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com>
---
 .../lettings_logs_field_import_service.rb     |  44 ++++++
 lib/tasks/data_import_field.rake              |   2 +-
 spec/lib/tasks/data_import_field_spec.rb      |  12 ++
 ...lettings_logs_field_import_service_spec.rb | 148 ++++++++++++++++++
 4 files changed, 205 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 4e10c7e1c..38111a214 100644
--- a/app/services/imports/lettings_logs_field_import_service.rb
+++ b/app/services/imports/lettings_logs_field_import_service.rb
@@ -12,6 +12,8 @@ module Imports
         import_from(folder, :update_offered)
       when "creation_method"
         import_from(folder, :update_creation_method)
+      when "address"
+        import_from(folder, :update_address)
       else
         raise "Updating #{field} is not supported by the field import service"
       end
@@ -132,5 +134,47 @@ module Imports
         @logger.warn("Could not find record matching legacy ID #{old_id}")
       end
     end
+
+    def update_address(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:)
+      return @logger.info("lettings log #{record.id} is from previous collection year, skipping") if record.collection_start_year < 2023
+
+      if record.present?
+        if string_or_nil(xml_doc, "AddressLine1").present? && string_or_nil(xml_doc, "TownCity").present?
+          record.la = string_or_nil(xml_doc, "Q28ONS")
+          record.postcode_full = compose_postcode(xml_doc, "POSTCODE", "POSTCOD2")
+          record.postcode_known = postcode_known(record)
+          record.address_line1 = string_or_nil(xml_doc, "AddressLine1")
+          record.address_line2 = string_or_nil(xml_doc, "AddressLine2")
+          record.town_or_city = string_or_nil(xml_doc, "TownCity")
+          record.county = string_or_nil(xml_doc, "County")
+          record.uprn = nil
+          record.uprn_known = 0
+          record.uprn_confirmed = 0
+          record.values_updated_at = Time.zone.now
+          record.save!
+          @logger.info("lettings log #{record.id} address_line1 value has been set to #{record.address_line1}")
+          @logger.info("lettings log #{record.id} address_line2 value has been set to #{record.address_line2}")
+          @logger.info("lettings log #{record.id} town_or_city value has been set to #{record.town_or_city}")
+          @logger.info("lettings log #{record.id} county value has been set to #{record.county}")
+          @logger.info("lettings log #{record.id} postcode_full value has been set to #{record.postcode_full}")
+        else
+          @logger.info("lettings log #{record.id} is missing either or both of address_line1 and town or city, skipping")
+        end
+      else
+        @logger.warn("Could not find record matching legacy ID #{old_id}")
+      end
+    end
+
+    def postcode_known(record)
+      if record.postcode_full.nil?
+        record.la.nil? ? nil : 0 # Assumes we selected No in the form since the LA is present
+      else
+        1
+      end
+    end
   end
 end
diff --git a/lib/tasks/data_import_field.rake b/lib/tasks/data_import_field.rake
index 80e42399e..7eb253277 100644
--- a/lib/tasks/data_import_field.rake
+++ b/lib/tasks/data_import_field.rake
@@ -7,7 +7,7 @@ namespace :core do
 
     # We only allow a reduced list of known fields to be updatable
     case field
-    when "tenancycode", "major_repairs", "lettings_allocation", "offered"
+    when "tenancycode", "major_repairs", "lettings_allocation", "offered", "address"
       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/lib/tasks/data_import_field_spec.rb b/spec/lib/tasks/data_import_field_spec.rb
index 84de7cf87..4b5f6085f 100644
--- a/spec/lib/tasks/data_import_field_spec.rb
+++ b/spec/lib/tasks/data_import_field_spec.rb
@@ -92,6 +92,18 @@ describe "data_import_field imports" do
         end
       end
 
+      context "and we update the address fields" do
+        let(:field) { "address" }
+
+        it "updates the 2023 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 "raises an exception if no parameters are provided" do
         expect { task.invoke }.to raise_error(/Usage/)
       end
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 7a9767aa2..d6222d355 100644
--- a/spec/services/imports/lettings_logs_field_import_service_spec.rb
+++ b/spec/services/imports/lettings_logs_field_import_service_spec.rb
@@ -323,4 +323,152 @@ RSpec.describe Imports::LettingsLogsFieldImportService do
       end
     end
   end
+
+  context "when updating address" do
+    let(:field) { "address" }
+
+    before do
+      WebMock.stub_request(:get, /api.postcodes.io\/postcodes\/B11BB/)
+      .to_return(status: 200, body: '{"status":200,"result":{"admin_district":"Westminster","codes":{"admin_district":"E08000035"}}}', headers: {})
+
+      stub_request(:get, "https://api.os.uk/search/places/v1/uprn?key=OS_DATA_KEY&uprn=123")
+        .to_return(status: 500, body: "{}", headers: {})
+
+      Timecop.freeze(2023, 5, 5)
+      Singleton.__init__(FormHandler)
+      Imports::LettingsLogsImportService.new(storage_service, logger).create_logs(fixture_directory)
+      lettings_log_file.rewind
+    end
+
+    after do
+      Timecop.unfreeze
+      Singleton.__init__(FormHandler)
+    end
+
+    context "when the lettings log has no address values" do
+      let(:lettings_log_id) { "00d2343e-d5fa-4c89-8400-ec3854b0f2b4" }
+      let(:lettings_log) { LettingsLog.find_by(old_id: lettings_log_id) }
+
+      before do
+        lettings_log.update!(uprn_known: nil,
+                             startdate: Time.zone.local(2023, 5, 5),
+                             uprn: nil,
+                             uprn_confirmed: nil,
+                             address_line1: nil,
+                             address_line2: nil,
+                             town_or_city: nil,
+                             county: nil,
+                             postcode_known: nil,
+                             postcode_full: nil,
+                             la: nil,
+                             is_la_inferred: nil)
+      end
+
+      context "and new address values include address" do
+        before do
+          lettings_log_xml.at_xpath("//xmlns:UPRN").content = "123456781234"
+          lettings_log_xml.at_xpath("//xmlns:AddressLine1").content = "address 1"
+          lettings_log_xml.at_xpath("//xmlns:AddressLine2").content = "address 2"
+          lettings_log_xml.at_xpath("//xmlns:TownCity").content = "towncity"
+          lettings_log_xml.at_xpath("//xmlns:County").content = "county"
+          lettings_log_xml.at_xpath("//xmlns:POSTCODE").content = "B1"
+          lettings_log_xml.at_xpath("//xmlns:POSTCOD2").content = "1BB"
+          lettings_log_xml.at_xpath("//xmlns:Q28ONS").content = nil
+        end
+
+        it "updates the lettings_log prioritising address values" do
+          expect(logger).to receive(:info).with(/lettings log #{lettings_log.id} address_line1 value has been set to address 1/)
+          expect(logger).to receive(:info).with(/lettings log #{lettings_log.id} address_line2 value has been set to address 2/)
+          expect(logger).to receive(:info).with(/lettings log #{lettings_log.id} town_or_city value has been set to towncity/)
+          expect(logger).to receive(:info).with(/lettings log #{lettings_log.id} county value has been set to county/)
+          expect(logger).to receive(:info).with(/lettings log #{lettings_log.id} postcode_full value has been set to B1 1BB/)
+          import_service.send(:update_address, lettings_log_xml)
+          lettings_log.reload
+
+          expect(lettings_log.uprn_known).to eq(0)
+          expect(lettings_log.uprn).to eq(nil)
+          expect(lettings_log.uprn_confirmed).to eq(nil)
+          expect(lettings_log.address_line1).to eq("address 1")
+          expect(lettings_log.address_line2).to eq("address 2")
+          expect(lettings_log.town_or_city).to eq("towncity")
+          expect(lettings_log.county).to eq("county")
+          expect(lettings_log.postcode_known).to eq(1)
+          expect(lettings_log.postcode_full).to eq("B1 1BB")
+          expect(lettings_log.la).to eq("E08000035")
+          expect(lettings_log.is_la_inferred).to eq(true)
+        end
+      end
+
+      context "and new address values don't include address" do
+        it "skips the update" do
+          expect(logger).to receive(:info).with(/lettings log #{lettings_log.id} is missing either or both of address_line1 and town or city, skipping/)
+          import_service.send(:update_address, lettings_log_xml)
+        end
+      end
+    end
+
+    context "when the lettings log has address values" do
+      let(:lettings_log_id) { "00d2343e-d5fa-4c89-8400-ec3854b0f2b4" }
+      let(:lettings_log) { LettingsLog.find_by(old_id: lettings_log_id) }
+
+      before do
+        lettings_log_xml.at_xpath("//xmlns:UPRN").content = "123456781234"
+        lettings_log_xml.at_xpath("//xmlns:AddressLine1").content = "address 1"
+        lettings_log_xml.at_xpath("//xmlns:AddressLine2").content = "address 2"
+        lettings_log_xml.at_xpath("//xmlns:TownCity").content = "towncity"
+        lettings_log_xml.at_xpath("//xmlns:County").content = "county"
+        lettings_log_xml.at_xpath("//xmlns:POSTCODE").content = "B1"
+        lettings_log_xml.at_xpath("//xmlns:POSTCOD2").content = "1BC"
+        lettings_log_xml.at_xpath("//xmlns:Q28ONS").content = nil
+        lettings_log.update!(uprn_known: 1,
+                             startdate: Time.zone.local(2023, 5, 5),
+                             uprn: "123",
+                             uprn_confirmed: 0,
+                             address_line1: "wrong address line1",
+                             address_line2: "wrong address 2",
+                             town_or_city: "wrong town",
+                             county: "wrong city",
+                             postcode_known: 1,
+                             postcode_full: "A11AA",
+                             la: "E06000064",
+                             is_la_inferred: true)
+      end
+
+      it "replaces the lettings_log address values" do
+        expect(logger).to receive(:info).with(/lettings log #{lettings_log.id} address_line1 value has been set to address 1/)
+        expect(logger).to receive(:info).with(/lettings log #{lettings_log.id} address_line2 value has been set to address 2/)
+        expect(logger).to receive(:info).with(/lettings log #{lettings_log.id} town_or_city value has been set to towncity/)
+        expect(logger).to receive(:info).with(/lettings log #{lettings_log.id} county value has been set to county/)
+        expect(logger).to receive(:info).with(/lettings log #{lettings_log.id} postcode_full value has been set to B11BC/)
+        import_service.send(:update_address, lettings_log_xml)
+        lettings_log.reload
+
+        expect(lettings_log.uprn_known).to eq(0)
+        expect(lettings_log.uprn).to eq(nil)
+        expect(lettings_log.uprn_confirmed).to eq(nil)
+        expect(lettings_log.address_line1).to eq("address 1")
+        expect(lettings_log.address_line2).to eq("address 2")
+        expect(lettings_log.town_or_city).to eq("towncity")
+        expect(lettings_log.county).to eq("county")
+        expect(lettings_log.postcode_known).to eq(1)
+        expect(lettings_log.postcode_full).to eq("B11BC")
+        expect(lettings_log.la).to eq(nil)
+        expect(lettings_log.is_la_inferred).to eq(false)
+      end
+    end
+
+    context "when the lettings log is from before collection 23/24" do
+      let(:lettings_log_id) { "00d2343e-d5fa-4c89-8400-ec3854b0f2b4" }
+      let(:lettings_log) { LettingsLog.find_by(old_id: lettings_log_id) }
+
+      before do
+        lettings_log.update!(startdate: Time.zone.local(2022, 5, 5))
+      end
+
+      it "skips the update" do
+        expect(logger).to receive(:info).with(/lettings log #{lettings_log.id} is from previous collection year, skipping/)
+        import_service.send(:update_address, lettings_log_xml)
+      end
+    end
+  end
 end

From e5e6e1e21a02076092f0eaf6c91460ead2bdb57e Mon Sep 17 00:00:00 2001
From: natdeanlewissoftwire
 <94526761+natdeanlewissoftwire@users.noreply.github.com>
Date: Thu, 7 Sep 2023 16:09:46 +0100
Subject: [PATCH 04/18] Enforce offered as integer (#1897)

* feat: ensure offered is an integer (decimal 0 was triggering validation errors)

* feat: allow decimals but clear when not integer equivalent as per new requirements
---
 app/models/validations/shared_validations.rb  |  2 +-
 .../imports/lettings_logs_import_service.rb   |  1 +
 .../lettings_logs_import_service_spec.rb      | 21 +++++++++++++++++++
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/app/models/validations/shared_validations.rb b/app/models/validations/shared_validations.rb
index 8cf2e53fe..81bda79de 100644
--- a/app/models/validations/shared_validations.rb
+++ b/app/models/validations/shared_validations.rb
@@ -49,7 +49,7 @@ module Validations::SharedValidations
       elsif incorrect_accuracy || value.to_d != value.to_i    # if the user enters a value in exponent notation (eg '4e1') the to_i method does not convert this to the correct value
         field = question.check_answer_label || question.id
         case question.step
-        when 1 then record.errors.add question.id.to_sym, I18n.t("validations.numeric.whole_number", field:)
+        when 1 then record.errors.add question.id.to_sym, :not_integer, message: I18n.t("validations.numeric.whole_number", field:)
         when 10 then record.errors.add question.id.to_sym, I18n.t("validations.numeric.nearest_ten", field:)
         end
       end
diff --git a/app/services/imports/lettings_logs_import_service.rb b/app/services/imports/lettings_logs_import_service.rb
index a53048d83..cbaeee01a 100644
--- a/app/services/imports/lettings_logs_import_service.rb
+++ b/app/services/imports/lettings_logs_import_service.rb
@@ -326,6 +326,7 @@ module Imports
         %i[prevten non_temp_accommodation] => %w[prevten rsnvac],
         %i[joint not_joint_tenancy] => %w[joint],
         %i[offered outside_the_range] => %w[offered],
+        %i[offered not_integer] => %w[offered],
         %i[earnings over_hard_max] => %w[ecstat1],
         %i[tshortfall no_outstanding_charges] => %w[tshortfall hbrentshortfall],
         %i[beds outside_the_range] => %w[beds],
diff --git a/spec/services/imports/lettings_logs_import_service_spec.rb b/spec/services/imports/lettings_logs_import_service_spec.rb
index 7dc3d578d..8367c7d71 100644
--- a/spec/services/imports/lettings_logs_import_service_spec.rb
+++ b/spec/services/imports/lettings_logs_import_service_spec.rb
@@ -545,6 +545,27 @@ RSpec.describe Imports::LettingsLogsImportService do
         end
       end
 
+      context "and the number of times the property was relet is 0.01" do
+        before do
+          lettings_log_xml.at_xpath("//xmlns:Q20").content = "0.01"
+        end
+
+        it "intercepts the relevant validation error" do
+          expect { lettings_log_service.send(:create_log, lettings_log_xml) }
+            .not_to raise_error
+        end
+
+        it "clears out the number offered answer" do
+          allow(logger).to receive(:warn)
+
+          lettings_log_service.send(:create_log, lettings_log_xml)
+          lettings_log = LettingsLog.find_by(old_id: lettings_log_id)
+
+          expect(lettings_log).not_to be_nil
+          expect(lettings_log.offered).to be_nil
+        end
+      end
+
       context "and the gender identity is refused" do
         before do
           lettings_log_xml.at_xpath("//xmlns:P1Sex").content = "Person prefers not to say"

From 41228e22958dcb2ab5585398409a7d3726553642 Mon Sep 17 00:00:00 2001
From: natdeanlewissoftwire
 <94526761+natdeanlewissoftwire@users.noreply.github.com>
Date: Fri, 8 Sep 2023 16:02:03 +0100
Subject: [PATCH 05/18] Fix hundredth error on migration of very small scharge
 values (#1898)

* feat: round all decimal values to 2dp in safe_string_as_decimal

* refactor: lint
---
 app/services/imports/logs_import_service.rb   |  2 +-
 .../lettings_logs_import_service_spec.rb      | 23 +++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/app/services/imports/logs_import_service.rb b/app/services/imports/logs_import_service.rb
index 930956f16..a82208bbe 100644
--- a/app/services/imports/logs_import_service.rb
+++ b/app/services/imports/logs_import_service.rb
@@ -72,7 +72,7 @@ module Imports
       if str.nil?
         nil
       else
-        BigDecimal(str, exception: false)
+        BigDecimal(str, exception: false).round(2)
       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 8367c7d71..6bfe53571 100644
--- a/spec/services/imports/lettings_logs_import_service_spec.rb
+++ b/spec/services/imports/lettings_logs_import_service_spec.rb
@@ -796,6 +796,29 @@ RSpec.describe Imports::LettingsLogsImportService do
         end
       end
 
+      context "and scharge is ever so slightly positive" do
+        let(:lettings_log_id) { "0b4a68df-30cc-474a-93c0-a56ce8fdad3b" }
+
+        before do
+          lettings_log_xml.at_xpath("//xmlns:Q18aii").content = "1.66533E-16"
+        end
+
+        it "does not raise an error" do
+          expect { lettings_log_service.send(:create_log, lettings_log_xml) }
+            .not_to raise_error
+        end
+
+        it "sets scharge to 0" do
+          allow(logger).to receive(:warn)
+
+          lettings_log_service.send(:create_log, lettings_log_xml)
+          lettings_log = LettingsLog.find_by(old_id: lettings_log_id)
+
+          expect(lettings_log).not_to be_nil
+          expect(lettings_log.scharge).to eq(0)
+        end
+      end
+
       context "and tshortfall is not positive" do
         let(:lettings_log_id) { "0b4a68df-30cc-474a-93c0-a56ce8fdad3b" }
 

From 19ad92c21d28cf03ca8255d00d4103aae0f047f2 Mon Sep 17 00:00:00 2001
From: natdeanlewissoftwire
 <94526761+natdeanlewissoftwire@users.noreply.github.com>
Date: Fri, 8 Sep 2023 16:26:45 +0100
Subject: [PATCH 06/18] feat: don't raise "no matching location" error (#1900)

---
 .../imports/lettings_logs_import_service.rb   |  3 ---
 .../lettings_logs_import_service_spec.rb      | 19 +++++++++++++++++++
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/app/services/imports/lettings_logs_import_service.rb b/app/services/imports/lettings_logs_import_service.rb
index cbaeee01a..49f1e4603 100644
--- a/app/services/imports/lettings_logs_import_service.rb
+++ b/app/services/imports/lettings_logs_import_service.rb
@@ -217,9 +217,6 @@ module Imports
 
         schemes = Scheme.where(old_visible_id: scheme_old_visible_id, owning_organisation_id: attributes["owning_organisation_id"])
         location = Location.find_by(old_visible_id: location_old_visible_id, scheme: schemes)
-        if location.nil? && [location_old_visible_id, scheme_old_visible_id].all?(&:present?) && previous_status != "saved"
-          raise "No matching location for scheme #{scheme_old_visible_id} and location #{location_old_visible_id} (visible IDs)"
-        end
 
         if location.present?
           # Set the scheme via location, because the scheme old visible ID can be duplicated at import
diff --git a/spec/services/imports/lettings_logs_import_service_spec.rb b/spec/services/imports/lettings_logs_import_service_spec.rb
index 6bfe53571..c1eac29bb 100644
--- a/spec/services/imports/lettings_logs_import_service_spec.rb
+++ b/spec/services/imports/lettings_logs_import_service_spec.rb
@@ -1248,6 +1248,25 @@ RSpec.describe Imports::LettingsLogsImportService do
         end
       end
 
+      context "and the scheme and location are not valid" do
+        let(:lettings_log_id) { "0b4a68df-30cc-474a-93c0-a56ce8fdad3b" }
+
+        before do
+          lettings_log_xml.at_xpath("//xmlns:_1cmangroupcode").content = "999"
+          lettings_log_xml.at_xpath("//xmlns:_1cschemecode").content = "999"
+        end
+
+        it "saves log without location and scheme" do
+          expect(logger).not_to receive(:warn)
+          lettings_log_service.send(:create_log, lettings_log_xml)
+          lettings_log = LettingsLog.find_by(old_id: lettings_log_id)
+
+          expect(lettings_log.scheme_id).to be_nil
+          expect(lettings_log.location_id).to be_nil
+          expect(lettings_log.status).to eq("in_progress")
+        end
+      end
+
       context "and this is a supported housing log with a single location under a scheme" do
         let(:lettings_log_id) { "0b4a68df-30cc-474a-93c0-a56ce8fdad3b" }
 

From 11ca1bbcff7db9670a7b05d8239f610e7497f96b Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Tue, 12 Sep 2023 08:21:04 +0100
Subject: [PATCH 07/18] CLDC-2724 Add missing data reporting for imported
 lettings logs (#1892)

* Add generate_missing_answers_report task and method

* Create example report

* change task description

* refactor

* Report old form id instead of old id
---
 app/services/imports/import_report_service.rb | 47 +++++++++++++++++++
 lib/tasks/full_import.rake                    |  9 ++++
 ...lettings_logs_missing_answers_examples.csv | 16 +++++++
 ...d_lettings_logs_missing_answers_report.csv |  4 ++
 spec/lib/tasks/full_import_spec.rb            | 26 ++++++++++
 .../imports/import_report_service_spec.rb     | 38 +++++++++++++++
 6 files changed, 140 insertions(+)
 create mode 100644 spec/fixtures/files/imported_lettings_logs_missing_answers_examples.csv
 create mode 100644 spec/fixtures/files/imported_lettings_logs_missing_answers_report.csv

diff --git a/app/services/imports/import_report_service.rb b/app/services/imports/import_report_service.rb
index 4e387c4c4..7dcde699c 100644
--- a/app/services/imports/import_report_service.rb
+++ b/app/services/imports/import_report_service.rb
@@ -84,5 +84,52 @@ module Imports
 
       @logger.info("Unassigned logs report available in s3 import bucket at #{report_name}")
     end
+
+    def generate_missing_answers_report(report_suffix)
+      Rails.logger.info("Generating missing imported lettings logs answers report")
+      unanswered_question_counts = {}
+      missing_answers_example_sets = {}
+
+      LettingsLog.where.not(old_id: nil).where(status: "in_progress").each do |lettings_log|
+        applicable_questions = lettings_log.form.subsections.map { |s| s.applicable_questions(lettings_log).select { |q| q.enabled?(lettings_log) } }.flatten
+        unanswered_questions = (applicable_questions.filter { |q| q.unanswered?(lettings_log) }.map(&:id) - lettings_log.optional_fields).join(", ")
+
+        if unanswered_question_counts[unanswered_questions].present?
+          unanswered_question_counts[unanswered_questions] += 1
+          missing_answers_example_sets[unanswered_questions] << { id: lettings_log.id, old_form_id: lettings_log.old_form_id, owning_organisation_id: lettings_log.owning_organisation_id } unless unanswered_question_counts[unanswered_questions] > 10
+        else
+          unanswered_question_counts[unanswered_questions] = 1
+          missing_answers_example_sets[unanswered_questions] = [{ id: lettings_log.id, old_form_id: lettings_log.old_form_id, owning_organisation_id: lettings_log.owning_organisation_id }]
+        end
+      end
+
+      rep = CSV.generate do |report|
+        headers = ["Missing answers", "Total number of affected logs"]
+        report << headers
+
+        unanswered_question_counts.each do |missing_answers, count|
+          report << [missing_answers, count]
+        end
+      end
+
+      missing_answers_examples = CSV.generate do |report|
+        headers = ["Missing answers", "Organisation ID", "Log ID", "Old Form ID"]
+        report << headers
+
+        missing_answers_example_sets.each do |missing_answers, examples|
+          examples.each do |example|
+            report << [missing_answers, example[:owning_organisation_id], example[:id], example[:old_form_id]]
+          end
+        end
+      end
+
+      report_name = "MissingAnswersReport_#{report_suffix}.csv"
+      @storage_service.write_file(report_name, BYTE_ORDER_MARK + rep)
+
+      examples_report_name = "MissingAnswersExamples_#{report_suffix}.csv"
+      @storage_service.write_file(examples_report_name, BYTE_ORDER_MARK + missing_answers_examples)
+
+      @logger.info("Missing answers report available in s3 import bucket at #{report_name}")
+    end
   end
 end
diff --git a/lib/tasks/full_import.rake b/lib/tasks/full_import.rake
index 3b74b458a..4c5fcf380 100644
--- a/lib/tasks/full_import.rake
+++ b/lib/tasks/full_import.rake
@@ -117,6 +117,15 @@ namespace :import do
     Imports::ImportReportService.new(s3_service, institutions_csv).create_reports(institutions_csv_name)
   end
 
+  desc "Generate missing answers report"
+  task :generate_missing_answers_report, %i[file_suffix] => :environment do |_task, args|
+    file_suffix = args[:file_suffix]
+    raise "Usage: rake import:generate_reports['file_suffix']" if file_suffix.blank?
+
+    s3_service = Storage::S3Service.new(PlatformHelper.is_paas? ? Configuration::PaasConfigurationService.new : Configuration::EnvConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
+    Imports::ImportReportService.new(s3_service, nil).generate_missing_answers_report(file_suffix)
+  end
+
   desc "Run import from logs step to end"
   task :logs_onwards, %i[institutions_csv_name] => %i[environment logs trigger_invites generate_reports]
 
diff --git a/spec/fixtures/files/imported_lettings_logs_missing_answers_examples.csv b/spec/fixtures/files/imported_lettings_logs_missing_answers_examples.csv
new file mode 100644
index 000000000..0c35bae0b
--- /dev/null
+++ b/spec/fixtures/files/imported_lettings_logs_missing_answers_examples.csv
@@ -0,0 +1,16 @@
+Missing answers,Organisation ID,Log ID,Old Form ID
+age1_known,{org_id0},{id0},1000
+age1_known,{org_id1},{id1},1001
+age1_known,{org_id2},{id2},1002
+age1_known,{org_id3},{id3},1003
+age1_known,{org_id4},{id4},1004
+age1_known,{org_id5},{id5},1005
+age1_known,{org_id6},{id6},1006
+age1_known,{org_id7},{id7},1007
+age1_known,{org_id8},{id8},1008
+age1_known,{org_id9},{id9},1009
+beds,{org_id2_0},{id2_0},2000
+beds,{org_id2_1},{id2_1},2001
+beds,{org_id2_2},{id2_2},2002
+beds,{org_id2_3},{id2_3},2003
+"beds, age1_known",{org_id},{id},300
diff --git a/spec/fixtures/files/imported_lettings_logs_missing_answers_report.csv b/spec/fixtures/files/imported_lettings_logs_missing_answers_report.csv
new file mode 100644
index 000000000..936dc82e0
--- /dev/null
+++ b/spec/fixtures/files/imported_lettings_logs_missing_answers_report.csv
@@ -0,0 +1,4 @@
+Missing answers,Total number of affected logs
+age1_known,11
+beds,4
+"beds, age1_known",1
diff --git a/spec/lib/tasks/full_import_spec.rb b/spec/lib/tasks/full_import_spec.rb
index b95b370cd..2bb85e9b6 100644
--- a/spec/lib/tasks/full_import_spec.rb
+++ b/spec/lib/tasks/full_import_spec.rb
@@ -54,6 +54,32 @@ describe "full import", type: :task do
     end
   end
 
+  describe "import:generate_missing_answers_report" do
+    subject(:task) { Rake::Task["import:generate_missing_answers_report"] }
+
+    before do
+      Rake.application.rake_require("tasks/full_import")
+      Rake::Task.define_task(:environment)
+      task.reenable
+    end
+
+    context "when generating a missing answers report" do
+      let(:import_report_service) { instance_double(Imports::ImportReportService) }
+
+      before do
+        allow(Imports::ImportReportService).to receive(:new).and_return(import_report_service)
+        allow(ENV).to receive(:[]).with("VCAP_SERVICES").and_return("dummy")
+      end
+
+      it "creates a missing answers report" do
+        expect(Storage::S3Service).to receive(:new).with(paas_config_service, instance_name)
+        expect(Imports::ImportReportService).to receive(:new).with(storage_service, nil)
+        expect(import_report_service).to receive(:generate_missing_answers_report).with("some_name")
+        task.invoke("some_name")
+      end
+    end
+  end
+
   describe "import:initial" do
     subject(:task) { Rake::Task["import:initial"] }
 
diff --git a/spec/services/imports/import_report_service_spec.rb b/spec/services/imports/import_report_service_spec.rb
index abd318a8d..ca6e81bc2 100644
--- a/spec/services/imports/import_report_service_spec.rb
+++ b/spec/services/imports/import_report_service_spec.rb
@@ -98,4 +98,42 @@ RSpec.describe Imports::ImportReportService do
       end
     end
   end
+
+  describe "#generate_missing_answers_report" do
+    context "when there are in progress imported logs" do
+      let(:institutions_csv) { nil }
+      let(:expected_content) { File.read("spec/fixtures/files/imported_lettings_logs_missing_answers_report.csv") }
+      let(:expected__answers_examples_content) { File.read("spec/fixtures/files/imported_lettings_logs_missing_answers_examples.csv") }
+
+      before do
+        create_list(:lettings_log, 11, :completed, age1_known: nil) do |log, i|
+          log.old_form_id = "100#{i}"
+          log.old_id = "old_id_age1_known_#{i}"
+          log.save!
+          expected__answers_examples_content.sub!("{id#{i}}", log.id.to_s)
+          expected__answers_examples_content.sub!("{org_id#{i}}", log.owning_organisation_id.to_s)
+        end
+        create_list(:lettings_log, 4, :completed, beds: nil) do |log, i|
+          log.old_form_id = "200#{i}"
+          log.old_id = "old_id_beds_#{i}"
+          expected__answers_examples_content.sub!("{id2_#{i}}", log.id.to_s)
+          expected__answers_examples_content.sub!("{org_id2_#{i}}", log.owning_organisation_id.to_s)
+          log.save!
+        end
+        create(:lettings_log, :completed, age1_known: nil, beds: nil, old_form_id: "300", old_id: "123") do |log|
+          expected__answers_examples_content.sub!("{id}", log.id.to_s)
+          expected__answers_examples_content.sub!("{org_id}", log.owning_organisation_id.to_s)
+        end
+
+        create_list(:lettings_log, 2, :completed, age1_known: nil)
+      end
+
+      it "generates a csv with expected missing fields" do
+        expect(storage_service).to receive(:write_file).with("MissingAnswersReport_report_suffix.csv", "#{expected_content}")
+        expect(storage_service).to receive(:write_file).with("MissingAnswersExamples_report_suffix.csv", "#{expected__answers_examples_content}")
+
+        report_service.generate_missing_answers_report("report_suffix")
+      end
+    end
+  end
 end

From f59c67b651894e336921fed135991ef8788eb5d2 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Tue, 12 Sep 2023 08:42:40 +0100
Subject: [PATCH 08/18] Add reimport reason update_field case (#1896)

---
 .../lettings_logs_field_import_service.rb     | 27 +++++++
 lib/tasks/data_import_field.rake              |  2 +-
 spec/lib/tasks/data_import_field_spec.rb      | 12 ++++
 ...lettings_logs_field_import_service_spec.rb | 70 +++++++++++++++++++
 4 files changed, 110 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 38111a214..68c3ba5f4 100644
--- a/app/services/imports/lettings_logs_field_import_service.rb
+++ b/app/services/imports/lettings_logs_field_import_service.rb
@@ -14,6 +14,8 @@ module Imports
         import_from(folder, :update_creation_method)
       when "address"
         import_from(folder, :update_address)
+      when "reason"
+        import_from(folder, :update_reason)
       else
         raise "Updating #{field} is not supported by the field import service"
       end
@@ -176,5 +178,30 @@ module Imports
         1
       end
     end
+
+    def update_reason(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:)
+
+      if record.present?
+        if record.reason.present?
+          @logger.info("lettings log #{record.id} has a value for reason, skipping update")
+        else
+          reason = unsafe_string_as_integer(xml_doc, "Q9a")
+          reasonother = string_or_nil(xml_doc, "Q9aa")
+          if reason == 20 && reasonother.blank?
+            @logger.info("lettings log #{record.id}'s reason is other but other reason is not provided, skipping update")
+          else
+            record.update!(reason:, reasonother:)
+            @logger.info("lettings log #{record.id}'s reason value has been set to #{reason}")
+            @logger.info("lettings log #{record.id}'s reasonother value has been set to #{reasonother}") if record.reasonother.present?
+          end
+        end
+      else
+        @logger.warn("lettings log with old id #{old_id} not found")
+      end
+    end
   end
 end
diff --git a/lib/tasks/data_import_field.rake b/lib/tasks/data_import_field.rake
index 7eb253277..c52d69d99 100644
--- a/lib/tasks/data_import_field.rake
+++ b/lib/tasks/data_import_field.rake
@@ -7,7 +7,7 @@ namespace :core do
 
     # We only allow a reduced list of known fields to be updatable
     case field
-    when "tenancycode", "major_repairs", "lettings_allocation", "offered", "address"
+    when "tenancycode", "major_repairs", "lettings_allocation", "offered", "address", "reason"
       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/lib/tasks/data_import_field_spec.rb b/spec/lib/tasks/data_import_field_spec.rb
index 4b5f6085f..18eb656bb 100644
--- a/spec/lib/tasks/data_import_field_spec.rb
+++ b/spec/lib/tasks/data_import_field_spec.rb
@@ -104,6 +104,18 @@ describe "data_import_field imports" do
         end
       end
 
+      context "and we update the reason field" do
+        let(:field) { "reason" }
+
+        it "updates the 2023 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 "raises an exception if no parameters are provided" do
         expect { task.invoke }.to raise_error(/Usage/)
       end
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 d6222d355..4bed2dc63 100644
--- a/spec/services/imports/lettings_logs_field_import_service_spec.rb
+++ b/spec/services/imports/lettings_logs_field_import_service_spec.rb
@@ -471,4 +471,74 @@ RSpec.describe Imports::LettingsLogsFieldImportService do
       end
     end
   end
+
+  context "when updating reason" do
+    let(:field) { "reason" }
+
+    context "when the lettings log has no reason 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!(reason: nil)
+        lettings_log_xml.at_xpath("//xmlns:Q9a").content = "47"
+      end
+
+      it "updates the lettings_log reason value" do
+        expect(logger).to receive(:info).with(/lettings log \d+'s reason value has been set to 47/)
+        expect { import_service.send(:update_reason, lettings_log_xml) }
+          .to(change { lettings_log.reload.reason }.from(nil).to(47))
+      end
+    end
+
+    context "when the lettings log has a different reason 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!(reason: 18)
+        lettings_log_xml.at_xpath("//xmlns:Q9a").content = "47"
+      end
+
+      it "does not update the lettings_log reason value" do
+        expect(logger).to receive(:info).with(/lettings log \d+ has a value for reason, skipping update/)
+        expect { import_service.send(:update_reason, lettings_log_xml) }
+          .not_to(change { lettings_log.reload.reason })
+      end
+    end
+
+    context "when the new value is 'other'" 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!(reason: nil)
+        lettings_log_xml.at_xpath("//xmlns:Q9a").content = "20"
+      end
+
+      context "and other value is given" do
+        before do
+          lettings_log_xml.at_xpath("//xmlns:Q9aa").content = "other"
+        end
+
+        it "updates the lettings_log reason value" do
+          expect(logger).to receive(:info).with(/lettings log \d+'s reason value has been set to 20/)
+          expect(logger).to receive(:info).with(/lettings log \d+'s reasonother value has been set to other/)
+          expect { import_service.send(:update_reason, lettings_log_xml) }
+            .to(change { lettings_log.reload.reason }.from(nil).to(20))
+        end
+      end
+
+      context "and other value is not given" do
+        it "does not update the lettings_log reason value" do
+          expect(logger).to receive(:info).with(/lettings log \d+'s reason is other but other reason is not provided, skipping update/)
+          expect { import_service.send(:update_reason, lettings_log_xml) }
+            .not_to(change { lettings_log.reload.reason })
+        end
+      end
+    end
+  end
 end

From 8726d231bd7334c6a2cd001c11c50e2607786133 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Tue, 12 Sep 2023 09:41:01 +0100
Subject: [PATCH 09/18] Remove homelessness validation from the import (#1901)

---
 app/services/imports/lettings_logs_import_service.rb | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/app/services/imports/lettings_logs_import_service.rb b/app/services/imports/lettings_logs_import_service.rb
index 49f1e4603..f400612fe 100644
--- a/app/services/imports/lettings_logs_import_service.rb
+++ b/app/services/imports/lettings_logs_import_service.rb
@@ -142,12 +142,6 @@ module Imports
       attributes["rp_hardship"] = unsafe_string_as_integer(xml_doc, "Q14b4").present? ? 1 : nil
       attributes["rp_dontknow"] = unsafe_string_as_integer(xml_doc, "Q14b5").present? ? 1 : nil
 
-      # Trips validation
-      if attributes["homeless"] == 1 && attributes["rp_homeless"] == 1
-        attributes["homeless"] = nil
-        attributes["rp_homeless"] = nil
-      end
-
       attributes["cbl"] = allocation_system(unsafe_string_as_integer(xml_doc, "Q15CBL"))
       attributes["chr"] = allocation_system(unsafe_string_as_integer(xml_doc, "Q15CHR"))
       attributes["cap"] = allocation_system(unsafe_string_as_integer(xml_doc, "Q15CAP"))

From bda7bc7d08eb46158b14428ea097d58dd32f81b8 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Tue, 12 Sep 2023 10:48:27 +0100
Subject: [PATCH 10/18] Only blank tenancy length when it is invalid for
 tenancy type (#1902)

* Only blank tenancy lenght when it is invalid for tenancy type

* Update spec/services/imports/lettings_logs_import_service_spec.rb

Co-authored-by: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com>

---------

Co-authored-by: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com>
---
 app/services/imports/lettings_logs_import_service.rb       | 2 +-
 spec/services/imports/lettings_logs_import_service_spec.rb | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/services/imports/lettings_logs_import_service.rb b/app/services/imports/lettings_logs_import_service.rb
index f400612fe..ca3cde434 100644
--- a/app/services/imports/lettings_logs_import_service.rb
+++ b/app/services/imports/lettings_logs_import_service.rb
@@ -312,7 +312,7 @@ module Imports
         %i[chcharge out_of_range] => %w[chcharge],
         %i[referral internal_transfer_non_social_housing] => %w[referral],
         %i[referral internal_transfer_fixed_or_lifetime] => %w[referral],
-        %i[tenancylength tenancylength_invalid] => %w[tenancylength tenancy],
+        %i[tenancylength tenancylength_invalid] => %w[tenancylength],
         %i[prevten over_25_foster_care] => %w[prevten age1],
         %i[prevten non_temp_accommodation] => %w[prevten rsnvac],
         %i[joint not_joint_tenancy] => %w[joint],
diff --git a/spec/services/imports/lettings_logs_import_service_spec.rb b/spec/services/imports/lettings_logs_import_service_spec.rb
index c1eac29bb..800955cfc 100644
--- a/spec/services/imports/lettings_logs_import_service_spec.rb
+++ b/spec/services/imports/lettings_logs_import_service_spec.rb
@@ -325,7 +325,7 @@ RSpec.describe Imports::LettingsLogsImportService do
             .not_to raise_error
         end
 
-        it "clears out the invalid answers" do
+        it "clears out the invalid tenancy length" do
           allow(logger).to receive(:warn)
 
           lettings_log_service.send(:create_log, lettings_log_xml)
@@ -333,7 +333,7 @@ RSpec.describe Imports::LettingsLogsImportService do
 
           expect(lettings_log).not_to be_nil
           expect(lettings_log.tenancylength).to be_nil
-          expect(lettings_log.tenancy).to be_nil
+          expect(lettings_log.tenancy).to eq(4)
         end
       end
 

From c049089684af25425730d7843c7cf0c14fe588e2 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Tue, 12 Sep 2023 10:49:41 +0100
Subject: [PATCH 11/18] CLDC-2725 Add missing sales data report (#1893)

* Add missing sales answers reporting

* Fix log name
---
 app/services/imports/import_report_service.rb | 49 ++++++++++-----
 .../imports/import_report_service_spec.rb     | 60 +++++++++++++++----
 2 files changed, 83 insertions(+), 26 deletions(-)

diff --git a/app/services/imports/import_report_service.rb b/app/services/imports/import_report_service.rb
index 7dcde699c..ffcd78fc5 100644
--- a/app/services/imports/import_report_service.rb
+++ b/app/services/imports/import_report_service.rb
@@ -86,24 +86,47 @@ module Imports
     end
 
     def generate_missing_answers_report(report_suffix)
-      Rails.logger.info("Generating missing imported lettings logs answers report")
+      create_missing_answers_report(report_suffix, LettingsLog)
+      create_missing_answers_report(report_suffix, SalesLog)
+    end
+
+    def create_missing_answers_report(report_suffix, log_class)
+      class_name = log_class.name.underscore.humanize
+      Rails.logger.info("Generating missing imported #{class_name}s answers report")
+
+      unanswered_question_counts, missing_answers_example_sets = process_missing_answers(log_class)
+
+      report_name = "MissingAnswersReport#{log_class.name}_#{report_suffix}.csv"
+      @storage_service.write_file(report_name, BYTE_ORDER_MARK + missing_answers_report(unanswered_question_counts))
+
+      examples_report_name = "MissingAnswersExamples#{log_class.name}_#{report_suffix}.csv"
+      @storage_service.write_file(examples_report_name, BYTE_ORDER_MARK + missing_answers_examples(missing_answers_example_sets))
+
+      @logger.info("Missing #{class_name}s answers report available in s3 import bucket at #{report_name}")
+    end
+
+    def process_missing_answers(log_class)
       unanswered_question_counts = {}
       missing_answers_example_sets = {}
 
-      LettingsLog.where.not(old_id: nil).where(status: "in_progress").each do |lettings_log|
-        applicable_questions = lettings_log.form.subsections.map { |s| s.applicable_questions(lettings_log).select { |q| q.enabled?(lettings_log) } }.flatten
-        unanswered_questions = (applicable_questions.filter { |q| q.unanswered?(lettings_log) }.map(&:id) - lettings_log.optional_fields).join(", ")
+      log_class.where.not(old_id: nil).where(status: "in_progress").each do |log|
+        applicable_questions = log.form.subsections.map { |s| s.applicable_questions(log).select { |q| q.enabled?(log) } }.flatten
+        unanswered_questions = (applicable_questions.filter { |q| q.unanswered?(log) }.map(&:id) - log.optional_fields).join(", ")
 
         if unanswered_question_counts[unanswered_questions].present?
           unanswered_question_counts[unanswered_questions] += 1
-          missing_answers_example_sets[unanswered_questions] << { id: lettings_log.id, old_form_id: lettings_log.old_form_id, owning_organisation_id: lettings_log.owning_organisation_id } unless unanswered_question_counts[unanswered_questions] > 10
+          missing_answers_example_sets[unanswered_questions] << { id: log.id, old_form_id: log.old_form_id, owning_organisation_id: log.owning_organisation_id } unless unanswered_question_counts[unanswered_questions] > 10
         else
           unanswered_question_counts[unanswered_questions] = 1
-          missing_answers_example_sets[unanswered_questions] = [{ id: lettings_log.id, old_form_id: lettings_log.old_form_id, owning_organisation_id: lettings_log.owning_organisation_id }]
+          missing_answers_example_sets[unanswered_questions] = [{ id: log.id, old_form_id: log.old_form_id, owning_organisation_id: log.owning_organisation_id }]
         end
       end
 
-      rep = CSV.generate do |report|
+      [unanswered_question_counts, missing_answers_example_sets]
+    end
+
+    def missing_answers_report(unanswered_question_counts)
+      CSV.generate do |report|
         headers = ["Missing answers", "Total number of affected logs"]
         report << headers
 
@@ -111,8 +134,10 @@ module Imports
           report << [missing_answers, count]
         end
       end
+    end
 
-      missing_answers_examples = CSV.generate do |report|
+    def missing_answers_examples(missing_answers_example_sets)
+      CSV.generate do |report|
         headers = ["Missing answers", "Organisation ID", "Log ID", "Old Form ID"]
         report << headers
 
@@ -122,14 +147,6 @@ module Imports
           end
         end
       end
-
-      report_name = "MissingAnswersReport_#{report_suffix}.csv"
-      @storage_service.write_file(report_name, BYTE_ORDER_MARK + rep)
-
-      examples_report_name = "MissingAnswersExamples_#{report_suffix}.csv"
-      @storage_service.write_file(examples_report_name, BYTE_ORDER_MARK + missing_answers_examples)
-
-      @logger.info("Missing answers report available in s3 import bucket at #{report_name}")
     end
   end
 end
diff --git a/spec/services/imports/import_report_service_spec.rb b/spec/services/imports/import_report_service_spec.rb
index ca6e81bc2..938ab9f1b 100644
--- a/spec/services/imports/import_report_service_spec.rb
+++ b/spec/services/imports/import_report_service_spec.rb
@@ -100,37 +100,77 @@ RSpec.describe Imports::ImportReportService do
   end
 
   describe "#generate_missing_answers_report" do
-    context "when there are in progress imported logs" do
+    context "when there are in progress imported lettings logs" do
       let(:institutions_csv) { nil }
       let(:expected_content) { File.read("spec/fixtures/files/imported_lettings_logs_missing_answers_report.csv") }
-      let(:expected__answers_examples_content) { File.read("spec/fixtures/files/imported_lettings_logs_missing_answers_examples.csv") }
+      let(:expected_answers_examples_content) { File.read("spec/fixtures/files/imported_lettings_logs_missing_answers_examples.csv") }
 
       before do
         create_list(:lettings_log, 11, :completed, age1_known: nil) do |log, i|
           log.old_form_id = "100#{i}"
           log.old_id = "old_id_age1_known_#{i}"
           log.save!
-          expected__answers_examples_content.sub!("{id#{i}}", log.id.to_s)
-          expected__answers_examples_content.sub!("{org_id#{i}}", log.owning_organisation_id.to_s)
+          expected_answers_examples_content.sub!("{id#{i}}", log.id.to_s)
+          expected_answers_examples_content.sub!("{org_id#{i}}", log.owning_organisation_id.to_s)
         end
         create_list(:lettings_log, 4, :completed, beds: nil) do |log, i|
           log.old_form_id = "200#{i}"
           log.old_id = "old_id_beds_#{i}"
-          expected__answers_examples_content.sub!("{id2_#{i}}", log.id.to_s)
-          expected__answers_examples_content.sub!("{org_id2_#{i}}", log.owning_organisation_id.to_s)
+          expected_answers_examples_content.sub!("{id2_#{i}}", log.id.to_s)
+          expected_answers_examples_content.sub!("{org_id2_#{i}}", log.owning_organisation_id.to_s)
           log.save!
         end
         create(:lettings_log, :completed, age1_known: nil, beds: nil, old_form_id: "300", old_id: "123") do |log|
-          expected__answers_examples_content.sub!("{id}", log.id.to_s)
-          expected__answers_examples_content.sub!("{org_id}", log.owning_organisation_id.to_s)
+          expected_answers_examples_content.sub!("{id}", log.id.to_s)
+          expected_answers_examples_content.sub!("{org_id}", log.owning_organisation_id.to_s)
         end
 
         create_list(:lettings_log, 2, :completed, age1_known: nil)
       end
 
       it "generates a csv with expected missing fields" do
-        expect(storage_service).to receive(:write_file).with("MissingAnswersReport_report_suffix.csv", "#{expected_content}")
-        expect(storage_service).to receive(:write_file).with("MissingAnswersExamples_report_suffix.csv", "#{expected__answers_examples_content}")
+        expect(storage_service).to receive(:write_file).with("MissingAnswersReportLettingsLog_report_suffix.csv", "#{expected_content}")
+        expect(storage_service).to receive(:write_file).with("MissingAnswersExamplesLettingsLog_report_suffix.csv", "#{expected_answers_examples_content}")
+        expect(storage_service).to receive(:write_file).with("MissingAnswersReportSalesLog_report_suffix.csv", "\uFEFFMissing answers,Total number of affected logs\n")
+        expect(storage_service).to receive(:write_file).with("MissingAnswersExamplesSalesLog_report_suffix.csv", "\uFEFFMissing answers,Organisation ID,Log ID,Old Form ID\n")
+
+        report_service.generate_missing_answers_report("report_suffix")
+      end
+    end
+
+    context "when there are in progress imported sales logs" do
+      let(:institutions_csv) { nil }
+      let(:expected_content) { File.read("spec/fixtures/files/imported_lettings_logs_missing_answers_report.csv") }
+      let(:expected_answers_examples_content) { File.read("spec/fixtures/files/imported_lettings_logs_missing_answers_examples.csv") }
+
+      before do
+        create_list(:sales_log, 11, :completed, age1_known: nil) do |log, i|
+          log.old_id = "age1_known_#{i}"
+          log.old_form_id = "100#{i}"
+          log.save!
+          expected_answers_examples_content.sub!("{id#{i}}", log.id.to_s)
+          expected_answers_examples_content.sub!("{org_id#{i}}", log.owning_organisation_id.to_s)
+        end
+        create_list(:sales_log, 4, :completed, beds: nil) do |log, i|
+          log.old_id = "beds_#{i}"
+          log.old_form_id = "200#{i}"
+          expected_answers_examples_content.sub!("{id2_#{i}}", log.id.to_s)
+          expected_answers_examples_content.sub!("{org_id2_#{i}}", log.owning_organisation_id.to_s)
+          log.save!
+        end
+        create(:sales_log, :completed, age1_known: nil, beds: nil, old_id: "beds_and_age", old_form_id: "300") do |log|
+          expected_answers_examples_content.sub!("{id}", log.id.to_s)
+          expected_answers_examples_content.sub!("{org_id}", log.owning_organisation_id.to_s)
+        end
+
+        create_list(:sales_log, 2, :completed, age1_known: nil)
+      end
+
+      it "generates a csv with expected missing fields" do
+        expect(storage_service).to receive(:write_file).with("MissingAnswersReportLettingsLog_report_suffix.csv", "\uFEFFMissing answers,Total number of affected logs\n")
+        expect(storage_service).to receive(:write_file).with("MissingAnswersExamplesLettingsLog_report_suffix.csv", "\uFEFFMissing answers,Organisation ID,Log ID,Old Form ID\n")
+        expect(storage_service).to receive(:write_file).with("MissingAnswersReportSalesLog_report_suffix.csv", "#{expected_content}")
+        expect(storage_service).to receive(:write_file).with("MissingAnswersExamplesSalesLog_report_suffix.csv", "#{expected_answers_examples_content}")
 
         report_service.generate_missing_answers_report("report_suffix")
       end

From 961aefa0956f7907430ada8f63e523b91b3f6c3c Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Tue, 12 Sep 2023 10:54:11 +0100
Subject: [PATCH 12/18] =?UTF-8?q?Add=20Children=E2=80=99s=20Social=20Care?=
 =?UTF-8?q?=20to=20all=20referral=20questions=20(#1905)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 app/models/form/lettings/questions/referral_prp.rb       | 3 +++
 .../lettings/questions/referral_supported_housing.rb     | 3 +++
 .../lettings/questions/referral_supported_housing_prp.rb | 1 +
 config/forms/2022_2023.json                              | 9 +++++++++
 4 files changed, 16 insertions(+)

diff --git a/app/models/form/lettings/questions/referral_prp.rb b/app/models/form/lettings/questions/referral_prp.rb
index 36dc247b7..69a42bed5 100644
--- a/app/models/form/lettings/questions/referral_prp.rb
+++ b/app/models/form/lettings/questions/referral_prp.rb
@@ -45,6 +45,9 @@ class Form::Lettings::Questions::ReferralPrp < ::Form::Question
     "13" => {
       "value" => "Youth offending team",
     },
+    "17" => {
+      "value" => "Children’s Social Care",
+    },
     "16" => {
       "value" => "Other",
     },
diff --git a/app/models/form/lettings/questions/referral_supported_housing.rb b/app/models/form/lettings/questions/referral_supported_housing.rb
index 5ec9de70e..68b9fd8e7 100644
--- a/app/models/form/lettings/questions/referral_supported_housing.rb
+++ b/app/models/form/lettings/questions/referral_supported_housing.rb
@@ -45,6 +45,9 @@ class Form::Lettings::Questions::ReferralSupportedHousing < ::Form::Question
     "13" => {
       "value" => "Youth offending team",
     },
+    "17" => {
+      "value" => "Children’s Social Care",
+    },
     "16" => {
       "value" => "Other",
     },
diff --git a/app/models/form/lettings/questions/referral_supported_housing_prp.rb b/app/models/form/lettings/questions/referral_supported_housing_prp.rb
index 4824dd6c5..32fb474ed 100644
--- a/app/models/form/lettings/questions/referral_supported_housing_prp.rb
+++ b/app/models/form/lettings/questions/referral_supported_housing_prp.rb
@@ -24,6 +24,7 @@ class Form::Lettings::Questions::ReferralSupportedHousingPrp < ::Form::Question
     "12" => { "value" => "Police, probation or prison" },
     "7" => { "value" => "Voluntary agency" },
     "13" => { "value" => "Youth offending team" },
+    "17" => { "value" => "Children’s Social Care" },
     "16" => { "value" => "Other" },
   }.freeze
 end
diff --git a/config/forms/2022_2023.json b/config/forms/2022_2023.json
index db11b0950..66b975d9c 100644
--- a/config/forms/2022_2023.json
+++ b/config/forms/2022_2023.json
@@ -7081,6 +7081,9 @@
                     "13": {
                       "value": "Youth offending team"
                     },
+                    "17": {
+                      "value": "Children’s Social Care"
+                    },
                     "16": {
                       "value": "Other"
                     }
@@ -7138,6 +7141,9 @@
                     "13": {
                       "value": "Youth offending team"
                     },
+                    "17": {
+                      "value": "Children’s Social Care"
+                    },
                     "16": {
                       "value": "Other"
                     }
@@ -7198,6 +7204,9 @@
                     "13": {
                       "value": "Youth offending team"
                     },
+                    "17": {
+                      "value": "Children’s Social Care"
+                    },
                     "16": {
                       "value": "Other"
                     }

From 564fa9ebb50f627a77a5db97f49c494f71b791ee Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Tue, 12 Sep 2023 11:15:07 +0100
Subject: [PATCH 13/18] Mark hodate value check confirmed (#1906)

* Mark hodate value check confirmed

* Add other missing value_checks for sales
---
 .../imports/sales_logs_import_service.rb      |  6 +++
 .../imports/sales_logs_import_service_spec.rb | 50 +++++++++++++++++++
 2 files changed, 56 insertions(+)

diff --git a/app/services/imports/sales_logs_import_service.rb b/app/services/imports/sales_logs_import_service.rb
index 157e797b4..68d8e95ce 100644
--- a/app/services/imports/sales_logs_import_service.rb
+++ b/app/services/imports/sales_logs_import_service.rb
@@ -152,6 +152,10 @@ module Imports
       attributes["discounted_sale_value_check"] = 0
       attributes["buyer_livein_value_check"] = 0
       attributes["percentage_discount_value_check"] = 0
+      attributes["hodate_check"] = 0
+      attributes["saledate_check"] = 0
+      attributes["combined_income_value_check"] = 0
+      attributes["stairowned_value_check"] = 0
 
       # 2023/34 attributes
       attributes["address_line1"] = string_or_nil(xml_doc, "AddressLine1")
@@ -330,6 +334,8 @@ module Imports
          discounted_sale_value_check
          buyer_livein_value_check
          percentage_discount_value_check
+         combined_income_value_check
+         stairowned_value_check
          uprn_known
          uprn_confirmed]
     end
diff --git a/spec/services/imports/sales_logs_import_service_spec.rb b/spec/services/imports/sales_logs_import_service_spec.rb
index df279e5d5..2357618d0 100644
--- a/spec/services/imports/sales_logs_import_service_spec.rb
+++ b/spec/services/imports/sales_logs_import_service_spec.rb
@@ -595,6 +595,56 @@ RSpec.describe Imports::SalesLogsImportService do
       end
     end
 
+    context "and the hodate soft validation is triggered (hodate_value_check)" do
+      let(:sales_log_id) { "shared_ownership_sales_log" }
+
+      before do
+        sales_log_xml.at_xpath("//xmlns:HODAY").content = "1"
+        sales_log_xml.at_xpath("//xmlns:HOMONTH").content = "1"
+        sales_log_xml.at_xpath("//xmlns:HOYEAR").content = "2018"
+      end
+
+      it "completes the log" do
+        sales_log_service.send(:create_log, sales_log_xml)
+        sales_log = SalesLog.find_by(old_id: sales_log_id)
+        expect(sales_log.status).to eq("completed")
+      end
+    end
+
+    context "and the combined income soft validation is triggered (combined_income_value_check)" do
+      let(:sales_log_id) { "shared_ownership_sales_log" }
+
+      before do
+        sales_log_xml.at_xpath("//xmlns:joint").content = "1 Yes"
+        sales_log_xml.at_xpath("//xmlns:Q2Person1Income").content = "45000"
+        sales_log_xml.at_xpath("//xmlns:P2IncKnown").content = "1 Yes"
+        sales_log_xml.at_xpath("//xmlns:Q2Person2Income").content = "45000"
+      end
+
+      it "completes the log" do
+        sales_log_service.send(:create_log, sales_log_xml)
+        sales_log = SalesLog.find_by(old_id: sales_log_id)
+        expect(sales_log.status).to eq("completed")
+      end
+    end
+
+    context "and the stairowned soft validation is triggered (stairowned_value_check)" do
+      let(:sales_log_id) { "shared_ownership_sales_log" }
+
+      before do
+        sales_log_xml.at_xpath("//xmlns:DerSaleType").content = "24"
+        sales_log_xml.at_xpath("//xmlns:PercentOwns").content = "77"
+        sales_log_xml.at_xpath("//xmlns:Q17aStaircase").content = "1"
+        sales_log_xml.at_xpath("//xmlns:PercentBought").content = "10"
+      end
+
+      it "completes the log" do
+        sales_log_service.send(:create_log, sales_log_xml)
+        sales_log = SalesLog.find_by(old_id: sales_log_id)
+        expect(sales_log.status).to eq("completed")
+      end
+    end
+
     context "and it has an invalid record with invalid child age" do
       let(:sales_log_id) { "discounted_ownership_sales_log" }
 

From 213a4bf2290a6ec2d0cdd587e16d4f6ba816ec6e Mon Sep 17 00:00:00 2001
From: natdeanlewissoftwire
 <94526761+natdeanlewissoftwire@users.noreply.github.com>
Date: Tue, 12 Sep 2023 13:45:01 +0100
Subject: [PATCH 14/18] Blank invalid combined income and proplen values on
 migration (#1907)

* feat: clear invalid combine incomes

* feat: clear out of range proplen values
---
 .../sales/financial_validations.rb            |  4 +-
 .../imports/sales_logs_import_service.rb      |  5 ++
 .../imports/sales_logs_import_service_spec.rb | 81 ++++++++++++++++++-
 3 files changed, 87 insertions(+), 3 deletions(-)

diff --git a/app/models/validations/sales/financial_validations.rb b/app/models/validations/sales/financial_validations.rb
index ffdee561a..82c57e75d 100644
--- a/app/models/validations/sales/financial_validations.rb
+++ b/app/models/validations/sales/financial_validations.rb
@@ -30,9 +30,9 @@ module Validations::Sales::FinancialValidations
     combined_income = record.income1 + record.income2
     relevant_fields = %i[income1 income2 ownershipsch uprn la postcode_full]
     if record.london_property? && combined_income > 90_000
-      relevant_fields.each { |field| record.errors.add field, I18n.t("validations.financial.income.combined_over_hard_max_for_london") }
+      relevant_fields.each { |field| record.errors.add field, :over_combined_hard_max_for_london, message: I18n.t("validations.financial.income.combined_over_hard_max_for_london") }
     elsif record.property_not_in_london? && combined_income > 80_000
-      relevant_fields.each { |field| record.errors.add field, I18n.t("validations.financial.income.combined_over_hard_max_for_outside_london") }
+      relevant_fields.each { |field| record.errors.add field, :over_combined_hard_max_for_outside_london, message: I18n.t("validations.financial.income.combined_over_hard_max_for_outside_london") }
     end
   end
 
diff --git a/app/services/imports/sales_logs_import_service.rb b/app/services/imports/sales_logs_import_service.rb
index 68d8e95ce..1a9fe54a0 100644
--- a/app/services/imports/sales_logs_import_service.rb
+++ b/app/services/imports/sales_logs_import_service.rb
@@ -250,15 +250,20 @@ module Imports
         %i[postcode_full postcodes_not_matching] => %w[ppcodenk ppostcode_full],
         %i[exdate over_a_year_from_saledate] => %w[exdate],
         %i[income1 over_hard_max_for_outside_london] => %w[income1],
+        %i[income1 over_combined_hard_max_for_outside_london] => %w[income1 income2],
         %i[income1 over_hard_max_for_london] => %w[income1],
+        %i[income1 over_combined_hard_max_for_london] => %w[income1 income2],
         %i[income2 over_hard_max_for_outside_london] => %w[income2],
+        %i[income2 over_combined_hard_max_for_outside_london] => %w[income1 income2],
         %i[income2 over_hard_max_for_london] => %w[income2],
+        %i[income2 over_combined_hard_max_for_london] => %w[income1 income2],
         %i[equity over_max] => %w[equity],
         %i[equity under_min] => %w[equity],
         %i[mscharge under_min] => %w[mscharge has_mscharge],
         %i[mortgage cannot_be_0] => %w[mortgage],
         %i[frombeds outside_the_range] => %w[frombeds],
         %i[age1 outside_the_range] => %w[age1 age1_known],
+        %i[proplen outside_the_range] => %w[proplen],
       }
 
       errors.each do |(error, fields)|
diff --git a/spec/services/imports/sales_logs_import_service_spec.rb b/spec/services/imports/sales_logs_import_service_spec.rb
index 2357618d0..e4d5b9473 100644
--- a/spec/services/imports/sales_logs_import_service_spec.rb
+++ b/spec/services/imports/sales_logs_import_service_spec.rb
@@ -900,7 +900,7 @@ RSpec.describe Imports::SalesLogsImportService do
           .not_to raise_error
       end
 
-      it "clears out the referral answer" do
+      it "clears out the mortgage answer" do
         allow(logger).to receive(:warn)
 
         sales_log_service.send(:create_log, sales_log_xml)
@@ -911,6 +911,29 @@ RSpec.describe Imports::SalesLogsImportService do
       end
     end
 
+    context "when proplen is out of range" do
+      let(:sales_log_id) { "shared_ownership_sales_log" }
+
+      before do
+        sales_log_xml.at_xpath("//xmlns:Q16aProplen2").content = "2000"
+      end
+
+      it "intercepts the relevant validation error" do
+        expect { sales_log_service.send(:create_log, sales_log_xml) }
+          .not_to raise_error
+      end
+
+      it "clears out the proplen answer" do
+        allow(logger).to receive(:warn)
+
+        sales_log_service.send(:create_log, sales_log_xml)
+        sales_log = SalesLog.find_by(old_id: sales_log_id)
+
+        expect(sales_log).not_to be_nil
+        expect(sales_log.proplen).to be_nil
+      end
+    end
+
     context "and it has an invalid income 1" do
       let(:sales_log_id) { "shared_ownership_sales_log" }
 
@@ -961,6 +984,34 @@ RSpec.describe Imports::SalesLogsImportService do
       end
     end
 
+    context "and it has an invalid combined income outside london" do
+      let(:sales_log_id) { "shared_ownership_sales_log" }
+
+      before do
+        sales_log_xml.at_xpath("//xmlns:joint").content = "1 Yes"
+        sales_log_xml.at_xpath("//xmlns:Q2Person1Income").content = "45000"
+        sales_log_xml.at_xpath("//xmlns:P2IncKnown").content = "1 Yes"
+        sales_log_xml.at_xpath("//xmlns:Q2Person2Income").content = "40000"
+        sales_log_xml.at_xpath("//xmlns:Q14ONSLACode").content = "E07000223"
+      end
+
+      it "intercepts the relevant validation error" do
+        expect { sales_log_service.send(:create_log, sales_log_xml) }
+          .not_to raise_error
+      end
+
+      it "clears out the invalid answers" do
+        allow(logger).to receive(:warn)
+
+        sales_log_service.send(:create_log, sales_log_xml)
+        sales_log = SalesLog.find_by(old_id: sales_log_id)
+
+        expect(sales_log).not_to be_nil
+        expect(sales_log.income1).to be_nil
+        expect(sales_log.income2).to be_nil
+      end
+    end
+
     context "and it has an invalid income 1 for london" do
       let(:sales_log_id) { "shared_ownership_sales_log" }
 
@@ -1011,6 +1062,34 @@ RSpec.describe Imports::SalesLogsImportService do
       end
     end
 
+    context "and it has an invalid combined income for london" do
+      let(:sales_log_id) { "shared_ownership_sales_log" }
+
+      before do
+        sales_log_xml.at_xpath("//xmlns:joint").content = "1 Yes"
+        sales_log_xml.at_xpath("//xmlns:Q2Person1Income").content = "50000"
+        sales_log_xml.at_xpath("//xmlns:P2IncKnown").content = "1 Yes"
+        sales_log_xml.at_xpath("//xmlns:Q2Person2Income").content = "45000"
+        sales_log_xml.at_xpath("//xmlns:Q14ONSLACode").content = "E09000012"
+      end
+
+      it "intercepts the relevant validation error" do
+        expect { sales_log_service.send(:create_log, sales_log_xml) }
+          .not_to raise_error
+      end
+
+      it "clears out the invalid answers" do
+        allow(logger).to receive(:warn)
+
+        sales_log_service.send(:create_log, sales_log_xml)
+        sales_log = SalesLog.find_by(old_id: sales_log_id)
+
+        expect(sales_log).not_to be_nil
+        expect(sales_log.income1).to be_nil
+        expect(sales_log.income2).to be_nil
+      end
+    end
+
     context "and it has an invalid mscharge" do
       let(:sales_log_id) { "shared_ownership_sales_log" }
 

From c80565a54d7baf7c0d251f85b63319020cf06c56 Mon Sep 17 00:00:00 2001
From: natdeanlewissoftwire
 <94526761+natdeanlewissoftwire@users.noreply.github.com>
Date: Tue, 12 Sep 2023 13:45:20 +0100
Subject: [PATCH 15/18] Make saledate nil safe(r) (#1904)

* feat: allow missing day/month/year values if CompletionDate exists as we don't always receive these from old core

* feat: make safe string as decimal nil safer too

* feat: add test
---
 app/services/imports/logs_import_service.rb   |  2 +-
 .../imports/sales_logs_import_service.rb      |  4 +--
 .../lettings_logs_import_service_spec.rb      | 21 ++++++++++++++++
 .../imports/sales_logs_import_service_spec.rb | 25 +++++++++++++++++++
 4 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/app/services/imports/logs_import_service.rb b/app/services/imports/logs_import_service.rb
index a82208bbe..f7456a3fb 100644
--- a/app/services/imports/logs_import_service.rb
+++ b/app/services/imports/logs_import_service.rb
@@ -72,7 +72,7 @@ module Imports
       if str.nil?
         nil
       else
-        BigDecimal(str, exception: false).round(2)
+        BigDecimal(str, exception: false)&.round(2)
       end
     end
 
diff --git a/app/services/imports/sales_logs_import_service.rb b/app/services/imports/sales_logs_import_service.rb
index 1a9fe54a0..7d10f52ca 100644
--- a/app/services/imports/sales_logs_import_service.rb
+++ b/app/services/imports/sales_logs_import_service.rb
@@ -15,7 +15,7 @@ module Imports
     def create_log(xml_doc)
       # only import sales logs from 22/23 collection period onwards
       return unless meta_field_value(xml_doc, "form-name").include?("Sales")
-      return unless compose_date(xml_doc, "DAY", "MONTH", "YEAR") >= Time.zone.local(2022, 4, 1)
+      return unless (compose_date(xml_doc, "DAY", "MONTH", "YEAR") || Time.zone.parse(field_value(xml_doc, "xmlns", "CompletionDate"))) >= Time.zone.local(2022, 4, 1)
 
       attributes = {}
 
@@ -24,7 +24,7 @@ module Imports
       # Required fields for status complete or logic to work
       # Note: order matters when we derive from previous values (attributes parameter)
 
-      attributes["saledate"] = compose_date(xml_doc, "DAY", "MONTH", "YEAR")
+      attributes["saledate"] = compose_date(xml_doc, "DAY", "MONTH", "YEAR") || Time.zone.parse(field_value(xml_doc, "xmlns", "CompletionDate"))
       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")
diff --git a/spec/services/imports/lettings_logs_import_service_spec.rb b/spec/services/imports/lettings_logs_import_service_spec.rb
index 800955cfc..dad2e68ed 100644
--- a/spec/services/imports/lettings_logs_import_service_spec.rb
+++ b/spec/services/imports/lettings_logs_import_service_spec.rb
@@ -566,6 +566,27 @@ RSpec.describe Imports::LettingsLogsImportService do
         end
       end
 
+      context "and the number of times the property was relet is a non nil string that is nil as a BigDecimal" do
+        before do
+          lettings_log_xml.at_xpath("//xmlns:Q20").content = "a"
+        end
+
+        it "doesn't throw an error" do
+          expect { lettings_log_service.send(:create_log, lettings_log_xml) }
+            .not_to raise_error
+        end
+
+        it "clears out the number offered answer" do
+          allow(logger).to receive(:warn)
+
+          lettings_log_service.send(:create_log, lettings_log_xml)
+          lettings_log = LettingsLog.find_by(old_id: lettings_log_id)
+
+          expect(lettings_log).not_to be_nil
+          expect(lettings_log.offered).to be_nil
+        end
+      end
+
       context "and the gender identity is refused" do
         before do
           lettings_log_xml.at_xpath("//xmlns:P1Sex").content = "Person prefers not to say"
diff --git a/spec/services/imports/sales_logs_import_service_spec.rb b/spec/services/imports/sales_logs_import_service_spec.rb
index e4d5b9473..6d067dbd4 100644
--- a/spec/services/imports/sales_logs_import_service_spec.rb
+++ b/spec/services/imports/sales_logs_import_service_spec.rb
@@ -228,6 +228,31 @@ RSpec.describe Imports::SalesLogsImportService do
       end
     end
 
+    context "and the log startdate is only present in the CompletionDate field" do
+      let(:sales_log_id) { "shared_ownership_sales_log" }
+
+      before do
+        sales_log_xml.at_xpath("//xmlns:DAY").content = nil
+        sales_log_xml.at_xpath("//xmlns:MONTH").content = nil
+        sales_log_xml.at_xpath("//xmlns:YEAR").content = nil
+        sales_log_xml.at_xpath("//xmlns:CompletionDate").content = "2022-10-9"
+        sales_log_xml.at_xpath("//xmlns:HODAY").content = 9
+        sales_log_xml.at_xpath("//xmlns:HOMONTH").content = 10
+        sales_log_xml.at_xpath("//xmlns:HOYEAR").content = 2022
+        sales_log_xml.at_xpath("//xmlns:EXDAY").content = 9
+        sales_log_xml.at_xpath("//xmlns:EXMONTH").content = 10
+        sales_log_xml.at_xpath("//xmlns:EXYEAR").content = 2022
+      end
+
+      it "creates the log with the correct saledate" do
+        expect(logger).not_to receive(:error)
+        expect(logger).not_to receive(:warn)
+        expect { sales_log_service.send(:create_log, sales_log_xml) }
+          .to change(SalesLog, :count).by(1)
+        expect(SalesLog.last.saledate).to eq(Time.zone.local(2022, 10, 9))
+      end
+    end
+
     context "when the log is valid" do
       let(:sales_log_id) { "shared_ownership_sales_log" }
 

From 077390bb5b5001ab09d6c949b37ed9f99506b144 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Tue, 12 Sep 2023 15:15:56 +0100
Subject: [PATCH 16/18] CLDC-2699 Update values updated at when updating reason
 field (#1909)

* Update values updated at

* Update tests
---
 .../imports/lettings_logs_field_import_service.rb      |  2 +-
 .../imports/lettings_logs_field_import_service_spec.rb | 10 +++++++---
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/app/services/imports/lettings_logs_field_import_service.rb b/app/services/imports/lettings_logs_field_import_service.rb
index 68c3ba5f4..8b4124796 100644
--- a/app/services/imports/lettings_logs_field_import_service.rb
+++ b/app/services/imports/lettings_logs_field_import_service.rb
@@ -194,7 +194,7 @@ module Imports
           if reason == 20 && reasonother.blank?
             @logger.info("lettings log #{record.id}'s reason is other but other reason is not provided, skipping update")
           else
-            record.update!(reason:, reasonother:)
+            record.update!(reason:, reasonother:, values_updated_at: Time.zone.now)
             @logger.info("lettings log #{record.id}'s reason value has been set to #{reason}")
             @logger.info("lettings log #{record.id}'s reasonother value has been set to #{reasonother}") if record.reasonother.present?
           end
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 4bed2dc63..ac35169a5 100644
--- a/spec/services/imports/lettings_logs_field_import_service_spec.rb
+++ b/spec/services/imports/lettings_logs_field_import_service_spec.rb
@@ -481,7 +481,7 @@ RSpec.describe Imports::LettingsLogsFieldImportService do
       before do
         Imports::LettingsLogsImportService.new(storage_service, logger).create_logs(fixture_directory)
         lettings_log_file.rewind
-        lettings_log.update!(reason: nil)
+        lettings_log.update!(reason: nil, values_updated_at: nil)
         lettings_log_xml.at_xpath("//xmlns:Q9a").content = "47"
       end
 
@@ -489,6 +489,7 @@ RSpec.describe Imports::LettingsLogsFieldImportService do
         expect(logger).to receive(:info).with(/lettings log \d+'s reason value has been set to 47/)
         expect { import_service.send(:update_reason, lettings_log_xml) }
           .to(change { lettings_log.reload.reason }.from(nil).to(47))
+        expect(lettings_log.values_updated_at).not_to be_nil
       end
     end
 
@@ -498,7 +499,7 @@ RSpec.describe Imports::LettingsLogsFieldImportService do
       before do
         Imports::LettingsLogsImportService.new(storage_service, logger).create_logs(fixture_directory)
         lettings_log_file.rewind
-        lettings_log.update!(reason: 18)
+        lettings_log.update!(reason: 18, values_updated_at: nil)
         lettings_log_xml.at_xpath("//xmlns:Q9a").content = "47"
       end
 
@@ -506,6 +507,7 @@ RSpec.describe Imports::LettingsLogsFieldImportService do
         expect(logger).to receive(:info).with(/lettings log \d+ has a value for reason, skipping update/)
         expect { import_service.send(:update_reason, lettings_log_xml) }
           .not_to(change { lettings_log.reload.reason })
+        expect(lettings_log.values_updated_at).to be_nil
       end
     end
 
@@ -515,7 +517,7 @@ RSpec.describe Imports::LettingsLogsFieldImportService do
       before do
         Imports::LettingsLogsImportService.new(storage_service, logger).create_logs(fixture_directory)
         lettings_log_file.rewind
-        lettings_log.update!(reason: nil)
+        lettings_log.update!(reason: nil, values_updated_at: nil)
         lettings_log_xml.at_xpath("//xmlns:Q9a").content = "20"
       end
 
@@ -529,6 +531,7 @@ RSpec.describe Imports::LettingsLogsFieldImportService do
           expect(logger).to receive(:info).with(/lettings log \d+'s reasonother value has been set to other/)
           expect { import_service.send(:update_reason, lettings_log_xml) }
             .to(change { lettings_log.reload.reason }.from(nil).to(20))
+          expect(lettings_log.values_updated_at).not_to be_nil
         end
       end
 
@@ -537,6 +540,7 @@ RSpec.describe Imports::LettingsLogsFieldImportService do
           expect(logger).to receive(:info).with(/lettings log \d+'s reason is other but other reason is not provided, skipping update/)
           expect { import_service.send(:update_reason, lettings_log_xml) }
             .not_to(change { lettings_log.reload.reason })
+          expect(lettings_log.values_updated_at).to be_nil
         end
       end
     end

From a1cc9d78636707c7f1fc5571bbf084946a918fcf Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Tue, 12 Sep 2023 16:29:03 +0100
Subject: [PATCH 17/18] Move person details if some persons are skipped (#1903)

* Move person details if some persons are skipped

* Check that undesired person details don't get saved

* Make sure we don't repeat people details
---
 .../imports/lettings_logs_import_service.rb   |  28 ++-
 .../lettings_logs_import_service_spec.rb      | 220 ++++++++++++++++++
 2 files changed, 241 insertions(+), 7 deletions(-)

diff --git a/app/services/imports/lettings_logs_import_service.rb b/app/services/imports/lettings_logs_import_service.rb
index ca3cde434..d40df0fc1 100644
--- a/app/services/imports/lettings_logs_import_service.rb
+++ b/app/services/imports/lettings_logs_import_service.rb
@@ -76,14 +76,19 @@ module Imports
       attributes["irproduct_other"] = string_or_nil(xml_doc, "IRProductOther")
       attributes["rent_type"] = rent_type(xml_doc, attributes["lar"], attributes["irproduct"])
       attributes["hhmemb"] = household_members(xml_doc, previous_status)
+
+      people_indexes = people_with_details_ids(xml_doc)
+      available_people_indexes = people_indexes + (people_indexes.max + 1..8).to_a
       (1..8).each do |index|
-        attributes["age#{index}"] = safe_string_as_integer(xml_doc, "P#{index}Age")
-        attributes["age#{index}_known"] = age_known(xml_doc, index, attributes["hhmemb"])
-        attributes["sex#{index}"] = sex(xml_doc, index)
-        attributes["ecstat#{index}"] = unsafe_string_as_integer(xml_doc, "P#{index}Eco")
+        person_index = available_people_indexes[index - 1]
+        attributes["age#{index}"] = safe_string_as_integer(xml_doc, "P#{person_index}Age")
+        attributes["age#{index}_known"] = age_known(xml_doc, index, person_index, attributes["hhmemb"])
+        attributes["sex#{index}"] = sex(xml_doc, person_index)
+        attributes["ecstat#{index}"] = unsafe_string_as_integer(xml_doc, "P#{person_index}Eco")
       end
       (2..8).each do |index|
-        attributes["relat#{index}"] = relat(xml_doc, index)
+        person_index = available_people_indexes[index - 1]
+        attributes["relat#{index}"] = relat(xml_doc, person_index)
         attributes["details_known_#{index}"] = details_known(index, attributes)
 
         # Trips validation
@@ -449,10 +454,10 @@ module Imports
       end
     end
 
-    def age_known(xml_doc, index, hhmemb)
+    def age_known(xml_doc, index, person_index, hhmemb)
       return nil if hhmemb.present? && index > hhmemb
 
-      age_refused = string_or_nil(xml_doc, "P#{index}AR")
+      age_refused = string_or_nil(xml_doc, "P#{person_index}AR")
       if age_refused.present?
         if age_refused.casecmp?("AGE_REFUSED") || age_refused.casecmp?("No")
           return 1 # No
@@ -577,6 +582,15 @@ module Imports
       ((2..8).map { |x| string_or_nil(xml_doc, "P#{x}Rel") } + [string_or_nil(xml_doc, "P1Sex")]).compact
     end
 
+    def people_with_details_ids(xml_doc)
+      [1] + (2..8).select do |x|
+        string_or_nil(xml_doc, "P#{x}Rel").present? ||
+          string_or_nil(xml_doc, "P#{x}Sex").present? ||
+          string_or_nil(xml_doc, "P#{x}Age").present? ||
+          string_or_nil(xml_doc, "P#{x}Eco").present?
+      end
+    end
+
     def tshortfall_known?(xml_doc, attributes)
       if attributes["tshortfall"].blank? && attributes["hbrentshortfall"] == 1 && overridden?(xml_doc, "xmlns", "Q18dyes")
         1
diff --git a/spec/services/imports/lettings_logs_import_service_spec.rb b/spec/services/imports/lettings_logs_import_service_spec.rb
index dad2e68ed..efef0997a 100644
--- a/spec/services/imports/lettings_logs_import_service_spec.rb
+++ b/spec/services/imports/lettings_logs_import_service_spec.rb
@@ -1303,6 +1303,226 @@ RSpec.describe Imports::LettingsLogsImportService do
           expect(lettings_log.status).to eq("completed")
         end
       end
+
+      context "and there are several household members" do
+        context "and one person details are skipped" do
+          before do
+            lettings_log_xml.at_xpath("//xmlns:HHMEMB").content = 3
+            lettings_log_xml.at_xpath("//xmlns:P2AR").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P2Age").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P2Sex").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P2Rel").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P2Eco").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P3Age").content = 7
+            lettings_log_xml.at_xpath("//xmlns:P3Sex").content = "Male"
+            lettings_log_xml.at_xpath("//xmlns:P3Rel").content = "Child"
+            lettings_log_xml.at_xpath("//xmlns:P3Eco").content = "9) Child under 16"
+          end
+
+          it "correctly moves person details" do
+            lettings_log_service.send(:create_log, lettings_log_xml)
+
+            lettings_log = LettingsLog.where(old_id: lettings_log_id).first
+            expect(lettings_log&.hhmemb).to eq(3)
+            expect(lettings_log&.details_known_2).to eq(0)
+            expect(lettings_log&.age2_known).to eq(0)
+            expect(lettings_log&.age2).to eq(7)
+            expect(lettings_log&.sex2).to eq("M")
+            expect(lettings_log&.relat2).to eq("C")
+
+            expect(lettings_log&.details_known_3).to eq(0)
+            expect(lettings_log&.age3_known).to eq(0)
+            expect(lettings_log&.age3).to eq(nil)
+            expect(lettings_log&.sex3).to eq(nil)
+            expect(lettings_log&.relat3).to eq(nil)
+
+            [4, 5].each do |i|
+              expect(lettings_log&.send("details_known_#{i}")).to eq(nil)
+              expect(lettings_log&.send("age#{i}_known")).to eq(nil)
+              expect(lettings_log&.send("age#{i}")).to eq(nil)
+              expect(lettings_log&.send("sex#{i}")).to eq(nil)
+              expect(lettings_log&.send("relat#{i}")).to eq(nil)
+              expect(lettings_log&.send("ecstat#{i}")).to eq(nil)
+            end
+          end
+        end
+
+        context "and several consecutive person details are skipped" do
+          before do
+            lettings_log_xml.at_xpath("//xmlns:HHMEMB").content = 4
+
+            lettings_log_xml.at_xpath("//xmlns:P2AR").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P2Age").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P2Sex").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P2Rel").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P2Eco").content = nil
+
+            lettings_log_xml.at_xpath("//xmlns:P3AR").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P3Age").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P3Sex").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P3Rel").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P3Eco").content = nil
+
+            lettings_log_xml.at_xpath("//xmlns:P4Age").content = 7
+            lettings_log_xml.at_xpath("//xmlns:P4Sex").content = "Male"
+            lettings_log_xml.at_xpath("//xmlns:P4Rel").content = "Child"
+            lettings_log_xml.at_xpath("//xmlns:P4Eco").content = "9) Child under 16"
+
+            lettings_log_xml.at_xpath("//xmlns:P5Age").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P5Sex").content = "Male"
+            lettings_log_xml.at_xpath("//xmlns:P5Rel").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P5Eco").content = nil
+
+            lettings_log_xml.at_xpath("//xmlns:P6Age").content = 12
+            lettings_log_xml.at_xpath("//xmlns:P6Sex").content = "Female"
+            lettings_log_xml.at_xpath("//xmlns:P6Rel").content = "Child"
+            lettings_log_xml.at_xpath("//xmlns:P6Eco").content = "9) Child under 16"
+          end
+
+          it "correctly moves person details" do
+            lettings_log_service.send(:create_log, lettings_log_xml)
+
+            lettings_log = LettingsLog.where(old_id: lettings_log_id).first
+            expect(lettings_log&.hhmemb).to eq(4)
+            expect(lettings_log&.details_known_2).to eq(0)
+            expect(lettings_log&.age2_known).to eq(0)
+            expect(lettings_log&.age2).to eq(7)
+            expect(lettings_log&.sex2).to eq("M")
+            expect(lettings_log&.relat2).to eq("C")
+
+            expect(lettings_log&.details_known_3).to eq(0)
+            expect(lettings_log&.age3_known).to eq(0)
+            expect(lettings_log&.age3).to eq(nil)
+            expect(lettings_log&.sex3).to eq("M")
+            expect(lettings_log&.relat3).to eq(nil)
+
+            expect(lettings_log&.details_known_4).to eq(0)
+            expect(lettings_log&.age4_known).to eq(0)
+            expect(lettings_log&.age4).to eq(12)
+            expect(lettings_log&.sex4).to eq("F")
+            expect(lettings_log&.relat4).to eq("C")
+
+            [5, 6, 7, 8].each do |i|
+              expect(lettings_log&.send("details_known_#{i}")).to eq(nil)
+              expect(lettings_log&.send("age#{i}_known")).to eq(nil)
+              expect(lettings_log&.send("age#{i}")).to eq(nil)
+              expect(lettings_log&.send("sex#{i}")).to eq(nil)
+              expect(lettings_log&.send("relat#{i}")).to eq(nil)
+              expect(lettings_log&.send("ecstat#{i}")).to eq(nil)
+            end
+          end
+        end
+
+        context "and several non consecutive person details are skipped" do
+          before do
+            lettings_log_xml.at_xpath("//xmlns:HHMEMB").content = 4
+            lettings_log_xml.at_xpath("//xmlns:P2AR").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P2Age").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P2Sex").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P2Rel").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P2Eco").content = nil
+
+            lettings_log_xml.at_xpath("//xmlns:P3Age").content = 7
+            lettings_log_xml.at_xpath("//xmlns:P3Sex").content = "Male"
+            lettings_log_xml.at_xpath("//xmlns:P3Rel").content = "Child"
+            lettings_log_xml.at_xpath("//xmlns:P3Eco").content = "9) Child under 16"
+
+            lettings_log_xml.at_xpath("//xmlns:P4AR").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P4Age").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P4Sex").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P4Rel").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P4Eco").content = nil
+
+            lettings_log_xml.at_xpath("//xmlns:P5Age").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P5Sex").content = "Male"
+            lettings_log_xml.at_xpath("//xmlns:P5Rel").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P5Eco").content = nil
+
+            lettings_log_xml.at_xpath("//xmlns:P6Age").content = 12
+            lettings_log_xml.at_xpath("//xmlns:P6Sex").content = "Female"
+            lettings_log_xml.at_xpath("//xmlns:P6Rel").content = "Child"
+            lettings_log_xml.at_xpath("//xmlns:P6Eco").content = "9) Child under 16"
+          end
+
+          it "correctly moves person details" do
+            lettings_log_service.send(:create_log, lettings_log_xml)
+
+            lettings_log = LettingsLog.where(old_id: lettings_log_id).first
+            expect(lettings_log&.hhmemb).to eq(4)
+            expect(lettings_log&.details_known_2).to eq(0)
+            expect(lettings_log&.age2_known).to eq(0)
+            expect(lettings_log&.age2).to eq(7)
+            expect(lettings_log&.sex2).to eq("M")
+            expect(lettings_log&.relat2).to eq("C")
+
+            expect(lettings_log&.details_known_3).to eq(0)
+            expect(lettings_log&.age3_known).to eq(0)
+            expect(lettings_log&.age3).to eq(nil)
+            expect(lettings_log&.sex3).to eq("M")
+            expect(lettings_log&.relat3).to eq(nil)
+
+            expect(lettings_log&.details_known_4).to eq(0)
+            expect(lettings_log&.age4_known).to eq(0)
+            expect(lettings_log&.age4).to eq(12)
+            expect(lettings_log&.sex4).to eq("F")
+            expect(lettings_log&.relat4).to eq("C")
+
+            [5, 6, 7, 8].each do |i|
+              expect(lettings_log&.send("details_known_#{i}")).to eq(nil)
+              expect(lettings_log&.send("age#{i}_known")).to eq(nil)
+              expect(lettings_log&.send("age#{i}")).to eq(nil)
+              expect(lettings_log&.send("sex#{i}")).to eq(nil)
+              expect(lettings_log&.send("relat#{i}")).to eq(nil)
+              expect(lettings_log&.send("ecstat#{i}")).to eq(nil)
+            end
+          end
+        end
+
+        context "with 3 houusehold members without any person data" do
+          before do
+            lettings_log_xml.at_xpath("//xmlns:HHMEMB").content = 3
+            lettings_log_xml.at_xpath("//xmlns:P2AR").content = "No"
+            lettings_log_xml.at_xpath("//xmlns:P2Age").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P2Sex").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P2Rel").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P2Eco").content = nil
+
+            lettings_log_xml.at_xpath("//xmlns:P3AR").content = "No"
+            lettings_log_xml.at_xpath("//xmlns:P3Age").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P3Sex").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P3Rel").content = nil
+            lettings_log_xml.at_xpath("//xmlns:P3Eco").content = nil
+          end
+
+          it "correctly sets person details" do
+            lettings_log_service.send(:create_log, lettings_log_xml)
+
+            lettings_log = LettingsLog.where(old_id: lettings_log_id).first
+            expect(lettings_log&.hhmemb).to eq(3)
+
+            expect(lettings_log&.details_known_2).to eq(0)
+            expect(lettings_log&.age2_known).to eq(1)
+            expect(lettings_log&.age2).to eq(nil)
+            expect(lettings_log&.sex2).to eq(nil)
+            expect(lettings_log&.relat2).to eq(nil)
+
+            expect(lettings_log&.details_known_3).to eq(0)
+            expect(lettings_log&.age3_known).to eq(1)
+            expect(lettings_log&.age3).to eq(nil)
+            expect(lettings_log&.sex3).to eq(nil)
+            expect(lettings_log&.relat3).to eq(nil)
+
+            [4, 5, 6, 7, 8].each do |i|
+              expect(lettings_log&.send("details_known_#{i}")).to eq(nil)
+              expect(lettings_log&.send("age#{i}_known")).to eq(nil)
+              expect(lettings_log&.send("age#{i}")).to eq(nil)
+              expect(lettings_log&.send("sex#{i}")).to eq(nil)
+              expect(lettings_log&.send("relat#{i}")).to eq(nil)
+              expect(lettings_log&.send("ecstat#{i}")).to eq(nil)
+            end
+          end
+        end
+      end
     end
   end
 

From d3dec60f3969d6e02df3892d21ba36d452dacd06 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Tue, 12 Sep 2023 16:41:00 +0100
Subject: [PATCH 18/18] Only set soc tenant fields for 2022 or if they're given
 (#1908)

* Only set soc tenant fields for 2022 or if they're given

* Refactor
---
 .../imports/sales_logs_import_service.rb      | 14 +++++--
 .../imports/sales_logs_import_service_spec.rb | 38 +++++++++++++++++++
 2 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/app/services/imports/sales_logs_import_service.rb b/app/services/imports/sales_logs_import_service.rb
index 7d10f52ca..c57f2d16a 100644
--- a/app/services/imports/sales_logs_import_service.rb
+++ b/app/services/imports/sales_logs_import_service.rb
@@ -1,5 +1,7 @@
 module Imports
   class SalesLogsImportService < LogsImportService
+    include CollectionTimeHelper
+
     def initialize(storage_service, logger = Rails.logger, allow_updates: false)
       @logs_with_discrepancies = Set.new
       @logs_overridden = Set.new
@@ -124,7 +126,7 @@ module Imports
       attributes["mortgagelenderother"] = mortgage_lender_other(xml_doc, attributes)
       attributes["postcode_full"] = parse_postcode(string_or_nil(xml_doc, "Q14Postcode"))
       attributes["pcodenk"] = 0 if attributes["postcode_full"].present? # known if given
-      attributes["soctenant"] = 0 if attributes["ownershipsch"] == 1
+      attributes["soctenant"] = 0 if set_soctenant_fields?(attributes)
 
       attributes["previous_la_known"] = 1 if attributes["prevloc"].present?
       if attributes["la"].present?
@@ -584,6 +586,12 @@ module Imports
       end
     end
 
+    def set_soctenant_fields?(attributes)
+      return false if attributes["ownershipsch"] != 1
+
+      %w[socprevten frombeds fromprop].any? { |field| attributes[field].present? } || collection_start_year_for_date(attributes["saledate"]) < 2023
+    end
+
     def set_default_values(attributes)
       attributes["armedforcesspouse"] ||= 7
       attributes["hhregres"] ||= 8
@@ -600,8 +608,8 @@ module Imports
       attributes["pcodenk"] ||= 1
       attributes["prevten"] ||= 0
       attributes["extrabor"] ||= 3 if attributes["mortgageused"] == 1
-      attributes["socprevten"] ||= 10 if attributes["ownershipsch"] == 1
-      attributes["fromprop"] ||= 0 if attributes["ownershipsch"] == 1
+      attributes["socprevten"] ||= 10 if set_soctenant_fields?(attributes)
+      attributes["fromprop"] ||= 0 if set_soctenant_fields?(attributes)
       attributes["mortgagelender"] ||= 0 if attributes["mortgageused"] == 1
 
       # buyer 1 characteristics
diff --git a/spec/services/imports/sales_logs_import_service_spec.rb b/spec/services/imports/sales_logs_import_service_spec.rb
index 6d067dbd4..615201f3a 100644
--- a/spec/services/imports/sales_logs_import_service_spec.rb
+++ b/spec/services/imports/sales_logs_import_service_spec.rb
@@ -409,6 +409,42 @@ RSpec.describe Imports::SalesLogsImportService do
           sales_log = SalesLog.find_by(old_id: sales_log_id)
           expect(sales_log.proplen_asked).to eq(1)
         end
+
+        context "when setting soctenant fields" do
+          it "does not set soctenant value if none of the soctenant questions are answered" do
+            sales_log_xml.at_xpath("//xmlns:Q20Bedrooms").content = nil
+            sales_log_xml.at_xpath("//xmlns:PrevRentType").content = nil
+            sales_log_xml.at_xpath("//xmlns:Q21PropertyType").content = nil
+
+            expect(logger).not_to receive(:error)
+            expect(logger).not_to receive(:warn)
+            expect(logger).not_to receive(:info)
+            expect { sales_log_service.send(:create_log, sales_log_xml) }
+            .to change(SalesLog, :count).by(1)
+            sales_log = SalesLog.find_by(old_id: sales_log_id)
+            expect(sales_log.soctenant).to eq(nil)
+            expect(sales_log.frombeds).to eq(nil)
+            expect(sales_log.fromprop).to eq(nil)
+            expect(sales_log.socprevten).to eq(nil)
+          end
+
+          it "sets soctenant to don't know if any of the soctenant questions are answered" do
+            sales_log_xml.at_xpath("//xmlns:Q20Bedrooms").content = "2"
+            sales_log_xml.at_xpath("//xmlns:PrevRentType").content = nil
+            sales_log_xml.at_xpath("//xmlns:Q21PropertyType").content = nil
+
+            expect(logger).not_to receive(:error)
+            expect(logger).not_to receive(:warn)
+            expect(logger).not_to receive(:info)
+            expect { sales_log_service.send(:create_log, sales_log_xml) }
+            .to change(SalesLog, :count).by(1)
+            sales_log = SalesLog.find_by(old_id: sales_log_id)
+            expect(sales_log.soctenant).to eq(0)
+            expect(sales_log.frombeds).to eq(2)
+            expect(sales_log.fromprop).to eq(0)
+            expect(sales_log.socprevten).to eq(10)
+          end
+        end
       end
 
       context "with discounted sale type" do
@@ -2091,6 +2127,7 @@ RSpec.describe Imports::SalesLogsImportService do
 
           sales_log = SalesLog.find_by(old_id: sales_log_id)
           expect(sales_log&.fromprop).to be(0)
+          expect(sales_log&.soctenant).to be(0)
         end
       end
 
@@ -2107,6 +2144,7 @@ RSpec.describe Imports::SalesLogsImportService do
 
           sales_log = SalesLog.find_by(old_id: sales_log_id)
           expect(sales_log&.socprevten).to be(10)
+          expect(sales_log&.soctenant).to be(0)
         end
       end