From 971c3909eefb3d97b4f4190f4860e19719700ad0 Mon Sep 17 00:00:00 2001 From: Matthew Phelan Date: Thu, 28 Oct 2021 15:36:11 +0100 Subject: [PATCH 1/5] validate_other_household_members --- app/models/case_log.rb | 75 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 2862eb087..2cb25397d 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -117,6 +117,81 @@ class CaseLogValidator < ActiveModel::Validator validate_other_field(record, "tenancy_type", "other_tenancy_type") end + def validate_other_household_members(record) + index = 0 + number_of_other_members = record.household_number_of_other_members + partner = false + + while index < number_of_other_members + + member_number = index+2 + relationship = record["person_#{member_number}_relationship"] + age = record["person_#{member_number}_age"] + gender = record["person_#{member_number}_gender"] + economic_status = record["person_#{member_number}_economic_status"] + + binding.pry + if relationship || age || gender || economic_status + if relationship.nil? || age.nil? || gender.nil? || economic_status.nil? + record.errors.add "person_#{member_number}_age", "If any of the person is filled out it must all be filled" + end + end + + if age<1 || age>120 + record.errors.add "person_#{member_number}_age", "Tenant #{member_number} age must be between 1 and 120 (i.e. infants must be entered as 1)" + end + + if age>70 && economic_status != "Retired" + record.errors.add "person_#{member_number}_economic_status", "Tenant #{member_number} must be retired if over 70" + end + + if gender=="Male" && economic_status == "Retired" && age<65 + record.errors.add "person_#{member_number}_age", "Male tenant who is retired must be 65 or over" + end + + if gender=="Female" && economic_status == "Retired" && age<60 + record.errors.add "person_#{member_number}_age", "Female tenant who is retired must be 60 or over" + end + + if age>70 && economic_status != "Retired" + record.errors.add "person_#{member_number}_economic_status", "Tenant #{member_number} must be retired if over 70" + end + + if age<16 + if relationship != "Child - includes young adult and grown-up" + record.errors.add "person_#{member_number}_relationship", "Tenant #{member_number}'s relationship to tenant 1 must be Child if their age is under 16" + end + if economic_status != "Child under 16" + record.errors.add "person_#{member_number}_economic_status", "Tenant #{member_number} economic status must be Child under 16 if their age is under 16" + end + end + + if relationship == "Partner" + if partner + record.errors.add "person_#{member_number}_relationship", "Tenant can not have multiple partners" + elsif age<16 || economic_status == "Child under 16" + record.errors.add "person_#{member_number}_relationship", "Tenant can not be tenant 1's partner if they are under 16" + else + partner = true + end + end + + if relationship == "Child - includes young adult and grown-up" + if economic_status!="Unable to work because of long term sick or disability" || economic_status!="Other" || economic_status!="Prefer not to say" + record.errors.add "person_#{member_number}_economic_status", "This is not a valid economic status for a child" + end + + if age>=16 && age<=19 + if economic_status != "Full-time student" || economic_status != "Prefer not to say" + record.errors.add "person_#{member_number}_economic_status", "If relationship is child and age is between 16 and 19 - tenant #{member_number} must be a full time student or prefer not to say." + end + end + end + + index = index+1 + end + end + def validate(record) # If we've come from the form UI we only want to validate the specific fields # that have just been submitted. If we're submitting a log via API or Bulk Upload From c3e1f07797d892c8a0ad7d128b3b20e5b66585a3 Mon Sep 17 00:00:00 2001 From: Matthew Phelan Date: Fri, 29 Oct 2021 11:17:21 +0100 Subject: [PATCH 2/5] validate_other_household_members refactor Co-authored-by: Daniel Baark --- app/models/case_log.rb | 87 +++--------------------- app/validations/household_validations.rb | 74 ++++++++++++++++++-- spec/models/case_log_spec.rb | 6 ++ 3 files changed, 83 insertions(+), 84 deletions(-) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 0123a06de..9e23699e1 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -5,90 +5,13 @@ class CaseLogValidator < ActiveModel::Validator include FinancialValidations include TenancyValidations - def validate_other_household_members(record) - index = 0 - number_of_other_members = record.household_number_of_other_members - partner = false - - while index < number_of_other_members - - member_number = index+2 - relationship = record["person_#{member_number}_relationship"] - age = record["person_#{member_number}_age"] - gender = record["person_#{member_number}_gender"] - economic_status = record["person_#{member_number}_economic_status"] - - binding.pry - if relationship || age || gender || economic_status - if relationship.nil? || age.nil? || gender.nil? || economic_status.nil? - record.errors.add "person_#{member_number}_age", "If any of the person is filled out it must all be filled" - end - end - - if age<1 || age>120 - record.errors.add "person_#{member_number}_age", "Tenant #{member_number} age must be between 1 and 120 (i.e. infants must be entered as 1)" - end - - if age>70 && economic_status != "Retired" - record.errors.add "person_#{member_number}_economic_status", "Tenant #{member_number} must be retired if over 70" - end - - if gender=="Male" && economic_status == "Retired" && age<65 - record.errors.add "person_#{member_number}_age", "Male tenant who is retired must be 65 or over" - end - - if gender=="Female" && economic_status == "Retired" && age<60 - record.errors.add "person_#{member_number}_age", "Female tenant who is retired must be 60 or over" - end - - if age>70 && economic_status != "Retired" - record.errors.add "person_#{member_number}_economic_status", "Tenant #{member_number} must be retired if over 70" - end - - if age<16 - if relationship != "Child - includes young adult and grown-up" - record.errors.add "person_#{member_number}_relationship", "Tenant #{member_number}'s relationship to tenant 1 must be Child if their age is under 16" - end - if economic_status != "Child under 16" - record.errors.add "person_#{member_number}_economic_status", "Tenant #{member_number} economic status must be Child under 16 if their age is under 16" - end - end - - if relationship == "Partner" - if partner - record.errors.add "person_#{member_number}_relationship", "Tenant can not have multiple partners" - elsif age<16 || economic_status == "Child under 16" - record.errors.add "person_#{member_number}_relationship", "Tenant can not be tenant 1's partner if they are under 16" - else - partner = true - end - end - - if relationship == "Child - includes young adult and grown-up" - if economic_status!="Unable to work because of long term sick or disability" || economic_status!="Other" || economic_status!="Prefer not to say" - record.errors.add "person_#{member_number}_economic_status", "This is not a valid economic status for a child" - end - - if age>=16 && age<=19 - if economic_status != "Full-time student" || economic_status != "Prefer not to say" - record.errors.add "person_#{member_number}_economic_status", "If relationship is child and age is between 16 and 19 - tenant #{member_number} must be a full time student or prefer not to say." - end - end - end - - index = index+1 - end - end - def validate(record) # If we've come from the form UI we only want to validate the specific fields # that have just been submitted. If we're submitting a log via API or Bulk Upload # we want to validate all data fields. question_to_validate = options[:previous_page] if question_to_validate - if respond_to?("validate_#{question_to_validate}") - public_send("validate_#{question_to_validate}", record) - end + public_send("validate_#{question_to_validate}", record) if respond_to?("validate_#{question_to_validate}") else validation_methods = public_methods.select { |method| method.starts_with?("validate_") } validation_methods.each { |meth| public_send(meth, record) } @@ -209,6 +132,14 @@ private dynamically_not_required << "net_income" dynamically_not_required << "net_income_frequency" end + + start_range = (household_number_of_other_members || 0) + 2 + (start_range..8).each do |n| + dynamically_not_required << "person_#{n}_age" + dynamically_not_required << "person_#{n}_gender" + dynamically_not_required << "person_#{n}_relationship" + dynamically_not_required << "person_#{n}_economic_status" + end required.delete_if { |key, _value| dynamically_not_required.include?(key) } end diff --git a/app/validations/household_validations.rb b/app/validations/household_validations.rb index 8ff4e684a..e0e17d97d 100644 --- a/app/validations/household_validations.rb +++ b/app/validations/household_validations.rb @@ -1,11 +1,5 @@ module HouseholdValidations # Validations methods need to be called 'validate_' to run on model save - def validate_person_1_age(record) - if record.person_1_age && !/^[1-9][0-9]?$|^120$/.match?(record.person_1_age.to_s) - record.errors.add :person_1_age, "Tenant age must be between 0 and 120" - end - end - def validate_reasonable_preference(record) if record.homelessness == "No" && record.reasonable_preference == "Yes" record.errors.add :reasonable_preference, "Can not be Yes if Not Homeless immediately prior to this letting has been selected" @@ -56,6 +50,16 @@ module HouseholdValidations end end + def validate_other_household_members(record) + (1..8).each do |n| + validate_person_age(record, n) + validate_person_age_matches_economic_status(record, n) + validate_person_age_matches_relationship(record, n) if n > 1 + validate_person_age_and_gender_match_economic_status(record, n) + end + validate_partner_count(record) + end + private def women_of_child_bearing_age_in_household(record) @@ -65,4 +69,62 @@ private record["person_#{n}_gender"] == "Female" && record["person_#{n}_age"] >= 16 && record["person_#{n}_age"] <= 50 end end + + def validate_person_age(record, person_num) + age = record.public_send("person_#{person_num}_age") + return unless age + + if !age.is_a?(Integer) || age < 1 || age > 120 + record.errors.add "person_#{person_num}_age".to_sym, "Tenant age must be an integer between 0 and 120" + end + end + + def validate_person_age_matches_economic_status(record, person_num) + age = record.public_send("person_#{person_num}_age") + economic_status = record.public_send("person_#{person_num}_economic_status") + return unless age && economic_status + + if age > 70 && economic_status != "Retired" + record.errors.add "person_#{person_num}_economic_status", "Tenant #{person_num} must be retired if over 70" + end + if age < 16 && economic_status != "Child under 16" + record.errors.add "person_#{person_num}_economic_status", "Tenant #{person_num} economic status must be Child under 16 if their age is under 16" + end + if age >= 16 && age <= 19 && (economic_status != "Full-time student" || economic_status != "Prefer not to say") + record.errors.add "person_#{person_num}_economic_status", "If age is between 16 and 19 - tenant #{person_num} must be a full time student or prefer not to say." + end + end + + def validate_person_age_matches_relationship(record, person_num) + age = record.public_send("person_#{person_num}_age") + relationship = record.public_send("person_#{person_num}_relationship") + return unless age && relationship + + if age < 16 && relationship != "Child - includes young adult and grown-up" + record.errors.add "person_#{person_num}_relationship", "Tenant #{person_num}'s relationship to tenant 1 must be Child if their age is under 16" + end + end + + def validate_person_age_and_gender_match_economic_status(record, person_num) + age = record.public_send("person_#{person_num}_age") + gender = record.public_send("person_#{person_num}_gender") + economic_status = record.public_send("person_#{person_num}_economic_status") + return unless age && economic_status && gender + + + if gender == "Male" && economic_status == "Retired" && age < 65 + record.errors.add "person_#{person_num}_age", "Male tenant who is retired must be 65 or over" + end + if gender == "Female" && economic_status == "Retired" && age < 60 + record.errors.add "person_#{person_num}_age", "Female tenant who is retired must be 60 or over" + end + end + + def validate_partner_count(record) + # TODO probably need to keep track of which specific field is wrong so we can highlight it in the UI + partner_count = (2..8).map { |n| record.public_send("person_#{n}_relationship") }.uniq.count + if partner_count > 1 + record.errors.add :base, "Number of partners cannot be greater than 1" + end + end end diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index 5e42ebce9..9c39c3a1a 100644 --- a/spec/models/case_log_spec.rb +++ b/spec/models/case_log_spec.rb @@ -206,6 +206,12 @@ RSpec.describe Form, type: :model do end end + context "household_member_validations" do + # it "validate that persons aged under 16 must have relationship Child" do + # expect { CaseLog.create!(person_2_age: 14, person_2_relationship: "Partner") }.to raise_error(ActiveRecord::RecordInvalid) + # end + end + context "other tenancy type validation" do it "must be provided if tenancy type was given as other" do expect { From 656c7bda2637d07375dfb56fef9a702c1114f2c9 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Fri, 29 Oct 2021 11:41:45 +0100 Subject: [PATCH 3/5] Specs passing --- app/models/case_log.rb | 5 +++-- app/validations/financial_validations.rb | 3 ++- app/validations/household_validations.rb | 23 ++++++++++++++--------- app/validations/property_validations.rb | 5 +++-- app/validations/tenancy_validations.rb | 3 ++- docs/api/DLUHC-CORE-Data.v1.json | 4 ++-- spec/requests/case_log_controller_spec.rb | 4 ++-- 7 files changed, 28 insertions(+), 19 deletions(-) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 9e23699e1..e9afa7807 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -1,5 +1,6 @@ class CaseLogValidator < ActiveModel::Validator - # Validations methods need to be called 'validate_' to run on model save + # Validations methods need to be called 'validate_' to run on model save + # or 'validate_' to run on submit as well include HouseholdValidations include PropertyValidations include FinancialValidations @@ -132,7 +133,7 @@ private dynamically_not_required << "net_income" dynamically_not_required << "net_income_frequency" end - + start_range = (household_number_of_other_members || 0) + 2 (start_range..8).each do |n| dynamically_not_required << "person_#{n}_age" diff --git a/app/validations/financial_validations.rb b/app/validations/financial_validations.rb index 90093e134..3567e334d 100644 --- a/app/validations/financial_validations.rb +++ b/app/validations/financial_validations.rb @@ -1,5 +1,6 @@ module FinancialValidations - # Validations methods need to be called 'validate_' to run on model save + # Validations methods need to be called 'validate_' to run on model save + # or 'validate_' to run on submit as well def validate_outstanding_rent_amount(record) if record.outstanding_rent_or_charges == "Yes" && record.outstanding_amount.blank? record.errors.add :outstanding_amount, "You must answer the oustanding amout question if you have outstanding rent or charges." diff --git a/app/validations/household_validations.rb b/app/validations/household_validations.rb index e0e17d97d..d458b7607 100644 --- a/app/validations/household_validations.rb +++ b/app/validations/household_validations.rb @@ -1,5 +1,6 @@ module HouseholdValidations - # Validations methods need to be called 'validate_' to run on model save + # Validations methods need to be called 'validate_' to run on model save + # or 'validate_' to run on submit as well def validate_reasonable_preference(record) if record.homelessness == "No" && record.reasonable_preference == "Yes" record.errors.add :reasonable_preference, "Can not be Yes if Not Homeless immediately prior to this letting has been selected" @@ -50,16 +51,20 @@ module HouseholdValidations end end - def validate_other_household_members(record) - (1..8).each do |n| + def validate_household_number_of_other_members(record) + (2..8).each do |n| validate_person_age(record, n) validate_person_age_matches_economic_status(record, n) - validate_person_age_matches_relationship(record, n) if n > 1 + validate_person_age_matches_relationship(record, n) validate_person_age_and_gender_match_economic_status(record, n) end validate_partner_count(record) end + def validate_person_1_age(record) + validate_person_age(record, 1) + end + private def women_of_child_bearing_age_in_household(record) @@ -72,8 +77,8 @@ private def validate_person_age(record, person_num) age = record.public_send("person_#{person_num}_age") - return unless age - + return unless age + if !age.is_a?(Integer) || age < 1 || age > 120 record.errors.add "person_#{person_num}_age".to_sym, "Tenant age must be an integer between 0 and 120" end @@ -98,7 +103,7 @@ private def validate_person_age_matches_relationship(record, person_num) age = record.public_send("person_#{person_num}_age") relationship = record.public_send("person_#{person_num}_relationship") - return unless age && relationship + return unless age && relationship if age < 16 && relationship != "Child - includes young adult and grown-up" record.errors.add "person_#{person_num}_relationship", "Tenant #{person_num}'s relationship to tenant 1 must be Child if their age is under 16" @@ -122,9 +127,9 @@ private def validate_partner_count(record) # TODO probably need to keep track of which specific field is wrong so we can highlight it in the UI - partner_count = (2..8).map { |n| record.public_send("person_#{n}_relationship") }.uniq.count + partner_count = (2..8).select { |n| record.public_send("person_#{n}_relationship") == "Partner" }.count if partner_count > 1 - record.errors.add :base, "Number of partners cannot be greater than 1" + record.errors.add :base, "Number of partners cannot be greater than 1" end end end diff --git a/app/validations/property_validations.rb b/app/validations/property_validations.rb index 9d40a2ee5..65dde4a08 100644 --- a/app/validations/property_validations.rb +++ b/app/validations/property_validations.rb @@ -1,8 +1,9 @@ module PropertyValidations - # Validations methods need to be called 'validate_' to run on model save + # Validations methods need to be called 'validate_' to run on model save + # or 'validate_' to run on submit as well def validate_property_number_of_times_relet(record) if record.property_number_of_times_relet && !/^[1-9]$|^0[1-9]$|^1[0-9]$|^20$/.match?(record.property_number_of_times_relet.to_s) - record.errors.add :property_number_of_times_relet, "Must be between 0 and 20" + record.errors.add :property_number_of_times_relet, "Property number of times relet must be between 0 and 20" end end end diff --git a/app/validations/tenancy_validations.rb b/app/validations/tenancy_validations.rb index 063b4672c..5738bc2a3 100644 --- a/app/validations/tenancy_validations.rb +++ b/app/validations/tenancy_validations.rb @@ -1,5 +1,6 @@ module TenancyValidations - # Validations methods need to be called 'validate_' to run on model save + # Validations methods need to be called 'validate_' to run on model save + # or 'validate_' to run on submit as well def validate_fixed_term_tenancy(record) is_present = record.fixed_term_tenancy.present? is_in_range = record.fixed_term_tenancy.to_i.between?(2, 99) diff --git a/docs/api/DLUHC-CORE-Data.v1.json b/docs/api/DLUHC-CORE-Data.v1.json index 3820d9aee..fed09b9d6 100644 --- a/docs/api/DLUHC-CORE-Data.v1.json +++ b/docs/api/DLUHC-CORE-Data.v1.json @@ -110,7 +110,7 @@ "value": { "errors": { "person_1_age": [ - "Tenant age must be between 0 and 120" + "Tenant age must be an integer between 0 and 120" ] } } @@ -220,7 +220,7 @@ "If reasonable preference is Yes, a reason must be given" ], "person_1_age": [ - "Tenant age must be between 0 and 120" + "Tenant age must be an integer between 0 and 120" ] } } diff --git a/spec/requests/case_log_controller_spec.rb b/spec/requests/case_log_controller_spec.rb index 6ddbd0130..50a2250ed 100644 --- a/spec/requests/case_log_controller_spec.rb +++ b/spec/requests/case_log_controller_spec.rb @@ -66,7 +66,7 @@ RSpec.describe CaseLogsController, type: :request do it "validates case log parameters" do json_response = JSON.parse(response.body) expect(response).to have_http_status(:unprocessable_entity) - expect(json_response["errors"]).to match_array([["property_number_of_times_relet", ["Must be between 0 and 20"]], ["person_1_age", ["Tenant age must be between 0 and 120"]]]) + expect(json_response["errors"]).to match_array([["property_number_of_times_relet", ["Property number of times relet must be between 0 and 20"]], ["person_1_age", ["Tenant age must be an integer between 0 and 120"]]]) end end @@ -165,7 +165,7 @@ RSpec.describe CaseLogsController, type: :request do it "returns an error message" do json_response = JSON.parse(response.body) - expect(json_response["errors"]).to eq({ "person_1_age" => ["Tenant age must be between 0 and 120"] }) + expect(json_response["errors"]).to eq({ "person_1_age" => ["Tenant age must be an integer between 0 and 120"] }) end end From d240e299bd27f363fba936196ab52b618babdeb2 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Fri, 29 Oct 2021 11:56:57 +0100 Subject: [PATCH 4/5] Add some test for our validations --- app/validations/household_validations.rb | 15 ++++++++-- spec/models/case_log_spec.rb | 36 +++++++++++++++++++++--- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/app/validations/household_validations.rb b/app/validations/household_validations.rb index d458b7607..347b87c59 100644 --- a/app/validations/household_validations.rb +++ b/app/validations/household_validations.rb @@ -57,6 +57,7 @@ module HouseholdValidations validate_person_age_matches_economic_status(record, n) validate_person_age_matches_relationship(record, n) validate_person_age_and_gender_match_economic_status(record, n) + validate_person_age_and_relationship_matches_economic_status(record, n) end validate_partner_count(record) end @@ -95,9 +96,6 @@ private if age < 16 && economic_status != "Child under 16" record.errors.add "person_#{person_num}_economic_status", "Tenant #{person_num} economic status must be Child under 16 if their age is under 16" end - if age >= 16 && age <= 19 && (economic_status != "Full-time student" || economic_status != "Prefer not to say") - record.errors.add "person_#{person_num}_economic_status", "If age is between 16 and 19 - tenant #{person_num} must be a full time student or prefer not to say." - end end def validate_person_age_matches_relationship(record, person_num) @@ -110,6 +108,17 @@ private end end + def validate_person_age_and_relationship_matches_economic_status(record, person_num) + age = record.public_send("person_#{person_num}_age") + economic_status = record.public_send("person_#{person_num}_economic_status") + relationship = record.public_send("person_#{person_num}_relationship") + return unless age && economic_status && relationship + + if age >= 16 && age <= 19 && relationship == "Child - includes young adult and grown-up" && (economic_status != "Full-time student" || economic_status != "Prefer not to say") + record.errors.add "person_#{person_num}_economic_status", "If age is between 16 and 19 - tenant #{person_num} must be a full time student or prefer not to say." + end + end + def validate_person_age_and_gender_match_economic_status(record, person_num) age = record.public_send("person_#{person_num}_age") gender = record.public_send("person_#{person_num}_gender") diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index 9c39c3a1a..80f45e64a 100644 --- a/spec/models/case_log_spec.rb +++ b/spec/models/case_log_spec.rb @@ -206,10 +206,38 @@ RSpec.describe Form, type: :model do end end - context "household_member_validations" do - # it "validate that persons aged under 16 must have relationship Child" do - # expect { CaseLog.create!(person_2_age: 14, person_2_relationship: "Partner") }.to raise_error(ActiveRecord::RecordInvalid) - # end + context "household_member_validations" do + it "validate that persons aged under 16 must have relationship Child" do + expect { CaseLog.create!(person_2_age: 14, person_2_relationship: "Partner") }.to raise_error(ActiveRecord::RecordInvalid) + end + + it "validate that persons aged over 70 must be retired" do + expect { CaseLog.create!(person_2_age: 71, person_2_economic_status: "Full-time - 30 hours or more") }.to raise_error(ActiveRecord::RecordInvalid) + end + + it "validate that a male, retired persons must be over 65" do + expect { CaseLog.create!(person_2_age: 64, person_2_gender: "Male", person_2_economic_status: "Retired") }.to raise_error(ActiveRecord::RecordInvalid) + end + + it "validate that a female, retired persons must be over 60" do + expect { CaseLog.create!(person_2_age: 59, person_2_gender: "Female", person_2_economic_status: "Retired") }.to raise_error(ActiveRecord::RecordInvalid) + end + + it "validate that persons aged under 16 must be a child (economically speaking)" do + expect { CaseLog.create!(person_2_age: 15, person_2_economic_status: "Full-time - 30 hours or more") }.to raise_error(ActiveRecord::RecordInvalid) + end + + it "validate that persons aged between 16 and 19 that are a child must be a full time student or economic status refused" do + expect { CaseLog.create!(person_2_age: 17, person_2_relationship: "Child - includes young adult and grown-up", person_2_economic_status: "Full-time - 30 hours or more") }.to raise_error(ActiveRecord::RecordInvalid) + end + + it "validate that persons aged under 16 must be a child relationship" do + expect { CaseLog.create!(person_2_age: 15, person_2_relationship: "Partner") }.to raise_error(ActiveRecord::RecordInvalid) + end + + it "validate that no more than 1 partner relationship exists" do + expect { CaseLog.create!(person_2_relationship: "Partner", person_3_relationship: "Partner") }.to raise_error(ActiveRecord::RecordInvalid) + end end context "other tenancy type validation" do From ba9cdd31a2799bfcb663602f1afce3014615d393 Mon Sep 17 00:00:00 2001 From: Matthew Phelan Date: Tue, 2 Nov 2021 09:00:35 +0000 Subject: [PATCH 5/5] Code review updates --- app/validations/household_validations.rb | 9 ++++++++- docs/api/DLUHC-CORE-Data.v1.json | 4 ++-- spec/factories/case_log.rb | 2 +- spec/features/case_log_spec.rb | 2 +- spec/requests/case_log_controller_spec.rb | 4 ++-- 5 files changed, 14 insertions(+), 7 deletions(-) diff --git a/app/validations/household_validations.rb b/app/validations/household_validations.rb index 347b87c59..45e881a4f 100644 --- a/app/validations/household_validations.rb +++ b/app/validations/household_validations.rb @@ -63,7 +63,14 @@ module HouseholdValidations end def validate_person_1_age(record) - validate_person_age(record, 1) + return unless record.person_1_age + if !record.person_1_age.is_a?(Integer) || record.person_1_age < 16 || record.person_1_age > 120 + record.errors.add "person_1_age", "Tenant age must be an integer between 16 and 120" + end + end + + def validate_person_1_economic(record) + validate_person_age_matches_economic_status(record, 1) end private diff --git a/docs/api/DLUHC-CORE-Data.v1.json b/docs/api/DLUHC-CORE-Data.v1.json index fed09b9d6..e4e08d15f 100644 --- a/docs/api/DLUHC-CORE-Data.v1.json +++ b/docs/api/DLUHC-CORE-Data.v1.json @@ -110,7 +110,7 @@ "value": { "errors": { "person_1_age": [ - "Tenant age must be an integer between 0 and 120" + "Tenant age must be an integer between 16 and 120" ] } } @@ -220,7 +220,7 @@ "If reasonable preference is Yes, a reason must be given" ], "person_1_age": [ - "Tenant age must be an integer between 0 and 120" + "Tenant age must be an integer between 16 and 120" ] } } diff --git a/spec/factories/case_log.rb b/spec/factories/case_log.rb index 6e5960329..bed593844 100644 --- a/spec/factories/case_log.rb +++ b/spec/factories/case_log.rb @@ -6,7 +6,7 @@ FactoryBot.define do tenant_code { "TH356" } property_postcode { "SW2 6HI" } previous_postcode { "P0 5ST" } - person_1_age { "12" } + person_1_age { "18" } end trait :completed do status { 2 } diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index 587fc80f1..6d26628f2 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -184,7 +184,7 @@ RSpec.describe "Test Features" do it "displays text answers in inputs if they are already saved" do visit("/case_logs/#{id}/person_1_age") - expect(page).to have_field("case-log-person-1-age-field", with: "12") + expect(page).to have_field("case-log-person-1-age-field", with: "18") end it "displays checkbox answers in inputs if they are already saved" do diff --git a/spec/requests/case_log_controller_spec.rb b/spec/requests/case_log_controller_spec.rb index 50a2250ed..d3f67683b 100644 --- a/spec/requests/case_log_controller_spec.rb +++ b/spec/requests/case_log_controller_spec.rb @@ -66,7 +66,7 @@ RSpec.describe CaseLogsController, type: :request do it "validates case log parameters" do json_response = JSON.parse(response.body) expect(response).to have_http_status(:unprocessable_entity) - expect(json_response["errors"]).to match_array([["property_number_of_times_relet", ["Property number of times relet must be between 0 and 20"]], ["person_1_age", ["Tenant age must be an integer between 0 and 120"]]]) + expect(json_response["errors"]).to match_array([["property_number_of_times_relet", ["Property number of times relet must be between 0 and 20"]], ["person_1_age", ["Tenant age must be an integer between 16 and 120"]]]) end end @@ -165,7 +165,7 @@ RSpec.describe CaseLogsController, type: :request do it "returns an error message" do json_response = JSON.parse(response.body) - expect(json_response["errors"]).to eq({ "person_1_age" => ["Tenant age must be an integer between 0 and 120"] }) + expect(json_response["errors"]).to eq({ "person_1_age" => ["Tenant age must be an integer between 16 and 120"] }) end end