ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix links to originality reports multiple attempts 

Commit 39d77b5e added an "attempt" parameter so plagiarism platform
providers can reliably set and update scores for online_text_entry
submissions with multiple attempts. However Canvas also needs to provide
a way for the user to access the report for a given attempt number,
rather than always showing the report for the first submission attempt.

closes PLAT-5232
flag=none

Test plan:
- As in https://github.com/instructure/canvas-lms/commit/39d77b5e test
  plan, make an assignment that accepts online_text_entry, and as a
  student, submit to it twice (making two attempts).
- As before, make originality score for each attempt using the API, this
  time also sending `resource_url`s
  curl -H "Authorization: Bearer MYTOKEN" http://web.canvas-lms.docker/api/lti/assignments/39/submissions/27/originality_report --data 'originality_report[attempt]=1&originality_report[originality_score]=11&originality_report[tool_setting][resource_type_code]=originality_reports&originality_report[tool_setting][resource_url]=http://example.com/eleven'
  (Note: if you comment out `raise Canvas::Security::TokenExpired` in
  lib/canvas/security.rb, you can use an old token from your Plagiarism
  Platform test tool)
- Go to the places in the UI where links to the report appear and make
  sure all links now point to a URL with an `attempt=1` (etc.) query
  param. Make sure the attempt number matches up. The four places are:
  1. In speed grader on the right side
  2. In speed grader on the left side of the page -- this is in a
  partial which is shown in other places like when the student clicks
  details for a submission
  3. In student grades -- as an instructor, go to gradebook and click on
  a student's name. In the table with assignments, on the right, there
  will be a red or green bubble -- clicking that takes you to the
  report. (You can only the last attempt's report here).
  4. In old gradebook there is a link too. In new gradebook there is
  not. (I didn't change gradebook stuff due to new gradebook
  uncertainty)
- Click the link, which should forward to an LTI 2 launch URL. For me,
  this page doesn't work (and didn't for me on Jesse's test account
  either) but you can get the `resource/XXXXX/` parameter in the URL and
  look up the URL by something like `Lti::Link.find_by(resource_link_id:
  'ec2fa300-2c64-432f-83db-8af709059a7c').resource_url`. Make sure this
  matches up to the correct URL you sent for that attempt.
- Optionally, resubmit originality reports using the API, using new
  scores and report URLs. Again click a link in the UI and get the
  resource_link_id (that shouldn't have changed) and check that the URL
  `Lti::Link.find_by(resource_link_id: '...').resource_url` did change.

Change-Id: Ifc8828b4d4486adc2988b7b71cb712821b2b42c2
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/221399
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
QA-Review: Weston Dransfield <wdransfield@instructure.com>
Product-Review: Evan Battaglia <ebattaglia@instructure.com>
This commit is contained in:
Evan Battaglia 2019-12-13 16:31:31 -07:00
parent 374bf0ca84
commit dffd8e03e4
8 changed files with 85 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/controllers/submissions_base_controller.rb
View File

@ -244,7 +244,7 @@ class SubmissionsBaseController < ApplicationController
@asset_string = params[:asset_string]
if authorized_action(@submission, @current_user, :read)
url = if type == 'originality_report'
@submission.originality_report_url(@asset_string, @current_user)
@submission.originality_report_url(@asset_string, @current_user, params[:attempt])
else
legacy_plagiarism_report(@submission, @asset_string, type)
end

9
app/models/submission.rb
View File

@ -776,8 +776,13 @@ class Submission < ActiveRecord::Base
end
# Preload OriginalityReport before using this method
def originality_report_url(asset_string, user)
if asset_string == self.asset_string
def originality_report_url(asset_string, user, attempt=nil)
if attempt
version = versions.find{ |v| v.model&.attempt&.to_s == attempt.to_s }
return nil unless version
report = originality_reports.find_by(submission_time: version.model.submitted_at)
report&.report_launch_path
elsif asset_string == self.asset_string
originality_reports.where(attachment_id: nil).first&.report_launch_path
elsif self.grants_right?(user, :view_turnitin_report)
requested_attachment = all_versioned_attachments.find_by_asset_string(asset_string)

2
app/views/gradebooks/grade_summary.html.erb
View File

@ -340,7 +340,7 @@
url = context_url(@context, :context_assignment_submission_turnitin_report_url, assignment.id, @presenter.student_id, submission.asset_string)
elsif assignment_presenter.originality_report? && turnitin
asset_string = assignment_presenter.file&.asset_string || assignment_presenter.submission&.asset_string
url = polymorphic_url([@context, assignment, submission, :originality_report], asset_string: asset_string)
url = polymorphic_url([ @context, assignment, submission, :originality_report], asset_string: asset_string, attempt: submission.attempt)
anchor_title = turnitin[:error_message] ? turnitin[:error_message] : t('Originality Report')
alt_text = turnitin[:error_message] ? turnitin[:error_message] : t('Originality Report')
elsif assignment_presenter.is_online_upload? && assignment_presenter.file

2
app/views/submissions/_originality_score.html.erb
View File

@ -49,7 +49,7 @@
<%= image_tag("turnitin_#{turnitin_score[:state]}_score.png", :alt => turnitin_score[:error_message]) %>
</a>
<% else %>
<a href="<%= polymorphic_url([@context, @assignment, @submission, :originality_report], asset_string: attachment&.asset_string || @submission.asset_string) %>"
<a href="<%= polymorphic_url([@context, @assignment, @submission, :originality_report], asset_string: attachment&.asset_string || @submission.asset_string, attempt: @submission.attempt) %>"
target="_blank"
title="<%= "Similarity score -- #{turnitin_score[:state]}" %>"
class="not_external turnitin_similarity_score <%= turnitin_score[:state] %>_score"

5
public/javascripts/speed_grader.js
View File

@ -1801,11 +1801,12 @@ EG = {
$assignment_submission_turnitin_report_url,
$assignment_submission_originality_report_url
)
const reportUrl = $.replaceTags(urlContainer.attr('href'), {
const tooltip = I18n.t('Similarity Score - See detailed report')
let reportUrl = $.replaceTags(urlContainer.attr('href'), {
[anonymizableUserId]: submission[anonymizableUserId],
asset_string: assetString
})
const tooltip = I18n.t('Similarity Score - See detailed report')
reportUrl += (reportUrl.includes('?') ? '&' : '?') + 'attempt=' + submission.attempt
$turnitinScoreContainer.html(
turnitinScoreTemplate({

28
spec/controllers/submissions_controller_spec.rb
View File

@ -770,6 +770,34 @@ describe SubmissionsController do
expect(response).to redirect_to originality_report.originality_report_url
end
context 'when there are multiple originality reports' do
let(:submission2) { assignment.submit_homework(test_student, body: 'hello world') }
let(:report_url2) { 'http://www.another-test-score.com/' }
let(:originality_report2) {
OriginalityReport.create!(attachment: nil,
submission: submission2,
originality_score: 0.4,
originality_report_url: report_url2)
}
it 'can use attempt number to find the report url for text entry submissions' do
originality_report2 # Create immediately
originality_report.update_attributes!(attachment: nil)
expect(submission2.id).to eq(submission.id) # submission2 is updated/reloaded with new version (last attempt number)
expect(submission2.attempt).to be > submission.attempt
get 'originality_report', params: {
course_id: assignment.context_id, assignment_id: assignment.id, submission_id: test_student.id,
asset_string: submission.asset_string, attempt: 1
}
expect(response).to redirect_to originality_report.originality_report_url
get 'originality_report', params: {
course_id: assignment.context_id, assignment_id: assignment.id, submission_id: test_student.id,
asset_string: submission.asset_string, attempt: 2
}
expect(response).to redirect_to originality_report2.originality_report_url
end
end
it 'returns bad_request if submission_id is not an integer' do
get 'originality_report', params: {
course_id: assignment.context_id,

31
spec/javascripts/jsx/speed_graderSpec.js
View File

@ -6354,9 +6354,9 @@ QUnit.module('SpeedGrader', rootHooks => {
<div id='submission_files_container'>
<span class='turnitin_info_container'></span>
</div>
<a id='assignment_submission_originality_report_url' href='#'></a>
<a id='assignment_submission_turnitin_report_url' href='#{{ user_id }}/{{ asset_string }}'></a>
<a id='assignment_submission_vericite_report_url' href='#'></a>
<a id='assignment_submission_originality_report_url' href='/orig/{{ user_id }}/{{ asset_string }}'></a>
<a id='assignment_submission_turnitin_report_url' href='/tii/{{ user_id }}/{{ asset_string }}'></a>
<a id='assignment_submission_vericite_report_url' href='/vericite/{{ user_id }}/{{ asset_string }}'></a>
`)
fakeENV.setup({
@ -6380,7 +6380,8 @@ QUnit.module('SpeedGrader', rootHooks => {
grading_period_id: 8,
id: '1',
user_id: '1',
submission_type: 'online_text_entry'
submission_type: 'online_text_entry',
attempt: 2
}
]
}
@ -6473,7 +6474,25 @@ QUnit.module('SpeedGrader', rootHooks => {
}
SpeedGrader.EG.handleSubmissionSelectionChange()
ok(document.querySelector(gradeSimilaritySelector).href.includes('#1/submission_1'))
ok(document.querySelector(gradeSimilaritySelector).href.includes('/tii/1/submission_1'))
})
test('includes the attempt ID for Originality Report submissions', () => {
submission.submission_history[0].turnitin_data = turnitinData
submission.submission_history[0].has_originality_score = true
submission.submission_history[0].has_originality_report = true
window.jsonData = testJsonData
SpeedGrader.EG.jsonReady()
SpeedGrader.EG.currentStudent = {
...student,
submission
}
SpeedGrader.EG.handleSubmissionSelectionChange()
const destinationURL = new URL(document.querySelector(gradeSimilaritySelector).href)
strictEqual(destinationURL.pathname, '/orig/1/submission_1')
strictEqual(destinationURL.search, '?attempt=2')
})
test('links to a detailed report for VeriCite submissions', () => {
@ -6529,7 +6548,7 @@ QUnit.module('SpeedGrader', rootHooks => {
SpeedGrader.EG.handleSubmissionSelectionChange()
const destinationURL = new URL(document.querySelector(gradeSimilaritySelector).href)
strictEqual(destinationURL.hash, '#abcde/submission_1')
strictEqual(destinationURL.pathname, '/tii/abcde/submission_1')
})
test('does not link to a report for VeriCite submissions', () => {

19
spec/models/submission_spec.rb
View File

@ -2487,6 +2487,25 @@ describe Submission do
expect(submission.originality_report_url(submission.asset_string, test_teacher)).to eq report_url
end
context 'when there are multiple originality reports' do
let(:submission2) { assignment.submit_homework(test_student, body: 'hello world') }
let(:report_url2) { 'http://www.another-test-score.com/' }
let(:originality_report2) {
OriginalityReport.create!(attachment: nil,
submission: submission2,
originality_score: 0.4,
originality_report_url: report_url2)
}
it 'can use attempt number to find the report url for text entry submissions' do
originality_report2
originality_report.update_attributes!(attachment: nil)
expect(submission2.attempt).to be > submission.attempt
expect(submission.originality_report_url(submission.asset_string, test_teacher, submission.attempt.to_s)).to eq report_url
expect(submission.originality_report_url(submission.asset_string, test_teacher, submission2.attempt.to_s)).to eq report_url2
end
end
it 'requires the :grade permission' do
unauthorized_user = User.new
expect(submission.originality_report_url(attachment.asset_string, unauthorized_user)).to be_nil