From d340517090cff9474807dfe10208da73b228e1ad Mon Sep 17 00:00:00 2001 From: Thomas Iles Date: Fri, 3 Jul 2026 09:24:59 +0100 Subject: [PATCH 1/5] Allow exit page access for multiple branches --- app/controllers/pages/exit_page_controller.rb | 8 ++++++-- spec/requests/pages/exit_page_controller_spec.rb | 13 +++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/app/controllers/pages/exit_page_controller.rb b/app/controllers/pages/exit_page_controller.rb index 9ba623ada1..2f9c74d823 100644 --- a/app/controllers/pages/exit_page_controller.rb +++ b/app/controllers/pages/exit_page_controller.rb @@ -1,6 +1,6 @@ class Pages::ExitPageController < PagesController before_action :can_add_page_routing, only: %i[new create delete destroy] - before_action :ensure_answer_value_present, only: %i[new create] + before_action :ensure_answer_value_present, only: %i[new create], unless: -> { current_form&.group.present? && FeatureService.new(group: current_form.group).enabled?(:multiple_branches) } def new exit_page_input = Pages::ExitPageInput.new(form: current_form, page:, answer_value: params[:answer_value]) @@ -81,7 +81,11 @@ def render_preview private def can_add_page_routing - authorize current_form, :can_add_page_routing_conditions? + if current_form&.group.present? && FeatureService.new(group: current_form.group).enabled?(:multiple_branches) + authorize current_form, :can_edit_form? + else + authorize current_form, :can_add_page_routing_conditions? + end end def exit_page_input_params diff --git a/spec/requests/pages/exit_page_controller_spec.rb b/spec/requests/pages/exit_page_controller_spec.rb index 92459c6e69..01b325a9cc 100644 --- a/spec/requests/pages/exit_page_controller_spec.rb +++ b/spec/requests/pages/exit_page_controller_spec.rb @@ -44,6 +44,12 @@ expect(response.status).to eq(403) expect(response).to render_template("errors/forbidden") end + + context "when mutlple branches is enabled", :multiple_branches do + it "renders the new exit page template" do + expect(response).to render_template("pages/exit_page/new") + end + end end context "when there is no answer value param" do @@ -93,6 +99,13 @@ expect(response.status).to eq(403) expect(response).to render_template("errors/forbidden") end + + context "when mutlple branches is enabled", :multiple_branches do + it "Renders the forbidden page" do + expect(response.status).to eq(403) + expect(response).to render_template("errors/forbidden") + end + end end end From 93eaa7600fd6c0ab02bbbd39cfcb4138ac918a30 Mon Sep 17 00:00:00 2001 From: Thomas Iles Date: Mon, 6 Jul 2026 08:57:28 +0100 Subject: [PATCH 2/5] fixup! Allow exit page access for multiple branches --- spec/requests/pages/exit_page_controller_spec.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/spec/requests/pages/exit_page_controller_spec.rb b/spec/requests/pages/exit_page_controller_spec.rb index 01b325a9cc..9e943c3e13 100644 --- a/spec/requests/pages/exit_page_controller_spec.rb +++ b/spec/requests/pages/exit_page_controller_spec.rb @@ -44,12 +44,6 @@ expect(response.status).to eq(403) expect(response).to render_template("errors/forbidden") end - - context "when mutlple branches is enabled", :multiple_branches do - it "renders the new exit page template" do - expect(response).to render_template("pages/exit_page/new") - end - end end context "when there is no answer value param" do From 0cb6a321a1bf1d54f1b79643d73484f2d9573e6d Mon Sep 17 00:00:00 2001 From: Thomas Iles Date: Mon, 6 Jul 2026 08:57:48 +0100 Subject: [PATCH 3/5] WIP: Read/Write from ExitPage model --- app/input_objects/pages/conditions_input.rb | 3 + app/models/condition.rb | 63 ++++++++++++++++++++- spec/models/condition_spec.rb | 24 ++++++++ 3 files changed, 87 insertions(+), 3 deletions(-) diff --git a/app/input_objects/pages/conditions_input.rb b/app/input_objects/pages/conditions_input.rb index 0c6de0b566..a72ff5ced4 100644 --- a/app/input_objects/pages/conditions_input.rb +++ b/app/input_objects/pages/conditions_input.rb @@ -32,6 +32,9 @@ def update_condition record.skip_to_end = skip_to_end record.goto_page_id = goto_page_id + exit_page_to_delete = record&.exit_page + record.exit_page_id = nil + exit_page_to_delete.destroy! if exit_page_to_delete record.exit_page_heading = nil record.exit_page_markdown = nil end diff --git a/app/models/condition.rb b/app/models/condition.rb index 30a1250a2b..dfb0540ba4 100644 --- a/app/models/condition.rb +++ b/app/models/condition.rb @@ -16,19 +16,44 @@ class Condition < ApplicationRecord translates :exit_page_heading translates :exit_page_markdown, presence: false # Without presence here, the value is nil if set to "" - def self.create_and_update_form!(...) - condition = Condition.new(...) + def self.create_and_update_form!(**args) + exit_page_attributes = args.slice(:exit_page_heading, :exit_page_markdown) + + condition = Condition.new(**args) + + if exit_page_attributes.present? + heading = exit_page_attributes.fetch(:exit_page_heading, "") + markdown = exit_page_attributes.fetch(:exit_page_markdown, "") + condition.exit_page = ExitPage.create(heading:, markdown:, question_page: condition.routing_page) + end + condition.save_and_update_form condition end - def save_and_update_form + def save_and_update_form(**args) + exit_page_attributes = args.slice(:exit_page_heading, :exit_page_markdown) + + if exit_page_attributes.present? + heading = exit_page_attributes.fetch(:exit_page_heading, "") + markdown = exit_page_attributes.fetch(:exit_page_markdown, "") + + if exit_page.present? + exit_page.update!(heading:, markdown:) + else + ExitPage.create!(heading:, markdown:, question_page: routing_page) + end + end + save! form.save_question_changes! end def destroy_and_update_form! + exit_page_to_delete = exit_page destroy! && form.save_question_changes! + + exit_page_to_delete.presence&.destroy! unless FeatureService.new(group: form.group).enabled?(:multiple_branches) end def validation_errors @@ -40,6 +65,38 @@ def validation_errors ].compact end + def exit_page_heading(**args) + return exit_page&.heading(**args) if exit_page.present? + + super(**args) + end + + def exit_page_markdown(**args) + return exit_page&.markdown(**args) if exit_page.present? + + super(**args) + end + + def exit_page_heading=(value, **args) + if exit_page.present? + exit_page.update!(heading: value, **args) + else + attributes[:exit_page] = ExitPage.new(heading: value, question_page: routing_page, **args) + end + + super(value, **args) + end + + def exit_page_markdown=(value, **args) + if exit_page.present? + exit_page.update!(markdown: value, **args) + else + attributes[:exit_page] = ExitPage.new(markdown: value, question_page: routing_page, **args) + end + + super(value, **args) + end + def warning_goto_page_doesnt_exist return nil if is_exit_page? # goto_page_id isn't needed if the route is skipping to the end of the form diff --git a/spec/models/condition_spec.rb b/spec/models/condition_spec.rb index b121be1340..ed81464532 100644 --- a/spec/models/condition_spec.rb +++ b/spec/models/condition_spec.rb @@ -720,4 +720,28 @@ end end end + + describe "using the new ExitPage model" do + describe "#exit_page_heading" do + let(:condition) { create :condition, exit_page_heading: "English heading", exit_page_heading_cy: "Welsh heading" } + + it "returns the heading" do + expect(condition.exit_page_heading).to eq("English heading") + end + + context "when the condition has an ExitPage" do + before do + condition.exit_page = create :exit_page, question_page: condition.routing_page, heading: "Exit page english heading", heading_cy: "Exit page welsh heading" + end + + it "returns the heading from the ExitPage" do + expect(condition.exit_page_heading).to eq("Exit page english heading") + end + + it "returns the welsh heading if the condition an ExitPage" do + expect(condition.exit_page_heading_cy).to eq("Exit page welsh heading") + end + end + end + end end From 6725fc5ef685932dd0b12d7c47ae9cbe3c7653d8 Mon Sep 17 00:00:00 2001 From: Thomas Iles Date: Mon, 6 Jul 2026 16:46:29 +0100 Subject: [PATCH 4/5] Fix heading/markdown setters for welsh I think this ensures the arg is passed on to the setters. --- app/models/condition.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/condition.rb b/app/models/condition.rb index dfb0540ba4..904165bd1c 100644 --- a/app/models/condition.rb +++ b/app/models/condition.rb @@ -79,7 +79,7 @@ def exit_page_markdown(**args) def exit_page_heading=(value, **args) if exit_page.present? - exit_page.update!(heading: value, **args) + exit_page.send(:heading=, value, **args) else attributes[:exit_page] = ExitPage.new(heading: value, question_page: routing_page, **args) end @@ -89,7 +89,7 @@ def exit_page_heading=(value, **args) def exit_page_markdown=(value, **args) if exit_page.present? - exit_page.update!(markdown: value, **args) + exit_page.send(:markdown=, value, **args) else attributes[:exit_page] = ExitPage.new(markdown: value, question_page: routing_page, **args) end From b79d37e562223211a2d514d45bb421e2ea3522fe Mon Sep 17 00:00:00 2001 From: Thomas Iles Date: Mon, 6 Jul 2026 16:47:05 +0100 Subject: [PATCH 5/5] Add accepts_nested_attributes_for to Condition model Try using accepts_nested_attributes_for to ensure exit_page is updated when the condition is updated. --- app/models/condition.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/models/condition.rb b/app/models/condition.rb index 904165bd1c..bc3d5c0e6c 100644 --- a/app/models/condition.rb +++ b/app/models/condition.rb @@ -11,6 +11,9 @@ class Condition < ApplicationRecord has_one :form, through: :routing_page belongs_to :exit_page, optional: true + accepts_nested_attributes_for :exit_page, update_only: true + validates_associated :exit_page + before_destroy :destroy_postconditions translates :exit_page_heading