Better error logs and responses for grade passback

This adds error reports for failed/unsupported outcomes

Fixes PLAT-1278

Test Plan:
TBD... Any sugestions

Change-Id: Ie29f5e92735d73268f9221bdba73d1ef7af87758
Reviewed-on: https://gerrit.instructure.com/66083
Reviewed-by: Nathan Mills <nathanm@instructure.com>
Tested-by: Jenkins
QA-Review: August Thornton <august@instructure.com>
Product-Review: Brad Horrocks <bhorrocks@instructure.com>
This commit is contained in:
Brad Horrocks 2015-10-29 11:22:50 -06:00
parent ff5dd90527
commit 020e580830
3 changed files with 23 additions and 6 deletions

View File

@ -35,7 +35,7 @@ class LtiApiController < ApplicationController
xml = Nokogiri::XML.parse(request.body) xml = Nokogiri::XML.parse(request.body)
lti_response = BasicLTI::BasicOutcomes.process_request(@tool, xml) lti_response = check_outcome BasicLTI::BasicOutcomes.process_request(@tool, xml)
render :text => lti_response.to_xml, :content_type => 'application/xml' render :text => lti_response.to_xml, :content_type => 'application/xml'
end end
@ -45,7 +45,7 @@ class LtiApiController < ApplicationController
def legacy_grade_passback def legacy_grade_passback
verify_oauth verify_oauth
lti_response = BasicLTI::BasicOutcomes.process_legacy_request(@tool, params) lti_response = check_outcome BasicLTI::BasicOutcomes.process_legacy_request(@tool, params)
render :text => lti_response.to_xml, :content_type => 'application/xml' render :text => lti_response.to_xml, :content_type => 'application/xml'
end end
@ -176,4 +176,15 @@ class LtiApiController < ApplicationController
generated_signature: @signature.signature generated_signature: @signature.signature
} }
end end
def check_outcome(outcome)
if ['unsupported', 'failure'].include? outcome.code_major
opts = {type: :grade_passback}
error_info = Canvas::Errors::Info.new(request, @domain_root_account, @current_user, opts).to_h
capture_outputs = Canvas::Errors.capture("Grade pass back #{outcome.code_major}", error_info)
outcome.description += "\n[EID_#{capture_outputs[:error_report]}]"
end
outcome
end
end end

View File

@ -66,6 +66,7 @@ module BasicLTI
unless res.handle_request(tool) unless res.handle_request(tool)
res.code_major = 'unsupported' res.code_major = 'unsupported'
res.description = 'Request could not be handled. ¯\_(ツ)_/¯'
end end
return res return res
end end
@ -75,6 +76,7 @@ module BasicLTI
unless res.handle_request(tool) unless res.handle_request(tool)
res.code_major = 'unsupported' res.code_major = 'unsupported'
res.description = 'Legacy request could not be handled. ¯\_(ツ)_/¯'
end end
return res return res
end end

View File

@ -259,8 +259,12 @@ XML
expect(response.content_type).to eq 'application/xml' expect(response.content_type).to eq 'application/xml'
xml = Nokogiri::XML.parse(response.body) xml = Nokogiri::XML.parse(response.body)
expect(xml.at_css('imsx_POXEnvelopeResponse > imsx_POXHeader > imsx_POXResponseHeaderInfo > imsx_statusInfo > imsx_codeMajor').content).to eq failure_type expect(xml.at_css('imsx_POXEnvelopeResponse > imsx_POXHeader > imsx_POXResponseHeaderInfo > imsx_statusInfo > imsx_codeMajor').content).to eq failure_type
expect(xml.at_css('imsx_description').content).to eq error_message if error_message
expect(@assignment.submissions.where(user_id: @student)).not_to be_exists expect(@assignment.submissions.where(user_id: @student)).not_to be_exists
desc = xml.at_css('imsx_description').content.match(/(?<description>.+)\n\[EID_(?<error_report>[^\]]+)\]/)
expect(desc[:description]).to eq error_message if error_message
expect(desc[:error_report]).to_not be_empty
end end
def check_success def check_success
@ -341,7 +345,7 @@ XML
expect(response).to be_success expect(response).to be_success
xml = Nokogiri::XML.parse(response.body) xml = Nokogiri::XML.parse(response.body)
expect(xml.at_css('imsx_codeMajor').content).to eq 'failure' expect(xml.at_css('imsx_codeMajor').content).to eq 'failure'
expect(xml.at_css('imsx_description').content).to eq "No score given" expect(xml.at_css('imsx_description').content).to match /^No score given/
expect(@assignment.submissions.where(user_id: @student)).not_to be_exists expect(@assignment.submissions.where(user_id: @student)).not_to be_exists
end end
@ -351,7 +355,7 @@ XML
expect(response).to be_success expect(response).to be_success
xml = Nokogiri::XML.parse(response.body) xml = Nokogiri::XML.parse(response.body)
expect(xml.at_css('imsx_codeMajor').content).to eq 'failure' expect(xml.at_css('imsx_codeMajor').content).to eq 'failure'
expect(xml.at_css('imsx_description').content).to eq "Score is not between 0 and 1" expect(xml.at_css('imsx_description').content).to match /^Score is not between 0 and 1/
expect(@assignment.submissions.where(user_id: @student)).not_to be_exists expect(@assignment.submissions.where(user_id: @student)).not_to be_exists
end end
@ -362,7 +366,7 @@ XML
expect(response).to be_success expect(response).to be_success
xml = Nokogiri::XML.parse(response.body) xml = Nokogiri::XML.parse(response.body)
expect(xml.at_css('imsx_codeMajor').content).to eq 'failure' expect(xml.at_css('imsx_codeMajor').content).to eq 'failure'
expect(xml.at_css('imsx_description').content).to eq "Assignment has no points possible." expect(xml.at_css('imsx_description').content).to match /^Assignment has no points possible\./
end end
it "should pass if assignment has 0 points possible" do it "should pass if assignment has 0 points possible" do