CLDC-2262 Bulk upload clear invalid data to create forms (#1618)

* feat: wip blank fields and dependent fields on upload tos ee if valid and can upload with missing info - this is not the exact ac on the ticket yet

* feat: update seed

* feat: wip commit

* feat: add postcodenk error so can clear on validation

* feat: add postcode validation back

* feat: move la vals to shared and move and add tests

* feat: add correct pluralisation to warning message

* feat: add blank compound invalid fields methods

* feat: update validations

* feat: update pluralisation

* refactor: lint

* feat: clear errors associated with blanked values so log status is set correctly on creation

* feat: validate instead

* feat: avoid duplicated errors

* feat: dont auto-refuse income, different to imports

* feat: validate after every blank method

* feat: delete la validator spec

* tests: update

* refactor: erblinting

* refactor: cleanup

* refactor: move pluralizer to helper

* feat: copy update

* feat: rename

* feat: refactor to avoid redundant re-validations and test

* refactor: rubocop

* test: update

* test: update

* test: update

* update sidekiq

* feat: clear errors

* feat: run clearing twice in case first clear creates different errors

* feat: remove moved file

* feat: undo validation file tweaks as shared/specific overlap could do with a more general refactor

* feat: update tests

app/helpers/logs_helper.rb

@@ -47,4 +47,10 @@ module LogsHelper
     when :sales then sales_logs_csv_download_organisation_path(organisation, search:, codes_only:)
     end
   end
+
+  def pluralize_logs_and_errors_warning(log_count, error_count)
+    is_or_are = log_count == 1 ? "is" : "are"
+    need_or_needs = error_count == 1 ? "needs" : "need"
+    "There #{is_or_are} #{pluralize(log_count, 'log')} in this bulk upload with #{pluralize(error_count, 'error')} that still #{need_or_needs} to be fixed after upload."
+  end
 end

app/models/lettings_log.rb

@@ -529,6 +529,16 @@ class LettingsLog < Log
     is_carehome == 1
   end
 
+  def blank_compound_invalid_non_setup_fields!
+    super
+
+    self.postcode_known = nil if errors.attribute_names.include? :postcode_full
+
+    if errors.of_kind?(:earnings, :under_hard_min)
+      self.incfreq = nil
+    end
+  end
+
 private
 
   def reset_invalid_unresolved_log_fields!

app/models/log.rb

@@ -98,11 +98,19 @@ class Log < ApplicationRecord
   def blank_invalid_non_setup_fields!
     setup_ids = form.setup_sections.flat_map(&:subsections).flat_map(&:questions).map(&:id)
 
+    2.times do
+      next if valid?
+
       errors.each do |error|
         next if setup_ids.include?(error.attribute.to_s)
 
         public_send("#{error.attribute}=", nil)
       end
+
+      blank_compound_invalid_non_setup_fields!
+
+      errors.clear
+    end
   end
 
   (1..8).each do |person_num|
@@ -136,6 +144,20 @@ class Log < ApplicationRecord
     format_as_currency(field_value)
   end
 
+  def blank_compound_invalid_non_setup_fields!
+    self.ppcodenk = nil if errors.attribute_names.include? :ppostcode_full
+
+    if errors.of_kind?(:uprn, :uprn_error)
+      self.uprn_known = nil
+      self.uprn_confirmed = nil
+      self.address_line1 = nil
+      self.address_line2 = nil
+      self.town_or_city = nil
+      self.postcode_full = nil
+      self.county = nil
+    end
+  end
+
   def creation_method
     bulk_upload_id ? "bulk upload" : "single log"
   end

app/models/sales_log.rb

@@ -2,7 +2,6 @@ class SalesLogValidator < ActiveModel::Validator
   include Validations::Sales::SetupValidations
   include Validations::Sales::HouseholdValidations
   include Validations::Sales::PropertyValidations
-  include Validations::SharedValidations
   include Validations::Sales::FinancialValidations
   include Validations::Sales::SaleInformationValidations
   include Validations::SharedValidations
@@ -386,4 +385,10 @@ class SalesLog < Log
     when 3 then "outright sale"
     end
   end
+
+  def blank_compound_invalid_non_setup_fields!
+    super
+
+    self.pcodenk = nil if errors.attribute_names.include? :postcode_full
+  end
 end

app/models/validations/financial_validations.rb

@@ -5,7 +5,8 @@ module Validations::FinancialValidations
   # or 'validate_' to run on submit as well
   def validate_outstanding_rent_amount(record)
     if !record.has_housing_benefit_rent_shortfall? && record.tshortfall.present?
-      record.errors.add :tshortfall, :no_outstanding_charges, message: I18n.t("validations.financial.tshortfall.outstanding_amount_not_required")
+      record.errors.add :tshortfall, :no_outstanding_charges, message: I18n.t("validations.financial.tshortfall.outstanding_amount_not_expected")
+      record.errors.add :hbrentshortfall, :no_outstanding_charges, message: I18n.t("validations.financial.hbrentshortfall.outstanding_amount_not_expected")
     end
   end
 

app/services/bulk_upload/lettings/validator.rb

@@ -44,6 +44,11 @@ class BulkUpload::Lettings::Validator
     return false if any_setup_errors?
     return false if row_parsers.any?(&:block_log_creation?)
     return false if any_logs_already_exist? && FeatureToggle.bulk_upload_duplicate_log_check_enabled?
+
+    row_parsers.each do |row_parser|
+      row_parser.log.blank_invalid_non_setup_fields!
+    end
+
     return false if any_logs_invalid?
 
     true

app/services/bulk_upload/processor.rb

@@ -14,14 +14,13 @@ class BulkUpload::Processor
 
     if validator.any_setup_errors?
       send_setup_errors_mail
+
     elsif validator.create_logs?
       create_logs
 
       if created_logs_but_incompleted?
         send_how_fix_upload_mail
-      end
+      elsif created_logs_and_all_completed?
-
-      if created_logs_and_all_completed?
         bulk_upload.unpend
         send_success_mail
       end

app/services/bulk_upload/sales/validator.rb

@@ -38,7 +38,13 @@ class BulkUpload::Sales::Validator
     return false if any_setup_errors?
     return false if row_parsers.any?(&:block_log_creation?)
 
-    row_parsers.all? { |row_parser| row_parser.log.valid? }
+    row_parsers.each do |row_parser|
+      row_parser.log.blank_invalid_non_setup_fields!
+    end
+
+    return false if any_logs_invalid?
+
+    true
   end
 
   def any_setup_errors?
@@ -51,6 +57,10 @@ class BulkUpload::Sales::Validator
 
 private
 
+  def any_logs_invalid?
+    row_parsers.any? { |row_parser| row_parser.log.invalid? }
+  end
+
   def csv_parser
     @csv_parser ||= case bulk_upload.year
                     when 2022

app/views/bulk_upload_lettings_resume/confirm.html.erb

@@ -7,7 +7,7 @@
     <span class="govuk-caption-l">Bulk upload for lettings (<%= @bulk_upload.year_combo %>)</span>
     <h1 class="govuk-heading-l">Are you sure you want to upload all logs from this bulk upload?</h1>
 
-    <p class="govuk-body">There are <%= pluralize(@bulk_upload.logs.count, "log") %> in this bulk upload with <%= pluralize(@bulk_upload.bulk_upload_errors.count, "error") %> that still need to be fixed after upload.</p>
+    <p class="govuk-body"><%= pluralize_logs_and_errors_warning(@bulk_upload.logs.count, @bulk_upload.bulk_upload_errors.count) %> </p>
 
     <%= govuk_warning_text(icon_fallback_text: "Danger") do %>
       You can not delete logs once you create them

config/locales/en.yml

@@ -256,10 +256,11 @@ en:
 
     financial:
       tshortfall:
-        outstanding_amount_not_required: "You cannot answer the outstanding amount question if you don’t have outstanding rent or charges"
+        outstanding_amount_not_expected: "You cannot answer the outstanding amount question if you don’t have outstanding rent or charges"
         more_than_rent: "Enter a value less less than the basic rent amount"
         must_be_positive: "Enter a value over £0.01 as you told us there is an outstanding amount"
       hbrentshortfall:
+        outstanding_amount_not_expected: "Answer must be ‘yes’ as you have answered the outstanding amount question"
         outstanding_no_benefits: "Answer cannot be ‘yes’ to outstanding amount for basic rent or charges if tenant does not receive housing benefit or Universal Credit or you‘re not sure"
       benefits:
         part_or_full_time: "Answer cannot be ‘all’ for income from Universal Credit, state pensions or benefits if the tenant or their partner works part-time or full-time"

spec/models/validations/financial_validations_spec.rb

@@ -81,7 +81,7 @@ RSpec.describe Validations::FinancialValidations do
         record.tshortfall = 99
         financial_validator.validate_outstanding_rent_amount(record)
         expect(record.errors["tshortfall"])
-          .to include(match I18n.t("validations.financial.tshortfall.outstanding_amount_not_required"))
+          .to include(match I18n.t("validations.financial.tshortfall.outstanding_amount_not_expected"))
       end
     end
 

spec/services/bulk_upload/lettings/log_creator_spec.rb

@@ -82,6 +82,36 @@ RSpec.describe BulkUpload::Lettings::LogCreator do
       end
     end
 
+    context "when a valid csv with row with compound errors on non setup field" do
+      let(:file) { Tempfile.new }
+      let(:path) { file.path }
+      let(:log) do
+        build(
+          :lettings_log,
+          :completed,
+          earnings: 0,
+          incfreq: 1,
+        )
+      end
+
+      before do
+        file.write(BulkUpload::LettingsLogToCsv.new(log:, col_offset: 0).to_2023_csv_row)
+        file.rewind
+      end
+
+      it "creates the log" do
+        expect { service.call }.to change(LettingsLog, :count).by(1)
+      end
+
+      it "blanks invalid fields" do
+        service.call
+
+        record = LettingsLog.last
+        expect(record.earnings).to be_blank
+        expect(record.incfreq).to be_blank
+      end
+    end
+
     context "when pre-creating logs" do
       subject(:service) { described_class.new(bulk_upload:, path:) }
 

spec/services/bulk_upload/sales/log_creator_spec.rb

@@ -75,6 +75,41 @@ RSpec.describe BulkUpload::Sales::LogCreator do
       end
     end
 
+    context "when a valid csv with row with compound errors on non setup field" do
+      let(:file) { Tempfile.new }
+      let(:path) { file.path }
+      let(:log) do
+        build(
+          :sales_log,
+          :completed,
+          ownershipsch: 2,
+          pcodenk: 0,
+          ppcodenk: 0,
+          postcode_full: "AA11AA",
+          ppostcode_full: "BB22BB",
+        )
+      end
+
+      before do
+        file.write(BulkUpload::SalesLogToCsv.new(log:, col_offset: 0).to_2022_csv_row)
+        file.rewind
+      end
+
+      it "creates the log" do
+        expect { service.call }.to change(SalesLog, :count).by(1)
+      end
+
+      it "blanks invalid field" do
+        service.call
+
+        record = SalesLog.last
+        expect(record.pcodenk).to be_blank
+        expect(record.postcode_full).to be_blank
+        expect(record.ppcodenk).to be_blank
+        expect(record.ppostcode_full).to be_blank
+      end
+    end
+
     context "when pre-creating logs" do
       subject(:service) { described_class.new(bulk_upload:, path:) }