CLDC-933 update household income questions (#273)

* Change net income known question

* earnings and frequency on same page

* check answers changes and other fixes

* some fixes

* delete unnecessary test

* fix failing spec

* Refactor answer label

Co-authored-by: baarkerlounger  <baarkerlounger@users.noreply.github.com>

* test and lint fixes

* Method args

* Fix specs

* Rubocop

* Incfreq doesn't have it's own check answers display

* Add suffix to actual form

* JSON linting

* Conditional suffix only applies to check answers

* Validate that earnings and incfreq must be provided together

* Rubocop

* Fix spec

* Fix page view specs

* form fixes

* update error messages

Co-authored-by: baarkerlounger  <baarkerlounger@users.noreply.github.com>
Co-authored-by: baarkerlounger <db@slothlife.xyz>

app/helpers/check_answers_helper.rb

@@ -12,12 +12,6 @@ module CheckAnswersHelper
   end
 
   def get_answer_label(question, case_log)
-    answer = question.prefix == "£" ? ActionController::Base.helpers.number_to_currency(question.answer_label(case_log), delimiter: ",", format: "%n") : question.answer_label(case_log)
-
-    if answer.present?
-      [question.prefix, answer, question.suffix].join("")
-    else
-      "<span class=\"app-!-colour-muted\">You didn’t answer this question</span>".html_safe
-    end
+    question.answer_label(case_log).presence || "<span class=\"app-!-colour-muted\">You didn’t answer this question</span>".html_safe
   end
 end

app/models/case_log.rb

@@ -243,22 +243,7 @@ private
       self.month = startdate.month
       self.year = startdate.year
     end
-    case net_income_known
-    when "Weekly"
-      self.incfreq = "Weekly"
-      self.incref = nil
-    when "Monthly"
-      self.incfreq = "Monthly"
-      self.incref = nil
-    when "Annually"
-      self.incfreq = "Yearly"
-      self.incref = nil
-    when "Tenant prefers not to say"
-      self.incref = 1
-      self.incfreq = nil
-    when "Don’t know"
-      self.incfreq = nil
-    end
+    self.incref = 1 if net_income_known == "Tenant prefers not to say"
     self.hhmemb = other_hhmemb + 1 if other_hhmemb.present?
     self.renttype = RENT_TYPE_MAPPING[rent_type]
     self.lettype = "#{renttype} #{needstype} #{owning_organisation[:provider_type]}" if renttype.present? && needstype.present? && owning_organisation[:provider_type].present?

app/models/constants/case_log.rb

@@ -1071,11 +1071,10 @@ module Constants::CaseLog
   }.freeze
 
   NET_INCOME_KNOWN = {
-    "Weekly" => 0,
-    "Monthly" => 1,
-    "Annually" => 2,
-    "Tenant prefers not to say" => 3,
-    "Don’t know" => 4,
+    "Yes" => 0,
+    "No" => 1,
+    "Tenant prefers not to say" => 2,
+    "Don’t know" => 3,
   }.freeze
 
   HAS_BENEFITS_OPTIONS = ["Housing benefit",

app/models/form.rb

@@ -24,6 +24,10 @@ class Form
     pages.find { |p| p.id == id.to_s.underscore }
   end
 
+  def get_question(id)
+    questions.find { |q| q.id == id.to_s.underscore }
+  end
+
   def subsection_for_page(page)
     subsections.find { |s| s.pages.find { |p| p.id == page.id } }
   end

app/models/form/question.rb

@@ -38,7 +38,10 @@ class Form::Question
     return checkbox_answer_label(case_log) if type == "checkbox"
     return case_log[id]&.to_formatted_s(:govuk_date).to_s if type == "date"
 
-    return case_log[id].to_s if case_log[id].present?
+    answer = case_log[id].to_s if case_log[id].present?
+    answer_label = [prefix, format_value(answer), suffix_label(case_log)].join("") if answer
+
+    return answer_label if answer_label
 
     has_inferred_check_answers_value?(case_log) ? inferred_check_answers_value["value"] : ""
   end
@@ -96,6 +99,28 @@ private
     answer.join(", ")
   end
 
+  def format_value(answer_label)
+    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.to_sym) ? ANSWER_SUFFIX_LABELS[answer.to_sym] : answer
+      end
+    end
+    label
+  end
+
   def conditional_on
     @conditional_on ||= form.conditional_question_conditions.select do |condition|
       condition[:to] == id
@@ -118,4 +143,10 @@ private
   def enabled_inferred_answers(inferred_answers, case_log)
     inferred_answers.filter { |_key, value| value.all? { |condition_key, condition_value| case_log[condition_key] == condition_value } }
   end
+
+  ANSWER_SUFFIX_LABELS = {
+    "Weekly": " every week",
+    "Monthly": " every month",
+    "Yearly": " every year",
+  }.freeze
 end

app/models/validations/financial_validations.rb

@@ -21,8 +21,7 @@ module Validations::FinancialValidations
   end
 
   def validate_net_income(record)
-    return unless record.ecstat1 && record.weekly_net_income
-
+    if record.ecstat1 && record.weekly_net_income
       if record.weekly_net_income > record.applicable_income_range.hard_max
         record.errors.add :earnings, I18n.t("validations.financial.earnings.under_hard_max", hard_max: record.applicable_income_range.hard_max)
       end
@@ -32,6 +31,15 @@ module Validations::FinancialValidations
       end
     end
 
+    if record.earnings.present? && record.incfreq.blank?
+      record.errors.add :incfreq, I18n.t("validations.financial.earnings.freq_missing")
+    end
+
+    if record.incfreq.present? && record.earnings.blank?
+      record.errors.add :earnings, I18n.t("validations.financial.earnings.earnings_missing")
+    end
+  end
+
   def validate_tshortfall(record)
     is_yes = record.hbrentshortfall == "Yes"
     hb_donotknow = record.hb == "Don’t know"

app/views/form/_numeric_question.html.erb

@@ -7,6 +7,6 @@
     min: question.min, max: question.max, step: question.step,
     width: question.width, :readonly => question.read_only?,
     prefix_text: question.prefix.to_s,
-    suffix_text: question.suffix.to_s,
+    suffix_text: question.suffix.is_a?(String) ? question.suffix : nil,
    **stimulus_html_attributes(question)
 %>

config/forms/2021_2022.json

@@ -2292,95 +2292,56 @@
           "depends_on": [{ "setup": "completed" }],
           "pages": {
             "net_income_known": {
-              "header": "Household’s combined income",
+              "header": "",
               "description": "",
               "questions": {
                 "net_income_known": {
-                  "check_answer_label": "How often household receives income",
-                  "header": "How often does the household receive income?",
-                  "guidance_partial": "what_counts_as_income",
+                  "check_answer_label": "Do you know the household’s combined income?",
+                  "header": "Do you know the household’s combined income?",
                   "hint_text": "",
                   "type": "radio",
                   "answer_options": {
-                    "0": "Weekly",
-                    "1": "Monthly",
-                    "2": "Annually",
+                    "0": "Yes",
+                    "1": "No",
                     "divider_a": true,
-                    "3": "Don’t know",
-                    "4": "Tenant prefers not to say"
+                    "2": "Don’t know",
+                    "3": "Tenant prefers not to say"
                   }
                 }
               }
             },
-            "weekly_net_income": {
-              "depends_on": [{ "net_income_known": "Weekly" }],
-              "header": "",
+            "net_income": {
+              "depends_on": [{ "net_income_known": "Yes" }],
+              "header": "Household’s combined income after tax",
               "description": "",
               "questions": {
                 "earnings": {
                   "check_answer_label": "Total household income",
-                  "header": "How much income does the household have in total every week?",
+                  "header": "How much income does the household have in total?",
+                  "guidance_partial": "what_counts_as_income",
                   "hint_text": "",
                   "type": "numeric",
                   "min": 0,
                   "step": "1",
                   "width": 5,
                   "prefix": "£",
-                  "suffix": " every week"
-                }
-              },
-              "soft_validations": {
-                "override_net_income_validation": {
-                  "check_answer_label": "Net income confirmed?",
-                  "type": "validation_override",
-                  "answer_options": {
-                    "override_net_income_validation": "Yes"
-                  }
-                }
-              }
+                  "suffix": [
+                    { "label": "every week", "depends_on" : { "incfreq": "Weekly" } },
+                    { "label": "every month", "depends_on" : { "incfreq": "Monthly" } },
+                    { "label": "every month", "depends_on" : { "incfreq": "Yearly" } }
+                  ]
                 },
-            "monthly_net_income": {
-              "depends_on": [{ "net_income_known": "Monthly" }],
-              "header": "",
-              "description": "",
-              "questions": {
-                "earnings": {
-                  "check_answer_label": "Total household income",
-                  "header": "How much income does the household have in total every month?",
+                "incfreq": {
+                  "check_answer_label": "How often does the household receive this amount?",
+                  "header": "How often does the household receive this amount?",
                   "hint_text": "",
-                  "type": "numeric",
-                  "min": 0,
-                  "step": "1",
-                  "width": 5,
-                  "prefix": "£",
-                  "suffix": " every month"
-                }
-              },
-              "soft_validations": {
-                "override_net_income_validation": {
-                  "check_answer_label": "Net income confirmed?",
-                  "type": "validation_override",
+                  "type": "radio",
                   "answer_options": {
-                    "override_net_income_validation": "Yes"
-                  }
-                }
-              }
+                    "0": "Weekly",
+                    "1": "Monthly",
+                    "2": "Yearly"
                   },
-            "yearly_net_income": {
-              "depends_on": [{ "net_income_known": "Annually" }],
-              "header": "",
-              "description": "",
-              "questions": {
-                "earnings": {
-                  "check_answer_label": "Total household income",
-                  "header": "How much income does the household have in total every year?",
-                  "hint_text": "",
-                  "type": "numeric",
-                  "min": 0,
-                  "step": "1",
-                  "width": 5,
-                  "prefix": "£",
-                  "suffix": " every year"
+                  "hidden_in_check_answers": true
                 }
               },
               "soft_validations": {

config/locales/en.yml

@@ -68,6 +68,8 @@ en:
       earnings:
         under_hard_max: "Net income cannot be greater than %{hard_max} given the tenant’s working situation"
         over_hard_min: "Net income cannot be less than %{hard_min} given the tenant’s working situation"
+        freq_missing: "Select how often the household receives income"
+        earnings_missing: "Enter how much income the household has in total"
 
     household:
       reasonpref:

spec/factories/case_log.rb

@@ -69,6 +69,7 @@ FactoryBot.define do
       offered { 2 }
       wchair { "Yes" }
       earnings { 68 }
+      incfreq { "Weekly" }
       benefits { "Some" }
       period { "Every 2 weeks" }
       brent { 200 }
@@ -110,7 +111,7 @@ FactoryBot.define do
       discarded_at { nil }
       tenancyother { nil }
       override_net_income_validation { nil }
-      net_income_known { "Weekly" }
+      net_income_known { "Yes" }
       property_owner_organisation { "Test" }
       property_manager_organisation { "Test" }
       renewal { 1 }

spec/fixtures/complete_case_log.json

@@ -74,8 +74,9 @@
     "mrcyear": 2020,
     "offered": 2,
     "wchair": "Yes",
-    "net_income_known": "Weekly",
+    "net_income_known": "Yes",
     "earnings": 150,
+    "incfreq": "Weekly",
     "benefits": "Some",
     "hb": "Universal Credit with housing element (excluding housing benefit)",
     "period": "Every 2 weeks",

spec/fixtures/forms/2021_2022.json

@@ -358,7 +358,11 @@
                     "step": 1,
                     "width": 5,
                     "prefix": "£",
-                    "suffix": "incfreq"
+                    "suffix": [
+                      { "label": "every week", "depends_on" : { "incfreq": "Weekly" } },
+                      { "label": "every month", "depends_on" : { "incfreq": "Monthly" } },
+                      { "label": "every month", "depends_on" : { "incfreq": "Yearly" } }
+                    ]
                   },
                   "incfreq": {
                     "check_answer_label": "Income Frequency",

spec/models/case_log_spec.rb

@@ -1092,17 +1092,6 @@ RSpec.describe CaseLog do
       end
     end
 
-    context "when saving net_income" do
-      it "infers the income frequency" do
-        case_log.update!(net_income_known: "Weekly")
-        expect(case_log.reload.incfreq).to eq("Weekly")
-        case_log.update!(net_income_known: "Monthly")
-        expect(case_log.reload.incfreq).to eq("Monthly")
-        case_log.update!(net_income_known: "Annually")
-        expect(case_log.reload.incfreq).to eq("Yearly")
-      end
-    end
-
     context "when saving rent and charges" do
       let!(:case_log) do
         described_class.create({

spec/models/form/question_spec.rb

@@ -99,16 +99,17 @@ RSpec.describe Form::Question, type: :model do
 
   context "with a case log" do
     let(:case_log) { FactoryBot.build(:case_log, :in_progress) }
+    let(:question_id) { "incfreq" }
 
     it "has an answer label" do
-      case_log.earnings = 100
-      expect(question.answer_label(case_log)).to eq("100")
+      case_log.incfreq = "Weekly"
+      expect(question.answer_label(case_log)).to eq("Weekly")
     end
 
     it "has an update answer link text helper" do
-      expect(question.update_answer_link_name(case_log)).to eq("Answer<span class=\"govuk-visually-hidden\"> income</span>")
-      case_log[question_id] = 5
-      expect(question.update_answer_link_name(case_log)).to eq("Change<span class=\"govuk-visually-hidden\"> income</span>")
+      expect(question.update_answer_link_name(case_log)).to match(/Answer/)
+      case_log["incfreq"] = "Weekly"
+      expect(question.update_answer_link_name(case_log)).to match(/Change/)
     end
 
     context "when type is date" do
@@ -155,6 +156,23 @@ RSpec.describe Form::Question, type: :model do
         expect(question.enabled?(case_log)).to be true
       end
     end
+
+    context "when answers have a suffix dependent on another answer" do
+      let(:section_id) { "rent_and_charges" }
+      let(:subsection_id) { "income_and_benefits" }
+      let(:page_id) { "net_income" }
+      let(:question_id) { "earnings" }
+
+      it "displays the correct label for given suffix and answer the suffix depends on" do
+        case_log.incfreq = "Weekly"
+        case_log.earnings = 500
+        expect(question.answer_label(case_log)).to eq("£500.00 every week")
+        case_log.incfreq = "Monthly"
+        expect(question.answer_label(case_log)).to eq("£500.00 every month")
+        case_log.incfreq = "Yearly"
+        expect(question.answer_label(case_log)).to eq("£500.00 every year")
+      end
+    end
   end
 
   describe ".completed?" do

spec/models/validations/financial_validations_spec.rb

@@ -0,0 +1,24 @@
+require "rails_helper"
+
+RSpec.describe Validations::FinancialValidations do
+  subject(:financial_validator) { validator_class.new }
+
+  let(:validator_class) { Class.new { include Validations::FinancialValidations } }
+  let(:record) { FactoryBot.create(:case_log) }
+
+  describe "earnings and income frequency" do
+    it "when earnings are provided it validates that income frequency must be provided" do
+      record.earnings = 500
+      record.incfreq = nil
+      financial_validator.validate_net_income(record)
+      expect(record.errors["incfreq"]).to include(match I18n.t("validations.financial.earnings.freq_missing"))
+    end
+
+    it "when income frequency is provided it validates that earnings must be provided" do
+      record.earnings = nil
+      record.incfreq = "Weekly"
+      financial_validator.validate_net_income(record)
+      expect(record.errors["earnings"]).to include(match I18n.t("validations.financial.earnings.earnings_missing"))
+    end
+  end
+end

spec/models/validations/property_validations_spec.rb

@@ -1,5 +1,4 @@
 require "rails_helper"
-require_relative "../../request_helper"
 
 RSpec.describe Validations::PropertyValidations do
   subject(:property_validator) { property_validator_class.new }

spec/views/form/page_view_spec.rb

@@ -70,6 +70,27 @@ RSpec.describe "form/page" do
       expect(rendered).to match(/govuk-input__suffix/)
       expect(rendered).to match("every week")
     end
+
+    context "when the suffix is conditional and not a string" do
+      let(:question_attributes) do
+        {
+          type: "numeric",
+          prefix: "£",
+          suffix: [
+            { "label": "every week", "depends_on": { "incfreq": "Weekly" } },
+            { "label": "every month", "depends_on": { "incfreq": "Monthly" } },
+            { "label": "every month", "depends_on": { "incfreq": "Yearly" } },
+          ],
+        }
+      end
+
+      it "does not render the suffix" do
+        expect(rendered).not_to match(/govuk-input__suffix/)
+        expect(rendered).not_to match("every week")
+        expect(rendered).not_to match("every month")
+        expect(rendered).not_to match("every year")
+      end
+    end
   end
 
   context "with a question containing extra guidance" do