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:

      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

        Gliffy Diagrams

          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: