Skip to content

Commit

Permalink
Fix and refactor ordering of template associations
Browse files Browse the repository at this point in the history
- Removed `.order()` as it was only applied to the main phases query and did not guarantee the order of associated records (e.g., `question_options`), which are loaded in separate queries.

- `default_scope { order(number: :asc) }` already exists for the `Phase`, `Section`, and `Question` models, ensuring the desired ordering by default.

- The `QuestionOption` model includes  `scope :by_number, -> { order(:number) }`, which is now explicitly applied when fetching the `question_options` association to enable proper ordering.

- `.order()` must've been auto-including `:id` in the `.select()` clause. Since it is required, we are explicitly adding it now.
  • Loading branch information
aaronskiba committed Jan 8, 2025
1 parent b18a454 commit c7921b4
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 6 deletions.
6 changes: 1 addition & 5 deletions app/controllers/org_admin/templates_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -350,11 +350,7 @@ def template_export
def fetch_template_phases(template)
template.phases
.includes(sections: { questions: :question_options })
.order('phases.number',
'sections.number',
'questions.number',
'question_options.number')
.select(:title, :description, :modifiable)
.select(:id, :title, :description, :modifiable)
end

def template_params
Expand Down
2 changes: 1 addition & 1 deletion app/views/org_admin/phases/_phase.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
<%= sanitize question.text %>
<% if question.question_options.length > 0 %>
<ul>
<% question.question_options.each do |option| %>
<% question.question_options.by_number.each do |option| %>
<li><%= option.text %></li>
<% end %>
</ul>
Expand Down

0 comments on commit c7921b4

Please sign in to comment.