From 3c42b890b9a7a46e9f236dfe90d6347be879e9ac Mon Sep 17 00:00:00 2001 From: Samuel Culley Date: Fri, 26 Jun 2026 08:55:10 +0100 Subject: [PATCH] Update warnings when deleting route questions --- app/controllers/pages_controller.rb | 30 +++++- app/views/pages/delete.html.erb | 28 ++++-- config/locales/en.yml | 29 ++++++ spec/requests/pages_controller_spec.rb | 112 +++++++++++++++++++++++ spec/views/pages/delete.html.erb_spec.rb | 36 +++++++- 5 files changed, 223 insertions(+), 12 deletions(-) diff --git a/app/controllers/pages_controller.rb b/app/controllers/pages_controller.rb index 94c3fafcb3..e7347c65b8 100644 --- a/app/controllers/pages_controller.rb +++ b/app/controllers/pages_controller.rb @@ -18,7 +18,14 @@ def delete @page_goto_conditions = page.goto_conditions - if page.routing_conditions.any? && page.routing_conditions.first.secondary_skip? + if FeatureService.new(group: current_form.group).enabled?(:multiple_branches) + @page_conditions = page.routing_conditions + @routing = @page_conditions + @page_goto_conditions + if @routing.present? + @routing_banner_heading = routing_banner_heading(@page_conditions, @page_goto_conditions) + @routing_banner_html = routing_banner_html(@page_conditions, @page_goto_conditions) + end + elsif page.routing_conditions.any? && page.routing_conditions.first.secondary_skip? @routing = :start_of_secondary_skip_route # route page is condition check page @@ -39,7 +46,6 @@ def delete # route page is condition routing page @route_page = @page_goto_conditions.first.routing_page end - @delete_confirmation_input = Pages::DeleteConfirmationInput.new render locals: { current_form: } @@ -182,4 +188,24 @@ def log_validation_errors CurrentLoggingAttributes.validation_errors = errors.map { |error| "PageList: #{error}" } if errors.any? end + + def routing_banner_heading(page_conditions, goto_conditions) + if page_conditions.present? && goto_conditions.present? + t("pages.delete.routing_and_goto_heading", question_number: page.position) + elsif page_conditions.present? + t("pages.delete.routing_heading", question_number: page.position, count: page_conditions.count) + elsif goto_conditions.present? + t("pages.delete.goto_heading", question_number: page.position, count: goto_conditions.count) + end + end + + def routing_banner_html(page_conditions, goto_conditions) + if page_conditions.present? && goto_conditions.present? + t("pages.delete.routing_and_goto_html", routes_href: routes_path(current_form.id)) + elsif page_conditions.present? + t("pages.delete.routing_html", question_number: page.position, count: page_conditions.count, routes_href: routes_path(current_form.id)) + elsif goto_conditions.present? + t("pages.delete.goto_html", question_number: page.position, count: goto_conditions.count, routes_href: routes_path(current_form.id)) + end + end end diff --git a/app/views/pages/delete.html.erb b/app/views/pages/delete.html.erb index 69f33a7514..ca28ed68e5 100644 --- a/app/views/pages/delete.html.erb +++ b/app/views/pages/delete.html.erb @@ -5,16 +5,26 @@
<% unless @delete_confirmation_input.errors.any? %> - <% if @routing.present? %> - <%= govuk_notification_banner(title_text: "Important") do |banner| - banner.with_heading(text: t(".notification_banner.#{@routing}.heading_text", question_number: @page.position), tag: :h3) + <% if FeatureService.new(group: current_form.group).enabled?(:multiple_branches) %> + <% if @routing.present? %> + <%= govuk_notification_banner(title_text: "Important") do |banner| + banner.with_heading(text: @routing_banner_heading, tag: :h3) + + @routing_banner_html + end %> + <% end %> + <% else %> + <% if @routing.present? %> + <%= govuk_notification_banner(title_text: "Important") do |banner| + banner.with_heading(text: t(".notification_banner.#{@routing}.heading_text", question_number: @page.position), tag: :h3) - t( - ".notification_banner.#{@routing}.html", - route_page_question_number: @route_page.position, - show_routes_href: show_routes_path(current_form.id, @route_page.id), - ) - end %> + t( + ".notification_banner.#{@routing}.html", + route_page_question_number: @route_page.position, + show_routes_href: show_routes_path(current_form.id, @route_page.id), + ) + end %> + <% end %> <% end %> <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index df76d3a7df..680812b282 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1640,6 +1640,18 @@ en: back_link: Back to question %{question_number}’s routes hint_html: "

You can skip people who select one specific answer to question %{question_number} to a later question or page. People who select any other answer will continue to question %{continue_to_question_number}.

" delete: + goto_heading: + one: There’s a route to question %{question_number} + other: There are routes to question %{question_number} + goto_html: + one: | +

+ If you delete this question, the route to it will also be deleted. View your question routes +

+ other: | +

+ If you delete this question, the routes to it will also be deleted. View your question routes +

notification_banner: end_of_route: heading_text: Question %{question_number} is at the end of a route @@ -1669,6 +1681,23 @@ en: Question %{route_page_question_number}’s route starts at this question. If you delete this question, the route from it will also be deleted.

+ routing_and_goto_heading: There are routes to and from question %{question_number} + routing_and_goto_html: | +

+ If you delete this question, its routes will also be deleted. View your question routes +

+ routing_heading: + one: Question %{question_number} has a route + other: Question %{question_number} has routes + routing_html: + one: | +

+ If you delete this question, its route will also be deleted. View your question routes +

+ other: | +

+ If you delete this question, its routes will also be deleted. View your question routes +

title: Are you sure you want to delete this question? delete_question: Delete question destroy: diff --git a/spec/requests/pages_controller_spec.rb b/spec/requests/pages_controller_spec.rb index 31b8175616..03a097343c 100644 --- a/spec/requests/pages_controller_spec.rb +++ b/spec/requests/pages_controller_spec.rb @@ -248,6 +248,118 @@ end end end + + context "when the multiple_branches feature is enabled", :feature_multiple_branches do + let(:form) { create(:form, :ready_for_routing) } + let(:pages) { form.pages } + let(:page) { pages.first } + + context "when page to delete has no routes" do + it "does not render a warning" do + get delete_page_path(form_id: form.id, page_id: page.id) + + expect(response.body).not_to include "Important" + end + end + + context "when page to delete is the start of one route" do + let(:page) { pages.first } + + before do + create(:condition, routing_page_id: page.id, answer_value: "Red", goto_page_id: pages.last.id) + end + + it "renders a warning about deleting this page" do + get delete_page_path(form_id: form.id, page_id: page.id) + + assert_select(".govuk-notification-banner", count: 1) do + assert_select "*", "Important" + assert_select "h3", "Question #{page.position} has a route" + assert_select "p.govuk-body", /If you delete this question, its route will also be deleted/ + assert_select "p.govuk-body a", "View your question routes" + end + end + end + + context "when page to delete is the start of mulltiple routes" do + let(:page) { pages.first } + + before do + create(:condition, routing_page_id: page.id, answer_value: "Red", goto_page_id: pages.last.id) + create(:condition, routing_page_id: page.id, answer_value: "Blue", goto_page_id: pages.last.id) + end + + it "renders a warning about deleting this page" do + get delete_page_path(form_id: form.id, page_id: page.id) + + assert_select(".govuk-notification-banner", count: 1) do + assert_select "*", "Important" + assert_select "h3", "Question #{page.position} has routes" + assert_select "p.govuk-body", /If you delete this question, its routes will also be deleted/ + assert_select "p.govuk-body a", "View your question routes" + end + end + end + + context "when page to delete is the end of a route" do + let(:page) { pages.last } + + before do + create(:condition, routing_page_id: pages.first.id, answer_value: "Red", goto_page_id: page.id) + end + + it "renders a warning about deleting this page" do + get delete_page_path(form_id: form.id, page_id: page.id) + + assert_select(".govuk-notification-banner", count: 1) do + assert_select "*", "Important" + assert_select "h3", "There’s a route to question #{page.position}" + assert_select "p.govuk-body", /If you delete this question, the route to it will also be deleted./ + assert_select "p.govuk-body a", "View your question routes" + end + end + end + + context "when page to delete is the end of multiple routes" do + let(:page) { pages.last } + + before do + create(:condition, routing_page_id: pages.first.id, answer_value: "Red", goto_page_id: page.id) + create(:condition, routing_page_id: pages.first.id, answer_value: "Blue", goto_page_id: page.id) + end + + it "renders a warning about deleting this page" do + get delete_page_path(form_id: form.id, page_id: page.id) + + assert_select(".govuk-notification-banner", count: 1) do + assert_select "*", "Important" + assert_select "h3", "There are routes to question #{page.position}" + assert_select "p.govuk-body", /If you delete this question, the routes to it will also be deleted./ + assert_select "p.govuk-body a", "View your question routes" + end + end + end + + context "when page to delete is the start and end of multiple routes" do + let(:page) { pages.third } + + before do + create(:condition, routing_page_id: pages.first.id, answer_value: "Red", goto_page_id: page.id) + create(:condition, routing_page_id: page.id, answer_value: "Blue", goto_page_id: pages.last.id) + end + + it "renders a warning about deleting this page" do + get delete_page_path(form_id: form.id, page_id: page.id) + + assert_select(".govuk-notification-banner", count: 1) do + assert_select "*", "Important" + assert_select "h3", "There are routes to and from question #{page.position}" + assert_select "p.govuk-body", /If you delete this question, its routes will also be deleted./ + assert_select "p.govuk-body a", "View your question routes" + end + end + end + end end end diff --git a/spec/views/pages/delete.html.erb_spec.rb b/spec/views/pages/delete.html.erb_spec.rb index 2ddd7d9d17..68ee3cfb69 100644 --- a/spec/views/pages/delete.html.erb_spec.rb +++ b/spec/views/pages/delete.html.erb_spec.rb @@ -1,6 +1,6 @@ require "rails_helper" -RSpec.describe "pages/delete" do +RSpec.describe "pages/delete", feature_multiple_branches: false do let(:delete_confirmation_input) { Forms::DeleteConfirmationInput.new } let(:current_form) { create :form } let(:page) { create :page, form: current_form } @@ -195,4 +195,38 @@ end end end + + context "when multiple_branches is enabled", :feature_multiple_branches do + context "when page to delete is not associated with any routes" do + before do + assign(:routing, []) + end + + it "does not render a notification banner" do + expect(rendered).not_to have_css ".govuk-notification-banner" + end + end + + context "when the page to delete is associated with a route" do + before do + assign(:routing, ["a route"]) + assign(:routing_banner_heading, "Question 1 has a route") + assign(:routing_banner_html, "

If you delete this question, its route will also be deleted.

".html_safe) + + render locals: { current_form: } + end + + it "renders a notification banner" do + expect(rendered).to have_css ".govuk-notification-banner" + end + + describe "notification banner" do + subject(:banner) { rendered.html.at_css(".govuk-notification-banner") } + + it { is_expected.to have_text "Important" } + it { is_expected.to have_css "h3.govuk-notification-banner__heading", text: "Question 1 has a route", count: 1 } + it { is_expected.to have_css "p.govuk-body", text: "If you delete this question, its route will also be deleted." } + end + end + end end