From c78f2ef5f97b79f3ad94f12ab6b0dab0ea65a28b Mon Sep 17 00:00:00 2001 From: Arthur Campbell <51094020+arfacamble@users.noreply.github.com> Date: Mon, 17 Apr 2023 13:08:31 +0100 Subject: [PATCH] CLDC-1945 numeric questions decimal letter entry (#1442) * rename variables in test file * write tests for new validations * validate that answers to numeric questions are given to the correct level of accuracy and in normal format with only digits * add error messages for new validations * add or alter step on numeric questions in sales, amend one test given step changes * copy change * remove validation that is specific to number format and fold that functionality into accuracy validation, rejig tests accordingly * fix various tests * remove reference to currency in validation message that concerns step only and doesn't check whether firled is currency related * alter copy to avoid redundant repetition in validaiton message * save changes made in rebase * changes after PO review two step values had been missed in previous work in various places there were custom validations applied that duplicated the functionality of the generic min max validation that is applied to all numeric questions, in these situations sometimes the min and max on the question class was inconsistent with the limit that triggered validations these have been corrected and made consistent various tests were affected by this and have been amended accordingly * remove tests introduced by mistake in rebase --- app/models/form/lettings/questions/beds.rb | 4 +- app/models/form/lettings/questions/offered.rb | 2 +- .../lettings/questions/offered_social_let.rb | 2 +- app/models/form/sales/questions/age1.rb | 1 + app/models/form/sales/questions/age2.rb | 1 + .../form/sales/questions/deposit_amount.rb | 3 +- .../form/sales/questions/deposit_discount.rb | 1 + app/models/form/sales/questions/discount.rb | 1 + app/models/form/sales/questions/equity.rb | 1 + app/models/form/sales/questions/grant.rb | 1 + .../form/sales/questions/leasehold_charges.rb | 1 + .../form/sales/questions/monthly_rent.rb | 1 + .../form/sales/questions/mortgage_amount.rb | 1 + .../form/sales/questions/mortgage_length.rb | 1 + .../questions/number_of_others_in_property.rb | 1 + app/models/form/sales/questions/person_age.rb | 1 + .../form/sales/questions/previous_bedrooms.rb | 1 + .../questions/property_number_of_bedrooms.rb | 1 + .../form/sales/questions/purchase_price.rb | 1 + app/models/form/sales/questions/savings.rb | 4 +- .../form/sales/questions/staircase_bought.rb | 1 + .../form/sales/questions/staircase_owned.rb | 1 + app/models/form/sales/questions/value.rb | 1 + .../validations/property_validations.rb | 26 --- .../sales/sale_information_validations.rb | 2 +- app/models/validations/shared_validations.rb | 21 +++ .../imports/lettings_logs_import_service.rb | 4 +- config/forms/2021_2022.json | 8 +- config/forms/2022_2023.json | 8 +- config/locales/en.yml | 9 +- spec/fixtures/forms/2021_2022.json | 26 +-- .../questions/offered_social_let_spec.rb | 8 +- .../form/sales/questions/savings_spec.rb | 4 +- spec/models/lettings_log_spec.rb | 8 +- spec/models/sales_log_spec.rb | 4 +- .../validations/property_validations_spec.rb | 49 ------ .../validations/shared_validations_spec.rb | 158 ++++++++++++------ .../requests/lettings_logs_controller_spec.rb | 2 +- .../lettings_logs_import_service_spec.rb | 4 +- 39 files changed, 200 insertions(+), 174 deletions(-) diff --git a/app/models/form/lettings/questions/beds.rb b/app/models/form/lettings/questions/beds.rb index 75d2a835d..6fa6c7c2b 100644 --- a/app/models/form/lettings/questions/beds.rb +++ b/app/models/form/lettings/questions/beds.rb @@ -7,8 +7,8 @@ class Form::Lettings::Questions::Beds < ::Form::Question @type = "numeric" @width = 2 @check_answers_card_number = 0 - @max = 150 - @min = 0 + @max = 12 + @min = 1 @hint_text = "If shared accommodation, enter the number of bedrooms occupied by this household. A bedsit has 1 bedroom." @step = 1 @question_number = 22 diff --git a/app/models/form/lettings/questions/offered.rb b/app/models/form/lettings/questions/offered.rb index e41def301..2c9828d31 100644 --- a/app/models/form/lettings/questions/offered.rb +++ b/app/models/form/lettings/questions/offered.rb @@ -7,7 +7,7 @@ class Form::Lettings::Questions::Offered < ::Form::Question @type = "numeric" @width = 2 @check_answers_card_number = 0 - @max = 150 + @max = 20 @min = 0 @hint_text = I18n.t("hints.offered") @step = 1 diff --git a/app/models/form/lettings/questions/offered_social_let.rb b/app/models/form/lettings/questions/offered_social_let.rb index d3ac29516..dd57acbfe 100644 --- a/app/models/form/lettings/questions/offered_social_let.rb +++ b/app/models/form/lettings/questions/offered_social_let.rb @@ -7,7 +7,7 @@ class Form::Lettings::Questions::OfferedSocialLet < ::Form::Question @type = "numeric" @width = 2 @check_answers_card_number = 0 - @max = 150 + @max = 20 @min = 0 @hint_text = I18n.t("hints.offered") @step = 1 diff --git a/app/models/form/sales/questions/age1.rb b/app/models/form/sales/questions/age1.rb index d32083b53..0ae3bf682 100644 --- a/app/models/form/sales/questions/age1.rb +++ b/app/models/form/sales/questions/age1.rb @@ -19,6 +19,7 @@ class Form::Sales::Questions::Age1 < ::Form::Question @check_answers_card_number = 1 @min = 16 @max = 110 + @step = 1 @question_number = 20 end end diff --git a/app/models/form/sales/questions/age2.rb b/app/models/form/sales/questions/age2.rb index c0dc567a1..c003eb02e 100644 --- a/app/models/form/sales/questions/age2.rb +++ b/app/models/form/sales/questions/age2.rb @@ -13,6 +13,7 @@ class Form::Sales::Questions::Age2 < ::Form::Question @check_answers_card_number = 2 @max = 110 @min = 0 + @step = 1 @question_number = 28 end end diff --git a/app/models/form/sales/questions/deposit_amount.rb b/app/models/form/sales/questions/deposit_amount.rb index fb6c0e563..784bb56a1 100644 --- a/app/models/form/sales/questions/deposit_amount.rb +++ b/app/models/form/sales/questions/deposit_amount.rb @@ -6,8 +6,9 @@ class Form::Sales::Questions::DepositAmount < ::Form::Question @header = "How much cash deposit was paid on the property?" @type = "numeric" @min = 0 - @width = 5 @max = 999_999 + @step = 1 + @width = 5 @prefix = "£" @hint_text = "Enter the total cash sum paid by the buyer towards the property that was not funded by the mortgage" @derived = true diff --git a/app/models/form/sales/questions/deposit_discount.rb b/app/models/form/sales/questions/deposit_discount.rb index 577da5a91..ae6521392 100644 --- a/app/models/form/sales/questions/deposit_discount.rb +++ b/app/models/form/sales/questions/deposit_discount.rb @@ -7,6 +7,7 @@ class Form::Sales::Questions::DepositDiscount < ::Form::Question @type = "numeric" @min = 0 @max = 999_999 + @step = 1 @width = 5 @prefix = "£" @hint_text = "Enter the total cash discount given on the property being purchased through the Social HomeBuy scheme" diff --git a/app/models/form/sales/questions/discount.rb b/app/models/form/sales/questions/discount.rb index 8c5cf7132..ee39b8916 100644 --- a/app/models/form/sales/questions/discount.rb +++ b/app/models/form/sales/questions/discount.rb @@ -7,6 +7,7 @@ class Form::Sales::Questions::Discount < ::Form::Question @type = "numeric" @min = 0 @max = 100 + @step = 1 @width = 5 @suffix = "%" @hint_text = "For Right to Buy (RTB), Preserved Right to Buy (PRTB), and Voluntary Right to Buy (VRTB)

diff --git a/app/models/form/sales/questions/equity.rb b/app/models/form/sales/questions/equity.rb index 0119fea69..4db09a31f 100644 --- a/app/models/form/sales/questions/equity.rb +++ b/app/models/form/sales/questions/equity.rb @@ -7,6 +7,7 @@ class Form::Sales::Questions::Equity < ::Form::Question @type = "numeric" @min = 0 @max = 100 + @step = 1 @width = 5 @suffix = "%" @hint_text = "Enter the amount of initial equity held by the purchaser (for example, 25% or 50%)" diff --git a/app/models/form/sales/questions/grant.rb b/app/models/form/sales/questions/grant.rb index 9b0fd3091..e113be536 100644 --- a/app/models/form/sales/questions/grant.rb +++ b/app/models/form/sales/questions/grant.rb @@ -7,6 +7,7 @@ class Form::Sales::Questions::Grant < ::Form::Question @type = "numeric" @min = 0 @max = 999_999 + @step = 1 @width = 5 @prefix = "£" @hint_text = "For all schemes except Right to Buy (RTB), Preserved Right to Buy (PRTB), Voluntary Right to Buy (VRTB) and Rent to Buy" diff --git a/app/models/form/sales/questions/leasehold_charges.rb b/app/models/form/sales/questions/leasehold_charges.rb index 22ed7246e..2b2afb3e4 100644 --- a/app/models/form/sales/questions/leasehold_charges.rb +++ b/app/models/form/sales/questions/leasehold_charges.rb @@ -6,6 +6,7 @@ class Form::Sales::Questions::LeaseholdCharges < ::Form::Question @header = "Enter the total monthly charge" @type = "numeric" @min = 1 + @step = 0.01 @width = 5 @prefix = "£" @ownershipsch = ownershipsch diff --git a/app/models/form/sales/questions/monthly_rent.rb b/app/models/form/sales/questions/monthly_rent.rb index 2a28bc691..75c4a7ce5 100644 --- a/app/models/form/sales/questions/monthly_rent.rb +++ b/app/models/form/sales/questions/monthly_rent.rb @@ -6,6 +6,7 @@ class Form::Sales::Questions::MonthlyRent < ::Form::Question @header = "What is the basic monthly rent?" @type = "numeric" @min = 0 + @step = 0.01 @width = 5 @prefix = "£" @hint_text = "Amount paid before any charges" diff --git a/app/models/form/sales/questions/mortgage_amount.rb b/app/models/form/sales/questions/mortgage_amount.rb index 9b199b2aa..ce4b548e1 100644 --- a/app/models/form/sales/questions/mortgage_amount.rb +++ b/app/models/form/sales/questions/mortgage_amount.rb @@ -6,6 +6,7 @@ class Form::Sales::Questions::MortgageAmount < ::Form::Question @header = "What is the mortgage amount?" @type = "numeric" @min = 1 + @step = 1 @width = 5 @prefix = "£" @hint_text = "Enter the amount of mortgage agreed with the mortgage lender. Exclude any deposits or cash payments. Numeric in pounds. Rounded to the nearest pound." diff --git a/app/models/form/sales/questions/mortgage_length.rb b/app/models/form/sales/questions/mortgage_length.rb index adaa94d01..218ea03a1 100644 --- a/app/models/form/sales/questions/mortgage_length.rb +++ b/app/models/form/sales/questions/mortgage_length.rb @@ -7,6 +7,7 @@ class Form::Sales::Questions::MortgageLength < ::Form::Question @type = "numeric" @min = 0 @max = 60 + @step = 1 @width = 5 @suffix = " years" @hint_text = "You should round up to the nearest year. Value should not exceed 60 years." 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 556ddf837..450b12eb0 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 @@ -9,6 +9,7 @@ class Form::Sales::Questions::NumberOfOthersInProperty < ::Form::Question @width = 2 @min = 0 @max = 15 + @step = 1 @question_number = 35 end diff --git a/app/models/form/sales/questions/person_age.rb b/app/models/form/sales/questions/person_age.rb index 779e09669..cdbc9f80b 100644 --- a/app/models/form/sales/questions/person_age.rb +++ b/app/models/form/sales/questions/person_age.rb @@ -12,6 +12,7 @@ class Form::Sales::Questions::PersonAge < ::Form::Question @check_answers_card_number = person_index @min = 0 @max = 110 + @step = 1 @question_number = 29 + (4 * person_index) end end diff --git a/app/models/form/sales/questions/previous_bedrooms.rb b/app/models/form/sales/questions/previous_bedrooms.rb index d9f9aaddb..8f36a8485 100644 --- a/app/models/form/sales/questions/previous_bedrooms.rb +++ b/app/models/form/sales/questions/previous_bedrooms.rb @@ -8,6 +8,7 @@ class Form::Sales::Questions::PreviousBedrooms < ::Form::Question @width = 5 @min = 1 @max = 6 + @step = 1 @hint_text = "For bedsits enter 1" @question_number = 85 end diff --git a/app/models/form/sales/questions/property_number_of_bedrooms.rb b/app/models/form/sales/questions/property_number_of_bedrooms.rb index 9dbdbf3e8..b37253244 100644 --- a/app/models/form/sales/questions/property_number_of_bedrooms.rb +++ b/app/models/form/sales/questions/property_number_of_bedrooms.rb @@ -9,6 +9,7 @@ class Form::Sales::Questions::PropertyNumberOfBedrooms < ::Form::Question @width = 10 @min = 1 @max = 9 + @step = 1 @question_number = 11 end end diff --git a/app/models/form/sales/questions/purchase_price.rb b/app/models/form/sales/questions/purchase_price.rb index c98a39ef3..f88938777 100644 --- a/app/models/form/sales/questions/purchase_price.rb +++ b/app/models/form/sales/questions/purchase_price.rb @@ -6,6 +6,7 @@ class Form::Sales::Questions::PurchasePrice < ::Form::Question @header = "What is the full purchase price?" @type = "numeric" @min = 0 + @step = 0.01 @width = 5 @prefix = "£" @hint_text = "For all schemes, including Right to Acquire (RTA), Right to Buy (RTB), Voluntary Right to Buy (VRTB) or Preserved Right to Buy (PRTB) sales, enter the full price of the property without any discount" diff --git a/app/models/form/sales/questions/savings.rb b/app/models/form/sales/questions/savings.rb index 086a0ec8d..6c01ed2fd 100644 --- a/app/models/form/sales/questions/savings.rb +++ b/app/models/form/sales/questions/savings.rb @@ -2,12 +2,12 @@ class Form::Sales::Questions::Savings < ::Form::Question def initialize(id, hsh, page) super @id = "savings" - @check_answer_label = "Buyer’s total savings (to nearest £10) before any deposit paid" + @check_answer_label = "Buyer’s total savings before any deposit paid" @header = "Enter their total savings to the nearest £10" @type = "numeric" @width = 5 @prefix = "£" - @step = 1 + @step = 10 @min = 0 @question_number = 72 end diff --git a/app/models/form/sales/questions/staircase_bought.rb b/app/models/form/sales/questions/staircase_bought.rb index 9e54e92d3..dfdb273f5 100644 --- a/app/models/form/sales/questions/staircase_bought.rb +++ b/app/models/form/sales/questions/staircase_bought.rb @@ -8,6 +8,7 @@ class Form::Sales::Questions::StaircaseBought < ::Form::Question @width = 5 @min = 0 @max = 100 + @step = 1 @suffix = "%" @question_number = 77 end diff --git a/app/models/form/sales/questions/staircase_owned.rb b/app/models/form/sales/questions/staircase_owned.rb index b8d5a65ec..2b4d89861 100644 --- a/app/models/form/sales/questions/staircase_owned.rb +++ b/app/models/form/sales/questions/staircase_owned.rb @@ -8,6 +8,7 @@ class Form::Sales::Questions::StaircaseOwned < ::Form::Question @width = 5 @min = 0 @max = 100 + @step = 1 @suffix = "%" @question_number = 78 end diff --git a/app/models/form/sales/questions/value.rb b/app/models/form/sales/questions/value.rb index 4736f37af..d349a99df 100644 --- a/app/models/form/sales/questions/value.rb +++ b/app/models/form/sales/questions/value.rb @@ -6,6 +6,7 @@ class Form::Sales::Questions::Value < ::Form::Question @header = "What was the full purchase price?" @type = "numeric" @min = 0 + @step = 1 @width = 5 @prefix = "£" @hint_text = "Enter the full purchase price of the property before any discounts are applied. For shared ownership, enter the full purchase price paid for 100% equity (this is equal to the value of the share owned by the PRP plus the value bought by the purchaser)" diff --git a/app/models/validations/property_validations.rb b/app/models/validations/property_validations.rb index d2b6d2a0b..e9680facf 100644 --- a/app/models/validations/property_validations.rb +++ b/app/models/validations/property_validations.rb @@ -2,24 +2,6 @@ module Validations::PropertyValidations # 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) - return unless record.offered - - # Since offered is an integer type ActiveRecord will automatically cast that for us - # but it's type casting is a little lax so "random" becomes 0. To make sure that doesn't pass - # validation and then get silently dropped we attempt strict type casting on the original value - # as part of our validation. - begin - Integer(record.offered_before_type_cast) - rescue ArgumentError - record.errors.add :offered, I18n.t("validations.property.offered.relet_number") - end - - if record.offered.negative? || record.offered > 20 - record.errors.add :offered, :over_20, message: I18n.t("validations.property.offered.relet_number") - end - end - REFERRAL_INVALID_TMP = [8, 10, 12, 13, 14, 15].freeze def validate_rsnvac(record) if !record.first_time_property_let_as_social_housing? && record.has_first_let_vacancy_reason? @@ -51,10 +33,6 @@ module Validations::PropertyValidations end def validate_shared_housing_rooms(record) - if record.beds.present? && record.beds <= 0 - record.errors.add :beds, I18n.t("validations.property.beds.non_positive") - end - unless record.unittype_gn.nil? if record.is_bedsit? && record.beds != 1 && record.beds.present? record.errors.add :unittype_gn, I18n.t("validations.property.unittype_gn.one_bedroom_bedsit") @@ -70,10 +48,6 @@ module Validations::PropertyValidations record.errors.add :beds, I18n.t("validations.property.unittype_gn.one_seven_bedroom_shared") end end - - if record.beds.present? && record.beds > 12 - record.errors.add :beds, :over_max, message: I18n.t("validations.property.beds.over_max") - end end def validate_uprn(record) diff --git a/app/models/validations/sales/sale_information_validations.rb b/app/models/validations/sales/sale_information_validations.rb index e4714e81a..f99d79cf6 100644 --- a/app/models/validations/sales/sale_information_validations.rb +++ b/app/models/validations/sales/sale_information_validations.rb @@ -12,7 +12,7 @@ module Validations::Sales::SaleInformationValidations end def validate_years_living_in_property_before_purchase(record) - return unless record.proplen && record.proplen.nonzero? + return unless record.proplen&.nonzero? case record.type when 18 diff --git a/app/models/validations/shared_validations.rb b/app/models/validations/shared_validations.rb index 57e1d05c6..0a0c3f63b 100644 --- a/app/models/validations/shared_validations.rb +++ b/app/models/validations/shared_validations.rb @@ -34,6 +34,27 @@ module Validations::SharedValidations end end + def validate_numeric_step(record) + record.form.numeric_questions.each do |question| + next unless question.step + next unless record[question.id] && question.page.routed_to?(record, nil) + + value = record.public_send("#{question.id}_before_type_cast") + field = question.check_answer_label || question.id + incorrect_accuracy = (value.to_d * 100) % (question.step * 100) != 0 + + if question.step < 1 && incorrect_accuracy + record.errors.add question.id.to_sym, I18n.t("validations.numeric.nearest_hundredth", field:) + elsif incorrect_accuracy || value.to_d != value.to_i # if the user enters a value in exponent notation (eg '4e1') the to_i method does not convert this to the correct value + field = question.check_answer_label || question.id + case question.step + when 1 then record.errors.add question.id.to_sym, I18n.t("validations.numeric.whole_number", field:) + when 10 then record.errors.add question.id.to_sym, I18n.t("validations.numeric.nearest_ten", field:) + end + end + end + end + def validate_property_postcode(record) postcode = record.postcode_full if record.postcode_known? && (postcode.blank? || !postcode.match(POSTCODE_REGEXP)) diff --git a/app/services/imports/lettings_logs_import_service.rb b/app/services/imports/lettings_logs_import_service.rb index 01c71faef..56fb0c0d2 100644 --- a/app/services/imports/lettings_logs_import_service.rb +++ b/app/services/imports/lettings_logs_import_service.rb @@ -290,10 +290,10 @@ module Imports %i[prevten over_20_foster_care] => %w[prevten age1], %i[prevten non_temp_accommodation] => %w[prevten rsnvac], %i[joint not_joint_tenancy] => %w[joint], - %i[offered over_20] => %w[offered], + %i[offered outside_the_range] => %w[offered], %i[earnings over_hard_max] => %w[ecstat1], %i[tshortfall no_outstanding_charges] => %w[tshortfall hbrentshortfall], - %i[beds over_max] => %w[beds], + %i[beds outside_the_range] => %w[beds], %i[tcharge complete_1_of_3] => %w[brent scharge pscharge supcharg tcharge], %i[scharge under_min] => %w[brent scharge pscharge supcharg tcharge], %i[tshortfall must_be_positive] => %w[tshortfall tshortfall_known], diff --git a/config/forms/2021_2022.json b/config/forms/2021_2022.json index 741195f01..cf8ac4cd9 100644 --- a/config/forms/2021_2022.json +++ b/config/forms/2021_2022.json @@ -566,7 +566,7 @@ "hint_text": "This is after the last tenancy ended. If the property is being offered for let for the first time, enter 0.", "type": "numeric", "min": 0, - "max": 150, + "max": 20, "step": 1, "width": 2 } @@ -588,7 +588,7 @@ "hint_text": "If the property is being offered for let for the first time, enter 0.", "type": "numeric", "min": 0, - "max": 150, + "max": 20, "step": 1, "width": 2 } @@ -702,8 +702,8 @@ "header": "How many bedrooms does the property have?", "hint_text": "If shared accommodation, enter the number of bedrooms occupied by this household. A bedsit has 1 bedroom.", "type": "numeric", - "min": 0, - "max": 150, + "min": 1, + "max": 12, "step": 1, "width": 2 } diff --git a/config/forms/2022_2023.json b/config/forms/2022_2023.json index e13ce5516..7c189849d 100644 --- a/config/forms/2022_2023.json +++ b/config/forms/2022_2023.json @@ -561,7 +561,7 @@ "hint_text": "This is after the last tenancy ended. If the property is being offered for let for the first time, enter 0.", "type": "numeric", "min": 0, - "max": 150, + "max": 20, "step": 1, "width": 2 } @@ -583,7 +583,7 @@ "hint_text": "If the property is being offered for let for the first time, enter 0.", "type": "numeric", "min": 0, - "max": 150, + "max": 20, "step": 1, "width": 2 } @@ -697,8 +697,8 @@ "header": "How many bedrooms does the property have?", "hint_text": "If shared accommodation, enter the number of bedrooms occupied by this household. A bedsit has 1 bedroom.", "type": "numeric", - "min": 0, - "max": 150, + "min": 1, + "max": 12, "step": 1, "width": 2 } diff --git a/config/locales/en.yml b/config/locales/en.yml index 994091689..f224a6d63 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -144,6 +144,11 @@ en: numeric: within_range: "%{field} must be between %{min} and %{max}" above_min: "%{field} must be at least %{min}" + whole_number: "%{field} must be a whole number" + nearest_ten: "%{field} must be given to the nearest ten" + nearest_hundredth: "%{field} must be given to the nearest hundredth" + normal_format: "Enter a number" + date: invalid_date: "Enter a date in the correct format, for example 31 1 2022" outside_collection_window: Enter a date within the 22/23 collection year, which is between 1st April 2022 and 31st March 2023 @@ -213,8 +218,6 @@ en: ten_years_before_tenancy_start: "Enter a void date no more than 10 years before the tenancy start date" before_tenancy_start: "Enter a void date that is before the tenancy start date" after_mrcdate: "Void date must be before the major repairs date if provided" - offered: - relet_number: "Enter a number between 0 and 20 for the amount of times the property has been re-let" la: la_invalid_for_org: "%{org_name} does not operate in %{la_name}" postcode_invalid_for_org: "Enter a postcode in an area covered by %{org_name}" @@ -230,8 +233,6 @@ en: one_seven_bedroom_shared: "A shared house must have 1 to 7 bedrooms" one_three_bedroom_single_tenant_shared: "A shared house with fewer than two tenants must have 1 to 3 bedrooms" beds: - non_positive: "Number of bedrooms has to be greater than 0" - over_max: "Number of bedrooms cannot be more than 12" bedsits_have_max_one_bedroom: "Number of bedrooms must be 1 if the property is a bedsit" proptype: bedsits_have_max_one_bedroom: "Answer cannot be 'Bedsit' if the property has 2 or more bedrooms" diff --git a/spec/fixtures/forms/2021_2022.json b/spec/fixtures/forms/2021_2022.json index ec13bdabb..9ea11f671 100644 --- a/spec/fixtures/forms/2021_2022.json +++ b/spec/fixtures/forms/2021_2022.json @@ -905,7 +905,7 @@ "hint_text": "Eligible for housing benefit or Universal Credit", "type": "numeric", "min": 0, - "step": 1, + "step": 0.01, "width": 4, "fields-to-add": [ "brent", @@ -921,7 +921,7 @@ "hint_text": "Eligible for housing benefit or Universal Credit", "type": "numeric", "min": 0, - "step": 1, + "step": 0.01, "width": 4, "fields-to-add": [ "brent", @@ -937,7 +937,7 @@ "hint_text": "Not eligible for housing benefit or Universal Credit. For example, hot water excluding water rates.", "type": "numeric", "min": 0, - "step": 1, + "step": 0.01, "width": 4, "fields-to-add": [ "brent", @@ -954,7 +954,7 @@ "type": "numeric", "min": 0, "max": 300, - "step": 1, + "step": 0.01, "width": 4, "fields-to-add": [ "brent", @@ -970,7 +970,7 @@ "hint_text": "This is the total of rent and all charges", "type": "numeric_output", "min": 0, - "step": 1, + "step": 0.01, "width": 4, "readonly": true, "requires_js": true @@ -987,7 +987,7 @@ "hint_text": "", "type": "numeric", "min": 0, - "step": "1", + "step": 1, "width": 5, "prefix": "£", "suffix": " every week" @@ -997,11 +997,12 @@ "care_home_charge": { "questions": { "offered": { - "check_answer_label": "Basic Rent", - "header": "What is the basic rent?", - "hint_text": "Eligible for housing benefit or Universal Credit", + "check_answer_label": "Times previously offered since becoming available", + "header": "Since becoming available for re-let, how many times has the property been previously offered?", + "hint_text": "This is after the last tenancy ended. If the property is being offered for let for the first time, enter 0.", "type": "numeric", "min": 0, + "max": 20, "step": 1, "width": 4 } @@ -1015,11 +1016,12 @@ "care_home_charge_bi_weekly": { "questions": { "offered": { - "check_answer_label": "Basic Rent", - "header": "What is the basic rent?", - "hint_text": "Eligible for housing benefit or Universal Credit", + "check_answer_label": "Times previously offered since becoming available", + "header": "Since becoming available for re-let, how many times has the property been previously offered?", + "hint_text": "This is after the last tenancy ended. If the property is being offered for let for the first time, enter 0.", "type": "numeric", "min": 0, + "max": 20, "step": 1, "width": 4 } diff --git a/spec/models/form/lettings/questions/offered_social_let_spec.rb b/spec/models/form/lettings/questions/offered_social_let_spec.rb index 6516c661b..ac1930495 100644 --- a/spec/models/form/lettings/questions/offered_social_let_spec.rb +++ b/spec/models/form/lettings/questions/offered_social_let_spec.rb @@ -6,7 +6,7 @@ RSpec.describe Form::Lettings::Questions::OfferedSocialLet, type: :model do let(:page) { instance_double(Form::Page) } it "has correct page" do - expect(question.page).to eq page + expect(question.page).to be page end it "has the correct id" do @@ -26,12 +26,12 @@ RSpec.describe Form::Lettings::Questions::OfferedSocialLet, type: :model do end it "has the correct minimum and maximum values" do - expect(question.min).to eq 0 - expect(question.max).to eq 150 + expect(question.min).to be 0 + expect(question.max).to be 20 end it "has the correct step" do - expect(question.step).to eq 1 + expect(question.step).to be 1 end it "is not marked as derived" do diff --git a/spec/models/form/sales/questions/savings_spec.rb b/spec/models/form/sales/questions/savings_spec.rb index 3f721d7ef..8e9c4daa4 100644 --- a/spec/models/form/sales/questions/savings_spec.rb +++ b/spec/models/form/sales/questions/savings_spec.rb @@ -20,7 +20,7 @@ RSpec.describe Form::Sales::Questions::Savings, type: :model do end it "has the correct check_answer_label" do - expect(question.check_answer_label).to eq("Buyer’s total savings (to nearest £10) before any deposit paid") + expect(question.check_answer_label).to eq("Buyer’s total savings before any deposit paid") end it "has the correct type" do @@ -40,7 +40,7 @@ RSpec.describe Form::Sales::Questions::Savings, type: :model do end it "has correct step" do - expect(question.step).to eq(1) + expect(question.step).to be 10 end it "has correct prefix" do diff --git a/spec/models/lettings_log_spec.rb b/spec/models/lettings_log_spec.rb index 2d349f295..291af1622 100644 --- a/spec/models/lettings_log_spec.rb +++ b/spec/models/lettings_log_spec.rb @@ -111,10 +111,6 @@ RSpec.describe LettingsLog do expect(validator).to receive(:validate_shared_housing_rooms) end - it "validates number of times the property has been relet" do - expect(validator).to receive(:validate_property_number_of_times_relet) - end - it "validates tenancy type" do expect(validator).to receive(:validate_fixed_term_tenancy) expect(validator).to receive(:validate_other_tenancy_type) @@ -3022,11 +3018,11 @@ RSpec.describe LettingsLog do end context "when a non setup field is invalid" do - subject(:model) { described_class.new(beds: 404) } + subject(:model) { build(:lettings_log, :completed, offered: 234) } it "blanks it" do model.valid? - expect { model.blank_invalid_non_setup_fields! }.to change(model, :beds) + expect { model.blank_invalid_non_setup_fields! }.to change(model, :offered) end end end diff --git a/spec/models/sales_log_spec.rb b/spec/models/sales_log_spec.rb index e9acd4548..986130d3f 100644 --- a/spec/models/sales_log_spec.rb +++ b/spec/models/sales_log_spec.rb @@ -508,9 +508,9 @@ RSpec.describe SalesLog, type: :model do let(:completed_sales_log) { create(:sales_log, :completed) } it "returns small numbers correctly formatted as currency" do - completed_sales_log.update!(savings: 4) + completed_sales_log.update!(savings: 20) - expect(completed_sales_log.field_formatted_as_currency("savings")).to eq("£4.00") + expect(completed_sales_log.field_formatted_as_currency("savings")).to eq("£20.00") end it "returns quite large numbers correctly formatted as currency" do diff --git a/spec/models/validations/property_validations_spec.rb b/spec/models/validations/property_validations_spec.rb index 3b3e42e9e..e918e3405 100644 --- a/spec/models/validations/property_validations_spec.rb +++ b/spec/models/validations/property_validations_spec.rb @@ -6,39 +6,6 @@ RSpec.describe Validations::PropertyValidations do let(:property_validator_class) { Class.new { include Validations::PropertyValidations } } let(:record) { FactoryBot.create(:lettings_log) } - describe "#validate_property_number_of_times_relet" do - let(:expected_error) { I18n.t("validations.property.offered.relet_number") } - - it "does not add an error if the record offered is missing" do - record.offered = nil - property_validator.validate_property_number_of_times_relet(record) - expect(record.errors).to be_empty - end - - it "does not add an error if offered is valid (number between 0 and 20)" do - record.offered = 0 - property_validator.validate_property_number_of_times_relet(record) - expect(record.errors).to be_empty - record.offered = 10 - property_validator.validate_property_number_of_times_relet(record) - expect(record.errors).to be_empty - record.offered = 20 - property_validator.validate_property_number_of_times_relet(record) - expect(record.errors).to be_empty - end - - it "does add an error when offered is invalid" do - record.offered = "invalid" - property_validator.validate_property_number_of_times_relet(record) - expect(record.errors).not_to be_empty - expect(record.errors["offered"]).to include(match(expected_error)) - record.offered = 21 - property_validator.validate_property_number_of_times_relet(record) - expect(record.errors).not_to be_empty - expect(record.errors["offered"]).to include(match(expected_error)) - end - end - describe "#validate_shared_housing_rooms" do context "when number of bedrooms has not been answered" do it "does not add an error" do @@ -129,22 +96,6 @@ RSpec.describe Validations::PropertyValidations do expect(record.errors["beds"]).to include(I18n.t("validations.property.unittype_gn.one_three_bedroom_single_tenant_shared")) end end - - context "when a negative number of bedrooms is entered" do - it "adds an error" do - record.beds = -4 - property_validator.validate_shared_housing_rooms(record) - expect(record.errors["beds"]).to include(I18n.t("validations.property.beds.non_positive")) - end - end - - context "when a room number higher than 12 has been entered" do - it "adds an error" do - record.beds = 13 - property_validator.validate_shared_housing_rooms(record) - expect(record.errors["beds"]).to include(I18n.t("validations.property.beds.over_max")) - end - end end describe "#validate_unitletas" do diff --git a/spec/models/validations/shared_validations_spec.rb b/spec/models/validations/shared_validations_spec.rb index 8f5038c5d..fe3612c3c 100644 --- a/spec/models/validations/shared_validations_spec.rb +++ b/spec/models/validations/shared_validations_spec.rb @@ -4,8 +4,8 @@ RSpec.describe Validations::SharedValidations do subject(:shared_validator) { validator_class.new } let(:validator_class) { Class.new { include Validations::SharedValidations } } - let(:record) { FactoryBot.create(:lettings_log) } - let(:sales_record) { FactoryBot.create(:sales_log, :completed) } + let(:lettings_log) { FactoryBot.create(:lettings_log) } + let(:sales_log) { FactoryBot.create(:sales_log, :completed) } let(:fake_2021_2022_form) { Form.new("spec/fixtures/forms/2021_2022.json") } describe "numeric min max validations" do @@ -15,102 +15,164 @@ RSpec.describe Validations::SharedValidations do context "when validating age" do it "validates that person 1's age is a number" do - record.age1 = "random" - shared_validator.validate_numeric_min_max(record) - expect(record.errors["age1"]) + lettings_log.age1 = "random" + shared_validator.validate_numeric_min_max(lettings_log) + expect(lettings_log.errors["age1"]) .to include(match I18n.t("validations.numeric.within_range", field: "Lead tenant’s age", min: 16, max: 120)) end it "validates that other household member ages are a number" do - record.age2 = "random" - shared_validator.validate_numeric_min_max(record) - expect(record.errors["age2"]) + lettings_log.age2 = "random" + shared_validator.validate_numeric_min_max(lettings_log) + expect(lettings_log.errors["age2"]) .to include(match I18n.t("validations.numeric.within_range", field: "Person 2’s age", min: 1, max: 120)) end it "validates that person 1's age is greater than 16" do - record.age1 = 15 - shared_validator.validate_numeric_min_max(record) - expect(record.errors["age1"]) + lettings_log.age1 = 15 + shared_validator.validate_numeric_min_max(lettings_log) + expect(lettings_log.errors["age1"]) .to include(match I18n.t("validations.numeric.within_range", field: "Lead tenant’s age", min: 16, max: 120)) end it "validates that other household member ages are greater than 1" do - record.age2 = 0 - shared_validator.validate_numeric_min_max(record) - expect(record.errors["age2"]) + lettings_log.age2 = 0 + shared_validator.validate_numeric_min_max(lettings_log) + expect(lettings_log.errors["age2"]) .to include(match I18n.t("validations.numeric.within_range", field: "Person 2’s age", min: 1, max: 120)) end it "validates that person 1's age is less than 121" do - record.age1 = 121 - shared_validator.validate_numeric_min_max(record) - expect(record.errors["age1"]) + lettings_log.age1 = 121 + shared_validator.validate_numeric_min_max(lettings_log) + expect(lettings_log.errors["age1"]) .to include(match I18n.t("validations.numeric.within_range", field: "Lead tenant’s age", min: 16, max: 120)) end it "validates that other household member ages are greater than 121" do - record.age2 = 123 - shared_validator.validate_numeric_min_max(record) - expect(record.errors["age2"]) + lettings_log.age2 = 123 + shared_validator.validate_numeric_min_max(lettings_log) + expect(lettings_log.errors["age2"]) .to include(match I18n.t("validations.numeric.within_range", field: "Person 2’s age", min: 1, max: 120)) end it "validates that person 1's age is between 16 and 120" do - record.age1 = 63 - shared_validator.validate_numeric_min_max(record) - expect(record.errors["age1"]).to be_empty + lettings_log.age1 = 63 + shared_validator.validate_numeric_min_max(lettings_log) + expect(lettings_log.errors["age1"]).to be_empty end it "validates that other household member ages are between 1 and 120" do - record.age6 = 45 - shared_validator.validate_numeric_min_max(record) - expect(record.errors["age6"]).to be_empty + lettings_log.age6 = 45 + shared_validator.validate_numeric_min_max(lettings_log) + expect(lettings_log.errors["age6"]).to be_empty end context "with sales log" do it "validates that person 2's age is between 0 and 110 for non joint purchase" do - sales_record.jointpur = 2 - sales_record.hholdcount = 1 - sales_record.details_known_2 = 1 - sales_record.age2 = 130 - shared_validator.validate_numeric_min_max(sales_record) - expect(sales_record.errors["age2"].first).to eq("Person 2’s age must be between 0 and 110") + sales_log.jointpur = 2 + sales_log.hholdcount = 1 + sales_log.details_known_2 = 1 + sales_log.age2 = 130 + shared_validator.validate_numeric_min_max(sales_log) + expect(sales_log.errors["age2"].first).to eq("Person 2’s age must be between 0 and 110") end it "validates that buyer 2's age is between 0 and 110 for joint purchase" do - sales_record.jointpur = 1 - sales_record.age2 = 130 - shared_validator.validate_numeric_min_max(sales_record) - expect(sales_record.errors["age2"].first).to eq("Buyer 2’s age must be between 0 and 110") + sales_log.jointpur = 1 + sales_log.age2 = 130 + shared_validator.validate_numeric_min_max(sales_log) + expect(sales_log.errors["age2"].first).to eq("Buyer 2’s age must be between 0 and 110") end end end it "adds the correct validation text when a question has a min but not a max" do - sales_record.savings = -10 - shared_validator.validate_numeric_min_max(sales_record) - expect(sales_record.errors["savings"]).to include(match I18n.t("validations.numeric.above_min", field: "Buyer’s total savings (to nearest £10) before any deposit paid", min: "£0")) + sales_log.savings = -10 + shared_validator.validate_numeric_min_max(sales_log) + expect(sales_log.errors["savings"]).to include(match I18n.t("validations.numeric.above_min", field: "Buyer’s total savings before any deposit paid", min: "£0")) end context "when validating percent" do it "validates that suffixes are added in the error message" do - sales_record.ownershipsch = 1 - sales_record.staircase = 1 - sales_record.stairbought = 150 - shared_validator.validate_numeric_min_max(sales_record) - expect(sales_record.errors["stairbought"]) + sales_log.ownershipsch = 1 + sales_log.staircase = 1 + sales_log.stairbought = 150 + shared_validator.validate_numeric_min_max(sales_log) + expect(sales_log.errors["stairbought"]) .to include(match I18n.t("validations.numeric.within_range", field: "Percentage bought in this staircasing transaction", min: "0%", max: "100%")) end end context "when validating price" do - it "validates that £ prefix and , is added in the error message" do - sales_record.income1 = -5 - shared_validator.validate_numeric_min_max(sales_record) - expect(sales_record.errors["income1"]) + it "validates that prefix £ and delimeter ',' is added in the error message" do + sales_log.income1 = -5 + shared_validator.validate_numeric_min_max(sales_log) + expect(sales_log.errors["income1"]) .to include(match I18n.t("validations.numeric.within_range", field: "Buyer 1’s gross annual income", min: "£0", max: "£999,999")) end end end + + describe "validating level of accuracy or rounding for numeric questions" do + context "when validating a question with a step of 1" do + it "adds an error if input is a decimal" do + sales_log.income1 = 30_000.5 + shared_validator.validate_numeric_step(sales_log) + expect(sales_log.errors[:income1]).to include I18n.t("validations.numeric.whole_number", field: "Buyer 1’s gross annual income") + end + + it "adds an error if the user attempts to input a number in exponent format" do + sales_log.income1 = "3e5" + shared_validator.validate_numeric_step(sales_log) + expect(sales_log.errors[:income1]).to include I18n.t("validations.numeric.whole_number", field: "Buyer 1’s gross annual income") + end + + it "does not add an error if input is an integer" do + sales_log.income1 = 30_000 + shared_validator.validate_numeric_step(sales_log) + expect(sales_log.errors).to be_empty + end + end + + context "when validating a question with a step of 10" do + it "adds an error if input is not a multiple of ten" do + sales_log.savings = 30_005 + shared_validator.validate_numeric_step(sales_log) + expect(sales_log.errors[:savings]).to include I18n.t("validations.numeric.nearest_ten", field: "Buyer’s total savings before any deposit paid") + end + + it "adds an error if the user attempts to input a number in exponent format" do + sales_log.savings = "3e5" + shared_validator.validate_numeric_step(sales_log) + expect(sales_log.errors[:savings]).to include I18n.t("validations.numeric.nearest_ten", field: "Buyer’s total savings before any deposit paid") + end + + it "does not add an error if input is a multiple of ten" do + sales_log.savings = 30_000 + shared_validator.validate_numeric_step(sales_log) + expect(sales_log.errors).to be_empty + end + end + + context "when validating a question with a step of 0.01" do + it "adds an error if input has more than 2 decimal places" do + sales_log.mscharge = 30.7418 + shared_validator.validate_numeric_step(sales_log) + expect(sales_log.errors[:mscharge]).to include I18n.t("validations.numeric.nearest_hundredth", field: "Monthly leasehold charges") + end + + it "does not add an error if the user attempts to input a number in exponent format" do + sales_log.mscharge = "3e1" + shared_validator.validate_numeric_step(sales_log) + expect(sales_log.errors).to be_empty + end + + it "does not add an error if input has 2 or fewer decimal places" do + sales_log.mscharge = 30.74 + shared_validator.validate_numeric_step(sales_log) + expect(sales_log.errors).to be_empty + end + end + end end diff --git a/spec/requests/lettings_logs_controller_spec.rb b/spec/requests/lettings_logs_controller_spec.rb index d95e0a3fd..6de5c4db2 100644 --- a/spec/requests/lettings_logs_controller_spec.rb +++ b/spec/requests/lettings_logs_controller_spec.rb @@ -82,7 +82,7 @@ RSpec.describe LettingsLogsController, type: :request do it "validates lettings log parameters" do json_response = JSON.parse(response.body) expect(response).to have_http_status(:unprocessable_entity) - expect(json_response["errors"]).to match_array([["offered", [I18n.t("validations.property.offered.relet_number")]], ["age1", [I18n.t("validations.numeric.within_range", field: "Lead tenant’s age", min: 16, max: 120)]]]) + expect(json_response["errors"]).to match_array([["offered", [I18n.t("validations.numeric.within_range", field: "Times previously offered since becoming available", min: 0, max: 20)]], ["age1", [I18n.t("validations.numeric.within_range", field: "Lead tenant’s age", min: 16, max: 120)]]]) end end diff --git a/spec/services/imports/lettings_logs_import_service_spec.rb b/spec/services/imports/lettings_logs_import_service_spec.rb index ccc0e5d11..f144a798d 100644 --- a/spec/services/imports/lettings_logs_import_service_spec.rb +++ b/spec/services/imports/lettings_logs_import_service_spec.rb @@ -406,7 +406,7 @@ RSpec.describe Imports::LettingsLogsImportService do end it "intercepts the relevant validation error" do - expect(logger).to receive(:warn).with(/Removing offered with error: Enter a number between 0 and 20 for the amount of times the property has been re-let/) + expect(logger).to receive(:warn).with(/Removing offered with error: Times previously offered since becoming available must be between 0 and 20/) expect { lettings_log_service.send(:create_log, lettings_log_xml) } .not_to raise_error end @@ -502,7 +502,7 @@ RSpec.describe Imports::LettingsLogsImportService do end it "intercepts the relevant validation error" do - expect(logger).to receive(:warn).with(/Removing beds with error: Number of bedrooms cannot be more than 12/) + expect(logger).to receive(:warn).with(/Removing beds with error: Number of bedrooms must be between 1 and 12/) expect { lettings_log_service.send(:create_log, lettings_log_xml) } .not_to raise_error end