Update shortfall validation (#709)

* Update shortfall validation

* Cop

* Fix suffix label for outstanding amount

app/models/derived_variables/case_log_variables.rb

@@ -34,15 +34,13 @@ module DerivedVariables::CaseLogVariables
       self.wpschrge = weekly_value(pscharge) if pscharge.present?
       self.wsupchrg = weekly_value(supcharg) if supcharg.present?
       self.wtcharge = weekly_value(tcharge) if tcharge.present?
-      if is_supported_housing? && chcharge.present?
+      self.wchchrg = weekly_value(chcharge) if is_supported_housing? && chcharge.present?
-        self.wchchrg = weekly_value(chcharge)
-      end
     end
-    self.has_benefits = get_has_benefits
+    self.wtshortfall = if tshortfall && receives_housing_related_benefits? && period
-    self.tshortfall_known = 0 if tshortfall
-    self.wtshortfall = if tshortfall && receives_housing_related_benefits?
                          weekly_value(tshortfall)
                        end
+    self.has_benefits = get_has_benefits
+    self.tshortfall_known = 0 if tshortfall
     self.nocharge = household_charge&.zero? ? 1 : 0
     self.housingneeds = get_housingneeds
     if is_renewal?

app/models/form/question.rb

@@ -173,6 +173,24 @@ class Form::Question
     type == "radio" && RADIO_REFUSED_VALUE[id.to_sym]&.include?(value)
   end
 
+  def suffix_label(case_log)
+    return "" unless suffix
+    return suffix if suffix.is_a?(String)
+
+    label = ""
+
+    suffix.each do |s|
+      condition = s["depends_on"]
+      next unless condition
+
+      answer = case_log.send(condition.keys.first)
+      if answer == condition.values.first
+        label = s["label"]
+      end
+    end
+    label
+  end
+
 private
 
   def selected_answer_option_is_derived?(case_log)
@@ -196,24 +214,6 @@ private
     prefix == "£" ? ActionController::Base.helpers.number_to_currency(answer_label, delimiter: ",", format: "%n") : answer_label
   end
 
-  def suffix_label(case_log)
-    return "" unless suffix
-    return suffix if suffix.is_a?(String)
-
-    label = ""
-
-    suffix.each do |s|
-      condition = s["depends_on"]
-      next unless condition
-
-      answer = case_log.send(condition.keys.first)
-      if answer == condition.values.first
-        label = ANSWER_SUFFIX_LABELS.key?(answer) ? ANSWER_SUFFIX_LABELS[answer] : answer
-      end
-    end
-    label
-  end
-
   def conditional_on
     @conditional_on ||= form.conditional_question_conditions.select do |condition|
       condition[:to] == id
@@ -237,12 +237,6 @@ private
     inferred_answers.filter { |_key, value| value.all? { |condition_key, condition_value| case_log[condition_key] == condition_value } }
   end
 
-  ANSWER_SUFFIX_LABELS = {
-    1 => " every week",
-    2 => " every month",
-    3 => " every year",
-  }.freeze
-
   RADIO_YES_VALUE = {
     renewal: [1],
     postcode_known: [1],

app/models/validations/financial_validations.rb

@@ -71,9 +71,13 @@ module Validations::FinancialValidations
   end
 
   def validate_rent_amount(record)
-    if record.brent.present? && record.tshortfall.present? && record.brent < record.tshortfall * 2
+    if record.wtshortfall
-      record.errors.add :brent, I18n.t("validations.financial.rent.less_than_double_shortfall", tshortfall: record.tshortfall * 2)
+      if record.wrent && (record.wtshortfall > record.wrent)
-      record.errors.add :tshortfall, I18n.t("validations.financial.tshortfall.more_than_rent")
+        record.errors.add :tshortfall, I18n.t("validations.financial.tshortfall.more_than_rent")
+        record.errors.add :brent, I18n.t("validations.financial.rent.less_than_shortfall")
+      elsif record.wtshortfall < 0.01
+        record.errors.add :tshortfall, I18n.t("validations.financial.tshortfall.must_be_positive")
+      end
     end
 
     if record.tcharge.present? && weekly_value_in_range(record, "tcharge", 0, 9.99)

app/views/form/_numeric_output_question.html.erb

@@ -17,6 +17,6 @@
             name="case_log[tcharge]"
             for="<%= question.fields_added.present? ? question.fields_added.map { |x| "case-log-#{x}-field" }.join(" ") : "" %>">
       <%= case_log[question.id] %></output>
-    <span class="govuk-input__suffix"><%= question.suffix %></span>
+    <span class="govuk-input__suffix"><%= question.suffix_label(case_log) %></span>
   </div>
 </div>

app/views/form/_numeric_question.html.erb

@@ -8,5 +8,5 @@
   width: question.width,
   readonly: question.read_only?,
   prefix_text: question.prefix.to_s,
-  suffix_text: question.suffix.is_a?(String) ? question.suffix : nil,
+  suffix_text: question.suffix_label(case_log),
   **stimulus_html_attributes(question) %>

config/forms/2021_2022.json

@@ -6799,19 +6799,19 @@
                   "prefix": "£",
                   "suffix": [
                     {
-                      "label": "every week",
+                      "label": " every week",
                       "depends_on": {
                         "incfreq": 1
                       }
                     },
                     {
-                      "label": "every month",
+                      "label": " every month",
                       "depends_on": {
                         "incfreq": 2
                       }
                     },
                     {
-                      "label": "every year",
+                      "label": " every year",
                       "depends_on": {
                         "incfreq": 3
                       }
@@ -7906,7 +7906,44 @@
                   "step": 0.01,
                   "width": 5,
                   "prefix": "£",
-                  "suffix": " every month"
+                  "suffix": [
+                    {
+                      "label": " every 2 weeks",
+                      "depends_on": { "period": 2 }
+                    },
+                    {
+                      "label": " every 4 weeks",
+                      "depends_on": { "period": 3 }
+                    },
+                    {
+                      "label": " every calendar month",
+                      "depends_on": { "period": 4 }
+                    },
+                    {
+                      "label": " every week for 50 weeks",
+                      "depends_on": { "period": 5 }
+                    },
+                    {
+                      "label": " every week for 49 weeks",
+                      "depends_on": { "period": 6 }
+                    },
+                    {
+                      "label": " every week for 48 weeks",
+                      "depends_on": { "period": 7 }
+                    },
+                    {
+                      "label": " every week for 47 weeks",
+                      "depends_on": { "period": 8 }
+                    },
+                    {
+                      "label": " every week for 46 weeks",
+                      "depends_on": { "period": 9 }
+                    },
+                    {
+                      "label": " every week for 52 weeks",
+                      "depends_on": { "period": 1 }
+                    }
+                  ]
                 }
               },
               "depends_on": [

config/forms/2022_2023.json

@@ -6758,19 +6758,19 @@
                   "prefix": "£",
                   "suffix": [
                     {
-                      "label": "every week",
+                      "label": " every week",
                       "depends_on": {
                         "incfreq": 1
                       }
                     },
                     {
-                      "label": "every month",
+                      "label": " every month",
                       "depends_on": {
                         "incfreq": 2
                       }
                     },
                     {
-                      "label": "every year",
+                      "label": " every year",
                       "depends_on": {
                         "incfreq": 3
                       }
@@ -7854,7 +7854,44 @@
                   "step": 0.01,
                   "width": 5,
                   "prefix": "£",
-                  "suffix": " every month"
+                  "suffix": [
+                    {
+                      "label": " every 2 weeks",
+                      "depends_on": { "period": 2 }
+                    },
+                    {
+                      "label": " every 4 weeks",
+                      "depends_on": { "period": 3 }
+                    },
+                    {
+                      "label": " every calendar month",
+                      "depends_on": { "period": 4 }
+                    },
+                    {
+                      "label": " every week for 50 weeks",
+                      "depends_on": { "period": 5 }
+                    },
+                    {
+                      "label": " every week for 49 weeks",
+                      "depends_on": { "period": 6 }
+                    },
+                    {
+                      "label": " every week for 48 weeks",
+                      "depends_on": { "period": 7 }
+                    },
+                    {
+                      "label": " every week for 47 weeks",
+                      "depends_on": { "period": 8 }
+                    },
+                    {
+                      "label": " every week for 46 weeks",
+                      "depends_on": { "period": 9 }
+                    },
+                    {
+                      "label": " every week for 52 weeks",
+                      "depends_on": { "period": 1 }
+                    }
+                  ]
                 }
               },
               "depends_on": [

config/locales/en.yml

@@ -104,7 +104,8 @@ en:
     financial:
       tshortfall:
         outstanding_amount_not_required: "You must not answer the outstanding amount question if you don’t have outstanding rent or charges"
-        more_than_rent: "Answer must be less than half of the basic rent amount"
+        more_than_rent: "Answer must be less than the basic rent amount"
+        must_be_positive: "Answer must be more than £0.01 as you told us there is an outstanding amount"
       hbrentshortfall:
         outstanding_no_benefits: "Answer cannot be ‘yes’ to outstanding amount for basic rent or charges if tenant does not receive housing benefit or Universal Credit or you‘re not sure"
       benefits:
@@ -116,7 +117,7 @@ en:
         earnings_missing: "Enter how much income the household has in total"
       negative_currency: "Enter an amount above 0"
       rent:
-        less_than_double_shortfall: "Answer must be more than double the shortfall in basic rent"
+        less_than_shortfall: "Answer must be more than the shortfall in basic rent"
         scharge:
           private_registered_provider:
             general_needs: "Service charge must be between £0 and £55 per week if the landlord is a private registered provider and it is a general needs letting"

spec/fixtures/forms/2021_2022.json

@@ -568,9 +568,9 @@
                     "width": 5,
                     "prefix": "£",
                     "suffix": [
-                      { "label": "every week", "depends_on" : { "incfreq": 1 } },
+                      { "label": " every week", "depends_on" : { "incfreq": 1 } },
-                      { "label": "every month", "depends_on" : { "incfreq": 2 } },
+                      { "label": " every month", "depends_on" : { "incfreq": 2 } },
-                      { "label": "every month", "depends_on" : { "incfreq": 3 } }
+                      { "label": " every year", "depends_on" : { "incfreq": 3 } }
                     ]
                   },
                   "incfreq": {

spec/models/validations/financial_validations_spec.rb

@@ -77,12 +77,41 @@ RSpec.describe Validations::FinancialValidations do
     end
 
     context "when outstanding rent or charges is yes" do
+      let(:record) { FactoryBot.create(:case_log, :about_completed) }
+
       it "expects that a shortfall is provided" do
         record.hbrentshortfall = 1
         record.tshortfall = 99
         financial_validator.validate_outstanding_rent_amount(record)
         expect(record.errors["tshortfall"]).to be_empty
       end
+
+      it "validates that the shortfall is a positive £ amount" do
+        record.hb = 6
+        record.hbrentshortfall = 1
+        record.tshortfall_known = 0
+        record.tshortfall = 0
+        record.period = 2
+        record.set_derived_fields!
+        financial_validator.validate_rent_amount(record)
+        expect(record.errors["tshortfall"])
+          .to include(match I18n.t("validations.financial.tshortfall.must_be_positive"))
+      end
+
+      it "validates that basic rent is no less than the shortfall" do
+        record.hb = 6
+        record.hbrentshortfall = 1
+        record.tshortfall_known = 0
+        record.tshortfall = 299.50
+        record.brent = 198
+        record.period = 2
+        record.set_derived_fields!
+        financial_validator.validate_rent_amount(record)
+        expect(record.errors["brent"])
+          .to include(match I18n.t("validations.financial.rent.less_than_shortfall"))
+        expect(record.errors["tshortfall"])
+          .to include(match I18n.t("validations.financial.tshortfall.more_than_rent"))
+      end
     end
   end
 
@@ -213,19 +242,6 @@ RSpec.describe Validations::FinancialValidations do
   end
 
   describe "rent and charges validations" do
-    context "when shortfall amount is provided" do
-      it "validates that basic rent is no less than double the shortfall" do
-        record.hbrentshortfall = 2
-        record.tshortfall = 99.50
-        record.brent = 198
-        financial_validator.validate_rent_amount(record)
-        expect(record.errors["brent"])
-          .to include(match I18n.t("validations.financial.rent.less_than_double_shortfall", shortfall: 198))
-        expect(record.errors["tshortfall"])
-          .to include(match I18n.t("validations.financial.tshortfall.more_than_rent"))
-      end
-    end
-
     context "when the owning organisation is a private registered provider" do
       before { record.owning_organisation.provider_type = 2 }