Moodle
  1. Moodle
  2. MDL-31039

Unable to update SCORM activity with External AICC URL

    Details

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

      go to admin > plugins > activity modules > SCORM
      make sure the setting "Enable direct AICC url" is set to "yes"

      Create a new SCORM in your site - select the scormtype "external AICC URL" and use the following url:
      https://secure.testcraft.com/dev7/Assess.aspx?aid=MOODLE-AICC-01&apass=PASSWORD123

      Hit Save and return to course - it doesn't matter if the SCORM/AICC actually loads or not in your player, it definitely won't if your site isn't publicly accessible - but ignore this as it isn't relevant to this test.

      Update the SCORM activity, check to make sure the URL entered above is still there.
      Save and return to course'

      Update the SCORM activity again, check to make sure the URL is still there after update.

      Show
      go to admin > plugins > activity modules > SCORM make sure the setting "Enable direct AICC url" is set to "yes" Create a new SCORM in your site - select the scormtype "external AICC URL" and use the following url: https://secure.testcraft.com/dev7/Assess.aspx?aid=MOODLE-AICC-01&apass=PASSWORD123 Hit Save and return to course - it doesn't matter if the SCORM/AICC actually loads or not in your player, it definitely won't if your site isn't publicly accessible - but ignore this as it isn't relevant to this test. Update the SCORM activity, check to make sure the URL entered above is still there. Save and return to course' Update the SCORM activity again, check to make sure the URL is still there after update.
    • Workaround:
      Hide

      I found 2 places where the new SCORM_TYPE_AICCURL scormtype was missing.

      In mod_form.php in the function set_data($default_values):
      from line 396 SCORM_TYPE_AICCURL should be added to the switch statement (this fixes the empty URL field):
      switch ($default_values['scormtype'])

      { case SCORM_TYPE_LOCALSYNC : case SCORM_TYPE_EXTERNAL: case SCORM_TYPE_IMSREPOSITORY: case SCORM_TYPE_AICCURL: <== fix for empty URL field $default_values['packageurl'] = $default_values['reference']; }

      In lib.php in the function scorm_update_instance($scorm, $mform=null):
      an extra else if needs to be added (like in the function scorm_add_instance). This fixes the update error.
      } else if ($scorm->scormtype === SCORM_TYPE_AICCURL) {
      $scorm->reference = $scorm->packageurl;

      Show
      I found 2 places where the new SCORM_TYPE_AICCURL scormtype was missing. In mod_form.php in the function set_data($default_values): from line 396 SCORM_TYPE_AICCURL should be added to the switch statement (this fixes the empty URL field): switch ($default_values ['scormtype'] ) { case SCORM_TYPE_LOCALSYNC : case SCORM_TYPE_EXTERNAL: case SCORM_TYPE_IMSREPOSITORY: case SCORM_TYPE_AICCURL: <== fix for empty URL field $default_values['packageurl'] = $default_values['reference']; } In lib.php in the function scorm_update_instance($scorm, $mform=null): an extra else if needs to be added (like in the function scorm_add_instance). This fixes the update error. } else if ($scorm->scormtype === SCORM_TYPE_AICCURL) { $scorm->reference = $scorm->packageurl;
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE
    • Pull Master Branch:
      master_MDL-31039
    • Rank:
      37447

      Description

      When trying to update a SCORM activity with an External AICC URL the URL input field is empty.
      Saving the form returns "ERROR: Could not update scorm" and the URL is not updated.

        Activity

        Hide
        Dan Marsden added a comment -

        thanks for the detailed report - I'll try to take a look at this soon.

        Show
        Dan Marsden added a comment - thanks for the detailed report - I'll try to take a look at this soon.
        Hide
        Dan Marsden added a comment -

        wow - that was incredibly stupid! - thanks for the report/fix!! - submitting this for integration.

        Show
        Dan Marsden added a comment - wow - that was incredibly stupid! - thanks for the report/fix!! - submitting this for integration.
        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
        Eloy Lafuente (stronk7) added a comment -

        Integrated (21, 22 & master), thanks!

        Show
        Eloy Lafuente (stronk7) added a comment - Integrated (21, 22 & master), thanks!
        Hide
        Ankit Agarwal added a comment -

        Hi,
        This works great in 22 and master.
        But I dont think we had Direct url support for AICC before 22
        Reference:- http://tracker.moodle.org/browse/MDL-30146

        Eloy you might want to revert the commit on 21 if am correct.
        Marking this as failed till the commit on 21 is reverted.
        Thanks

        Show
        Ankit Agarwal added a comment - Hi, This works great in 22 and master. But I dont think we had Direct url support for AICC before 22 Reference:- http://tracker.moodle.org/browse/MDL-30146 Eloy you might want to revert the commit on 21 if am correct. Marking this as failed till the commit on 21 is reverted. Thanks
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Aha, well spotted! In fact I was talking with Dan about this (if I'm not wrong) and 21, but obviously we missed the point completely, thanks!

        Reverting the 21_STABLE commit...

        Show
        Eloy Lafuente (stronk7) added a comment - Aha, well spotted! In fact I was talking with Dan about this (if I'm not wrong) and 21, but obviously we missed the point completely, thanks! Reverting the 21_STABLE commit...
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Done, I've reverted the commit in 21_STABLE. So this is now only 22 and master. Thanks Ankit!

        Show
        Eloy Lafuente (stronk7) added a comment - Done, I've reverted the commit in 21_STABLE. So this is now only 22 and master. Thanks Ankit!
        Hide
        Eloy Lafuente (stronk7) added a comment -

        (sent to another testing round)

        Show
        Eloy Lafuente (stronk7) added a comment - (sent to another testing round)
        Hide
        Ankit Agarwal added a comment -

        passing since the commit is now reverted for 21
        Thanks

        Show
        Ankit Agarwal added a comment - passing since the commit is now reverted for 21 Thanks
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Many thanks for your collaboration, this code has been integrated upstream and it's available in all the repositories.

        Closing, ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Many thanks for your collaboration, this code has been integrated upstream and it's available in all the repositories. Closing, ciao

          People

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

            Dates

            • Created:
              Updated:
              Resolved: