Faster AssignmentGroupsController#index

refs CNVS-2500, closes CNVS-6577

Test plan:
  * make sure the assignment groups index api action still works
    - be sure to include assignments
      * test with and without overrides
  * make sure gradebook2 still works (focus on anything related to
    assignment due dates)

Change-Id: I440a8fdeffdb497884356f08c19b5b7792566a3a
Reviewed-on: https://gerrit.instructure.com/22102
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
Product-Review: Cameron Matheson <cameron@instructure.com>

Author: cmatheson

app/controllers/assignment_groups_controller.rb

@@ -58,22 +58,38 @@ class AssignmentGroupsController < ApplicationController
   #
   # @argument include[] ["assignments","discussion_topic"] Associations to include with the group.
   # "discussion_topic" is only valid if "assignments" is also included
+  # @argument override_assignment_dates [Optional, Boolean]
+  #   Apply assignment overrides for each assignment, defaults to true.
   #
   # @returns [Assignment Group]
   def index
-    @groups = @context.assignment_groups.active
+    if authorized_action(@context.assignment_groups.new, @current_user, :read)
+      @groups = @context.assignment_groups.active
 
-    params[:include] = Array(params[:include])
-    if params[:include].include? 'assignments'
-      params[:include] << "discussion_topic"
-      @groups = @groups.includes(:assignments => [:rubric, :discussion_topic])
-    end
+      params[:include] = Array(params[:include])
+      if params[:include].include? 'assignments'
+        assignment_includes = [:rubric, :quiz, :external_tool_tag]
+        assignment_includes.concat(params[:include] & [:discussion_topic])
+        @groups = @groups.includes(:active_assignments => assignment_includes)
+
+        assignment_descriptions = @groups
+          .flat_map(&:active_assignments)
+          .map(&:description)
+        user_content_attachments = api_bulk_load_user_content_attachments(
+          assignment_descriptions, @context, @current_user
+        )
+
+        params[:override_assignment_dates] ||= true
+        override_dates = value_to_boolean(params[:override_assignment_dates])
+      end
 
-    if authorized_action(@context.assignment_groups.new, @current_user, :read)
       respond_to do |format|
         format.json {
           json = @groups.map { |g|
-            assignment_group_json(g, @current_user, session, params[:include])
+            g.context = @context
+            assignment_group_json(g, @current_user, session, params[:include],
+                                  override_assignment_dates: override_dates,
+                                  preloaded_user_content_attachments: user_content_attachments)
           }
           render :json => json
         }

app/controllers/gradebook2_controller.rb

@@ -10,7 +10,7 @@ def show
       per_page = Setting.get_cached('api_max_per_page', '50').to_i
       js_env  :GRADEBOOK_OPTIONS => {
         :chunk_size => Setting.get_cached('gradebook2.submissions_chunk_size', '35').to_i,
-        :assignment_groups_url => api_v1_course_assignment_groups_url(@context, :include => [:assignments]),
+        :assignment_groups_url => api_v1_course_assignment_groups_url(@context, :include => [:assignments], :override_assignment_dates => "false"),
         :sections_url => api_v1_course_sections_url(@context),
         :students_url => api_v1_course_enrollments_url(@context, :include => [:avatar_url], :type => ['StudentEnrollment', 'StudentViewEnrollment'], :per_page => per_page),
         :students_url_with_concluded_enrollments => api_v1_course_enrollments_url(@context, :include => [:avatar_url], :type => ['StudentEnrollment', 'StudentViewEnrollment'], :state => ['active', 'invited', 'completed'], :per_page => per_page),

app/models/course.rb

@@ -1734,8 +1734,9 @@ def self.copy_authorized_content(html, to_context, user)
   def turnitin_settings
     # check if somewhere up the account chain turnitin is enabled and
     # has valid settings
-    self.account.turnitin_settings
+    account.turnitin_settings
   end
+  memoize :turnitin_settings
 
   def turnitin_pledge
     self.account.closest_turnitin_pledge

lib/api.rb

@@ -255,7 +255,28 @@ def media_comment_json(media_object_or_hash)
   # a hash of allowed html attributes that represent urls, like { 'a' => ['href'], 'img' => ['src'] }
   UrlAttributes = Instructure::SanitizeField::SANITIZE[:protocols].inject({}) { |h,(k,v)| h[k] = v.keys; h }
 
-  def api_user_content(html, context = @context, user = @current_user)
+  def api_bulk_load_user_content_attachments(htmls, context = @context, user = @current_user)
+    rewriter = UserContent::HtmlRewriter.new(context, user)
+    attachment_ids = []
+    rewriter.set_handler('files') do |m|
+      attachment_ids << m.obj_id if m.obj_id
+    end
+
+    htmls.each { |html| rewriter.translate_content(html) }
+
+    if attachment_ids.blank?
+      {}
+    else
+      attachments = if context.is_a?(User) || context.nil?
+                      Attachment.where(id: attachment_ids)
+                    else
+                      context.attachments.where(id: attachment_ids)
+                    end
+      attachments.index_by(&:id)
+    end
+  end
+
+  def api_user_content(html, context = @context, user = @current_user, preloaded_attachments = {})
     return html if html.blank?
 
     # if we're a controller, use the host of the request, otherwise let HostUrl
@@ -271,11 +292,12 @@ def api_user_content(html, context = @context, user = @current_user)
     rewriter = UserContent::HtmlRewriter.new(context, user)
     rewriter.set_handler('files') do |match|
       if match.obj_id
-        if match.obj_class == Attachment && context && !context.is_a?(User)
-          obj = context.attachments.find(match.obj_id) rescue nil
-        else
-          obj = match.obj_class.find_by_id(match.obj_id)
-        end
+        obj   = preloaded_attachments[match.obj_id]
+        obj ||= if context.is_a?(User) || context.nil?
+                  Attachment.find_by_id(match.obj_id)
+                else
+                  context.attachments.find_by_id(match.obj_id)
+                end
       end
       next unless obj && rewriter.user_can_view_content?(obj)
 

lib/api/v1/assignment.rb

@@ -76,7 +76,10 @@ def assignment_json(assignment, user, session, opts = {})
 
     # use already generated hash['description'] because it is filtered by
     # Assignment#filter_attributes_for_user when the assignment is locked
-    hash['description'] = api_user_content(hash['description'], @context || assignment.context)
+    hash['description'] = api_user_content(hash['description'],
+                                           @context || assignment.context,
+                                           user,
+                                           opts[:preloaded_user_content_attachments] || {})
     hash['muted'] = assignment.muted?
     hash['html_url'] = course_assignment_url(assignment.context_id, assignment)
 

lib/api/v1/assignment_group.rb

@@ -27,16 +27,28 @@ module Api::V1::AssignmentGroup
     rules
   )
 
-  def assignment_group_json(group, user, session, includes = [])
+  def assignment_group_json(group, user, session, includes = [], opts = {})
     includes ||= []
+    opts.reverse_merge! override_assignment_dates: true
 
     hash = api_json(group, user, session,
                     :only => %w(id name position group_weight))
     hash['rules'] = group.rules_hash
 
     if includes.include?('assignments')
-      hash['assignments'] = group.assignments.active.map { |a|
-        assignment_json(a, user, session, include_discussion_topic: includes.include?('discussion_topic'))
+      include_discussion_topic = includes.include?('discussion_topic')
+      user_content_attachments   = opts[:preloaded_user_content_attachments]
+      user_content_attachments ||= api_bulk_load_user_content_attachments(
+        group.active_assignments.map(&:description),
+        group.context,
+        user
+      )
+      hash['assignments'] = group.active_assignments.map { |a|
+        a.context = group.context
+        assignment_json(a, user, session,
+          include_discussion_topic: include_discussion_topic,
+          override_dates: opts[:override_assignment_dates],
+          preloaded_user_content_attachments: user_content_attachments)
       }
     end
 

spec/apis/user_content_spec.rb

@@ -389,4 +389,27 @@ class Tester
       doc.at_css('a')['href'].should == url
     end
   end
+
+  describe ".api_bulk_load_user_content_attachments" do
+    it "returns a hash of assignment_id => assignment" do
+      course_with_teacher(:active_all => true)
+      a1, a2, a3 = attachment_model, attachment_model, attachment_model
+      html1, html2 = <<-HTML1, <<-HTML2
+        <a href="/courses/#{@course.id}/files/#{a1.id}/download">uh...</a>
+        <img src="/courses/#{@course.id}/files/#{a2.id}/download">
+      HTML1
+        <a href="/courses/#{@course.id}/files/#{a3.id}/download">Hi</a>
+      HTML2
+
+      class ApiClass
+        include Api
+      end
+
+      ApiClass.new.api_bulk_load_user_content_attachments(
+        [html1, html2],
+        @course,
+        @teacher
+      ).should == {a1.id => a1, a2.id => a2, a3.id => a3}
+    end
+  end
 end