CLDC-2143 Set more default fields and don't clear setup values (#1450)

* Set default hhregresstill

* Don't clear type if it errors

* Make mortgagelender mandatory and add a don't know option

* Infer mortgagelender as don't know by default

app/models/form/sales/questions/mortgage_lender.rb

@@ -56,8 +56,55 @@ class Form::Sales::Questions::MortgageLender < ::Form::Question
     "38" =>	"West Bromwich Building Society",
     "39" =>	"Yorkshire Building Society",
     "40" =>	"Other",
+    "0" =>	"Don’t know",
   }.freeze
 
+  def displayed_answer_options(_log, _user = nil)
+    {
+      "" => "Select an option",
+      "1" =>	"Atom Bank",
+      "2" =>	"Barclays Bank PLC",
+      "3" =>	"Bath Building Society",
+      "4" =>	"Buckinghamshire Building Society",
+      "5" =>	"Cambridge Building Society",
+      "6" =>	"Coventry Building Society",
+      "7" =>	"Cumberland Building Society",
+      "8" =>	"Darlington Building Society",
+      "9" =>	"Dudley Building Society",
+      "10" =>	"Ecology Building Society",
+      "11" =>	"Halifax",
+      "12" =>	"Hanley Economic Building Society",
+      "13" =>	"Hinckley and Rugby Building Society",
+      "14" =>	"Holmesdale Building Society",
+      "15" =>	"Ipswich Building Society",
+      "16" =>	"Leeds Building Society",
+      "17" =>	"Lloyds Bank",
+      "18" =>	"Mansfield Building Society",
+      "19" =>	"Market Harborough Building Society",
+      "20" =>	"Melton Mowbray Building Society",
+      "21" =>	"Nationwide Building Society",
+      "22" =>	"Natwest",
+      "23" =>	"Nedbank Private Wealth",
+      "24" =>	"Newbury Building Society",
+      "25" =>	"OneSavings Bank",
+      "26" =>	"Parity Trust",
+      "27" =>	"Penrith Building Society",
+      "28" =>	"Pepper Homeloans",
+      "29" =>	"Royal Bank of Scotland",
+      "30" =>	"Santander",
+      "31" =>	"Skipton Building Society",
+      "32" =>	"Teachers Building Society",
+      "33" =>	"The Co-operative Bank",
+      "34" =>	"Tipton & Coseley Building Society",
+      "35" =>	"TSB",
+      "36" =>	"Ulster Bank",
+      "37" =>	"Virgin Money",
+      "38" =>	"West Bromwich Building Society",
+      "39" =>	"Yorkshire Building Society",
+      "40" =>	"Other",
+    }
+  end
+
   def question_number
     case @ownershipsch
     when 1

app/models/sales_log.rb

@@ -42,7 +42,7 @@ class SalesLog < Log
   }
   scope :filter_by_organisation, ->(org, _user = nil) { where(owning_organisation: org) }
 
-  OPTIONAL_FIELDS = %w[saledate_check purchid monthly_charges_value_check old_persons_shared_ownership_value_check mortgagelender othtype discounted_sale_value_check].freeze
+  OPTIONAL_FIELDS = %w[saledate_check purchid monthly_charges_value_check old_persons_shared_ownership_value_check othtype discounted_sale_value_check].freeze
   RETIREMENT_AGES = { "M" => 65, "F" => 60, "X" => 65 }.freeze
 
   def lettings?

app/services/imports/sales_logs_import_service.rb

@@ -189,8 +189,10 @@ module Imports
     def rescue_validation_or_raise(sales_log, attributes, previous_status, exception)
       if %w[saved submitted-invalid].include?(previous_status)
         sales_log.errors.each do |error|
-          @logger.warn("Log #{sales_log.old_id}: Removing field #{error.attribute} from log triggering validation: #{error.type}")
-          attributes.delete(error.attribute.to_s)
+          unless error.attribute == :type || error.attribute == :ownershipsch
+            @logger.warn("Log #{sales_log.old_id}: Removing field #{error.attribute} from log triggering validation: #{error.type}")
+            attributes.delete(error.attribute.to_s)
+          end
           attributes.delete("pcodenk") if error.attribute == :postcode_full
           attributes.delete("ppcodenk") if error.attribute == :ppostcode_full
         end
@@ -504,6 +506,7 @@ module Imports
     def set_default_values(attributes)
       attributes["armedforcesspouse"] ||= 7
       attributes["hhregres"] ||= 8
+      attributes["hhregresstill"] ||= 7 if attributes["hhregres"] == 1
       attributes["disabled"] ||= 3
       attributes["wheel"] ||= 3
       attributes["hb"] ||= 4
@@ -518,6 +521,7 @@ module Imports
       attributes["extrabor"] ||= 3 if attributes["mortgageused"] == 1
       attributes["socprevten"] ||= 10 if attributes["ownershipsch"] == 1
       attributes["fromprop"] ||= 0 if attributes["ownershipsch"] == 1
+      attributes["mortgagelender"] ||= 0 if attributes["mortgageused"] == 1
 
       # buyer 1 characteristics
       attributes["age1_known"] ||= 1

spec/models/form/sales/questions/mortgage_lender_spec.rb

@@ -42,6 +42,53 @@ RSpec.describe Form::Sales::Questions::MortgageLender, type: :model do
 
   it "has the correct answer_options" do
     expect(question.answer_options).to eq({
+      "" => "Select an option",
+      "0" => "Don’t know",
+      "1" =>	"Atom Bank",
+      "2" =>	"Barclays Bank PLC",
+      "3" =>	"Bath Building Society",
+      "4" =>	"Buckinghamshire Building Society",
+      "5" =>	"Cambridge Building Society",
+      "6" =>	"Coventry Building Society",
+      "7" =>	"Cumberland Building Society",
+      "8" =>	"Darlington Building Society",
+      "9" =>	"Dudley Building Society",
+      "10" =>	"Ecology Building Society",
+      "11" =>	"Halifax",
+      "12" =>	"Hanley Economic Building Society",
+      "13" =>	"Hinckley and Rugby Building Society",
+      "14" =>	"Holmesdale Building Society",
+      "15" =>	"Ipswich Building Society",
+      "16" =>	"Leeds Building Society",
+      "17" =>	"Lloyds Bank",
+      "18" =>	"Mansfield Building Society",
+      "19" =>	"Market Harborough Building Society",
+      "20" =>	"Melton Mowbray Building Society",
+      "21" =>	"Nationwide Building Society",
+      "22" =>	"Natwest",
+      "23" =>	"Nedbank Private Wealth",
+      "24" =>	"Newbury Building Society",
+      "25" =>	"OneSavings Bank",
+      "26" =>	"Parity Trust",
+      "27" =>	"Penrith Building Society",
+      "28" =>	"Pepper Homeloans",
+      "29" =>	"Royal Bank of Scotland",
+      "30" =>	"Santander",
+      "31" =>	"Skipton Building Society",
+      "32" =>	"Teachers Building Society",
+      "33" =>	"The Co-operative Bank",
+      "34" =>	"Tipton & Coseley Building Society",
+      "35" =>	"TSB",
+      "36" =>	"Ulster Bank",
+      "37" =>	"Virgin Money",
+      "38" =>	"West Bromwich Building Society",
+      "39" =>	"Yorkshire Building Society",
+      "40" =>	"Other",
+    })
+  end
+
+  it "has the correct displayed_answer_options" do
+    expect(question.displayed_answer_options(nil, nil)).to eq({
       "" => "Select an option",
       "1" =>	"Atom Bank",
       "2" =>	"Barclays Bank PLC",

spec/models/sales_log_spec.rb

@@ -59,7 +59,6 @@ RSpec.describe SalesLog, type: :model do
           purchid
           monthly_charges_value_check
           old_persons_shared_ownership_value_check
-          mortgagelender
           othtype
           discounted_sale_value_check
           proplen
@@ -78,7 +77,6 @@ RSpec.describe SalesLog, type: :model do
           purchid
           monthly_charges_value_check
           old_persons_shared_ownership_value_check
-          mortgagelender
           othtype
           discounted_sale_value_check
           address_line2

spec/services/imports/sales_logs_import_service_spec.rb

@@ -167,6 +167,23 @@ RSpec.describe Imports::SalesLogsImportService do
       end
     end
 
+    context "when the mortgage lender is not set" do
+      let(:sales_log_id) { "discounted_ownership_sales_log" }
+
+      before do
+        sales_log_xml.at_xpath("//xmlns:Q34a").content = ""
+        allow(logger).to receive(:warn).and_return(nil)
+      end
+
+      it "correctly sets mortgage lender and mortgage lender other" do
+        sales_log_service.send(:create_log, sales_log_xml)
+
+        sales_log = SalesLog.find_by(old_id: sales_log_id)
+        expect(sales_log&.mortgagelender).to be(0)
+        expect(sales_log&.mortgagelenderother).to be_nil
+      end
+    end
+
     context "with shared ownership type" do
       let(:sales_log_id) { "shared_ownership_sales_log" }
 
@@ -422,6 +439,33 @@ RSpec.describe Imports::SalesLogsImportService do
       end
     end
 
+    context "and setup field has validation error in incomplete log" do
+      let(:sales_log_id) { "shared_ownership_sales_log" }
+
+      before do
+        sales_log_xml.at_xpath("//meta:status").content = "saved"
+        sales_log_xml.at_xpath("//xmlns:Q17aStaircase").content = "1 Yes"
+        sales_log_xml.at_xpath("//xmlns:PercentBought").content = "5"
+        sales_log_xml.at_xpath("//xmlns:PercentOwns").content = "40"
+        sales_log_xml.at_xpath("//xmlns:Q17Resale").content = ""
+        sales_log_xml.at_xpath("//xmlns:EXDAY").content = ""
+        sales_log_xml.at_xpath("//xmlns:EXMONTH").content = ""
+        sales_log_xml.at_xpath("//xmlns:EXYEAR").content = ""
+        sales_log_xml.at_xpath("//xmlns:HODAY").content = ""
+        sales_log_xml.at_xpath("//xmlns:HOMONTH").content = ""
+        sales_log_xml.at_xpath("//xmlns:HOYEAR").content = ""
+      end
+
+      it "intercepts the relevant validation error but does not clear setup fields" do
+        expect(logger).to receive(:warn).with(/Log shared_ownership_sales_log: Removing field stairbought from log triggering validation: The minimum increase in equity while staircasing is 10%/)
+        expect { sales_log_service.send(:create_log, sales_log_xml) }
+          .not_to raise_error
+        sales_log = SalesLog.find_by(old_id: sales_log_id)
+
+        expect(sales_log.type).to eq(2)
+      end
+    end
+
     context "and it has an invalid record with invalid postcodes" do
       let(:sales_log_id) { "discounted_ownership_sales_log" }
 
@@ -735,6 +779,31 @@ RSpec.describe Imports::SalesLogsImportService do
         end
       end
 
+      context "when inferring armed forces still" do
+        let(:sales_log_id) { "discounted_ownership_sales_log" }
+
+        before do
+          sales_log_xml.at_xpath("//xmlns:ArmedF").content = "1 Yes"
+          allow(logger).to receive(:warn).and_return(nil)
+        end
+
+        it "sets hhregresstill to don't know if not answered" do
+          sales_log_xml.at_xpath("//xmlns:LeftArmedF").content = ""
+          sales_log_service.send(:create_log, sales_log_xml)
+
+          sales_log = SalesLog.find_by(old_id: sales_log_id)
+          expect(sales_log&.hhregresstill).to eq(7)
+        end
+
+        it "sets hhregresstill correctly if answered" do
+          sales_log_xml.at_xpath("//xmlns:LeftArmedF").content = "4"
+          sales_log_service.send(:create_log, sales_log_xml)
+
+          sales_log = SalesLog.find_by(old_id: sales_log_id)
+          expect(sales_log&.hhregresstill).to eq(4)
+        end
+      end
+
       context "when inferring disability" do
         let(:sales_log_id) { "discounted_ownership_sales_log" }