MDL-20534

I ran the software through the certification and caught a few nits:

The error return is 'failure' not 'error'
The spec says that it needs to return 'failure' for out of range or non-numeric grades
The result score needs a language tag, hard-coded as 'en'
Setting a grade multiplied by 100 but reading the grade did not divide by 100
All those are now fixed with this patch as well as this bit of cruft:

I removed the "extension service url" as it is not implemented in service.php

Feel free to review and adjust - probably the one place you might want to refactor
is that I put code to catch out-of-range-and non-numeric in
lti_parse_grade_replace_message and threw an exception on error and then caught
it in service.php and sent back the 'failure' message. Feel free to refactor a
bit if you see this done in a cleaner manner.
This commit is contained in:
Charles Severance 2011-11-15 12:46:48 -05:00 committed by Chris Scribner
parent 0297a3d4b7
commit ddcfda87d6
3 changed files with 45 additions and 24 deletions

View File

@ -253,14 +253,18 @@ function lti_build_request($instance, $typeconfig, $course) {
if ( isset($placementsecret) &&
( $typeconfig['acceptgrades'] == LTI_SETTING_ALWAYS ||
( $typeconfig['acceptgrades'] == LTI_SETTING_DELEGATE && $instance->instructorchoiceacceptgrades == LTI_SETTING_ALWAYS ) ) ) {
$requestparams['lis_result_sourcedid'] = $sourcedid;
$requestparams["lis_result_sourcedid"] = $sourcedid;
$serviceurl = $CFG->wwwroot . '/mod/lti/service.php';
/* $serviceurl = $CFG->wwwroot . '/mod/lti/service.php';
if ($typeconfig['forcessl'] == '1') {
$serviceurl = lti_ensure_url_is_https($serviceurl);
}
$requestparams['ext_ims_lis_basic_outcome_url'] = $serviceurl;
$requestparams["ext_ims_lis_basic_outcome_url"] = $serviceurl; */
//Add outcome service URL
$url = new moodle_url('/mod/lti/service.php');
$requestparams['lis_outcome_service_url'] = $url->out();
}
/*if ( isset($placementsecret) &&
@ -273,20 +277,16 @@ function lti_build_request($instance, $typeconfig, $course) {
// Send user's name and email data if appropriate
if ( $typeconfig['sendname'] == LTI_SETTING_ALWAYS ||
( $typeconfig['sendname'] == LTI_SETTING_DELEGATE && $instance->instructorchoicesendname == LTI_SETTING_ALWAYS ) ) {
$requestparams['lis_person_name_given'] = $USER->firstname;
$requestparams['lis_person_name_family'] = $USER->lastname;
$requestparams['lis_person_name_full'] = $USER->firstname." ".$USER->lastname;
$requestparams["lis_person_name_given"] = $USER->firstname;
$requestparams["lis_person_name_family"] = $USER->lastname;
$requestparams["lis_person_name_full"] = $USER->firstname." ".$USER->lastname;
}
if ( $typeconfig['sendemailaddr'] == LTI_SETTING_ALWAYS ||
( $typeconfig['sendemailaddr'] == LTI_SETTING_DELEGATE && $instance->instructorchoicesendemailaddr == LTI_SETTING_ALWAYS ) ) {
$requestparams['lis_person_contact_email_primary'] = $USER->email;
$requestparams["lis_person_contact_email_primary"] = $USER->email;
}
//Add outcome service URL
$url = new moodle_url('/mod/lti/service.php');
$requestparams['lis_outcome_service_url'] = $url->out();
// Concatenate the custom parameters from the administrator and the instructor
// Instructor parameters are only taken into consideration if the administrator
// has giver permission
@ -313,20 +313,18 @@ function lti_build_request($instance, $typeconfig, $course) {
}
// Make sure we let the tool know what LMS they are being called from
$requestparams['ext_lms'] = 'moodle-2';
$requestparams['tool_consumer_info_product_family_code'] = 'moodle';
$requestparams['tool_consumer_info_version'] = strval($CFG->version);
$requestparams["ext_lms"] = "moodle-2";
// Add oauth_callback to be compliant with the 1.0A spec
$requestparams['oauth_callback'] = 'about:blank';
$requestparams["oauth_callback"] = "about:blank";
//The submit button needs to be part of the signature as it gets posted with the form.
//This needs to be here to support launching without javascript.
$submittext = get_string('press_to_submit', 'lti');
$requestparams['ext_submit'] = $submittext;
$requestparams["ext_submit"] = $submittext;
$requestparams['lti_version'] = 'LTI-1p0';
$requestparams['lti_message_type'] = 'basic-lti-launch-request';
$requestparams["lti_version"] = "LTI-1p0";
$requestparams["lti_message_type"] = "basic-lti-launch-request";
/* Suppress this for now - Chuck
if ( $orgdesc ) $requestparams["tool_consumer_instance_description"] = $orgdesc;
*/

View File

@ -61,7 +61,18 @@ foreach ($body->children() as $child) {
switch ($messagetype) {
case 'replaceResultRequest':
$parsed = lti_parse_grade_replace_message($xml);
try {
$parsed = lti_parse_grade_replace_message($xml);
} catch (Exception $e) {
$responsexml = lti_get_response_xml(
'failure',
$e->getMessage(),
uniqid(),
'replaceResultResponse');
echo $responsexml->asXML();
break;
}
$ltiinstance = $DB->get_record('lti', array('id' => $parsed->instanceid));
@ -70,7 +81,7 @@ switch ($messagetype) {
$gradestatus = lti_update_grade($ltiinstance, $parsed->userid, $parsed->launchid, $parsed->gradeval);
$responsexml = lti_get_response_xml(
$gradestatus ? 'success' : 'error',
$gradestatus ? 'success' : 'failure',
'Grade replace response',
$parsed->messageid,
'replaceResultResponse'
@ -94,14 +105,16 @@ switch ($messagetype) {
$grade = lti_read_grade($ltiinstance, $parsed->userid);
$responsexml = lti_get_response_xml(
isset($grade) ? 'success' : 'error',
isset($grade) ? 'success' : 'failure',
'Result read',
$parsed->messageid,
'readResultResponse'
);
$node = $responsexml->imsx_POXBody->readResultResponse;
$node->addChild('result')->addChild('resultScore')->addChild('textString', isset($grade) ? $grade : '');
$node = $node->addChild('result')->addChild('resultScore');
$node->addChild('language', 'en');
$node->addChild('textString', isset($grade) ? $grade : '');
echo $responsexml->asXML();
@ -117,7 +130,7 @@ switch ($messagetype) {
$gradestatus = lti_delete_grade($ltiinstance, $parsed->userid);
$responsexml = lti_get_response_xml(
$gradestatus ? 'success' : 'error',
$gradestatus ? 'success' : 'failure',
'Grade delete request',
$parsed->messageid,
'deleteResultResponse'

View File

@ -66,7 +66,15 @@ function lti_parse_grade_replace_message($xml) {
$resultjson = json_decode((string)$node);
$node = $xml->imsx_POXBody->replaceResultRequest->resultRecord->result->resultScore->textString;
$grade = floatval((string)$node);
$score = (string) $node;
if ( ! is_numeric($score) ) {
throw new Exception('Score must be numeric');
}
$grade = floatval($score);
if ( $grade < 0.0 || $grade > 1.0 ) {
throw new Exception('Score not between 0.0 and 1.0');
}
$parsed = new stdClass();
$parsed->gradeval = $grade * 100;
@ -163,6 +171,7 @@ function lti_read_grade($ltiinstance, $userid) {
if (isset($grades) && isset($grades->items[0]) && is_array($grades->items[0]->grades)) {
foreach ($grades->items[0]->grades as $agrade) {
$grade = $agrade->grade;
$grade = $grade / 100.0;
break;
}
}
@ -211,3 +220,4 @@ function lti_verify_sourcedid($ltiinstance, $parsed) {
throw new Exception('SourcedId hash not valid');
}
}