Stubbing out Crocodoc filter for moderated_grading
closes CNVS-22009 closes CNVS-22012 Test Plan: * Enable "Moderated Grading" Feature * Create an assignment that takes file uploads as submissions and has Moderated Grading checked * Submit a PDF as a student * As a teacher, open the assignment in speed grader and make comments on the document preview in the left pane (not to be confused with comments on the submission) * As a TA, open the assignment in speed grader and make comments on the document preview (as above) * As the student that submitted the assignment, go to your submission and make comments on the document preview * As a teacher, you should be able to view all the comments made on the assignment * As a TA, you should see the annotations that TA has made * As a student, you should see the annotations that student has made Change-Id: I3bbbade5a8bdfa1d0a7d2d1e16e667e95d1a7a9a Reviewed-on: https://gerrit.instructure.com/61396 Reviewed-by: Jeremy Stanley <jeremy@instructure.com> Tested-by: Jenkins QA-Review: Clare Strong <clare@instructure.com> Product-Review: Jeremy Stanley <jeremy@instructure.com>
This commit is contained in:
parent
964c89b3dc
commit
9aadc4c543
|
@ -33,7 +33,8 @@ class CrocodocSessionsController < ApplicationController
|
|||
|
||||
crocodoc = attachment.crocodoc_document
|
||||
url = crocodoc.session_url(:user => @current_user,
|
||||
:annotations => annotations)
|
||||
:annotations => annotations,
|
||||
:crocodoc_ids => blob["crocodoc_ids"])
|
||||
redirect_to url
|
||||
else
|
||||
render :text => "Not found", :status => :not_found
|
||||
|
|
|
@ -440,6 +440,7 @@ class GradebooksController < ApplicationController
|
|||
'SpeedGrader is enabled only for published content.')
|
||||
return redirect_to polymorphic_url([@context, @assignment])
|
||||
end
|
||||
|
||||
grading_role = if moderated_grading_enabled_and_no_grades_published
|
||||
if @context.grants_right?(@current_user, :moderate_grades)
|
||||
:moderator
|
||||
|
@ -450,6 +451,9 @@ class GradebooksController < ApplicationController
|
|||
:grader
|
||||
end
|
||||
|
||||
# TODO: Handle for moderator when behavior implemented
|
||||
crocodoc_ids = [:provisional_grader].include?(grading_role) && [@current_user.crocodoc_id!]
|
||||
|
||||
respond_to do |format|
|
||||
format.html do
|
||||
@headers = false
|
||||
|
@ -474,7 +478,8 @@ class GradebooksController < ApplicationController
|
|||
format.json do
|
||||
render :json => @assignment.speed_grader_json(@current_user,
|
||||
avatars: service_enabled?(:avatars),
|
||||
grading_role: grading_role)
|
||||
grading_role: grading_role,
|
||||
crocodoc_ids: crocodoc_ids)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -112,7 +112,7 @@ class SubmissionsController < ApplicationController
|
|||
id = @current_user.try(:id)
|
||||
end
|
||||
@user = @context.all_students.find(params[:id]) rescue nil
|
||||
if !@user
|
||||
unless @user
|
||||
flash[:error] = t('errors.student_not_enrolled', "The specified user is not a student in this course")
|
||||
respond_to do |format|
|
||||
format.html { redirect_to named_context_url(@context, :context_assignment_url, @assignment.id) }
|
||||
|
@ -127,6 +127,11 @@ class SubmissionsController < ApplicationController
|
|||
|
||||
@submission = @assignment.submissions.where(user_id: @user).first
|
||||
@submission ||= @assignment.submissions.build(:user => @user)
|
||||
if @context.feature_enabled?(:moderated_grading) && @assignment.moderated_grading?
|
||||
student = User.where(id: @submission.user_id).first
|
||||
grader = User.where(id: @submission.grader_id).first
|
||||
@crocodoc_ids = [student, grader].compact.map(&:crocodoc_id!)
|
||||
end
|
||||
@rubric_association = @assignment.rubric_association
|
||||
@rubric_association.assessing_user_id = @submission.user_id if @rubric_association
|
||||
# can't just check the permission, because peer reviewiers can never read the grade
|
||||
|
@ -194,7 +199,7 @@ class SubmissionsController < ApplicationController
|
|||
@submission.limit_comments(@current_user, session)
|
||||
format.html
|
||||
end
|
||||
if !json_handled
|
||||
unless json_handled
|
||||
format.json {
|
||||
@submission.limit_comments(@current_user, session)
|
||||
excludes = @assignment.grants_right?(@current_user, session, :grade) ? [:grade, :score] : []
|
||||
|
|
|
@ -21,7 +21,7 @@ module AttachmentHelper
|
|||
def doc_preview_attributes(attachment, attrs={})
|
||||
if attachment.crocodoc_available?
|
||||
begin
|
||||
attrs[:crocodoc_session_url] = attachment.crocodoc_url(@current_user)
|
||||
attrs[:crocodoc_session_url] = attachment.crocodoc_url(@current_user, attrs[:crocodoc_ids])
|
||||
rescue => e
|
||||
Canvas::Errors.capture_exception(:crocodoc, e)
|
||||
end
|
||||
|
|
|
@ -1305,7 +1305,7 @@ class Assignment < ActiveRecord::Base
|
|||
!moderated_grading? || grades_published_at.present?
|
||||
end
|
||||
|
||||
def speed_grader_json(user, avatars: false, grading_role: :grader)
|
||||
def speed_grader_json(user, avatars: false, grading_role: :grader, crocodoc_ids: nil)
|
||||
Attachment.skip_thumbnails = true
|
||||
submission_fields = [:user_id, :id, :submitted_at, :workflow_state,
|
||||
:grade, :grade_matches_current_submission,
|
||||
|
@ -1408,7 +1408,7 @@ class Assignment < ActiveRecord::Base
|
|||
:methods => [:view_inline_ping_url]
|
||||
).tap { |json|
|
||||
json[:attachment][:canvadoc_url] = a.canvadoc_url(user)
|
||||
json[:attachment][:crocodoc_url] = a.crocodoc_url(user)
|
||||
json[:attachment][:crocodoc_url] = a.crocodoc_url(user, crocodoc_ids)
|
||||
json[:attachment][:submitted_to_crocodoc] = a.crocodoc_document.present?
|
||||
}
|
||||
end
|
||||
|
|
|
@ -1441,20 +1441,21 @@ class Attachment < ActiveRecord::Base
|
|||
"/api/v1/canvadoc_session?#{preview_params(user, "canvadoc")}"
|
||||
end
|
||||
|
||||
def crocodoc_url(user)
|
||||
def crocodoc_url(user, crocodoc_ids = nil)
|
||||
return unless crocodoc_available?
|
||||
"/api/v1/crocodoc_session?#{preview_params(user, "crocodoc")}"
|
||||
"/api/v1/crocodoc_session?#{preview_params(user, "crocodoc", crocodoc_ids)}"
|
||||
end
|
||||
|
||||
def previewable_media?
|
||||
self.content_type && self.content_type.match(/\A(video|audio)/)
|
||||
end
|
||||
|
||||
def preview_params(user, type)
|
||||
def preview_params(user, type, crocodoc_ids = nil)
|
||||
blob = {
|
||||
user_id: user.try(:global_id),
|
||||
attachment_id: id,
|
||||
type: type,
|
||||
crocodoc_ids: crocodoc_ids
|
||||
}.to_json
|
||||
hmac = Canvas::Security.hmac_sha1(blob)
|
||||
"blob=#{URI.encode blob}&hmac=#{URI.encode hmac}"
|
||||
|
|
|
@ -66,7 +66,7 @@ class CrocodocDocument < ActiveRecord::Base
|
|||
opts[:user] = user.crocodoc_user
|
||||
end
|
||||
|
||||
opts.merge! permissions_for_user(user)
|
||||
opts.merge! permissions_for_user(user, opts[:crocodoc_ids])
|
||||
|
||||
unless annotations_on
|
||||
opts[:filter] = 'none'
|
||||
|
@ -80,7 +80,7 @@ class CrocodocDocument < ActiveRecord::Base
|
|||
end
|
||||
end
|
||||
|
||||
def permissions_for_user(user)
|
||||
def permissions_for_user(user, whitelist = nil)
|
||||
opts = {
|
||||
:filter => 'none',
|
||||
:admin => false,
|
||||
|
@ -114,6 +114,18 @@ class CrocodocDocument < ActiveRecord::Base
|
|||
opts[:filter] = 'none'
|
||||
end
|
||||
|
||||
if whitelist
|
||||
opts[:filter] = case opts[:filter]
|
||||
when "all"
|
||||
whitelist.join(",")
|
||||
when "none"
|
||||
"none"
|
||||
else
|
||||
ids = whitelist.map(&:to_i) & [opts[:filter].to_i]
|
||||
ids.any? && ids.join(",") || "none"
|
||||
end
|
||||
end
|
||||
|
||||
opts
|
||||
end
|
||||
|
||||
|
|
|
@ -2544,7 +2544,7 @@ class User < ActiveRecord::Base
|
|||
return cid if cid
|
||||
|
||||
Setting.transaction do
|
||||
s = Setting.lock.where(name: 'crocodoc_counter').first
|
||||
s = Setting.lock.where(name: 'crocodoc_counter').first_or_create(value: 0)
|
||||
cid = s.value = s.value.to_i + 1
|
||||
s.save!
|
||||
end
|
||||
|
|
|
@ -125,7 +125,7 @@
|
|||
%>
|
||||
<% end %>
|
||||
<% if attachment.crocodoc_available? || attachment.canvadocable? %>
|
||||
<a href=<%= preview_url %> data-tooltip title="<%= preview_document %>" class="modal_preview_link Button--link" role="button" data-attachment_id="<%= attachment.id %>" data-submission_id="<%= @submission.id %>" data-dialog-title="<%= attachment.display_name %>" <%= doc_preview_attributes(attachment) %>><%= button_text %></a>
|
||||
<a href=<%= preview_url %> data-tooltip title="<%= preview_document %>" class="modal_preview_link Button--link" role="button" data-attachment_id="<%= attachment.id %>" data-submission_id="<%= @submission.id %>" data-dialog-title="<%= attachment.display_name %>" <%= doc_preview_attributes(attachment, crocodoc_ids: @crocodoc_ids) %>><%= button_text %></a>
|
||||
<% end %>
|
||||
</div>
|
||||
|
||||
|
|
|
@ -86,6 +86,11 @@ def student_in_course(opts={})
|
|||
course_with_student(opts)
|
||||
end
|
||||
|
||||
def ta_in_course(opts={})
|
||||
opts[:course] = @course if @course && !opts[:course]
|
||||
course_with_ta(opts)
|
||||
end
|
||||
|
||||
def student_in_section(section, opts={})
|
||||
student = opts.fetch(:user) { user }
|
||||
enrollment = section.course.enroll_user(student, 'StudentEnrollment', :section => section, :force_update => true)
|
||||
|
|
|
@ -70,6 +70,13 @@ describe Attachment do
|
|||
end
|
||||
|
||||
context "crocodoc" do
|
||||
include HmacHelper
|
||||
let_once(:user) { user_model }
|
||||
let_once(:course) { course_model }
|
||||
let_once(:student) do
|
||||
course.enroll_student(user_model).accept
|
||||
@user
|
||||
end
|
||||
before { configure_crocodoc }
|
||||
|
||||
it "crocodocable?" do
|
||||
|
@ -77,6 +84,20 @@ describe Attachment do
|
|||
expect(@attachment).to be_crocodocable
|
||||
end
|
||||
|
||||
it "should include a whitelist of crocodoc_ids in the url blob" do
|
||||
crocodocable_attachment_model
|
||||
crocodoc_ids = [user.crocodoc_id!, student.crocodoc_id!]
|
||||
@attachment.submit_to_crocodoc
|
||||
|
||||
url = Rack::Utils.parse_nested_query(@attachment.crocodoc_url(user, crocodoc_ids).sub(/^.*\?{1}/, ""))
|
||||
blob = extract_blob(url["hmac"], url["blob"],
|
||||
"user_id" => user.id,
|
||||
"type" => "crocodoc")
|
||||
|
||||
expect(blob["crocodoc_ids"]).to be_present
|
||||
expect(blob["crocodoc_ids"]).to eq(crocodoc_ids & blob["crocodoc_ids"])
|
||||
end
|
||||
|
||||
it "should submit to crocodoc" do
|
||||
crocodocable_attachment_model
|
||||
expect(@attachment.crocodoc_available?).to be_falsey
|
||||
|
|
|
@ -33,6 +33,7 @@ describe 'CrocodocDocument' do
|
|||
before :once do
|
||||
teacher_in_course(:active_all => true)
|
||||
student_in_course
|
||||
ta_in_course
|
||||
@submitter = @student
|
||||
student_in_course
|
||||
@other_student = @student
|
||||
|
@ -54,6 +55,14 @@ describe 'CrocodocDocument' do
|
|||
})
|
||||
end
|
||||
|
||||
it "should only include ids specified in the whitelist" do
|
||||
expect(@crocodoc.permissions_for_user(@teacher, [@teacher.crocodoc_id!, @submitter.crocodoc_id!])).to eq({
|
||||
:filter => "#{@teacher.crocodoc_id!},#{@submitter.crocodoc_id!}",
|
||||
:admin => true,
|
||||
:editable => true,
|
||||
})
|
||||
end
|
||||
|
||||
context "submitter permissions" do
|
||||
it "should see everything (unless the assignment is muted)" do
|
||||
expect(@crocodoc.permissions_for_user(@submitter)).to eq({
|
||||
|
|
Loading…
Reference in New Issue