From 65a762b15c5b9b52b7a71946f72edf60da5d3f06 Mon Sep 17 00:00:00 2001 From: Thomas Iles Date: Mon, 22 Jun 2026 10:21:30 +0100 Subject: [PATCH 1/6] Ensure conditions match question type When a question is changed to selection type which supports routing from multiple options, we need to make sure the conditions are updated to match. This commit adds a new method which is called when a question is changed. It updates conditions to based on the selection options for the new question. --- app/input_objects/pages/question_input.rb | 38 +++++++++++++- .../pages/question_input_spec.rb | 52 +++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/app/input_objects/pages/question_input.rb b/app/input_objects/pages/question_input.rb index c639728fa..72f4b9ab0 100644 --- a/app/input_objects/pages/question_input.rb +++ b/app/input_objects/pages/question_input.rb @@ -62,7 +62,10 @@ def update_page(page) page.assign_attributes(**attrs) ActiveRecord::Base.transaction do - remove_conditions_without_valid_answer_values(page) if draft_question.form&.group&.multiple_branches_enabled? + if draft_question.form&.group&.multiple_branches_enabled? + remove_conditions_without_valid_answer_values(page) + ensure_conditions_match_question_type(page) + end page.save_and_update_form end end @@ -145,4 +148,37 @@ def remove_conditions_without_valid_answer_values(page) page.routing_conditions.where.not(answer_value: valid_answer_values).destroy_all end + + def ensure_conditions_match_question_type(page) + if Forms::RoutesInput.route_with_selection_options?(page) + generic_condition = page.routing_conditions.where(answer_value: nil)&.first + + return if generic_condition.nil? + + attributes = if generic_condition.skip_to_end? + { + goto_page_id: nil, + skip_to_end: true, + check_page_id: page.id, + } + else + { + goto_page_id: generic_condition.goto_page_id, + skip_to_end: false, + check_page_id: page.id, + } + end + + options = page.answer_settings[:selection_options].map { |option| option["value"] } + options << Condition::NONE_OF_THE_ABOVE if page.is_optional + + options.each do |option| + condition_for_option = Condition.find_or_initialize_by(routing_page_id: page.id, answer_value: option) + condition_for_option.assign_attributes(attributes) + condition_for_option.save! + end + + generic_condition.destroy! + end + end end diff --git a/spec/input_objects/pages/question_input_spec.rb b/spec/input_objects/pages/question_input_spec.rb index 8beaa495e..8699629d0 100644 --- a/spec/input_objects/pages/question_input_spec.rb +++ b/spec/input_objects/pages/question_input_spec.rb @@ -535,6 +535,58 @@ end end end + + context "when the original question is being changed to a selection question and has a condition" do + let(:answer_type) { "selection" } + let(:answer_settings) { { only_one_option: "true", selection_options: [{ name: "Option 1", value: "Option 1" }, { name: "Option 2", value: "Option 2" }] } } + let!(:condition) { create(:condition, routing_page_id: page.id) } + + context "when the form has multiple branches enabled" do + let(:form) { create :form, :with_group, group: create(:group, multiple_branches_enabled: true) } + + it "creates a condition for each option" do + expect { question_input.update_page(page) }.to change { page.routing_conditions.count }.from(1).to(2) + + expect(Condition.where(answer_value: "Option 1", routing_page_id: page.id).exists?).to be true + expect(Condition.where(answer_value: "Option 2", routing_page_id: page.id).exists?).to be true + + expect(Condition.exists?(condition.id)).to be false + end + + context "when the question is optional" do + let(:is_optional) { "true" } + + it "creates a condition for each option and none of the above" do + expect { question_input.update_page(page) }.to change { page.routing_conditions.count }.from(1).to(3) + + expect(Condition.where(answer_value: Condition::NONE_OF_THE_ABOVE).exists?).to be true + + expect(Condition.exists?(condition.id)).to be false + end + end + + context "when the original condition skips to the end of the form" do + let!(:condition) { create(:condition, routing_page: page, skip_to_end: true) } + + it "creates a condition for each option which skips to the end" do + expect { question_input.update_page(page) }.to change { page.routing_conditions.count }.from(1).to(2) + + expect(Condition.where(answer_value: "Option 1", routing_page_id: page.id, skip_to_end: true, goto_page_id: nil).exists?).to be true + + expect(Condition.exists?(condition.id)).to be false + end + end + end + + context "when the form does not have multiple branches enabled" do + let(:form) { create :form, :with_group, group: create(:group, multiple_branches_enabled: false) } + + it "does not create a condition for each option or remove the original condition" do + expect { question_input.update_page(page) }.not_to change { page.routing_conditions.count }.from(1) + expect(Condition.exists?(condition.id)).to be true + end + end + end end describe "#update_draft_question!" do From 28c75e11b0e9b964e11fd66225a975b97871dbf6 Mon Sep 17 00:00:00 2001 From: Thomas Iles Date: Fri, 26 Jun 2026 09:34:08 +0100 Subject: [PATCH 2/6] Hide answer type banner for multiple branches When the answer type for a question with conditions is changed, we show a banner warning the user that the routes will be deleted. When changing the answer type in a form with multiple branches, we do not want to show this banner as the conditions are not being deleted. This commit hides the banner when the answer type is changed in a form with multiple branches. --- app/views/pages/type_of_answer.html.erb | 2 +- spec/views/pages/type_of_answer.html.erb_spec.rb | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/app/views/pages/type_of_answer.html.erb b/app/views/pages/type_of_answer.html.erb index f0456a62d..47b232a06 100644 --- a/app/views/pages/type_of_answer.html.erb +++ b/app/views/pages/type_of_answer.html.erb @@ -3,7 +3,7 @@
- <% if @page.present? && @page.routing_conditions.any? %> + <% if !current_form&.group&.multiple_branches_enabled? && @page.present? && @page.routing_conditions.any? %> <%= govuk_notification_banner(title_text: t("banner.default.title")) do |banner| %> <% banner.with_heading(text: t("type_of_answer.routing_warning_about_change_answer_type_heading"), tag: "h3") %> <%= t("type_of_answer.routing_warning_about_change_answer_type_html", pages_link_url: form_pages_path(current_form.id)) %> diff --git a/spec/views/pages/type_of_answer.html.erb_spec.rb b/spec/views/pages/type_of_answer.html.erb_spec.rb index 79d55c74b..7a8cfaee6 100644 --- a/spec/views/pages/type_of_answer.html.erb_spec.rb +++ b/spec/views/pages/type_of_answer.html.erb_spec.rb @@ -77,6 +77,15 @@ .text(normalize_ws: true)) end + context "when multiple branches are enabled" do + let(:form) { create(:form, :with_group, group:) } + let(:group) { create(:group, multiple_branches_enabled: true) } + + it "does not display a warning about routes being deleted if answer type changes" do + expect(rendered).not_to have_selector(".govuk-notification-banner__content") + end + end + context "when no routing conditions set" do let(:routing_conditions) { [] } From 539b5514dd879357f6cce224b281b87a7aba055b Mon Sep 17 00:00:00 2001 From: Thomas Iles Date: Fri, 26 Jun 2026 16:37:59 +0100 Subject: [PATCH 3/6] Use FeatureService in question_input Co-authored-by: Laurence de Bruxelles --- app/views/pages/type_of_answer.html.erb | 2 +- spec/views/pages/type_of_answer.html.erb_spec.rb | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/app/views/pages/type_of_answer.html.erb b/app/views/pages/type_of_answer.html.erb index 47b232a06..67779624b 100644 --- a/app/views/pages/type_of_answer.html.erb +++ b/app/views/pages/type_of_answer.html.erb @@ -3,7 +3,7 @@
- <% if !current_form&.group&.multiple_branches_enabled? && @page.present? && @page.routing_conditions.any? %> + <% if !FeatureService.new(group: current_form&.group).enabled?(:multiple_branches) && @page.present? && @page.routing_conditions.any? %> <%= govuk_notification_banner(title_text: t("banner.default.title")) do |banner| %> <% banner.with_heading(text: t("type_of_answer.routing_warning_about_change_answer_type_heading"), tag: "h3") %> <%= t("type_of_answer.routing_warning_about_change_answer_type_html", pages_link_url: form_pages_path(current_form.id)) %> diff --git a/spec/views/pages/type_of_answer.html.erb_spec.rb b/spec/views/pages/type_of_answer.html.erb_spec.rb index 7a8cfaee6..3aec31ece 100644 --- a/spec/views/pages/type_of_answer.html.erb_spec.rb +++ b/spec/views/pages/type_of_answer.html.erb_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" describe "pages/type_of_answer.html.erb", type: :view do - let(:form) { create :form } + let(:form) { create :form, :with_group } let(:type_of_answer_input) { build :type_of_answer_input } let(:page) { OpenStruct.new(routing_conditions: [], answer_type: "number") } let(:question_number) { 1 } @@ -77,10 +77,7 @@ .text(normalize_ws: true)) end - context "when multiple branches are enabled" do - let(:form) { create(:form, :with_group, group:) } - let(:group) { create(:group, multiple_branches_enabled: true) } - + context "when multiple branches are enabled", :feature_multiple_branches do it "does not display a warning about routes being deleted if answer type changes" do expect(rendered).not_to have_selector(".govuk-notification-banner__content") end From d274ba25cfdba6286a4ee37f551bb930178f7a75 Mon Sep 17 00:00:00 2001 From: Thomas Iles Date: Fri, 26 Jun 2026 16:35:12 +0100 Subject: [PATCH 4/6] Add page to DraftQuestion --- app/models/draft_question.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/draft_question.rb b/app/models/draft_question.rb index 13618c153..b555c5d52 100644 --- a/app/models/draft_question.rb +++ b/app/models/draft_question.rb @@ -1,5 +1,6 @@ class DraftQuestion < ApplicationRecord belongs_to :user + belongs_to :page, optional: true validates :form_id, presence: true From 5d6d958d7a56373e99d95e7a72b4be76177c6f90 Mon Sep 17 00:00:00 2001 From: Thomas Iles Date: Fri, 26 Jun 2026 16:33:18 +0100 Subject: [PATCH 5/6] Add #show_routing_warning? to Selection::TypeInput --- .../pages/selection/type_input.rb | 8 ++++ .../pages/selection/type_input_spec.rb | 45 +++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/app/input_objects/pages/selection/type_input.rb b/app/input_objects/pages/selection/type_input.rb index ab40c88d5..f949ff3f2 100644 --- a/app/input_objects/pages/selection/type_input.rb +++ b/app/input_objects/pages/selection/type_input.rb @@ -24,4 +24,12 @@ def need_to_reduce_options? selection_options = draft_question&.answer_settings&.[](:selection_options) selection_options.present? && selection_options.length > 30 end + + def show_routing_warning? + return true unless FeatureService.new(group: draft_question.form&.group).enabled?(:multiple_branches) + + return false if draft_question.page.nil? + + draft_question.page.answer_type == "selection" && draft_question.page.answer_settings.only_one_option == "true" + end end diff --git a/spec/input_objects/pages/selection/type_input_spec.rb b/spec/input_objects/pages/selection/type_input_spec.rb index 33707b4bc..b84aad6a5 100644 --- a/spec/input_objects/pages/selection/type_input_spec.rb +++ b/spec/input_objects/pages/selection/type_input_spec.rb @@ -111,4 +111,49 @@ end end end + + describe "show_routing_warning?" do + let(:draft_question) { build :draft_question, form_id: form.id, page: } + let(:page) { build(:page) } + + context "when multiple branches is disabled", feature_multiple_branches: false do + it "returns true" do + expect(input.show_routing_warning?).to be true + end + end + + context "when multiple branches is enabled", :feature_multiple_branches do + context "when the page is nil" do + let(:page) { nil } + + it "returns false" do + expect(input.show_routing_warning?).to be false + end + end + + context "when the page is not a selection question" do + let(:page) { build(:page, answer_type: "text") } + + it "returns false" do + expect(input.show_routing_warning?).to be false + end + end + + context "when the selection type is only one option" do + let(:page) { build(:page, :selection_with_radios) } + + it "returns false" do + expect(input.show_routing_warning?).to be true + end + end + + context "when the selection type is multiple options" do + let(:page) { build(:page, :selection_with_checkboxes) } + + it "returns true" do + expect(input.show_routing_warning?).to be false + end + end + end + end end From 688e10df2a8d98cd17248ac9a1e918a02e5d8e0a Mon Sep 17 00:00:00 2001 From: Thomas Iles Date: Fri, 26 Jun 2026 16:34:48 +0100 Subject: [PATCH 6/6] Use #show_routing_warning? in type_input view --- app/views/pages/selection/type.html.erb | 2 +- .../views/pages/selection/type.html.erb_spec.rb | 17 ++++++++++++++--- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/app/views/pages/selection/type.html.erb b/app/views/pages/selection/type.html.erb index a13af9bde..6f42bf7e1 100644 --- a/app/views/pages/selection/type.html.erb +++ b/app/views/pages/selection/type.html.erb @@ -10,7 +10,7 @@ <% banner.with_heading(text: t("selection_type.routing_and_reduce_your_options_combined_warning.heading"), tag: "h3") %>

<%= t("selection_type.routing_and_reduce_your_options_combined_warning.body", pages_link_url: form_pages_path(current_form.id)) %>

<% end %> - <% else %> + <% elsif @selection_type_input.show_routing_warning? %> <%= govuk_notification_banner(title_text: t("banner.default.title")) do |banner| %> <% banner.with_heading(text: t("selection_type.routing_warning")) %> <% end %> diff --git a/spec/views/pages/selection/type.html.erb_spec.rb b/spec/views/pages/selection/type.html.erb_spec.rb index 155f4d36a..8850c1d32 100644 --- a/spec/views/pages/selection/type.html.erb_spec.rb +++ b/spec/views/pages/selection/type.html.erb_spec.rb @@ -81,15 +81,26 @@ context "when a routing condition is set" do let(:routing_conditions) { [build(:condition)] } + let(:show_routing_warning) { true } context "when the options will not need to be reduced" do before do - allow(selection_type_input).to receive(:need_to_reduce_options?).and_return false + allow(selection_type_input).to receive_messages(need_to_reduce_options?: false, show_routing_warning?: show_routing_warning) render(template: "pages/selection/type") end - it "displays a warning about routes being deleted" do - expect(rendered).to have_selector(".govuk-notification-banner__content", text: I18n.t("selection_type.routing_warning")) + context "when show_routing_warning returns true" do + it "displays a warning about routes being deleted" do + expect(rendered).to have_selector(".govuk-notification-banner__content", text: I18n.t("selection_type.routing_warning")) + end + end + + context "when show_routing_warning returns false" do + let(:show_routing_warning) { false } + + it "does not display a warning about routes being deleted" do + expect(rendered).not_to have_selector(".govuk-notification-banner__content") + end end end