Refactor other field validation

Gemfile

@@ -54,17 +54,18 @@ gem "sentry-rails"
 gem "sentry-ruby"
 
 group :development, :test do
-  # Call 'byebug' anywhere in the code to stop execution and get a debugger console
+  # Check gems for known vulnerabilities
   gem "bundler-audit"
+  # Call 'byebug' anywhere in the code to stop execution and get a debugger console
   gem "byebug", platforms: %i[mri mingw x64_mingw]
   gem "dotenv-rails"
   gem "pry-byebug"
 end
 
 group :development do
-  # Access an interactive console on exception pages or by calling 'console' anywhere in the code.
   gem "listen", "~> 3.3"
   gem "overcommit", ">= 0.37.0"
+  # Access an interactive console on exception pages or by calling 'console' anywhere in the code.
   gem "web-console", ">= 4.1.0"
   # Display performance information such as SQL time and flame graphs for each request in your browser.
   # Can be configured to work on production as well see: https://github.com/MiniProfiler/rack-mini-profiler/blob/master/README.md

app/models/case_log.rb

@@ -13,20 +13,6 @@ class CaseLogValidator < ActiveModel::Validator
     validation_methods = public_methods.select { |method| method.starts_with?("validate_") }
     validation_methods.each { |meth| public_send(meth, record) }
   end
-
-private
-
-  def validate_other_field(record, main_field, other_field)
-    main_field_label = main_field.humanize(capitalize: false)
-    other_field_label = other_field.humanize(capitalize: false)
-    if record[main_field] == "Other" && record[other_field].blank?
-      record.errors.add other_field.to_sym, "If #{main_field_label} is other then #{other_field_label} must be provided"
-    end
-
-    if record[main_field] != "Other" && record[other_field].present?
-      record.errors.add other_field.to_sym, "#{other_field_label} must not be provided if #{main_field_label} was not other"
-    end
-  end
 end
 
 class CaseLog < ApplicationRecord

app/models/validations/household_validations.rb

@@ -1,4 +1,5 @@
 module Validations::HouseholdValidations
+  include Validations::SharedValidations
   include Constants::CaseLog
 
   # Validations methods need to be called 'validate_<page_name>' to run on model save
@@ -14,14 +15,11 @@ module Validations::HouseholdValidations
     end
   end
 
-  def validate_other_reason_for_leaving_last_settled_home(record)
-    validate_other_field(record, "reason", "other_reason_for_leaving_last_settled_home")
-  end
-
   def validate_reason_for_leaving_last_settled_home(record)
     if record.reason == "Don’t know" && record.underoccupation_benefitcap != "Don’t know"
       record.errors.add :underoccupation_benefitcap, I18n.t("validations.household.underoccupation_benefitcap.dont_know_required")
     end
+    validate_other_field(record, :reason, :other_reason_for_leaving_last_settled_home)
   end
 
   def validate_armed_forces_injured(record)

app/models/validations/shared_validations.rb

@@ -0,0 +1,15 @@
+module Validations::SharedValidations
+  def validate_other_field(record, main_field = nil, other_field = nil)
+    return unless main_field || other_field
+
+    main_field_label = main_field.to_s.humanize(capitalize: false)
+    other_field_label = other_field.to_s.humanize(capitalize: false)
+    if record[main_field] == "Other" && record[other_field].blank?
+      record.errors.add other_field.to_sym, I18n.t("validations.other_field_missing", main_field_label:, other_field_label:)
+    end
+
+    if record[main_field] != "Other" && record[other_field].present?
+      record.errors.add other_field.to_sym, I18n.t("validations.other_field_not_required", main_field_label:, other_field_label:)
+    end
+  end
+end

app/models/validations/tenancy_validations.rb

@@ -1,6 +1,8 @@
 module Validations::TenancyValidations
   # Validations methods need to be called 'validate_<page_name>' to run on model save
   # or 'validate_' to run on submit as well
+  include Validations::SharedValidations
+
   def validate_fixed_term_tenancy(record)
     is_present = record.tenancylength.present?
     is_in_range = record.tenancylength.to_i.between?(2, 99)
@@ -21,6 +23,6 @@ module Validations::TenancyValidations
   end
 
   def validate_other_tenancy_type(record)
-    validate_other_field(record, "tenancy", "tenancyother")
+    validate_other_field(record, :tenancy, :tenancyother)
   end
 end

config/locales/en.yml

@@ -35,6 +35,8 @@ en:
     updated: "Organisation details updated"
 
   validations:
+    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"
     date:
       invalid_date: "Please enter a valid date"
       outside_collection_window: "Date must be within the current collection windows"

spec/models/case_log_spec.rb

@@ -56,15 +56,6 @@ RSpec.describe CaseLog do
     end
 
     context "with reason for leaving last settled home validation set to other" do
-      it "must be provided if main reason for leaving last settled home was given as other" do
-        expect {
-          described_class.create!(reason: "Other",
-                                  other_reason_for_leaving_last_settled_home: nil,
-                                  owning_organisation:,
-                                  managing_organisation:)
-        }.to raise_error(ActiveRecord::RecordInvalid)
-      end
-
       it "must not be provided if the main reason for leaving settled home is not other" do
         expect {
           described_class.create!(reason: "Repossession",

spec/models/validations/household_validations_spec.rb

@@ -165,4 +165,20 @@ RSpec.describe Validations::HouseholdValidations do
       end
     end
   end
+
+  describe "reason for leaving last settled home validations" do
+    context "when reason is other" do
+      it "validates that a reason is provided" do
+        record.reason = "Other"
+        record.other_reason_for_leaving_last_settled_home = nil
+        household_validator.validate_reason_for_leaving_last_settled_home(record)
+        expect(record.errors["other_reason_for_leaving_last_settled_home"])
+          .to include(match I18n.t(
+            "validations.other_field_missing",
+            main_field_label: "reason",
+            other_field_label: "other reason for leaving last settled home",
+          ))
+      end
+    end
+  end
 end