optimize dashcard course images

fixes CORE-2811

test-plan:
* turn on course_card_images feature flag
* upload a large image for a course
* load the course dashboard
* check that the image being returned is a thumbnail

Change-Id: I5866a6839b1125dff944ae6249736b662a075687
Reviewed-on: https://gerrit.instructure.com/194600
Tested-by: Jenkins
Reviewed-by: James Williams <jamesw@instructure.com>
Reviewed-by: Ryan Shaw <ryan@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Nathan Mills <nathanm@instructure.com>

app/helpers/dashboard_helper.rb

@@ -151,7 +151,7 @@ module DashboardHelper
 
   def map_courses_for_menu(courses, opts={})
     mapped = courses.map do |course|
-      presenter = CourseForMenuPresenter.new(course, @current_user, @domain_root_account, session, opts)
+      presenter = CourseForMenuPresenter.new(course, @current_user, @domain_root_account, session, file_authenticator, opts)
       presenter.to_h
     end
 

app/models/course.rb

@@ -447,14 +447,16 @@ class Course < ActiveRecord::Base
 
   def image
     @image ||= if self.image_id.present?
-      self.shard.activate do
-        self.attachments.active.where(id: self.image_id).take&.public_download_url
-      end
+      image_attachment&.public_download_url
     elsif self.image_url
       self.image_url
     end
   end
 
+  def image_attachment
+    @image_attachment ||= self.shard.activate { self.attachments.active.where(id: self.image_id).take }
+  end
+
   def course_visibility_options
     options = [
         'course',

app/presenters/course_for_menu_presenter.rb

@@ -19,11 +19,12 @@ class CourseForMenuPresenter
   include I18nUtilities
   include Rails.application.routes.url_helpers
 
-  def initialize(course, user = nil, context = nil, session = nil, opts={})
+  def initialize(course, user = nil, context = nil, session = nil, file_authenticator = nil, opts={})
     @course = course
     @user = user
     @context = context
     @session = session
+    @file_authenticator = file_authenticator
     @opts = opts
   end
   attr_reader :course
@@ -40,7 +41,7 @@ class CourseForMenuPresenter
       subtitle: subtitle,
       enrollmentType: course.primary_enrollment_type,
       id: course.id,
-      image: course.feature_enabled?(:course_card_images) ? course.image : nil,
+      image: course.feature_enabled?(:course_card_images) ? thumbnail : nil,
       position: @user.dashboard_positions[course.asset_string] || nil,
     }.tap do |hash|
       if @opts[:tabs]
@@ -77,6 +78,15 @@ class CourseForMenuPresenter
     [ label, role.try(:label) ].join(' ')
   end
 
+  def thumbnail
+    attachment = course.image_attachment
+    if attachment
+      @file_authenticator.thumbnail_url(attachment)
+    else
+      course.image
+    end
+  end
+
   def term
     course.enrollment_term.name unless course.enrollment_term.default_term?
   end

lib/api/v1/course.rb

@@ -43,8 +43,7 @@ module Api::V1::Course
     settings[:home_page_announcement_limit] = course.home_page_announcement_limit
     settings[:image_url] = course.image_url
     settings[:image_id] = course.image_id
-    settings[:image] = course.image
-
+    settings[:image] = course.image_attachment ? authenticated_thumbnail_url(course.image_attachment) : course.image
     settings
   end
 

spec/helpers/dashboard_helper_spec.rb

@@ -79,6 +79,7 @@ describe DashboardHelper do
         @account = Account.default
         @domain_root_account = @account
       end
+      let(:file_authenticator){ double } # Used by the 'map_courses_for_menu method'
 
       it "returns the list of courses sorted by position" do
         course1 = @account.courses.create!

spec/models/course_spec.rb

@@ -5369,6 +5369,23 @@ describe Course, "#image" do
   end
 end
 
+describe Course, "#image_attachment" do
+  before(:once) do
+    course_with_teacher(active_all: true)
+    attachment_with_context(@course)
+  end
+
+  it "returns the attachment for a course file if image_id is set" do
+    @course.image_id = @attachment.id
+    @course.save!
+    expect(@course.image_attachment).to eq @attachment
+  end
+
+  it "returns nil if image_id and image_url are not set" do
+    expect(@course.image_attachment).to be_nil
+  end
+end
+
 describe Course, "#filter_users_by_permission" do
   it "filters out course users that don't have a permission based on their enrollment roles" do
     permission = :moderate_forum # happens to be true for ta's, but available to students

spec/presenters/course_for_menu_presenter_spec.rb

@@ -24,7 +24,7 @@ describe CourseForMenuPresenter do
   let(:dashboard_card_tabs) { UsersController::DASHBOARD_CARD_TABS }
 
   let_once(:presenter) do
-    CourseForMenuPresenter.new(course, user, nil, nil, {tabs: dashboard_card_tabs})
+    CourseForMenuPresenter.new(course, user, nil, nil, nil,{tabs: dashboard_card_tabs})
   end
 
   describe '#to_h' do
@@ -91,6 +91,35 @@ describe CourseForMenuPresenter do
       end
     end
 
+    context 'Image' do
+      before {course.account.enable_feature!(:course_card_images)}
+      let!(:attachment) {attachment_with_context(course)}
+      let(:thumbnail_url){ 'http://example.com/thumbnail'}
+      let(:file_authenticator){ double(thumbnail_url: thumbnail_url)}
+      let(:auth_presenter) do
+        CourseForMenuPresenter.new(course, user, nil, nil, file_authenticator,{tabs: dashboard_card_tabs})
+      end
+
+      it "returns the image_url when image_url is set" do
+        course.image_url = "http://example.com"
+        course.save!
+        h = auth_presenter.to_h
+        expect(h[:image]).to eq "http://example.com"
+      end
+
+      it "returns the download_url for a course file if image_id is set" do
+        course.image_id = attachment.id
+        course.save!
+        h = auth_presenter.to_h
+        expect(h[:image]).to eq thumbnail_url
+      end
+
+      it "returns nil if image_id and image_url are not set" do
+        h = auth_presenter.to_h
+        expect(h[:image]).to be_nil
+      end
+    end
+
   end
 
   describe '#role' do

spec/selenium/dashboard/planner/student_planner_spec.rb

@@ -79,8 +79,7 @@ describe "student planner" do
     url = driver.current_url
     # validate the background image url
     expect(elem[:style]).
-      to include("#{url}courses/#{@course.id}/files/#{@course_attachment.id}/download?verifier=#{@course_attachment.uuid}")
-
+      to include("#{url}images/thumbnails/show")
   end
 
   context "responsive layout" do