Moodle
  1. Moodle
  2. MDL-31652

SCORM updatefreq setting shouldn't allow use when only uploaded files being used

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1, 2.2
    • Fix Version/s: 2.1.5, 2.2.2
    • Component/s: SCORM
    • Labels:
    • Testing Instructions:
      Hide

      In Admin > plugins > activity modules > SCORM
      make sure the following items are disabled:
      allowtypeexternal
      allowtypelocalsync
      allowtypeimsrepository
      allowtypeexternalaicc

      Then go to a course and create a New SCORM and check the settings to make sure the updatefreq setting doesn't appear (make sure you hit the show advanced setting)

      Now go back to:
      In Admin > plugins > activity modules > SCORM
      enable some of the settings you checked before:
      allowtypeexternal
      allowtypelocalsync
      allowtypeimsrepository
      allowtypeexternalaicc

      Then go to a course and create a new SCORM and check the settings to make sure the updatefreq setting appears now and is disabled when the "type" selector (appears above the filepicker) is set to "uploaded package" but if any other type is selected it is enabled.

      also check to make sure the old item "onchanges" option in the updatefreq list no longer appears.

      Show
      In Admin > plugins > activity modules > SCORM make sure the following items are disabled: allowtypeexternal allowtypelocalsync allowtypeimsrepository allowtypeexternalaicc Then go to a course and create a New SCORM and check the settings to make sure the updatefreq setting doesn't appear (make sure you hit the show advanced setting) Now go back to: In Admin > plugins > activity modules > SCORM enable some of the settings you checked before: allowtypeexternal allowtypelocalsync allowtypeimsrepository allowtypeexternalaicc Then go to a course and create a new SCORM and check the settings to make sure the updatefreq setting appears now and is disabled when the "type" selector (appears above the filepicker) is set to "uploaded package" but if any other type is selected it is enabled. also check to make sure the old item "onchanges" option in the updatefreq list no longer appears.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull Master Branch:
      master_MDL-31652
    • Rank:
      38223

      Description

      SCORM has a setting that allows the package in Moodle to be updated based on an external file - so if SCORM links to a zip hosted outside Moodle or a manifest file stored somewhere else the teacher can modify that content and have it automatically updated in Moodle.

      But - this setting still shows (and performs unnecessary work) when a teacher uploads a scorm - we should hide the setting completely if external options aren't enabled and we should disable the field if others are enabled and user uploads normally.

      We should also update the help strings/docs.

        Issue Links

          Activity

          Hide
          Dan Marsden added a comment -

          also - something in the 2.x upgrade re-introduced the onchanges freq type which doesn't do anything and was removed in MDL-15881

          Show
          Dan Marsden added a comment - also - something in the 2.x upgrade re-introduced the onchanges freq type which doesn't do anything and was removed in MDL-15881
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Dan Marsden added a comment -

          rebased

          Show
          Dan Marsden added a comment - rebased
          Hide
          Sam Hemelryk added a comment -

          Thanks Dan, I've just integrated these changes now

          Show
          Sam Hemelryk added a comment - Thanks Dan, I've just integrated these changes now
          Hide
          Rossiani Wijaya added a comment -

          Hi Dan,

          The test passed for 2.2 and master.

          However there is regression for M2.1, there is a missing property for $upgrade_freq when these options are enable: allowtypeexternal
          allowtypelocalsync, and allowtypeimsrepository.

          Notice: Undefined property: stdClass::$updatefreq_adv in /im21/mod/scorm/mod_form.php on line 219

          Failing this to get it fix.

          Show
          Rossiani Wijaya added a comment - Hi Dan, The test passed for 2.2 and master. However there is regression for M2.1, there is a missing property for $upgrade_freq when these options are enable: allowtypeexternal allowtypelocalsync, and allowtypeimsrepository. Notice: Undefined property: stdClass::$updatefreq_adv in /im21/mod/scorm/mod_form.php on line 219 Failing this to get it fix.
          Hide
          Dan Marsden added a comment -

          Thanks Rosie - good spotting... have pushed a commit to the 21stable branch above with a fix.....

          I keep forgetting to test 2.1 branches before I submit - will be good when we can drop support for it....

          Show
          Dan Marsden added a comment - Thanks Rosie - good spotting... have pushed a commit to the 21stable branch above with a fix..... I keep forgetting to test 2.1 branches before I submit - will be good when we can drop support for it....
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The extra commit taking rid of 'updatefreq_adv' has been added to 21_STABLE. No changes in other branches.

          So this can be re-tested under 21_STABLE... ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The extra commit taking rid of 'updatefreq_adv' has been added to 21_STABLE. No changes in other branches. So this can be re-tested under 21_STABLE... ciao
          Hide
          Rossiani Wijaya added a comment -

          Thanks for updating Dan and Eloy.

          This is working great now.

          Test passed.

          Show
          Rossiani Wijaya added a comment - Thanks for updating Dan and Eloy. This is working great now. Test passed.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Some changes to Moodle should be milestones in the project by themselves.

          This is not the case and your fix is not so important, but your collaboration is highly appreciated, thanks!

          Closing as fixed, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Some changes to Moodle should be milestones in the project by themselves. This is not the case and your fix is not so important, but your collaboration is highly appreciated, thanks! Closing as fixed, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: