Fix and Refactor Querying of template.phases and Associations Within OrgAdmin TemplatesController

CHANGELOG.md

@@ -1,5 +1,6 @@
 # Changelog
 
+- Fix and Refactor Querying of `template.phases` and Associations Within `OrgAdmin TemplatesController` [#3472](https://github.com/DMPRoadmap/roadmap/pull/3472)
 - Refactor Plan.deep_copy(plan) [#3469](https://github.com/DMPRoadmap/roadmap/pull/3469)
 - Fixed a bug in the deep copy of plans where the old identifier was being copied into the new plan. We now copy the generated id of the new plan to the identifier field.
 - Fixed bar chart click function in the Usage dashboard (GitHub issue #3443)

app/controllers/org_admin/templates_controller.rb

@@ -2,7 +2,6 @@
 
 module OrgAdmin
   # Controller that handles templates
-  # rubocop:disable Metrics/ClassLength
   class TemplatesController < ApplicationController
     include Paginable
     include Versionable
@@ -105,12 +104,7 @@ def show
       template = Template.find(params[:id])
       authorize template
       # Load the info needed for the overview section if the authorization check passes!
-      phases = template.phases
-                       .includes(sections: { questions: :question_options })
-                       .order('phases.number', 'sections.number', 'questions.number',
-                              'question_options.number')
-                       .select('phases.title', 'phases.description', 'phases.modifiable',
-                               'sections.title', 'questions.text', 'question_options.text')
+      phases = fetch_template_phases(template)
       unless template.latest?
         # rubocop:disable Layout/LineLength
         flash[:notice] = _('You are viewing a historical version of this template. You will not be able to make changes.')
@@ -125,22 +119,11 @@ def show
     end
 
     # GET /org_admin/templates/:id/edit
-    # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
     def edit
       template = Template.includes(:org, :phases).find(params[:id])
       authorize template
       # Load the info needed for the overview section if the authorization check passes!
-      phases = template.phases.includes(sections: { questions: :question_options })
-                       .order('phases.number',
-                              'sections.number',
-                              'questions.number',
-                              'question_options.number')
-                       .select('phases.title',
-                               'phases.description',
-                               'phases.modifiable',
-                               'sections.title',
-                               'questions.text',
-                               'question_options.text')
+      phases = fetch_template_phases(template)
       if template.latest?
         render 'container', locals: {
           partial_path: 'edit',
@@ -152,7 +135,6 @@ def edit
         redirect_to org_admin_template_path(id: template.id)
       end
     end
-    # rubocop:enable Metrics/AbcSize, Metrics/MethodLength
 
     # GET /org_admin/templates/new
     def new
@@ -365,6 +347,12 @@ def template_export
 
     private
 
+    def fetch_template_phases(template)
+      template.phases
+              .includes(sections: { questions: :question_options })
+              .select(:id, :title, :description, :modifiable)
+    end
+
     def template_params
       # TODO: For some reason the sample plans and funder links are sent outside
       #       the context of the form as :template-links like this:
@@ -415,5 +403,4 @@ def get_referrer(template, referrer)
       end
     end
   end
-  # rubocop:enable Metrics/ClassLength
 end

app/views/org_admin/phases/_phase.html.erb

@@ -53,9 +53,9 @@
                       <% section.questions.each do |question| %>
                         <li>
                           <%= sanitize question.text %>
-                          <% if question.question_options.length > 0 %>
+                          <% if question.question_options.any? %>
                             <ul>
-                              <% question.question_options.each do |option| %>
+                              <% question.question_options.by_number.each do |option| %>
                                 <li><%= option.text %></li>
                               <% end %>
                             </ul>