optimize uncached loading of courses menu

closes CNVS-7294

test plan:
 * go to your dashboard, it should show your courses in the menu
 * reload, should be the same
 * clear the cache, and retry, should be the same
 * repeat with a user enrolled in courses on multiple shards

Change-Id: I8e1450abb289e192642b3197781a208ab5a501ec
Reviewed-on: https://gerrit.instructure.com/22938
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Jacob Fugal <jacob@instructure.com>
QA-Review: Clare Strong <clare@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>

app/models/enrollment.rb

@@ -246,6 +246,11 @@ class Enrollment < ActiveRecord::Base
     types_with_indefinite_article[type] || types_with_indefinite_article['StudentEnrollment']
   end
 
+  def reload(options = nil)
+    @enrollment_dates = nil
+    super
+  end
+
   def should_update_user_account_association?
     self.new_record? || self.course_id_changed? || self.course_section_id_changed? || self.root_account_id_changed?
   end
@@ -543,7 +548,8 @@ class Enrollment < ActiveRecord::Base
   end
 
   def enrollment_dates
-    Canvas::Builders::EnrollmentDateBuilder.build(self)
+    Canvas::Builders::EnrollmentDateBuilder.preload([self]) unless @enrollment_dates
+    @enrollment_dates
   end
 
   def state_based_on_date

app/models/user.rb

@@ -1554,6 +1554,10 @@ class User < ActiveRecord::Base
 
           unless options[:include_completed_courses]
             enrollments = Enrollment.where(:id => courses.map(&:primary_enrollment_id)).all
+            courses_hash = courses.index_by(&:id)
+            # prepopulate the reverse association
+            enrollments.each { |e| e.course = courses_hash[e.course_id] }
+            Canvas::Builders::EnrollmentDateBuilder.preload(enrollments)
             date_restricted_ids = enrollments.select{ |e| e.completed? || e.inactive? }.map(&:id)
             courses.reject! { |course| date_restricted_ids.include?(course.primary_enrollment_id.to_i) }
           end

lib/canvas/builders/enrollment_date_builder.rb

@@ -28,28 +28,51 @@ class EnrollmentDateBuilder
     @enrollment_dates = []
   end
 
+  def self.preload(enrollments)
+    return if enrollments.empty?
+    Enrollment.send(:preload_associations, enrollments, :course) unless enrollments.first.loaded_course?
+
+    to_preload = enrollments.reject { |e| fetch(e) }
+    return if to_preload.empty?
+    Enrollment.send(:preload_associations, to_preload, :course_section)
+    Course.send(:preload_associations, to_preload.map(&:course).uniq, :enrollment_term)
+    to_preload.each { |e| build(e) }
+  end
+
+  def self.cache_key(enrollment)
+    [enrollment, enrollment.course, 'enrollment_date_ranges'].cache_key
+  end
+
+  def self.fetch(enrollment)
+    result = Rails.cache.fetch(cache_key(enrollment))
+    enrollment.instance_variable_set(:@enrollment_dates, result)
+  end
+
   def self.build(enrollment)
     EnrollmentDateBuilder.new(enrollment).build
   end
 
-  def build
-    Rails.cache.fetch([@enrollment, @course, 'enrollment_date_ranges'].cache_key) do
-      if enrollment_is_restricted?
-        add_enrollment_dates(@enrollment)
-      elsif section_is_restricted?
-        add_enrollment_dates(@section)
-        add_term_dates if @enrollment.admin?
-      elsif course_is_restricted?
-        add_enrollment_dates(@course)
-        add_term_dates if @enrollment.admin?
-      elsif @term
-        add_term_dates
-      else
-        @enrollment_dates << default_dates
-      end
+  def cache_key
+    @cache_key ||= self.class.cache_key(@enrollment)
+  end
 
-      @enrollment_dates
+  def build
+    if enrollment_is_restricted?
+      add_enrollment_dates(@enrollment)
+    elsif section_is_restricted?
+      add_enrollment_dates(@section)
+      add_term_dates if @enrollment.admin?
+    elsif course_is_restricted?
+      add_enrollment_dates(@course)
+      add_term_dates if @enrollment.admin?
+    elsif @term
+      add_term_dates
+    else
+      @enrollment_dates << default_dates
     end
+
+    Rails.cache.write(cache_key, @enrollment_dates)
+    @enrollment.instance_variable_set(:@enrollment_dates, @enrollment_dates)
   end
 
   private

spec/integration/enrollment_date_restrictions_spec.rb

@@ -139,7 +139,7 @@ describe "enrollment_date_restrictions" do
     @course.conclude_at = 4.days.from_now
     @course.restrict_enrollments_to_course_dates = true
     @course.save!
-    @enrollment.state_based_on_date.should == :inactive
+    @enrollment.reload.state_based_on_date.should == :inactive
 
     get '/calendar'
     html = Nokogiri::HTML(response.body)

spec/lib/canvas/builders/enrollment_date_builder_spec.rb

@@ -134,4 +134,40 @@ describe Canvas::Builders::EnrollmentDateBuilder do
     end
 
   end
+
+  describe ".preload" do
+    it "should work" do
+      course_with_teacher(:active_all => true)
+      @enrollment.reload
+      @enrollment.loaded_course?.should be_false
+      Canvas::Builders::EnrollmentDateBuilder.preload([@enrollment])
+      @enrollment.loaded_course?.should be_true
+      @enrollment.loaded_course_section?.should be_true
+      @enrollment.course.loaded_enrollment_term?.should be_true
+
+      # should already be cached on the object
+      Rails.cache.expects(:fetch).never
+      @enrollment.enrollment_dates
+    end
+
+    it "should not have to load stuff if already in cache" do
+      enable_cache do
+        course_with_teacher(:active_all => true)
+        # prime the cache
+        Canvas::Builders::EnrollmentDateBuilder.preload([@enrollment])
+
+        # now reload
+        @enrollment = Enrollment.find(@enrollment)
+        Canvas::Builders::EnrollmentDateBuilder.preload([@enrollment])
+        @enrollment.loaded_course?.should be_true
+        # it shouldn't have had to load these associations
+        @enrollment.loaded_course_section?.should be_false
+        @enrollment.course.loaded_enrollment_term?.should be_false
+        # should already be cached on the object
+
+        Rails.cache.expects(:fetch).never
+        @enrollment.enrollment_dates
+      end
+    end
+  end
 end

spec/models/course_spec.rb

@@ -214,6 +214,7 @@ describe Course do
       @enrollment.start_at = 4.days.ago
       @enrollment.end_at = 2.days.ago
       @enrollment.save!
+      @enrollment.reload
       @enrollment.state_based_on_date.should == :completed
     end
 
@@ -252,7 +253,7 @@ describe Course do
       @enrollment.start_at = 4.days.ago
       @enrollment.end_at = 2.days.ago
       @enrollment.save!
-      @enrollment.state_based_on_date.should == :completed
+      @enrollment.reload.state_based_on_date.should == :completed
       @course.prior_enrollments.should == []
       @course.grants_right?(@designer, nil, :read_as_admin).should be_true
       @course.grants_right?(@designer, nil, :read_roster).should be_true
@@ -311,7 +312,7 @@ describe Course do
       course_with_teacher(:active_user => 1)
       @course.enrollment_term.update_attributes(:start_at => 2.days.from_now, :end_at => 4.days.from_now)
       @enrollment.update_attribute(:workflow_state, 'active')
-      @enrollment.state_based_on_date.should == :inactive
+      @enrollment.reload.state_based_on_date.should == :inactive
       @course.grants_right?(:read, @teacher).should be_false
     end
 
@@ -2575,7 +2576,7 @@ describe Course, "conclusions" do
     @user.reload
     @user.cached_current_enrollments(:reload)
 
-    enrollment.state.should == :active
+    enrollment.reload.state.should == :active
     enrollment.state_based_on_date.should == :completed
     enrollment.should_not be_participating_student
 

spec/models/enrollment_spec.rb

@@ -341,21 +341,21 @@ describe Enrollment do
         @enrollment.state.should eql(:invited)
         @enrollment.state_based_on_date.should eql(:invited)
         @enrollment.accept
-        @enrollment.state.should eql(:active)
+        @enrollment.reload.state.should eql(:active)
         @enrollment.state_based_on_date.should eql(:active)
 
         @enrollment.start_at = 4.days.ago
         @enrollment.end_at = 2.days.ago
         @enrollment.workflow_state = 'invited'
         @enrollment.save!
-        @enrollment.state.should eql(:invited)
+        @enrollment.reload.state.should eql(:invited)
         @enrollment.state_based_on_date.should eql(:completed)
         @enrollment.accept.should be_false
 
         @enrollment.start_at = 2.days.from_now
         @enrollment.end_at = 4.days.from_now
         @enrollment.save!
-        @enrollment.state.should eql(:invited)
+        @enrollment.reload.state.should eql(:invited)
         @enrollment.state_based_on_date.should eql(:invited)
         @enrollment.accept.should be_true
       end
@@ -372,7 +372,7 @@ describe Enrollment do
         @section.save!
         @enrollment.state.should eql(:invited)
         @enrollment.accept
-        @enrollment.state.should eql(:active)
+        @enrollment.reload.state.should eql(:active)
         @enrollment.state_based_on_date.should eql(:active)
 
         @section.start_at = 4.days.ago
@@ -380,7 +380,7 @@ describe Enrollment do
         @section.save!
         @enrollment.workflow_state = 'invited'
         @enrollment.save!
-        @enrollment.state.should eql(:invited)
+        @enrollment.reload.state.should eql(:invited)
         if should_be_invited
           @enrollment.state_based_on_date.should eql(:invited)
           @enrollment.accept.should be_true
@@ -393,6 +393,7 @@ describe Enrollment do
         @section.end_at = 4.days.from_now
         @section.save!
         @enrollment.save!
+        @enrollment.reload
         if should_be_invited
           @enrollment.state.should eql(:active)
           @enrollment.state_based_on_date.should eql(:active)
@@ -549,7 +550,7 @@ describe Enrollment do
 
           @enrollment.workflow_state = 'invited'
           @enrollment.save!
-          @enrollment.state.should == :invited
+          @enrollment.reload.state.should == :invited
           @enrollment.state_based_on_date.should == :completed
         end
       end
@@ -604,14 +605,14 @@ describe Enrollment do
         @enrollment.start_at = 4.days.ago
         @enrollment.end_at = 2.days.ago
         @enrollment.save!
-        @enrollment.state.should eql(:active)
+        @enrollment.reload.state.should eql(:active)
         @enrollment.state_based_on_date.should eql(:completed)
 
         sleep 1
         @enrollment.start_at = 2.days.from_now
         @enrollment.end_at = 4.days.from_now
         @enrollment.save!
-        @enrollment.state.should eql(:active)
+        @enrollment.reload.state.should eql(:active)
         @enrollment.state_based_on_date.should eql(:inactive)
       end
 
@@ -800,16 +801,16 @@ describe Enrollment do
       @course.conclude_at = 4.days.from_now
       @course.save!
 
-      @teacher_enrollment.state_based_on_date.should == :active
-      @student_enrollment.state_based_on_date.should == :inactive
+      @teacher_enrollment.reload.state_based_on_date.should == :active
+      @student_enrollment.reload.state_based_on_date.should == :inactive
 
       # Term dates superset of course dates, now in ending non-overlap
       @course.start_at = 4.days.ago
       @course.conclude_at = 2.days.ago
       @course.save!
 
-      @teacher_enrollment.state_based_on_date.should == :active
-      @student_enrollment.state_based_on_date.should == :completed
+      @teacher_enrollment.reload.state_based_on_date.should == :active
+      @student_enrollment.reload.state_based_on_date.should == :completed
 
       # Course dates completely before term dates, now in term dates
       @course.start_at = 6.days.ago
@@ -819,32 +820,32 @@ describe Enrollment do
       @term.end_at = 2.days.from_now
       @term.save!
 
-      @teacher_enrollment.state_based_on_date.should == :active
-      @student_enrollment.state_based_on_date.should == :completed
+      @teacher_enrollment.reload.state_based_on_date.should == :active
+      @student_enrollment.reload.state_based_on_date.should == :completed
 
       # Course dates completely after term dates, now in term dates
       @course.start_at = 4.days.from_now
       @course.conclude_at = 6.days.from_now
       @course.save!
 
-      @teacher_enrollment.state_based_on_date.should == :active
-      @student_enrollment.state_based_on_date.should == :inactive
+      @teacher_enrollment.reload.state_based_on_date.should == :active
+      @student_enrollment.reload.state_based_on_date.should == :inactive
 
       # Now between course and term dates, term first
       @term.start_at = 4.days.ago
       @term.end_at = 2.days.ago
       @term.save!
 
-      @teacher_enrollment.state_based_on_date.should == :completed
-      @student_enrollment.state_based_on_date.should == :inactive
+      @teacher_enrollment.reload.state_based_on_date.should == :completed
+      @student_enrollment.reload.state_based_on_date.should == :inactive
 
       # Now after both dates
       @course.start_at = 4.days.ago
       @course.conclude_at = 2.days.ago
       @course.save!
 
-      @teacher_enrollment.state_based_on_date.should == :completed
-      @student_enrollment.state_based_on_date.should == :completed
+      @teacher_enrollment.reload.state_based_on_date.should == :completed
+      @student_enrollment.reload.state_based_on_date.should == :completed
 
       # Now before both dates
       @course.start_at = 2.days.from_now
@@ -854,16 +855,16 @@ describe Enrollment do
       @term.end_at = 4.days.from_now
       @term.save!
 
-      @teacher_enrollment.state_based_on_date.should == :inactive
-      @student_enrollment.state_based_on_date.should == :inactive
+      @teacher_enrollment.reload.state_based_on_date.should == :inactive
+      @student_enrollment.reload.state_based_on_date.should == :inactive
 
       # Now between course and term dates, course first
       @course.start_at = 4.days.ago
       @course.conclude_at = 2.days.ago
       @course.save!
 
-      @teacher_enrollment.state_based_on_date.should == :completed
-      @student_enrollment.state_based_on_date.should == :completed
+      @teacher_enrollment.reload.state_based_on_date.should == :completed
+      @student_enrollment.reload.state_based_on_date.should == :completed
 
     end
 
@@ -881,6 +882,7 @@ describe Enrollment do
       @enrollment.start_at = 4.days.ago
       @enrollment.end_at = 2.days.ago
       @enrollment.save!
+      @enrollment.reload
       @enrollment.active?.should be_false
       @enrollment.inactive?.should be_false
       @enrollment.completed?.should be_true
@@ -889,6 +891,7 @@ describe Enrollment do
       @enrollment.start_at = 2.days.from_now
       @enrollment.end_at = 4.days.from_now
       @enrollment.save!
+      @enrollment.reload
       @enrollment.active?.should be_false
       @enrollment.inactive?.should be_true
       @enrollment.completed?.should be_false
@@ -937,6 +940,7 @@ describe Enrollment do
       @enrollment.end_at = 2.days.ago
       @enrollment.completed_at = nil
       @enrollment.save!
+      @enrollment.reload
 
       @enrollment.completed_at.should == @enrollment.end_at
       @enrollment.completed_at = yesterday