From 4b17ab94df0c22435f2d0c9642dac78d8340feed Mon Sep 17 00:00:00 2001 From: James Rose Date: Tue, 28 Feb 2023 11:13:21 +0000 Subject: [PATCH 01/14] Add better validation error logging to lettings import service (#1355) --- app/services/imports/lettings_logs_import_service.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/app/services/imports/lettings_logs_import_service.rb b/app/services/imports/lettings_logs_import_service.rb index a215e8f5a..a474f0c45 100644 --- a/app/services/imports/lettings_logs_import_service.rb +++ b/app/services/imports/lettings_logs_import_service.rb @@ -289,6 +289,15 @@ module Imports save_lettings_log(attributes, previous_status) else @logger.error("Log #{lettings_log.old_id}: Failed to import") + lettings_log.errors.each do |error| + @logger.error("Validation error: Field #{error.attribute}:") + @logger.error("\tOwning Organisation: #{lettings_log.owning_organisation&.name}") + @logger.error("\tManaging Organisation: #{lettings_log.managing_organisation&.name}") + @logger.error("\tOld CORE ID: #{lettings_log.old_id}") + @logger.error("\tOld CORE: #{attributes[error.attribute.to_s]&.inspect}") + @logger.error("\tNew CORE: #{lettings_log.read_attribute(error.attribute)&.inspect}") + @logger.error("\tError message: #{error.type}") + end raise exception end end From cb2c13ed7a2e9c6d18529a9e4fbfa7ebd1415eb1 Mon Sep 17 00:00:00 2001 From: James Rose Date: Wed, 1 Mar 2023 09:07:23 +0000 Subject: [PATCH 02/14] Blank out incompatible data when importing from CDS. (#1357) This covers the following errors: - Where the income is 0, set earnings and income to blank and set incref to refused - Removing invalid tenancylength and tenancy values where tenancylength is invalid - Removing prevten and age1 where incompatible --- .../validations/financial_validations.rb | 4 +- .../validations/household_validations.rb | 2 +- .../imports/lettings_logs_import_service.rb | 21 ++++++ .../lettings_logs_import_service_spec.rb | 75 +++++++++++++++++++ 4 files changed, 99 insertions(+), 3 deletions(-) diff --git a/app/models/validations/financial_validations.rb b/app/models/validations/financial_validations.rb index a5430bbea..175b0b1bd 100644 --- a/app/models/validations/financial_validations.rb +++ b/app/models/validations/financial_validations.rb @@ -24,11 +24,11 @@ module Validations::FinancialValidations def validate_net_income(record) if record.ecstat1 && record.weekly_net_income if record.weekly_net_income > record.applicable_income_range.hard_max - record.errors.add :earnings, I18n.t("validations.financial.earnings.over_hard_max", hard_max: record.applicable_income_range.hard_max) + record.errors.add :earnings, :over_hard_max, message: I18n.t("validations.financial.earnings.over_hard_max", hard_max: record.applicable_income_range.hard_max) end if record.weekly_net_income < record.applicable_income_range.hard_min - record.errors.add :earnings, I18n.t("validations.financial.earnings.under_hard_min", hard_min: record.applicable_income_range.hard_min) + record.errors.add :earnings, :under_hard_min, message: I18n.t("validations.financial.earnings.under_hard_min", hard_min: record.applicable_income_range.hard_min) end end diff --git a/app/models/validations/household_validations.rb b/app/models/validations/household_validations.rb index 4ffd0b1f8..afa7867ff 100644 --- a/app/models/validations/household_validations.rb +++ b/app/models/validations/household_validations.rb @@ -67,7 +67,7 @@ module Validations::HouseholdValidations end if record.age1.present? && record.age1 > 19 && record.previous_tenancy_was_foster_care? - record.errors.add :prevten, I18n.t("validations.household.prevten.over_20_foster_care") + record.errors.add :prevten, :over_20_foster_care, message: I18n.t("validations.household.prevten.over_20_foster_care") record.errors.add :age1, I18n.t("validations.household.age.lead.over_20") end diff --git a/app/services/imports/lettings_logs_import_service.rb b/app/services/imports/lettings_logs_import_service.rb index a474f0c45..3aa73db4b 100644 --- a/app/services/imports/lettings_logs_import_service.rb +++ b/app/services/imports/lettings_logs_import_service.rb @@ -287,6 +287,27 @@ module Imports @logs_overridden << lettings_log.old_id attributes.delete("referral") save_lettings_log(attributes, previous_status) + elsif lettings_log.errors.of_kind?(:earnings, :under_hard_min) + @logger.warn("Log #{lettings_log.old_id}: Where the income is 0, set earnings and income to blank and set incref to refused") + @logs_overridden << lettings_log.old_id + + attributes.delete("earnings") + attributes.delete("incfreq") + attributes["incref"] = 1 + attributes["net_income_known"] = 2 + save_lettings_log(attributes, previous_status) + elsif lettings_log.errors.include?(:tenancylength) && lettings_log.errors.include?(:tenancy) + @logger.warn("Log #{lettings_log.old_id}: Removing tenancylength as invalid") + @logs_overridden << lettings_log.old_id + attributes.delete("tenancylength") + attributes.delete("tenancy") + save_lettings_log(attributes, previous_status) + elsif lettings_log.errors.of_kind?(:prevten, :over_20_foster_care) + @logger.warn("Log #{lettings_log.old_id}: Removing age1 and prevten as incompatible") + @logs_overridden << lettings_log.old_id + attributes.delete("prevten") + attributes.delete("age1") + save_lettings_log(attributes, previous_status) else @logger.error("Log #{lettings_log.old_id}: Failed to import") lettings_log.errors.each do |error| diff --git a/spec/services/imports/lettings_logs_import_service_spec.rb b/spec/services/imports/lettings_logs_import_service_spec.rb index 7850008a1..e3f04b74c 100644 --- a/spec/services/imports/lettings_logs_import_service_spec.rb +++ b/spec/services/imports/lettings_logs_import_service_spec.rb @@ -205,6 +205,81 @@ RSpec.describe Imports::LettingsLogsImportService do end end + context "and it has zero earnings" do + before do + lettings_log_xml.at_xpath("//meta:status").content = "submitted" + lettings_log_xml.at_xpath("//xmlns:Q8Money").content = 0 + end + + it "intercepts the relevant validation error" do + expect(logger).to receive(:warn).with(/Where the income is 0, set earnings and income to blank and set incref to refused/) + expect { lettings_log_service.send(:create_log, lettings_log_xml) } + .not_to raise_error + 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.earnings).to be_nil + expect(lettings_log.incref).to eq(1) + expect(lettings_log.net_income_known).to eq(2) + end + end + + context "and an invalid tenancy length for tenancy type" do + before do + lettings_log_xml.at_xpath("//meta:status").content = "submitted" + lettings_log_xml.at_xpath("//xmlns:_2cYears").content = "1" + lettings_log_xml.at_xpath("//xmlns:Q2b").content = "4" + end + + it "intercepts the relevant validation error" do + expect(logger).to receive(:warn).with(/Removing tenancylength as invalid/) + expect { lettings_log_service.send(:create_log, lettings_log_xml) } + .not_to raise_error + 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.tenancylength).to be_nil + expect(lettings_log.tenancy).to be_nil + end + end + + context "and an lead tenant must be under 20 if childrens home or foster care" do + before do + lettings_log_xml.at_xpath("//meta:status").content = "submitted" + lettings_log_xml.at_xpath("//xmlns:Q11").content = "13" + lettings_log_xml.at_xpath("//xmlns:P1Age").content = "22" + end + + it "intercepts the relevant validation error" do + expect(logger).to receive(:warn).with(/Removing age1 and prevten as incompatible/) + expect { lettings_log_service.send(:create_log, lettings_log_xml) } + .not_to raise_error + 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.age1).to be_nil + expect(lettings_log.prevten).to be_nil + end + end + context "and this is an internal transfer from a non social housing" do before do lettings_log_xml.at_xpath("//xmlns:Q11").content = "9 Residential care home" From e22414cd7397261848412309a5377800e3f1f1c4 Mon Sep 17 00:00:00 2001 From: James Rose Date: Wed, 1 Mar 2023 14:34:49 +0000 Subject: [PATCH 03/14] Allow empty `chcharge` values whilst `is_carehome` is set to true (#1359) - This validation is new to this service. The old CORE did not do it. - A decision was decided to move this to a soft validation in CLDC-2074 - As a temporary fix to allow us to migrate values that are currently incompatible with our validation, we will relax this constraint. --- .../validations/financial_validations.rb | 6 ++-- .../validations/financial_validations_spec.rb | 2 +- .../lettings_logs_import_service_spec.rb | 29 +++++++++++++++++++ 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/app/models/validations/financial_validations.rb b/app/models/validations/financial_validations.rb index 175b0b1bd..6179d05aa 100644 --- a/app/models/validations/financial_validations.rb +++ b/app/models/validations/financial_validations.rb @@ -111,9 +111,11 @@ module Validations::FinancialValidations def validate_care_home_charges(record) if record.is_carehome? period = record.form.get_question("period", record).label_from_value(record.period).downcase + # NOTE: This is a temporary change to allow `ccharge` values despite `is_carehome` being true. This value + # is going to be moved to a soft validation in CLDC-2074, so we can safely do this. if record.chcharge.blank? - record.errors.add :is_carehome, I18n.t("validations.financial.carehome.not_provided", period:) - record.errors.add :chcharge, I18n.t("validations.financial.carehome.not_provided", period:) + # record.errors.add :is_carehome, I18n.t("validations.financial.carehome.not_provided", period:) + # record.errors.add :chcharge, I18n.t("validations.financial.carehome.not_provided", period:) elsif !weekly_value_in_range(record, "chcharge", 10, 1000) max_chcharge = record.weekly_to_value_per_period(1000) min_chcharge = record.weekly_to_value_per_period(10) diff --git a/spec/models/validations/financial_validations_spec.rb b/spec/models/validations/financial_validations_spec.rb index ae7875e08..bdd76cded 100644 --- a/spec/models/validations/financial_validations_spec.rb +++ b/spec/models/validations/financial_validations_spec.rb @@ -990,7 +990,7 @@ RSpec.describe Validations::FinancialValidations do end context "and charges are not provided" do - it "throws and error" do + xit "throws and error" do record.period = 3 record.chcharge = nil financial_validator.validate_care_home_charges(record) diff --git a/spec/services/imports/lettings_logs_import_service_spec.rb b/spec/services/imports/lettings_logs_import_service_spec.rb index e3f04b74c..be650c484 100644 --- a/spec/services/imports/lettings_logs_import_service_spec.rb +++ b/spec/services/imports/lettings_logs_import_service_spec.rb @@ -280,6 +280,35 @@ RSpec.describe Imports::LettingsLogsImportService do end end + context "and is a carehome but missing carehome charge" do + let(:lettings_log_id) { "0b4a68df-30cc-474a-93c0-a56ce8fdad3b" } + + before do + lettings_log_xml.at_xpath("//meta:status").content = "submitted" + lettings_log_xml.at_xpath("//xmlns:_1cmangroupcode").content = scheme2.old_visible_id + scheme2.update!(registered_under_care_act: 2) + lettings_log_xml.at_xpath("//xmlns:Q18b").content = "" + end + + it "intercepts the relevant validation error" do + allow(logger).to receive(:warn) + + expect { lettings_log_service.send(:create_log, lettings_log_xml) } + .not_to raise_error + 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.is_carehome).to be_truthy + expect(lettings_log.chcharge).to be_nil + end + end + context "and this is an internal transfer from a non social housing" do before do lettings_log_xml.at_xpath("//xmlns:Q11").content = "9 Residential care home" From 6db3b06d8f542f8d2e9debe55b7c165d345d3c46 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Wed, 1 Mar 2023 15:10:04 +0000 Subject: [PATCH 04/14] Add don't know options (#1356) * Add don't know options * Add mortgage length known question * Update mortgage known question options * Display fewer answer options * Add don't know undisplayed answer option to mortgage used * Run validations even if mortgageused is don't know * Remove unconfirmed changes * Add a hidden pregblank option to buyers_organisations --- app/models/form/sales/questions/buyer1_mortgage.rb | 12 ++++++++++-- .../form/sales/questions/buyer1_previous_tenure.rb | 4 ++-- app/models/form/sales/questions/buyer2_mortgage.rb | 12 ++++++++++-- .../form/sales/questions/buyers_organisations.rb | 10 ++++++++++ app/models/form/sales/questions/mortgageused.rb | 8 ++++++++ .../sales/sale_information_validations.rb | 2 +- app/models/validations/sales/soft_validations.rb | 2 +- db/migrate/20230301144555_add_pregblank.rb | 5 +++++ db/schema.rb | 3 ++- .../form/sales/questions/buyer1_mortgage_spec.rb | 13 +++++++++++-- .../sales/questions/buyer1_previous_tenure_spec.rb | 5 +++-- .../form/sales/questions/buyer2_mortgage_spec.rb | 13 +++++++++++-- .../sales/questions/buyers_organisations_spec.rb | 12 ++++++++++++ .../form/sales/questions/mortgageused_spec.rb | 9 +++++++++ 14 files changed, 95 insertions(+), 15 deletions(-) create mode 100644 db/migrate/20230301144555_add_pregblank.rb diff --git a/app/models/form/sales/questions/buyer1_mortgage.rb b/app/models/form/sales/questions/buyer1_mortgage.rb index f226f956c..baaf00d4c 100644 --- a/app/models/form/sales/questions/buyer1_mortgage.rb +++ b/app/models/form/sales/questions/buyer1_mortgage.rb @@ -2,8 +2,8 @@ class Form::Sales::Questions::Buyer1Mortgage < ::Form::Question def initialize(id, hsh, page) super @id = "inc1mort" - @check_answer_label = "Buyer 1's income used for mortgage application" - @header = "Was buyer 1's income used for a mortgage application?" + @check_answer_label = "Buyer 1’s income used for mortgage application" + @header = "Was buyer 1’s income used for a mortgage application?" @type = "radio" @answer_options = ANSWER_OPTIONS @check_answers_card_number = 1 @@ -12,5 +12,13 @@ class Form::Sales::Questions::Buyer1Mortgage < ::Form::Question ANSWER_OPTIONS = { "1" => { "value" => "Yes" }, "2" => { "value" => "No" }, + "3" => { "value" => "Don’t know" }, }.freeze + + def displayed_answer_options(_log, _user = nil) + { + "1" => { "value" => "Yes" }, + "2" => { "value" => "No" }, + } + end end diff --git a/app/models/form/sales/questions/buyer1_previous_tenure.rb b/app/models/form/sales/questions/buyer1_previous_tenure.rb index f075e615c..b41fbbabd 100644 --- a/app/models/form/sales/questions/buyer1_previous_tenure.rb +++ b/app/models/form/sales/questions/buyer1_previous_tenure.rb @@ -2,8 +2,8 @@ class Form::Sales::Questions::Buyer1PreviousTenure < ::Form::Question def initialize(id, hsh, page) super @id = "prevten" - @check_answer_label = "Buyer 1's previous tenure" - @header = "What was buyer 1's previous tenure?" + @check_answer_label = "Buyer 1’s previous tenure" + @header = "What was buyer 1’s previous tenure?" @type = "radio" @answer_options = ANSWER_OPTIONS end diff --git a/app/models/form/sales/questions/buyer2_mortgage.rb b/app/models/form/sales/questions/buyer2_mortgage.rb index 5697aea15..884137df1 100644 --- a/app/models/form/sales/questions/buyer2_mortgage.rb +++ b/app/models/form/sales/questions/buyer2_mortgage.rb @@ -2,8 +2,8 @@ class Form::Sales::Questions::Buyer2Mortgage < ::Form::Question def initialize(id, hsh, page) super @id = "inc2mort" - @check_answer_label = "Buyer 2's income used for mortgage application" - @header = "Was buyer 2's income used for a mortgage application?" + @check_answer_label = "Buyer 2’s income used for mortgage application" + @header = "Was buyer 2’s income used for a mortgage application?" @type = "radio" @answer_options = ANSWER_OPTIONS @check_answers_card_number = 2 @@ -12,5 +12,13 @@ class Form::Sales::Questions::Buyer2Mortgage < ::Form::Question ANSWER_OPTIONS = { "1" => { "value" => "Yes" }, "2" => { "value" => "No" }, + "3" => { "value" => "Don’t know" }, }.freeze + + def displayed_answer_options(_log, _user = nil) + { + "1" => { "value" => "Yes" }, + "2" => { "value" => "No" }, + } + end end diff --git a/app/models/form/sales/questions/buyers_organisations.rb b/app/models/form/sales/questions/buyers_organisations.rb index 735a1563d..ec76aebcd 100644 --- a/app/models/form/sales/questions/buyers_organisations.rb +++ b/app/models/form/sales/questions/buyers_organisations.rb @@ -14,8 +14,18 @@ class Form::Sales::Questions::BuyersOrganisations < ::Form::Question "pregother" => { "value" => "Other private registered provider (PRP) - housing association" }, "pregla" => { "value" => "Local Authority" }, "pregghb" => { "value" => "Help to Buy Agent" }, + "pregblank" => { "value" => "None of the above" }, }.freeze + def displayed_answer_options(_log, _user = nil) + { + "pregyrha" => { "value" => "Their private registered provider (PRP) - housing association" }, + "pregother" => { "value" => "Other private registered provider (PRP) - housing association" }, + "pregla" => { "value" => "Local Authority" }, + "pregghb" => { "value" => "Help to Buy Agent" }, + } + end + def unanswered_error_message "At least one option must be selected of these four" end diff --git a/app/models/form/sales/questions/mortgageused.rb b/app/models/form/sales/questions/mortgageused.rb index 8c75750d8..ad18652c4 100644 --- a/app/models/form/sales/questions/mortgageused.rb +++ b/app/models/form/sales/questions/mortgageused.rb @@ -11,5 +11,13 @@ class Form::Sales::Questions::Mortgageused < ::Form::Question ANSWER_OPTIONS = { "1" => { "value" => "Yes" }, "2" => { "value" => "No" }, + "3" => { "value" => "Don’t know" }, }.freeze + + def displayed_answer_options(_log, _user = nil) + { + "1" => { "value" => "Yes" }, + "2" => { "value" => "No" }, + } + end end diff --git a/app/models/validations/sales/sale_information_validations.rb b/app/models/validations/sales/sale_information_validations.rb index c4ce023bb..a1deb94f5 100644 --- a/app/models/validations/sales/sale_information_validations.rb +++ b/app/models/validations/sales/sale_information_validations.rb @@ -46,7 +46,7 @@ module Validations::Sales::SaleInformationValidations def validate_discounted_ownership_value(record) return unless record.value && record.deposit && record.ownershipsch - return unless record.mortgage || record.mortgageused == 2 + return unless record.mortgage || record.mortgageused == 2 || record.mortgageused == 3 return unless record.discount || record.grant || record.type == 29 discount_amount = record.discount ? record.value * record.discount / 100 : 0 diff --git a/app/models/validations/sales/soft_validations.rb b/app/models/validations/sales/soft_validations.rb index c1704d948..a44e0a293 100644 --- a/app/models/validations/sales/soft_validations.rb +++ b/app/models/validations/sales/soft_validations.rb @@ -60,7 +60,7 @@ module Validations::Sales::SoftValidations end def shared_ownership_deposit_invalid? - return unless mortgage || mortgageused == 2 + return unless mortgage || mortgageused == 2 || mortgageused == 3 return unless cashdis || !is_type_discount? return unless deposit && value && equity diff --git a/db/migrate/20230301144555_add_pregblank.rb b/db/migrate/20230301144555_add_pregblank.rb new file mode 100644 index 000000000..308ca5e10 --- /dev/null +++ b/db/migrate/20230301144555_add_pregblank.rb @@ -0,0 +1,5 @@ +class AddPregblank < ActiveRecord::Migration[7.0] + def change + add_column :sales_logs, :pregblank, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index d1668b738..bc7a504a7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_02_15_112932) do +ActiveRecord::Schema[7.0].define(version: 2023_03_01_144555) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -531,6 +531,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_02_15_112932) do t.integer "prevshared" t.integer "staircasesale" t.string "old_id" + t.integer "pregblank" t.index ["bulk_upload_id"], name: "index_sales_logs_on_bulk_upload_id" t.index ["created_by_id"], name: "index_sales_logs_on_created_by_id" t.index ["old_id"], name: "index_sales_logs_on_old_id", unique: true diff --git a/spec/models/form/sales/questions/buyer1_mortgage_spec.rb b/spec/models/form/sales/questions/buyer1_mortgage_spec.rb index 59ae45afc..df47cd1ce 100644 --- a/spec/models/form/sales/questions/buyer1_mortgage_spec.rb +++ b/spec/models/form/sales/questions/buyer1_mortgage_spec.rb @@ -6,6 +6,7 @@ RSpec.describe Form::Sales::Questions::Buyer1Mortgage, type: :model do let(:question_id) { nil } let(:question_definition) { nil } let(:page) { instance_double(Form::Page) } + let(:log) { create(:sales_log) } it "has correct page" do expect(question.page).to eq(page) @@ -16,11 +17,11 @@ RSpec.describe Form::Sales::Questions::Buyer1Mortgage, type: :model do end it "has the correct header" do - expect(question.header).to eq("Was buyer 1's income used for a mortgage application?") + expect(question.header).to eq("Was buyer 1’s income used for a mortgage application?") end it "has the correct check_answer_label" do - expect(question.check_answer_label).to eq("Buyer 1's income used for mortgage application") + expect(question.check_answer_label).to eq("Buyer 1’s income used for mortgage application") end it "has the correct type" do @@ -35,6 +36,14 @@ RSpec.describe Form::Sales::Questions::Buyer1Mortgage, type: :model do expect(question.answer_options).to eq({ "1" => { "value" => "Yes" }, "2" => { "value" => "No" }, + "3" => { "value" => "Don’t know" }, + }) + end + + it "has the correct displayed_answer_options" do + expect(question.displayed_answer_options(log)).to eq({ + "1" => { "value" => "Yes" }, + "2" => { "value" => "No" }, }) end diff --git a/spec/models/form/sales/questions/buyer1_previous_tenure_spec.rb b/spec/models/form/sales/questions/buyer1_previous_tenure_spec.rb index ae1da940a..a63b695a5 100644 --- a/spec/models/form/sales/questions/buyer1_previous_tenure_spec.rb +++ b/spec/models/form/sales/questions/buyer1_previous_tenure_spec.rb @@ -6,6 +6,7 @@ RSpec.describe Form::Sales::Questions::Buyer1PreviousTenure, type: :model do let(:question_id) { nil } let(:question_definition) { nil } let(:page) { instance_double(Form::Page) } + let(:log) { create(:sales_log) } it "has correct page" do expect(question.page).to eq(page) @@ -16,11 +17,11 @@ RSpec.describe Form::Sales::Questions::Buyer1PreviousTenure, type: :model do end it "has the correct header" do - expect(question.header).to eq("What was buyer 1's previous tenure?") + expect(question.header).to eq("What was buyer 1’s previous tenure?") end it "has the correct check_answer_label" do - expect(question.check_answer_label).to eq("Buyer 1's previous tenure") + expect(question.check_answer_label).to eq("Buyer 1’s previous tenure") end it "has the correct type" do diff --git a/spec/models/form/sales/questions/buyer2_mortgage_spec.rb b/spec/models/form/sales/questions/buyer2_mortgage_spec.rb index bb561af43..5bf83a2d0 100644 --- a/spec/models/form/sales/questions/buyer2_mortgage_spec.rb +++ b/spec/models/form/sales/questions/buyer2_mortgage_spec.rb @@ -6,6 +6,7 @@ RSpec.describe Form::Sales::Questions::Buyer2Mortgage, type: :model do let(:question_id) { nil } let(:question_definition) { nil } let(:page) { instance_double(Form::Page) } + let(:log) { create(:sales_log) } it "has correct page" do expect(question.page).to eq(page) @@ -16,11 +17,11 @@ RSpec.describe Form::Sales::Questions::Buyer2Mortgage, type: :model do end it "has the correct header" do - expect(question.header).to eq("Was buyer 2's income used for a mortgage application?") + expect(question.header).to eq("Was buyer 2’s income used for a mortgage application?") end it "has the correct check_answer_label" do - expect(question.check_answer_label).to eq("Buyer 2's income used for mortgage application") + expect(question.check_answer_label).to eq("Buyer 2’s income used for mortgage application") end it "has the correct type" do @@ -35,6 +36,14 @@ RSpec.describe Form::Sales::Questions::Buyer2Mortgage, type: :model do expect(question.answer_options).to eq({ "1" => { "value" => "Yes" }, "2" => { "value" => "No" }, + "3" => { "value" => "Don’t know" }, + }) + end + + it "has the correct displayed_answer_options" do + expect(question.displayed_answer_options(log)).to eq({ + "1" => { "value" => "Yes" }, + "2" => { "value" => "No" }, }) end diff --git a/spec/models/form/sales/questions/buyers_organisations_spec.rb b/spec/models/form/sales/questions/buyers_organisations_spec.rb index d9f61df5f..88af9917d 100644 --- a/spec/models/form/sales/questions/buyers_organisations_spec.rb +++ b/spec/models/form/sales/questions/buyers_organisations_spec.rb @@ -41,6 +41,18 @@ RSpec.describe Form::Sales::Questions::BuyersOrganisations, type: :model do it "has the correct answer_options" do expect(question.answer_options).to eq( + { + "pregyrha" => { "value" => "Their private registered provider (PRP) - housing association" }, + "pregother" => { "value" => "Other private registered provider (PRP) - housing association" }, + "pregla" => { "value" => "Local Authority" }, + "pregghb" => { "value" => "Help to Buy Agent" }, + "pregblank" => { "value" => "None of the above" }, + }, + ) + end + + it "has the correct displayed_answer_options" do + expect(question.displayed_answer_options(FactoryBot.create(:sales_log))).to eq( { "pregyrha" => { "value" => "Their private registered provider (PRP) - housing association" }, "pregother" => { "value" => "Other private registered provider (PRP) - housing association" }, diff --git a/spec/models/form/sales/questions/mortgageused_spec.rb b/spec/models/form/sales/questions/mortgageused_spec.rb index e958de8ea..a31f063c9 100644 --- a/spec/models/form/sales/questions/mortgageused_spec.rb +++ b/spec/models/form/sales/questions/mortgageused_spec.rb @@ -6,6 +6,7 @@ RSpec.describe Form::Sales::Questions::Mortgageused, type: :model do let(:question_id) { nil } let(:question_definition) { nil } let(:page) { instance_double(Form::Page) } + let(:log) { create(:sales_log) } it "has correct page" do expect(question.page).to eq(page) @@ -35,6 +36,7 @@ RSpec.describe Form::Sales::Questions::Mortgageused, type: :model do expect(question.answer_options).to eq({ "1" => { "value" => "Yes" }, "2" => { "value" => "No" }, + "3" => { "value" => "Don’t know" }, }) end @@ -45,4 +47,11 @@ RSpec.describe Form::Sales::Questions::Mortgageused, type: :model do it "has the correct hint" do expect(question.hint_text).to be_nil end + + it "has the correct displayed_answer_options" do + expect(question.displayed_answer_options(log)).to eq({ + "1" => { "value" => "Yes" }, + "2" => { "value" => "No" }, + }) + end end From dd6db911fd2059b01502991a59f342470dc4be3a Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com> Date: Wed, 1 Mar 2023 16:06:10 +0000 Subject: [PATCH 05/14] feat: fix sale range data (#1360) --- config/sale_range_data/2023.csv | 32 ++++++++++++++++---------------- db/schema.rb | 6 +++--- db/seeds.rb | 1 - 3 files changed, 19 insertions(+), 20 deletions(-) diff --git a/config/sale_range_data/2023.csv b/config/sale_range_data/2023.csv index ebe873c07..0106122d8 100644 --- a/config/sale_range_data/2023.csv +++ b/config/sale_range_data/2023.csv @@ -87,10 +87,10 @@ Boston,E07000136,1,58000,258000 Boston,E07000136,2,58000,258000 Boston,E07000136,3,111000,308000 Boston,E07000136,4,212000,510000 -Bournemouth, Christchurch and Poole,E06000058,1,140000,500000 -Bournemouth, Christchurch and Poole,E06000058,2,197000,500000 -Bournemouth, Christchurch and Poole,E06000058,3,285000,657000 -Bournemouth, Christchurch and Poole,E06000058,4,370000,1321000 +"Bournemouth, Christchurch and Poole",E06000058,1,140000,500000 +"Bournemouth, Christchurch and Poole",E06000058,2,197000,500000 +"Bournemouth, Christchurch and Poole",E06000058,3,285000,657000 +"Bournemouth, Christchurch and Poole",E06000058,4,370000,1321000 Bracknell Forest,E06000036,1,95000,488000 Bracknell Forest,E06000036,2,157000,488000 Bracknell Forest,E06000036,3,341000,654000 @@ -119,10 +119,10 @@ Brighton and Hove,E06000043,1,185000,526000 Brighton and Hove,E06000043,2,270000,610000 Brighton and Hove,E06000043,3,350000,846000 Brighton and Hove,E06000043,4,458000,1487000 -Bristol, City of,E06000023,1,145000,417000 -Bristol, City of,E06000023,2,184000,562000 -Bristol, City of,E06000023,3,242000,685000 -Bristol, City of,E06000023,4,331000,1394000 +"Bristol, City of",E06000023,1,145000,417000 +"Bristol, City of",E06000023,2,184000,562000 +"Bristol, City of",E06000023,3,242000,685000 +"Bristol, City of",E06000023,4,331000,1394000 Broadland,E07000144,1,126000,334000 Broadland,E07000144,2,140000,334000 Broadland,E07000144,3,225000,433000 @@ -459,10 +459,10 @@ Havering,E09000016,1,137000,472000 Havering,E09000016,2,204000,481000 Havering,E09000016,3,336000,657000 Havering,E09000016,4,412000,1232000 -Herefordshire, County of,E06000019,1,98000,419000 -Herefordshire, County of,E06000019,2,105000,419000 -Herefordshire, County of,E06000019,3,162000,499000 -Herefordshire, County of,E06000019,4,283000,885000 +"Herefordshire, County of",E06000019,1,98000,419000 +"Herefordshire, County of",E06000019,2,105000,419000 +"Herefordshire, County of",E06000019,3,162000,499000 +"Herefordshire, County of",E06000019,4,283000,885000 Hertsmere,E07000098,1,178000,666000 Hertsmere,E07000098,2,316000,666000 Hertsmere,E07000098,3,440000,918000 @@ -515,10 +515,10 @@ King's Lynn and West Norfolk,E07000146,1,77000,346000 King's Lynn and West Norfolk,E07000146,2,123000,346000 King's Lynn and West Norfolk,E07000146,3,161000,408000 King's Lynn and West Norfolk,E07000146,4,243000,778000 -Kingston upon Hull, City of,E06000010,1,63000,189000 -Kingston upon Hull, City of,E06000010,2,67000,189000 -Kingston upon Hull, City of,E06000010,3,84000,259000 -Kingston upon Hull, City of,E06000010,4,110000,415000 +"Kingston upon Hull, City of",E06000010,1,63000,189000 +"Kingston upon Hull, City of",E06000010,2,67000,189000 +"Kingston upon Hull, City of",E06000010,3,84000,259000 +"Kingston upon Hull, City of",E06000010,4,110000,415000 Kingston upon Thames,E09000021,1,156000,649000 Kingston upon Thames,E09000021,2,325000,708000 Kingston upon Thames,E09000021,3,398000,935000 diff --git a/db/schema.rb b/db/schema.rb index bc7a504a7..2880ecd2d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -522,14 +522,14 @@ ActiveRecord::Schema[7.0].define(version: 2023_03_01_144555) do t.integer "old_persons_shared_ownership_value_check" t.integer "staircase_bought_value_check" t.integer "monthly_charges_value_check" - t.integer "saledate_check" t.integer "details_known_5" t.integer "details_known_6" + t.integer "saledate_check" + t.integer "prevshared" + t.integer "staircasesale" t.integer "ethnic_group2" t.integer "ethnicbuy2" t.integer "proplen_asked" - t.integer "prevshared" - t.integer "staircasesale" t.string "old_id" t.integer "pregblank" t.index ["bulk_upload_id"], name: "index_sales_logs_on_bulk_upload_id" diff --git a/db/seeds.rb b/db/seeds.rb index 10823c62d..341677b27 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -324,6 +324,5 @@ unless Rails.env.test? service.call end end - puts LaSaleRange.count end # rubocop:enable Rails/Output From af1fe815de2d0ce36c818d39a9d3864d664d8c7f Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Wed, 1 Mar 2023 16:28:44 +0000 Subject: [PATCH 06/14] CLDC-2065 Bulk upload error summary minimum limit (#1347) * bulk upload error summary min limit - the errors summary table only show errors for column where there at least 16 errors for that column - if there are fewer than 16 errors that can see detailed report to pin point and fix those specific issues * add #errors? to summary component * show full error report if not enough errors - if errors summary table is empty as below threshold - send users to full report - remove the back button so they cannot access summary report * add upload again cta otherwise a dead end --- ...lk_upload_error_summary_table_component.rb | 13 +++++++ app/mailers/bulk_upload_mailer.rb | 8 ++++- .../show.html.erb | 8 +++-- ...load_error_summary_table_component_spec.rb | 35 +++++++++++++++++++ spec/mailers/bulk_upload_mailer_spec.rb | 4 ++- 5 files changed, 64 insertions(+), 4 deletions(-) diff --git a/app/components/bulk_upload_error_summary_table_component.rb b/app/components/bulk_upload_error_summary_table_component.rb index ac236af4d..a3d295110 100644 --- a/app/components/bulk_upload_error_summary_table_component.rb +++ b/app/components/bulk_upload_error_summary_table_component.rb @@ -1,4 +1,6 @@ class BulkUploadErrorSummaryTableComponent < ViewComponent::Base + DISPLAY_THRESHOLD = 16 + attr_reader :bulk_upload def initialize(bulk_upload:) @@ -11,7 +13,18 @@ class BulkUploadErrorSummaryTableComponent < ViewComponent::Base @sorted_errors ||= bulk_upload .bulk_upload_errors .group(:col, :field, :error) + .having("count(*) > ?", display_threshold) .count .sort_by { |el| el[0][0].rjust(3, "0") } end + + def errors? + sorted_errors.present? + end + +private + + def display_threshold + DISPLAY_THRESHOLD + end end diff --git a/app/mailers/bulk_upload_mailer.rb b/app/mailers/bulk_upload_mailer.rb index be0a71e05..94079828f 100644 --- a/app/mailers/bulk_upload_mailer.rb +++ b/app/mailers/bulk_upload_mailer.rb @@ -46,6 +46,12 @@ class BulkUploadMailer < NotifyMailer def send_correct_and_upload_again_mail(bulk_upload:) error_description = "We noticed that you have a lot of similar errors in column #{columns_with_errors(bulk_upload:)}. Please correct your data export and upload again." + summary_report_link = if BulkUploadErrorSummaryTableComponent.new(bulk_upload:).errors? + summary_bulk_upload_lettings_result_url(bulk_upload) + else + bulk_upload_lettings_result_url(bulk_upload) + end + send_email( bulk_upload.user.email, BULK_UPLOAD_FAILED_CSV_ERRORS_TEMPLATE_ID, @@ -55,7 +61,7 @@ class BulkUploadMailer < NotifyMailer year_combo: bulk_upload.year_combo, lettings_or_sales: bulk_upload.log_type, error_description:, - summary_report_link: summary_bulk_upload_lettings_result_url(bulk_upload), + summary_report_link:, }, ) end diff --git a/app/views/bulk_upload_lettings_results/show.html.erb b/app/views/bulk_upload_lettings_results/show.html.erb index 84374e26a..9a8ccddec 100644 --- a/app/views/bulk_upload_lettings_results/show.html.erb +++ b/app/views/bulk_upload_lettings_results/show.html.erb @@ -1,5 +1,7 @@ -<% content_for :before_content do %> - <%= govuk_back_link(text: "Back", href: summary_bulk_upload_lettings_result_path(@bulk_upload)) %> +<% if BulkUploadErrorSummaryTableComponent.new(bulk_upload: @bulk_upload).errors? %> + <% content_for :before_content do %> + <%= govuk_back_link(text: "Back", href: summary_bulk_upload_lettings_result_path(@bulk_upload)) %> + <% end %> <% end %>
@@ -22,3 +24,5 @@ <% end %>
+ +<%= govuk_button_link_to "Upload your file again", start_bulk_upload_lettings_logs_path %> diff --git a/spec/components/bulk_upload_error_summary_table_component_spec.rb b/spec/components/bulk_upload_error_summary_table_component_spec.rb index cdb0b58bf..a7468d70a 100644 --- a/spec/components/bulk_upload_error_summary_table_component_spec.rb +++ b/spec/components/bulk_upload_error_summary_table_component_spec.rb @@ -5,6 +5,10 @@ RSpec.describe BulkUploadErrorSummaryTableComponent, type: :component do let(:bulk_upload) { create(:bulk_upload) } + before do + stub_const("BulkUploadErrorSummaryTableComponent::DISPLAY_THRESHOLD", 0) + end + context "when no errors" do it "does not renders any rows" do result = render_inline(component) @@ -12,6 +16,19 @@ RSpec.describe BulkUploadErrorSummaryTableComponent, type: :component do end end + context "when below threshold" do + before do + stub_const("BulkUploadErrorSummaryTableComponent::DISPLAY_THRESHOLD", 16) + + create(:bulk_upload_error, bulk_upload:, col: "A", row: 1) + end + + it "does not render rows" do + result = render_inline(component) + expect(result).to have_selector("tbody tr", count: 0) + end + end + context "when there are 2 independent errors" do let!(:error_2) { create(:bulk_upload_error, bulk_upload:, col: "B", row: 2) } let!(:error_1) { create(:bulk_upload_error, bulk_upload:, col: "A", row: 1) } @@ -78,4 +95,22 @@ RSpec.describe BulkUploadErrorSummaryTableComponent, type: :component do ]) end end + + describe "#errors?" do + context "when there are no errors" do + it "returns false" do + expect(component).not_to be_errors + end + end + + context "when there are errors" do + before do + create(:bulk_upload_error, bulk_upload:, col: "A", row: 2, field: "field_1") + end + + it "returns true" do + expect(component).to be_errors + end + end + end end diff --git a/spec/mailers/bulk_upload_mailer_spec.rb b/spec/mailers/bulk_upload_mailer_spec.rb index d0217bfaf..a0042cf69 100644 --- a/spec/mailers/bulk_upload_mailer_spec.rb +++ b/spec/mailers/bulk_upload_mailer_spec.rb @@ -93,7 +93,7 @@ RSpec.describe BulkUploadMailer do year_combo: bulk_upload.year_combo, lettings_or_sales: bulk_upload.log_type, error_description: "We noticed that you have a lot of similar errors in column A, B. Please correct your data export and upload again.", - summary_report_link: "http://localhost:3000/lettings-logs/bulk-upload-results/#{bulk_upload.id}/summary", + summary_report_link: "http://localhost:3000/lettings-logs/bulk-upload-results/#{bulk_upload.id}", }, ) @@ -103,6 +103,8 @@ RSpec.describe BulkUploadMailer do context "when 4 columns with errors" do before do + stub_const("BulkUploadErrorSummaryTableComponent::DISPLAY_THRESHOLD", 0) + create(:bulk_upload_error, bulk_upload:, col: "A") create(:bulk_upload_error, bulk_upload:, col: "B") create(:bulk_upload_error, bulk_upload:, col: "C") From fb6d25b33c233dcaceda0e3506a1a14187e518c9 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Thu, 2 Mar 2023 09:28:39 +0000 Subject: [PATCH 07/14] CLDC-1938 Import relevant logs in sales and lettings log import (#1353) * Only process sales logs in sales import job * Only process lettings logs in lettings import job * Add sales log import task to the full import * change review app import bucket name * Update mortgageused and joinmore * Update jointmore and ownership scheme * Fix ownershipsch so that it doesn't override to nil * Set default relat2, update default household count * het ownership from type if not given * Improve logging * Remove mortgageused method * Look at Q16aProplensec2 column if Q16aProplen2 is empty * Set default income used and pregblank * Remove fields calculated internally * Comment out sales import from full import task * typo and change bucket name --- .../imports/lettings_logs_import_service.rb | 2 + .../imports/sales_logs_import_service.rb | 40 +- lib/tasks/full_import.rake | 1 + .../logs/shared_ownership_sales_log.xml | 333 +++++++++++ .../imports/sales_logs/lettings_log.xml | 524 ++++++++++++++++++ spec/lib/tasks/full_import_spec.rb | 4 + .../lettings_logs_import_service_spec.rb | 6 +- .../imports/sales_logs_import_service_spec.rb | 73 ++- 8 files changed, 970 insertions(+), 13 deletions(-) create mode 100644 spec/fixtures/imports/logs/shared_ownership_sales_log.xml create mode 100644 spec/fixtures/imports/sales_logs/lettings_log.xml diff --git a/app/services/imports/lettings_logs_import_service.rb b/app/services/imports/lettings_logs_import_service.rb index 3aa73db4b..34582091d 100644 --- a/app/services/imports/lettings_logs_import_service.rb +++ b/app/services/imports/lettings_logs_import_service.rb @@ -55,6 +55,8 @@ module Imports }.freeze def create_log(xml_doc) + return if meta_field_value(xml_doc, "form-name").include?("Sales") + attributes = {} previous_status = meta_field_value(xml_doc, "status") diff --git a/app/services/imports/sales_logs_import_service.rb b/app/services/imports/sales_logs_import_service.rb index deea3fab6..b8fa80453 100644 --- a/app/services/imports/sales_logs_import_service.rb +++ b/app/services/imports/sales_logs_import_service.rb @@ -16,6 +16,8 @@ module Imports private def create_log(xml_doc) + return unless meta_field_value(xml_doc, "form-name").include?("Sales") + attributes = {} previous_status = meta_field_value(xml_doc, "status") @@ -31,9 +33,10 @@ module Imports attributes["updated_at"] = Time.zone.parse(meta_field_value(xml_doc, "modified-date")) attributes["purchid"] = string_or_nil(xml_doc, "PurchaserCode") attributes["ownershipsch"] = unsafe_string_as_integer(xml_doc, "Ownership") + attributes["ownershipsch"] = ownership_from_type(attributes) if attributes["ownershipsch"].blank? # sometimes Ownership is missing, but type is set attributes["othtype"] = string_or_nil(xml_doc, "Q38OtherSale") - attributes["jointmore"] = unsafe_string_as_integer(xml_doc, "JointMore") attributes["jointpur"] = unsafe_string_as_integer(xml_doc, "joint") + attributes["jointmore"] = unsafe_string_as_integer(xml_doc, "JointMore") if attributes["jointpur"] == 1 attributes["beds"] = safe_string_as_integer(xml_doc, "Q11Bedrooms") attributes["companybuy"] = unsafe_string_as_integer(xml_doc, "company") if attributes["ownershipsch"] == 3 attributes["hhmemb"] = safe_string_as_integer(xml_doc, "HHMEMB") @@ -102,10 +105,12 @@ module Imports attributes["previous_la_known"] = nil attributes["hhregres"] = unsafe_string_as_integer(xml_doc, "ArmedF") attributes["hhregresstill"] = still_serving(xml_doc) - attributes["proplen"] = safe_string_as_integer(xml_doc, "Q16aProplen2") + attributes["proplen"] = safe_string_as_integer(xml_doc, "Q16aProplen2") || safe_string_as_integer(xml_doc, "Q16aProplensec2") attributes["mscharge"] = monthly_charges(xml_doc, attributes) attributes["mscharge_known"] = 1 if attributes["mscharge"].present? attributes["prevten"] = unsafe_string_as_integer(xml_doc, "Q6PrevTenure") + attributes["mortlen"] = mortgage_length(xml_doc, attributes) + attributes["extrabor"] = borrowing(xml_doc, attributes) attributes["mortgageused"] = unsafe_string_as_integer(xml_doc, "MORTGAGEUSED") attributes["wchair"] = unsafe_string_as_integer(xml_doc, "Q15Wheelchair") attributes["armedforcesspouse"] = unsafe_string_as_integer(xml_doc, "ARMEDFORCESSPOUSE") @@ -117,11 +122,6 @@ module Imports attributes["socprevten"] = unsafe_string_as_integer(xml_doc, "PrevRentType") attributes["mortgagelender"] = mortgage_lender(xml_doc, attributes) attributes["mortgagelenderother"] = mortgage_lender_other(xml_doc, attributes) - attributes["mortlen"] = mortgage_length(xml_doc, attributes) - attributes["extrabor"] = borrowing(xml_doc, attributes) - attributes["totadult"] = safe_string_as_integer(xml_doc, "TOTADULT") # would get overridden - attributes["totchild"] = safe_string_as_integer(xml_doc, "TOTCHILD") # would get overridden - attributes["hhtype"] = unsafe_string_as_integer(xml_doc, "HHTYPE") attributes["pcode1"] = string_or_nil(xml_doc, "PCODE1") attributes["pcode2"] = string_or_nil(xml_doc, "PCODE2") attributes["postcode_full"] = compose_postcode(xml_doc, "PCODE1", "PCODE2") @@ -396,6 +396,17 @@ module Imports end end + def ownership_from_type(attributes) + case attributes["type"] + when 2, 24, 18, 16, 28, 31, 30 + 1 # shared ownership + when 8, 14, 27, 9, 29, 21, 22 + 2 # discounted ownership + when 10, 12 + 3 # outright sale + end + end + def set_default_values(attributes) attributes["armedforcesspouse"] ||= 7 attributes["hhregres"] ||= 8 @@ -404,7 +415,11 @@ module Imports attributes["hb"] ||= 4 attributes["prevown"] ||= 3 attributes["savingsnk"] ||= attributes["savings"].present? ? 0 : 1 - # attributes["noint"] = 1 # not interviewed + attributes["jointmore"] ||= 3 if attributes["jointpur"] == 1 + attributes["inc1mort"] ||= 3 + if [attributes["pregyrha"], attributes["pregla"], attributes["pregghb"], attributes["pregother"]].all?(&:blank?) + attributes["pregblank"] = 1 + end # buyer 1 characteristics attributes["age1_known"] ||= 1 @@ -422,6 +437,8 @@ module Imports attributes["sex2"] ||= "R" attributes["ecstat2"] ||= 10 attributes["income2nk"] ||= attributes["income2"].present? ? 0 : 1 + attributes["relat2"] ||= "R" + attributes["inc2mort"] ||= 3 end # other household members characteristics @@ -434,15 +451,16 @@ module Imports end def missing_answers(sales_log) - applicable_questions = sales_log.form.subsections.map { |s| s.applicable_questions(sales_log) }.flatten + applicable_questions = sales_log.form.subsections.map { |s| s.applicable_questions(sales_log).select { |q| q.enabled?(sales_log) } }.flatten applicable_questions.filter { |q| q.unanswered?(sales_log) }.map(&:id) end - # just for testing, logic might need to change + # just for testing, logic will need to change to match the number of people details known def default_household_count(attributes) return 0 if attributes["hhmemb"].zero? || attributes["hhmemb"].blank? - attributes["jointpur"] == 1 ? attributes["hhmemb"] - 2 : attributes["hhmemb"] - 1 + household_count = attributes["jointpur"] == 1 ? attributes["hhmemb"] - 2 : attributes["hhmemb"] - 1 + household_count.positive? ? household_count : 0 end end end diff --git a/lib/tasks/full_import.rake b/lib/tasks/full_import.rake index e141b6520..35943db2d 100644 --- a/lib/tasks/full_import.rake +++ b/lib/tasks/full_import.rake @@ -18,6 +18,7 @@ namespace :core do Import.new(Imports::DataProtectionConfirmationImportService, :create_data_protection_confirmations, "dataprotect"), Import.new(Imports::OrganisationRentPeriodImportService, :create_organisation_rent_periods, "rent-period"), Import.new(Imports::LettingsLogsImportService, :create_logs, "logs"), + # Import.new(Imports::SalesLogsImportService, :create_logs, "logs"), ] import_list.each do |step| diff --git a/spec/fixtures/imports/logs/shared_ownership_sales_log.xml b/spec/fixtures/imports/logs/shared_ownership_sales_log.xml new file mode 100644 index 000000000..6e0c11174 --- /dev/null +++ b/spec/fixtures/imports/logs/shared_ownership_sales_log.xml @@ -0,0 +1,333 @@ + + + 2022-CORE-Sales + shared_ownership_sales_log + c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa + 7c5bd5fb549c09a2c55d7cb90d7ba84927e64618 + 7c5bd5fb549c09a2c55d7cb90d7ba84927e64618 + 2023-02-21T11:48:28.255968Z + 2023-02-22T11:00:06.575832Z + submitted-valid + 2022 + Manual Entry + + + + + Yes + 2023-01-17 + Shared ownership example + 1 Yes - a shared ownership scheme + 2 Shared Ownership + + + + 2 No + 1 Yes + 2 No + + 2 Yes + + + 2 + 1 Flat or maisonette + 1 Purpose built + SW1A 1AA + Westminster + E09000033 + 3 Don’t know + + + 30 + Male + 1 Full Time - 30 hours or more a week + 2 White: Irish + 18 United Kingdom + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 1 Yes + + + + + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + + + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 76000 + 1 + 47000 + 0 + 235000 + 0 +
300204
+ + + + + + + + + + + + + + + + + + + + + + + + + 1 + 0 + 0 + 1 + 1 + 2 + 3 + 17 + 1 + 2023 + 6 + 9 + 2022 + 8 + 1 + 2023 + SW14 + 7QP + SW1A + 1AA + + + + 2 Private registered provider (PRP) or housing association tenant + SW14 7QP + + Richmond-upon-Thames + E09000027 + Yes + + + + + + + 8 Don’t know + + + 2 No + 2 No + + + 1 Yes + 47000 + 1 Yes + + + + 4 Don’t know + 1 Yes + 89000 + 1 Yes + + + + + 1 + 2 No + + 30 + 2 No + 2023-01-08 + 2022-09-06 + 2 No + + + + 550000 + 30 + 1 Yes + 76000 + Nationwide + 33 + 2 No + 89000 + + 912.00 + 134.24 + + + + + + + + + + + + + + + + + + + + + + + + + 1 + 1 + 0 + + + 3 = 1 adult + + + 2 Shared Ownership + + + 1 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 1 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 1 + 0 + 0 + 0 + 0 + 1 + + + + + E12000007 + 1 + 1 Test + 655 + + diff --git a/spec/fixtures/imports/sales_logs/lettings_log.xml b/spec/fixtures/imports/sales_logs/lettings_log.xml new file mode 100644 index 000000000..1f7f794f6 --- /dev/null +++ b/spec/fixtures/imports/sales_logs/lettings_log.xml @@ -0,0 +1,524 @@ + + + 2021-CORE-SR-SH + lettings_log + c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa + 7c5bd5fb549c09z2c55d9cb90d7ba84927e64618 + 7c5bd5fb549c09z2c55d9cb90d7ba84927e64618 + 2022-01-05T12:50:20.39153Z + 2022-01-05T12:50:20.39153Z + submitted-valid + 2021 + Manual Entry + + + + + Yes + 2021-11-05 +
123456
+ 2 Local Authority + + <_1cmangroupcode>0123 + <_1cschemecode>10 + + 3 No + + + + <_2a>2 No + 1 Secure (inc flexible) + + <_2bTenCode>14044912001 + <_2cYears>2 + + + 72 + + Female + 5) Retired + 1 White: English/Scottish/Welsh/Northern Irish/British + 1 UK national resident in UK + 74 + + Male + Partner + 5) Retired + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 2 No + + + 2 No + + + 9 Not in receipt of either UC or HB + + + 2 Some + Refused + + + + + 13 Property unsuitable because of ill health / disability + + + + <_9b override-field="">2 No + + + + + Yes + + 1 Yes + + + Yes + + + + + + + + 26 Owner occupation (private) + DLUHC + E08000035 + S80 4DJ + + 5 5 years or more + 9 3 years but under 4 years + + + 1 Not homeless + 2 No + + + + + + + + 1 Yes + 1 Yes + 1 Yes + + + 2 Tenant applied direct (no referral or nomination) + + + + 7 Weekly for 48 weeks + 125.00 + + + 7.00 + 132.00 + + + + + 2021-08-24 + + + 0 + 14044912 + + + 1 Yes + 15 First let of newbuild property + + + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 1 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 1 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + <_100>0 + 1 + 1 + 0 + 0 + 0 + 0 + 0 + 0 + <_70>1 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 1 + 0 + 0 + 0 + 0 + 0 + 0 + 1 + 1 + 1 + 2 + 1 + 1 + 0 + 0 + 0 + 0 + 0 + 0 + 2 + 74 + 0 + 0 + 0 + 0 + 0 + 0 + 74 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 72 + 0 + 74 + 2022-01-05Z + 2022-01-20Z + + + + + + + + + + + + + 0 + 0 + 0 + 0 + 20 + 0 + 20 + A + 1 + 1 + 1 + 1 + 4 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + + + 0.00 + 2 + 1 New Tenant + + 73 + 2 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 1 + 1 + 0 + 0 + 0 + 0 + 0 + 0 + 2 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 1 + 0 + 0 + 1 + 2 = 2 Adults at least one is an Elder + + + 15.00 + + + 2 Local Authority + + + E12000004 + 1 + DLUHC + DLUHC + N/A + N/A + + 2 + false + + + + + + + + + + + + + + + 15 + 15 + 000001005048 + D + 15 + 6 + 7 + 1 + 2 + A + P + M + + DLUHC + E08000035 + S80 4QE + + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + + + + + 115.38 + 121.85 + + + 6.46 + + 115.38 + 121.85 + + + 6.46 + + + 1 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 1 + + 115.38 + 0 + + 0 + 117.4 + 10 + + + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 5 + 11 + 2021 + 24 + 8 + 2021 + + + + LS16 + 6FT + LS16 + 6FT + + diff --git a/spec/lib/tasks/full_import_spec.rb b/spec/lib/tasks/full_import_spec.rb index a735db40d..67e8ca715 100644 --- a/spec/lib/tasks/full_import_spec.rb +++ b/spec/lib/tasks/full_import_spec.rb @@ -22,6 +22,7 @@ describe "rake core:full_import", type: :task do context "when starting a full import" do let(:lettings_logs_service) { instance_double(Imports::LettingsLogsImportService) } + let(:sales_logs_service) { instance_double(Imports::SalesLogsImportService) } let(:rent_period_service) { instance_double(Imports::OrganisationRentPeriodImportService) } let(:data_protection_service) { instance_double(Imports::DataProtectionConfirmationImportService) } let(:user_service) { instance_double(Imports::UserImportService) } @@ -37,6 +38,7 @@ describe "rake core:full_import", type: :task do allow(Imports::DataProtectionConfirmationImportService).to receive(:new).and_return(data_protection_service) allow(Imports::OrganisationRentPeriodImportService).to receive(:new).and_return(rent_period_service) allow(Imports::LettingsLogsImportService).to receive(:new).and_return(lettings_logs_service) + allow(Imports::SalesLogsImportService).to receive(:new).and_return(sales_logs_service) end it "raises an exception if no parameters are provided" do @@ -54,6 +56,7 @@ describe "rake core:full_import", type: :task do expect(data_protection_service).to receive(:create_data_protection_confirmations).with("dataprotect") expect(rent_period_service).to receive(:create_organisation_rent_periods).with("rent-period") expect(lettings_logs_service).to receive(:create_logs).with("logs") + # expect(sales_logs_service).to receive(:create_logs).with("logs") task.invoke(fixture_path) end @@ -73,6 +76,7 @@ describe "rake core:full_import", type: :task do expect(data_protection_service).to receive(:create_data_protection_confirmations) expect(rent_period_service).to receive(:create_organisation_rent_periods) expect(lettings_logs_service).to receive(:create_logs) + # expect(sales_logs_service).to receive(:create_logs) expect(scheme_service).not_to receive(:create_schemes) expect(location_service).not_to receive(:create_scheme_locations) diff --git a/spec/services/imports/lettings_logs_import_service_spec.rb b/spec/services/imports/lettings_logs_import_service_spec.rb index be650c484..846c266a5 100644 --- a/spec/services/imports/lettings_logs_import_service_spec.rb +++ b/spec/services/imports/lettings_logs_import_service_spec.rb @@ -47,11 +47,12 @@ RSpec.describe Imports::LettingsLogsImportService do let(:lettings_log_id2) { "166fc004-392e-47a8-acb8-1c018734882b" } let(:lettings_log_id3) { "00d2343e-d5fa-4c89-8400-ec3854b0f2b4" } let(:lettings_log_id4) { "0b4a68df-30cc-474a-93c0-a56ce8fdad3b" } + let(:sales_log) { "shared_ownership_sales_log" } before do # Stub the S3 file listing and download allow(storage_service).to receive(:list_files) - .and_return(%W[#{remote_folder}/#{lettings_log_id}.xml #{remote_folder}/#{lettings_log_id2}.xml #{remote_folder}/#{lettings_log_id3}.xml #{remote_folder}/#{lettings_log_id4}.xml]) + .and_return(%W[#{remote_folder}/#{lettings_log_id}.xml #{remote_folder}/#{lettings_log_id2}.xml #{remote_folder}/#{lettings_log_id3}.xml #{remote_folder}/#{lettings_log_id4}.xml #{remote_folder}/#{sales_log}.xml]) allow(storage_service).to receive(:get_file_io) .with("#{remote_folder}/#{lettings_log_id}.xml") .and_return(open_file(fixture_directory, lettings_log_id), open_file(fixture_directory, lettings_log_id)) @@ -64,6 +65,9 @@ RSpec.describe Imports::LettingsLogsImportService do allow(storage_service).to receive(:get_file_io) .with("#{remote_folder}/#{lettings_log_id4}.xml") .and_return(open_file(fixture_directory, lettings_log_id4), open_file(fixture_directory, lettings_log_id4)) + allow(storage_service).to receive(:get_file_io) + .with("#{remote_folder}/#{sales_log}.xml") + .and_return(open_file(fixture_directory, sales_log), open_file(fixture_directory, sales_log)) end it "successfully create all lettings logs" do diff --git a/spec/services/imports/sales_logs_import_service_spec.rb b/spec/services/imports/sales_logs_import_service_spec.rb index f57e2f3be..bb51b9ad3 100644 --- a/spec/services/imports/sales_logs_import_service_spec.rb +++ b/spec/services/imports/sales_logs_import_service_spec.rb @@ -38,7 +38,7 @@ RSpec.describe Imports::SalesLogsImportService do before do # Stub the S3 file listing and download allow(storage_service).to receive(:list_files) - .and_return(%W[#{remote_folder}/shared_ownership_sales_log.xml #{remote_folder}/shared_ownership_sales_log2.xml #{remote_folder}/outright_sale_sales_log.xml #{remote_folder}/discounted_ownership_sales_log.xml]) + .and_return(%W[#{remote_folder}/shared_ownership_sales_log.xml #{remote_folder}/shared_ownership_sales_log2.xml #{remote_folder}/outright_sale_sales_log.xml #{remote_folder}/discounted_ownership_sales_log.xml #{remote_folder}/lettings_log.xml]) allow(storage_service).to receive(:get_file_io) .with("#{remote_folder}/shared_ownership_sales_log.xml") .and_return(open_file(fixture_directory, "shared_ownership_sales_log"), open_file(fixture_directory, "shared_ownership_sales_log")) @@ -51,6 +51,9 @@ RSpec.describe Imports::SalesLogsImportService do allow(storage_service).to receive(:get_file_io) .with("#{remote_folder}/discounted_ownership_sales_log.xml") .and_return(open_file(fixture_directory, "discounted_ownership_sales_log"), open_file(fixture_directory, "discounted_ownership_sales_log")) + allow(storage_service).to receive(:get_file_io) + .with("#{remote_folder}/lettings_log.xml") + .and_return(open_file(fixture_directory, "lettings_log"), open_file(fixture_directory, "lettings_log")) end it "successfully creates all sales logs" do @@ -575,6 +578,74 @@ RSpec.describe Imports::SalesLogsImportService do expect(sales_log&.hholdcount).to eq(0) end end + + context "when inferring income used" do + let(:sales_log_id) { "discounted_ownership_sales_log" } + + before do + allow(logger).to receive(:warn).and_return(nil) + end + + it "sets inc1mort and inc2mort to don't know if not answered" do + sales_log_xml.at_xpath("//xmlns:joint").content = "1 Yes" + sales_log_xml.at_xpath("//xmlns:Q2Person1Mortgage").content = "" + sales_log_xml.at_xpath("//xmlns:Q2Person2MortApplication").content = "" + sales_log_service.send(:create_log, sales_log_xml) + + sales_log = SalesLog.find_by(old_id: sales_log_id) + expect(sales_log&.inc1mort).to eq(3) + expect(sales_log&.inc2mort).to eq(3) + end + + it "sets inc1mort and inc2mort correctly if answered" do + sales_log_xml.at_xpath("//xmlns:joint").content = "1 Yes" + sales_log_xml.at_xpath("//xmlns:Q2Person1Mortgage").content = "1 Yes" + sales_log_xml.at_xpath("//xmlns:Q2Person2MortApplication").content = "2 No" + sales_log_service.send(:create_log, sales_log_xml) + + sales_log = SalesLog.find_by(old_id: sales_log_id) + expect(sales_log&.inc1mort).to eq(1) + expect(sales_log&.inc2mort).to eq(2) + end + end + + context "when inferring buyer organisation" do + let(:sales_log_id) { "discounted_ownership_sales_log" } + + before do + allow(logger).to receive(:warn).and_return(nil) + end + + it "sets pregblank to true know if no other organisations are selected" do + sales_log_xml.at_xpath("//xmlns:PREGYRHA").content = "" + sales_log_xml.at_xpath("//xmlns:PREGLA").content = "" + sales_log_xml.at_xpath("//xmlns:PREGHBA").content = "" + sales_log_xml.at_xpath("//xmlns:PREGOTHER").content = "" + sales_log_service.send(:create_log, sales_log_xml) + + sales_log = SalesLog.find_by(old_id: sales_log_id) + expect(sales_log&.pregyrha).to eq(nil) + expect(sales_log&.pregla).to eq(nil) + expect(sales_log&.pregghb).to eq(nil) + expect(sales_log&.pregother).to eq(nil) + expect(sales_log&.pregblank).to eq(1) + end + + it "sets pregblank and other organisation fields correctly if answered" do + sales_log_xml.at_xpath("//xmlns:PREGYRHA").content = "Yes" + sales_log_xml.at_xpath("//xmlns:PREGLA").content = "Yes" + sales_log_xml.at_xpath("//xmlns:PREGHBA").content = "Yes" + sales_log_xml.at_xpath("//xmlns:PREGOTHER").content = "Yes" + sales_log_service.send(:create_log, sales_log_xml) + + sales_log = SalesLog.find_by(old_id: sales_log_id) + expect(sales_log&.pregyrha).to eq(1) + expect(sales_log&.pregla).to eq(1) + expect(sales_log&.pregghb).to eq(1) + expect(sales_log&.pregother).to eq(1) + expect(sales_log&.pregblank).to eq(nil) + end + end end end end From 567f2b5ce0fa425db3f90ebb8b2ce106ec12b0b0 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Thu, 2 Mar 2023 09:37:50 +0000 Subject: [PATCH 08/14] CLDC-1847 Add person 6 for non joint purchases (#1297) * Display the correct hint text depending on joint purchase * update validation for non joint purchase * Add person 6 questions and refactor check_answers_card_title * update hint text * Reuse join purchase page --- ...eck_answers_summary_list_card_component.rb | 19 ++------- .../pages/number_of_others_in_property.rb | 10 +++-- .../questions/number_of_others_in_property.rb | 18 +++++++-- .../subsections/household_characteristics.rb | 11 ++++- .../form/headers/_person_6_known_page.erb | 1 + .../number_of_others_in_property_spec.rb | 40 ++++++++++++++++++- .../number_of_others_in_property_spec.rb | 25 +++++++++++- .../household_characteristics_spec.rb | 18 +++++++++ 8 files changed, 114 insertions(+), 28 deletions(-) create mode 100644 app/views/form/headers/_person_6_known_page.erb diff --git a/app/components/check_answers_summary_list_card_component.rb b/app/components/check_answers_summary_list_card_component.rb index af8ebd36a..e8ed971bc 100644 --- a/app/components/check_answers_summary_list_card_component.rb +++ b/app/components/check_answers_summary_list_card_component.rb @@ -18,21 +18,10 @@ class CheckAnswersSummaryListCardComponent < ViewComponent::Base end def check_answers_card_title(question) - if question.form.type == "lettings" - case question.check_answers_card_number - when 1 - "Lead tenant" - when 2..8 - "Person #{question.check_answers_card_number}" - end - else - case question.check_answers_card_number - when 1..number_of_buyers - "Buyer #{question.check_answers_card_number}" - when (number_of_buyers + 1)..(number_of_buyers + 4) - "Person #{question.check_answers_card_number}" - end - end + return "Lead tenant" if question.form.type == "lettings" && question.check_answers_card_number == 1 + return "Buyer #{question.check_answers_card_number}" if question.check_answers_card_number <= number_of_buyers + + "Person #{question.check_answers_card_number}" end private diff --git a/app/models/form/sales/pages/number_of_others_in_property.rb b/app/models/form/sales/pages/number_of_others_in_property.rb index c090422fd..ebf9817bd 100644 --- a/app/models/form/sales/pages/number_of_others_in_property.rb +++ b/app/models/form/sales/pages/number_of_others_in_property.rb @@ -1,20 +1,22 @@ class Form::Sales::Pages::NumberOfOthersInProperty < ::Form::Page - def initialize(id, hsh, subsection) - super - @id = "number_of_others_in_property" + def initialize(id, hsh, subsection, joint_purchase:) + super(id, hsh, subsection) @depends_on = [ { "privacynotice" => 1, + "jointpur" => joint_purchase ? 1 : 2, }, { "noint" => 1, + "jointpur" => joint_purchase ? 1 : 2, }, ] + @joint_purchase = joint_purchase end def questions @questions ||= [ - Form::Sales::Questions::NumberOfOthersInProperty.new(nil, nil, self), + Form::Sales::Questions::NumberOfOthersInProperty.new(nil, nil, self, joint_purchase: @joint_purchase), ] end end diff --git a/app/models/form/sales/questions/number_of_others_in_property.rb b/app/models/form/sales/questions/number_of_others_in_property.rb index cf590291b..97a873430 100644 --- a/app/models/form/sales/questions/number_of_others_in_property.rb +++ b/app/models/form/sales/questions/number_of_others_in_property.rb @@ -1,13 +1,23 @@ class Form::Sales::Questions::NumberOfOthersInProperty < ::Form::Question - def initialize(id, hsh, page) - super + def initialize(id, hsh, page, joint_purchase:) + super(id, hsh, page) @id = "hholdcount" @check_answer_label = "Number of other people living in the property" @header = "Besides the buyer(s), how many other people live or will live in the property?" @type = "numeric" - @hint_text = "You can provide details for a maximum of 4 other people." + @hint_text = hint(joint_purchase) @width = 2 @min = 0 - @max = 4 + @max = joint_purchase ? 4 : 5 + end + +private + + def hint(joint_purchase) + if joint_purchase + "You can provide details for a maximum of 4 other people for a joint purchase." + else + "You can provide details for a maximum of 5 other people if there is only one buyer." + end end end diff --git a/app/models/form/sales/subsections/household_characteristics.rb b/app/models/form/sales/subsections/household_characteristics.rb index 181460ae1..571968c8f 100644 --- a/app/models/form/sales/subsections/household_characteristics.rb +++ b/app/models/form/sales/subsections/household_characteristics.rb @@ -37,7 +37,8 @@ class Form::Sales::Subsections::HouseholdCharacteristics < ::Form::Subsection Form::Sales::Pages::RetirementValueCheck.new("working_situation_2_retirement_value_check_joint_purchase", nil, self, person_index: 2), Form::Sales::Pages::Buyer2IncomeValueCheck.new("working_situation_buyer_2_income_value_check", nil, self), Form::Sales::Pages::Buyer2LiveInProperty.new(nil, nil, self), - Form::Sales::Pages::NumberOfOthersInProperty.new(nil, nil, self), + Form::Sales::Pages::NumberOfOthersInProperty.new("number_of_others_in_property", nil, self, joint_purchase: false), + Form::Sales::Pages::NumberOfOthersInProperty.new("number_of_others_in_property_joint_purchase", nil, self, joint_purchase: true), Form::Sales::Pages::PersonKnown.new("person_2_known", nil, self, person_index: 2), Form::Sales::Pages::PersonRelationshipToBuyer1.new("person_2_relationship_to_buyer_1", nil, self, person_index: 2), Form::Sales::Pages::PersonAge.new("person_2_age", nil, self, person_index: 2), @@ -70,6 +71,14 @@ class Form::Sales::Subsections::HouseholdCharacteristics < ::Form::Subsection Form::Sales::Pages::RetirementValueCheck.new("gender_5_retirement_value_check", nil, self, person_index: 5), Form::Sales::Pages::PersonWorkingSituation.new("person_5_working_situation", nil, self, person_index: 5), Form::Sales::Pages::RetirementValueCheck.new("working_situation_5_retirement_value_check", nil, self, person_index: 5), + Form::Sales::Pages::PersonKnown.new("person_6_known", nil, self, person_index: 6), + Form::Sales::Pages::PersonRelationshipToBuyer1.new("person_6_relationship_to_buyer_1", nil, self, person_index: 6), + Form::Sales::Pages::PersonAge.new("person_6_age", nil, self, person_index: 6), + Form::Sales::Pages::RetirementValueCheck.new("age_6_retirement_value_check", nil, self, person_index: 6), + Form::Sales::Pages::PersonGenderIdentity.new("person_6_gender_identity", nil, self, person_index: 6), + Form::Sales::Pages::RetirementValueCheck.new("gender_6_retirement_value_check", nil, self, person_index: 6), + Form::Sales::Pages::PersonWorkingSituation.new("person_6_working_situation", nil, self, person_index: 6), + Form::Sales::Pages::RetirementValueCheck.new("working_situation_6_retirement_value_check", nil, self, person_index: 6), ].flatten.compact end diff --git a/app/views/form/headers/_person_6_known_page.erb b/app/views/form/headers/_person_6_known_page.erb new file mode 100644 index 000000000..96cc94fd3 --- /dev/null +++ b/app/views/form/headers/_person_6_known_page.erb @@ -0,0 +1 @@ +You have given us the details for <%= log.joint_purchase? ? 3 : 4 %> of the <%= log.hholdcount %> other people in the household diff --git a/spec/models/form/sales/pages/number_of_others_in_property_spec.rb b/spec/models/form/sales/pages/number_of_others_in_property_spec.rb index c2efdcb21..3503a1b9f 100644 --- a/spec/models/form/sales/pages/number_of_others_in_property_spec.rb +++ b/spec/models/form/sales/pages/number_of_others_in_property_spec.rb @@ -1,10 +1,11 @@ require "rails_helper" RSpec.describe Form::Sales::Pages::NumberOfOthersInProperty, type: :model do - subject(:page) { described_class.new(page_id, page_definition, subsection) } + subject(:page) { described_class.new(page_id, page_definition, subsection, joint_purchase:) } - let(:page_id) { nil } + let(:page_id) { "number_of_others_in_property" } let(:page_definition) { nil } + let(:joint_purchase) { false } let(:subsection) { instance_double(Form::Subsection) } it "has correct subsection" do @@ -26,4 +27,39 @@ RSpec.describe Form::Sales::Pages::NumberOfOthersInProperty, type: :model do it "has the correct description" do expect(page.description).to be_nil end + + it "has the correct depends_on" do + expect(page.depends_on).to eq([ + { + "privacynotice" => 1, + "jointpur" => 2, + }, + { + "noint" => 1, + "jointpur" => 2, + }, + ]) + end + + context "with joint purchase" do + let(:page_id) { "number_of_others_in_property_joint_purchase" } + let(:joint_purchase) { true } + + it "has the correct id" do + expect(page.id).to eq("number_of_others_in_property_joint_purchase") + end + + it "has the correct depends_on" do + expect(page.depends_on).to eq([ + { + "privacynotice" => 1, + "jointpur" => 1, + }, + { + "noint" => 1, + "jointpur" => 1, + }, + ]) + end + end end diff --git a/spec/models/form/sales/questions/number_of_others_in_property_spec.rb b/spec/models/form/sales/questions/number_of_others_in_property_spec.rb index 81c49770a..2bb50f80b 100644 --- a/spec/models/form/sales/questions/number_of_others_in_property_spec.rb +++ b/spec/models/form/sales/questions/number_of_others_in_property_spec.rb @@ -1,11 +1,12 @@ require "rails_helper" RSpec.describe Form::Sales::Questions::NumberOfOthersInProperty, type: :model do - subject(:question) { described_class.new(question_id, question_definition, page) } + subject(:question) { described_class.new(question_id, question_definition, page, joint_purchase:) } let(:question_id) { nil } let(:question_definition) { nil } let(:page) { instance_double(Form::Page) } + let(:joint_purchase) { true } it "has correct page" do expect(question.page).to eq(page) @@ -32,6 +33,26 @@ RSpec.describe Form::Sales::Questions::NumberOfOthersInProperty, type: :model do end it "has the correct hint" do - expect(question.hint_text).to eq("You can provide details for a maximum of 4 other people.") + expect(question.hint_text).to eq("You can provide details for a maximum of 4 other people for a joint purchase.") + end + + it "has the correct min" do + expect(question.min).to eq(0) + end + + it "has the correct max" do + expect(question.max).to eq(4) + end + + context "with non joint purchase" do + let(:joint_purchase) { false } + + it "has the correct hint" do + expect(question.hint_text).to eq("You can provide details for a maximum of 5 other people if there is only one buyer.") + end + + it "has the correct max" do + expect(question.max).to eq(5) + end end end diff --git a/spec/models/form/sales/subsections/household_characteristics_spec.rb b/spec/models/form/sales/subsections/household_characteristics_spec.rb index 33e9a1409..4645b7231 100644 --- a/spec/models/form/sales/subsections/household_characteristics_spec.rb +++ b/spec/models/form/sales/subsections/household_characteristics_spec.rb @@ -50,6 +50,7 @@ RSpec.describe Form::Sales::Subsections::HouseholdCharacteristics, type: :model working_situation_buyer_2_income_value_check buyer_2_live_in_property number_of_others_in_property + number_of_others_in_property_joint_purchase person_2_known person_2_relationship_to_buyer_1 person_2_age @@ -82,6 +83,14 @@ RSpec.describe Form::Sales::Subsections::HouseholdCharacteristics, type: :model gender_5_retirement_value_check person_5_working_situation working_situation_5_retirement_value_check + person_6_known + person_6_relationship_to_buyer_1 + person_6_age + age_6_retirement_value_check + person_6_gender_identity + gender_6_retirement_value_check + person_6_working_situation + working_situation_6_retirement_value_check ], ) end @@ -131,6 +140,7 @@ RSpec.describe Form::Sales::Subsections::HouseholdCharacteristics, type: :model working_situation_buyer_2_income_value_check buyer_2_live_in_property number_of_others_in_property + number_of_others_in_property_joint_purchase person_2_known person_2_relationship_to_buyer_1 person_2_age @@ -163,6 +173,14 @@ RSpec.describe Form::Sales::Subsections::HouseholdCharacteristics, type: :model gender_5_retirement_value_check person_5_working_situation working_situation_5_retirement_value_check + person_6_known + person_6_relationship_to_buyer_1 + person_6_age + age_6_retirement_value_check + person_6_gender_identity + gender_6_retirement_value_check + person_6_working_situation + working_situation_6_retirement_value_check ], ) end From cb07cafe2e2bf44425dcad3dd2f8f3a050e1f17c Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Thu, 2 Mar 2023 14:34:40 +0000 Subject: [PATCH 09/14] Make proplen optional for 22/23 collection (#1361) --- app/models/lettings_log.rb | 5 ----- app/models/log.rb | 5 +++++ app/models/sales_log.rb | 16 +++++++++++++++- spec/models/sales_log_spec.rb | 24 ++++++++++++++++++++++++ 4 files changed, 44 insertions(+), 6 deletions(-) diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb index d9fe89509..6864738cf 100644 --- a/app/models/lettings_log.rb +++ b/app/models/lettings_log.rb @@ -65,11 +65,6 @@ class LettingsLog < Log FormHandler.instance.get_form(form_name) || FormHandler.instance.current_lettings_form end - def recalculate_start_year! - @start_year = nil - collection_start_year - end - def lettings? true end diff --git a/app/models/log.rb b/app/models/log.rb index d8aa9e236..505839c69 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -39,6 +39,11 @@ class Log < ApplicationRecord @start_year = startdate < window_end_date ? startdate.year - 1 : startdate.year end + def recalculate_start_year! + @start_year = nil + collection_start_year + end + def lettings? false end diff --git a/app/models/sales_log.rb b/app/models/sales_log.rb index 447f37597..b2c6b08da 100644 --- a/app/models/sales_log.rb +++ b/app/models/sales_log.rb @@ -24,6 +24,7 @@ class SalesLog < Log has_paper_trail validates_with SalesLogValidator + before_validation :recalculate_start_year!, if: :saledate_changed? before_validation :reset_invalidated_dependent_fields! before_validation :process_postcode_changes!, if: :postcode_full_changed? before_validation :process_previous_postcode_changes!, if: :ppostcode_full_changed? @@ -65,7 +66,20 @@ class SalesLog < Log end def optional_fields - OPTIONAL_FIELDS + OPTIONAL_FIELDS + dynamically_not_required + end + + def dynamically_not_required + not_required = [] + not_required << "proplen" if proplen_optional? + + not_required + end + + def proplen_optional? + return false unless collection_start_year + + collection_start_year < 2023 end def not_started? diff --git a/spec/models/sales_log_spec.rb b/spec/models/sales_log_spec.rb index 7b9f7c7ef..ecf04c439 100644 --- a/spec/models/sales_log_spec.rb +++ b/spec/models/sales_log_spec.rb @@ -90,6 +90,30 @@ RSpec.describe SalesLog, type: :model do expect(completed_sales_log.not_started?).to be(false) expect(completed_sales_log.completed?).to be(true) end + + context "when proplen is not given" do + before do + Timecop.freeze(Time.zone.local(2023, 5, 1)) + end + + after do + Timecop.unfreeze + end + + it "is set to completed for a log with a saledate before 23/24" do + completed_sales_log.update!(proplen: nil, saledate: Time.zone.local(2022, 5, 1)) + expect(completed_sales_log.in_progress?).to be(false) + expect(completed_sales_log.not_started?).to be(false) + expect(completed_sales_log.completed?).to be(true) + end + + it "is set to in_progress for a log with a saledate after 23/24" do + completed_sales_log.update!(proplen: nil, saledate: Time.zone.local(2023, 5, 1)) + expect(completed_sales_log.in_progress?).to be(true) + expect(completed_sales_log.not_started?).to be(false) + expect(completed_sales_log.completed?).to be(false) + end + end end context "when filtering by organisation" do From 79f7b9473b32afc67076be0c555e0d66334ec12e Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Thu, 2 Mar 2023 14:45:40 +0000 Subject: [PATCH 10/14] CLDC-1846 Derive hhmemb from hholdcount (#1369) * Derive household members from household count * Calculate hholdcount if not given and calculate hhmemb * Check if economic status is answered --- .../derived_variables/sales_log_variables.rb | 9 +++- .../imports/sales_logs_import_service.rb | 43 +++++++++++++----- .../discounted_ownership_sales_log.xml | 2 +- spec/models/sales_log_spec.rb | 6 +++ .../imports/sales_logs_import_service_spec.rb | 45 +++++++++++++++---- 5 files changed, 83 insertions(+), 22 deletions(-) diff --git a/app/models/derived_variables/sales_log_variables.rb b/app/models/derived_variables/sales_log_variables.rb index 16fb6ebd7..525c12936 100644 --- a/app/models/derived_variables/sales_log_variables.rb +++ b/app/models/derived_variables/sales_log_variables.rb @@ -21,12 +21,19 @@ module DerivedVariables::SalesLogVariables self.pcode1, self.pcode2 = postcode_full.split(" ") if postcode_full.present? self.totchild = total_child self.totadult = total_adult + total_elder - self.hhmemb = totchild + totadult + self.hhmemb = number_of_household_members self.hhtype = household_type end private + def number_of_household_members + return unless hholdcount.present? && jointpur.present? + + number_of_buyers = joint_purchase? ? 2 : 1 + hholdcount + number_of_buyers + end + def total_elder ages = [age1, age2, age3, age4, age5, age6] ages.count { |age| age.present? && age >= 60 } diff --git a/app/services/imports/sales_logs_import_service.rb b/app/services/imports/sales_logs_import_service.rb index b8fa80453..fd749f89d 100644 --- a/app/services/imports/sales_logs_import_service.rb +++ b/app/services/imports/sales_logs_import_service.rb @@ -39,7 +39,8 @@ module Imports attributes["jointmore"] = unsafe_string_as_integer(xml_doc, "JointMore") if attributes["jointpur"] == 1 attributes["beds"] = safe_string_as_integer(xml_doc, "Q11Bedrooms") attributes["companybuy"] = unsafe_string_as_integer(xml_doc, "company") if attributes["ownershipsch"] == 3 - attributes["hhmemb"] = safe_string_as_integer(xml_doc, "HHMEMB") + attributes["hholdcount"] = other_household_members(xml_doc, attributes) + attributes["hhmemb"] = household_members(xml_doc, attributes) (1..6).each do |index| attributes["age#{index}"] = safe_string_as_integer(xml_doc, "P#{index}Age") attributes["sex#{index}"] = sex(xml_doc, index) @@ -62,7 +63,6 @@ module Imports attributes["noint"] = unsafe_string_as_integer(xml_doc, "PartAPurchaser") attributes["buy2livein"] = unsafe_string_as_integer(xml_doc, "LiveInBuyer2") attributes["wheel"] = unsafe_string_as_integer(xml_doc, "Q10Wheelchair") - attributes["hholdcount"] = safe_string_as_integer(xml_doc, "LiveInOther") attributes["la"] = string_or_nil(xml_doc, "Q14ONSLACode") attributes["income1"] = safe_string_as_integer(xml_doc, "Q2Person1Income") attributes["income1nk"] = income_known(unsafe_string_as_integer(xml_doc, "P1IncKnown")) @@ -407,6 +407,36 @@ module Imports end end + def other_household_members(xml_doc, attributes) + hholdcount = safe_string_as_integer(xml_doc, "LiveInOther") + return hholdcount if hholdcount.present? + + other_people_with_details(xml_doc, attributes) + end + + def other_people_with_details(xml_doc, attributes) + number_of_buyers = attributes["jointpur"] == 1 ? 2 : 1 + highest_person_index_with_details = number_of_buyers + + (2..6).each do |person_index| + age = string_or_nil(xml_doc, "P#{person_index}Age") + gender = string_or_nil(xml_doc, "P#{person_index}Sex") + relationship = string_or_nil(xml_doc, "P#{person_index}Rel") + economic_status = string_or_nil(xml_doc, "P#{person_index}Eco") + if gender.present? || age.present? || relationship.present? || economic_status.present? + highest_person_index_with_details = person_index + end + end + + highest_person_index_with_details - number_of_buyers + end + + def household_members(_xml_doc, attributes) + return attributes["hholdcount"] + 2 if attributes["jointpur"] == 1 + + attributes["hholdcount"] + 1 if attributes["jointpur"] == 2 + end + def set_default_values(attributes) attributes["armedforcesspouse"] ||= 7 attributes["hhregres"] ||= 8 @@ -429,7 +459,6 @@ module Imports attributes["national"] ||= 13 attributes["ecstat1"] ||= 10 attributes["income1nk"] ||= attributes["income1"].present? ? 0 : 1 - attributes["hholdcount"] ||= default_household_count(attributes) # just for testing, might need to change # buyer 2 characteristics if attributes["jointpur"] == 1 @@ -454,13 +483,5 @@ module Imports applicable_questions = sales_log.form.subsections.map { |s| s.applicable_questions(sales_log).select { |q| q.enabled?(sales_log) } }.flatten applicable_questions.filter { |q| q.unanswered?(sales_log) }.map(&:id) end - - # just for testing, logic will need to change to match the number of people details known - def default_household_count(attributes) - return 0 if attributes["hhmemb"].zero? || attributes["hhmemb"].blank? - - household_count = attributes["jointpur"] == 1 ? attributes["hhmemb"] - 2 : attributes["hhmemb"] - 1 - household_count.positive? ? household_count : 0 - end end end diff --git a/spec/fixtures/imports/sales_logs/discounted_ownership_sales_log.xml b/spec/fixtures/imports/sales_logs/discounted_ownership_sales_log.xml index bc1d5f274..b697c4960 100644 --- a/spec/fixtures/imports/sales_logs/discounted_ownership_sales_log.xml +++ b/spec/fixtures/imports/sales_logs/discounted_ownership_sales_log.xml @@ -292,7 +292,7 @@ - 0 + 1 0 0 diff --git a/spec/models/sales_log_spec.rb b/spec/models/sales_log_spec.rb index ecf04c439..b82fe6fe3 100644 --- a/spec/models/sales_log_spec.rb +++ b/spec/models/sales_log_spec.rb @@ -307,6 +307,12 @@ RSpec.describe SalesLog, type: :model do expect(record_from_db["hhmemb"]).to eq(6) end + it "correctly derives and saves hhmemb if it's a joint purchase" do + sales_log.update!(jointpur: 2, jointmore: 2) + record_from_db = ActiveRecord::Base.connection.execute("select hhmemb from sales_logs where id=#{sales_log.id}").to_a[0] + expect(record_from_db["hhmemb"]).to eq(5) + end + it "correctly derives and saves totchild" do record_from_db = ActiveRecord::Base.connection.execute("select totchild from sales_logs where id=#{sales_log.id}").to_a[0] expect(record_from_db["totchild"]).to eq(2) diff --git a/spec/services/imports/sales_logs_import_service_spec.rb b/spec/services/imports/sales_logs_import_service_spec.rb index bb51b9ad3..7aa479dd7 100644 --- a/spec/services/imports/sales_logs_import_service_spec.rb +++ b/spec/services/imports/sales_logs_import_service_spec.rb @@ -543,34 +543,61 @@ RSpec.describe Imports::SalesLogsImportService do allow(logger).to receive(:warn).and_return(nil) end - it "sets hholdcount to hhmemb - 1 if not answered and not joint purchase" do - sales_log_xml.at_xpath("//xmlns:HHMEMB").content = "3" + it "sets hholdcount to last person the information is given for if HHMEMB is not set" do sales_log_xml.at_xpath("//xmlns:joint").content = "2 No" - sales_log_xml.at_xpath("//xmlns:LiveInOther").content = "" + sales_log_xml.at_xpath("//xmlns:HHMEMB").content = "" + sales_log_xml.at_xpath("//xmlns:P2Age").content = "20" + sales_log_xml.at_xpath("//xmlns:P3Sex").content = "R" + sales_log_xml.at_xpath("//xmlns:P4Age").content = "23" sales_log_service.send(:create_log, sales_log_xml) sales_log = SalesLog.find_by(old_id: sales_log_id) - expect(sales_log&.hholdcount).to eq(2) + expect(sales_log&.hholdcount).to eq(3) end - it "sets hholdcount to hhmemb - 2 if not answered and joint purchase" do + it "sets hholdcount to last person the information is given for - buyers if HHMEMB is 0" do sales_log_xml.at_xpath("//xmlns:joint").content = "1 Yes" sales_log_xml.at_xpath("//xmlns:JointMore").content = "2 No" - sales_log_xml.at_xpath("//xmlns:HHMEMB").content = "3" - sales_log_xml.at_xpath("//xmlns:LiveInOther").content = "" + sales_log_xml.at_xpath("//xmlns:HHMEMB").content = "" + sales_log_xml.at_xpath("//xmlns:P2Age").content = "20" + sales_log_xml.at_xpath("//xmlns:P3Sex").content = "R" + sales_log_xml.at_xpath("//xmlns:P4Age").content = "23" sales_log_service.send(:create_log, sales_log_xml) sales_log = SalesLog.find_by(old_id: sales_log_id) - expect(sales_log&.hholdcount).to eq(1) + expect(sales_log&.hholdcount).to eq(2) end - it "sets hholdcount to 0 if HHMEMB is 0" do + it "sets hholdcount to 0 no information for people is given and HHMEMB is not set" do sales_log_xml.at_xpath("//xmlns:joint").content = "1 Yes" sales_log_xml.at_xpath("//xmlns:JointMore").content = "2 No" + sales_log_xml.at_xpath("//xmlns:HHMEMB").content = "" + sales_log_xml.at_xpath("//xmlns:LiveInOther").content = "" + sales_log_xml.at_xpath("//xmlns:P2Age").content = "" + sales_log_xml.at_xpath("//xmlns:P2Sex").content = "" + sales_log_xml.at_xpath("//xmlns:P3Age").content = "" + sales_log_xml.at_xpath("//xmlns:P3Sex").content = "" + sales_log_xml.at_xpath("//xmlns:P4Age").content = "" + sales_log_xml.at_xpath("//xmlns:P4Sex").content = "" + + sales_log_service.send(:create_log, sales_log_xml) + + sales_log = SalesLog.find_by(old_id: sales_log_id) + expect(sales_log&.hholdcount).to eq(0) + end + + it "sets hholdcount to the 0 if no information for people is given and HHMEMB is 0" do + sales_log_xml.at_xpath("//xmlns:joint").content = "2 No" sales_log_xml.at_xpath("//xmlns:HHMEMB").content = "0" sales_log_xml.at_xpath("//xmlns:LiveInOther").content = "" + sales_log_xml.at_xpath("//xmlns:P2Age").content = "" + sales_log_xml.at_xpath("//xmlns:P2Sex").content = "" + sales_log_xml.at_xpath("//xmlns:P3Age").content = "" + sales_log_xml.at_xpath("//xmlns:P3Sex").content = "" + sales_log_xml.at_xpath("//xmlns:P4Age").content = "" + sales_log_xml.at_xpath("//xmlns:P4Sex").content = "" sales_log_service.send(:create_log, sales_log_xml) From 7e4d40cd9666a2c8535e6d7c5b3ac9c6f440c5d7 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Thu, 2 Mar 2023 17:08:09 +0000 Subject: [PATCH 11/14] Set default buy2livein (#1371) --- .../imports/sales_logs_import_service.rb | 1 + .../imports/sales_logs_import_service_spec.rb | 76 ++++++++++++++++++- 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/app/services/imports/sales_logs_import_service.rb b/app/services/imports/sales_logs_import_service.rb index fd749f89d..d31b7b1d8 100644 --- a/app/services/imports/sales_logs_import_service.rb +++ b/app/services/imports/sales_logs_import_service.rb @@ -468,6 +468,7 @@ module Imports attributes["income2nk"] ||= attributes["income2"].present? ? 0 : 1 attributes["relat2"] ||= "R" attributes["inc2mort"] ||= 3 + attributes["buy2livein"] ||= 1 unless attributes["ownershipsch"] == 3 end # other household members characteristics diff --git a/spec/services/imports/sales_logs_import_service_spec.rb b/spec/services/imports/sales_logs_import_service_spec.rb index 7aa479dd7..1d2fecdb4 100644 --- a/spec/services/imports/sales_logs_import_service_spec.rb +++ b/spec/services/imports/sales_logs_import_service_spec.rb @@ -643,7 +643,7 @@ RSpec.describe Imports::SalesLogsImportService do allow(logger).to receive(:warn).and_return(nil) end - it "sets pregblank to true know if no other organisations are selected" do + it "sets pregblank to true if no other organisations are selected" do sales_log_xml.at_xpath("//xmlns:PREGYRHA").content = "" sales_log_xml.at_xpath("//xmlns:PREGLA").content = "" sales_log_xml.at_xpath("//xmlns:PREGHBA").content = "" @@ -673,6 +673,80 @@ RSpec.describe Imports::SalesLogsImportService do expect(sales_log&.pregblank).to eq(nil) end end + + context "when setting default buyer 2 live in for discounted ownership" do + let(:sales_log_id) { "discounted_ownership_sales_log" } + + before do + allow(logger).to receive(:warn).and_return(nil) + end + + it "sets buy2livein to true if it is joint purchase and it's not answered" do + sales_log_xml.at_xpath("//xmlns:joint").content = "1 Yes" + sales_log_xml.at_xpath("//xmlns:JointMore").content = "2 No" + sales_log_xml.at_xpath("//xmlns:LiveInBuyer2").content = "" + sales_log_service.send(:create_log, sales_log_xml) + + sales_log = SalesLog.find_by(old_id: sales_log_id) + expect(sales_log&.buy2livein).to eq(1) + end + + it "sets buy2livein correctly if it's answered" do + sales_log_xml.at_xpath("//xmlns:joint").content = "1 Yes" + sales_log_xml.at_xpath("//xmlns:JointMore").content = "2 No" + sales_log_xml.at_xpath("//xmlns:LiveInBuyer2").content = "1 Yes" + sales_log_service.send(:create_log, sales_log_xml) + + sales_log = SalesLog.find_by(old_id: sales_log_id) + expect(sales_log&.buy2livein).to eq(1) + end + end + + context "when setting default buyer 2 live in for shared ownership" do + let(:sales_log_id) { "shared_ownership_sales_log" } + + before do + allow(logger).to receive(:warn).and_return(nil) + end + + it "sets buy2livein to true if it is joint purchase and it's not answered" do + sales_log_xml.at_xpath("//xmlns:joint").content = "1 Yes" + sales_log_xml.at_xpath("//xmlns:JointMore").content = "2 No" + sales_log_xml.at_xpath("//xmlns:LiveInBuyer2").content = "" + sales_log_service.send(:create_log, sales_log_xml) + + sales_log = SalesLog.find_by(old_id: sales_log_id) + expect(sales_log&.buy2livein).to eq(1) + end + + it "sets buy2livein correctly if it's answered" do + sales_log_xml.at_xpath("//xmlns:joint").content = "1 Yes" + sales_log_xml.at_xpath("//xmlns:JointMore").content = "2 No" + sales_log_xml.at_xpath("//xmlns:LiveInBuyer2").content = "2 No" + sales_log_service.send(:create_log, sales_log_xml) + + sales_log = SalesLog.find_by(old_id: sales_log_id) + expect(sales_log&.buy2livein).to eq(2) + end + end + + context "when setting default buyer 2 live in for outright sale" do + let(:sales_log_id) { "outright_sale_sales_log" } + + before do + allow(logger).to receive(:warn).and_return(nil) + end + + it "does not set buy2livein if it is joint purchase and it's not answered" do + sales_log_xml.at_xpath("//xmlns:joint").content = "1 Yes" + sales_log_xml.at_xpath("//xmlns:JointMore").content = "2 No" + sales_log_xml.at_xpath("//xmlns:LiveInBuyer2").content = "" + sales_log_service.send(:create_log, sales_log_xml) + + sales_log = SalesLog.find_by(old_id: sales_log_id) + expect(sales_log&.buy2livein).to eq(nil) + end + end end end end From 246b34a7271e5bb47faaf7b7f7495309814d5693 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Fri, 3 Mar 2023 08:02:28 +0000 Subject: [PATCH 12/14] CLDC-1938 Fix household members import (#1372) * Fix household members * Add test --- app/services/imports/sales_logs_import_service.rb | 8 +++++--- spec/services/imports/sales_logs_import_service_spec.rb | 7 +++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/app/services/imports/sales_logs_import_service.rb b/app/services/imports/sales_logs_import_service.rb index d31b7b1d8..95af610b6 100644 --- a/app/services/imports/sales_logs_import_service.rb +++ b/app/services/imports/sales_logs_import_service.rb @@ -432,9 +432,11 @@ module Imports end def household_members(_xml_doc, attributes) - return attributes["hholdcount"] + 2 if attributes["jointpur"] == 1 - - attributes["hholdcount"] + 1 if attributes["jointpur"] == 2 + if attributes["jointpur"] == 2 + attributes["hholdcount"] + 1 + else + attributes["hholdcount"] + 2 + end end def set_default_values(attributes) diff --git a/spec/services/imports/sales_logs_import_service_spec.rb b/spec/services/imports/sales_logs_import_service_spec.rb index 1d2fecdb4..7fd5de7de 100644 --- a/spec/services/imports/sales_logs_import_service_spec.rb +++ b/spec/services/imports/sales_logs_import_service_spec.rb @@ -604,6 +604,13 @@ RSpec.describe Imports::SalesLogsImportService do sales_log = SalesLog.find_by(old_id: sales_log_id) expect(sales_log&.hholdcount).to eq(0) end + + it "doesn't hang if jointpur is not given" do + sales_log_xml.at_xpath("//xmlns:joint").content = "" + sales_log_xml.at_xpath("//xmlns:HHMEMB").content = "0" + + sales_log_service.send(:create_log, sales_log_xml) + end end context "when inferring income used" do From 5b4df03a4138de5cb2167f78ad24ac407a84440b Mon Sep 17 00:00:00 2001 From: Arthur Campbell <51094020+arfacamble@users.noreply.github.com> Date: Fri, 3 Mar 2023 09:40:49 +0000 Subject: [PATCH 13/14] CLDC-1806 update options for shared ownership type (#1323) * update options for shared ownership type for 23_24 various minor copy changes reordering one new option update tests to reflect this change * alter way of setting question options to reduce future tech debt * add in hyphen to match with the paper form --- .../sales/questions/shared_ownership_type.rb | 35 ++++++++++----- .../sales/pages/shared_ownership_type_spec.rb | 4 +- .../questions/shared_ownership_type_spec.rb | 44 ++++++++++++++----- 3 files changed, 61 insertions(+), 22 deletions(-) diff --git a/app/models/form/sales/questions/shared_ownership_type.rb b/app/models/form/sales/questions/shared_ownership_type.rb index 6351cc185..73b6a4e17 100644 --- a/app/models/form/sales/questions/shared_ownership_type.rb +++ b/app/models/form/sales/questions/shared_ownership_type.rb @@ -6,16 +6,31 @@ class Form::Sales::Questions::SharedOwnershipType < ::Form::Question @header = "What is the type of shared ownership sale?" @hint_text = "A shared ownership sale is when the purchaser buys up to 75% of the property value and pays rent to the Private Registered Provider (PRP) on the remaining portion" @type = "radio" - @answer_options = ANSWER_OPTIONS + @answer_options = answer_options end - ANSWER_OPTIONS = { - "2" => { "value" => "Shared Ownership" }, - "24" => { "value" => "Old Persons Shared Ownership" }, - "18" => { "value" => "Social HomeBuy (shared ownership purchase)" }, - "16" => { "value" => "Home Ownership for people with Long Term Disabilities (HOLD)" }, - "28" => { "value" => "Rent to Buy - Shared Ownership" }, - "31" => { "value" => "Right to Shared Ownership" }, - "30" => { "value" => "Shared Ownership - 2021 model lease" }, - }.freeze + def answer_options + if form.start_date.year >= 2023 + { + "2" => { "value" => "Shared Ownership (old model lease)" }, + "30" => { "value" => "Shared Ownership (new model lease)" }, + "18" => { "value" => "Social HomeBuy — shared ownership purchase" }, + "16" => { "value" => "Home Ownership for people with Long-Term Disabilities (HOLD)" }, + "24" => { "value" => "Older Persons Shared Ownership" }, + "28" => { "value" => "Rent to Buy — Shared Ownership" }, + "31" => { "value" => "Right to Shared Ownership (RtSO)" }, + "32" => { "value" => "London Living Rent — Shared Ownership" }, + } + else + { + "2" => { "value" => "Shared Ownership" }, + "24" => { "value" => "Old Persons Shared Ownership" }, + "18" => { "value" => "Social HomeBuy (shared ownership purchase)" }, + "16" => { "value" => "Home Ownership for people with Long-Term Disabilities (HOLD)" }, + "28" => { "value" => "Rent to Buy - Shared Ownership" }, + "31" => { "value" => "Right to Shared Ownership" }, + "30" => { "value" => "Shared Ownership - 2021 model lease" }, + } + end + end end diff --git a/spec/models/form/sales/pages/shared_ownership_type_spec.rb b/spec/models/form/sales/pages/shared_ownership_type_spec.rb index b8b71080f..09c277766 100644 --- a/spec/models/form/sales/pages/shared_ownership_type_spec.rb +++ b/spec/models/form/sales/pages/shared_ownership_type_spec.rb @@ -5,7 +5,9 @@ RSpec.describe Form::Sales::Pages::SharedOwnershipType, type: :model do let(:page_id) { nil } let(:page_definition) { nil } - let(:subsection) { instance_double(Form::Subsection) } + let(:start_date) { Time.utc(2022, 4, 1) } + let(:form) { instance_double(Form, start_date:) } + let(:subsection) { instance_double(Form::Subsection, form:) } it "has correct subsection" do expect(page.subsection).to eq(subsection) diff --git a/spec/models/form/sales/questions/shared_ownership_type_spec.rb b/spec/models/form/sales/questions/shared_ownership_type_spec.rb index c164c5794..3f60155bf 100644 --- a/spec/models/form/sales/questions/shared_ownership_type_spec.rb +++ b/spec/models/form/sales/questions/shared_ownership_type_spec.rb @@ -5,7 +5,10 @@ RSpec.describe Form::Sales::Questions::SharedOwnershipType, type: :model do let(:question_id) { nil } let(:question_definition) { nil } - let(:page) { instance_double(Form::Page) } + let(:start_date) { Time.utc(2022, 4, 1) } + let(:form) { instance_double(Form, start_date:) } + let(:subsection) { instance_double(Form::Subsection, form:) } + let(:page) { instance_double(Form::Page, subsection:) } it "has correct page" do expect(question.page).to eq(page) @@ -35,15 +38,34 @@ RSpec.describe Form::Sales::Questions::SharedOwnershipType, type: :model do expect(question.hint_text).to eq("A shared ownership sale is when the purchaser buys up to 75% of the property value and pays rent to the Private Registered Provider (PRP) on the remaining portion") end - it "has the correct answer_options" do - expect(question.answer_options).to eq({ - "2" => { "value" => "Shared Ownership" }, - "24" => { "value" => "Old Persons Shared Ownership" }, - "18" => { "value" => "Social HomeBuy (shared ownership purchase)" }, - "16" => { "value" => "Home Ownership for people with Long Term Disabilities (HOLD)" }, - "28" => { "value" => "Rent to Buy - Shared Ownership" }, - "31" => { "value" => "Right to Shared Ownership" }, - "30" => { "value" => "Shared Ownership - 2021 model lease" }, - }) + context "when form start date is 2022" do + it "has the correct answer_options" do + expect(question.answer_options).to eq({ + "2" => { "value" => "Shared Ownership" }, + "24" => { "value" => "Old Persons Shared Ownership" }, + "18" => { "value" => "Social HomeBuy (shared ownership purchase)" }, + "16" => { "value" => "Home Ownership for people with Long-Term Disabilities (HOLD)" }, + "28" => { "value" => "Rent to Buy - Shared Ownership" }, + "31" => { "value" => "Right to Shared Ownership" }, + "30" => { "value" => "Shared Ownership - 2021 model lease" }, + }) + end + end + + context "when form start date is 2023" do + let(:start_date) { Time.utc(2023, 4, 2) } + + it "has the correct answer_options" do + expect(question.answer_options).to eq({ + "2" => { "value" => "Shared Ownership (old model lease)" }, + "30" => { "value" => "Shared Ownership (new model lease)" }, + "18" => { "value" => "Social HomeBuy — shared ownership purchase" }, + "16" => { "value" => "Home Ownership for people with Long-Term Disabilities (HOLD)" }, + "24" => { "value" => "Older Persons Shared Ownership" }, + "28" => { "value" => "Rent to Buy — Shared Ownership" }, + "31" => { "value" => "Right to Shared Ownership (RtSO)" }, + "32" => { "value" => "London Living Rent — Shared Ownership" }, + }) + end end end From 7b43250c565618fe0dbf9e2c66cda7f14faf9d3e Mon Sep 17 00:00:00 2001 From: Arthur Campbell <51094020+arfacamble@users.noreply.github.com> Date: Fri, 3 Mar 2023 09:41:59 +0000 Subject: [PATCH 14/14] CLDC-1850 add don't know option to buyer 1 previous tenure (#1345) * add don't know option to buyer 1 previous tenure * some minor tidying and reformatiing --- .../sales/questions/buyer1_previous_tenure.rb | 25 +++++++++++-------- .../pages/buyer1_previous_tenure_spec.rb | 4 +-- .../questions/buyer1_previous_tenure_spec.rb | 5 ++-- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/app/models/form/sales/questions/buyer1_previous_tenure.rb b/app/models/form/sales/questions/buyer1_previous_tenure.rb index b41fbbabd..f3482a03f 100644 --- a/app/models/form/sales/questions/buyer1_previous_tenure.rb +++ b/app/models/form/sales/questions/buyer1_previous_tenure.rb @@ -5,17 +5,20 @@ class Form::Sales::Questions::Buyer1PreviousTenure < ::Form::Question @check_answer_label = "Buyer 1’s previous tenure" @header = "What was buyer 1’s previous tenure?" @type = "radio" - @answer_options = ANSWER_OPTIONS + @answer_options = answer_options end - ANSWER_OPTIONS = { - "1" => { "value" => "Local Authority" }, - "2" => { "value" => "Private registered provider or housing association tenant" }, - "3" => { "value" => "Private tenant" }, - "5" => { "value" => "Owner occupier" }, - "4" => { "value" => "Tied home or renting with job" }, - "6" => { "value" => "Living with family or friends" }, - "7" => { "value" => "Temporary accomodation" }, - "9" => { "value" => "Other" }, - }.freeze + def answer_options + { + "1" => { "value" => "Local Authority" }, + "2" => { "value" => "Private registered provider or housing association tenant" }, + "3" => { "value" => "Private tenant" }, + "5" => { "value" => "Owner occupier" }, + "4" => { "value" => "Tied home or renting with job" }, + "6" => { "value" => "Living with family or friends" }, + "7" => { "value" => "Temporary accomodation" }, + "9" => { "value" => "Other" }, + "0" => { "value" => "Don’t know" }, + } + end end diff --git a/spec/models/form/sales/pages/buyer1_previous_tenure_spec.rb b/spec/models/form/sales/pages/buyer1_previous_tenure_spec.rb index e38f287f5..a67fa3e16 100644 --- a/spec/models/form/sales/pages/buyer1_previous_tenure_spec.rb +++ b/spec/models/form/sales/pages/buyer1_previous_tenure_spec.rb @@ -1,10 +1,8 @@ require "rails_helper" RSpec.describe Form::Sales::Pages::Buyer1PreviousTenure, type: :model do - subject(:page) { described_class.new(page_id, page_definition, subsection) } + subject(:page) { described_class.new(nil, nil, subsection) } - let(:page_id) { nil } - let(:page_definition) { nil } let(:subsection) { instance_double(Form::Subsection) } it "has correct subsection" do diff --git a/spec/models/form/sales/questions/buyer1_previous_tenure_spec.rb b/spec/models/form/sales/questions/buyer1_previous_tenure_spec.rb index a63b695a5..4321700b4 100644 --- a/spec/models/form/sales/questions/buyer1_previous_tenure_spec.rb +++ b/spec/models/form/sales/questions/buyer1_previous_tenure_spec.rb @@ -1,10 +1,8 @@ require "rails_helper" RSpec.describe Form::Sales::Questions::Buyer1PreviousTenure, type: :model do - subject(:question) { described_class.new(question_id, question_definition, page) } + subject(:question) { described_class.new(nil, nil, page) } - let(:question_id) { nil } - let(:question_definition) { nil } let(:page) { instance_double(Form::Page) } let(:log) { create(:sales_log) } @@ -42,6 +40,7 @@ RSpec.describe Form::Sales::Questions::Buyer1PreviousTenure, type: :model do "6" => { "value" => "Living with family or friends" }, "7" => { "value" => "Temporary accomodation" }, "9" => { "value" => "Other" }, + "0" => { "value" => "Don’t know" }, }) end end