Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change "Question x's routes" page #1753

Merged
merged 7 commits into from
Feb 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion app/controllers/pages/routes_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ def show
back_link_url = form_pages_path(current_form.id)
pages = FormRepository.pages(current_form)
routes = PageRoutesService.new(form: current_form, pages:, page:).routes
render locals: { current_form:, page:, pages:, routes:, back_link_url: }

# to be eligible for route question has to have one question after it, so should always have next_page
next_page = pages.find(proc { raise "Cannot find page with id #{page.next_page.inspect}" }) { _1.id == page.next_page }

render locals: { current_form:, page:, pages:, next_page:, routes:, back_link_url: }
end

def delete
Expand Down
7 changes: 4 additions & 3 deletions app/presenters/route_summary_card_data_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ def initialize(form:, pages:, page:, routes:)
end

def summary_card_data
conditional_cards = conditional_route_cards
conditional_cards + [default_route_card]
cards = conditional_route_cards
cards << secondary_skip_card if secondary_skip
cards
end

private
Expand Down Expand Up @@ -55,7 +56,7 @@ def conditional_route_card(routing_condition, index)
}
end

def default_route_card
def secondary_skip_card
continue_to_name = page.has_next_page? ? page_name(page.next_page) : end_page_name

actions = if FeatureService.new(group: form.group).enabled?(:branch_routing) && secondary_skip
Expand Down
28 changes: 17 additions & 11 deletions app/views/pages/routes/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,23 @@
<%= govuk_summary_list(**card) %>
<% end %>

<% if page.routing_conditions.present? %>
<ul class="govuk-list govuk-list--spaced">
<li>
<%= govuk_button_link_to t("page_route_card.delete_route"), delete_routes_path(current_form.id, page.id), warning: true %>
</li>
<li>
<%= govuk_link_to t("pages.go_to_your_questions"), form_pages_path(current_form.id) %>
</li>
</ul>
<% else %>
<%= govuk_link_to t("pages.go_to_your_questions"), form_pages_path(current_form.id) %>
<% if routes.map(&:secondary_skip?).none? %>
<h2 class="govuk-heading-m"><%= t(".any_other_answer.heading") %></h2>

<p class="govuk-body"><%= t(".any_other_answer.will_continue_to", next_page_position: next_page.position) %></p>

<% if FeatureService.new(group: current_form.group).enabled? :branch_routing %>
<p class="govuk-body"><%= t(".any_other_answer.skip_later") %></p>

<%= govuk_button_link_to t(".any_other_answer.set_questions_to_skip"), new_secondary_skip_path(current_form.id, page.id), secondary: true %>
<% end %>
<% end %>

<% if routes.many? %>
<%= govuk_button_link_to t("page_route_card.delete_route"), delete_routes_path(current_form.id, page.id), warning: true %>
<% end %>

<p class="govuk-body">
<%= govuk_link_to t("pages.go_to_your_questions"), form_pages_path(current_form.id) %>
</p>
</div>
6 changes: 6 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1199,6 +1199,12 @@ en:
back: Back to question %{page_number}’s routes
caption: Question %{page_number}’s routes
title: Are you sure you want to delete all question %{page_number}’s routes?
show:
any_other_answer:
heading: Any other answer
set_questions_to_skip: Set questions to skip
skip_later: If you need to, you can make these people skip one or more questions later in the form. For example, you might want them to skip route 1’s questions to create 2 different ‘branches’ of questions.
will_continue_to: People who select any other answer will continue to question %{next_page_position}.
submit_save: Save question
payment_link_input:
body_html: |
Expand Down
13 changes: 13 additions & 0 deletions spec/factories/models/forms.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,19 @@
pages do
Array.new(pages_count) { association(:page, :with_selection_settings) }
end

after(:build) do |form|
if form.pages.present?
# assign position and next_page to each page
form.pages.each.with_index(1).each_cons(2) do |page_with_index, next_page_with_index|
page, page_index = page_with_index
next_page, _next_page_index = next_page_with_index

page.position = page_index
page.next_page = next_page&.id
end
end
end
end

trait :missing_pages do
Expand Down
102 changes: 30 additions & 72 deletions spec/presenters/route_summary_card_data_presenter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,64 +3,39 @@
describe RouteSummaryCardDataPresenter do
include Capybara::RSpecMatchers

subject(:service) { described_class.new(form:, pages:, page: current_page, routes:) }
subject(:service) { described_class.new(form:, pages:, page:, routes:) }

let(:form) { build :form, id: 99, pages: }
include_context "with pages with routing"

let(:current_page) do
build(:page, id: 1, position: 1, question_text: "Current Question", next_page: next_page.id, routing_conditions:)
end
let(:form) { build :form, id: 99, pages: }
let(:page) { page_with_skip_route }

let(:routes) do
PageRoutesService.new(form:, pages:, page: current_page).routes
PageRoutesService.new(form:, pages:, page:).routes
end

let(:next_page) do
build(:page, id: 2, position: 2, question_text: "Next Question", routing_conditions: next_page_routing_conditions)
end

let(:pages) { [current_page, next_page] }

let(:routing_condition) do
build(:condition, id: 1, routing_page_id: 1, check_page_id: 1, answer_value: "Yes", goto_page_id: 2, skip_to_end: false)
end

let(:routing_conditions) { [routing_condition] }

let(:next_page_routing_conditions) { [] }

before do
allow(form).to receive(:group).and_return(build(:group))
end

describe "#summary_card_data" do
context "with conditional routes" do
it "returns an array of route cards including conditional and default routes" do
it "returns an array of route cards" do
result = service.summary_card_data
expect(result.length).to eq(2) # 1 conditional + 1 default route
expect(result.length).to eq(1) # 1 conditional route

# conditional route
expect(result[0][:card][:title]).to eq("Route 1")
expect(result[0][:card][:actions].first).to have_link("Edit", href: "/forms/99/pages/1/conditions/1")
expect(result[0][:rows][0][:value][:text]).to eq("Yes")
expect(result[0][:rows][1][:value][:text]).to eq("2. Next Question")

# default route
expect(result[1][:card][:title]).to eq("Route for any other answer")
expect(result[1][:card][:actions][0]).to be_nil
expect(result[1][:rows][0][:value][:text]).to eq("2. Next Question")
end

context "with branch_routing enabled", :feature_branch_routing do
it "has the link to create a secondary skip" do
result = service.summary_card_data
expect(result[1][:rows][1][:value][:text]).to have_link("Set one or more questions to skip later in the form (optional)", href: "/forms/99/pages/1/routes/any-other-answer/questions-to-skip/new")
end
expect(result[0][:card][:actions].first).to have_link("Edit", href: "/forms/99/pages/10/conditions/3")
expect(result[0][:rows][0][:value][:text]).to eq("Skip")
expect(result[0][:rows][1][:value][:text]).to eq("12. Question")
end

context "when the goto_page does not exist" do
let(:routing_condition) do
build(:condition, id: 1, routing_page_id: 1, check_page_id: 1, answer_value: "Yes", goto_page_id: 999, skip_to_end: false)
before do
page.routing_conditions.first.tap do |condition|
condition.goto_page_id = 999
end
end

it "uses placeholder text instead of question text" do
Expand All @@ -71,8 +46,11 @@
end

context "with skip to end condition" do
let(:routing_condition) do
build(:condition, id: 1, routing_page_id: 1, check_page_id: 1, answer_value: "No", goto_page_id: nil, skip_to_end: true)
before do
page.routing_conditions.first.tap do |condition|
condition.goto_page_id = nil
condition.skip_to_end = true
end
end

it 'shows "Check your answers" as destination' do
Expand All @@ -81,22 +59,14 @@
end
end

context "with a route with no answer_value" do
let(:routing_conditions) do
[build(:condition, id: 1, routing_page_id: 1, check_page_id: 1, answer_value: "Yes", goto_page_id: 2, skip_to_end: false)]
end
context "with conditional routes and secondary skip routes" do
let(:page) { page_with_skip_and_secondary_skip }

let(:next_page_routing_conditions) do
[
build(:condition, id: 2, routing_page_id: 2, check_page_id: 1, answer_value: nil, goto_page_id: nil, skip_to_end: true),
]
end

it 'shows "Check your answers" as destination' do
it "shows the destination of the secondary skip route" do
result = service.summary_card_data
expect(result[1][:rows][0][:value][:text]).to eq("2. Next Question")
expect(result[1][:rows][1][:value][:text]).to eq("2. Next Question")
expect(result[1][:rows][2][:value][:text]).to eq("Check your answers before submitting")
expect(result[1][:rows][0][:value][:text]).to eq("3. Question in branch 1")
expect(result[1][:rows][1][:value][:text]).to eq("4. Question at the end of branch 1 (start of a secondary skip)")
expect(result[1][:rows][2][:value][:text]).to eq("8. Question after a branch route (end of a secondary skip)")
end

context "when branch routing is not enabled", feature_branch_routing: false do
Expand All @@ -109,34 +79,22 @@
context "with branch_routing enabled", :feature_branch_routing do
it "shows the edit secondary skip link" do
result = service.summary_card_data
expect(result[1][:card][:actions].first).to have_link("Edit", href: "/forms/99/pages/1/routes/any-other-answer/questions-to-skip")
expect(result[1][:card][:actions].first).to have_link("Edit", href: "/forms/99/pages/2/routes/any-other-answer/questions-to-skip")
end

it "shows the delete secondary skip link" do
result = service.summary_card_data
expect(result[1][:card][:actions].second).to have_link("Delete", href: "/forms/99/pages/1/routes/any-other-answer/questions-to-skip/delete")
expect(result[1][:card][:actions].second).to have_link("Delete", href: "/forms/99/pages/2/routes/any-other-answer/questions-to-skip/delete")
end
end
end

context "with no conditional routes" do
let(:routing_conditions) { [] }

it "returns only the default route card" do
result = service.summary_card_data
expect(result.length).to eq(1)
expect(result[0][:card][:title]).to eq("Route for any other answer")
end
end

context "when page is the last page" do
let(:current_page) do
build(:page, id: 1, position: 1, question_text: "Current Question", next_page: nil)
end
let(:page) { page_with_no_routes }

it 'shows "Check your answers" as default destination' do
it "returns an empty array" do
result = service.summary_card_data
expect(result[0][:rows][0][:value][:text]).to eq("Check your answers before submitting")
expect(result).to eq []
end
end
end
Expand Down
26 changes: 26 additions & 0 deletions spec/support/shared_context/pages_with_routing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,22 @@
build(
:page,
id: 1,
position: 1,
next_page: 2,
question_text: "Question",
),
build(
:page,
:with_selection_settings,
id: 2,
position: 2,
next_page: 3,
question_text: "Branch question (start of a route)",
selection_options: [{ name: "First branch" }, { name: "Second branch" }],
routing_conditions: [
build(
:condition,
id: 1,
answer_value: "Second branch",
check_page_id: 2,
goto_page_id: 5,
Expand All @@ -25,15 +30,20 @@
build(
:page,
id: 3,
position: 3,
next_page: 4,
question_text: "Question in branch 1",
),
build(
:page,
id: 4,
position: 4,
next_page: 5,
question_text: "Question at the end of branch 1 (start of a secondary skip)",
routing_conditions: [
build(
:condition,
id: 2,
answer_value: nil,
check_page_id: 2,
goto_page_id: 8,
Expand All @@ -44,37 +54,50 @@
build(
:page,
id: 5,
position: 5,
next_page: 6,
question_text: "Question at the start of branch 2 (end of a route)",
),
build(
:page,
id: 6,
position: 6,
next_page: 7,
question_text: "Question in branch 2",
),
build(
:page,
id: 7,
position: 7,
next_page: 8,
question_text: "Question at the end of branch 2",
),
build(
:page,
id: 8,
position: 8,
next_page: 9,
question_text: "Question after a branch route (end of a secondary skip)",
),
build(
:page,
id: 9,
position: 9,
next_page: 10,
question_text: "Question",
),
build(
:page,
:with_selection_settings,
id: 10,
position: 10,
next_page: 11,
question_text: "Skip question",
selection_options: [{ name: "Skip" }, { name: "Don't skip" }],
routing_conditions: [
build(
:condition,
id: 3,
answer_value: "Skip",
check_page_id: 10,
goto_page_id: 12,
Expand All @@ -85,11 +108,14 @@
build(
:page,
id: 11,
position: 11,
next_page: 12,
question_text: "Question to be skipped",
),
build(
:page,
id: 12,
position: 12,
question_text: "Question",
),
]
Expand Down
Loading