CLDC-3837: Fix bug with step validation for numeric fields with step 0.1 (#2893)

* CLDC-3837: Fix bug with step validation for numeric fields with step 0.1

* Refactor tests

* Fix test

app/models/validations/shared_validations.rb

@@ -54,14 +54,15 @@ module Validations::SharedValidations
       field = question.check_answer_label || question.id
       incorrect_accuracy = (value.to_d * 100) % (question.step * 100) != 0
 
-      if question.step < 1 && incorrect_accuracy
+      next unless incorrect_accuracy
-        record.errors.add question.id.to_sym, I18n.t("validations.shared.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
+      case question.step
-        field = question.check_answer_label || question.id
+      when 0.01 then record.errors.add question.id.to_sym, I18n.t("validations.shared.numeric.nearest_hundredth", field:)
-        case question.step
+      when 0.1 then record.errors.add question.id.to_sym, I18n.t("validations.shared.numeric.nearest_tenth", field:)
-        when 1 then record.errors.add question.id.to_sym, :not_integer, message: I18n.t("validations.shared.numeric.whole_number", field:)
+      when 1 then record.errors.add question.id.to_sym, :not_integer, message: I18n.t("validations.shared.numeric.whole_number", field:)
-        when 10 then record.errors.add question.id.to_sym, I18n.t("validations.shared.numeric.nearest_ten", field:)
+      when 10 then record.errors.add question.id.to_sym, I18n.t("validations.shared.numeric.nearest_ten", field:)
-        end
+      else
+        record.errors.add question.id.to_sym, I18n.t("validations.shared.numeric.nearest_step", field:, step: question.step)
       end
     end
   end

config/locales/validations/shared.en.yml

@@ -9,7 +9,9 @@ en:
         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_tenth: "%{field} must be given to the nearest tenth."
         nearest_hundredth: "%{field} must be given to the nearest hundredth."
+        nearest_step: "${field} must be given to the nearest %{step}."
         normal_format: "Enter a number."
         format: "%{field} must be a number."
 

spec/models/validations/shared_validations_spec.rb

@@ -116,19 +116,23 @@ RSpec.describe Validations::SharedValidations do
   end
 
   describe "validating level of accuracy or rounding for numeric questions" do
+    let(:sales_log) { build(:sales_log, :completed) }
+
+    before do
+      income_question = instance_double(Form::Question, step:, type: "numeric", id: "income1", check_answer_label: "Buyer 1’s gross annual income", page: instance_double(Form::Page, routed_to?: true))
+      form = instance_double(Form, numeric_questions: [income_question])
+      allow(FormHandler.instance).to receive(:get_form).and_return(form)
+    end
+
     context "when validating a question with a step of 1" do
+      let(:step) { 1 }
+
       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.shared.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.shared.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)
@@ -137,80 +141,96 @@ RSpec.describe Validations::SharedValidations do
     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
+      let(:step) { 10 }
-        sales_log.savings = 30_005
-        sales_log.jointpur = 1
-        shared_validator.validate_numeric_step(sales_log)
-        expect(sales_log.errors[:savings]).to include I18n.t("validations.shared.numeric.nearest_ten", field: "Buyers’ total savings before any deposit paid")
-      end
 
-      it "adds an error if the user attempts to input a number in exponent format" do
+      it "adds an error if input is not a multiple of ten" do
-        sales_log.savings = "3e5"
+        sales_log.income1 = 30_005
-        sales_log.jointpur = 1
         shared_validator.validate_numeric_step(sales_log)
-        expect(sales_log.errors[:savings]).to include I18n.t("validations.shared.numeric.nearest_ten", field: "Buyers’ total savings before any deposit paid")
+        expect(sales_log.errors[:income1]).to include I18n.t("validations.shared.numeric.nearest_ten", field: "Buyer 1’s gross annual income")
       end
 
       it "does not add an error if input is a multiple of ten" do
-        sales_log.savings = 30_000
+        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 0.01" do
+      let(:step) { 0.01 }
+
       it "adds an error if input has more than 2 decimal places" do
-        sales_log.mscharge = 30.7418
+        sales_log.income1 = 30_123.7418
         shared_validator.validate_numeric_step(sales_log)
-        expect(sales_log.errors[:mscharge]).to include I18n.t("validations.shared.numeric.nearest_hundredth", field: "Monthly leasehold charges")
+        expect(sales_log.errors[:income1]).to include I18n.t("validations.shared.numeric.nearest_hundredth", field: "Buyer 1’s gross annual income")
       end
 
-      it "does not add an error if the user attempts to input a number in exponent format" do
+      it "does not add an error if input has 2 or fewer decimal places" do
-        sales_log.mscharge = "3e1"
+        sales_log.income1 = 30_123.74
         shared_validator.validate_numeric_step(sales_log)
         expect(sales_log.errors).to be_empty
       end
+    end
 
-      it "does not add an error if input has 2 or fewer decimal places" do
+    context "when validating a question with a step of 0.1" do
-        sales_log.mscharge = 30.74
+      let(:step) { 0.1 }
+
+      it "adds an error if input has more than 1 decimal place" do
+        sales_log.income1 = 30_123.74
+        shared_validator.validate_numeric_step(sales_log)
+        expect(sales_log.errors[:income1]).to include I18n.t("validations.shared.numeric.nearest_tenth", field: "Buyer 1’s gross annual income")
+      end
+
+      it "does not add an error if input has 1 or fewer decimal places" do
+        sales_log.income1 = 30_123.8
         shared_validator.validate_numeric_step(sales_log)
         expect(sales_log.errors).to be_empty
       end
     end
 
-    %i[sales_log lettings_log].each do |log_type|
+    context "when validating a question with an unusual step" do
-      describe "validate_owning_organisation_data_sharing_agremeent_signed" do
+      let(:step) { 0.001 }
-        it "is valid if the Data Protection Confirmation is signed" do
-          log = build(log_type, :in_progress, owning_organisation: create(:organisation))
 
-          expect(log).to be_valid
+      it "adds an appropriate error if input does not match" do
-        end
+        sales_log.income1 = 30_123.7419
+        shared_validator.validate_numeric_step(sales_log)
+        expect(sales_log.errors[:income1]).to include I18n.t("validations.shared.numeric.nearest_step", field: "Buyer 1’s gross annual income", step: 0.001)
+      end
+    end
+  end
 
-        it "is valid when owning_organisation nil" do
+  %i[sales_log lettings_log].each do |log_type|
-          log = build(log_type, owning_organisation: nil)
+    describe "validate_owning_organisation_data_sharing_agremeent_signed" do
+      it "is valid if the Data Protection Confirmation is signed" do
+        log = build(log_type, :in_progress, owning_organisation: create(:organisation))
 
-          expect(log).to be_valid
+        expect(log).to be_valid
-        end
+      end
 
-        it "is not valid if the Data Protection Confirmation is not signed" do
+      it "is valid when owning_organisation nil" do
-          log = build(log_type, owning_organisation: create(:organisation, :without_dpc))
+        log = build(log_type, owning_organisation: nil)
 
-          expect(log).not_to be_valid
+        expect(log).to be_valid
-          expect(log.errors[:owning_organisation_id]).to eq(["The organisation must accept the Data Sharing Agreement before it can be selected as the owning organisation."])
+      end
-        end
+
+      it "is not valid if the Data Protection Confirmation is not signed" do
+        log = build(log_type, owning_organisation: create(:organisation, :without_dpc))
 
-        context "when updating" do
+        expect(log).not_to be_valid
-          let(:log) { create(log_type, :in_progress) }
+        expect(log.errors[:owning_organisation_id]).to eq(["The organisation must accept the Data Sharing Agreement before it can be selected as the owning organisation."])
-          let(:org_with_dpc) { create(:organisation) }
+      end
-          let(:org_without_dpc) { create(:organisation, :without_dpc) }
 
-          it "is valid when changing to another org with a signed Data Protection Confirmation" do
+      context "when updating" do
-            expect { log.owning_organisation = org_with_dpc }.not_to change(log, :valid?)
+        let(:log) { create(log_type, :in_progress) }
-          end
+        let(:org_with_dpc) { create(:organisation) }
+        let(:org_without_dpc) { create(:organisation, :without_dpc) }
+
+        it "is valid when changing to another org with a signed Data Protection Confirmation" do
+          expect { log.owning_organisation = org_with_dpc }.not_to change(log, :valid?)
+        end
 
-          it "invalid when changing to another org without a signed Data Protection Confirmation" do
+        it "invalid when changing to another org without a signed Data Protection Confirmation" do
-            expect { log.owning_organisation = org_without_dpc }.to change(log, :valid?).from(true).to(false).and(change { log.errors[:owning_organisation_id] }.to(["The organisation must accept the Data Sharing Agreement before it can be selected as the owning organisation."]))
+          expect { log.owning_organisation = org_without_dpc }.to change(log, :valid?).from(true).to(false).and(change { log.errors[:owning_organisation_id] }.to(["The organisation must accept the Data Sharing Agreement before it can be selected as the owning organisation."]))
-          end
         end
       end
     end
@@ -229,6 +249,12 @@ RSpec.describe Validations::SharedValidations do
       expect(sales_log.errors[:income1]).to include I18n.t("validations.shared.numeric.format", field: "Buyer 1’s gross annual income")
     end
 
+    it "does not allow exponent format" do
+      sales_log.income1 = "1e4"
+      shared_validator.validate_numeric_input(sales_log)
+      expect(sales_log.errors[:income1]).to include I18n.t("validations.shared.numeric.format", field: "Buyer 1’s gross annual income")
+    end
+
     it "allows a digit" do
       sales_log.income1 = "300"
       shared_validator.validate_numeric_input(sales_log)