Skip to content

Commit fcc4341

Browse files
committed
Fix routes showing by goto page order
We were grouping conditions by goto_page_id, and sorting them by this as well. This meant that when page ids were not in numerical order, they would be sorted out of sequence relating to their position. This change ensures that page positions are used when sorting the final condition groups.
1 parent 203da13 commit fcc4341

3 files changed

Lines changed: 50 additions & 21 deletions

File tree

app/components/page_list_component/view.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ def answer_value_groups(page)
106106
page.routing_conditions.to_a
107107
.in_order_of(:answer_value, answer_order, filter: false)
108108
.group_by(&:goto_page_id)
109-
.sort_by { |goto_page_id, _| goto_page_id || Float::INFINITY }
109+
.sort_by { |goto_page_id, _| goto_page_id ? @pages.ids.index(goto_page_id) : Float::INFINITY }
110110
end
111111
end
112112
end

spec/components/page_list_component/page_list_component_preview.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,8 @@ def with_multiple_branches
9999
(build :page, id: 5, position: 5, question_text: "What is your favourite colour?", routing_conditions: []),
100100
]
101101

102+
pages.define_singleton_method(:ids) { map(&:id) }
103+
102104
form = build(:form, :with_group, id: 1, pages:)
103105
form.group.multiple_branches_enabled = true
104106

spec/components/page_list_component/view_spec.rb

Lines changed: 47 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -520,20 +520,19 @@
520520
end
521521

522522
describe "#answer_value_groups" do
523-
let(:form) { build_stubbed :form, :ready_for_routing }
523+
let(:form) { create :form }
524524

525-
let(:pages) do
525+
let!(:pages) do
526526
[
527-
build_stubbed(
527+
create(
528528
:page,
529529
:with_selection_settings,
530-
id: 101,
530+
form:,
531531
selection_options:,
532-
routing_conditions: conditions,
533532
),
534-
build_stubbed(:page, id: 102),
535-
build_stubbed(:page, id: 103),
536-
build_stubbed(:page, id: 104),
533+
create(:page, form:),
534+
create(:page, form:),
535+
create(:page, form:),
537536
]
538537
end
539538

@@ -547,20 +546,26 @@
547546

548547
let(:conditions) do
549548
[
550-
build_stubbed(:condition, routing_page_id: 101, answer_value: "Option 1", goto_page_id: 103),
551-
build_stubbed(:condition, routing_page_id: 101, answer_value: "Option 2", goto_page_id: 103),
552-
build_stubbed(:condition, routing_page_id: 101, answer_value: "Option 3", goto_page_id: 104),
553-
build_stubbed(:condition, routing_page_id: 101, answer_value: nil, goto_page_id: nil, skip_to_end: true),
549+
create(:condition, routing_page_id: pages[0].id, answer_value: "Option 1", goto_page_id: pages[2].id),
550+
create(:condition, routing_page_id: pages[0].id, answer_value: "Option 2", goto_page_id: pages[2].id),
551+
create(:condition, routing_page_id: pages[0].id, answer_value: "Option 3", goto_page_id: pages[3].id),
552+
create(:condition, routing_page_id: pages[0].id, answer_value: nil, goto_page_id: nil, skip_to_end: true),
554553
]
555554
end
556555

556+
let(:page_list_component) { described_class.new(pages: form.reload.pages, form:) }
557+
558+
before do
559+
conditions
560+
end
561+
557562
it "groups conditions by goto_page_id" do
558563
expected = [
559-
[103, [conditions.first, conditions.second]],
560-
[104, [conditions.third]],
561-
[nil, [conditions.fourth]],
564+
[pages[2].id, [conditions[0], conditions[1]]],
565+
[pages[3].id, [conditions[2]]],
566+
[nil, [conditions[3]]],
562567
]
563-
result = page_list_component.answer_value_groups(pages.first)
568+
result = page_list_component.answer_value_groups(form.pages.first)
564569
expect(result).to eq expected
565570
end
566571

@@ -575,12 +580,34 @@
575580

576581
it "returns the conditions in the same order" do
577582
expected = [
578-
[103, [conditions.second, conditions.first]],
579-
[104, [conditions.third]],
580-
[nil, [conditions.fourth]],
583+
[pages[2].id, [conditions[1], conditions[0]]],
584+
[pages[3].id, [conditions[2]]],
585+
[nil, [conditions[3]]],
586+
]
587+
588+
result = page_list_component.answer_value_groups(form.pages.first)
589+
expect(result).to eq expected
590+
end
591+
end
592+
593+
context "when page positions are not in ID order" do
594+
let!(:pages) do
595+
[
596+
create(:page, :with_selection_settings, form:, selection_options:, position: 1),
597+
create(:page, form:, position: 4),
598+
create(:page, form:, position: 2),
599+
create(:page, form:, position: 3),
600+
]
601+
end
602+
603+
it "sorts groups by page position, not page id" do
604+
expected = [
605+
[pages[2].id, [conditions[0], conditions[1]]],
606+
[pages[3].id, [conditions[2]]],
607+
[nil, [conditions[3]]],
581608
]
582609

583-
result = page_list_component.answer_value_groups(pages.first)
610+
result = page_list_component.answer_value_groups(form.pages.first)
584611
expect(result).to eq expected
585612
end
586613
end

0 commit comments

Comments
 (0)