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

Unify create and update OriginalityReport endpoints 

Closes PLAT-2818

Test Plan:
- Using the Create endpoint API found at
  https://canvas.instructure.com/doc/api/originality_reports.html
  and specify an originality score.
- Verify the originality report is created with the specified
  originality score and other data.
- Repeat the exact same post request, but change the originality
  score value.
- Verify the originality report is updated and a new one is not
  created.
- Verify you can still update the originality report via the
  PUT endpoint found in the same docs.

Change-Id: I4a2b8ad6f5b68e49a6195862cf7826fe31152bca
Reviewed-on: https://gerrit.instructure.com/125944
Tested-by: Jenkins
Reviewed-by: Andrew Butterfield <abutterfield@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Product-Review: Weston Dransfield <wdransfield@instructure.com>
This commit is contained in:
wdransfield 2017-09-13 11:46:43 -06:00 committed by Weston Dransfield
parent e53c819c44
commit 3b00907443
6 changed files with 135 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
app/controllers/lti/ims/authorization_controller.rb
View File

@ -83,7 +83,7 @@ module Lti
Lti::Oauth2::AuthorizationValidator::SecretNotFound,
Lti::Oauth2::AuthorizationValidator::MissingAuthorizationCode,
InvalidGrant do |e|
log_error(e)
Lti::Errors::ErrorLogger.log_error(e)
render json: {error: 'invalid_grant'}, status: :bad_request
end
# @API authorize
@ -129,13 +129,6 @@ module Lti
expires_in: Setting.get('lti.oauth2.access_token.expiration', 1.hour.to_s)
}
end
private
def log_error(e)
ErrorReport.log_error(e.class, {message: e.message, exception_message: e.message + "\n\n#{e.backtrace.join("\n")}"})
end
end
end
end

55
app/controllers/lti/originality_reports_api_controller.rb
View File

@ -109,10 +109,15 @@ module Lti
}.freeze
].freeze
rescue_from Lti::Errors::UnauthorizedToolError do |e|
Lti::Errors::ErrorLogger.log_error(e)
render_unauthorized_action
end
skip_before_action :load_user
before_action :authorized_lti2_tool, :plagiarism_feature_flag_enabled
before_action :attachment_in_context, only: [:create]
before_action :report_in_context, only: [:update, :show]
before_action :report_in_context, only: [:show]
# @API Create an Originality Report
# Create a new OriginalityReport for the specified file
@ -151,18 +156,22 @@ module Lti
#
# @returns OriginalityReport
def create
render_unauthorized_action and return unless tool_proxy_associated?
@report = OriginalityReport.new(create_report_params)
if @report.save
render json: api_json(@report, @current_user, session), status: :created
raise Lti::Errors::UnauthorizedToolError unless tool_proxy_associated?
if originality_report.present?
update
else
render json: @report.errors, status: :bad_request
@report = OriginalityReport.new(create_report_params)
if @report.save
render json: api_json(@report, @current_user, session), status: :created
else
render json: @report.errors, status: :bad_request
end
end
end
# @API Edit an Originality Report
# Modify an existing originality report
# Modify an existing originality report. An alternative to this endpoint is
# to POST the same parameters listed below to the CREATE endpoint.
#
# @argument originality_report[originality_score] [Float]
# A number between 0 and 100 representing the measure of the
@ -195,11 +204,12 @@ module Lti
#
# @returns OriginalityReport
def update
render_unauthorized_action and return unless tool_proxy_associated?
if @report.update_attributes(update_report_params)
render json: api_json(@report, @current_user, session)
report_in_context
raise Lti::Errors::UnauthorizedToolError unless tool_proxy_associated?
if originality_report.update_attributes(update_report_params)
render json: api_json(originality_report, @current_user, session)
else
render json: @report.errors, status: :bad_request
render json: originality_report.errors, status: :bad_request
end
end
@ -208,8 +218,8 @@ module Lti
#
# @returns OriginalityReport
def show
render_unauthorized_action and return unless tool_proxy_associated?
render json: api_json(@report, @current_user, session)
raise Lti::Errors::UnauthorizedToolError unless tool_proxy_associated?
render json: api_json(originality_report, @current_user, session)
end
def lti2_service_name
@ -288,7 +298,7 @@ module Lti
def attachment_association
@_attachment_association ||= begin
file = @report&.attachment || attachment
file = originality_report&.attachment || attachment
file.attachment_associations.find { |a| a.context == submission }
end
end
@ -315,15 +325,22 @@ module Lti
verify_submission_attachment(attachment, submission)
end
def originality_report
@_originality_report ||= begin
OriginalityReport.find_by(id: params[:id]) ||
OriginalityReport.find_by(attachment_id: attachment&.id)
end
end
def report_in_context
@report = OriginalityReport.find_by(id: params[:id]) || OriginalityReport.find_by!(attachment_id: attachment&.id)
verify_submission_attachment(@report.attachment, submission)
raise ActiveRecord::RecordNotFound if originality_report.blank?
verify_submission_attachment(originality_report.attachment, submission)
end
def verify_submission_attachment(attachment, submission)
head :not_found and return unless attachment.present? && submission.present?
raise ActiveRecord::RecordNotFound unless attachment.present? && submission.present?
unless submission.assignment == assignment && submission.attachments.include?(attachment)
head :unauthorized
raise Lti::Errors::UnauthorizedToolError
end
end

1
lib/lti/errors.rb
View File

@ -21,5 +21,6 @@ module Lti
class UnsupportedExportTypeError < StandardError; end
class UnsupportedMessageTypeError < StandardError; end
class InvalidMediaTypeError < StandardError; end
class UnauthorizedToolError < StandardError; end
end
end

28
lib/lti/errors/error_logger.rb Normal file
View File

@ -0,0 +1,28 @@
#
# Copyright (C) 2017 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
# Filters added to this controller apply to all controllers in the application.
# Likewise, all the methods added will be available for all controllers.
module Lti::Errors
module ErrorLogger
def self.log_error(e)
ErrorReport.log_error(e.class, {message: e.message, exception_message: e.message + "\n\n#{e.backtrace.join("\n")}"})
end
end
end

43
spec/apis/lti/originality_reports_api_spec.rb
View File

@ -26,8 +26,8 @@ module Lti
include_context 'lti2_api_spec_helper'
let(:service_name) { OriginalityReportsApiController::ORIGINALITY_REPORT_SERVICE }
let(:aud) { host }
before(:once) { attachment_model }
before :each do
attachment_model
message_handler.update_attributes(message_type: 'basic-lti-launch-request')
course_factory(active_all: true)
student_in_course active_all: true
@ -123,7 +123,6 @@ module Lti
].freeze
get @endpoints[:show], headers: request_headers
expect(response).to be_success
expect(JSON.parse(response.body).keys).to match_array(expected_keys)
end
@ -356,11 +355,9 @@ module Lti
end
it "verifies the report is in the same context as the assignment" do
@submission.attachments = []
@submission.save!
put @endpoints[:update], params: {originality_report: {originality_report_lti_url: "http://www.lti-test.com"}}, headers: request_headers
expect(response.status).to eq 401
end
@ -645,8 +642,6 @@ module Lti
end
it "checks for required params" do
post @endpoints[:create], headers: request_headers
expect(response.status).to eq 400
@ -658,7 +653,6 @@ module Lti
end
it "checks that the specified assignment exists" do
invalid_attach_url = "/api/lti/assignments/#{@assignment.id + 1}/submissions/#{@submission.id}/originality_report"
post invalid_attach_url, params: {originality_report: {file_id: @attachment.id, originality_score: 0.4}}
expect(response).not_to be_success
@ -676,17 +670,6 @@ module Lti
expect(response.code).to eq '401'
end
it "gives useful error message on non unique tool/file combinations" do
post @endpoints[:create], params: {originality_report: {file_id: @attachment.id, originality_score: 0.4}}, headers: request_headers
expect(response.status).to eq 201
post @endpoints[:create], params: {originality_report: {file_id: @attachment.id, originality_score: 0.4}}, headers: request_headers
expect(response.status).to eq 400
attachment_error = JSON.parse(response.body)['errors']['attachment'].first
expect(attachment_error['type']).to eq 'taken'
end
it "requires the plagiarism feature flag" do
allow_any_instance_of(Account).to receive(:feature_enabled?).with(:plagiarism_detection_platform).and_return(false)
@ -780,6 +763,30 @@ module Lti
expect(response_body['tool_setting']['resource_url']).to eq launch_url
end
it 'updates the originality report if it has already been created' do
originality_score = 50
post @endpoints[:create],
params: {
originality_report: {
file_id: @attachment.id,
workflow_state: 'pending'
}
},
headers: request_headers
post @endpoints[:create],
params: {
originality_report: {
file_id: @attachment.id,
originality_score: originality_score
}
},
headers: request_headers
response_body = JSON.parse(response.body)
expect(response_body['originality_score']).to eq 50
end
context "optional params" do
before :each do

44
spec/lib/lti/errors/error_logger_spec.rb Normal file
View File

@ -0,0 +1,44 @@
#
# Copyright (C) 2017 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
require File.expand_path(File.dirname(__FILE__) + '/../../../spec_helper')
describe Lti::Errors::ErrorLogger do
describe '#log_error' do
let(:message) { 'An error message!' }
let(:error) { StandardError.new(message) }
before do
allow(error).to receive(:backtrace) { ['afile.rb line 23', 'another_file.rb line 33'] }
end
it 'creates a new error report' do
expect { Lti::Errors::ErrorLogger.log_error(error) }.to change { ErrorReport.count }.from(0).to(1)
end
it 'sets the error report message to the message of the error' do
Lti::Errors::ErrorLogger.log_error(error)
expect(ErrorReport.last.message).to eq message
end
it 'includes the backtrace in the exception message' do
Lti::Errors::ErrorLogger.log_error(error)
expect(ErrorReport.last.data['exception_message']).to eq message + "\n\n#{error.backtrace.join("\n")}"
end
end
end