Permalink
Browse files

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.
  • Loading branch information...
1 parent 0297a3d commit ddcfda87d6f3cc4dfec9947a15bee84b880a76a6 @csev csev committed with scriby Nov 15, 2011
Showing with 45 additions and 24 deletions.
  1. +16 −18 mod/lti/locallib.php
  2. +18 −5 mod/lti/service.php
  3. +11 −1 mod/lti/servicelib.php
View
@@ -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
@@ -61,7 +61,18 @@
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 @@
$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 @@
$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 @@
$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
@@ -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');
}
}
+

0 comments on commit ddcfda8

Please sign in to comment.