From 9e9ff8a4e2311c52f3cfbd1176638555741c2cba Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com> Date: Fri, 20 Jan 2023 09:30:24 +0000 Subject: [PATCH] CLDC-850 Add relationship to buyer 1 validation to sales log (#1178) * feat: add sales partner count validation * test: update tests * test: add new test * refactor: DRYing * feat: DRY general household member validation * feat: add tests * db: update * feat: update shared validations * test: update tests * feat: add more validations * feat: add more validation and update tests * feat: add update method test * test: update sales_log factory * refactor: linting * feat: remove numbering * test: update tests * feat: rename i18n vars, add validations everywhere * tests: add new tests for new validation occurences * db: update * feat: add partner number validation to relevant fields rather than base * test: update tests * tests: refactor * test: update --- .../buyer2_relationship_to_buyer1.rb | 2 +- .../validations/household_validations.rb | 19 ++---- .../sales/household_validations.rb | 55 ++++++++++++++++ app/models/validations/shared_validations.rb | 15 +++++ config/locales/en.yml | 15 +++-- db/schema.rb | 10 +-- spec/factories/sales_log.rb | 12 ++-- .../buyer2_relationship_to_buyer1_spec.rb | 2 +- spec/models/sales_log_spec.rb | 13 ++++ .../validations/household_validations_spec.rb | 12 ++-- .../sales/household_validations_spec.rb | 65 +++++++++++++++++++ 11 files changed, 184 insertions(+), 36 deletions(-) diff --git a/app/models/form/sales/questions/buyer2_relationship_to_buyer1.rb b/app/models/form/sales/questions/buyer2_relationship_to_buyer1.rb index a37605332..5a2b51ba7 100644 --- a/app/models/form/sales/questions/buyer2_relationship_to_buyer1.rb +++ b/app/models/form/sales/questions/buyer2_relationship_to_buyer1.rb @@ -10,7 +10,7 @@ class Form::Sales::Questions::Buyer2RelationshipToBuyer1 < ::Form::Question end ANSWER_OPTIONS = { - "P" => { "value" => "Parent" }, + "P" => { "value" => "Partner" }, "C" => { "value" => "Child", "hint" => "Must be eligible for child benefit, aged under 16 or under 20 if still in full-time education." }, "X" => { "value" => "Other" }, "R" => { "value" => "Buyer prefers not to say" }, diff --git a/app/models/validations/household_validations.rb b/app/models/validations/household_validations.rb index 7053d3571..4ffd0b1f8 100644 --- a/app/models/validations/household_validations.rb +++ b/app/models/validations/household_validations.rb @@ -47,7 +47,7 @@ module Validations::HouseholdValidations validate_person_age_matches_relationship(record, n) validate_person_age_and_relationship_matches_economic_status(record, n) end - validate_partner_count(record) + shared_validate_partner_count(record, 8) end def validate_person_1_economic(record) @@ -177,16 +177,9 @@ private return unless age && economic_status && relationship if age >= 16 && age <= 19 && tenant_is_child?(relationship) && (!tenant_is_fulltime_student?(economic_status) && !tenant_economic_status_refused?(economic_status)) - record.errors.add "ecstat#{person_num}", I18n.t("validations.household.ecstat.student_16_19", person_num:) - record.errors.add "age#{person_num}", I18n.t("validations.household.age.student_16_19", person_num:) - record.errors.add "relat#{person_num}", I18n.t("validations.household.relat.student_16_19", person_num:) - end - end - - def validate_partner_count(record) - partner_count = (2..8).count { |n| tenant_is_partner?(record["relat#{n}"]) } - if partner_count > 1 - record.errors.add :base, I18n.t("validations.household.relat.one_partner") + record.errors.add "ecstat#{person_num}", I18n.t("validations.household.ecstat.not_student_16_19", person_num:) + record.errors.add "age#{person_num}", I18n.t("validations.household.age.not_student_16_19", person_num:) + record.errors.add "relat#{person_num}", I18n.t("validations.household.relat.not_student_16_19", person_num:) end end @@ -202,10 +195,6 @@ private economic_status == 10 end - def tenant_is_partner?(relationship) - relationship == "P" - end - def tenant_is_child?(relationship) relationship == "C" end diff --git a/app/models/validations/sales/household_validations.rb b/app/models/validations/sales/household_validations.rb index 940c3d6c7..385659c9a 100644 --- a/app/models/validations/sales/household_validations.rb +++ b/app/models/validations/sales/household_validations.rb @@ -1,4 +1,6 @@ module Validations::Sales::HouseholdValidations + include Validations::SharedValidations + def validate_number_of_other_people_living_in_the_property(record) return if record.hholdcount.blank? @@ -6,4 +8,57 @@ module Validations::Sales::HouseholdValidations record.errors.add :hholdcount, I18n.t("validations.numeric.valid", field: "Number of other people living in the property", min: 0, max: 4) end end + + def validate_household_number_of_other_members(record) + (2..6).each do |n| + validate_person_age_matches_relationship(record, n) + validate_person_age_and_relationship_matches_economic_status(record, n) + end + shared_validate_partner_count(record, 6) + end + +private + + def validate_person_age_matches_relationship(record, person_num) + age = record.public_send("age#{person_num}") + relationship = record.public_send("relat#{person_num}") + return unless age && relationship + + if age < 16 && person_is_partner?(relationship) + record.errors.add "age#{person_num}", I18n.t("validations.household.age.partner_under_16") + record.errors.add "relat#{person_num}", I18n.t("validations.household.relat.partner_under_16") + elsif age >= 20 && person_is_child?(relationship) + record.errors.add "age#{person_num}", I18n.t("validations.household.age.child_over_20") + record.errors.add "relat#{person_num}", I18n.t("validations.household.relat.child_over_20") + end + end + + def validate_person_age_and_relationship_matches_economic_status(record, person_num) + age = record.public_send("age#{person_num}") + economic_status = record.public_send("ecstat#{person_num}") + relationship = record.public_send("relat#{person_num}") + return unless age && economic_status && relationship + + if age >= 16 && age <= 19 && person_is_fulltime_student?(economic_status) && !person_is_child?(relationship) + record.errors.add "age#{person_num}", I18n.t("validations.household.age.student_16_19") + record.errors.add "ecstat#{person_num}", I18n.t("validations.household.ecstat.student_16_19") + record.errors.add "relat#{person_num}", I18n.t("validations.household.relat.student_16_19") + end + end + + def person_is_partner?(relationship) + relationship == "P" + end + + def person_is_fulltime_student?(economic_status) + economic_status == 7 + end + + def person_economic_status_refused?(economic_status) + economic_status == 10 + end + + def person_is_child?(relationship) + relationship == "C" + end end diff --git a/app/models/validations/shared_validations.rb b/app/models/validations/shared_validations.rb index d8d4e2059..93a81e938 100644 --- a/app/models/validations/shared_validations.rb +++ b/app/models/validations/shared_validations.rb @@ -89,4 +89,19 @@ module Validations::SharedValidations record.errors.add(question_id, I18n.t("validations.invalid_option", question: question.check_answer_label&.downcase)) end end + + def shared_validate_partner_count(record, max_people) + partner_numbers = (2..max_people).select { |n| person_is_partner?(record["relat#{n}"]) } + if partner_numbers.count > 1 + partner_numbers.each do |n| + record.errors.add "relat#{n}", I18n.t("validations.household.relat.one_partner") + end + end + end + +private + + def person_is_partner?(relationship) + relationship == "P" + end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 788b3a710..d67864d6d 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -295,20 +295,27 @@ en: child_under_16_relat: "Answer cannot be under 16 as person %{person_num} is not a child of the lead tenant" child_under_16: "Answer cannot be under 16 as person’s %{person_num} working situation is not ‘child under 16’" child_over_16: "Answer cannot be over 16 as person’s %{person_num} working situation is ‘child under 16‘" - student_16_19: "Answer cannot be between 16 and 19 as person %{person_num} is a child of the lead tenant but is not a full-time student" + child_over_20: "Answer cannot be 20 or over as the relationship is ‘child’" + not_student_16_19: "Answer cannot be between 16 and 19 as person %{person_num} is a child of the lead tenant but is not a full-time student" + student_16_19: "Person cannot be aged 16-19 if they are a student but don't have relationship ‘child’" + partner_under_16: "Cannot be under 16 if the relationship is partner" lead: over_20: "The lead tenant must be under 20 as you told us their housing situation immediately before this letting was a children’s home or foster care" ecstat: retired_over_70: "Person %{person_num} must be retired if over 70" child_under_16: "Person’s %{person_num} working situation must be ’child under 16‘ as you told us they’re under 16" child_over_16: "Answer cannot be ‘child under 16’ as you told us the person %{person_num} is older than 16" - student_16_19: "Person’s %{person_num} working situation must be full-time student or prefers not to say as you told us they’re between 16 and 19." + not_student_16_19: "Person’s %{person_num} working situation must be full-time student or prefers not to say as you told us they’re between 16 and 19." + student_16_19: "Person cannot be a student if they are aged 16-19 and but don't have relationship ‘child’" retired_male: "Answer cannot be ‘retired’ as the male tenant is under 65" retired_female: "Answer cannot be ‘retired’ as the female tenant is under 60" relat: - child_under_16: "Person’s %{person_num}’s relationship to tenant 1 must be ‘child’ as you told us they’re under 16" + partner_under_16: "Answer cannot be ‘partner’ if the person's age is under 16" + child_under_16: "Person’s relationship to tenant 1 must be ‘child’ as you told us they’re under 16" + child_over_20: "Answer cannot be ‘child’ if the person's age is 20 or over" one_partner: "Number of partners cannot be greater than 1" - student_16_19: "Answer cannot be ‘child’ as you told us the person %{person_num} is between 16 and 19 and is not a full-time student" + not_student_16_19: "Answer cannot be ‘child’ as you told us the person %{person_num} is between 16 and 19 and is not a full-time student" + student_16_19: "Answer must be ‘child’ if the person is aged 16-19 and a student" housingneeds_a: one_or_two_choices: "You can only select one option or ‘other disabled access needs’ plus ‘wheelchair-accessible housing’, ‘wheelchair access to essential rooms’ or ‘level access housing’" prevten: diff --git a/db/schema.rb b/db/schema.rb index 5b8e58122..c133a5935 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -488,16 +488,16 @@ ActiveRecord::Schema[7.0].define(version: 2023_01_13_125117) do t.integer "mortgagelender" t.string "mortgagelenderother" t.integer "mortlen" - t.string "pcode1" - t.string "pcode2" - t.integer "pcodenk" - t.string "postcode_full" - t.boolean "is_la_inferred" t.integer "extrabor" t.integer "hhmemb" t.integer "totadult" t.integer "totchild" t.integer "hhtype" + t.string "pcode1" + t.string "pcode2" + t.integer "pcodenk" + t.string "postcode_full" + t.boolean "is_la_inferred" t.index ["created_by_id"], name: "index_sales_logs_on_created_by_id" t.index ["managing_organisation_id"], name: "index_sales_logs_on_managing_organisation_id" t.index ["owning_organisation_id"], name: "index_sales_logs_on_owning_organisation_id" diff --git a/spec/factories/sales_log.rb b/spec/factories/sales_log.rb index cac259bc3..166f43e30 100644 --- a/spec/factories/sales_log.rb +++ b/spec/factories/sales_log.rb @@ -41,10 +41,10 @@ FactoryBot.define do wheel { 1 } details_known_1 { 1 } age3_known { 0 } - age3 { 40 } + age3 { 14 } details_known_2 { 1 } age4_known { 0 } - age4 { 40 } + age4 { 18 } details_known_3 { 1 } age5_known { 0 } age5 { 40 } @@ -85,10 +85,10 @@ FactoryBot.define do ppcodenk { 1 } prevten { 1 } previous_la_known { 0 } - relat3 { "P" } - relat4 { "P" } - relat5 { "P" } - relat6 { "P" } + relat3 { "C" } + relat4 { "X" } + relat5 { "R" } + relat6 { "R" } hb { 4 } mortgageused { 1 } wchair { 1 } diff --git a/spec/models/form/sales/questions/buyer2_relationship_to_buyer1_spec.rb b/spec/models/form/sales/questions/buyer2_relationship_to_buyer1_spec.rb index 059e5c084..d3d680e18 100644 --- a/spec/models/form/sales/questions/buyer2_relationship_to_buyer1_spec.rb +++ b/spec/models/form/sales/questions/buyer2_relationship_to_buyer1_spec.rb @@ -37,7 +37,7 @@ RSpec.describe Form::Sales::Questions::Buyer2RelationshipToBuyer1, type: :model it "has the correct answer_options" do expect(question.answer_options).to eq({ - "P" => { "value" => "Parent" }, + "P" => { "value" => "Partner" }, "C" => { "value" => "Child", "hint" => "Must be eligible for child benefit, aged under 16 or under 20 if still in full-time education." }, "X" => { "value" => "Other" }, "R" => { "value" => "Buyer prefers not to say" }, diff --git a/spec/models/sales_log_spec.rb b/spec/models/sales_log_spec.rb index 2b7e43331..3720dba56 100644 --- a/spec/models/sales_log_spec.rb +++ b/spec/models/sales_log_spec.rb @@ -30,6 +30,19 @@ RSpec.describe SalesLog, type: :model do end end + describe "#update" do + let(:sales_log) { FactoryBot.create(:sales_log, created_by: created_by_user) } + let(:validator) { sales_log._validators[nil].first } + + after do + sales_log.update(age1: 25) + end + + it "validates other household member details" do + expect(validator).to receive(:validate_household_number_of_other_members) + end + end + describe "#optional_fields" do let(:sales_log) { build(:sales_log) } diff --git a/spec/models/validations/household_validations_spec.rb b/spec/models/validations/household_validations_spec.rb index e5b1aacab..0d1be05ae 100644 --- a/spec/models/validations/household_validations_spec.rb +++ b/spec/models/validations/household_validations_spec.rb @@ -332,8 +332,12 @@ RSpec.describe Validations::HouseholdValidations do record.relat2 = "P" record.relat3 = "P" household_validator.validate_household_number_of_other_members(record) - expect(record.errors["base"]) + expect(record.errors["relat2"]) .to include(match I18n.t("validations.household.relat.one_partner")) + expect(record.errors["relat3"]) + .to include(match I18n.t("validations.household.relat.one_partner")) + expect(record.errors["relat4"]) + .not_to include(match I18n.t("validations.household.relat.one_partner")) end it "expects that a tenant can have a partner" do @@ -398,11 +402,11 @@ RSpec.describe Validations::HouseholdValidations do record.ecstat2 = 1 household_validator.validate_household_number_of_other_members(record) expect(record.errors["ecstat2"]) - .to include(match I18n.t("validations.household.ecstat.student_16_19", person_num: 2)) + .to include(match I18n.t("validations.household.ecstat.not_student_16_19", person_num: 2)) expect(record.errors["age2"]) - .to include(match I18n.t("validations.household.age.student_16_19", person_num: 2)) + .to include(match I18n.t("validations.household.age.not_student_16_19", person_num: 2)) expect(record.errors["relat2"]) - .to include(match I18n.t("validations.household.relat.student_16_19", person_num: 2)) + .to include(match I18n.t("validations.household.relat.not_student_16_19", person_num: 2)) end it "expects that person can be a full time student" do diff --git a/spec/models/validations/sales/household_validations_spec.rb b/spec/models/validations/sales/household_validations_spec.rb index b7ea4f663..b20ac15f4 100644 --- a/spec/models/validations/sales/household_validations_spec.rb +++ b/spec/models/validations/sales/household_validations_spec.rb @@ -46,4 +46,69 @@ RSpec.describe Validations::Sales::HouseholdValidations do end end end + + describe "household member validations" do + let(:record) { build(:sales_log) } + + it "validates that only 1 partner exists" do + record.relat2 = "P" + record.relat3 = "P" + household_validator.validate_household_number_of_other_members(record) + expect(record.errors["relat2"]) + .to include(match I18n.t("validations.household.relat.one_partner")) + expect(record.errors["relat3"]) + .to include(match I18n.t("validations.household.relat.one_partner")) + expect(record.errors["relat4"]) + .not_to include(match I18n.t("validations.household.relat.one_partner")) + end + + it "expects that a tenant can have a partner" do + record.relat3 = "P" + household_validator.validate_household_number_of_other_members(record) + expect(record.errors["base"]).to be_empty + end + + context "when the household contains a person under 16" do + it "expects that person is a child of the tenant" do + record.age2 = 14 + record.relat2 = "C" + household_validator.validate_household_number_of_other_members(record) + expect(record.errors["relat2"]).to be_empty + expect(record.errors["age2"]).to be_empty + end + + it "validates that a person under 16 must not be a partner of the buyer" do + record.age2 = 14 + record.relat2 = "P" + household_validator.validate_household_number_of_other_members(record) + expect(record.errors["relat2"]) + .to include(match I18n.t("validations.household.relat.partner_under_16")) + expect(record.errors["age2"]) + .to include(match I18n.t("validations.household.age.partner_under_16")) + end + end + + it "validates that a person over 20 must not be a child of the buyer" do + record.age2 = 21 + record.relat2 = "C" + household_validator.validate_household_number_of_other_members(record) + expect(record.errors["relat2"]) + .to include(match I18n.t("validations.household.relat.child_over_20")) + expect(record.errors["age2"]) + .to include(match I18n.t("validations.household.age.child_over_20")) + end + + it "validates that a person aged 16-19 who is a student must be a child of the buyer" do + record.age2 = 18 + record.ecstat2 = "7" + record.relat2 = "P" + household_validator.validate_household_number_of_other_members(record) + expect(record.errors["relat2"]) + .to include(match I18n.t("validations.household.relat.student_16_19")) + expect(record.errors["age2"]) + .to include(match I18n.t("validations.household.age.student_16_19")) + expect(record.errors["ecstat2"]) + .to include(match I18n.t("validations.household.ecstat.student_16_19")) + end + end end