[CRIMAPP-1403] Update payment fieldset form validation (#1307)

* Update payment fieldset form validation

* Fix failing specs

* Fix linting error

* Fix failing spec

* Refactor

* Code coverage fix

app/forms/concerns/steps/payment_fieldset_validation.rb

@@ -0,0 +1,19 @@
+module Steps
+  module PaymentFieldsetValidation
+    extend ActiveSupport::Concern
+
+    def validate_frequency
+      errors.add(:frequency, :blank, payment_type: payment_type_label) if frequency.blank?
+      return unless frequencies.exclude?(frequency)
+
+      errors.add(:frequency, :inclusion,
+                 payment_type: payment_type_label&.capitalize)
+    end
+
+    def validate_amount
+      errors.add(:amount, :blank, payment_type: payment_type_label) if amount.blank?
+      errors.add(:amount, :not_a_number, payment_type: payment_type_label) if Type::Pence.new.serialize(amount).nil?
+      errors.add(:amount, :greater_than, payment_type: payment_type_label) if amount.to_i <= 0
+    end
+  end
+end

app/forms/steps/income/income_benefit_fieldset_form.rb

@@ -1,22 +1,17 @@
 module Steps
   module Income
     class IncomeBenefitFieldsetForm < Steps::BaseFormObject
+      include Steps::PaymentFieldsetValidation
+
       attribute :id, :string
       attribute :payment_type, :string
       attribute :amount, :pence
       attribute :frequency, :string
       attribute :details, :string
 
-      validate { presence_with_payment_type :amount }
-      validate { presence_with_payment_type :frequency }
-
-      validates :amount, numericality: {
-        greater_than: 0
-      }
-
+      validate :validate_amount
+      validate :validate_frequency
       validates :payment_types, presence: true, inclusion: { in: :payment_types }
-      validates :frequency, inclusion: { in: :frequencies }
-
       validate :details_only_when_other?
 
       # Needed for `#fields_for` to render the uuids as hidden fields
@@ -51,24 +46,15 @@ def details_only_when_other?
         if (payment_type == IncomeBenefitType::OTHER.to_s) && details.blank?
           errors.add(:details, :blank)
         elsif (payment_type != IncomeBenefitType::OTHER.to_s) && details.present?
-          errors.add(:details, :invalid)
+          errors.add(:details, :invalid, payment_type: payment_type_label)
         end
       end
 
-      def presence_with_payment_type(attribute)
-        return if send(attribute).present?
-
-        payment_type_str = I18n.t(
+      def payment_type_label
+        I18n.t(
           payment_type,
           scope: [:helpers, :label, :steps_income_income_benefits_form, :types_options]
-        )
-        payment_type_str&.downcase! if payment_type == IncomeBenefitType::OTHER.to_s
-
-        errors.add(
-          attribute,
-          :blank,
-          payment_type: payment_type_str
-        )
+        )&.downcase!
       end
     end
   end

app/forms/steps/income/income_payment_fieldset_form.rb

@@ -1,22 +1,17 @@
 module Steps
   module Income
     class IncomePaymentFieldsetForm < Steps::BaseFormObject
+      include Steps::PaymentFieldsetValidation
+
       attribute :id, :string
       attribute :payment_type, :string
       attribute :amount, :pence
       attribute :frequency, :string
       attribute :details, :string
 
-      validate { presence_with_payment_type :amount }
-      validate { presence_with_payment_type :frequency }
-
-      validates :amount, numericality: {
-        greater_than: 0
-      }
-
+      validate :validate_amount
+      validate :validate_frequency
       validates :payment_types, presence: true, inclusion: { in: :payment_types }
-      validates :frequency, inclusion: { in: :frequencies }
-
       validate :details_only_when_other?
 
       def payment_types
@@ -51,23 +46,15 @@ def details_only_when_other?
         if (payment_type == IncomePaymentType::OTHER.to_s) && details.blank?
           errors.add(:details, :blank)
         elsif (payment_type != IncomePaymentType::OTHER.to_s) && details.present?
-          errors.add(:details, :invalid)
+          errors.add(:details, :invalid, payment_type: payment_type_label)
         end
       end
 
-      def presence_with_payment_type(attribute)
-        return if send(attribute).present?
-
-        payment_type_str = I18n.t(
+      def payment_type_label
+        I18n.t(
           payment_type,
           scope: [:helpers, :label, :steps_income_income_payments_form, :types_options]
         )&.downcase!
-
-        errors.add(
-          attribute,
-          :blank,
-          payment_type: payment_type_str
-        )
       end
     end
   end

app/forms/steps/income/partner/income_benefit_fieldset_form.rb

@@ -2,22 +2,17 @@ module Steps
   module Income
     module Partner
       class IncomeBenefitFieldsetForm < Steps::BaseFormObject
+        include Steps::PaymentFieldsetValidation
+
         attribute :id, :string
         attribute :payment_type, :string
         attribute :amount, :pence
         attribute :frequency, :string
         attribute :details, :string
 
-        validate { presence_with_payment_type :amount }
-        validate { presence_with_payment_type :frequency }
-
-        validates :amount, numericality: {
-          greater_than: 0
-        }
-
+        validate :validate_amount
+        validate :validate_frequency
         validates :payment_types, presence: true, inclusion: { in: :payment_types }
-        validates :frequency, inclusion: { in: :frequencies }
-
         validate :details_only_when_other?
 
         # Needed for `#fields_for` to render the uuids as hidden fields
@@ -56,20 +51,11 @@ def details_only_when_other?
           end
         end
 
-        def presence_with_payment_type(attribute)
-          return if send(attribute).present?
-
-          payment_type_str = I18n.t(
+        def payment_type_label
+          I18n.t(
             payment_type,
             scope: [:helpers, :label, :steps_income_income_benefits_form, :types_options]
-          )
-          payment_type_str&.downcase! if payment_type == IncomeBenefitType::OTHER.to_s
-
-          errors.add(
-            attribute,
-            :blank,
-            payment_type: payment_type_str
-          )
+          )&.downcase!
         end
       end
     end

app/forms/steps/income/partner/income_payment_fieldset_form.rb

@@ -2,22 +2,17 @@ module Steps
   module Income
     module Partner
       class IncomePaymentFieldsetForm < Steps::BaseFormObject
+        include Steps::PaymentFieldsetValidation
+
         attribute :id, :string
         attribute :payment_type, :string
         attribute :amount, :pence
         attribute :frequency, :string
         attribute :details, :string
 
-        validate { presence_with_payment_type :amount }
-        validate { presence_with_payment_type :frequency }
-
-        validates :amount, numericality: {
-          greater_than: 0
-        }
-
+        validate :validate_amount
+        validate :validate_frequency
         validates :payment_types, presence: true, inclusion: { in: :payment_types }
-        validates :frequency, inclusion: { in: :frequencies }
-
         validate :details_only_when_other?
 
         def payment_types
@@ -52,23 +47,15 @@ def details_only_when_other?
           if (payment_type == IncomePaymentType::OTHER.to_s) && details.blank?
             errors.add(:details, :blank)
           elsif (payment_type != IncomePaymentType::OTHER.to_s) && details.present?
-            errors.add(:details, :invalid)
+            errors.add(:details, :invalid, payment_type: payment_type_label)
           end
         end
 
-        def presence_with_payment_type(attribute)
-          return if send(attribute).present?
-
-          payment_type_str = I18n.t(
+        def payment_type_label
+          I18n.t(
             payment_type,
             scope: [:helpers, :label, :steps_income_income_payments_form, :types_options]
           )&.downcase!
-
-          errors.add(
-            attribute,
-            :blank,
-            payment_type: payment_type_str
-          )
         end
       end
     end

app/forms/steps/outgoings/outgoing_payment_fieldset_form.rb

@@ -1,19 +1,17 @@
 module Steps
   module Outgoings
     class OutgoingPaymentFieldsetForm < Steps::BaseFormObject
+      include Steps::PaymentFieldsetValidation
+
       attribute :id, :string
       attribute :payment_type, :string
       attribute :amount, :pence
       attribute :frequency, :string
       attribute :case_reference, :string
 
-      validates :amount, numericality: {
-        greater_than: 0
-      }
-
+      validate :validate_amount
+      validate :validate_frequency
       validates :payment_types, presence: true, inclusion: { in: :payment_types }
-      validates :frequency, presence: true, inclusion: { in: :frequencies }
-
       validate :case_ref_when_legal_aid_contribution?
 
       # Needed for `#fields_for` to render the uuids as hidden fields
@@ -51,6 +49,13 @@ def case_ref_when_legal_aid_contribution?
           errors.add(:case_reference, :invalid)
         end
       end
+
+      def payment_type_label
+        I18n.t(
+          payment_type,
+          scope: [:helpers, :label, :steps_outgoings_outgoings_payments_form, :types_options]
+        )&.downcase!
+      end
     end
   end
 end

app/validators/base_payments_validator.rb

@@ -0,0 +1,38 @@
+class BasePaymentsValidator < ActiveModel::Validator
+  attr_reader :record
+
+  def validate(record)
+    @record = record
+
+    record.types.each do |type|
+      next if type == 'none'
+
+      payment = record.public_send(type)
+      add_errors(payment) unless payment.valid?
+    end
+
+    return unless record.types.empty?
+
+    record.errors.add(:base, :none_selected) if has_no_payments?
+  end
+
+  # :nocov:
+  def has_no_payments?
+    raise 'must be implemented in subclasses'
+  end
+  # :nocov:
+
+  private
+
+  def add_errors(payment)
+    payment.errors.each do |error|
+      attr_name = "#{payment.payment_type.dasherize}-#{error.attribute}"
+      record.errors.add(attr_name, error.type, message: error.message)
+
+      # We define the attribute getter as it doesn't really exist
+      record.define_singleton_method(attr_name) do
+        payment.public_send(error.attribute)
+      end
+    end
+  end
+end

app/validators/income_benefits_validator.rb

@@ -1,56 +1,5 @@
-class IncomeBenefitsValidator < ActiveModel::Validator
-  attr_reader :record
-
-  def validate(record)
-    @record = record
-
-    record.types.each_with_index do |type, index|
-      next if type == 'none'
-
-      income_benefit = record.public_send(type)
-      add_indexed_errors(income_benefit, index) unless income_benefit.valid?
-    end
-
-    return unless record.types.empty?
-
-    record.errors.add(:base, :none_selected) if record.has_no_income_benefits.blank?
-  end
-
-  private
-
-  def add_indexed_errors(income_benefit, index)
-    income_benefit.errors.each do |error|
-      attr_name = indexed_attribute(index, income_benefit, error.attribute)
-
-      record.errors.add(
-        attr_name,
-        error.type,
-        message: error_message(income_benefit, error)
-      )
-
-      # We define the attribute getter as it doesn't really exist
-      record.define_singleton_method(attr_name) do
-        income_benefit.public_send(error.attribute)
-      end
-    end
-  end
-
-  def indexed_attribute(_index, income_benefit, attr)
-    "#{income_benefit.payment_type.dasherize}-#{attr}"
-  end
-
-  # `activemodel.errors.models.steps/income/income_benefit_fieldset_form.summary.x.y`
-  def error_message(obj, error)
-    payment_type = I18n.t(
-      obj.payment_type,
-      scope: [:helpers, :label, :steps_income_income_benefits_form, :types_options]
-    )
-    payment_type&.downcase! if obj.payment_type == IncomeBenefitType::OTHER.to_s
-
-    I18n.t(
-      "#{obj.model_name.i18n_key}.summary.#{error.attribute}.#{error.type}",
-      scope: [:activemodel, :errors, :models],
-      payment_type: payment_type
-    )
+class IncomeBenefitsValidator < BasePaymentsValidator
+  def has_no_payments?
+    record.has_no_income_benefits.blank?
   end
 end