CLDC-1923 amend validation message for single ended ranges (#1253)

* replace range validation with multiple to account for single-ended ranges

* add test for question with min but no max

* add min and max to household count question and remove bespoke validation. Remove code allowing max and no min given this situation does not seem to exist int eh form

* amend minor typo, add back a soft validation that had mysteriously gone missing

app/models/form/sales/pages/shared_ownership_deposit_value_check.rb

@@ -8,7 +8,7 @@ class Form::Sales::Pages::SharedOwnershipDepositValueCheck < ::Form::Page
     ]
     @informative_text = {}
     @title_text = {
-      "translation" => "soft_validations.shared_owhership_deposit.title_text",
+      "translation" => "soft_validations.shared_ownership_deposit.title_text",
       "arguments" => [
         {
           "key" => "expected_shared_ownership_deposit_value",

app/models/form/sales/questions/number_of_others_in_property.rb

@@ -7,5 +7,7 @@ class Form::Sales::Questions::NumberOfOthersInProperty < ::Form::Question
     @type = "numeric"
     @hint_text = "You can provide details for a maximum of 4 other people."
     @width = 2
+    @min = 0
+    @max = 4
   end
 end

app/models/validations/sales/household_validations.rb

@@ -1,14 +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?
-
-    unless record.hholdcount >= 0 && record.hholdcount <= 4
-      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)

app/models/validations/shared_validations.rb

@@ -20,20 +20,16 @@ module Validations::SharedValidations
       next unless question.min || question.max
       next unless record[question.id]
 
-      field = question.check_answer_label || question.id
-      min = [question.prefix, number_with_delimiter(question.min, delimiter: ","), question.suffix].join("")
-      max = [question.prefix, number_with_delimiter(question.max, delimiter: ","), question.suffix].join("")
-
       begin
         answer = Float(record.public_send("#{question.id}_before_type_cast"))
       rescue ArgumentError
-        record.errors.add question.id.to_sym, I18n.t("validations.numeric.valid", field:, min:, max:)
+        add_range_error(record, question)
       end
 
       next unless answer
 
       if (question.min && question.min > answer) || (question.max && question.max < answer)
-        record.errors.add question.id.to_sym, I18n.t("validations.numeric.valid", field:, min:, max:)
+        add_range_error(record, question)
       end
     end
   end
@@ -108,4 +104,16 @@ private
   def person_is_partner?(relationship)
     relationship == "P"
   end
+
+  def add_range_error(record, question)
+    field = question.check_answer_label || question.id
+    min = [question.prefix, number_with_delimiter(question.min, delimiter: ","), question.suffix].join("") if question.min
+    max = [question.prefix, number_with_delimiter(question.max, delimiter: ","), question.suffix].join("") if question.max
+
+    if min && max
+      record.errors.add question.id.to_sym, I18n.t("validations.numeric.within_range", field:, min:, max:)
+    elsif min
+      record.errors.add question.id.to_sym, I18n.t("validations.numeric.above_min", field:, min:)
+    end
+  end
 end

config/locales/en.yml

@@ -131,7 +131,8 @@ en:
     other_field_missing: "If %{main_field_label} is other then %{other_field_label} must be provided"
     other_field_not_required: "%{other_field_label} must not be provided if %{main_field_label} was not other"
     numeric:
-      valid: "%{field} must be between %{min} and %{max}"
+      within_range: "%{field} must be between %{min} and %{max}"
+      above_min: "%{field} must be at least %{min}"
     date:
       invalid_date: "Enter a date in the correct format, for example 31 1 2022"
       outside_collection_window: "Enter a date within the current collection windows"
@@ -470,9 +471,10 @@ en:
       title_text: "You told us the time between the start of the tenancy and the major repairs completion date is more than 2 years"
     void_date:
       title_text: "You told us the time between the start of the tenancy and the void date is more than 2 years"
-    shared_owhership_deposit:
+    shared_ownership_deposit:
       title_text: "Mortgage, deposit and cash discount total should equal £%{expected_shared_ownership_deposit_value}"
     old_persons_shared_ownership: "At least one buyer should be aged over 64 for Older persons’ shared ownership scheme"
+    staircase_bought_seems_high: "You said %{percentage}% was bought in this staircasing transaction, which seems high. Are you sure?"
     monthly_charges_over_soft_max: 
       title_text: "The amount of monthly charges is high for this type of property and sale type"
 

spec/models/form/sales/pages/shared_ownership_deposit_value_check_spec.rb

@@ -33,7 +33,7 @@ RSpec.describe Form::Sales::Pages::SharedOwnershipDepositValueCheck, type: :mode
 
   it "has the correct title_text" do
     expect(page.title_text).to eq({
-      "translation" => "soft_validations.shared_owhership_deposit.title_text",
+      "translation" => "soft_validations.shared_ownership_deposit.title_text",
       "arguments" => [
         {
           "key" => "expected_shared_ownership_deposit_value",

spec/models/validations/household_validations_spec.rb

@@ -463,14 +463,14 @@ RSpec.describe Validations::HouseholdValidations do
         record.hhmemb = -1
         household_validator.validate_numeric_min_max(record)
         expect(record.errors["hhmemb"])
-          .to include(match I18n.t("validations.numeric.valid", field: "Number of Household Members", min: 0, max: 8))
+          .to include(match I18n.t("validations.numeric.within_range", field: "Number of Household Members", min: 0, max: 8))
       end
 
       it "validates that the number of household members cannot be more than 8" do
         record.hhmemb = 9
         household_validator.validate_numeric_min_max(record)
         expect(record.errors["hhmemb"])
-          .to include(match I18n.t("validations.numeric.valid", field: "Number of Household Members", min: 0, max: 8))
+          .to include(match I18n.t("validations.numeric.within_range", field: "Number of Household Members", min: 0, max: 8))
       end
 
       it "expects that the number of other household members is between the min and max" do

spec/models/validations/sales/household_validations_spec.rb

@@ -5,48 +5,6 @@ RSpec.describe Validations::Sales::HouseholdValidations do
 
   let(:validator_class) { Class.new { include Validations::Sales::HouseholdValidations } }
 
-  describe "#validate_number_of_other_people_living_in_the_property" do
-    context "when within permitted bounds" do
-      let(:record) { build(:sales_log, hholdcount: 2) }
-
-      it "does not add an error" do
-        household_validator.validate_number_of_other_people_living_in_the_property(record)
-
-        expect(record.errors[:hholdcount]).not_to be_present
-      end
-    end
-
-    context "when blank" do
-      let(:record) { build(:sales_log, hholdcount: nil) }
-
-      it "does not add an error" do
-        household_validator.validate_number_of_other_people_living_in_the_property(record)
-
-        expect(record.errors[:hholdcount]).not_to be_present
-      end
-    end
-
-    context "when below lower bound" do
-      let(:record) { build(:sales_log, hholdcount: -1) }
-
-      it "adds an error" do
-        household_validator.validate_number_of_other_people_living_in_the_property(record)
-
-        expect(record.errors[:hholdcount]).to be_present
-      end
-    end
-
-    context "when higher than upper bound" do
-      let(:record) { build(:sales_log, hholdcount: 5) }
-
-      it "adds an error" do
-        household_validator.validate_number_of_other_people_living_in_the_property(record)
-
-        expect(record.errors[:hholdcount]).to be_present
-      end
-    end
-  end
-
   describe "household member validations" do
     let(:record) { build(:sales_log) }
 

spec/models/validations/shared_validations_spec.rb

@@ -18,42 +18,42 @@ RSpec.describe Validations::SharedValidations do
         record.age1 = "random"
         shared_validator.validate_numeric_min_max(record)
         expect(record.errors["age1"])
-          .to include(match I18n.t("validations.numeric.valid", field: "Lead tenant’s age", min: 16, max: 120))
+          .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"])
-          .to include(match I18n.t("validations.numeric.valid", field: "Person 2’s age", min: 1, max: 120))
+          .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"])
-          .to include(match I18n.t("validations.numeric.valid", field: "Lead tenant’s age", min: 16, max: 120))
+          .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"])
-          .to include(match I18n.t("validations.numeric.valid", field: "Person 2’s age", min: 1, max: 120))
+          .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"])
-          .to include(match I18n.t("validations.numeric.valid", field: "Lead tenant’s age", min: 16, max: 120))
+          .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"])
-          .to include(match I18n.t("validations.numeric.valid", field: "Person 2’s age", min: 1, max: 120))
+          .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
@@ -69,21 +69,27 @@ RSpec.describe Validations::SharedValidations do
       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"))
+    end
+
     context "when validating percent" do
       it "validates that suffixes are added in the error message" do
         sales_record.stairbought = 150
         shared_validator.validate_numeric_min_max(sales_record)
         expect(sales_record.errors["stairbought"])
-          .to include(match I18n.t("validations.numeric.valid", field: "Percentage bought in this staircasing transaction", min: "0%", max: "100%"))
+          .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 = "random"
+        sales_record.income1 = -5
         shared_validator.validate_numeric_min_max(sales_record)
         expect(sales_record.errors["income1"])
-          .to include(match I18n.t("validations.numeric.valid", field: "Buyer 1’s gross annual income", min: "£0", max: "£999,999"))
+          .to include(match I18n.t("validations.numeric.within_range", field: "Buyer 1’s gross annual income", min: "£0", max: "£999,999"))
       end
     end
   end

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.valid", field: "Lead tenant’s age", min: 16, max: 120)]]])
+          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)]]])
         end
       end