From 4378e83709df4441adf5a69905a752967da2883d Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Thu, 7 Oct 2021 10:12:54 +0100 Subject: [PATCH 1/4] fix the ids in numeric questions (#37) --- app/helpers/numeric_questions_helper.rb | 2 +- .../controllers/numeric_question_controller.js | 2 +- spec/features/case_log_spec.rb | 16 ++++++++-------- spec/helpers/numeric_questions_helper_spec.rb | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/app/helpers/numeric_questions_helper.rb b/app/helpers/numeric_questions_helper.rb index 464f42723..c0fe05ce3 100644 --- a/app/helpers/numeric_questions_helper.rb +++ b/app/helpers/numeric_questions_helper.rb @@ -5,7 +5,7 @@ module NumericQuestionsHelper { "data-controller": "numeric-question", "data-action": "numeric-question#calculateFields", - "data-target": "#{question['result-field'].to_s.dasherize}-field", + "data-target": "case-log-#{question['result-field'].to_s.dasherize}-field", "data-calculated": question["fields-to-add"].to_json, } end diff --git a/app/javascript/controllers/numeric_question_controller.js b/app/javascript/controllers/numeric_question_controller.js index 6a0a58d78..355e1686b 100644 --- a/app/javascript/controllers/numeric_question_controller.js +++ b/app/javascript/controllers/numeric_question_controller.js @@ -3,7 +3,7 @@ import { Controller } from "@hotwired/stimulus" export default class extends Controller { calculateFields() { const affectedField = this.element.dataset.target; - const fieldsToAdd = JSON.parse(this.element.dataset.calculated).map(x => `${x.replaceAll("_","-")}-field`); + const fieldsToAdd = JSON.parse(this.element.dataset.calculated).map(x => `case-log-${x.replaceAll("_","-")}-field`); const valuesToAdd = fieldsToAdd.map(x => document.getElementById(x).value).filter(x => x); const newValue = valuesToAdd.map(x => parseInt(x)).reduce((a, b) => a + b, 0); const elementToUpdate = document.getElementById(affectedField); diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index 3f2b8039f..b86320615 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -118,17 +118,17 @@ RSpec.describe "Test Features" do it "updates total value of the rent", js: true do visit("/case_logs/#{id}/rent") - fill_in("basic_rent", with: 3) - expect(page).to have_field("total-charge-field", with: "3") + fill_in("case-log-basic-rent-field", with: 3) + expect(page).to have_field("case-log-total-charge-field", with: "3") - fill_in("service_charge", with: 2) - expect(page).to have_field("total-charge-field", with: "5") + fill_in("case-log-service-charge-field", with: 2) + expect(page).to have_field("case-log-total-charge-field", with: "5") - fill_in("personal_service_charge", with: 1) - expect(page).to have_field("total-charge-field", with: "6") + fill_in("case-log-personal-service-charge-field", with: 1) + expect(page).to have_field("case-log-total-charge-field", with: "6") - fill_in("support_charge", with: 4) - expect(page).to have_field("total-charge-field", with: "10") + fill_in("case-log-support-charge-field", with: 4) + expect(page).to have_field("case-log-total-charge-field", with: "10") end end diff --git a/spec/helpers/numeric_questions_helper_spec.rb b/spec/helpers/numeric_questions_helper_spec.rb index 48877babe..5302d9eaa 100644 --- a/spec/helpers/numeric_questions_helper_spec.rb +++ b/spec/helpers/numeric_questions_helper_spec.rb @@ -13,7 +13,7 @@ RSpec.describe NumericQuestionsHelper do expect(numeric_question_html_attributes(questions["basic_rent"])).to eq({ "data-controller": "numeric-question", "data-action": "numeric-question#calculateFields", - "data-target": "#{questions['basic_rent']['result-field'].to_s.dasherize}-field", + "data-target": "case-log-#{questions['basic_rent']['result-field'].to_s.dasherize}-field", "data-calculated": questions["basic_rent"]["fields-to-add"].to_json, }) end From e095e940092b7abcb9fe2175a15ec81b4beaae0d Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Thu, 7 Oct 2021 13:15:06 +0100 Subject: [PATCH 2/4] CLDC-512: Display answered fields (#38) * Add tests for prepopulating answered questions * Move test values to the factory --- spec/factories/case_log.rb | 2 ++ spec/features/case_log_spec.rb | 22 ++++++++++++++++------ spec/helpers/tasklist_helper_spec.rb | 2 +- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/spec/factories/case_log.rb b/spec/factories/case_log.rb index e2f132c53..d55408242 100644 --- a/spec/factories/case_log.rb +++ b/spec/factories/case_log.rb @@ -5,6 +5,8 @@ FactoryBot.define do status { 0 } tenant_code { "TH356" } postcode { "SW2 6HI" } + previous_postcode { "P0 5ST" } + tenant_age { "12" } end trait :submitted do status { 1 } diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index b86320615..238897c94 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -15,6 +15,12 @@ RSpec.describe "Test Features" do household_number_of_other_members: { type: "numeric", answer: 2 }, } + def fill_in_number_question(case_log_id, question, value) + visit("/case_logs/#{case_log_id}/#{question}") + fill_in("case-log-#{question.to_s.dasherize}-field", with: value) + click_button("Save and continue") + end + def answer_all_questions_in_income_subsection visit("/case_logs/#{empty_case_log.id}/net_income") fill_in("case-log-net-income-field", with: 18_000) @@ -130,6 +136,16 @@ RSpec.describe "Test Features" do fill_in("case-log-support-charge-field", with: 4) expect(page).to have_field("case-log-total-charge-field", with: "10") end + + it "displays number answers in inputs if they are already saved" do + visit("/case_logs/#{id}/previous_postcode") + expect(page).to have_field("case-log-previous-postcode-field", with: "P0 5ST") + end + + it "displays text answers in inputs if they are already saved" do + visit("/case_logs/#{id}/tenant_age") + expect(page).to have_field("case-log-tenant-age-field", with: "12") + end end describe "Back link directs correctly" do @@ -164,12 +180,6 @@ RSpec.describe "Test Features" do let(:subsection) { "household_characteristics" } context "when the user needs to check their answers for a subsection" do - def fill_in_number_question(case_log_id, question, value) - visit("/case_logs/#{case_log_id}/#{question}") - fill_in("case-log-#{question.to_s.dasherize}-field", with: value) - click_button("Save and continue") - end - it "can be visited by URL" do visit("case_logs/#{id}/#{subsection}/check_answers") expect(page).to have_content("Check the answers you gave for #{subsection.tr('_', ' ')}") diff --git a/spec/helpers/tasklist_helper_spec.rb b/spec/helpers/tasklist_helper_spec.rb index b8fbaae0f..ec07092f6 100644 --- a/spec/helpers/tasklist_helper_spec.rb +++ b/spec/helpers/tasklist_helper_spec.rb @@ -73,7 +73,7 @@ RSpec.describe TasklistHelper do it "returns the number of sections in progress" do @form = Form.new(2021, 2022) - expect(get_sections_count(@form, case_log, :in_progress)).to eq(1) + expect(get_sections_count(@form, case_log, :in_progress)).to eq(2) end it "returns 0 for invalid state" do From f6734a60e035cb0424991d7a2a1fe08090c36c9e Mon Sep 17 00:00:00 2001 From: Daniel Baark <5101747+baarkerlounger@users.noreply.github.com> Date: Fri, 8 Oct 2021 12:33:00 +0100 Subject: [PATCH 3/4] CLDC-341 - Conditional Questions (#31) * Make outstanding amount question conditional * Make major repairs date conditional on major repairs having been done * Add Other reason for leaving as a conditional question * Make reason for reasonable preference conditional on reasonable preference being given * Refactor conditional html attributes * Pretty print config json * Add numeric conditional * Add the rest of the conditionals * Rubocop * Refactor how we add stimulus controller html attributes so that we can combine them * Spec attrib merging * Fix question counts for sections with conditional questions * Some linting * Spec question counting behaviour * Linting * Add some more readme explanation * Rename controller helper * Rename variables for clarity --- Gemfile.lock | 4 +- README.md | 7 + app/controllers/case_logs_controller.rb | 4 +- app/helpers/check_answers_helper.rb | 56 +- app/helpers/numeric_questions_helper.rb | 12 - app/helpers/question_attribute_helper.rb | 39 ++ .../conditional_question_controller.js | 36 +- app/models/form.rb | 9 +- app/views/form/_checkbox_question.html.erb | 2 +- app/views/form/_date_question.html.erb | 3 +- app/views/form/_numeric_question.html.erb | 2 +- app/views/form/_radio_question.html.erb | 8 +- app/views/form/_text_question.html.erb | 3 +- app/views/form/check_answers.html.erb | 10 +- config/forms/2021_2022.json | 537 ++++++++++++++++-- config/routes.rb | 2 +- ...813_add_conditional_fields_to_case_logs.rb | 9 + db/schema.rb | 5 +- spec/helpers/check_answers_helper_spec.rb | 92 ++- spec/helpers/numeric_questions_helper_spec.rb | 21 - .../helpers/question_attribute_helper_spec.rb | 51 ++ 21 files changed, 766 insertions(+), 146 deletions(-) delete mode 100644 app/helpers/numeric_questions_helper.rb create mode 100644 app/helpers/question_attribute_helper.rb create mode 100644 db/migrate/20211005115813_add_conditional_fields_to_case_logs.rb delete mode 100644 spec/helpers/numeric_questions_helper_spec.rb create mode 100644 spec/helpers/question_attribute_helper_spec.rb diff --git a/Gemfile.lock b/Gemfile.lock index b21879881..328d2a9ef 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -26,7 +26,7 @@ GIT GIT remote: https://github.com/rspec/rspec-rails.git - revision: fdcd1df0b13f9b6547336b4d37dffb66f70f7228 + revision: 3f0e35085f5765decf96fee179ec9a2e132a67c1 branch: main specs: rspec-rails (5.1.0.pre) @@ -294,7 +294,7 @@ GEM actionpack (>= 4.0) activesupport (>= 4.0) sprockets (>= 3.0.0) - stimulus-rails (0.6.1) + stimulus-rails (0.7.0) rails (>= 6.0.0) thor (1.1.0) turbo-rails (0.8.1) diff --git a/README.md b/README.md index 5028acab2..7a7a2efc5 100644 --- a/README.md +++ b/README.md @@ -111,6 +111,10 @@ The JSON should follow the structure: "answer_options": { // checkbox and radio only "0": String, "1": String + }, + "conditional_for": { + "[snake_case_question_to_enable_1_name_string]": ["condition-that-enables"], + "[snake_case_question_to_enable_2_name_string]": ["condition-that-enables"] } } } @@ -131,6 +135,9 @@ Assumptions made by the format: - All pages have at least 1 question - The ActiveRecord case log model has a field for each question name (must match) - Text not required by a page/question such as a header or hint text should be passed as an empty string +- For conditionally shown questions conditions that have been implemented and can be used are: + - Radio question answer option selected matches one of conditional e.g. ["answer-options-1-string", "answer-option-3-string"] + - Numeric question value matches condition e.g. [">2"], ["<7"] or ["== 6"] ## Useful documentation (external dependencies) diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index 45bc05ee4..9dffdc533 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -37,11 +37,9 @@ class CaseLogsController < ApplicationController def check_answers @case_log = CaseLog.find(params[:case_log_id]) - form = Form.new(2021, 2022) current_url = request.env["PATH_INFO"] subsection = current_url.split("/")[-2] - subsection_pages = form.pages_for_subsection(subsection) - render "form/check_answers", locals: { case_log: @case_log, subsection_pages: subsection_pages, subsection: subsection.humanize(capitalize: false) } + render "form/check_answers", locals: { case_log: @case_log, subsection: subsection } end form = Form.new(2021, 2022) diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb index 4fd46ce9d..d11706012 100644 --- a/app/helpers/check_answers_helper.rb +++ b/app/helpers/check_answers_helper.rb @@ -1,16 +1,44 @@ module CheckAnswersHelper - def get_answered_questions_total(subsection_pages, case_log) - questions = subsection_pages.values.flat_map do |page| - page["questions"].keys + def total_answered_questions(subsection, case_log) + total_questions(subsection, case_log).keys.count do |question_key| + case_log[question_key].present? end - questions.count { |question| case_log[question].present? } end - def get_total_number_of_questions(subsection_pages) - questions = subsection_pages.values.flat_map do |page| - page["questions"].keys + def total_number_of_questions(subsection, case_log) + total_questions(subsection, case_log).count + end + + def total_questions(subsection, case_log) + form = Form.new(2021, 2022) + questions = form.questions_for_subsection(subsection) + questions_not_applicable = [] + questions.reject do |question_key, question| + question.fetch("conditional_for", []).map do |conditional_question_key, condition| + if condition_not_met(case_log, question_key, question, condition) + questions_not_applicable << conditional_question_key + end + end + questions_not_applicable.include?(question_key) + end + end + + def condition_not_met(case_log, question_key, question, condition) + case question["type"] + when "numeric" + # rubocop:disable Security/Eval + case_log[question_key].blank? || !eval(case_log[question_key].to_s + condition) + # rubocop:enable Security/Eval + when "radio" + case_log[question_key].blank? || !condition.include?(case_log[question_key]) + else + raise "Not implemented yet" end - questions.count + end + + def subsection_pages(subsection) + form = Form.new(2021, 2022) + form.pages_for_subsection(subsection) end def create_update_answer_link(case_log_answer, case_log_id, page) @@ -18,9 +46,9 @@ module CheckAnswersHelper link_to(link_name, "/case_logs/#{case_log_id}/#{page}", class: "govuk-link").html_safe end - def create_next_missing_question_link(case_log_id, subsection_pages, case_log) + def create_next_missing_question_link(case_log_id, subsection, case_log) pages_to_fill_in = [] - subsection_pages.each do |page_title, page_info| + subsection_pages(subsection).each do |page_title, page_info| page_info["questions"].any? { |question| case_log[question].blank? } pages_to_fill_in << page_title end @@ -28,12 +56,12 @@ module CheckAnswersHelper link_to("Answer the missing questions", url, class: "govuk-link").html_safe end - def display_answered_questions_summary(subsection_pages, case_log) - if get_answered_questions_total(subsection_pages, case_log) == get_total_number_of_questions(subsection_pages) + def display_answered_questions_summary(subsection, case_log) + if total_answered_questions(subsection, case_log) == total_number_of_questions(subsection, case_log) '
You answered all the questions
'.html_safe else - "You answered #{get_answered_questions_total(subsection_pages, case_log)} of #{get_total_number_of_questions(subsection_pages)} questions
- #{create_next_missing_question_link(case_log['id'], subsection_pages, case_log)}".html_safe + "You answered #{total_answered_questions(subsection, case_log)} of #{total_number_of_questions(subsection, case_log)} questions
+ #{create_next_missing_question_link(case_log['id'], subsection, case_log)}".html_safe end end end diff --git a/app/helpers/numeric_questions_helper.rb b/app/helpers/numeric_questions_helper.rb deleted file mode 100644 index c0fe05ce3..000000000 --- a/app/helpers/numeric_questions_helper.rb +++ /dev/null @@ -1,12 +0,0 @@ -module NumericQuestionsHelper - def numeric_question_html_attributes(question) - return {} if question["fields-to-add"].blank? || question["result-field"].blank? - - { - "data-controller": "numeric-question", - "data-action": "numeric-question#calculateFields", - "data-target": "case-log-#{question['result-field'].to_s.dasherize}-field", - "data-calculated": question["fields-to-add"].to_json, - } - end -end diff --git a/app/helpers/question_attribute_helper.rb b/app/helpers/question_attribute_helper.rb new file mode 100644 index 000000000..1010591f0 --- /dev/null +++ b/app/helpers/question_attribute_helper.rb @@ -0,0 +1,39 @@ +module QuestionAttributeHelper + def stimulus_html_attributes(question) + attribs = [ + numeric_question_html_attributes(question), + conditional_html_attributes(question), + ] + merge_controller_attributes(*attribs) + end + +private + + def numeric_question_html_attributes(question) + return {} if question["fields-to-add"].blank? || question["result-field"].blank? + + { + "data-controller": "numeric-question", + "data-action": "numeric-question#calculateFields", + "data-target": "case-log-#{question['result-field'].to_s.dasherize}-field", + "data-calculated": question["fields-to-add"].to_json, + } + end + + def conditional_html_attributes(question) + return {} if question["conditional_for"].blank? + + { + "data-controller": "conditional-question", + "data-action": "conditional-question#displayConditional", + "data-info": question["conditional_for"].to_json, + } + end +end + +def merge_controller_attributes(*args) + args.flat_map(&:keys).uniq.each_with_object({}) do |key, hsh| + hsh[key] = args.map { |a| a.fetch(key, "") }.join(" ").strip + hsh + end +end diff --git a/app/javascript/controllers/conditional_question_controller.js b/app/javascript/controllers/conditional_question_controller.js index 0d058da12..c49ba490a 100644 --- a/app/javascript/controllers/conditional_question_controller.js +++ b/app/javascript/controllers/conditional_question_controller.js @@ -6,17 +6,29 @@ export default class extends Controller { } displayConditional() { + switch(this.element.type) { + case "number": + this.displayConditionalNumeric() + case "radio": + this.displayConditionalRadio() + default: + console.log("Not yet implemented for " + this.element.type) + break; + } + } + + displayConditionalRadio() { if(this.element.checked) { - let selected = this.element.value + let selectedValue = this.element.value let conditional_for = JSON.parse(this.element.dataset.info) - Object.entries(conditional_for).forEach(([key, values]) => { - let div = document.getElementById(key + "_div") - if(values.includes(selected)) { + Object.entries(conditional_for).forEach(([targetQuestion, conditions]) => { + let div = document.getElementById(targetQuestion + "_div") + if(conditions.includes(selectedValue)) { div.style.display = "block" } else { div.style.display = "none" - let buttons = document.getElementsByName(`case_log[${key}]`) + let buttons = document.getElementsByName(`case_log[${targetQuestion}]`) Object.entries(buttons).forEach(([idx, button]) => { button.checked = false; }) @@ -24,4 +36,18 @@ export default class extends Controller { }) } } + + displayConditionalNumeric() { + let enteredValue = this.element.value + let conditional_for = JSON.parse(this.element.dataset.info) + + Object.entries(conditional_for).forEach(([targetQuestion, condition]) => { + let div = document.getElementById(targetQuestion + "_div") + if(eval((enteredValue + condition))) { + div.style.display = "block" + } else { + div.style.display = "none" + } + }) + } } diff --git a/app/models/form.rb b/app/models/form.rb index 9812d890f..031c0093c 100644 --- a/app/models/form.rb +++ b/app/models/form.rb @@ -37,6 +37,11 @@ class Form all_pages[page]["questions"] end + # Returns a hash with the questions as keys + def questions_for_subsection(subsection) + pages_for_subsection(subsection).map { |title, _value| questions_for_page(title) }.reduce(:merge) + end + def first_page_for_subsection(subsection) pages_for_subsection(subsection).keys.first end @@ -70,8 +75,4 @@ class Form pages_for_subsection(subsection).keys[current_page_idx - 1] end - - def questions_for_subsection(subsection) - pages_for_subsection(subsection).map { |title, _value| questions_for_page(title) }.reduce(:merge) - end end diff --git a/app/views/form/_checkbox_question.html.erb b/app/views/form/_checkbox_question.html.erb index 7d8d82b1b..99bd3fd49 100644 --- a/app/views/form/_checkbox_question.html.erb +++ b/app/views/form/_checkbox_question.html.erb @@ -6,7 +6,7 @@ <% if key.starts_with?("divider") %> <%= f.govuk_check_box_divider %> <% else %> - <%= f.govuk_check_box question_key, val, label: { text: val } %> + <%= f.govuk_check_box question_key, val, label: { text: val }, **stimulus_html_attributes(question) %> <% end %> <% end %> <% end %> diff --git a/app/views/form/_date_question.html.erb b/app/views/form/_date_question.html.erb index f4a4ddf93..417c7e163 100644 --- a/app/views/form/_date_question.html.erb +++ b/app/views/form/_date_question.html.erb @@ -1,5 +1,6 @@ <%= f.govuk_date_field question_key, hint: { text: question["hint_text"] }, legend: { text: question["header"].html_safe, size: "l"}, - width: 20 + width: 20, + **stimulus_html_attributes(question) %> diff --git a/app/views/form/_numeric_question.html.erb b/app/views/form/_numeric_question.html.erb index 6de29a111..47909c0ba 100644 --- a/app/views/form/_numeric_question.html.erb +++ b/app/views/form/_numeric_question.html.erb @@ -3,5 +3,5 @@ label: { text: question["header"].html_safe, size: "l"}, min: question["min"], max: question["max"], step: question["step"], width: 20, :readonly => question["readonly"], - **numeric_question_html_attributes(question) + **stimulus_html_attributes(question) %> diff --git a/app/views/form/_radio_question.html.erb b/app/views/form/_radio_question.html.erb index a74153456..3f653eb7d 100644 --- a/app/views/form/_radio_question.html.erb +++ b/app/views/form/_radio_question.html.erb @@ -6,14 +6,8 @@ <% question["answer_options"].map do |key, val| %> <% if key.starts_with?("divider") %> <%= f.govuk_radio_divider %> - <% elsif question["conditional_for"] %> - <%= f.govuk_radio_button question_key, val, label: { text: val }, - "data-controller": "conditional-question", - "data-action": "conditional-question#displayConditional", - "data-info": question["conditional_for"].to_json - %> <% else %> - <%= f.govuk_radio_button question_key, val, label: { text: val } %> + <%= f.govuk_radio_button question_key, val, label: { text: val }, **stimulus_html_attributes(question) %> <% end %> <% end %> <% end %> diff --git a/app/views/form/_text_question.html.erb b/app/views/form/_text_question.html.erb index 6bf73338e..4b45a8ceb 100644 --- a/app/views/form/_text_question.html.erb +++ b/app/views/form/_text_question.html.erb @@ -1,5 +1,6 @@ <%= f.govuk_text_field question_key, hint: { text: question["hint_text"] }, label: { text: question["header"].html_safe, size: "l"}, - width: 20 + width: 20, + **stimulus_html_attributes(question) %> diff --git a/app/views/form/check_answers.html.erb b/app/views/form/check_answers.html.erb index a2102146d..b6533b4c2 100644 --- a/app/views/form/check_answers.html.erb +++ b/app/views/form/check_answers.html.erb @@ -1,11 +1,13 @@ <%= turbo_frame_tag "case_log_form", target: "_top" do %>