CLDC-358 conditional page routing (#49)

* Add next page path method with conditional routing

* Rename method

* Fix the tests and add files to .gitignore

* Make get_next_page_path a private method

* WIP test for total question count

* Change back button to use rails navigation

* Make total number of questions work with conditional routing and run rubocop

* Add tests for conditional question displaying

* Add conditional routing to the check answers page

* push 1 failure

* make total_questions step over the form like a user would and gather relevand questions

* Fix the tests

* fixed checks answers labels

* small refactors and rubocop

* remove unused code from get next page path

Co-authored-by: Kat <katrina@madetech.com>
Co-authored-by: magicmilo <milobascombe@gmail.com>

.gitignore

@@ -43,3 +43,6 @@ yarn-debug.log*
 #IDE specific files
 /.idea
 /.idea/*
+.DS_Store
+.generators
+.rakeTasks

app/controllers/case_logs_controller.rb

@@ -61,7 +61,7 @@ class CaseLogsController < ApplicationController
     responses_for_page = question_responses(questions_for_page)
     @case_log.previous_page = previous_page
     if @case_log.update(responses_for_page)
-      redirect_path = form.next_page_redirect_path(previous_page)
+      redirect_path = get_next_page_path(form, previous_page, responses_for_page)
       redirect_to(send(redirect_path, @case_log))
     else
       page_info = form.all_pages[previous_page]
@@ -128,4 +128,19 @@ private
 
     params.require(:case_log).permit(CaseLog.editable_fields)
   end
+
+  def get_next_page_path(form, previous_page, responses_for_page = {})
+    questions_for_page = form.questions_for_page(previous_page)
+    questions_for_page.each do |question, content|
+      next unless content.key?("conditional_route_to")
+
+      content["conditional_route_to"].each do |route, answer|
+        if responses_for_page[question].present? && answer.include?(responses_for_page[question])
+          return "case_log_#{route}_path"
+        end
+      end
+    end
+
+    form.next_page_redirect_path(previous_page)
+  end
 end

app/helpers/check_answers_helper.rb

@@ -10,16 +10,46 @@ module CheckAnswersHelper
   end
 
   def total_questions(subsection, case_log, form)
-    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
+    total_questions = {}
+    page_name = form.pages_for_subsection(subsection).keys.first
+
+    while page_name.to_s != "check_answers"
+      questions = form.questions_for_page(page_name)
+      question_key = questions.keys[0]
+      question_value = questions.values[0]
+
+      applicable_questions = filter_conditional_questions(questions, case_log)
+      total_questions = total_questions.merge(applicable_questions)
+
+      page_name = get_next_page_name(form, page_name, applicable_questions, question_key, case_log, question_value)
+    end
+
+    total_questions
+  end
+
+  def filter_conditional_questions(questions, case_log)
+    applicable_questions = questions
+
+    questions.each do |k, question|
+      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 }
         end
       end
-      questions_not_applicable.include?(question_key)
     end
+    applicable_questions
+  end
+
+  def get_next_page_name(form, page_name, applicable_questions, question_key, case_log, question_value)
+    if applicable_questions[question_key].key?("conditional_route_to")
+      applicable_questions[question_key]["conditional_route_to"].each do |conditional_page_key, condition|
+        unless condition_not_met(case_log, question_key, question_value, condition)
+          return conditional_page_key
+        end
+      end
+    end
+
+    form.next_page(page_name)
   end
 
   def condition_not_met(case_log, question_key, question, condition)

app/helpers/tasklist_helper.rb

@@ -29,7 +29,7 @@ module TasklistHelper
     subsections.find { |subsection| is_incomplete?(subsection, case_log, form.questions_for_subsection(subsection).keys) }
   end
 
-  def get_sections_count(form, case_log, status = :all)
+  def get_subsections_count(form, case_log, status = :all)
     subsections = form.all_subsections.keys
     return subsections.count if status == :all
 

app/models/form.rb

@@ -52,6 +52,13 @@ class Form
   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"
+
+      return next_page
+    end
+
     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

app/views/case_logs/edit.html.erb

@@ -6,7 +6,7 @@
 
       <h2 class="govuk-heading-s govuk-!-margin-bottom-2">This submission is
         <%= @case_log.status %></h2>
-      <p class="govuk-body govuk-!-margin-bottom-7">You've completed <%= get_sections_count(@form, @case_log, :completed) %> of <%= get_sections_count(@form, @case_log, :all) %> sections.</p>
+      <p class="govuk-body govuk-!-margin-bottom-7">You've completed <%= get_subsections_count(@form, @case_log, :completed) %> of <%= get_subsections_count(@form, @case_log, :all) %> sections.</p>
       <p class="govuk-body govuk-!-margin-bottom-7">
         <% next_incomplete_section=get_next_incomplete_section(@form, @case_log) %>
         <a href="#<%= next_incomplete_section %>"

app/views/form/_check_answers_table.html.erb

@@ -1,7 +1,7 @@
 <dl class="govuk-summary-list govuk-!-margin-bottom-9">
   <div class="govuk-summary-list__row">
     <dt class="govuk-summary-list__key">
-      <%= question_info["check_answer_label"].to_s %>
+      <%= question_info["check_answer_label"].to_s.present? ?  question_info["check_answer_label"].to_s : question_info["header"].to_s%>
     <dt>
     <dd class="govuk-summary-list__value">
       <%= @case_log[question_title] %>

app/views/form/page.html.erb

@@ -1,8 +1,5 @@
-<% previous_page = form.previous_page(page_key) %>
 <% content_for :before_content do %>
-  <%= govuk_back_link href: "/case_logs/#{@case_log.id}/#{previous_page}" do %>
-    Back
-  <% end %>
+  <%= link_to 'Back', 'javascript:history.back()', class: "govuk-back-link" %>
 <% end %>
 
 <%= turbo_frame_tag "case_log_form", target: "_top" do %>

db/schema.rb

@@ -84,6 +84,7 @@ ActiveRecord::Schema.define(version: 2021_10_15_090040) do
     t.string "property_void_date"
     t.string "property_major_repairs"
     t.string "property_major_repairs_date"
+    t.integer "property_number_of_times_relet"
     t.string "property_wheelchair_accessible"
     t.string "net_income"
     t.string "net_income_frequency"
@@ -131,7 +132,6 @@ ActiveRecord::Schema.define(version: 2021_10_15_090040) do
     t.boolean "reasonable_preference_reason_avoid_hardship"
     t.boolean "reasonable_preference_reason_do_not_know"
     t.datetime "discarded_at"
-    t.integer "property_number_of_times_relet"
     t.index ["discarded_at"], name: "index_case_logs_on_discarded_at"
   end
 

spec/controllers/case_logs_controller_spec.rb

@@ -122,5 +122,52 @@ RSpec.describe CaseLogsController, type: :controller do
         expect(case_log.tenant_code).to eq(tenant_code)
       end
     end
+
+    context "conditional routing" do
+      let(:case_log_form_conditional_question_yes_params) do
+        {
+          pregnancy: "Yes",
+          previous_page: "conditional_question",
+        }
+      end
+
+      let(:case_log_form_conditional_question_no_params) do
+        {
+          pregnancy: "No",
+          previous_page: "conditional_question",
+        }
+      end
+
+      it "routes to the appropriate conditional page based on the question answer of the current page" do
+        post :submit_form, params: { id: id, case_log: case_log_form_conditional_question_yes_params }
+        expect(response).to redirect_to("/case_logs/#{id}/conditional_question_yes_page")
+
+        post :submit_form, params: { id: id, case_log: case_log_form_conditional_question_no_params }
+        expect(response).to redirect_to("/case_logs/#{id}/conditional_question_no_page")
+      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["pregnancy"] = "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

spec/features/case_log_spec.rb

@@ -53,7 +53,7 @@ RSpec.describe "Test Features" do
       it "displays a section status" do
         visit("/case_logs/#{empty_case_log.id}")
 
-        assert_selector ".govuk-tag", text: /Not started/, count: 7
+        assert_selector ".govuk-tag", text: /Not started/, count: 8
         assert_selector ".govuk-tag", text: /Completed/, count: 0
         assert_selector ".govuk-tag", text: /Cannot start yet/, count: 1
       end
@@ -62,7 +62,7 @@ RSpec.describe "Test Features" do
         answer_all_questions_in_income_subsection
         visit("/case_logs/#{empty_case_log.id}")
 
-        assert_selector ".govuk-tag", text: /Not started/, count: 6
+        assert_selector ".govuk-tag", text: /Not started/, count: 7
         assert_selector ".govuk-tag", text: /Completed/, count: 1
         assert_selector ".govuk-tag", text: /Cannot start yet/, count: 1
       end
@@ -74,13 +74,13 @@ RSpec.describe "Test Features" do
 
       it "shows the number of completed sections if no sections are completed" do
         visit("/case_logs/#{empty_case_log.id}")
-        expect(page).to have_content("You've completed 0 of 8 sections.")
+        expect(page).to have_content("You've completed 0 of 9 sections.")
       end
 
       it "shows the number of completed sections if one section is completed" do
         answer_all_questions_in_income_subsection
         visit("/case_logs/#{empty_case_log.id}")
-        expect(page).to have_content("You've completed 1 of 8 sections.")
+        expect(page).to have_content("You've completed 1 of 9 sections.")
       end
     end
 
@@ -162,14 +162,17 @@ RSpec.describe "Test Features" do
       end
     end
 
-    describe "Back link directs correctly" do
+    describe "Back link directs correctly", js: true do
       it "go back to tasklist page from tenant code" do
+        visit("/case_logs/#{id}")
         visit("/case_logs/#{id}/tenant_code")
         click_link(text: "Back")
         expect(page).to have_content("Tasklist for log #{id}")
       end
 
-      it "go back to tenant code page from tenant age page" do
+      it "go back to tenant code page from tenant age page", js: true do
+        visit("/case_logs/#{id}/tenant_code")
+        click_button("Save and continue")
         visit("/case_logs/#{id}/tenant_age")
         click_link(text: "Back")
         expect(page).to have_field("case-log-tenant-code-field")
@@ -192,6 +195,7 @@ RSpec.describe "Test Features" do
 
   describe "check answers page" do
     let(:subsection) { "household_characteristics" }
+    let(:conditional_subsection) { "conditional_question" }
 
     context "when the user needs to check their answers for a subsection" do
       it "can be visited by URL" do
@@ -256,6 +260,35 @@ RSpec.describe "Test Features" do
         expect(page).to have_content("You answered all the questions")
         assert_selector "a", text: "Answer the missing questions", count: 0
       end
+
+      it "does not display conditional questions that were not visited" do
+        visit("case_logs/#{id}/#{conditional_subsection}/check_answers")
+        question_labels = ["Has the condition been met?"]
+        question_labels.each do |label|
+          expect(page).to have_content(label)
+        end
+
+        excluded_question_labels = ["Has the next condition been met?", "Has the condition not been met?"]
+        excluded_question_labels.each do |label|
+          expect(page).not_to have_content(label)
+        end
+      end
+
+      it "displays conditional question that were visited" do
+        visit("/case_logs/#{id}/conditional_question")
+        choose("case-log-pregnancy-no-field")
+        click_button("Save and continue")
+        visit("/case_logs/#{id}/#{conditional_subsection}/check_answers")
+        question_labels = ["Has the condition been met?", "Has the condition not been met?"]
+        question_labels.each do |label|
+          expect(page).to have_content(label)
+        end
+
+        excluded_question_labels = ["Has the next condition been met?"]
+        excluded_question_labels.each do |label|
+          expect(page).not_to have_content(label)
+        end
+      end
     end
   end
 
@@ -300,4 +333,33 @@ RSpec.describe "Test Features" do
       end
     end
   end
+
+  describe "conditional page routing", js: true do
+    it "can route the user to a different page based on their answer on the current page" do
+      visit("case_logs/#{id}/conditional_question")
+      # using a question name that is already in the db to avoid
+      # having to add a new column to the db for this test
+      choose("case-log-pregnancy-yes-field", allow_label_click: true)
+      click_button("Save and continue")
+      expect(page).to have_current_path("/case_logs/#{id}/conditional_question_yes_page")
+      click_link(text: "Back")
+      expect(page).to have_current_path("/case_logs/#{id}/conditional_question")
+      choose("case-log-pregnancy-no-field", allow_label_click: true)
+      click_button("Save and continue")
+      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-letting-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
+  end
 end

spec/fixtures/forms/test_form.json

@@ -235,6 +235,70 @@
               }
             }
           }
+        },
+        "conditional_question": {
+          "label": "Conditional question",
+          "pages": {
+            "conditional_question": {
+              "questions": {
+                "pregnancy": {
+                  "check_answer_label": "Has the condition been met?",
+                  "header": "Has the condition been met?",
+                  "type": "radio",
+                  "answer_options": {
+                    "0": "Yes",
+                    "1": "No"
+                  },
+                  "conditional_route_to": {
+                    "conditional_question_yes_page": "Yes",
+                    "conditional_question_no_page": "No"
+                  }
+                }
+              },
+              "default_next_page": "check_answers"
+            },
+            "conditional_question_yes_page": {
+              "questions": {
+                "cbl_letting": {
+                  "check_answer_label": "Has the next condition been met?",
+                  "header": "Has the next condition been met?",
+                  "type": "radio",
+                  "answer_options": {
+                    "0": "Yes",
+                    "1": "No"
+                  }
+                }
+              },
+              "default_next_page": "check_answers"
+            },
+            "conditional_question_no_page": {
+              "questions": {
+                "conditional_question_no_question": {
+                  "check_answer_label": "Has the condition not been met?",
+                  "header": "Has the next condition not been met?",
+                  "type": "radio",
+                  "answer_options": {
+                    "0": "Yes",
+                    "1": "No"
+                  }
+                }
+              },
+              "default_next_page": "conditional_question_no_second_page"
+            },
+            "conditional_question_no_second_page": {
+              "questions": {
+                "conditional_question_no_second_question": {
+                  "check_answer_label": "Has the condition not been met again?",
+                  "header": "Has the next condition not been met again?",
+                  "type": "radio",
+                  "answer_options": {
+                    "0": "Yes",
+                    "1": "No"
+                  }
+                }
+              }
+            }
+          }
         }
       }
     },
@@ -432,7 +496,8 @@
                   "check_answer_label": "Postcode of previous accomodation if the household has moved from settled accommodation",
                   "header": "Postcode for the previous accommodation",
                   "hint_text": "If the household has moved from settled accommodation immediately prior to being re-housed",
-                  "type": "text"
+                  "type": "text",
+                  "conditional_for": { "faake_key": "fake_condition" }
                 }
               }
             }

spec/helpers/check_answers_helper_spec.rb

@@ -16,6 +16,8 @@ RSpec.describe CheckAnswersHelper do
   let(:subsection) { "income_and_benefits" }
   let(:subsection_with_numeric_conditionals) { "household_characteristics" }
   let(:subsection_with_radio_conditionals) { "household_needs" }
+  let(:conditional_routing_subsection) { "conditional_question" }
+  let(:conditional_page_subsection) { "household_needs" }
   form_handler = FormHandler.instance
   let(:form) { form_handler.get_form("test_form") }
 
@@ -89,24 +91,73 @@ RSpec.describe CheckAnswersHelper do
 
     context "conditional questions with type that hasn't been implemented yet" do
       let(:unimplemented_conditional) do
-        { "question_1" =>
+        { "previous_postcode" =>
           { "header" => "The actual question?",
             "hint_text" => "",
             "type" => "date",
             "check_answer_label" => "Question Label",
-            "conditional_for" => { "question_2" => %w[12-12-2021] } },
-          "question_2" =>
-          { "header" => "The second actual question?",
-            "hint_text" => "",
-            "type" => "radio",
-            "check_answer_label" => "The second question label",
-            "answer_options" => { "0" => "Yes", "1" => "No" } } }
+            "conditional_for" => { "question_2" => %w[12-12-2021] } } }
       end
 
       it "raises an error" do
-        allow_any_instance_of(Form).to receive(:questions_for_subsection).and_return(unimplemented_conditional)
+        allow_any_instance_of(Form).to receive(:pages_for_subsection).and_return(unimplemented_conditional)
         expect { total_number_of_questions(subsection, case_log, form) }.to raise_error(RuntimeError, "Not implemented yet")
       end
     end
+
+    context "conditional routing" do
+      it "ignores not visited questions when no questions are answered" do
+        expect(total_number_of_questions(conditional_routing_subsection, case_log, form)).to eq(1)
+      end
+
+      it "counts correct questions when the conditional question is answered" do
+        case_log["pregnancy"] = "Yes"
+        expect(total_number_of_questions(conditional_routing_subsection, case_log, form)).to eq(2)
+      end
+
+      it "counts correct questions when the conditional question is answered" do
+        case_log["pregnancy"] = "No"
+        expect(total_number_of_questions(conditional_routing_subsection, case_log, form)).to eq(3)
+      end
+    end
+
+    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[net_income net_income_frequency net_income_uc_proportion housing_benefit]
+        expect(result.keys).to eq(expected_keys)
+      end
+
+      context "conditional questions on the same page" do
+        it "it filters out conditional questions that were not displayed" do
+          result = total_questions(conditional_page_subsection, case_log, form)
+          expected_keys = %w[armed_forces medical_conditions accessibility_requirements condition_effects]
+          expect(result.keys).to eq(expected_keys)
+        end
+
+        it "it includes conditional questions that were displayed" do
+          case_log["armed_forces"] = "Yes - a regular"
+          result = total_questions(conditional_page_subsection, case_log, form)
+          expected_keys = %w[armed_forces armed_forces_active armed_forces_injured medical_conditions accessibility_requirements condition_effects]
+          expect(result.keys).to eq(expected_keys)
+        end
+      end
+
+      context "conditional routing" do
+        it "it ignores skipped pages and the questions therein when conditional routing" do
+          result = total_questions(conditional_routing_subsection, case_log, form)
+          expected_keys = %w[pregnancy]
+          expect(result.keys).to match_array(expected_keys)
+        end
+
+        it "it includes conditional pages and questions that were displayed" do
+          case_log["pregnancy"] = "Yes"
+          result = total_questions(conditional_routing_subsection, case_log, form)
+          expected_keys = %w[pregnancy cbl_letting]
+          expect(result.keys).to match_array(expected_keys)
+        end
+      end
+    end
   end
 end

spec/helpers/tasklist_helper_spec.rb

@@ -54,23 +54,23 @@ RSpec.describe TasklistHelper do
 
   describe "get sections count" do
     it "returns the total of sections if no status is given" do
-      expect(get_sections_count(form, empty_case_log)).to eq(8)
+      expect(get_subsections_count(form, empty_case_log)).to eq(9)
     end
 
     it "returns 0 sections for completed sections if no sections are completed" do
-      expect(get_sections_count(form, empty_case_log, :completed)).to eq(0)
+      expect(get_subsections_count(form, empty_case_log, :completed)).to eq(0)
     end
 
     it "returns the number of not started sections" do
-      expect(get_sections_count(form, empty_case_log, :not_started)).to eq(7)
+      expect(get_subsections_count(form, empty_case_log, :not_started)).to eq(8)
     end
 
     it "returns the number of sections in progress" do
-      expect(get_sections_count(form, case_log, :in_progress)).to eq(2)
+      expect(get_subsections_count(form, case_log, :in_progress)).to eq(2)
     end
 
     it "returns 0 for invalid state" do
-      expect(get_sections_count(form, case_log, :fake)).to eq(0)
+      expect(get_subsections_count(form, case_log, :fake)).to eq(0)
     end
   end
 

spec/models/case_log_spec.rb

@@ -30,12 +30,11 @@ RSpec.describe Form, type: :model do
       it "if given reasonable preference is yes a reason must be selected" do
         expect {
           CaseLog.create!(reasonable_preference: "Yes",
-            reasonable_preference_reason_homeless: nil,
-            reasonable_preference_reason_unsatisfactory_housing: nil,
-            reasonable_preference_reason_medical_grounds: nil,
-            reasonable_preference_reason_avoid_hardship: nil,
-            reasonable_preference_reason_do_not_know: nil
-          ) 
+                          reasonable_preference_reason_homeless: nil,
+                          reasonable_preference_reason_unsatisfactory_housing: nil,
+                          reasonable_preference_reason_medical_grounds: nil,
+                          reasonable_preference_reason_avoid_hardship: nil,
+                          reasonable_preference_reason_do_not_know: nil)
         }.to raise_error(ActiveRecord::RecordInvalid)
       end
 
@@ -43,7 +42,7 @@ RSpec.describe Form, type: :model do
         expect {
           CaseLog.create!(
             homelessness: "No",
-            reasonable_preference: "Yes"
+            reasonable_preference: "Yes",
           )
         }.to raise_error(ActiveRecord::RecordInvalid)
       end
@@ -53,7 +52,7 @@ RSpec.describe Form, type: :model do
           CaseLog.create!(
             homelessness: "Yes",
             reasonable_preference: "No",
-            reasonable_preference_reason_homeless: true
+            reasonable_preference_reason_homeless: true,
           )
         }.to raise_error(ActiveRecord::RecordInvalid)
       end

spec/models/form_handler_spec.rb

@@ -15,7 +15,7 @@ RSpec.describe FormHandler do
       form_handler = FormHandler.instance
       form = form_handler.get_form("test_form")
       expect(form).to be_a(Form)
-      expect(form.all_pages.count).to eq(18)
+      expect(form.all_pages.count).to eq(22)
     end
   end