fix some n+1's on user dashboard

don't re-fetch dashboard positions and preload roles

test plan:
* dashboard should load

closes #LA-911

Change-Id: I324b9a31ecb90660348320eb72163ec7358796fa
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/233661
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>

app/models/course.rb

@@ -30,7 +30,7 @@ class Course < ActiveRecord::Base
   include Courses::ItemVisibilityHelper
   include OutcomeImportContext
 
-  attr_accessor :teacher_names, :master_course
+  attr_accessor :teacher_names, :master_course, :primary_enrollment_role
   attr_writer :student_count, :teacher_count, :primary_enrollment_type, :primary_enrollment_role_id, :primary_enrollment_rank, :primary_enrollment_state, :primary_enrollment_date, :invitation, :master_migration
 
   time_zone_attribute :time_zone

app/models/user.rb

@@ -1479,10 +1479,11 @@ class User < ActiveRecord::Base
   end
 
   def dashboard_positions
-    get_preference(:dashboard_positions) || {}
+    @dashboard_positions ||= get_preference(:dashboard_positions) || {}
   end
 
   def set_dashboard_positions(new_positions)
+    @dashboard_positions = nil
     set_preference(:dashboard_positions, new_positions)
   end
 
@@ -1743,6 +1744,10 @@ class User < ActiveRecord::Base
         end
       end
 
+      Shard.partition_by_shard(res, ->(c){ c.shard }) do |shard_courses|
+        roles = Role.where(:id => shard_courses.map(&:primary_enrollment_role_id).uniq).to_a.index_by(&:id)
+        shard_courses.each{|c| c.primary_enrollment_role = roles[c.primary_enrollment_role_id]}
+      end
       @courses_with_primary_enrollment[cache_key] =
         res.sort_by{ |c| [c.primary_enrollment_rank, Canvas::ICU.collation_key(c.name)] }
     end

app/presenters/course_for_menu_presenter.rb

@@ -72,10 +72,6 @@ class CourseForMenuPresenter
   end
 
   private
-  def role
-    Role.get_role_by_id(Shard.relative_id_for(course.primary_enrollment_role_id, course.shard, Shard.current)) ||
-      Enrollment.get_built_in_role_for_type(course.primary_enrollment_type)
-  end
 
   def subtitle
     label = if course.primary_enrollment_state == 'invited'
@@ -83,7 +79,7 @@ class CourseForMenuPresenter
     else
       before_label('#shared.menu_enrollment.labels.enrolled_as', 'enrolled as')
     end
-    [ label, role.try(:label) ].join(' ')
+    [ label, course.primary_enrollment_role.try(:label) ].join(' ')
   end
 
   def term

spec/models/user_spec.rb

@@ -769,6 +769,19 @@ describe User do
         @user.favorites.create!(:context => @course)
         expect(@user.courses_with_primary_enrollment(:favorite_courses)).to eq [@course]
       end
+
+      it "loads the roles correctly" do
+        @user = User.create!(:name => 'user')
+        @shard1.activate do
+          account = Account.create!
+          @course = account.courses.create!(:workflow_state => 'available')
+          @role = account.roles.create!(:name => "custom student", :base_role_type => "StudentEnrollment")
+          StudentEnrollment.create!(:course => @course, :user => @user, :workflow_state => 'active', :role => @role)
+        end
+        fetched_courses = @user.courses_with_primary_enrollment(:current_and_invited_courses, nil, :include_completed_courses => true)
+        expect(fetched_courses.count).to eq 1
+        expect(fetched_courses.first.primary_enrollment_role).to eq @role
+      end
     end
   end
 

spec/presenters/course_for_menu_presenter_spec.rb

@@ -123,21 +123,4 @@ describe CourseForMenuPresenter do
     end
 
   end
-
-  describe '#role' do
-    specs_require_sharding
-    it "should retrieve the correct role for cross-shard enrollments" do
-      @shard1.activate do
-        account = Account.create
-        @role = account.roles.create :name => "1337 Student"
-        @role.base_role_type = 'StudentEnrollment'
-        @role.save!
-
-        @cs_course = account.courses.create!
-        @cs_course.primary_enrollment_role_id = @role.local_id
-      end
-      cs_presenter = CourseForMenuPresenter.new(@cs_course)
-      expect(cs_presenter.send(:role)).to eq @role
-    end
-  end
 end