Skip to content

Commit aa302f8

Browse files
authored
Merge pull request #2765 from govuk-forms/remove-n-queries-from-routes
Remove n+1 queries from multiple routes
2 parents 5c5a503 + f1e1dca commit aa302f8

4 files changed

Lines changed: 30 additions & 3 deletions

File tree

app/models/form.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,14 @@ def set_task_status_service(service)
224224
self.task_status_service = service
225225
end
226226

227+
# Return the next page or nil if there is no next page
228+
# Use this when all pages are loaded to avoid N+1 queries,
229+
# prefer Page.next_page for individual queries.
230+
def next_page_after(current_page)
231+
pair = pages.each_cons(2).find { |p, _next_p| p == current_page }
232+
pair&.last
233+
end
234+
227235
private
228236

229237
def set_external_id

app/services/routes/build_service.rb

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,16 @@ def build_routes
2626
end
2727

2828
def options_for_page(page)
29-
return [] unless page.has_next_page?
29+
next_page = form.next_page_after(page)
30+
31+
return [] unless next_page
3032

3133
# Don't include the current page in the options
3234
options_without_page = all_goto_options.reject { |_, value| value == page.id }
3335

3436
# Replace the next page with the default option
3537
options_without_page.map do |name, value|
36-
if value == page.next_page
38+
if value == next_page.id
3739
["Go to question #{page.position.next}", Forms::RouteInput::DEFAULT_VALUE]
3840
else
3941
[name, value]

app/views/routes/show.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
row.with_value do
2323
capture do %>
2424
<p> <%= page.question_text %> </p>
25-
<% if page.has_next_page? %>
25+
<% if @routes_input.form.next_page_after(page).present? %>
2626
<ul class="govuk-list">
2727
<% routes.each do |route| %>
2828
<li>

spec/models/form_spec.rb

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1464,4 +1464,21 @@
14641464
end
14651465
end
14661466
end
1467+
1468+
describe "#next_page_after" do
1469+
let(:pages) { [] }
1470+
let(:form) { build :form, pages: }
1471+
1472+
it "returns nil if there is no next page" do
1473+
expect(form.next_page_after(form.pages.first)).to be_nil
1474+
end
1475+
1476+
context "when there is a next page" do
1477+
let(:pages) { build_list :page, 5 }
1478+
1479+
it "returns the next page" do
1480+
expect(form.next_page_after(form.pages.second)).to eq(form.pages.third)
1481+
end
1482+
end
1483+
end
14671484
end

0 commit comments

Comments
 (0)