fix mgp related N+1s in courses API

This patchset cleans up a few N+1s related to grading periods and
scores when the courses API is called for a student with
include=current_grading_period_scores and include=total_scores

fixes GRADE-300

test plan:
 - Have a few courses in an enrollment term with multiple grading
   periods set up.
 - Have a few courses in an enrollment term without MGP set up.
 - Register a student in those courses
 - As the student hit the courses API endpoint as:
   /api/v1/courses?include%5B%5D=needs_grading_count&include%5B%5D=syllabus_body&include%5B%5D=total_scores&include%5B%5D=term&include%5B%5D=permissions&include%5B%5D=current_grading_period_scores&include%5B%5D=favorites&include%5B%5D=tabs&per_page=99
 - Observe in the logs that grading period information and score
   information is only loaded once and not once per course

Change-Id: I5c2caf23bb7f51274a0a482ac8b4f3135e887cd6
Reviewed-on: https://gerrit.instructure.com/127447
Reviewed-by: Shahbaz Javeed <[email protected]>
Tested-by: Jenkins
Reviewed-by: Jeremy Neander <[email protected]>
QA-Review: Derek Bender <[email protected]>
Product-Review: Keith T. Garner <[email protected]>

app/controllers/courses_controller.rb

@@ -2859,11 +2859,19 @@ def courses_for_user(user, paginate_url: api_v1_courses_url)
     end
     enrollments_by_course = Api.paginate(enrollments_by_course, self, paginate_url) if api_request?
     courses = enrollments_by_course.map(&:first).map(&:course)
-    preloads = [:account, :root_account]
+    preloads = %i/account root_account/
     preloads << :teachers if includes.include?('teachers')
     preloads << :grading_standard if includes.include?('total_scores')
+    if includes.include?('current_grading_period_scores')
+      preloads << { enrollment_term: { grading_period_group: :grading_periods } }
+      preloads << { grading_period_groups: :grading_periods }
+    end
     ActiveRecord::Associations::Preloader.new.preload(courses, preloads)
 
+    if includes.include?('total_scores') || includes.include?('current_grading_period_scores')
+      ActiveRecord::Associations::Preloader.new.preload(enrollments, scores: :course)
+    end
+
     enrollments_by_course.each do |course_enrollments|
       course = course_enrollments.first.course
       hash << course_json(course, user, session, includes, course_enrollments)

app/models/course.rb

@@ -3042,36 +3042,38 @@ def any_assignment_in_closed_grading_period?
     effective_due_dates.any_in_closed_grading_period?
   end
 
+  def relevant_grading_period_group
+    return @relevant_grading_period_group if defined?(@relevant_grading_period_group)
+
+    @relevant_grading_period_group = grading_period_groups.detect { |gpg| gpg.workflow_state == 'active' }
+    return @relevant_grading_period_group unless @relevant_grading_period_group.nil?
+
+    if enrollment_term.grading_period_group&.workflow_state == 'active'
+      @relevant_grading_period_group = enrollment_term.grading_period_group
+    end
+  end
+
   # Does this course have grading periods?
   # checks for both legacy and account-level grading period groups
   def grading_periods?
     return @has_grading_periods unless @has_grading_periods.nil?
     return @has_grading_periods = true if @has_weighted_grading_periods
 
-    @has_grading_periods = shard.activate do
-      GradingPeriodGroup.active.
-        where("id = ? or course_id = ?", enrollment_term.grading_period_group_id, id).
-        exists?
-    end
+    @has_grading_periods = relevant_grading_period_group.present?
   end
 
   def display_totals_for_all_grading_periods?
     return @display_totals_for_all_grading_periods if defined?(@display_totals_for_all_grading_periods)
 
-    @display_totals_for_all_grading_periods =
-      !!GradingPeriodGroup.for_course(self)&.display_totals_for_all_grading_periods?
+    @display_totals_for_all_grading_periods = !!relevant_grading_period_group&.display_totals_for_all_grading_periods?
   end
 
   def weighted_grading_periods?
     return @has_weighted_grading_periods unless @has_weighted_grading_periods.nil?
     return @has_weighted_grading_periods = false if @has_grading_periods == false
 
-    @has_weighted_grading_periods = shard.activate do
-      grading_period_groups.active.none? &&
-      GradingPeriodGroup.active.
-        where(id: enrollment_term.grading_period_group_id, weighted: true).
-        exists?
-    end
+    @has_weighted_grading_periods = grading_period_groups.to_a.none? { |gpg| gpg.workflow_state == 'active' } &&
+      !!relevant_grading_period_group&.weighted?
   end
 
   def quiz_lti_tool

lib/api/v1/course_json.rb

@@ -184,7 +184,7 @@ def grading_period_score_or_grade(enrollment, current_or_final, score_or_grade)
 
     def current_grading_period
       return @current_grading_period if defined?(@current_grading_period)
-      @current_grading_period = GradingPeriod.current_period_for(@course)
+      @current_grading_period = @course.relevant_grading_period_group&.grading_periods&.detect(&:current?)
     end
 
     def include_current_grading_period_scores?

spec/models/course_spec.rb

@@ -264,6 +264,34 @@
     end
   end
 
+  describe "#relevant_grading_period_group" do
+    it "favors legacy over enrollment term grading_period_groups" do
+      @course.save!
+      account_group = Factories::GradingPeriodGroupHelper.new.create_for_account(@course.root_account)
+      account_group.enrollment_terms << @course.enrollment_term
+      grading_period_group = Factories::GradingPeriodGroupHelper.new.legacy_create_for_course(@course)
+      expect(@course.relevant_grading_period_group).to eq(grading_period_group)
+    end
+
+    it "returns a legacy grading_period_group" do
+      @course.save!
+      grading_period_group = Factories::GradingPeriodGroupHelper.new.legacy_create_for_course(@course)
+      expect(@course.relevant_grading_period_group).to eq(grading_period_group)
+    end
+
+    it "returns an enrollment term grading_period_group" do
+      @course.save!
+      grading_period_group = Factories::GradingPeriodGroupHelper.new.create_for_account(@course.root_account)
+      grading_period_group.enrollment_terms << @course.enrollment_term
+      expect(@course.relevant_grading_period_group).to eq(grading_period_group)
+    end
+
+    it "returns nil when there are no relevant grading_period_group" do
+      @course.save!
+      expect(@course.relevant_grading_period_group).to be nil
+    end
+  end
+
   describe "#weighted_grading_periods?" do
     it "returns false if course has legacy grading periods" do
       @course.save!