Moodle

AICC Tracking Issues (SkillSoft content and others utilising multi line suspend data) - UPDATE

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.8, 1.9
  • Fix Version/s: 1.9.5, 2.0
  • Component/s: SCORM
  • Labels:
    None
  • Environment:
    Moodle 1.8.5, and 1.9.3 on XAMPP 1.6.6 on Windows
  • Database:
    MySQL
  • Affected Branches:
    MOODLE_18_STABLE, MOODLE_19_STABLE
  • Fixed Branches:
    MOODLE_19_STABLE, MOODLE_20_STABLE

Description

I had identified a number of issues in report MDL-14455 regarding tracking of AICC content.

Since these patches I have identified some additional issues for SkillSoft content and legacy NETG content (NETG were bought by SkillSoft).

The information below builds on the patch included with MDL-14455.

For testing of AICC content you can download metdata samples at http://emeatest.skillsoft.com/integration/olsa/olsa.htm

---------------------

1. Formatting of output to be text/plain.
The AICC standard states that the output should be text/plain however NETG KNET content that uses , "Javscript Remote Scripting using IFRAME" - see this page for full details http://developer.apple.com/internet/webcontent/iframe.html will in IE display a prompt asking if you wish to download the content.

This issue is caused by the IE's mime sniffing. See here http://msdn.microsoft.com/en-us/library/ms537641(VS.85).aspx#sniffing

The workaround while not strictly AICC compliant is to serve the AICC response as HTML.

To do this the AICC.PHP file included in MDL-14455 should be editted and this line commented out:

//Content Type Header
//Martin Holden, SkillSoft
//21st January 2009
//Disabled content type header due to issues with MIME type sniffing in IE
//See http://msdn.microsoft.com/en-us/library/ms537641(VS.85).aspx#sniffing
//With content-type set certain content would prompt to Save the "AICC.PHP" page in IE
//header("Content-Type: text/plain;");

---------------------------

2. Line Termination
The AICC standard states that the output should have CR LF line termination not just LF. Replace all instances where data is output to have a line termination of \r\n rather than just \n

----------------------------

3. Processing of mulit-line [core_lesson]
There was a flaw in my logic for processing multiline suspend data, I was failing to correctly include the CR LF before encoding the data.

To do this the AICC.PHP file included in MDL-14455 should be editted and this line changed:

while ((($datarow = current($datarows)) !== false) && (substr($datarow,0,1) != '[')) {
//Martin Holden, SkillSoft
//21st January 2009
// Reinstate CR/LF back onto data before encoding
$value .= $datarow."\r\n";
next($datarows);
}

4. Ensuring the Moodle AICC.PHP is CASE INSENSITIVE when processing the AICC request. The method I use may not be the most elegant BUT I am not a PHP developer.

//Martin Holden, SkillSoft
//21st January 2009
//
// Case sensitivity fix for KEY values in post. This code adds new elements to the $_POST array
// with the key value forced to lowercase

foreach ($_POST as $key => $value)

{ //Print "$key has a value of $value"; $tempkey = strtolower($key); $_POST[$tempkey] = $value; }

//
// END

Issue Links

This issue has a clone:
MDL-14455 AICC Tracking Issues (SkillSoft content and others utilising multi line suspend data) Major Closed
This issue will help resolve:
MDL-12281 AICC packages failing to communicate with LMS Major Closed
MDL-10746 moodle does not add up cmi.session_time to cmi.total_time Minor Closed
This issue has a non-specific relationship to:
MDL-14485 AICC - Mastery_Score, Max_Time_Allowed and Time_Limit_Action not correctly returned to courses Major Closed
MDL-14486 AICC - Incorrect logic in the AICC.PHP code for handling CMI judgement of passed/failed status based on mastery_score Major Closed
MDL-12387 CMI.Evaluation Javascript Error Minor Closed

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Piers Harding added a comment -

Thanks Martin - I'll look at applying these patches during the week.

Show
Piers Harding added a comment - Thanks Martin - I'll look at applying these patches during the week.
Hide
Permalink
Piers Harding added a comment -

AICC patches for 1.9

Show
Piers Harding added a comment - AICC patches for 1.9
Hide
Permalink
Piers Harding added a comment -

Hi Martin -

I have reviewed the patched files you attached, and converted them to a patch for 1.9.4+. Can you please test it, as I rather lack in AICC test material, and it would be good to have a second opinion.

Cheers,
Piers Harding.

Show
Piers Harding added a comment - Hi Martin - I have reviewed the patched files you attached, and converted them to a patch for 1.9.4+. Can you please test it, as I rather lack in AICC test material, and it would be good to have a second opinion. Cheers, Piers Harding.
Hide
Permalink
Martin Holden added a comment -

Piers

Apologies for slow response, after such prompt work on your side.

I have tested locally and all seems okay, but our official QA team are looking to go through a test cycle later this week to give their sign-off as sson as it is done I will let you know.

Thanks

Martin

Show
Martin Holden added a comment - Piers Apologies for slow response, after such prompt work on your side. I have tested locally and all seems okay, but our official QA team are looking to go through a test cycle later this week to give their sign-off as sson as it is done I will let you know. Thanks Martin
Hide
Permalink
Martin Holden added a comment -

Piers

I found one more change that I think I recommend, it is actually a workaround to a bug in one of our legacy course types that we can't change - BUT is also probably a good recommendation based on the AICC guidelines.

The issue is to do with handling of lesson_status.

Our legacy course sends a status of Complete - rather than completed.

However in the AICC guidelines its defines that the format of lesson_status is:
"A word, character or phrase, optionally followed by a comma and one additional word or character. Only the first character is significant in each word."

So strictly speaking we adhere to guidelines - I know its a point of interpretation etc.

The fix is relatively easy and I was wondering if you would incorporate it in the patch.

File: AICC.PHP including pathc file here
Start Line: 201

Replace current code (with this):

if (count($values) > 1) {
$value = trim(strtolower($values[1]));

//Martin Holden 24-2-2009
//Just check the first characters from the left $value[0]
$value = $value[0];

if (isset($exites[$value])) { $value = $exites[$value]; }
}

if (empty($value) || isset($exites[$value])) {
$subelement = 'cmi.core.exit';
$id = scorm_insert_track($USER->id, $scorm->id, $sco->id, $attempt, $subelement, $value);
}

$value = trim(strtolower($values[0]));

//Martin Holden 24-2-2009
//Just check the first characters from the left $value[0]
$value = $value[0];

if (isset($statuses[$value]) && ($mode == 'normal')) {
$value = $statuses[$value];
$id = scorm_insert_track($USER->id, $scorm->id, $sco->id, $attempt, $element, $value);
}
$lessonstatus = $value;

Thanks

Show
Martin Holden added a comment - Piers I found one more change that I think I recommend, it is actually a workaround to a bug in one of our legacy course types that we can't change - BUT is also probably a good recommendation based on the AICC guidelines. The issue is to do with handling of lesson_status. Our legacy course sends a status of Complete - rather than completed. However in the AICC guidelines its defines that the format of lesson_status is: "A word, character or phrase, optionally followed by a comma and one additional word or character. Only the first character is significant in each word." So strictly speaking we adhere to guidelines - I know its a point of interpretation etc. The fix is relatively easy and I was wondering if you would incorporate it in the patch. File: AICC.PHP including pathc file here Start Line: 201 Replace current code (with this): if (count($values) > 1) { $value = trim(strtolower($values[1])); //Martin Holden 24-2-2009 //Just check the first characters from the left $value[0] $value = $value[0]; if (isset($exites[$value])) { $value = $exites[$value]; } } if (empty($value) || isset($exites[$value])) { $subelement = 'cmi.core.exit'; $id = scorm_insert_track($USER->id, $scorm->id, $sco->id, $attempt, $subelement, $value); } $value = trim(strtolower($values[0])); //Martin Holden 24-2-2009 //Just check the first characters from the left $value[0] $value = $value[0]; if (isset($statuses[$value]) && ($mode == 'normal')) { $value = $statuses[$value]; $id = scorm_insert_track($USER->id, $scorm->id, $sco->id, $attempt, $element, $value); } $lessonstatus = $value; Thanks
Hide
Permalink
Martin Holden added a comment -

Piers

Apologies two more issues (again one related to a bug on SkillSoft Simulation content) and one actually in the aicc.php code.

1. Request this change that is due to a bug on our side, but will not affect AICC compliance.
Our Simulation player can not handle whitespace around the = signs in the AICC responses. Moodle is "correct" but implementing this change will not affect the operation of Moodle with other AICC content and so I am requesting it.

Basically everywhere that the AICC data is return in the getparam remove leading and trailing spaces around the = sign

2. Issue in ExitAU code that means an AICC response is never sent.
In the branch fro processing the ExitAU we have a call to:

scorm_update_grades($scorm, $USER->id);

This function is in the include file lib.php BUT this is not included and so the page "crashes" and never reaches the point where it will send the response for the ExitAU.

The fix is to place this line before the scorm_update_grades($scorm, $USER->id);

include_once('lib.php'); <-----------------
scorm_update_grades($scorm, $USER->id);

I have created a patch file based on my WINDOWS install using WinMerge I hope this is usable, I am not an expert on the use of WinMerge and Patch etc

Thanks

Martin

Show
Martin Holden added a comment - Piers Apologies two more issues (again one related to a bug on SkillSoft Simulation content) and one actually in the aicc.php code. 1. Request this change that is due to a bug on our side, but will not affect AICC compliance. Our Simulation player can not handle whitespace around the = signs in the AICC responses. Moodle is "correct" but implementing this change will not affect the operation of Moodle with other AICC content and so I am requesting it. Basically everywhere that the AICC data is return in the getparam remove leading and trailing spaces around the = sign 2. Issue in ExitAU code that means an AICC response is never sent. In the branch fro processing the ExitAU we have a call to: scorm_update_grades($scorm, $USER->id); This function is in the include file lib.php BUT this is not included and so the page "crashes" and never reaches the point where it will send the response for the ExitAU. The fix is to place this line before the scorm_update_grades($scorm, $USER->id); include_once('lib.php'); <----------------- scorm_update_grades($scorm, $USER->id); I have created a patch file based on my WINDOWS install using WinMerge I hope this is usable, I am not an expert on the use of WinMerge and Patch etc Thanks Martin
Hide
Permalink
Piers Harding added a comment -

2nd attempt - aicc patches

Show
Piers Harding added a comment - 2nd attempt - aicc patches
Hide
Permalink
Piers Harding added a comment -

Hi Martin -

Thanks for the testing.

I have created a new patch including all 3 changes you have noted above (+ the original), and would be greateful if you could test/review.

File: aicc_stable19.patch

Cheers,
Piers Harding

Show
Piers Harding added a comment - Hi Martin - Thanks for the testing. I have created a new patch including all 3 changes you have noted above (+ the original), and would be greateful if you could test/review. File: aicc_stable19.patch Cheers, Piers Harding
Hide
Permalink
Martin Holden added a comment -

Piers

Pleased to confirm our QA team have run through all our normal tracking tests and the patch works perfectly.

Just for completeness the test used:

1. Base Install of 1.9.4+ (CVS download of MOODLE_19_WEEKLY yesterday)
2. Applied patch aicc_stable19.patch from this Tracker.

Can we look at getting this patch rolled into the MOODLE_19_WEEKLY release vehicle now?

Thanks

Martin

Show
Martin Holden added a comment - Piers Pleased to confirm our QA team have run through all our normal tracking tests and the patch works perfectly. Just for completeness the test used: 1. Base Install of 1.9.4+ (CVS download of MOODLE_19_WEEKLY yesterday) 2. Applied patch aicc_stable19.patch from this Tracker. Can we look at getting this patch rolled into the MOODLE_19_WEEKLY release vehicle now? Thanks Martin
Hide
Permalink
Piers Harding added a comment -

Hi Martin -

I have forward ported this to 2.0, and backported to 1.8 as well as commiting to 1.9. It should all be in the respective weekly builds by the end of next week.

As I had to rework the patch for 2.0, would you beable to run your test suite against this, just to be sure?

Thanks,
Piers Harding.

Show
Piers Harding added a comment - Hi Martin - I have forward ported this to 2.0, and backported to 1.8 as well as commiting to 1.9. It should all be in the respective weekly builds by the end of next week. As I had to rework the patch for 2.0, would you beable to run your test suite against this, just to be sure? Thanks, Piers Harding.
Hide
Permalink
Petr Škoda (skodak) added a comment -

I just noticed stripslashes() in latest HEAD commit, I doubt we need that anymore

Show
Petr Škoda (skodak) added a comment - I just noticed stripslashes() in latest HEAD commit, I doubt we need that anymore
Hide
Permalink
Piers Harding added a comment -

OK - thanks Petr. I'm going to create a new tracker as I think there is a largish task to sort out the addslashes/stripslashes situation in mod/scorm.

Cheers.

Show
Piers Harding added a comment - OK - thanks Petr. I'm going to create a new tracker as I think there is a largish task to sort out the addslashes/stripslashes situation in mod/scorm. Cheers.
Hide
Permalink
Martin Holden added a comment -

Piers

Just looked at the 1.29 checkin of AICC.PHP you did and have a couple of comments:

1. Your checking still has the line formatting the output as plain text, this needs to be removed as per the patch file (aicc_stable19.patch)

Two I am not sure about?

2. In the ExitAU calls you dont create the $track object, in the old 1.21 patch we explicitly create a new object $track = new object(); in the else block
3. In the ExitAU calls you dont call scorm_update_grades($scorm, $USER->id);

Thanks

Martin

Show
Martin Holden added a comment - Piers Just looked at the 1.29 checkin of AICC.PHP you did and have a couple of comments: 1. Your checking still has the line formatting the output as plain text, this needs to be removed as per the patch file (aicc_stable19.patch) Two I am not sure about? 2. In the ExitAU calls you dont create the $track object, in the old 1.21 patch we explicitly create a new object $track = new object(); in the else block 3. In the ExitAU calls you dont call scorm_update_grades($scorm, $USER->id); Thanks Martin
Hide
Permalink
Piers Harding added a comment -

Added missing lines as per:

2. In the ExitAU calls you dont create the $track object, in the old 1.21 patch we explicitly create a new object $track = new object(); in the else block
3. In the ExitAU calls you dont call scorm_update_grades($scorm, $USER->id);

header() call removed on prior commit.

Cheers.

Show
Piers Harding added a comment - Added missing lines as per: 2. In the ExitAU calls you dont create the $track object, in the old 1.21 patch we explicitly create a new object $track = new object(); in the else block 3. In the ExitAU calls you dont call scorm_update_grades($scorm, $USER->id); header() call removed on prior commit. Cheers.
Hide
Permalink
Dan Marsden added a comment -

marking this as fixed - let us know if there's anything we've missed!

Thanks to Martin for all the help!

Show
Dan Marsden added a comment - marking this as fixed - let us know if there's anything we've missed! Thanks to Martin for all the help!

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.