Moodle
  1. Moodle
  2. MDL-18148

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

    Details

    • Type: Bug Bug
    • Status: 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
    • Rank:
      31545

      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

      1. aicc_stable19.patch
        21 kB
        Piers Harding
      2. aicc.patch
        20 kB
        Piers Harding
      3. aicc2.patch
        29 kB
        Martin Holden

        Issue Links

          Activity

          Hide
          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
          Piers Harding added a comment -

          AICC patches for 1.9

          Show
          Piers Harding added a comment - AICC patches for 1.9
          Hide
          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
          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
          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
          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
          Piers Harding added a comment -

          2nd attempt - aicc patches

          Show
          Piers Harding added a comment - 2nd attempt - aicc patches
          Hide
          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
          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
          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
          Petr Škoda added a comment -

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

          Show
          Petr Škoda added a comment - I just noticed stripslashes() in latest HEAD commit, I doubt we need that anymore
          Hide
          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
          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
          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
          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

            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: