Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-41476

SCORM - increase 1.2 datamodel lengths

    Details

    • Type: New Feature
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.6
    • Fix Version/s: 2.7
    • Component/s: SCORM
    • Labels:
    • Testing Instructions:
      Hide

      Log as Administrator and activate SCORM API Debugging
      Add this SCORM package to a course: https://tracker.moodle.org/secure/attachment/36979/Golf%20Example.zip

      Test 1:

      1. Make sure the checkbox admin > plugins > activities > SCORM "scorm12standard" is ticked (enabled by default)
      2. Enter the SCORM activity (a new popup|tab will be opened too with the SCORM API Activity Log)
      3. On the SCORM API Debugger window
      4. Selec the datamodel element cmi.suspend_data and set some text bigger than 4096 characters and hit the LMSSetValue() button.
      5. The data should appear in red with the number "405" at the end.

      Test 2:

      1. Make sure the checkbox admin > plugins > activities > SCORM "scorm12standard" is unticked
      2. Clear all caches - browser and Moodle (just to be safe)
      3. Delete all Attempts to this SCORM before entering.
      4. Enter the SCORM activity (a new popup|tab will be opened too with the SCORM API Activity Log)
      5. On the SCORM API Debugger window
      6. Select the datamodel element cmi.suspend_data and set some text bigger than 4096 characters and hit the LMSSetValue() button.
      7. The data should appear in black as a normal succesful save with the number "0" (no error) at the end.
      Show
      Log as Administrator and activate SCORM API Debugging Add this SCORM package to a course: https://tracker.moodle.org/secure/attachment/36979/Golf%20Example.zip Test 1: Make sure the checkbox admin > plugins > activities > SCORM "scorm12standard" is ticked (enabled by default) Enter the SCORM activity (a new popup|tab will be opened too with the SCORM API Activity Log) On the SCORM API Debugger window Selec the datamodel element cmi.suspend_data and set some text bigger than 4096 characters and hit the LMSSetValue() button. The data should appear in red with the number "405" at the end. Test 2: Make sure the checkbox admin > plugins > activities > SCORM "scorm12standard" is unticked Clear all caches - browser and Moodle (just to be safe) Delete all Attempts to this SCORM before entering. Enter the SCORM activity (a new popup|tab will be opened too with the SCORM API Activity Log) On the SCORM API Debugger window Select the datamodel element cmi.suspend_data and set some text bigger than 4096 characters and hit the LMSSetValue() button. The data should appear in black as a normal succesful save with the number "0" (no error) at the end.
    • Affected Branches:
      MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_27_STABLE
    • Pull Master Branch:

      Description

      SCORM 1.2 spec states that suspend_data should be no more than 4096 characters - problem is that many SCORM authoring tools create packages that go over this limit and fail in various ways. (SCORM 2004 spec allows a higher number of chars)

      People have previously published their packages as SCORM 2004 to get around this limit but as we are no longer continuing to develop SCORM 2004 in Moodle I think it would be useful to add a setting for people to allow 1.2 packages to store more data.

      We also enforce 1.2 limits on other areas like student response:
      https://moodle.org/mod/forum/discuss.php?d=216092

      I'd like to add an admin level setting "SCORM 1.2 standards mode" that is enabled by default (eg compliant with the standard) - if people want to allow their packages to break the standard they can untick the box. - unticking this box would increase the CMIString4096 and CMIString256 in mod/scorm/datamodels/scorm_12.js.php to a higher value.

        Gliffy Diagrams

          Activity

          Hide
          hughedwards Hugh Edwards added a comment -

          Hi Dan.

          This def affects me, so I'd like to volunteer to do any testing you may need and help where I can.

          Hugh.

          Show
          hughedwards Hugh Edwards added a comment - Hi Dan. This def affects me, so I'd like to volunteer to do any testing you may need and help where I can. Hugh.
          Hide
          nobelium vignesh p added a comment - - edited

          Hi Dan,
          I'm working on this issue right now. So far I have
          1. Added a row to config_plugins table ('scorm12standard'=>'1')
          2. Added a field(checkbox) in SCORM admin page
          3. Written update script in mod/scorm/db/upgrade.php
          4. Change CMIString length in mod/scorm/datamodels/scorm_12.js.php
          (CMIString256 to 4096 & CMIString to 16384 if scorm12standard is '0')
          (CMIString256 to 256 & CMIString to 4096 if scorm12standard is '1')

          Please take a look at my repository.https://github.com/nobelium/moodle/compare/MDL-41476
          Let me know if I have missed something.

          Show
          nobelium vignesh p added a comment - - edited Hi Dan, I'm working on this issue right now. So far I have 1. Added a row to config_plugins table ('scorm12standard'=>'1') 2. Added a field(checkbox) in SCORM admin page 3. Written update script in mod/scorm/db/upgrade.php 4. Change CMIString length in mod/scorm/datamodels/scorm_12.js.php (CMIString256 to 4096 & CMIString to 16384 if scorm12standard is '0') (CMIString256 to 256 & CMIString to 4096 if scorm12standard is '1') Please take a look at my repository. https://github.com/nobelium/moodle/compare/MDL-41476 Let me know if I have missed something.
          Hide
          nobelium vignesh p added a comment -

          Hi Hugh, Can you help me with testing the patch on my repository?

          Show
          nobelium vignesh p added a comment - Hi Hugh, Can you help me with testing the patch on my repository?
          Hide
          danmarsden Dan Marsden added a comment -

          Hi Vignesh,

          looks like a great start - thanks for working on this!

          Couple of comments:
          Can you please add a description of the new setting that explains what it does (3rd param of admin_setting_configcheckbox) something like
          "Disabling this setting will allow Moodle to store more data than the SCORM 1.2 specification allows. If your SCORM packages allow users to enter large amounts of text or if your package tries to store large amounts of data in the suspend_data field disabling this setting can help."

          lets use the same limits for the SCORM 2004 CMIString64000 for both 256 and 4096:

          '^[\\u0000-\\uFFFF]{0,64000}$';

          Can you also add a comment in scorm12.js.php before line 33 something like:
          // If SCORM standards mode is disabled allow higher datamodel limits.

          Side note (only for reference, you don't need to do anything here)
          In the long term I'd like to separate the PHP from the javascript in the scorm12.js.php file - the scorm12/2004 .js.php files are the only javascript files generated in this way in Moodle so I'm trying to keep all the PHP at the top and use javascript parameters instead of inline JS - I've started some of this work in a patch on MDL-35870 - long term I'd like the files to be pure JS so they can be cached by the browser.

          Show
          danmarsden Dan Marsden added a comment - Hi Vignesh, looks like a great start - thanks for working on this! Couple of comments: Can you please add a description of the new setting that explains what it does (3rd param of admin_setting_configcheckbox) something like "Disabling this setting will allow Moodle to store more data than the SCORM 1.2 specification allows. If your SCORM packages allow users to enter large amounts of text or if your package tries to store large amounts of data in the suspend_data field disabling this setting can help." lets use the same limits for the SCORM 2004 CMIString64000 for both 256 and 4096: '^[\\u0000-\\uFFFF]{0,64000}$'; Can you also add a comment in scorm12.js.php before line 33 something like: // If SCORM standards mode is disabled allow higher datamodel limits. Side note (only for reference, you don't need to do anything here) In the long term I'd like to separate the PHP from the javascript in the scorm12.js.php file - the scorm12/2004 .js.php files are the only javascript files generated in this way in Moodle so I'm trying to keep all the PHP at the top and use javascript parameters instead of inline JS - I've started some of this work in a patch on MDL-35870 - long term I'd like the files to be pure JS so they can be cached by the browser.
          Hide
          nobelium vignesh p added a comment -

          Hi Dan,
          I have changed the limits of CMIStrings and I have added a description for the new setting. I have also added a comment before the if statement. I'm not sure about the update script can you please check on it?

          About separation of JS and php: I think I saw a ticket about it. I'm willing to work on it. It would be wise to put those php variables into a Global JS variable and the JS functions can get the values from the global variable.

          Show
          nobelium vignesh p added a comment - Hi Dan, I have changed the limits of CMIStrings and I have added a description for the new setting. I have also added a comment before the if statement. I'm not sure about the update script can you please check on it? About separation of JS and php: I think I saw a ticket about it. I'm willing to work on it. It would be wise to put those php variables into a Global JS variable and the JS functions can get the values from the global variable.
          Hide
          hughedwards Hugh Edwards added a comment -

          Hi Vignesh. I can indeed - please let me know what I need to do? Thanks!

          Show
          hughedwards Hugh Edwards added a comment - Hi Vignesh. I can indeed - please let me know what I need to do? Thanks!
          Hide
          danmarsden Dan Marsden added a comment -

          we're close - couple of things:
          PHP var names in Moodle are supposed to be all lower case ($cmistring not $CMIstring)

          I personally prefer declarations to be one per line:
          var = var = string
          instead use something like:
          var = string
          var = var

          as it just reads slightly better to me but that's not a hard rule and I couldn't find it mentioned in our coding style. (there is probably existing Moodle code that does it the same way you have.)

          So - if you could just fix up the parameter names to be lower case (and move them to 2 lines to keep me happy) we can push this up for integration!

          thanks!

          Show
          danmarsden Dan Marsden added a comment - we're close - couple of things: PHP var names in Moodle are supposed to be all lower case ($cmistring not $CMIstring) I personally prefer declarations to be one per line: var = var = string instead use something like: var = string var = var as it just reads slightly better to me but that's not a hard rule and I couldn't find it mentioned in our coding style. (there is probably existing Moodle code that does it the same way you have.) So - if you could just fix up the parameter names to be lower case (and move them to 2 lines to keep me happy) we can push this up for integration! thanks!
          Hide
          nobelium vignesh p added a comment -

          Hi Dan, I have edited the variable names and moved the assignment to two lines. The branch is ready!

          Show
          nobelium vignesh p added a comment - Hi Dan, I have edited the variable names and moved the assignment to two lines. The branch is ready!
          Hide
          nobelium vignesh p added a comment - - edited

          Hi Hugh, can you pull from the branch that I have mentioned (or apply this patch https://github.com/nobelium/moodle/compare/MDL-41476.diff) add a row to config_plugins table('scorm12standard'=>'1') and test it using your scorm package where you have had this problem.

          Show
          nobelium vignesh p added a comment - - edited Hi Hugh, can you pull from the branch that I have mentioned (or apply this patch https://github.com/nobelium/moodle/compare/MDL-41476.diff ) add a row to config_plugins table('scorm12standard'=>'1') and test it using your scorm package where you have had this problem.
          Hide
          danmarsden Dan Marsden added a comment -

          Just realised I missed your comment about the update script - you don't actually need the code in mod/scorm/db/upgrade.php as during an upgrade all new settings are picked up and presented to the admin for setting.

          Side note for info on upgrade scripts just for future reference...
          for that upgrade script to be triggered you would also need to update $plugin->version = 2014030900 in mod/scorm/version.php

          thanks!

          Show
          danmarsden Dan Marsden added a comment - Just realised I missed your comment about the update script - you don't actually need the code in mod/scorm/db/upgrade.php as during an upgrade all new settings are picked up and presented to the admin for setting. Side note for info on upgrade scripts just for future reference... for that upgrade script to be triggered you would also need to update $plugin->version = 2014030900 in mod/scorm/version.php thanks!
          Hide
          nobelium vignesh p added a comment -

          Hi Dan, I have reset all the changes done to upgrade.php n thanks for the tip on upgrade script. Should this commit be pushed to v2.5 n v2.6 ?

          Show
          nobelium vignesh p added a comment - Hi Dan, I have reset all the changes done to upgrade.php n thanks for the tip on upgrade script. Should this commit be pushed to v2.5 n v2.6 ?
          Hide
          danmarsden Dan Marsden added a comment -

          Great! - thanks for working on this! - it is classed as an improvement/feature so it only goes in master. Stable releases are only supported for bug fixes/security fixes so any improvement or feature work only occurs in the master branch.

          We do need to add some testing instructions here to allow the QA team to perform testing after the commit - we need to add some instructions similar to the ones shown at MDL-38678 - I'll try to do this later this week if you don't beat me to it.

          the testing instructions should state what should occur when the patch has been applied - with a test for entering text over the limits with the setting turned on and then the setting turned off and should describe what messages should appear in the debugging log in each case.

          Show
          danmarsden Dan Marsden added a comment - Great! - thanks for working on this! - it is classed as an improvement/feature so it only goes in master. Stable releases are only supported for bug fixes/security fixes so any improvement or feature work only occurs in the master branch. We do need to add some testing instructions here to allow the QA team to perform testing after the commit - we need to add some instructions similar to the ones shown at MDL-38678 - I'll try to do this later this week if you don't beat me to it. the testing instructions should state what should occur when the patch has been applied - with a test for entering text over the limits with the setting turned on and then the setting turned off and should describe what messages should appear in the debugging log in each case.
          Hide
          danmarsden Dan Marsden added a comment -

          bouncing up for integration - Thank Vignesh!

          Show
          danmarsden Dan Marsden added a comment - bouncing up for integration - Thank Vignesh!
          Hide
          cibot CiBoT added a comment -

          Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!

          Show
          cibot CiBoT added a comment - Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!
          Hide
          marina Marina Glancy added a comment -

          Thanks Vignesh and Dan, integrated in master

          Show
          marina Marina Glancy added a comment - Thanks Vignesh and Dan, integrated in master
          Hide
          nobelium vignesh p added a comment -

          Thanks Marina.
          @Dan, @Marina : Should this feature be documented some where? or is it enough to post it in the moodle forum?

          Show
          nobelium vignesh p added a comment - Thanks Marina. @Dan, @Marina : Should this feature be documented some where? or is it enough to post it in the moodle forum?
          Hide
          danmarsden Dan Marsden added a comment -

          Thanks Vignesh, yes we will need to add something about this in the docs when the 2.7 docs are generated (3 weeks prior to release)
          http://docs.moodle.org/dev/Release_process_%28Combined%29#3_weeks_prior

          Show
          danmarsden Dan Marsden added a comment - Thanks Vignesh, yes we will need to add something about this in the docs when the 2.7 docs are generated (3 weeks prior to release) http://docs.moodle.org/dev/Release_process_%28Combined%29#3_weeks_prior
          Hide
          nobelium vignesh p added a comment -

          Thanks for the info Dan.

          Show
          nobelium vignesh p added a comment - Thanks for the info Dan.
          Hide
          ankit_frenz Ankit Agarwal added a comment -

          Works as described, error codes were noticed as indicated.

          Show
          ankit_frenz Ankit Agarwal added a comment - Works as described, error codes were noticed as indicated.
          Hide
          poltawski Dan Poltawski added a comment -

          Thanks for your efforts, this change is now part of Moodle!

          Show
          poltawski Dan Poltawski added a comment - Thanks for your efforts, this change is now part of Moodle!
          Hide
          marycooch Mary Cooch added a comment -

          Removing docs_required label as I have added information about the setting to https://docs.moodle.org/27/en/SCORM_settings

          Show
          marycooch Mary Cooch added a comment - Removing docs_required label as I have added information about the setting to https://docs.moodle.org/27/en/SCORM_settings
          Hide
          danmarsden Dan Marsden added a comment -

          Thanks Mary!

          Show
          danmarsden Dan Marsden added a comment - Thanks Mary!

            People

            • Votes:
              2 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                12/May/14