From d547bfd038323d904a57feb77217fec5ba232f28 Mon Sep 17 00:00:00 2001 From: Thomas Iles Date: Wed, 26 Aug 2026 15:46:34 +0100 Subject: [PATCH 1/3] Add exit pages to revert_draft_form_service We need to ensure we are reverting exit pages, even when they are not attached to conditions. This commit adds exit pages to the revert draft service. --- app/services/revert_draft_form_service.rb | 38 +++++- .../revert_draft_form_service_spec.rb | 116 ++++++++++++++++++ 2 files changed, 151 insertions(+), 3 deletions(-) diff --git a/app/services/revert_draft_form_service.rb b/app/services/revert_draft_form_service.rb index a2cbb57863..c312a1d275 100644 --- a/app/services/revert_draft_form_service.rb +++ b/app/services/revert_draft_form_service.rb @@ -62,6 +62,8 @@ def revert_pages_and_nested_associations(steps_data) revert_pages(steps_data) + revert_exit_pages(steps_data) + # revert conditions after pages are created to make sure conditions don't # have validation errors if the page hasn't been created yet revert_routing_conditions(steps_data) @@ -83,6 +85,26 @@ def revert_pages(steps_data) end end + def revert_exit_pages(steps_data) + form.exit_pages.reload + + form_document_exit_page_ids = steps_data.flat_map { |step| step["exit_pages"] || [] }.map { |ep| ep["id"] } + form.exit_pages.where.not(id: form_document_exit_page_ids).destroy_all + + steps_data.each do |step_data| + page = form.pages.find_by(external_id: step_data["id"]) + next if page.blank? + + (step_data["exit_pages"] || []).each do |exit_page_data| + exit_page = ExitPage.find_or_initialize_by(id: exit_page_data["id"]) + exit_page.question_page = page + exit_page.heading = exit_page_data["heading"] + exit_page.markdown = exit_page_data["markdown"] + exit_page.save! + end + end + end + # steps in a form_document store the page attributes under "data" def assign_page_attributes(page, step_data) page_data = step_data["data"] @@ -116,11 +138,21 @@ def revert_routing_conditions(steps_data) def assign_condition_attributes(condition, condition_data) condition.answer_value = condition_data["answer_value"] - condition.routing_page = Page.find_by!(external_id: condition_data["routing_page_id"]) if condition_data["routing_page_id"] - condition.check_page = Page.find_by!(external_id: condition_data["check_page_id"]) if condition_data["check_page_id"] - condition.goto_page = Page.find_by!(external_id: condition_data["goto_page_id"]) if condition_data["goto_page_id"] + condition.routing_page = if condition_data["routing_page_id"] + Page.find_by!(external_id: condition_data["routing_page_id"]) + end + + condition.check_page = if condition_data["check_page_id"] + Page.find_by!(external_id: condition_data["check_page_id"]) + end + + condition.goto_page = if condition_data["goto_page_id"].present? + Page.find_by!(external_id: condition_data["goto_page_id"]) + end + condition.exit_page_heading = condition_data["exit_page_heading"] condition.exit_page_markdown = condition_data["exit_page_markdown"] + condition.exit_page_id = condition_data["exit_page_id"] end def revert_welsh_translations(welsh_form_document) diff --git a/spec/services/revert_draft_form_service_spec.rb b/spec/services/revert_draft_form_service_spec.rb index eb2ab26b1b..b76dd5466b 100644 --- a/spec/services/revert_draft_form_service_spec.rb +++ b/spec/services/revert_draft_form_service_spec.rb @@ -200,6 +200,122 @@ def revert_draft(tag) end end + describe "with multiple branches and exit pages", :feature_multiple_branches do + let(:live_form) { create(:form, :live_with_draft, pages_count: 2) } + let(:exit_page) { create(:exit_page, heading: "Heading", markdown: "Markdown", question_page_id: live_form.pages.first.id) } + + before do + live_form.pages.first.update!(answer_type: "selection", answer_settings: { "only_one_option" => "true", "selection_options" => [{ "name" => "Yes" }, { "name" => "No" }] }) + live_form.pages.first.routing_conditions.create!( + answer_value: "No", + routing_page_id: live_form.pages.first.id, + check_page_id: live_form.pages.first.id, + exit_page_id: exit_page.id, + ) + live_form.latest_form_document.update!(content: live_form.as_form_document(live_at: live_form.updated_at)) + end + + context "when the exit page is removed from the condition in the draft" do + before do + condition = live_form.pages.first.routing_conditions.first + condition.update!(goto_page_id: live_form.pages.second.id, exit_page_id: nil) + end + + it "restores the original condition" do + revert_draft(live_tag) + + live_form.reload + condition = live_form.pages.first.routing_conditions.first + + expect(condition.exit_page.heading).to eq("Heading") + expect(condition.exit_page.markdown).to eq("Markdown") + expect(condition.goto_page).to be_nil + end + end + + context "when the exit page is removed from the condition and the exit page is removed" do + before do + condition = live_form.pages.first.routing_conditions.first + condition.update!(goto_page_id: live_form.pages.second.id, exit_page_id: nil) + exit_page.destroy! + end + + it "restores the original condition and exit page" do + revert_draft(live_tag) + + live_form.reload + condition = live_form.pages.first.routing_conditions.first + + expect(condition.exit_page.heading).to eq("Heading") + expect(condition.exit_page.markdown).to eq("Markdown") + expect(condition.goto_page).to be_nil + end + end + end + + context "when there are exit pages not connected to any conditions" do + let(:live_form) { create(:form, :live_with_draft, pages_count: 2) } + + before do + create(:exit_page, heading: "Heading", markdown: "Markdown", question_page_id: live_form.pages.first.id) + live_form.latest_form_document.update!(content: live_form.as_form_document(live_at: live_form.updated_at)) + end + + context "when the exit page is removed in the draft" do + before do + live_form.exit_pages.first.destroy! + end + + it "restores the original exit page and its ExitPage record" do + revert_draft(live_tag) + + live_form.reload + exit_page = live_form.exit_pages.first + + expect(exit_page).to be_present + expect(exit_page.heading).to eq("Heading") + expect(exit_page.markdown).to eq("Markdown") + expect(exit_page.question_page_id).to eq(live_form.pages.first.id) + end + end + end + + context "when exit pages not in conditions have been modified in the draft" do + let(:live_form) { create(:form, :live_with_draft, pages_count: 2) } + + before do + create(:exit_page, heading: "Original heading", markdown: "Original markdown", question_page_id: live_form.pages.first.id) + live_form.latest_form_document.update!(content: live_form.as_form_document(live_at: live_form.updated_at)) + live_form.exit_pages.first.update!(heading: "Modified heading", markdown: "Modified markdown") + end + + it "reverts the modified exit page to the original content" do + revert_draft(live_tag) + + live_form.reload + exit_page = live_form.exit_pages.first + + expect(exit_page.heading).to eq("Original heading") + expect(exit_page.markdown).to eq("Original markdown") + end + end + + context "when an exit page is added to the draft but not in the live form document" do + let(:live_form) { create(:form, :live_with_draft, pages_count: 2) } + + before do + live_form.latest_form_document.update!(content: live_form.as_form_document(live_at: live_form.updated_at)) + create(:exit_page, heading: "Draft only exit page", markdown: "Draft only markdown", question_page_id: live_form.pages.first.id) + end + + it "removes the exit page added in the draft" do + revert_draft(live_tag) + + live_form.reload + expect(live_form.exit_pages).to be_empty + end + end + context "when the delivery configurations are changed in the draft" do let(:live_form) do create(:form, :live, delivery_configurations: [ From b4b1bd0b2835fa87fefdba9d8181e107a922ba88 Mon Sep 17 00:00:00 2001 From: Thomas Iles Date: Thu, 3 Sep 2026 11:25:49 +0100 Subject: [PATCH 2/3] Add exit pages to copy form service We need to ensure we are copying exit pages, even when they are not attached to conditions. This commit makes sure exit pages are correctly copied. We also make sure welsh translations are copied correctly. --- app/services/form_copy_service.rb | 39 ++++++++++ spec/services/form_copy_service_spec.rb | 98 +++++++++++++++++++++++++ 2 files changed, 137 insertions(+) diff --git a/app/services/form_copy_service.rb b/app/services/form_copy_service.rb index a0e4aaf8dc..bea6aab2c7 100644 --- a/app/services/form_copy_service.rb +++ b/app/services/form_copy_service.rb @@ -14,6 +14,7 @@ def initialize(form, logged_in_user) @form = form @copied_form = Form.new @logged_in_user = logged_in_user + @exit_page_id_mapping = {} end def copy(tag: "draft") @@ -28,6 +29,7 @@ def copy(tag: "draft") prepend_name_for_language(:en) copy_pages(content["steps"]) + copy_exit_pages(content["steps"]) copy_routing_conditions(content["steps"]) @copied_form.copied_from_id = @form.id @@ -71,6 +73,23 @@ def copy_pages(steps) end end + def copy_exit_pages(steps) + return if steps.blank? + + steps.each_with_index do |step, index| + page = @copied_form.pages[index] + + (step["exit_pages"] || []).each do |ep_data| + exit_page = page.exit_pages.build( + heading: ep_data["heading"], + markdown: ep_data["markdown"], + ) + exit_page.save! + @exit_page_id_mapping[ep_data["id"]] = exit_page + end + end + end + def copy_page_attributes(page, step) data = step["data"] page.assign_attributes( @@ -139,6 +158,10 @@ def copy_condition(condition_data, page_id_mapping) exit_page_markdown: condition_data["exit_page_markdown"], ) + if condition_data["exit_page_id"].present? + condition.exit_page = @exit_page_id_mapping[condition_data["exit_page_id"]] + end + condition.save! end @@ -161,6 +184,7 @@ def copy_welsh_page_translations(welsh_steps) page.send("#{attr}=", data[attr]) if data[attr].present? end copy_welsh_exit_page_conditions(step, page) + copy_welsh_exit_pages(step) page.save!(validate: false) end end @@ -177,4 +201,19 @@ def copy_welsh_exit_page_conditions(step, page) end end end + + def copy_welsh_exit_pages(step) + # Exit pages already linked to a condition are handled by copy_welsh_exit_page_conditions + # Here we are only copying exit pages that are not linked to a condition. + condition_exit_page_ids = (step["routing_conditions"] || []).filter_map { |c| c["exit_page_id"] } + standalone_exit_pages = (step["exit_pages"] || []).reject { |ep| condition_exit_page_ids.include?(ep["id"]) } + + standalone_exit_pages.each do |ep_data| + exit_page = @exit_page_id_mapping[ep_data["id"]] + + exit_page.heading = ep_data["heading"] if ep_data["heading"].present? + exit_page.markdown = ep_data["markdown"] if ep_data["markdown"].present? + exit_page.save! + end + end end diff --git a/spec/services/form_copy_service_spec.rb b/spec/services/form_copy_service_spec.rb index b0d8f226fa..f259f2a21d 100644 --- a/spec/services/form_copy_service_spec.rb +++ b/spec/services/form_copy_service_spec.rb @@ -444,6 +444,104 @@ end end + context "when source form has standalone exit pages not connected to conditions" do + let(:source_form) { create(:form, :live_with_draft, :with_pages, pages_count: 2) } + let!(:source_exit_page) do + create(:exit_page, heading: "Exit heading", markdown: "Exit markdown", + question_page: source_form.pages.first) + end + + before do + source_form.latest_form_document.update!(content: source_form.as_form_document) + end + + it "copies all exit pages" do + expect(copied_form.exit_pages.count).to eq(1) + end + + it "creates new exit page records with different IDs" do + expect(copied_form.exit_pages.pluck(:id)).not_to include(source_exit_page.id) + end + + it "copies exit page content" do + copied_exit_page = copied_form.exit_pages.first + + expect(copied_exit_page.heading).to eq("Exit heading") + expect(copied_exit_page.markdown).to eq("Exit markdown") + end + + it "associates the exit page with the correct copied page" do + copied_exit_page = copied_form.exit_pages.first + + expect(copied_exit_page.question_page).to eq(copied_form.pages.first) + expect(copied_exit_page.question_page).not_to eq(source_form.pages.first) + end + end + + context "when a condition references an exit page via exit_page_id", :feature_multiple_branches do + let(:source_form) { create(:form, :live_with_draft, :ready_for_routing, pages_count: 2) } + let!(:source_exit_page) do + create(:exit_page, heading: "Condition exit heading", markdown: "Condition exit markdown", + question_page: source_form.pages.first) + end + + before do + create(:condition, + form: source_form, + routing_page: source_form.pages.first, + check_page: source_form.pages.first, + answer_value: "No", + exit_page: source_exit_page) + source_form.reload + source_form.latest_form_document.update!(content: source_form.as_form_document) + end + + it "copies the exit page for the condition" do + expect(copied_form.pages.first.routing_conditions.first.exit_page).to be_present + end + + it "copies the exit page content" do + copied_condition = copied_form.pages.first.routing_conditions.first + + expect(copied_condition.exit_page.heading).to eq("Condition exit heading") + expect(copied_condition.exit_page.markdown).to eq("Condition exit markdown") + end + + it "creates a new exit page record distinct from the source" do + copied_condition = copied_form.pages.first.routing_conditions.first + + expect(copied_condition.exit_page_id).not_to eq(source_exit_page.id) + end + + it "links the copied exit page to the copied page" do + copied_exit_page = copied_form.pages.first.routing_conditions.first.exit_page + + expect(copied_exit_page.question_page).to eq(copied_form.pages.first) + end + end + + context "when source form has standalone exit pages with Welsh translations" do + let(:source_form) do + form = create(:form, :live, :with_pages, pages_count: 2, available_languages: %w[en cy]) + exit_page = create(:exit_page, heading: "Exit heading", markdown: "Exit markdown", + question_page: form.pages.first) + exit_page.heading_cy = "Pennaw ymadael" + exit_page.markdown_cy = "Markdown ymadael" + exit_page.save! + FormDocumentSyncService.new(form).synchronize_live_form + form.reload + form + end + + it "copies Welsh heading for the exit page" do + expect(copied_form.exit_pages.first.heading_cy).to eq("Pennaw ymadael") + end + + it "copies Welsh markdown for the exit page" do + expect(copied_form.exit_pages.first.markdown_cy).to eq("Markdown ymadael") + end + end + context "when Welsh copy fails" do let(:source_form) do form = create(:form, :live, available_languages: %w[en cy]) From 0509da052ab1f4021b7b02a3126b813fa869c466 Mon Sep 17 00:00:00 2001 From: Thomas Iles Date: Wed, 2 Sep 2026 15:32:52 +0100 Subject: [PATCH 3/3] Show ExitPages in feature reports Change the reports to forms which use the new FormDocument structure. We use a fallback to also show exit pages for legacy form documents. --- .../reports/form_documents_service.rb | 5 +- .../reports/form_documents_service_spec.rb | 78 ++++++++++++++++++- 2 files changed, 78 insertions(+), 5 deletions(-) diff --git a/app/services/reports/form_documents_service.rb b/app/services/reports/form_documents_service.rb index 3c77f57b50..fbc8692470 100644 --- a/app/services/reports/form_documents_service.rb +++ b/app/services/reports/form_documents_service.rb @@ -74,9 +74,8 @@ def has_s3_submissions(form_document) def has_exit_pages?(form_document) form_document["content"]["steps"].any? do |step| - step["routing_conditions"].any? do |condition| - condition["exit_page_markdown"].present? - end + step["exit_pages"]&.any? || + step["routing_conditions"]&.any? { |c| c["exit_page_markdown"].present? } end end diff --git a/spec/services/reports/form_documents_service_spec.rb b/spec/services/reports/form_documents_service_spec.rb index 1733665b92..38c3c850bc 100644 --- a/spec/services/reports/form_documents_service_spec.rb +++ b/spec/services/reports/form_documents_service_spec.rb @@ -231,8 +231,27 @@ described_class.has_exit_pages?(form_document) end - context "when form has one step with one exit page" do - let(:form_document) { branch_route_form.latest_form_document } + context "when a step has one exit page via the ExitPage model" do + let(:form_with_exit_page) do + form = create(:form, :live) + create(:exit_page, question_page: form.pages.first) + form.latest_form_document.update!(content: form.reload.as_form_document(live_at: form.updated_at)) + form + end + let(:form_document) { form_with_exit_page.latest_form_document } + + it { is_expected.to be true } + end + + context "when a step has multiple exit pages via the ExitPage model" do + let(:form_with_multiple_exit_pages) do + form = create(:form, :live) + create(:exit_page, question_page: form.pages.first) + create(:exit_page, question_page: form.pages.first) + form.latest_form_document.update!(content: form.reload.as_form_document(live_at: form.updated_at)) + form + end + let(:form_document) { form_with_multiple_exit_pages.latest_form_document } it { is_expected.to be true } end @@ -242,6 +261,61 @@ it { is_expected.to be false } end + + context "when the form document is a legacy snapshot with exit_page_markdown on a routing condition" do + let(:form_document) do + { + "content" => { + "steps" => [ + { + "routing_conditions" => [ + { "exit_page_markdown" => "Exit page markdown" }, + ], + }, + ], + }, + } + end + + it { is_expected.to be true } + end + + context "when the form document is a legacy snapshot with an empty exit_pages array and exit_page_markdown on a routing condition" do + let(:form_document) do + { + "content" => { + "steps" => [ + { + "exit_pages" => [], + "routing_conditions" => [ + { "exit_page_markdown" => "Exit page markdown" }, + ], + }, + ], + }, + } + end + + it { is_expected.to be true } + end + + context "when the form document is a legacy snapshot with no exit pages" do + let(:form_document) do + { + "content" => { + "steps" => [ + { + "routing_conditions" => [ + { "exit_page_markdown" => nil }, + ], + }, + ], + }, + } + end + + it { is_expected.to be false } + end end describe ".has_add_another_answer?" do