Moodle
  1. Moodle
  2. MDL-21761

Data missing in SCORM datamodel elements under some circumstances

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 1.9.7
    • Fix Version/s: 1.9.10
    • Component/s: SCORM
    • Labels:
      None
    • Environment:
      LAMP, PHP 5.2.12
    • Database:
      MySQL
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE
    • Rank:
      26824

      Description

      Scenario:

      • SCORM 1.2 course;
      • Single attempt allowed.
        If the content has already been launched and e.g. the status of an item is "failed", at launch time the "failed" status is the default value. Suppose that during the life cycle of the tracking session the status is set to:
      • incomplete + LMSCommit() => transferred into the DB
      • failed + LMSCommit() => no transfer at all into the DB
        At the end of the session (LMSFinish()) the status of the item is "incomplete" instead of the expected value, "failed".

      The fault is within the architecture of the datamodel object, as defined in datamodels/scorm_12.js.php: it mixes default datamodel values with initialization values from past session. This causes a fault in the logic coded in CollectData() in which only differences against default values, when available, are exposed to the server for being stored into the DB.

        Issue Links

          Activity

          Hide
          Matteo Scaramuccia added a comment -

          Find below our patch proposal, to keep benefits of sending just differences but without any data corruption/missing as described above:

          @@ -557,6 +557,7 @@
                                       if ((typeof eval('datamodel["'+elementmodel+'"].defaultvalue')) != "undefined") {
                                           if (eval('datamodel["'+elementmodel+'"].defaultvalue') != data[property] || eval('typeof(datamodel["'+elementmodel+'"].defaultvalue)') != typeof(data[property])) {
                                               datastring += elementstring;
          +                                    eval('datamodel["'+elementmodel+'"].defaultvalue=data[property];');
                                           }
                                       } else {
                                           datastring += elementstring;
          
          Show
          Matteo Scaramuccia added a comment - Find below our patch proposal, to keep benefits of sending just differences but without any data corruption/missing as described above: @@ -557,6 +557,7 @@ if ((typeof eval('datamodel[ "'+elementmodel+'" ].defaultvalue')) != "undefined" ) { if (eval('datamodel[ "'+elementmodel+'" ].defaultvalue') != data[property] || eval('typeof(datamodel[ "'+elementmodel+'" ].defaultvalue)') != typeof(data[property])) { datastring += elementstring; + eval('datamodel[ "'+elementmodel+'" ].defaultvalue=data[property];'); } } else { datastring += elementstring;
          Hide
          Matteo Scaramuccia added a comment -

          Please note that the patch proposal doesn't correctly address situations derived from server/network issues on LMSCommit()/LMSFinish() calls. As described in MDL-17891 it could be possibile these calls to hang: in those cases we should revert the default values to the previous ones. Since this is not currently possible unless rewriting the two API functions from scratch using maybe something different from the current CollectData() function, we could accept the issue of possible data missing 'cause it's difficult to find content that has a nice remediation path on an LMSFinish() failure but saying the student "Unable to save data. Please contact your system administrator".

          Show
          Matteo Scaramuccia added a comment - Please note that the patch proposal doesn't correctly address situations derived from server/network issues on LMSCommit() / LMSFinish() calls. As described in MDL-17891 it could be possibile these calls to hang: in those cases we should revert the default values to the previous ones. Since this is not currently possible unless rewriting the two API functions from scratch using maybe something different from the current CollectData() function, we could accept the issue of possible data missing 'cause it's difficult to find content that has a nice remediation path on an LMSFinish() failure but saying the student "Unable to save data. Please contact your system administrator".
          Hide
          Piers Harding added a comment -

          Hi -

          Patched for both HEAD and 1.9. There is also a possibility of the same problem appearing in the "else" of the check outlined above.

          Cheers.

          Show
          Piers Harding added a comment - Hi - Patched for both HEAD and 1.9. There is also a possibility of the same problem appearing in the "else" of the check outlined above. Cheers.
          Hide
          Dan Marsden added a comment -

          Hi Matteo,

          I've assigned you as QA for this bug - can you please QA the commit and post back here? - as the QA Assignee you should be able to "close" this bug directly - link on the left hand side.

          thanks!

          Show
          Dan Marsden added a comment - Hi Matteo, I've assigned you as QA for this bug - can you please QA the commit and post back here? - as the QA Assignee you should be able to "close" this bug directly - link on the left hand side. thanks!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: