moar preloads

found by API specs

refs CORE-1541

Change-Id: I6a4cf04a121a5cd41673561fc57f0f55e5001440
Reviewed-on: https://gerrit.instructure.com/159903
Tested-by: Jenkins
Reviewed-by: Rob Orton <rob@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>

Gemfile.d/assets.rb

@@ -26,5 +26,5 @@ group :development, :test do
   gem 'yard', '0.9.5'
   gem 'yard-appendix', '0.1.8'
 
-  gem 'bullet', '5.7.5', require: false, github: 'flyerhzm/bullet', ref: '0e852d87bc9c461d4a9b807c12af1c0d27c1d1b6'
+  gem 'bullet', '5.7.5', require: false, github: 'flyerhzm/bullet', ref: '1677bc0ff78ed550e82a8fd5ecb97f64c7e83e36'
 end

app/controllers/calendar_events_api_controller.rb

@@ -372,6 +372,10 @@ class CalendarEventsApiController < ApplicationController
     end
 
     if @errors.empty?
+      calendar_events, assignments = events.partition { |e| e.is_a?(CalendarEvent) }
+      ActiveRecord::Associations::Preloader.new.preload(calendar_events, [:context, :parent_event])
+      ActiveRecord::Associations::Preloader.new.preload(assignments, Api::V1::Assignment::PRELOADS)
+
       json = events.map do |event|
         subs = submissions[event.id] if submissions
         sub = subs.sort_by(&:submitted_at).last if subs

app/controllers/context_module_items_api_controller.rb

@@ -550,7 +550,8 @@ class ContextModuleItemsApiController < ApplicationController
     else
       assignment_ids = response[:body]['assignments'].map {|a| a['assignment_id'].try(&:to_i) }
       # assignment occurs in delayed job, may not be fully visible to user until job completes
-      assignments = @context.assignments.published.where(id: assignment_ids)
+      assignments = @context.assignments.published.where(id: assignment_ids).
+        preload(Api::V1::Assignment::PRELOADS)
 
       Assignment.preload_context_module_tags(assignments)
 

app/controllers/submissions_api_controller.rb

@@ -235,6 +235,8 @@ class SubmissionsApiController < ApplicationController
         submissions = submissions.order(:user_id)
 
         submissions = submissions.preload(:group) if includes.include?("group")
+        submissions = submissions.preload(:quiz_submission) unless params[:exclude_response_fields]&.include?('preview_url')
+        submissions = submissions.preload(:attachment) unless params[:exclude_response_fields]&.include?('attachments')
 
         submissions = Api.paginate(submissions, self,
                                    polymorphic_url([:api_v1, @section || @context, @assignment, :submissions]))
@@ -465,6 +467,7 @@ class SubmissionsApiController < ApplicationController
       if params[:workflow_state].present?
         submissions_scope = submissions_scope.where(:workflow_state => params[:workflow_state])
       end
+      submissions_scope = submissions_scope.preload(:attachment) unless params[:exclude_response_fields]&.include?('attachments')
       submissions = submissions_scope.preload(:originality_reports, :quiz_submission).to_a
       bulk_load_attachments_and_previews(submissions)
       submissions_for_user = submissions.group_by(&:user_id)
@@ -523,6 +526,7 @@ class SubmissionsApiController < ApplicationController
       submissions = submissions.where("submitted_at>?", submitted_since_date) if submitted_since_date
       submissions = submissions.where("graded_at>?", graded_since_date) if graded_since_date
       submissions = submissions.preload(:user, :originality_reports, :quiz_submission)
+      submissions = submissions.preload(:attachment) unless params[:exclude_response_fields]&.include?('attachments')
 
       # this will speed up pagination for large collections when order_direction is asc
       if order_by == 'graded_at' && order_direction == 'asc'

app/helpers/avatar_helper.rb

@@ -94,7 +94,7 @@ module AvatarHelper
     default_avatar = User.avatar_fallback_url(
         blank_fallback ? '/images/blank.png' : User.default_avatar_fallback,
         request)
-    url = if user.account.service_enabled?(:avatars)
+    url = if (@domain_root_account || user.account).service_enabled?(:avatars)
       user.avatar_url(nil,
                       (@domain_root_account && @domain_root_account.settings[:avatars] || 'enabled'),
                       default_avatar,

app/helpers/search_helper.rb

@@ -129,7 +129,7 @@ module SearchHelper
         add_sections.call context.course.sections_visible_to(@current_user, sections)
       else
         add_courses.call @current_user.concluded_courses.shard(@current_user).to_a, :concluded
-        add_courses.call @current_user.courses.shard(@current_user).to_a, :current
+        add_courses.call @current_user.courses.preload(:enrollment_term).shard(@current_user).to_a, :current
         add_sections.call @current_user.address_book.sections
         add_groups.call @current_user.address_book.groups
       end

app/models/assignment.rb

@@ -1527,7 +1527,7 @@ class Assignment < ActiveRecord::Base
 
     ensure_grader_can_adjudicate(grader: opts[:grader], provisional: opts[:provisional]) do
       if grade_group_students
-        find_or_create_submissions(students, Submission.preload(:grading_period)) do |submission|
+        find_or_create_submissions(students, Submission.preload(:grading_period, :stream_item)) do |submission|
           submissions << save_grade_to_submission(submission, original_student, group, opts)
         end
       else

app/models/discussion_topic.rb

@@ -1269,7 +1269,7 @@ class DiscussionTopic < ActiveRecord::Base
   def visible_for?(user = nil)
     RequestCache.cache('discussion_visible_for', self, user) do
       # user is the topic's author
-      next true if user && user == self.user
+      next true if user && user.id == self.user_id
 
       next false unless (is_announcement ? context.grants_right?(user, :read_announcements) : context.grants_right?(user, :read_forum))
 

app/models/role.rb

@@ -35,10 +35,10 @@ class Role < ActiveRecord::Base
   module AssociationHelper
     # this is an override to take advantage of built-in role caching since those are by far the most common
     def role
-      self.association(:role).target ||= self.shard.activate do
+      return super if association(:role).loaded?
+      self.role = self.shard.activate do
         Role.get_role_by_id(read_attribute(:role_id)) || (self.respond_to?(:default_role) ? self.default_role : nil)
       end
-      super
     end
 
     def role=(role)

app/models/submission.rb

@@ -2399,6 +2399,7 @@ class Submission < ActiveRecord::Base
             submissions.each { |s| graded_user_ids << s.user_id }
             submission = submissions.first
           end
+          submission.user = user
 
           assessment = user_data[:rubric_assessment]
           if assessment.is_a?(Hash) && assignment.rubric_association

config/environments/bullet.rb

@@ -6,6 +6,9 @@ Bullet.counter_cache_enable = false
 
 Bullet.rails_logger = true
 Bullet.stacktrace_excludes = [
+  # chains to root_topic, but it should be cached fairly often, so we don't want
+  # to unnecessarily preload
+  ['app/models/discussion_topic.rb', 'low_level_locked_for?'],
   # impossible to know what should be preloaded ahead-of-time, and we only process
   # messages one-at-a-time anyway
   ['app/models/message.rb', 'infer_defaults'],

lib/api/v1/assignment.rb

@@ -25,6 +25,11 @@ module Api::V1::Assignment
   include SubmittablesGradingPeriodProtection
   include Api::V1::PlannerOverride
 
+  PRELOADS = [:external_tool_tag,
+              :duplicate_of,
+              :rubric,
+              :rubric_association].freeze
+
   API_ALLOWED_ASSIGNMENT_OUTPUT_FIELDS = {
     :only => %w(
       id

lib/api/v1/discussion_topics.rb

@@ -67,6 +67,7 @@ module Api::V1::DiscussionTopics
     if opts[:include_sections_user_count] && context
       opts[:context_user_count] = context.enrollments.not_fake.active_or_pending_by_date_ignoring_access.count
     end
+    ActiveRecord::Associations::Preloader.new.preload(topics, [:user, :attachment])
     topics.inject([]) do |result, topic|
       if topic.visible_for?(user)
         result << discussion_topic_api_json(topic, context || topic.context, user, session, opts, root_topics)

lib/api/v1/role.rb

@@ -31,7 +31,7 @@ module Api::V1::Role
       :permissions => {}
     }
 
-    json[:account] = account_json(role.account, current_user, session, []) if role.account
+    json[:account] = account_json(role.account, current_user, session, []) if role.account_id
 
     RoleOverride.manageable_permissions(account).keys.each do |permission|
       perm = RoleOverride.permission_for(account, permission, role, account)

spec/apis/v1/stream_items_api_spec.rb

@@ -91,7 +91,7 @@ describe UsersController, type: :request do
         @assignment.update_attribute(:due_at, 1.week.from_now)
         @assignment.update_attribute(:due_at, 2.weeks.from_now)
         # manually set the pre-datafixup state for one of them
-        val = StreamItem.where(:asset_type => "Message", :id => @user.visible_stream_item_instances.map(&:stream_item)).
+        val = StreamItem.where(:asset_type => "Message", :id => @user.visible_stream_item_instances.map(&:stream_item_id)).
           limit(1).update_all(:notification_category => nil)
       end
       json = api_call(:get, "/api/v1/users/self/activity_stream/summary.json",