From d9c6fa0fb1f312080f7ef12cc36ebda1ea70fdec Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Tue, 28 Apr 2026 09:21:46 +0300 Subject: [PATCH 1/2] Refactor specs for Step#next_step_slug_after_routing Change `id` and the `next_step_id` for the step being tested so that they're different from the step IDs for steps we're routing to; this makes it harder to write tests that pass with a false positive (for instance, if you expect `second_step_id`, before that could have been because the routing was returning the default route, instead of routing to the second step). --- spec/models/step_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/models/step_spec.rb b/spec/models/step_spec.rb index bbad0f24b..74b0629f8 100644 --- a/spec/models/step_spec.rb +++ b/spec/models/step_spec.rb @@ -127,11 +127,11 @@ end describe "#next_step_slug_after_routing" do - let(:default_next_step_id) { second_step_id } + let(:default_next_step_id) { "11111111" } # dummy value to make the default next step ID obvious when it appears let(:selection) { "Yes" } let(:question) { instance_double(Question::Selection, selection:) } let(:routing_conditions) { [] } - let(:form_document_step) { build(:v2_question_step, id: first_step_id, position: 1, next_step_id: default_next_step_id, routing_conditions:) } + let(:form_document_step) { build(:v2_question_step, id: "00000000", position: 1, next_step_id: default_next_step_id, routing_conditions:) } describe "basic routing" do context "without any routing conditions" do From bece5322c26a95fc5c4ab48fa41f37f8679044de Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Tue, 28 Apr 2026 10:32:57 +0300 Subject: [PATCH 2/2] Allow routing to work with multiple conditions Update the logic in Step#next_step_slug_after_routing so that it will match a routing condition even if it isn't the first condition. This should allow forms-runner to support forms with multiple branches. Note that this still won't match a "default" condition (answer_value of `nil`) that isn't the first in the list of routing conditions; our plans for routing don't currently have a use case for a default condition on a selection question, so there shouldn't be a situation where there are multiple conditions and one of them is a default condition. --- app/models/step.rb | 24 ++++++++++------- spec/models/step_spec.rb | 56 ++++++++++++++++++++++++---------------- 2 files changed, 49 insertions(+), 31 deletions(-) diff --git a/app/models/step.rb b/app/models/step.rb index 5882c2238..78d55750c 100644 --- a/app/models/step.rb +++ b/app/models/step.rb @@ -86,8 +86,8 @@ def next_step_slug_after_routing return goto_condition_step_slug(routing_conditions.first) end - if first_condition_matches? - return goto_condition_step_slug(routing_conditions.first) + if (matching_condition = find_matching_condition) + return goto_condition_step_slug(matching_condition) end next_step_slug @@ -141,19 +141,25 @@ def goto_condition_step_slug(condition) end end - def first_condition_matches? + def find_matching_condition return unless question.respond_to?(:selection) - return question.selection == I18n.t("page.none_of_the_above") if routing_condition_none_of_the_above + routing_conditions.find { condition_matches? it } + end + + def condition_matches?(condition) + return question.selection == I18n.t("page.none_of_the_above") if condition.answer_value == :none_of_the_above.to_s - routing_conditions.any? && (routing_conditions.first.answer_value == question.selection) + condition.answer_value == question.selection end - def first_condition_default? - routing_conditions.any? && routing_conditions.first.answer_value.blank? + def first_condition_matches? + return unless question.respond_to?(:selection) + + routing_conditions.any? && condition_matches?(routing_conditions.first) end - def routing_condition_none_of_the_above - routing_conditions.any? && routing_conditions.first.answer_value == :none_of_the_above.to_s + def first_condition_default? + routing_conditions.any? && routing_conditions.first.answer_value.blank? end end diff --git a/spec/models/step_spec.rb b/spec/models/step_spec.rb index 74b0629f8..b3d82830e 100644 --- a/spec/models/step_spec.rb +++ b/spec/models/step_spec.rb @@ -224,46 +224,58 @@ end end - describe "with invalid states" do - context "with multiple conditions and second matches" do - let(:routing_conditions) do - [ - OpenStruct.new(answer_value: "No", goto_page_id: first_step_id), - OpenStruct.new(answer_value: "Yes", goto_page_id: second_step_id), - OpenStruct.new(answer_value: "Maybe", goto_page_id: third_step_id), - ] + context "with multiple conditions" do + let(:routing_conditions) do + [ + OpenStruct.new(answer_value: "No", goto_page_id: first_step_id), + OpenStruct.new(answer_value: "Yes", goto_page_id: second_step_id), + OpenStruct.new(answer_value: "Maybe", goto_page_id: third_step_id), + ] + end + + context "and first matches" do + let(:selection) { "No" } + + it "returns the next_step_slug" do + expect(step.next_step_slug_after_routing).to eq(first_step_id) end + end + + context "and second matches" do let(:selection) { "Yes" } it "returns the next_step_slug" do - expect(step.next_step_slug_after_routing).to eq(default_next_step_id) + expect(step.next_step_slug_after_routing).to eq(second_step_id) end end - context "with multiple conditions and second is default" do - let(:routing_conditions) do - [ - OpenStruct.new(answer_value: "No", goto_page_id: first_step_id), - OpenStruct.new(answer_value: "Yes", goto_page_id: second_step_id), - OpenStruct.new(answer_value: "", goto_page_id: third_step_id), - ] + context "and third matches" do + let(:selection) { "Maybe" } + + it "returns the next_step_slug" do + expect(step.next_step_slug_after_routing).to eq(third_step_id) end - let(:selection) { "Something else" } + end + + context "but none match" do + let(:selection) { "Something Else" } it "returns the next_step_slug" do expect(step.next_step_slug_after_routing).to eq(default_next_step_id) end end + end - context "with multiple conditions but no matches" do + describe "with invalid states" do + context "with multiple conditions and default matches" do let(:routing_conditions) do [ - OpenStruct.new(answer_value: "Yes", goto_page_id: first_step_id), - OpenStruct.new(answer_value: "No", goto_page_id: second_step_id), - OpenStruct.new(answer_value: "Maybe", goto_page_id: third_step_id), + OpenStruct.new(answer_value: "No", goto_page_id: first_step_id), + OpenStruct.new(answer_value: "Yes", goto_page_id: second_step_id), + OpenStruct.new(answer_value: nil, goto_page_id: third_step_id), ] end - let(:selection) { "Something Else" } + let(:selection) { "Something else" } it "returns the next_step_slug" do expect(step.next_step_slug_after_routing).to eq(default_next_step_id)