diff --git a/app/controllers/forms/exit_pages_controller.rb b/app/controllers/forms/exit_pages_controller.rb index a9e83aa01..b7a102cf7 100644 --- a/app/controllers/forms/exit_pages_controller.rb +++ b/app/controllers/forms/exit_pages_controller.rb @@ -1,10 +1,23 @@ module Forms class ExitPagesController < StepController def show - return redirect_to form_step_path(@form.id, @form.form_slug, current_context.next_step_slug) unless current_context.can_visit?(@step.id) + unless current_context.can_visit?(@step.id) && @step.exit_page_condition_matches? + return redirect_to form_step_path(@form.id, @form.form_slug, current_context.next_step_slug) + end @back_link = form_step_path(@form.id, @form.form_slug, @step.id) @condition = @step.routing_conditions.first + + if @condition.new_style_exit_page? + @exit_page = @step.exit_pages.find { it.id == @condition.exit_page_id } + raise KeyError, "Couldn't find ExitPage with id=#{@condition.exit_page_id}" if @exit_page.nil? + else + @exit_page = ExitPage.new( + id: nil, + heading: @condition.exit_page_heading, + markdown: @condition.exit_page_markdown, + ) + end end end end diff --git a/app/models/condition.rb b/app/models/condition.rb index 6dd03ea1d..8b93ddf2e 100644 --- a/app/models/condition.rb +++ b/app/models/condition.rb @@ -4,7 +4,6 @@ class Condition delegate( :id, :answer_value, - :exit_page_id, :exit_page_heading, :exit_page_markdown, :validation_errors, @@ -28,7 +27,11 @@ def check_page_id end def goto_page_id - form_document_condition.goto_page_id.to_s + form_document_condition.goto_page_id&.to_s + end + + def exit_page_id + form_document_condition.try(:exit_page_id)&.to_s end def default? @@ -41,10 +44,12 @@ def match?(answer_value) self.answer_value == answer_value end - def exit_page? - return false unless form_document_condition.respond_to?(:exit_page_markdown) + def new_style_exit_page? + form_document_condition.respond_to?(:exit_page_id) + end - form_document_condition.try(:exit_page_id).present? || form_document_condition.exit_page_markdown.is_a?(String) + def exit_page? + form_document_condition.try(:exit_page_id).present? || form_document_condition.try(:exit_page_markdown).is_a?(String) end def skip_to_end? diff --git a/app/models/exit_page.rb b/app/models/exit_page.rb index 067f0ab24..506ff6866 100644 --- a/app/models/exit_page.rb +++ b/app/models/exit_page.rb @@ -2,7 +2,7 @@ class ExitPage attr_reader :id, :heading, :markdown def initialize(id:, heading:, markdown:) - @id = id + @id = id.nil? ? id : id.to_s @heading = heading @markdown = markdown end @@ -18,6 +18,7 @@ def self.from_form_document(form_document_exit_page) def ==(other) super || other.class == self.class && + !!id && other.id == id end end diff --git a/app/views/forms/exit_pages/show.html.erb b/app/views/forms/exit_pages/show.html.erb index 425004924..448f21aa8 100644 --- a/app/views/forms/exit_pages/show.html.erb +++ b/app/views/forms/exit_pages/show.html.erb @@ -1,4 +1,4 @@ -<% set_page_title(form_title(form_name: @current_context.form.name, page_name: @condition.exit_page_heading, mode: @mode)) %> +<% set_page_title(form_title(form_name: @current_context.form.name, page_name: @exit_page.heading, mode: @mode)) %> <% content_for :back_link do %> <%= link_to t("forms.back"), @back_link, class: "govuk-back-link" %> @@ -6,8 +6,8 @@
-

<%= @condition.exit_page_heading %>

- <%= HtmlMarkdownSanitizer.new.render_scrubbed_markdown(@condition.exit_page_markdown) %> +

<%= @exit_page.heading %>

+ <%= HtmlMarkdownSanitizer.new.render_scrubbed_markdown(@exit_page.markdown) %> <%= render SupportDetailsComponent::View.new(@support_details) %>
diff --git a/spec/factories/v2_step.rb b/spec/factories/v2_step.rb index 2951784fd..e4c96162f 100644 --- a/spec/factories/v2_step.rb +++ b/spec/factories/v2_step.rb @@ -5,6 +5,7 @@ sequence(:position) { |n| n } next_step_id { nil } + exit_pages { [] } routing_conditions { [] } type { nil } diff --git a/spec/features/fill_in_form_with_exit_page_spec.rb b/spec/features/fill_in_form_with_exit_page_spec.rb index 88fdeea02..3229c9bdc 100644 --- a/spec/features/fill_in_form_with_exit_page_spec.rb +++ b/spec/features/fill_in_form_with_exit_page_spec.rb @@ -1,8 +1,9 @@ require "rails_helper" feature "Fill in and submit a form with an exit page", type: :feature do - let(:routing_conditions) { [build(:v2_condition, :with_exit_page, routing_page_id: 1, answer_value: "Option 1", exit_page_heading: "This is an exit_page", exit_page_markdown: "This is the contents")] } - let(:steps) { [build(:v2_selection_question_step, id: 1, routing_conditions:, question_text:)] } + let(:exit_page) { build(:v2_exit_page, heading: "This is an exit_page", markdown: "This is the contents") } + let(:routing_conditions) { [build(:v2_condition, :with_exit_page, routing_page_id: 1, answer_value: "Option 1", exit_page:)] } + let(:steps) { [build(:v2_selection_question_step, :with_exit_page, id: 1, routing_conditions:, question_text:, exit_page:)] } let(:form) { build :v2_form_document, :live, form_id: 1, name: "Fill in this form", steps:, start_page: 1, send_copy_of_answers: "enabled" } let(:question_text) { Faker::Lorem.question } let(:reference) { Faker::Alphanumeric.alphanumeric(number: 8).upcase } diff --git a/spec/models/condition_spec.rb b/spec/models/condition_spec.rb index 78faa2153..6ebd2c06c 100644 --- a/spec/models/condition_spec.rb +++ b/spec/models/condition_spec.rb @@ -40,6 +40,38 @@ end end + describe "#goto_page_id" do + context "when goto_page_id is nil" do + let(:form_document_condition) { build(:v2_condition, goto_page_id: nil) } + + it "returns nil" do + expect(condition.goto_page_id).to be_nil + end + end + end + + describe "#exit_page_id" do + context "when form document does not have exit_page_id" do + let(:form_document_condition) do + condition = build(:v2_condition, exit_page_id: nil) + condition.attributes.delete(:exit_page_id) + condition + end + + it "returns nil" do + expect(condition.exit_page_id).to be_nil + end + end + + context "when exit_page_id is nil" do + let(:form_document_condition) { build(:v2_condition, exit_page_id: nil) } + + it "returns nil" do + expect(condition.exit_page_id).to be_nil + end + end + end + describe "#default?" do context "when condition.answer_value is nil" do let(:form_document_condition) do @@ -131,6 +163,30 @@ end end + describe "#new_style_exit_page?" do + context "when condition has exit_page_id attribute" do + let(:form_document_condition) do + build(:v2_condition, exit_page_id: nil) + end + + it "returns true" do + expect(condition.new_style_exit_page?).to be true + end + end + + context "when condition does not have exit_page_id attribute" do + let(:form_document_condition) do + condition = build(:v2_condition, exit_page_id: nil) + condition.attributes.delete(:exit_page_id) + condition + end + + it "returns false" do + expect(condition.new_style_exit_page?).to be false + end + end + end + describe "#exit_page?" do context "when condition has goto page id" do let(:form_document_condition) do @@ -143,6 +199,31 @@ it "returns false" do expect(condition.exit_page?).to be false end + + context "and does not have exit_page_id attribute" do + let(:form_document_condition) do + condition = build(:v2_condition, goto_page_id: Faker::Alphanumeric.alphanumeric(number: 8)) + condition.attributes.delete(:exit_page_id) + condition + end + + it "returns false" do + expect(condition.exit_page?).to be false + end + end + + context "and does not have exit page content attributes" do + let(:form_document_condition) do + condition = build(:v2_condition, goto_page_id: Faker::Alphanumeric.alphanumeric(number: 8)) + condition.attributes.delete(:exit_page_heading) + condition.attributes.delete(:exit_page_markdown) + condition + end + + it "returns false" do + expect(condition.exit_page?).to be false + end + end end context "when condition has exit_page_id" do @@ -156,6 +237,19 @@ it "returns true" do expect(condition.exit_page?).to be true end + + context "and does not have exit page content attributes" do + let(:form_document_condition) do + condition = build(:v2_condition, exit_page_id: 10) + condition.attributes.delete(:exit_page_heading) + condition.attributes.delete(:exit_page_markdown) + condition + end + + it "returns true" do + expect(condition.exit_page?).to be true + end + end end context "when condition has exit page content" do @@ -170,6 +264,18 @@ it "returns true" do expect(condition.exit_page?).to be true end + + context "and does not have exit_page_id attribute" do + let(:form_document_condition) do + condition = build(:v2_condition, exit_page_heading: Faker::Lorem.sentence, exit_page_markdown: Faker::Lorem.paragraph) + condition.attributes.delete(:exit_page_id) + condition + end + + it "returns true" do + expect(condition.exit_page?).to be true + end + end end end diff --git a/spec/models/exit_page_spec.rb b/spec/models/exit_page_spec.rb index 7016dd2f9..85e405eb7 100644 --- a/spec/models/exit_page_spec.rb +++ b/spec/models/exit_page_spec.rb @@ -4,13 +4,13 @@ describe "#initialize" do it "sets the attributes" do exit_page = described_class.new( - id: 1, + id: "1", heading: "You are not elegible for this service", markdown: "Here’s what to do next: ...", ) expect(exit_page).to have_attributes( - id: 1, + id: "1", heading: "You are not elegible for this service", markdown: "Here’s what to do next: ...", ) @@ -24,7 +24,7 @@ expect(exit_page).to be_an described_class expect(exit_page).to have_attributes( - id: form_document_exit_page.id, + id: form_document_exit_page.id.to_s, heading: form_document_exit_page.heading, markdown: form_document_exit_page.markdown, ) diff --git a/spec/requests/forms/exit_pages_controller_spec.rb b/spec/requests/forms/exit_pages_controller_spec.rb new file mode 100644 index 000000000..d43cef278 --- /dev/null +++ b/spec/requests/forms/exit_pages_controller_spec.rb @@ -0,0 +1,114 @@ +require "rails_helper" + +RSpec.describe Forms::ExitPagesController, type: :request do + let(:exit_page) { build(:v2_exit_page) } + let(:steps) { [first_step_in_form, step_with_exit_page, next_step_in_form] } + let(:first_step_in_form) { build(:v2_question_step, :with_text_settings, id: 1, next_step_id: 2) } + let(:step_with_exit_page) { build(:v2_selection_question_step, :with_exit_page, id: 2, next_step_id: 3, exit_page:) } + let(:next_step_in_form) { build(:v2_question_step, id: 3, next_step_id: nil) } + let(:form) { build(:v2_form_document, steps:, start_page: 1) } + + let(:answer) { "Option 1" } + + let(:store) do + { + answers: { + form.form_id.to_s => { + first_step_in_form.id.to_s => { text: "first answer" }, + step_with_exit_page.id.to_s => { selection: answer }, + }, + }, + } + end + + before do + ActiveResource::HttpMock.respond_to do |mock| + mock.get "/api/v2/forms/#{form.form_id}/live", { "Accept" => "application/json" }, form.to_json, 200 + end + + allow(Flow::Context).to receive(:new).and_wrap_original do |original_method, **kwargs| + original_method.call(**kwargs, store:) + end + end + + describe "GET #show" do + it "returns http success" do + get exit_page_path(mode: "form", form_id: form.form_id, form_slug: form.form_slug, step_slug: step_with_exit_page.id) + expect(response).to have_http_status(:success) + end + + it "renders an exit page" do + get exit_page_path(mode: "form", form_id: form.form_id, form_slug: form.form_slug, step_slug: step_with_exit_page.id) + expect(response).to render_template(:show) + expect(assigns(:exit_page)).to eq ExitPage.from_form_document(exit_page) + end + + context "when the question with an exit page has been answered and the exit page should not be shown" do + let(:answer) { "Option 2" } + + it "redirects to the next unanswered question" do + get exit_page_path(mode: "form", form_id: form.form_id, form_slug: form.form_slug, step_slug: step_with_exit_page.id) + expect(response).to redirect_to form_step_path(mode: "form", form_id: form.form_id, form_slug: form.form_slug, step_slug: next_step_in_form.id) + end + end + + context "when the form filler has not answered any questions" do + let(:store) { { answers: {} } } + + it "redirects to the start of the form" do + get exit_page_path(mode: "form", form_id: form.form_id, form_slug: form.form_slug, step_slug: step_with_exit_page.id) + expect(response).to redirect_to form_step_path(mode: "form", form_id: form.form_id, form_slug: form.form_slug, step_slug: first_step_in_form.id) + end + end + + context "when the step does not have an exit page" do + let(:step_without_exit_page) { build(:v2_selection_question_step, id: 2, next_step_id: 3) } + let(:step_with_exit_page) { step_without_exit_page } + + it "redirects to the next unanswered question" do + get exit_page_path(mode: "form", form_id: form.form_id, form_slug: form.form_slug, step_slug: step_without_exit_page.id) + expect(response).to redirect_to form_step_path(mode: "form", form_id: form.form_id, form_slug: form.form_slug, step_slug: next_step_in_form.id) + end + end + end + + context "when the exit page is missing from the form document" do + let(:condition_with_exit_page) { build(:v2_condition, :with_exit_page, routing_page_id: 2, exit_page:) } + let(:step_without_exit_page) { build(:v2_selection_question_step, routing_conditions: [condition_with_exit_page], id: 2, next_step_id: 3) } + let(:step_with_exit_page) { step_without_exit_page } + + it "raises an error" do + expect { + get exit_page_path(mode: "form", form_id: form.form_id, form_slug: form.form_slug, step_slug: step_with_exit_page.id) + }.to raise_error(/Couldn't find ExitPage/) + end + end + + context "when the form document is using old-style exit pages" do + let(:exit_page_heading) { Faker::Lorem.sentence } + let(:exit_page_markdown) { Faker::Lorem.paragraph } + + let(:condition_with_exit_page) do + condition = build(:v2_condition, routing_page_id: 2, goto_page_id: nil, skip_to_end: nil, exit_page_id: nil, exit_page_heading:, exit_page_markdown:) + condition.attributes.delete(:exit_page_id) + condition + end + + let(:step_without_exit_page) do + step = build(:v2_selection_question_step, routing_conditions: [condition_with_exit_page], id: 2, next_step_id: 3) + step.attributes.delete(:exit_pages) + step + end + + let(:step_with_exit_page) { step_without_exit_page } + + it "falls back to the exit page content in the condition" do + get exit_page_path(mode: "form", form_id: form.form_id, form_slug: form.form_slug, step_slug: step_with_exit_page.id) + expect(response).to render_template(:show) + expect(assigns(:exit_page)).to have_attributes( + heading: exit_page_heading, + markdown: exit_page_markdown, + ) + end + end +end diff --git a/spec/requests/forms/step_controller_spec.rb b/spec/requests/forms/step_controller_spec.rb index fa879b2f3..3b6186e69 100644 --- a/spec/requests/forms/step_controller_spec.rb +++ b/spec/requests/forms/step_controller_spec.rb @@ -1049,12 +1049,14 @@ end context "when the page is a an exit question" do + let(:exit_page) { build(:v2_exit_page, heading: "Exit page heading", markdown: "Exit page markdown") } let(:first_step_in_form) do - build :v2_selection_question_step, + build(:v2_selection_question_step, + :with_exit_page, id: 1, next_step_id: 2, - routing_conditions: [build(:v2_condition, :with_exit_page, id: 1, routing_page_id: 1, check_page_id: 1, answer_value: "Option 1", exit_page_markdown: "Exit page markdown", exit_page_heading: "exit page heading")], - is_optional: false + is_optional: false, + exit_page:) end it "redirects to the exit page when exit page answer given" do diff --git a/spec/views/forms/exit_pages/show.html.erb_spec.rb b/spec/views/forms/exit_pages/show.html.erb_spec.rb index 4b777f1bb..9ad24299d 100644 --- a/spec/views/forms/exit_pages/show.html.erb_spec.rb +++ b/spec/views/forms/exit_pages/show.html.erb_spec.rb @@ -1,17 +1,24 @@ require "rails_helper" describe "forms/exit_pages/show.html.erb" do + let(:exit_page) do + ExitPage.new( + id: 99, + heading: "heading", + markdown: " * first line\n * second line\n", + ) + end + let(:form) { build :form, :with_support, name: "exit page form" } let(:mode) { OpenStruct.new(preview_draft?: false, preview_archived?: false, preview_live?: false) } - let(:condition) { build(:v2_condition, :with_exit_page, exit_page_heading: "heading", exit_page_markdown: " * first line\n * second line\n") } let(:support_details) { OpenStruct.new(email: form.support_email) } before do assign(:current_context, OpenStruct.new(form:)) assign(:mode, mode) - assign(:condition, condition) assign(:back_link, "/back") assign(:support_details, support_details) + assign(:exit_page, exit_page) render end @@ -25,7 +32,7 @@ end it "has the correct heading" do - expect(rendered).to have_css("h1", text: condition.exit_page_heading) + expect(rendered).to have_css("h1", text: exit_page.heading) end it "displays the markdown" do