From aad771b3841ad57cf1ab6af3c614f22605c830a3 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Tue, 21 Mar 2023 15:51:26 +0000 Subject: [PATCH] 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 --- .../form/sales/questions/mortgage_lender.rb | 47 +++++++++++++ app/models/sales_log.rb | 2 +- .../imports/sales_logs_import_service.rb | 8 ++- .../sales/questions/mortgage_lender_spec.rb | 47 +++++++++++++ spec/models/sales_log_spec.rb | 2 - .../imports/sales_logs_import_service_spec.rb | 69 +++++++++++++++++++ 6 files changed, 170 insertions(+), 5 deletions(-) diff --git a/app/models/form/sales/questions/mortgage_lender.rb b/app/models/form/sales/questions/mortgage_lender.rb index 1fee10b8b..ad584aab5 100644 --- a/app/models/form/sales/questions/mortgage_lender.rb +++ b/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 diff --git a/app/models/sales_log.rb b/app/models/sales_log.rb index 5b0d8f17c..ba2ea4fa8 100644 --- a/app/models/sales_log.rb +++ b/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? diff --git a/app/services/imports/sales_logs_import_service.rb b/app/services/imports/sales_logs_import_service.rb index af38106a2..c2844e019 100644 --- a/app/services/imports/sales_logs_import_service.rb +++ b/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 diff --git a/spec/models/form/sales/questions/mortgage_lender_spec.rb b/spec/models/form/sales/questions/mortgage_lender_spec.rb index 84caef701..811746dbe 100644 --- a/spec/models/form/sales/questions/mortgage_lender_spec.rb +++ b/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", diff --git a/spec/models/sales_log_spec.rb b/spec/models/sales_log_spec.rb index d8f4ca5bc..231dc68c4 100644 --- a/spec/models/sales_log_spec.rb +++ b/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 diff --git a/spec/services/imports/sales_logs_import_service_spec.rb b/spec/services/imports/sales_logs_import_service_spec.rb index be271778c..e41ffabaf 100644 --- a/spec/services/imports/sales_logs_import_service_spec.rb +++ b/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" }