Include pdfjs in preferred_plugins array
Instead of sending a separate param to canvadocs for pdfjs as a preferred plugin, just put it in the preferred_plugins array (which used to be called preferred_renders) Refs CNVS-30582 Test Plan: - This is very difficult to test manually, particularly without changing any code. The reason for this is that canvadocs doesn't currently do anything with the pdfjs preferred plugin so you can't just check for that behavior. However, this is something you can do even tho it isn't ideal (and this is how I tested this patch in development): - get canvas working with a canvadocs instance on which you can modify code. By modifying the canvadocs instead of the canvas, the integrity of this patch is maintained for test purposes. - In the canvadocs session controller (app/controllers/session_controller.js), add a log statement to display what comes in in the body of the request. Something like: - console.log('Request body: ') - console.dir(req.body) - Submit an assignment to a course as a student. - As a teacher, open speedgrader from that assignment. This will cause a new canvadocs viewing session to be created. - In the console output of the canvadocs instance, see that the body did not include a preferred_plugin of pdfjs. - Toggle the feature flag to enable new annotations. - Repeat opening speedgrader but this time observe that the body includes pdfjs in the list of preferred_plugins. Change-Id: Ie3e16e1331fcdd757e96888f033e8341e7d6a0bc Reviewed-on: https://gerrit.instructure.com/86549 Tested-by: Jenkins QA-Review: Caleb Guanzon <cguanzon@instructure.com> Reviewed-by: Brad Horrocks <bhorrocks@instructure.com> Product-Review: Benjamin Porter <bporter@instructure.com>
This commit is contained in:
parent
3d5e6e4cfc
commit
5a975d407f
|
@ -36,8 +36,17 @@ class CanvadocSessionsController < ApplicationController
|
|||
if @domain_root_account.settings[:canvadocs_prefer_office_online]
|
||||
opts[:preferred_plugins].unshift Canvadocs::RENDER_O365
|
||||
end
|
||||
|
||||
# We can't set the pdfjs preference here during upload because with
|
||||
# only an attachment object we lack the requisite context
|
||||
attachment.submit_to_canvadocs(1, opts) unless attachment.canvadoc_available?
|
||||
url = attachment.canvadoc.session_url(user: @current_user)
|
||||
|
||||
if pdfjs_feature_flag_enabled?(attachment.try(:canvadoc))
|
||||
# Office 365 should take priority over pdfjs
|
||||
index = opts[:preferred_plugins].first == Canvadocs::RENDER_O365 ? 1 : 0
|
||||
opts[:preferred_plugins].insert(index, Canvadocs::RENDER_PDFJS)
|
||||
end
|
||||
url = attachment.canvadoc.session_url(opts.merge(user: @current_user))
|
||||
|
||||
# For the purposes of reporting student viewership, we only
|
||||
# care if the original attachment owner is looking
|
||||
|
@ -54,4 +63,13 @@ class CanvadocSessionsController < ApplicationController
|
|||
render :text => "Service is currently unavailable. Try again later.",
|
||||
:status => :service_unavailable
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def pdfjs_feature_flag_enabled?(canvadoc)
|
||||
course = Course.find(canvadoc.preferred_plugin_course_id)
|
||||
return course.feature_enabled?(:new_annotations)
|
||||
rescue NoMethodError, ActiveRecord::RecordNotFound => e
|
||||
false
|
||||
end
|
||||
end
|
||||
|
|
|
@ -29,7 +29,6 @@ class Canvadoc < ActiveRecord::Base
|
|||
url = attachment.authenticated_s3_url(:expires => 1.day)
|
||||
|
||||
opts.delete(:annotatable) unless Canvadocs.annotations_supported?
|
||||
opts[:preferred_plugin] = preferred_plugin
|
||||
|
||||
response = Canvas.timeout_protection("canvadocs") {
|
||||
canvadocs_api.upload(url, opts)
|
||||
|
@ -50,7 +49,6 @@ class Canvadoc < ActiveRecord::Base
|
|||
def session_url(opts = {})
|
||||
user = opts.delete(:user)
|
||||
opts.merge! annotation_opts(user)
|
||||
opts[:preferred_plugin] = preferred_plugin
|
||||
|
||||
Canvas.timeout_protection("canvadocs", raise_on_timeout: true) do
|
||||
session = canvadocs_api.session(document_id, opts)
|
||||
|
@ -58,16 +56,6 @@ class Canvadoc < ActiveRecord::Base
|
|||
end
|
||||
end
|
||||
|
||||
def preferred_plugin
|
||||
begin
|
||||
course = Course.find(self.preferred_plugin_course_id)
|
||||
return course.feature_enabled?(:new_annotations) ? 'pdfjs' : nil
|
||||
rescue ActiveRecord::RecordNotFound => e
|
||||
end
|
||||
nil
|
||||
end
|
||||
private :preferred_plugin
|
||||
|
||||
def annotation_opts(user)
|
||||
return {} if !user || !has_annotations?
|
||||
|
||||
|
|
|
@ -4,9 +4,10 @@ require 'net/https'
|
|||
require 'json'
|
||||
|
||||
module Canvadocs
|
||||
RENDER_O365 = 'office_365'
|
||||
RENDER_BOX = 'box_view'
|
||||
RENDER_O365 = 'office_365'
|
||||
RENDER_BOX = 'box_view'
|
||||
RENDER_CROCODOC = 'crocodoc'
|
||||
RENDER_PDFJS = 'pdfjs'
|
||||
|
||||
# Public: A small ruby client that wraps the Box View api.
|
||||
#
|
||||
|
|
|
@ -87,42 +87,4 @@ describe 'Canvadoc' do
|
|||
expect(@doc).not_to be_available
|
||||
end
|
||||
end
|
||||
|
||||
describe '#preferred_plugin' do
|
||||
let(:course) { Course.create! }
|
||||
|
||||
let(:set_preferred_plugin_course_id) do
|
||||
->(course_id = course.id) do
|
||||
@doc.preferred_plugin_course_id = course_id
|
||||
@doc.save!
|
||||
end
|
||||
end
|
||||
|
||||
let(:feature_name) { 'new_annotations' }
|
||||
|
||||
let(:set_feature_flag) do
|
||||
->(enabled) do
|
||||
course.account.set_feature_flag!(feature_name, enabled ? 'on' : 'off')
|
||||
course.set_feature_flag!(feature_name, enabled ? 'on' : 'off')
|
||||
end
|
||||
end
|
||||
|
||||
it 'has a preferred plugin of nil when new annotations are disabled' do
|
||||
set_preferred_plugin_course_id.call
|
||||
set_feature_flag.call(false)
|
||||
expect(@doc.send(:preferred_plugin)).to be_nil
|
||||
end
|
||||
|
||||
it 'has a preferred plugin of nil when preferred_plugin_course_id is nil' do
|
||||
set_preferred_plugin_course_id.call(nil)
|
||||
set_feature_flag.call(true)
|
||||
expect(@doc.send(:preferred_plugin)).to be_nil
|
||||
end
|
||||
|
||||
it 'has a preferred plugin of "pdfjs" when new annotations are enabled' do
|
||||
set_preferred_plugin_course_id.call
|
||||
set_feature_flag.call(true)
|
||||
expect(@doc.send(:preferred_plugin)).to eq('pdfjs')
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue