diff --git a/app/controllers/pages/routes_controller.rb b/app/controllers/pages/routes_controller.rb index 8ec885754..07229ba21 100644 --- a/app/controllers/pages/routes_controller.rb +++ b/app/controllers/pages/routes_controller.rb @@ -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 diff --git a/app/presenters/route_summary_card_data_presenter.rb b/app/presenters/route_summary_card_data_presenter.rb index c6529f36c..3f77d98ab 100644 --- a/app/presenters/route_summary_card_data_presenter.rb +++ b/app/presenters/route_summary_card_data_presenter.rb @@ -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 @@ -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 diff --git a/app/views/pages/routes/show.html.erb b/app/views/pages/routes/show.html.erb index 17da9e2c4..cc05a9fac 100644 --- a/app/views/pages/routes/show.html.erb +++ b/app/views/pages/routes/show.html.erb @@ -20,17 +20,23 @@ <%= govuk_summary_list(**card) %> <% end %> - <% if page.routing_conditions.present? %> -
<%= t(".any_other_answer.will_continue_to", next_page_position: next_page.position) %>
+ + <% if FeatureService.new(group: current_form.group).enabled? :branch_routing %> +<%= t(".any_other_answer.skip_later") %>
+ + <%= 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 %> ++ <%= govuk_link_to t("pages.go_to_your_questions"), form_pages_path(current_form.id) %> +
diff --git a/config/locales/en.yml b/config/locales/en.yml index 473d1fcad..f1f85210b 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -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: | diff --git a/spec/factories/models/forms.rb b/spec/factories/models/forms.rb index 0e05e47d9..a84811c1e 100644 --- a/spec/factories/models/forms.rb +++ b/spec/factories/models/forms.rb @@ -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 diff --git a/spec/presenters/route_summary_card_data_presenter_spec.rb b/spec/presenters/route_summary_card_data_presenter_spec.rb index a5d1b7bc9..1c6c4018b 100644 --- a/spec/presenters/route_summary_card_data_presenter_spec.rb +++ b/spec/presenters/route_summary_card_data_presenter_spec.rb @@ -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 @@ -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 @@ -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 @@ -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 diff --git a/spec/support/shared_context/pages_with_routing.rb b/spec/support/shared_context/pages_with_routing.rb index 17976d952..362130e85 100644 --- a/spec/support/shared_context/pages_with_routing.rb +++ b/spec/support/shared_context/pages_with_routing.rb @@ -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, @@ -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, @@ -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, @@ -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", ), ] diff --git a/spec/views/pages/routes/show.html.erb_spec.rb b/spec/views/pages/routes/show.html.erb_spec.rb index fe66a93ef..4eff0c6dd 100644 --- a/spec/views/pages/routes/show.html.erb_spec.rb +++ b/spec/views/pages/routes/show.html.erb_spec.rb @@ -2,8 +2,10 @@ describe "pages/routes/show.html.erb" do let(:form) { build :form, id: 1, pages: [page] } - let(:page) { build :page, id: 1, position: 1 } - let(:routes) { [build(:condition)] } + let(:pages) { [page, next_page] } + let(:page) { build :page, id: 1, position: 1, next_page: 2, routing_conditions: [build(:condition)] } + let(:next_page) { build :page, id: 2 } + let(:routes) { PageRoutesService.new(form:, pages:, page:).routes } let(:route_cards) do [ @@ -20,7 +22,8 @@ before do allow(RouteSummaryCardDataPresenter).to receive(:new).and_return(route_summary_card_data_service) - render template: "pages/routes/show", locals: { current_form: form, page:, pages: form.pages, routes:, back_link_url: "/back" } + allow(form).to receive(:group).and_return(build(:group)) + render template: "pages/routes/show", locals: { current_form: form, page:, pages:, next_page:, routes:, back_link_url: "/back" } end it "has the correct title" do @@ -50,11 +53,61 @@ expect(rendered).to have_link(I18n.t("pages.go_to_your_questions"), href: form_pages_path(form.id)) end - context "when the page has routing conditions" do - let(:page) { build :page, id: 1, position: 1, routing_conditions: [build(:condition, id: 101)] } + context "when the page has a single skip route" do + include_context "with pages with routing" + + let(:page) { page_with_skip_route } + + it "does not have a link to delete all routes" do + expect(rendered).not_to have_link(I18n.t("page_route_card.delete_route"), href: delete_routes_path(form.id, page.id)) + end + end + + context "when the page does not have a secondary skip route" do + include_context "with pages with routing" + + let(:page) { page_with_skip_route } + let(:next_page) { pages.find { _1.id == page_with_skip_route.next_page } } + + it "has an any other answer section" do + expect(rendered).to have_css "h2.govuk-heading-m", text: "Any other answer" + end + + describe "any other answer section" do + it "shows the number of the next question in the form" do + expect(rendered).to have_text "People who select any other answer will continue to question 11." + end + + context "when the page is the last question" do + it "shows the check your answers page as the next question in the form" do + expect(rendered).to have_text "People who select any other answer will continue to question 11." + end + end + + context "when branch routing is enabled", :feature_branch_routing do + it "has a link to set questions to skip" do + expect(rendered).to have_link( + "Set questions to skip", + class: "govuk-button--secondary", + href: new_secondary_skip_path(form.id, page.id), + ) + end + end + end + end + + context "when the page has a skip and a secondary skip" do + include_context "with pages with routing" + + let(:page) { page_with_skip_and_secondary_skip } it "has a link to delete all routes" do expect(rendered).to have_link(I18n.t("page_route_card.delete_route"), href: delete_routes_path(form.id, page.id)) end + + it "does not have an any other answer section" do + expect(rendered).not_to have_css "h2", text: "Any other answer" + expect(rendered).not_to have_link "Set questions to skip" + end end end