diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index 1f6936520..e5741df88 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -59,7 +59,7 @@ class CaseLogsController < ApplicationController @case_log.page = params[:case_log][:page] responses_for_page = responses_for_page(@case_log.page) if @case_log.update(responses_for_page) && @case_log.has_no_unresolved_soft_errors? - redirect_path = get_next_page_path(form, @case_log.page, @case_log) + redirect_path = form.next_page_redirect_path(@case_log.page, @case_log) redirect_to(send(redirect_path, @case_log)) else page_info = form.all_pages[@case_log.page] @@ -137,17 +137,4 @@ private params.require(:case_log).permit(CaseLog.editable_fields) end - - def get_next_page_path(form, page, case_log = {}) - content = form.all_pages[page] - - if content.key?("conditional_route_to") - content["conditional_route_to"].each do |route, conditions| - if conditions.keys.all? { |x| case_log[x].present? } && conditions.all? { |k, v| v.include?(case_log[k]) } - return "case_log_#{route}_path" - end - end - end - form.next_page_redirect_path(page) - end end diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb index ba6373b72..8e23ac340 100644 --- a/app/helpers/check_answers_helper.rb +++ b/app/helpers/check_answers_helper.rb @@ -10,19 +10,8 @@ module CheckAnswersHelper end def total_questions(subsection, case_log, form) - total_questions = {} - subsection_keys = form.pages_for_subsection(subsection).keys - page_name = subsection_keys.first - - while page_name.to_s != "check_answers" && subsection_keys.include?(page_name) - questions = form.questions_for_page(page_name) - applicable_questions = form.filter_conditional_questions(questions, case_log) - total_questions = total_questions.merge(applicable_questions) - - page_name = get_next_page_name(form, page_name, case_log) - end - - total_questions + questions = form.questions_for_subsection(subsection) + form.filter_conditional_questions(questions, case_log) end def get_next_page_name(form, page_name, case_log) @@ -37,9 +26,10 @@ module CheckAnswersHelper form.next_page(page_name) end - def create_update_answer_link(case_log_answer, case_log_id, page) - link_name = case_log_answer.blank? ? "Answer" : "Change" - link_to(link_name, "/case_logs/#{case_log_id}/#{page}", class: "govuk-link").html_safe + def create_update_answer_link(question_title, case_log, form) + page = form.page_for_question(question_title) + link_name = case_log[question_title].blank? ? "Answer" : "Change" + 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, case_log, form) diff --git a/app/models/form.rb b/app/models/form.rb index a2479e20b..a0c709912 100644 --- a/app/models/form.rb +++ b/app/models/form.rb @@ -60,37 +60,30 @@ class Form }.first end - def next_page(previous_page) - if all_pages[previous_page].key?("default_next_page") - next_page = all_pages[previous_page]["default_next_page"] - return :check_answers if next_page == "check_answers" + def page_for_question(question) + all_pages.find { |_page_key, page_value| page_value["questions"].key?(question) }.first + end - return next_page - end + def next_page(page, case_log) + subsection = subsection_for_page(page) + page_idx = pages_for_subsection(subsection).keys.index(page) + nxt_page = pages_for_subsection(subsection).keys[page_idx + 1] + return :check_answers if nxt_page.nil? + return nxt_page if page_routed_to?(nxt_page, case_log) - subsection = subsection_for_page(previous_page) - previous_page_idx = pages_for_subsection(subsection).keys.index(previous_page) - pages_for_subsection(subsection).keys[previous_page_idx + 1] || :check_answers + next_page(nxt_page, case_log) end - def next_page_redirect_path(previous_page) - next_page = next_page(previous_page) - if next_page == :check_answers - subsection = subsection_for_page(previous_page) + def next_page_redirect_path(page, case_log) + nxt_page = next_page(page, case_log) + if nxt_page == :check_answers + subsection = subsection_for_page(page) "case_log_#{subsection}_check_answers_path" else - "case_log_#{next_page}_path" + "case_log_#{nxt_page}_path" end end - def previous_page(current_page) - subsection = subsection_for_page(current_page) - current_page_idx = pages_for_subsection(subsection).keys.index(current_page) - return unless current_page_idx.positive? - - pages_for_subsection(subsection).keys[current_page_idx - 1] - end - def all_questions @all_questions ||= all_pages.map { |_page_key, page_value| page_value["questions"] @@ -101,6 +94,10 @@ class Form applicable_questions = questions questions.each do |k, question| + unless page_routed_to?(page_for_question(k), case_log) + applicable_questions = applicable_questions.reject { |z| z == k } + end + question.fetch("conditional_for", []).each do |conditional_question_key, condition| if condition_not_met(case_log, k, question, condition) applicable_questions = applicable_questions.reject { |z| z == conditional_question_key } @@ -110,6 +107,25 @@ class Form applicable_questions end + def page_routed_to?(page, case_log) + return true unless pages_with_routing_conditions.include?(page) + + applicable_conditions = all_routing_conditions.flat_map { |conditions| + conditions.select { |k, _condition| k == page } + }.flat_map(&:values) + applicable_conditions.all? do |condition| + condition.all? { |k, v| case_log[k].present? && case_log[k] == v } + end + end + + def pages_with_routing_conditions + @pages_with_routing_conditions ||= all_routing_conditions.flat_map(&:keys) + end + + def all_routing_conditions + @all_routing_conditions ||= all_pages.map { |_page_key, page| page["conditional_route_to"] }.compact + end + def condition_not_met(case_log, question_key, question, condition) case question["type"] when "numeric" diff --git a/app/views/form/_check_answers_table.html.erb b/app/views/form/_check_answers_table.html.erb index eaa54075a..69ff5cae2 100644 --- a/app/views/form/_check_answers_table.html.erb +++ b/app/views/form/_check_answers_table.html.erb @@ -7,7 +7,7 @@ <%= form.get_answer_label(@case_log, question_title) %>
- <%= create_update_answer_link(@case_log[question_title], @case_log.id, page)%> + <%= create_update_answer_link(question_title, @case_log, form) %>
diff --git a/app/views/form/check_answers.html.erb b/app/views/form/check_answers.html.erb index 46cb5a4f9..0772d2a48 100644 --- a/app/views/form/check_answers.html.erb +++ b/app/views/form/check_answers.html.erb @@ -3,12 +3,8 @@

Check the answers you gave for <%= subsection.humanize(capitalize: false) %>

<%= display_answered_questions_summary(subsection, @case_log, form) %> - <% form.pages_for_subsection(subsection).each do |page, page_info| %> - <% page_info["questions"].each do |question_title, question_info| %> - <% if total_questions(subsection, @case_log, form).include?(question_title) %> - <%= render partial: 'form/check_answers_table', locals: { question_title: question_title, question_info: question_info, case_log: @case_log, page: page, form: form } %> - <%end %> - <%end %> + <% total_questions(subsection, @case_log, form).each do |question_title, question_info| %> + <%= render partial: 'form/check_answers_table', locals: { question_title: question_title, question_info: question_info, case_log: @case_log, form: form } %> <% end %> <%= form_with model: @case_log, method: "get", builder: GOVUKDesignSystemFormBuilder::FormBuilder do |f| %> <%= f.govuk_submit "Save and continue" %> diff --git a/config/forms/2021_2022.json b/config/forms/2021_2022.json index b0ac1a524..e4f68b94f 100644 --- a/config/forms/2021_2022.json +++ b/config/forms/2021_2022.json @@ -26,8 +26,7 @@ }, "conditional_route_to": { "organisation_details": { "gdpr_acceptance": "Yes" } - }, - "default_next_page": "gdpr_declined" + } }, "gdpr_declined": { "header": "You cannot use this service", @@ -35,8 +34,7 @@ "description": "We cannot accept data about a tenant or buyer unless they’ve seen the DLUHC privacy notice.", "questions": { - }, - "default_next_page" : "check_answers" + } }, "organisation_details": { "header": "About this log", @@ -159,8 +157,7 @@ "hint_text": "", "type": "text" } - }, - "default_next_page": "check_answers" + } }, "sale_completion_date": { "header": "Sale Completion Date", @@ -184,8 +181,7 @@ "hint_text": "", "type": "text" } - }, - "default_next_page": "check_answers" + } } } } diff --git a/spec/controllers/case_logs_controller_spec.rb b/spec/controllers/case_logs_controller_spec.rb index 1b4c1fcf6..d9fce8559 100644 --- a/spec/controllers/case_logs_controller_spec.rb +++ b/spec/controllers/case_logs_controller_spec.rb @@ -200,27 +200,4 @@ RSpec.describe CaseLogsController, type: :controller do end end end - - describe "get_next_page_path" do - let(:previous_page) { "net_income" } - let(:last_previous_page) { "housing_benefit" } - let(:previous_conditional_page) { "conditional_question" } - let(:form_handler) { FormHandler.instance } - let(:form) { form_handler.get_form("test_form") } - let(:case_log_controller) { CaseLogsController.new } - - it "returns a correct page path if there is no conditional routing" do - expect(case_log_controller.send(:get_next_page_path, form, previous_page)).to eq("case_log_net_income_uc_proportion_path") - end - - it "returns a check answers page if previous page is the last page" do - expect(case_log_controller.send(:get_next_page_path, form, last_previous_page)).to eq("case_log_income_and_benefits_check_answers_path") - end - - it "returns a correct page path if there is conditional routing" do - responses_for_page = {} - responses_for_page["preg_occ"] = "No" - expect(case_log_controller.send(:get_next_page_path, form, previous_conditional_page, responses_for_page)).to eq("case_log_conditional_question_no_page_path") - end - end end diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index 6db2cbe6a..68309b715 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -466,27 +466,17 @@ RSpec.describe "Test Features" do expect(page).to have_current_path("/case_logs/#{id}/conditional_question_no_page") end - it "can route based on page inclusion rules" do - visit("/case_logs/#{id}/conditional_question_yes_page") - choose("case-log-cbl-yes-field", allow_label_click: true) - click_button("Save and continue") - expect(page).to have_current_path("/case_logs/#{id}/conditional_question/check_answers") - end - - it "can route to the default next page" do - visit("/case_logs/#{id}/conditional_question") - click_button("Save and continue") - expect(page).to have_current_path("/case_logs/#{id}/conditional_question/check_answers") - end - it "can route based on multiple conditions" do visit("/case_logs/#{id}/person_1_gender") choose("case-log-sex1-female-field", allow_label_click: true) click_button("Save and continue") + expect(page).to have_current_path("/case_logs/#{id}/household_number_of_other_members") visit("/case_logs/#{id}/conditional_question") - choose("case-log-preg-occ-yes-field", allow_label_click: true) + choose("case-log-preg-occ-no-field", allow_label_click: true) click_button("Save and continue") - expect(page).to have_current_path("/case_logs/#{id}/rent") + expect(page).to have_current_path("/case_logs/#{id}/conditional_question_no_page") + click_button("Save and continue") + expect(page).to have_current_path("/case_logs/#{id}/conditional_question/check_answers") end end end diff --git a/spec/fixtures/forms/test_aboutthislog.json b/spec/fixtures/forms/test_aboutthislog.json index 2cfd8ff26..8b206c064 100644 --- a/spec/fixtures/forms/test_aboutthislog.json +++ b/spec/fixtures/forms/test_aboutthislog.json @@ -24,8 +24,7 @@ }, "conditional_route_to": { "organisation_details": { "gdpr_acceptance": "Yes" } - }, - "default_next_page": "gdpr_declined" + } }, "gdpr_declined": { "header": "You cannot use this service", @@ -33,8 +32,7 @@ "description": "We cannot accept data about a tenant or buyer unless they’ve seen the DLUHC privacy notice.", "questions": { - }, - "default_next_page" : "check_answers" + } }, "organisation_details": { "header": "About this log", @@ -79,8 +77,7 @@ }, "conditional_route_to": { "tenant_same_property_renewal": { "sale_or_letting": "Letting" } - }, - "default_next_page" : "check_answers" + } }, "tenant_same_property_renewal": { "header": "About this log", diff --git a/spec/fixtures/forms/test_form.json b/spec/fixtures/forms/test_form.json index 28feb44b9..077767e06 100644 --- a/spec/fixtures/forms/test_form.json +++ b/spec/fixtures/forms/test_form.json @@ -246,11 +246,10 @@ } }, "conditional_route_to": { - "rent": { "preg_occ": "Yes", "sex1": "Female" }, "conditional_question_yes_page": { "preg_occ": "Yes" }, - "conditional_question_no_page": { "preg_occ": "No" } - }, - "default_next_page": "check_answers" + "conditional_question_no_page": { "preg_occ": "No" }, + "conditional_question_no_second_page": { "preg_occ": "No", "sex1": "Male" } + } }, "conditional_question_yes_page": { "questions": { @@ -263,8 +262,7 @@ "1": "No" } } - }, - "default_next_page": "check_answers" + } }, "conditional_question_no_page": { "questions": { @@ -277,8 +275,7 @@ "1": "No" } } - }, - "default_next_page": "conditional_question_no_second_page" + } }, "conditional_question_no_second_page": { "questions": { @@ -545,4 +542,4 @@ } } } - } \ No newline at end of file + } diff --git a/spec/helpers/check_answers_helper_spec.rb b/spec/helpers/check_answers_helper_spec.rb index ac3000f6b..5b0612d21 100644 --- a/spec/helpers/check_answers_helper_spec.rb +++ b/spec/helpers/check_answers_helper_spec.rb @@ -119,6 +119,7 @@ RSpec.describe CheckAnswersHelper do it "counts correct questions when the conditional question is answered" do case_log["preg_occ"] = "No" + case_log["sex1"] = "Male" expect(total_number_of_questions(conditional_routing_subsection, case_log, form)).to eq(3) end end @@ -126,7 +127,6 @@ RSpec.describe CheckAnswersHelper do context "total questions" do it "returns total questions" do result = total_questions(subsection, case_log, form) - expect(result.class).to eq(Hash) expected_keys = %w[earnings incfreq benefits hb] expect(result.keys).to eq(expected_keys) end @@ -157,14 +157,14 @@ RSpec.describe CheckAnswersHelper do case_log["preg_occ"] = "Yes" case_log["sex1"] = "Female" result = total_questions(conditional_routing_subsection, case_log, form) - expected_keys = %w[preg_occ] + expected_keys = %w[preg_occ cbl] expect(result.keys).to match_array(expected_keys) end it "it includes conditional pages and questions that were displayed" do case_log["preg_occ"] = "No" result = total_questions(conditional_routing_subsection, case_log, form) - expected_keys = %w[preg_occ conditional_question_no_question conditional_question_no_second_question] + expected_keys = %w[preg_occ conditional_question_no_question] expect(result.keys).to match_array(expected_keys) end end diff --git a/spec/models/form_spec.rb b/spec/models/form_spec.rb index 18e15c6b4..98a8114f4 100644 --- a/spec/models/form_spec.rb +++ b/spec/models/form_spec.rb @@ -3,11 +3,12 @@ require "rails_helper" RSpec.describe Form, type: :model do form_handler = FormHandler.instance let(:form) { form_handler.get_form("test_form") } + let(:case_log) { FactoryBot.build(:case_log, :in_progress) } describe ".next_page" do let(:previous_page) { "person_1_age" } it "returns the next page given the previous" do - expect(form.next_page(previous_page)).to eq("person_1_gender") + expect(form.next_page(previous_page, case_log)).to eq("person_1_gender") end end @@ -18,22 +19,6 @@ RSpec.describe Form, type: :model do end end - describe ".previous_page" do - context "given a page in the middle of a subsection" do - let(:current_page) { "person_1_age" } - it "returns the previous page given the current" do - expect(form.previous_page(current_page)).to eq("tenant_code") - end - end - - context "given the first page in a subsection" do - let(:current_page) { "tenant_code" } - it "returns empty string" do - expect(form.previous_page(current_page)).to be_nil - end - end - end - describe ".questions_for_subsection" do let(:subsection) { "income_and_benefits" } it "returns all questions for subsection" do @@ -42,4 +27,23 @@ RSpec.describe Form, type: :model do expect(result.keys).to eq(%w[earnings incfreq benefits hb]) end end + + describe "next_page_redirect_path" do + let(:previous_page) { "net_income" } + let(:last_previous_page) { "housing_benefit" } + let(:previous_conditional_page) { "conditional_question" } + + it "returns a correct page path if there is no conditional routing" do + expect(form.next_page_redirect_path(previous_page, case_log)).to eq("case_log_net_income_uc_proportion_path") + end + + it "returns a check answers page if previous page is the last page" do + expect(form.next_page_redirect_path(last_previous_page, case_log)).to eq("case_log_income_and_benefits_check_answers_path") + end + + it "returns a correct page path if there is conditional routing" do + case_log["preg_occ"] = "No" + expect(form.next_page_redirect_path(previous_conditional_page, case_log)).to eq("case_log_conditional_question_no_page_path") + end + end end