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

Add warnings when deleting questions that are associated with routes #1663

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
29 changes: 28 additions & 1 deletion app/controllers/pages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,34 @@ def delete
@item_name = @page.question_text
@back_url = edit_question_path(current_form.id, @page.id)

all_form_conditions = current_form.pages.flat_map(&:routing_conditions).compact_blank
@page_goto_conditions = all_form_conditions.select { |condition| condition.goto_page_id == @page.id }

if @page.routing_conditions.any? && @page.routing_conditions.first.secondary_skip?
@routing = :start_of_secondary_skip_route

# route owner is condition check page
@route_owner = PageRepository.find(page_id: @page.routing_conditions.first.check_page_id, form_id: current_form.id)
elsif @page.routing_conditions.any?
@routing = :start_of_route

# route owner is us
@route_owner = @page
elsif @page_goto_conditions.any? && @page_goto_conditions.first.secondary_skip?
@routing = :end_of_secondary_skip_route

# route owner is condition check page
@route_owner = PageRepository.find(page_id: @page_goto_conditions.first.check_page_id, form_id: current_form.id)
elsif @page_goto_conditions.any?
@routing = :end_of_route

# route owner is condition routing page
@route_owner = PageRepository.find(page_id: @page_goto_conditions.first.routing_page_id, form_id: current_form.id)
end

@delete_confirmation_input = Forms::DeleteConfirmationInput.new

render locals: { current_form: }
end

def destroy
Expand All @@ -35,7 +62,7 @@ def destroy
redirect_to @back_url
end
else
render :delete
render :delete, locals: { current_form: }
end
rescue StandardError
flash[:message] = "Deletion unsuccessful"
Expand Down
15 changes: 15 additions & 0 deletions app/views/pages/delete.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,21 @@

<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds">

<% 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", page_position: @page.position), tag: :h3)

t(
".notification_banner.#{@routing}.html",
route_owner_position: @route_owner.position,
show_routes_href: show_routes_path(current_form.id, @route_owner.id),
)
end %>
<% end %>
<% end %>

<%= render(
@delete_confirmation_input,
url: @url,
Expand Down
29 changes: 29 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1133,6 +1133,35 @@ en:
pages:
answer_settings: Answer settings
delete:
notification_banner:
end_of_route:
heading_text: Question %{page_position} is at the end of a route
html: |
<p class="govuk-body">
<a class="govuk-notification-banner__link" href="%{show_routes_href}">Question %{route_owner_position}’s route</a>
goes to this question. If you delete this question, question %{route_owner_position}’s routes will also be deleted.
</p>
end_of_secondary_skip_route:
heading_text: Question %{page_position} is at the end of a route
html: |
<p class="govuk-body">
<a class="govuk-notification-banner__link" href="%{show_routes_href}">Question %{route_owner_position}’s route</a>
goes to this question. If you delete this question, the route to it will also be deleted.
</p>
start_of_route:
heading_text: Question %{page_position} is the start of a route
html: |
<p class="govuk-body">
If you delete this question, its routes will also be deleted.
<a class="govuk-notification-banner__link" href="%{show_routes_href}">View question %{route_owner_position}’s routes</a>.
</p>
start_of_secondary_skip_route:
heading_text: Question %{page_position} is the start of a route
html: |
<p class="govuk-body">
<a class="govuk-notification-banner__link" href="%{show_routes_href}">Question %{route_owner_position}’s route</a>
starts at this question. If you delete this question, the route from it will also be deleted.
</p>
title: Are you sure you want to delete this question?
delete_question: Delete question
go_to_your_questions: Back to your questions
Expand Down
179 changes: 174 additions & 5 deletions spec/requests/pages_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
require "rails_helper"

RSpec.describe PagesController, type: :request do
let(:form_response) { build :form, id: 2 }
let(:form_response) { Form.new(attributes_for(:form, id: 2), true) }

let(:group) { create(:group, organisation: standard_user.organisation) }
let(:membership) { create :membership, group:, user: standard_user }
Expand Down Expand Up @@ -82,24 +82,37 @@
describe "#delete" do
describe "given a valid page" do
let(:page) do
Page.new({
build(
:page,
id: 1,
form_id: 2,
question_text: "What is your work address?",
hint_text: "This should be the location stated in your contract.",
answer_type: "address",
next_page: nil,
is_optional: false,
})
)
end

let(:pages) { [page] }

before do
ActiveResource::HttpMock.respond_to do |mock|
mock.get "/api/v1/forms/2/pages", headers, pages.to_json, 200
end

allow(FormRepository).to receive(:find).and_return(form_response)
allow(PageRepository).to receive_messages(find: page, destroy: true)

pages.each do |page|
allow(PageRepository).to receive(:find).with(page_id: page.id.to_s, form_id: 2).and_return(page)
allow(PageRepository).to receive(:find).with(page_id: page.id, form_id: 2).and_return(page)
end

allow(PageRepository).to receive_messages(destroy: true)

GroupForm.create!(form_id: 2, group_id: group.id)

get delete_page_path(form_id: 2, page_id: 1)
get delete_page_path(form_id: 2, page_id: page.id)
end

it "renders the delete page template" do
Expand All @@ -117,6 +130,162 @@
expect(response).to have_http_status :forbidden
end
end

context "when page to delete has no routes" do
it "does not render a warning" do
expect(response.body).not_to include "Important"
end
end

context "when page to delete is start of one or more routes" do
let(:page) do
build(
:page,
:with_selection_settings,
id: 1,
form_id: 2,
question_text: "What is your favourite colour?",
selection_options: [{ name: "Red" }, { name: "Green" }, { name: "Blue" }],
only_one_option: true,
routing_conditions: [
build(:condition, routing_page_id: 1, check_page_id: 1, value: "red", skip_to_end: true),
build(:condition, routing_page_id: 1, check_page_id: 1, value: "green", goto_page_id: 3),
],
)
end

it "renders a warning about deleting this page" do
assert_select(".govuk-notification-banner", count: 1) do
assert_select "*", "Important"
assert_select "h3", "Question #{page.position} is the start of a route"
assert_select "p.govuk-body", /If you delete this question, its routes will also be deleted/
assert_select "p.govuk-body a", "View question #{page.position}’s routes"
end
end
end

context "when page to delete is at the end of a route" do
let(:pages) do
[
build(
:page,
:with_selection_settings,
id: 1,
form_id: 2,
position: 1,
question_text: "What is your favourite colour?",
selection_options: [{ name: "Red" }, { name: "Green" }, { name: "Blue" }],
only_one_option: true,
routing_conditions: [
build(:condition, routing_page_id: 1, check_page_id: 1, value: "green", goto_page_id: 3),
],
),
build(
:page,
id: 3,
form_id: 2,
position: 3,
),
]
end

let(:page) { pages.last }

it "renders a warning about deleting this page" do
assert_select(".govuk-notification-banner", count: 1) do
assert_select "*", "Important"
assert_select "h3", "Question 3 is at the end of a route"
assert_select "p.govuk-body a", "Question 1’s route"
assert_select "p.govuk-body", /Question 1’s route\s*goes to this question. If you delete this question, question 1’s routes will also be deleted./
end
end
end

context "when page to delete is start of a secondary skip route" do
let(:pages) do
[
build(
:page,
:with_selection_settings,
id: 1,
form_id: 2,
position: 1,
question_text: "What is your favourite colour?",
selection_options: [{ name: "Red" }, { name: "Green" }, { name: "Blue" }],
only_one_option: true,
routing_conditions: [
build(:condition, routing_page_id: 1, check_page_id: 1, value: "green", goto_page_id: 3),
],
),
build(
:page,
id: 5,
form_id: 2,
position: 5,
routing_conditions: [
build(:condition, routing_page_id: 5, check_page_id: 1, value: nil, goto_page_id: 8),
],
),
]
end

let(:page) { pages.last }

it "renders a warning about deleting this page" do
assert_select(".govuk-notification-banner", count: 1) do
assert_select "*", "Important"
assert_select "h3", "Question 5 is the start of a route"
assert_select "p.govuk-body a", "Question 1’s route"
assert_select "p.govuk-body", /Question 1’s route\s*starts at this question. If you delete this question, the route from it will also be deleted./
end
end
end

context "when page to delete is at the end of a secondary skip route" do
let(:pages) do
[
build(
:page,
:with_selection_settings,
id: 1,
form_id: 2,
position: 1,
question_text: "What is your favourite colour?",
selection_options: [{ name: "Red" }, { name: "Green" }, { name: "Blue" }],
only_one_option: true,
routing_conditions: [
build(:condition, routing_page_id: 1, check_page_id: 1, value: "green", goto_page_id: 3),
],
),
build(
:page,
id: 5,
form_id: 2,
position: 5,
routing_conditions: [
build(:condition, routing_page_id: 5, check_page_id: 1, value: nil, goto_page_id: 8),
],
),
build(
:page,
id: 8,
form_id: 2,
position: 8,
),
]
end

let(:page) { pages.last }

it "renders a warning about deleting this page" do
assert_select(".govuk-notification-banner", count: 1) do
assert_select "*", "Important"
assert_select "h3", "Question 8 is at the end of a route"
assert_select "p.govuk-body a", "Question 1’s route"
assert_select "p.govuk-body", /Question 1’s route\s*goes to this question. If you delete this question, the route to it will also be deleted./
end
end
end
end
end

Expand Down
Loading
Loading