From 85b4ae81e38ae2186add638ea88c32f241e7e133 Mon Sep 17 00:00:00 2001 From: baarkerlounger <5101747+baarkerlounger@users.noreply.github.com> Date: Tue, 10 May 2022 13:10:35 +0100 Subject: [PATCH] Make tshortfall optional based on hidden tshortfall_known question (#563) * Make tshortfall optional based on hidden tshortfall_known question * Add test for optional * Add test for JSON derived and dependent on false options * Test routing * Fix optionality --- app/models/case_log.rb | 22 +++++---- app/models/form.rb | 6 ++- app/models/form/page.rb | 3 +- app/models/form/subsection.rb | 2 +- .../imports/case_logs_import_service.rb | 16 +++---- config/forms/2021_2022.json | 23 ++++++++++ config/forms/2022_2023.json | 23 ++++++++++ ...10091620_add_tshortfall_known_case_logs.rb | 5 ++ db/schema.rb | 3 +- spec/fixtures/exports/case_logs.xml | 1 + spec/fixtures/forms/2022_2023.json | 23 ++++++++++ spec/models/case_log_spec.rb | 46 +++++++++++++++++++ spec/models/form_spec.rb | 12 +++++ 13 files changed, 163 insertions(+), 22 deletions(-) create mode 100644 db/migrate/20220510091620_add_tshortfall_known_case_logs.rb diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 68cbef3ac..5e78990f3 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -186,6 +186,10 @@ class CaseLog < ApplicationRecord previous_la_known == 1 end + def tshortfall_unknown? + tshortfall_known == 1 + end + def is_secure_tenancy? # 1: Secure (including flexible) tenancy == 1 @@ -367,6 +371,10 @@ class CaseLog < ApplicationRecord "#{soft_value_for_period(soft_max)} #{SUFFIX_FROM_PERIOD[period].presence || 'every week'}" end + def optional_fields + OPTIONAL_FIELDS + dynamically_not_required + end + private PIO = Postcodes::IO.new @@ -425,7 +433,8 @@ private def dynamically_not_required previous_la_known_field = postcode_known? ? %w[previous_la_known] : [] - ((form.invalidated_questions(self) + form.readonly_questions).map(&:id) + previous_la_known_field).uniq + tshortfall_field = tshortfall_unknown? ? %w[tshortfall] : [] + previous_la_known_field + tshortfall_field end def set_derived_fields! @@ -458,6 +467,7 @@ private end end self.has_benefits = get_has_benefits + self.tshortfall_known = 0 if tshortfall self.wtshortfall = if tshortfall && receives_housing_related_benefits? weekly_value(tshortfall) end @@ -622,13 +632,9 @@ private end def all_fields_nil? - init_fields = %w[owning_organisation_id managing_organisation_id] - fields = mandatory_fields.difference(init_fields) - fields.none? { |field| public_send(field).present? if respond_to?(field) } - end - - def mandatory_fields - form.questions.map(&:id).difference(OPTIONAL_FIELDS, dynamically_not_required) + not_started_statuses = %i[not_started cannot_start_yet] + subsection_statuses = form.subsections.map { |subsection| subsection.status(self) }.uniq + subsection_statuses.all? { |status| not_started_statuses.include?(status) } end def age_refused? diff --git a/app/models/form.rb b/app/models/form.rb index e339f6fdc..7f48d7d4c 100644 --- a/app/models/form.rb +++ b/app/models/form.rb @@ -124,8 +124,8 @@ class Form end def enabled_page_questions(case_log) - pages_that_are_routed_to = pages.select { |p| p.routed_to?(case_log) } - pages_that_are_routed_to.flat_map(&:questions) || [] + pages_that_are_routed_to_or_derived = pages.select { |p| p.routed_to?(case_log) || p.derived } + pages_that_are_routed_to_or_derived.flat_map(&:questions) || [] end def invalidated_conditional_questions(case_log) @@ -155,6 +155,8 @@ class Form return true unless depends_on depends_on.any? do |conditions_set| + return false unless conditions_set + conditions_set.all? do |question, value| if value.is_a?(Hash) && value.key?("operator") operator = value["operator"] diff --git a/app/models/form/page.rb b/app/models/form/page.rb index 155f85d20..7822c2b68 100644 --- a/app/models/form/page.rb +++ b/app/models/form/page.rb @@ -1,5 +1,5 @@ class Form::Page - attr_accessor :id, :header, :description, :questions, + attr_accessor :id, :header, :description, :questions, :derived, :depends_on, :title_text, :informative_text, :subsection, :hide_subsection_label def initialize(id, hsh, subsection) @@ -8,6 +8,7 @@ class Form::Page @description = hsh["description"] @questions = hsh["questions"].map { |q_id, q| Form::Question.new(q_id, q, self) } @depends_on = hsh["depends_on"] + @derived = hsh["derived"] @title_text = hsh["title_text"] @informative_text = hsh["informative_text"] @hide_subsection_label = hsh["hide_subsection_label"] diff --git a/app/models/form/subsection.rb b/app/models/form/subsection.rb index 95216c9d4..20b65ad9f 100644 --- a/app/models/form/subsection.rb +++ b/app/models/form/subsection.rb @@ -31,7 +31,7 @@ class Form::Subsection end qs = applicable_questions(case_log) - qs_optional_removed = qs.reject { |q| CaseLog::OPTIONAL_FIELDS.include?(q.id) } + qs_optional_removed = qs.reject { |q| case_log.optional_fields.include?(q.id) } return :not_started if qs.all? { |question| case_log[question.id].blank? || question.read_only? } return :completed if qs_optional_removed.all? { |question| question.completed?(case_log) } diff --git a/app/services/imports/case_logs_import_service.rb b/app/services/imports/case_logs_import_service.rb index cf7cd4cee..56ccbd8d8 100644 --- a/app/services/imports/case_logs_import_service.rb +++ b/app/services/imports/case_logs_import_service.rb @@ -125,8 +125,9 @@ module Imports attributes["supcharg"] = safe_string_as_decimal(xml_doc, "Q18aiv") attributes["tcharge"] = safe_string_as_decimal(xml_doc, "Q18av") + attributes["hbrentshortfall"] = unsafe_string_as_integer(xml_doc, "Q18d") attributes["tshortfall"] = safe_string_as_decimal(xml_doc, "Q18dyes") - attributes["hbrentshortfall"] = hbshortfall(xml_doc, attributes) + attributes["tshortfall_known"] = tshortfall_known?(xml_doc, attributes) attributes["voiddate"] = compose_date(xml_doc, "VDAY", "VMONTH", "VYEAR") attributes["mrcdate"] = compose_date(xml_doc, "MRCDAY", "MRCMONTH", "MRCYEAR") @@ -214,7 +215,7 @@ module Imports end def fields_not_present_in_softwire_data - %w[majorrepairs illness_type_0] + %w[majorrepairs illness_type_0 tshortfall_known] end def check_status_completed(case_log, previous_status) @@ -520,14 +521,11 @@ module Imports ((2..8).map { |x| string_or_nil(xml_doc, "P#{x}Rel") } + [string_or_nil(xml_doc, "P1Sex")]).compact end - def hbshortfall(xml_doc, attributes) - shortfall = unsafe_string_as_integer(xml_doc, "Q18d") - if attributes["tshortfall"].blank? && shortfall == 1 && overridden?(xml_doc, "xmlns", "Q18dyes") - # If they have said there is a shortfall but then not entered one, and that has been - # manually overridden we instead infer that they actually didn't know whether there is a shortfall. - 3 + def tshortfall_known?(xml_doc, attributes) + if attributes["tshortfall"].blank? && attributes["hbrentshortfall"] == 1 && overridden?(xml_doc, "xmlns", "Q18dyes") + 1 else - shortfall + 0 end end end diff --git a/config/forms/2021_2022.json b/config/forms/2021_2022.json index 8715e1a46..1fe6bfb0d 100644 --- a/config/forms/2021_2022.json +++ b/config/forms/2021_2022.json @@ -5717,6 +5717,29 @@ } ] }, + "outstanding_amount_known": { + "header": "", + "description": "", + "questions": { + "tshortfall_known": { + "check_answer_label": "", + "header": "", + "hint_text": "", + "hidden_in_check_answers": true, + "type": "radio", + "answer_options": { + "0": { + "value": "Yes" + }, + "1": { + "value": "No" + } + } + } + }, + "derived": true, + "depends_on": [false] + }, "outstanding_amount": { "header": "", "description": "", diff --git a/config/forms/2022_2023.json b/config/forms/2022_2023.json index 513a67e15..f445ebe7c 100644 --- a/config/forms/2022_2023.json +++ b/config/forms/2022_2023.json @@ -5740,6 +5740,29 @@ } ] }, + "outstanding_amount_known": { + "header": "", + "description": "", + "questions": { + "tshortfall_known": { + "check_answer_label": "", + "header": "", + "hint_text": "", + "hidden_in_check_answers": true, + "type": "radio", + "answer_options": { + "0": { + "value": "Yes" + }, + "1": { + "value": "No" + } + } + } + }, + "derived": true, + "depends_on": [false] + }, "outstanding_amount": { "header": "", "description": "", diff --git a/db/migrate/20220510091620_add_tshortfall_known_case_logs.rb b/db/migrate/20220510091620_add_tshortfall_known_case_logs.rb new file mode 100644 index 000000000..1059324ba --- /dev/null +++ b/db/migrate/20220510091620_add_tshortfall_known_case_logs.rb @@ -0,0 +1,5 @@ +class AddTshortfallKnownCaseLogs < ActiveRecord::Migration[7.0] + def change + add_column :case_logs, :tshortfall_known, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index ddab93cfe..ba9001e74 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2022_05_06_092350) do +ActiveRecord::Schema[7.0].define(version: 2022_05_10_091620) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -222,6 +222,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_05_06_092350) do t.integer "joint" t.bigint "created_by_id" t.integer "illness_type_0" + t.integer "tshortfall_known" t.index ["created_by_id"], name: "index_case_logs_on_created_by_id" t.index ["managing_organisation_id"], name: "index_case_logs_on_managing_organisation_id" t.index ["old_id"], name: "index_case_logs_on_old_id", unique: true diff --git a/spec/fixtures/exports/case_logs.xml b/spec/fixtures/exports/case_logs.xml index 00dfd5243..66795680e 100644 --- a/spec/fixtures/exports/case_logs.xml +++ b/spec/fixtures/exports/case_logs.xml @@ -164,6 +164,7 @@ {created_by_id} + 0 1 diff --git a/spec/fixtures/forms/2022_2023.json b/spec/fixtures/forms/2022_2023.json index 8d856aa1c..e9fcb6990 100644 --- a/spec/fixtures/forms/2022_2023.json +++ b/spec/fixtures/forms/2022_2023.json @@ -94,6 +94,29 @@ "width": 10 } } + }, + "outstanding_amount_known": { + "header": "", + "description": "", + "questions": { + "tshortfall_known": { + "check_answer_label": "", + "header": "", + "hint_text": "", + "hidden_in_check_answers": true, + "type": "radio", + "answer_options": { + "0": { + "value": "Yes" + }, + "1": { + "value": "No" + } + } + } + }, + "derived": true, + "depends_on": [false] } } } diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index 7fdbd6381..f420ade1c 100644 --- a/spec/models/case_log_spec.rb +++ b/spec/models/case_log_spec.rb @@ -1660,6 +1660,26 @@ RSpec.describe CaseLog do expect(relet_case_log["newprop"]).to eq(2) end end + + context "when a total shortfall is provided" do + it "derives that tshortfall is known" do + case_log.update!({ tshortfall: 10 }) + record_from_db = ActiveRecord::Base.connection.execute("select tshortfall_known from case_logs where id=#{case_log.id}").to_a[0] + expect(record_from_db["tshortfall_known"]).to eq(0) + expect(case_log["tshortfall_known"]).to eq(0) + end + end + end + + describe "optional fields" do + let(:case_log) { FactoryBot.create(:case_log) } + + context "when tshortfall is marked as not known" do + it "makes tshortfall optional" do + case_log.update!({ tshortfall: nil, tshortfall_known: 1 }) + expect(case_log.optional_fields).to include("tshortfall") + end + end end describe "resetting invalidated fields" do @@ -1751,6 +1771,32 @@ RSpec.describe CaseLog do end end + describe "tshortfall_unknown?" do + context "when tshortfall is nil" do + let(:case_log) { FactoryBot.create(:case_log, :in_progress, tshortfall_known: nil) } + + it "returns false" do + expect(case_log.tshortfall_unknown?).to be false + end + end + + context "when tshortfall is No" do + let(:case_log) { FactoryBot.create(:case_log, :in_progress, tshortfall_known: 1) } + + it "returns false" do + expect(case_log.tshortfall_unknown?).to be true + end + end + + context "when tshortfall is Yes" do + let(:case_log) { FactoryBot.create(:case_log, :in_progress, tshortfall_known: 0) } + + it "returns false" do + expect(case_log.tshortfall_unknown?).to be false + end + end + end + describe "paper trail" do let(:case_log) { FactoryBot.create(:case_log, :in_progress) } diff --git a/spec/models/form_spec.rb b/spec/models/form_spec.rb index 43628ab78..08cd1718e 100644 --- a/spec/models/form_spec.rb +++ b/spec/models/form_spec.rb @@ -190,5 +190,17 @@ RSpec.describe Form, type: :model do expect(form.invalidated_page_questions(case_log).map(&:id).uniq).to eq(expected_invalid) end end + + context "when a page is marked as `derived` and `depends_on: false`" do + let(:case_log) { FactoryBot.build(:case_log, :in_progress, startdate: Time.utc(2023, 2, 2, 10, 36, 49)) } + + it "does not count it's questions as invalidated" do + expect(form.enabled_page_questions(case_log).map(&:id).uniq).to include("tshortfall_known") + end + + it "does not route to the page" do + expect(form.invalidated_pages(case_log).map(&:id)).to include("outstanding_amount_known") + end + end end end