Moodle
  1. Moodle
  2. MDL-36107

Error when saving a SCORM activity

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.3.2
    • Fix Version/s: 2.3.3
    • Component/s: SCORM
    • Labels:
    • Testing Instructions:
      Hide

      Enable completion under site-wide advanced settings
      Create a new course, enable completion
      Add a scorm object to course
      Attempt scorm object
      View scorm object again and select "Delete all SCORM attempts", and the confirm

      Expected:
      Attempts deleted

      What happens:
      Fatal error occurs

      Show
      Enable completion under site-wide advanced settings Create a new course, enable completion Add a scorm object to course Attempt scorm object View scorm object again and select "Delete all SCORM attempts", and the confirm Expected: Attempts deleted What happens: Fatal error occurs
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
    • Rank:
      44873

      Description

      When saving any SCORM activity:

      Strict Standards: Non-static method completion_info::is_enabled() should not be called statically in /home/jknight1/html/moodle2/mod/scorm/lib.php on line 1331
      Fatal error: Using $this when not in object context in <moodle_dir>/lib/completionlib.php on line 225

      Using Moodle 2.3.2+ (Build: 20121014)

      Looks like it was introduced by MDL-35227

        Issue Links

          Activity

          Hide
          Dan Marsden added a comment -

          adding Aaron as a watcher as he asked me to use completion_info:: instead of the $CFG check - Aaron any chance you could take a look at this?

          Show
          Dan Marsden added a comment - adding Aaron as a watcher as he asked me to use completion_info:: instead of the $CFG check - Aaron any chance you could take a look at this?
          Hide
          Aaron Barnes added a comment -

          How does that look Dan?

          Cheers,
          Aaron

          Show
          Aaron Barnes added a comment - How does that look Dan? Cheers, Aaron
          Hide
          Jay Knight added a comment -

          That change seems to work for me.

          Show
          Jay Knight added a comment - That change seems to work for me.
          Hide
          Dan Marsden added a comment -

          I guess I'd prefer to not need those extra SQL calls if completion was disabled at site level - maybe we could do that in conjunction with a $CFG check at the top? - what are your thoughts?

          Show
          Dan Marsden added a comment - I guess I'd prefer to not need those extra SQL calls if completion was disabled at site level - maybe we could do that in conjunction with a $CFG check at the top? - what are your thoughts?
          Hide
          Aaron Barnes added a comment -

          Fair enough Dan, I'll give it a tweak.

          Thanks for the test Jay!

          Show
          Aaron Barnes added a comment - Fair enough Dan, I'll give it a tweak. Thanks for the test Jay!
          Hide
          Aaron Barnes added a comment -

          Hey Dan,

          Is this function likely to get used multiple times in the same HTTP request? Just wondering if it's worth caching completion enabled results in a static variable inside the function.

          Cheers,
          Aaron

          Show
          Aaron Barnes added a comment - Hey Dan, Is this function likely to get used multiple times in the same HTTP request? Just wondering if it's worth caching completion enabled results in a static variable inside the function. Cheers, Aaron
          Hide
          Dan Marsden added a comment -

          I don't think it gets called multiple times - took a quick look and couldn't see it being used like that.

          Show
          Dan Marsden added a comment - I don't think it gets called multiple times - took a quick look and couldn't see it being used like that.
          Hide
          Aaron Barnes added a comment -

          Hey Dan,

          Here is some updated code for testing!

          Cheers,
          Aaron

          Show
          Aaron Barnes added a comment - Hey Dan, Here is some updated code for testing! Cheers, Aaron
          Hide
          Dan Marsden added a comment -

          [Y] Syntax
          [-] Output
          [Y] Whitespace
          [-] Language
          [Y] Databases
          [N] Testing
          [Y] Security
          [-] Documentation
          [Y] Git
          [Y] Sanity check

          Thanks Aaron - I'm happy for this to go up for integration but we need to add testing instructions so the QA team can reproduce the bug.

          Show
          Dan Marsden added a comment - [Y] Syntax [-] Output [Y] Whitespace [-] Language [Y] Databases [N] Testing [Y] Security [-] Documentation [Y] Git [Y] Sanity check Thanks Aaron - I'm happy for this to go up for integration but we need to add testing instructions so the QA team can reproduce the bug.
          Hide
          Dan Marsden added a comment -

          great - thanks Aaron!

          Show
          Dan Marsden added a comment - great - thanks Aaron!
          Hide
          Juan Gabriel Sáenz added a comment -

          Hi,

          I have the same problem and I can´t open any scorm recource marked as "asset".

          Reading the error line Fatal error: Using $this when not in object context in <moodle_dir>/lib/completionlib.php on line 225 I disabled enablecompletion in admin/settings.php and scorm works again.

          I hope this would be helpful to resolve this bug.

          Best regards,

          Juan Sáenz
          Edu Labs

          Show
          Juan Gabriel Sáenz added a comment - Hi, I have the same problem and I can´t open any scorm recource marked as "asset". Reading the error line Fatal error: Using $this when not in object context in <moodle_dir>/lib/completionlib.php on line 225 I disabled enablecompletion in admin/settings.php and scorm works again. I hope this would be helpful to resolve this bug. Best regards, Juan Sáenz Edu Labs
          Hide
          Steve Clay added a comment -

          There is data loss as well. My SCORM package POSTs (to mod/scorm/datamodel.php) the last student interaction data along with the completion notification. The same fatal error occurs (accessing $this in static call), but this is before the interaction data is saved, so Moodle records the the activity as completed, but has lost the last interaction data from the user.

          FYI this is 2.3.2+ (Build: 20121018) displaying a SCORM 1.2 package.

          Show
          Steve Clay added a comment - There is data loss as well. My SCORM package POSTs (to mod/scorm/datamodel.php) the last student interaction data along with the completion notification. The same fatal error occurs (accessing $this in static call), but this is before the interaction data is saved, so Moodle records the the activity as completed, but has lost the last interaction data from the user. FYI this is 2.3.2+ (Build: 20121018) displaying a SCORM 1.2 package.
          Hide
          Dan Marsden added a comment -

          sure - this fix is queued to be integrated hopefully next week - it missed this weeks integration as the integrators were all at the Perth Hackfest last week and didn't manage to integrate last weeks issues until this week.

          Show
          Dan Marsden added a comment - sure - this fix is queued to be integrated hopefully next week - it missed this weeks integration as the integrators were all at the Perth Hackfest last week and didn't manage to integrate last weeks issues until this week.
          Hide
          Dan Poltawski 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
          Dan Poltawski 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
          Sam Hemelryk added a comment -

          Thanks Dan, has been integrated now

          Show
          Sam Hemelryk added a comment - Thanks Dan, has been integrated now
          Hide
          Paul Vaughan added a comment -

          Sorry for the noise, getting my minor release version numbering mixed up. :/

          Show
          Paul Vaughan added a comment - Sorry for the noise, getting my minor release version numbering mixed up. :/
          Hide
          David Monllaó added a comment -

          Tested in master and 23. It passes

          Show
          David Monllaó added a comment - Tested in master and 23. It passes
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Amazed. Inspired. Grateful. That’s how your generosity makes me feel.

          (not really)

          Closing, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Amazed. Inspired. Grateful. That’s how your generosity makes me feel. (not really) Closing, thanks!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: