From 4d60926b18f276cde8406bf6a3f8484d0f2daa83 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Wed, 12 Apr 2023 12:26:33 +0100 Subject: [PATCH] feat: make tests pass, simplify code and incorporate tasklist/check answers back routing --- app/controllers/form_controller.rb | 4 +-- app/helpers/tasklist_helper.rb | 2 +- app/models/form.rb | 45 +++++++++++++++--------------- app/views/form/page.html.erb | 6 +--- spec/models/form_spec.rb | 14 +++++----- 5 files changed, 33 insertions(+), 38 deletions(-) diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb index 16ae09b57..563c3a7e5 100644 --- a/app/controllers/form_controller.rb +++ b/app/controllers/form_controller.rb @@ -124,9 +124,9 @@ private if is_referrer_check_answers? page_ids = form.subsection_for_page(@page).pages.map(&:id) page_index = page_ids.index(@page.id) - next_page_id = form.next_page(@page, @log, current_user) + next_page_id = form.next_page_id(@page, @log, current_user) next_page = form.get_page(next_page_id) - previous_page = form.previous_page(@page, @log, current_user) + previous_page = form.previous_page_id(@page, @log, current_user) if next_page&.interruption_screen? || next_page_id == previous_page return send("#{@log.class.name.underscore}_#{next_page_id}_path", @log, { referrer: "check_answers" }) diff --git a/app/helpers/tasklist_helper.rb b/app/helpers/tasklist_helper.rb index 30112894f..34086c4c7 100644 --- a/app/helpers/tasklist_helper.rb +++ b/app/helpers/tasklist_helper.rb @@ -15,7 +15,7 @@ module TasklistHelper if subsection.pages.first.routed_to?(log, current_user) subsection.pages.first.id else - log.form.next_page(subsection.pages.first, log, current_user) + log.form.next_page_id(subsection.pages.first, log, current_user) end end diff --git a/app/models/form.rb b/app/models/form.rb index b590ed23b..80fed4444 100644 --- a/app/models/form.rb +++ b/app/models/form.rb @@ -61,55 +61,54 @@ class Form subsections.find { |s| s.pages.find { |p| p.id == page.id } } end - def next_page(page, log, current_user) + def next_page_id(page, log, current_user) return page.next_unresolved_page_id || :check_answers if log.unresolved page_ids = subsection_for_page(page).pages.map(&:id) page_index = page_ids.index(page.id) page_id = if page.interruption_screen? && log[page.questions[0].id] == 1 && page.routed_to?(log, current_user) - previous_page(page, log, current_user) + previous_page_id(page, log, current_user) else page_ids[page_index + 1] end - nxt_page = get_page(page_id) + next_page = get_page(page_id) - return :check_answers if nxt_page.nil? - return nxt_page.id if nxt_page.routed_to?(log, current_user) + return :check_answers if next_page.nil? + return next_page.id if next_page.routed_to?(log, current_user) - next_page(nxt_page, log, current_user) + next_page_id(next_page, log, current_user) end def next_page_redirect_path(page, log, current_user) - nxt_page = next_page(page, log, current_user) - if nxt_page == :check_answers + next_page_id = next_page_id(page, log, current_user) + if next_page_id == :check_answers "#{type}_log_#{subsection_for_page(page).id}_check_answers_path" else - "#{type}_log_#{nxt_page}_path" + "#{type}_log_#{next_page_id}_path" end end - def previous_page(page, log, current_user) + def previous_page_id(page, log, current_user) page_ids = subsection_for_page(page).pages.map(&:id) page_index = page_ids.index(page.id) - page_id = if page_index.zero? - :check_answers - else - page_ids[page_index - 1] - end - prvs_page = get_page(page_id) + return :tasklist if page_index.zero? + + page_id = page_ids[page_index - 1] + previous_page = get_page(page_id) - return :check_answers if prvs_page.nil? - return prvs_page.id if prvs_page.routed_to?(log, current_user) + return previous_page.id if previous_page.routed_to?(log, current_user) - previous_page(prvs_page, log, current_user) + previous_page_id(previous_page, log, current_user) end - def previous_page_redirect_path(page, log, current_user) - previous_page = previous_page(page, log, current_user) - if previous_page == :check_answers + def previous_page_redirect_path(page, log, current_user, referrer) + previous_page_id = previous_page_id(page, log, current_user) + if referrer == "check_answers" "#{type}_log_#{subsection_for_page(page).id}_check_answers_path" + elsif previous_page_id == :tasklist + "#{type}_log_path" else - "#{type}_log_#{previous_page}_path" + "#{type}_log_#{previous_page_id}_path" end end diff --git a/app/views/form/page.html.erb b/app/views/form/page.html.erb index 410492a04..9b4d14d9d 100644 --- a/app/views/form/page.html.erb +++ b/app/views/form/page.html.erb @@ -1,11 +1,7 @@ <% content_for :title, @page.header.presence || @page.questions.first.header.html_safe %> <% content_for :before_content do %> - <% if url_for(:back).include?("check_answers") %> - <%= govuk_back_link(href: :back) %> - <% else %> - <%= govuk_back_link(href: send(@log.form.previous_page_redirect_path(@page, @log, current_user), @log)) %> - <% end %> + <%= govuk_back_link(href: send(@log.form.previous_page_redirect_path(@page, @log, current_user, params[:referrer]), @log)) %> <% end %>
diff --git a/spec/models/form_spec.rb b/spec/models/form_spec.rb index a1cf650c0..209306201 100644 --- a/spec/models/form_spec.rb +++ b/spec/models/form_spec.rb @@ -17,11 +17,11 @@ RSpec.describe Form, type: :model do let(:conditional_section_complete_lettings_log) { FactoryBot.build(:lettings_log, :conditional_section_complete) } describe ".next_page" do - let(:previous_page) { form.get_page("person_1_age") } + let(:previous_page_id) { form.get_page("person_1_age") } let(:value_check_previous_page) { form.get_page("net_income_value_check") } it "returns the next page given the previous" do - expect(form.next_page(previous_page, lettings_log, user)).to eq("person_1_gender") + expect(form.next_page_id(previous_page, lettings_log, user)).to eq("person_1_gender") end context "when the current page is a value check page" do @@ -33,12 +33,12 @@ RSpec.describe Form, type: :model do it "returns the previous page if answer is `No` and the page is routed to" do lettings_log.net_income_value_check = 1 - expect(form.next_page(value_check_previous_page, lettings_log, user)).to eq("net_income") + expect(form.next_page_id(value_check_previous_page, lettings_log, user)).to eq("net_income") end it "returns the next page if answer is `Yes` answer and the page is routed to" do lettings_log.net_income_value_check = 0 - expect(form.next_page(value_check_previous_page, lettings_log, user)).to eq("net_income_uc_proportion") + expect(form.next_page_id(value_check_previous_page, lettings_log, user)).to eq("net_income_uc_proportion") end end end @@ -54,18 +54,18 @@ RSpec.describe Form, type: :model do it "returns the previous page if the page is routed to" do page_index = page_ids.index("conditional_question_no_second_page") - expect(form.previous_page(page_ids, page_index, lettings_log, user)).to eq("conditional_question_no_page") + expect(form.previous_page_id(page_ids, page_index, lettings_log, user)).to eq("conditional_question_no_page") end it "returns the page before the previous one if the previous page is not routed to" do page_index = page_ids.index("conditional_question_no_page") - expect(form.previous_page(page_ids, page_index, lettings_log, user)).to eq("conditional_question") + expect(form.previous_page_id(page_ids, page_index, lettings_log, user)).to eq("conditional_question") end end end describe "next_page_redirect_path" do - let(:previous_page) { form.get_page("net_income") } + let(:previous_page_id) { form.get_page("net_income") } let(:last_previous_page) { form.get_page("housing_benefit") } let(:previous_conditional_page) { form.get_page("conditional_question") }