Update import to handle validations (#1605)

* Clear all the charges if the error is on tcharge

* Round earnings value upon import

* Rounds savings to the nearest 10

* Clear income on over_hard_max_for_london validation

* Refactor sale import validations to be consistent with lettings

* Extract charges attributes into a variable

app/models/validations/sales/financial_validations.rb

@@ -7,7 +7,7 @@ module Validations::Sales::FinancialValidations
 
     relevant_fields = %i[income1 ownershipsch uprn la postcode_full]
     if record.london_property? && record.income1 > 90_000
-      relevant_fields.each { |field| record.errors.add field, I18n.t("validations.financial.income.over_hard_max_for_london") }
+      relevant_fields.each { |field| record.errors.add field, :over_hard_max_for_london, message: I18n.t("validations.financial.income.over_hard_max_for_london") }
     elsif record.property_not_in_london? && record.income1 > 80_000
       relevant_fields.each { |field| record.errors.add field, :over_hard_max_for_outside_london, message: I18n.t("validations.financial.income.over_hard_max_for_outside_london") }
     end
@@ -18,7 +18,7 @@ module Validations::Sales::FinancialValidations
 
     relevant_fields = %i[income2 ownershipsch uprn la postcode_full]
     if record.london_property? && record.income2 > 90_000
-      relevant_fields.each { |field| record.errors.add field, I18n.t("validations.financial.income.over_hard_max_for_london") }
+      relevant_fields.each { |field| record.errors.add field, :over_hard_max_for_london, message: I18n.t("validations.financial.income.over_hard_max_for_london") }
     elsif record.property_not_in_london? && record.income2 > 80_000
       relevant_fields.each { |field| record.errors.add field, I18n.t("validations.financial.income.over_hard_max_for_outside_london") }
     end

app/services/imports/lettings_logs_import_service.rb

@@ -105,7 +105,7 @@ module Imports
 
       attributes["hb"] = unsafe_string_as_integer(xml_doc, "Q6Ben")
       attributes["benefits"] = unsafe_string_as_integer(xml_doc, "Q7Ben")
-      attributes["earnings"] = safe_string_as_decimal(xml_doc, "Q8Money")
+      attributes["earnings"] = safe_string_as_decimal(xml_doc, "Q8Money")&.round
       attributes["net_income_known"] = net_income_known(xml_doc, attributes["earnings"])
       attributes["incfreq"] = unsafe_string_as_integer(xml_doc, "Q8a")
 
@@ -284,11 +284,14 @@ module Imports
     end
 
     def rescue_validation_or_raise(lettings_log, attributes, previous_status, exception)
+      charges_attributes = %w[brent scharge pscharge supcharg tcharge]
+
       # Blank out all invalid fields for in-progress logs
       if %w[saved submitted-invalid].include?(previous_status)
         lettings_log.errors.each do |error|
           @logger.warn("Log #{lettings_log.old_id}: Removing field #{error.attribute} from log triggering validation: #{error.type}")
           attributes.delete(error.attribute.to_s)
+          charges_attributes.each { |attribute| attributes.delete(attribute) } if error.attribute == :tcharge
         end
         @logs_overridden << lettings_log.old_id
         return save_lettings_log(attributes, previous_status)
@@ -306,16 +309,16 @@ module Imports
         %i[earnings over_hard_max] => %w[ecstat1],
         %i[tshortfall no_outstanding_charges] => %w[tshortfall hbrentshortfall],
         %i[beds outside_the_range] => %w[beds],
-        %i[tcharge complete_1_of_3] => %w[brent scharge pscharge supcharg tcharge],
+        %i[tcharge complete_1_of_3] => charges_attributes,
-        %i[scharge under_min] => %w[brent scharge pscharge supcharg tcharge],
+        %i[scharge under_min] => charges_attributes,
         %i[tshortfall must_be_positive] => %w[tshortfall tshortfall_known],
         %i[referral referral_invalid] => %w[referral],
-        %i[pscharge outside_the_range] => %w[brent scharge pscharge supcharg tcharge],
+        %i[pscharge outside_the_range] => charges_attributes,
-        %i[supcharg outside_the_range] => %w[brent scharge pscharge supcharg tcharge],
+        %i[supcharg outside_the_range] => charges_attributes,
-        %i[scharge outside_the_range] => %w[brent scharge pscharge supcharg tcharge],
+        %i[scharge outside_the_range] => charges_attributes,
         %i[location_id not_active] => %w[location_id scheme_id],
-        %i[tcharge under_10] => %w[brent scharge pscharge supcharg tcharge],
+        %i[tcharge under_10] => charges_attributes,
-        %i[brent over_hard_max] => %w[brent scharge pscharge supcharg tcharge],
+        %i[brent over_hard_max] => charges_attributes,
       }
 
       (2..8).each do |person|

app/services/imports/sales_logs_import_service.rb

@@ -70,7 +70,7 @@ module Imports
       attributes["inc1mort"] = unsafe_string_as_integer(xml_doc, "Q2Person1Mortgage")
       attributes["income2"] = safe_string_as_integer(xml_doc, "Q2Person2Income")
       attributes["income2nk"] = income_known(unsafe_string_as_integer(xml_doc, "P2IncKnown"))
-      attributes["savings"] = safe_string_as_integer(xml_doc, "Q3Savings")
+      attributes["savings"] = safe_string_as_integer(xml_doc, "Q3Savings")&.round(-1)
       attributes["savingsnk"] = savings_known(xml_doc)
       attributes["prevown"] = unsafe_string_as_integer(xml_doc, "Q4PrevOwnedProperty")
       attributes["mortgage"] = safe_string_as_decimal(xml_doc, "CALCMORT")
@@ -217,39 +217,38 @@ module Imports
           attributes.delete("ppcodenk") if error.attribute == :ppostcode_full
         end
         @logs_overridden << sales_log.old_id
-        save_sales_log(attributes, previous_status)
+        return save_sales_log(attributes, previous_status)
-      elsif sales_log.errors.of_kind?(:postcode_full, :postcodes_not_matching)
+      end
-        @logger.warn("Log #{sales_log.old_id}: Removing previous postcode known and previous postcode as the postcode is invalid")
+
-        @logs_overridden << sales_log.old_id
+      errors = {
-        attributes.delete("ppcodenk")
+        %i[postcode_full postcodes_not_matching] => %w[ppcodenk ppostcode_full],
-        attributes.delete("ppostcode_full")
+        %i[exdate over_a_year_from_saledate] => %w[exdate],
-        save_sales_log(attributes, previous_status)
+        %i[income1 over_hard_max_for_outside_london] => %w[income1],
-      elsif sales_log.errors.of_kind?(:exdate, :over_a_year_from_saledate)
+        %i[income1 over_hard_max_for_london] => %w[income1],
-        @logger.warn("Log #{sales_log.old_id}: Removing exchange date as the exchange date is invalid")
+        %i[income2 over_hard_max_for_london] => %w[income2],
-        @logs_overridden << sales_log.old_id
+        %i[equity over_max] => %w[equity],
-        attributes.delete("exdate")
+        %i[equity under_min] => %w[equity],
-        save_sales_log(attributes, previous_status)
+        %i[mortgage cannot_be_0] => %w[mortgage],
-      elsif sales_log.errors.of_kind?(:income1, :over_hard_max_for_outside_london)
+      }
-        @logger.warn("Log #{sales_log.old_id}: Removing income1 as the income1 is invalid")
+
-        @logs_overridden << sales_log.old_id
+      errors.each do |(error, fields)|
-        attributes.delete("income1")
+        next unless sales_log.errors.of_kind?(*error)
-        save_sales_log(attributes, previous_status)
+
-      elsif sales_log.errors.of_kind?(:equity, :over_max) || sales_log.errors.of_kind?(:equity, :under_min)
+        attribute, _type = error
-        @logger.warn("Log #{sales_log.old_id}: Removing equity as the equity is invalid")
+        fields.each do |field|
+          @logger.warn("Log #{sales_log.old_id}: Removing #{field} with error: #{sales_log.errors[attribute].sort.join(', ')}")
+          attributes.delete(field)
+        end
         @logs_overridden << sales_log.old_id
-        attributes.delete("equity")
+        return save_sales_log(attributes, previous_status)
-        save_sales_log(attributes, previous_status)
+      end
-      elsif sales_log.errors.of_kind?(:postcode_full, :wrong_format)
+
+      if sales_log.errors.of_kind?(:postcode_full, :wrong_format)
         @logger.warn("Log #{sales_log.old_id}: Removing postcode as the postcode is invalid")
         @logs_overridden << sales_log.old_id
         attributes.delete("postcode_full")
         attributes["pcodenk"] = attributes["la"].present? ? 1 : nil
         save_sales_log(attributes, previous_status)
-      elsif sales_log.errors.of_kind?(:mortgage, :cannot_be_0)
-        @logger.warn("Log #{sales_log.old_id}: Removing mortgage because it cannot be 0")
-        @logs_overridden << sales_log.old_id
-        attributes.delete("mortgage")
-        save_sales_log(attributes, previous_status)
       elsif sales_log.errors.of_kind?(:uprn, :uprn_error)
         @logger.warn("Log #{sales_log.old_id}: Setting uprn_known to no with error: #{sales_log.errors[:uprn].join(', ')}")
         @logs_overridden << sales_log.old_id

spec/services/imports/lettings_logs_import_service_spec.rb

@@ -1173,6 +1173,68 @@ RSpec.describe Imports::LettingsLogsImportService do
       end
     end
 
+    context "and an error is added to tcharge for in progress log" do
+      let(:lettings_log_id) { "00d2343e-d5fa-4c89-8400-ec3854b0f2b4" }
+      let(:lettings_log_file) { open_file(fixture_directory, lettings_log_id) }
+      let(:lettings_log_xml) { Nokogiri::XML(lettings_log_file) }
+
+      before do
+        lettings_log_xml.at_xpath("//meta:status").content = "saved"
+        lettings_log_xml.at_xpath("//xmlns:Q18ai").content = "1"
+        lettings_log_xml.at_xpath("//xmlns:Q18aii").content = "2"
+        lettings_log_xml.at_xpath("//xmlns:Q18aiii").content = "3"
+        lettings_log_xml.at_xpath("//xmlns:Q18aiv").content = "3"
+        lettings_log_xml.at_xpath("//xmlns:Q18av").content = "9"
+      end
+
+      it "intercepts the relevant validation error" do
+        expect(logger).to receive(:warn).with("Log 00d2343e-d5fa-4c89-8400-ec3854b0f2b4: Removing field tcharge from log triggering validation: under_10")
+        lettings_log_service.send(:create_log, lettings_log_xml)
+      end
+
+      it "clears out the invalid answers" 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.tcharge).to be_nil
+        expect(lettings_log.brent).to be_nil
+        expect(lettings_log.scharge).to be_nil
+        expect(lettings_log.pscharge).to be_nil
+        expect(lettings_log.supcharg).to be_nil
+      end
+    end
+
+    context "and the earnings is not a whole number" do
+      let(:lettings_log_id) { "00d2343e-d5fa-4c89-8400-ec3854b0f2b4" }
+      let(:lettings_log_file) { open_file(fixture_directory, lettings_log_id) }
+      let(:lettings_log_xml) { Nokogiri::XML(lettings_log_file) }
+
+      before do
+        lettings_log_xml.at_xpath("//meta:status").content = "submitted"
+        lettings_log_xml.at_xpath("//xmlns:Q8a").content = "1 Weekly"
+        lettings_log_xml.at_xpath("//xmlns:Q8Money").content = 100.59
+        lettings_log_xml.at_xpath("//xmlns:Q8Refused").content = ""
+      end
+
+      it "does not error" do
+        expect { lettings_log_service.send(:create_log, lettings_log_xml) }
+          .not_to raise_error
+      end
+
+      it "rounds the earnings value" 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.earnings).to eq(101)
+      end
+    end
+
     context "when setting location fields for 23/24 logs" do
       let(:lettings_log_id) { "00d2343e-d5fa-4c89-8400-ec3854b0f2b4" }
       let(:lettings_log_file) { open_file(fixture_directory, lettings_log_id) }

spec/services/imports/sales_logs_import_service_spec.rb

@@ -528,7 +528,7 @@ RSpec.describe Imports::SalesLogsImportService do
       end
 
       it "intercepts the relevant validation error" do
-        expect(logger).to receive(:warn).with(/Log shared_ownership_sales_log: Removing equity as the equity is invalid/)
+        expect(logger).to receive(:warn).with(/Removing equity with error: The maximum initial equity stake is 75%/)
         expect { sales_log_service.send(:create_log, sales_log_xml) }
           .not_to raise_error
       end
@@ -552,7 +552,7 @@ RSpec.describe Imports::SalesLogsImportService do
       end
 
       it "intercepts the relevant validation error" do
-        expect(logger).to receive(:warn).with(/Log shared_ownership_sales_log: Removing equity as the equity is invalid/)
+        expect(logger).to receive(:warn).with(/Removing equity with error: The minimum initial equity stake for this type of shared ownership sale is 25%/)
         expect { sales_log_service.send(:create_log, sales_log_xml) }
           .not_to raise_error
       end
@@ -631,7 +631,8 @@ RSpec.describe Imports::SalesLogsImportService do
       end
 
       it "intercepts the relevant validation error" do
-        expect(logger).to receive(:warn).with(/Removing previous postcode known and previous postcode as the postcode is invalid/)
+        expect(logger).to receive(:warn).with(/Removing ppcodenk with error: Buyer's last accommodation and discounted ownership postcodes must match, Last settled accommodation and discounted ownership property postcodes must match/)
+        expect(logger).to receive(:warn).with(/Removing ppostcode_full with error: Buyer's last accommodation and discounted ownership postcodes must match, Last settled accommodation and discounted ownership property postcodes must match/)
         expect { sales_log_service.send(:create_log, sales_log_xml) }
           .not_to raise_error
       end
@@ -701,7 +702,7 @@ RSpec.describe Imports::SalesLogsImportService do
       end
 
       it "intercepts the relevant validation error" do
-        expect(logger).to receive(:warn).with(/Removing exchange date as the exchange date is invalid/)
+        expect(logger).to receive(:warn).with(/Removing exdate with error: Contract exchange date must be less than 1 year before sale completion date/)
         expect { sales_log_service.send(:create_log, sales_log_xml) }
           .not_to raise_error
       end
@@ -726,7 +727,7 @@ RSpec.describe Imports::SalesLogsImportService do
       end
 
       it "intercepts the relevant validation error" do
-        expect(logger).to receive(:warn).with(/Removing mortgage because it cannot be 0/)
+        expect(logger).to receive(:warn).with(/Removing mortgage with error: Mortgage amount must be at least £1, Mortgage value cannot be £0 if a mortgage was used for the purchase of this property/)
         expect { sales_log_service.send(:create_log, sales_log_xml) }
           .not_to raise_error
       end
@@ -742,7 +743,7 @@ RSpec.describe Imports::SalesLogsImportService do
       end
     end
 
-    context "and it has an invalid income" do
+    context "and it has an invalid income 1" do
       let(:sales_log_id) { "shared_ownership_sales_log" }
 
       before do
@@ -751,7 +752,7 @@ RSpec.describe Imports::SalesLogsImportService do
       end
 
       it "intercepts the relevant validation error" do
-        expect(logger).to receive(:warn).with(/Removing income1 as the income1 is invalid/)
+        expect(logger).to receive(:warn).with(/Removing income1 with error: Income must be £80,000 or lower for properties outside London local authority/)
         expect { sales_log_service.send(:create_log, sales_log_xml) }
           .not_to raise_error
       end
@@ -767,6 +768,56 @@ RSpec.describe Imports::SalesLogsImportService do
       end
     end
 
+    context "and it has an invalid income 1 for london" do
+      let(:sales_log_id) { "shared_ownership_sales_log" }
+
+      before do
+        sales_log_xml.at_xpath("//xmlns:Q2Person1Income").content = "95000"
+        sales_log_xml.at_xpath("//xmlns:Q14ONSLACode").content = "E09000012"
+      end
+
+      it "intercepts the relevant validation error" do
+        expect(logger).to receive(:warn).with(/Removing income1 with error: Income must be £90,000 or lower for properties within a London local authority/)
+        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
+      end
+    end
+
+    context "and it has an invalid income 2 for london" do
+      let(:sales_log_id) { "shared_ownership_sales_log" }
+
+      before do
+        sales_log_xml.at_xpath("//xmlns:Q2Person2Income").content = "95000"
+        sales_log_xml.at_xpath("//xmlns:Q14ONSLACode").content = "E09000012"
+      end
+
+      it "intercepts the relevant validation error" do
+        expect(logger).to receive(:warn).with(/Removing income2 with error: Combined income must be £90,000 or lower for properties within a London local authority, Income must be £90,000 or lower for properties within a London local authority/)
+        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.income2).to be_nil
+      end
+    end
+
     context "when inferring default answers for completed sales logs" do
       context "when the armedforcesspouse is not answered" do
         let(:sales_log_id) { "discounted_ownership_sales_log" }
@@ -817,6 +868,26 @@ RSpec.describe Imports::SalesLogsImportService do
         end
       end
 
+      context "when the savings is given not to the nearest 10" do
+        let(:sales_log_id) { "discounted_ownership_sales_log" }
+
+        before do
+          sales_log_xml.at_xpath("//xmlns:Q3Savings").content = "10013"
+          allow(logger).to receive(:warn).and_return(nil)
+        end
+
+        it "does not error" do
+          expect { sales_log_service.send(:create_log, sales_log_xml) }.not_to raise_error
+        end
+
+        it "sets savings to the nearest 10" do
+          sales_log_service.send(:create_log, sales_log_xml)
+
+          sales_log = SalesLog.find_by(old_id: sales_log_id)
+          expect(sales_log&.savings).to be(10_010)
+        end
+      end
+
       context "when inferring age known" do
         let(:sales_log_id) { "discounted_ownership_sales_log" }