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

Error when saving a SCORM activity

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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
            danmarsden 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
            danmarsden 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
            sry_not4sale Aaron Barnes added a comment -

            How does that look Dan?

            Cheers,
            Aaron

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

            That change seems to work for me.

            Show
            jk3us Jay Knight added a comment - That change seems to work for me.
            Hide
            danmarsden 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
            danmarsden 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
            sry_not4sale Aaron Barnes added a comment -

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

            Thanks for the test Jay!

            Show
            sry_not4sale Aaron Barnes added a comment - Fair enough Dan, I'll give it a tweak. Thanks for the test Jay!
            Hide
            sry_not4sale 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
            sry_not4sale 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
            danmarsden 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
            danmarsden 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
            sry_not4sale Aaron Barnes added a comment -

            Hey Dan,

            Here is some updated code for testing!

            Cheers,
            Aaron

            Show
            sry_not4sale Aaron Barnes added a comment - Hey Dan, Here is some updated code for testing! Cheers, Aaron
            Hide
            danmarsden 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
            danmarsden 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
            danmarsden Dan Marsden added a comment -

            great - thanks Aaron!

            Show
            danmarsden Dan Marsden added a comment - great - thanks Aaron!
            Hide
            edulabs 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
            edulabs 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
            mrclay 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
            mrclay 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
            danmarsden 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
            danmarsden 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
            poltawski 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
            poltawski 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
            samhemelryk Sam Hemelryk added a comment -

            Thanks Dan, has been integrated now

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

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

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

            Tested in master and 23. It passes

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

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

            (not really)

            Closing, thanks!

            Show
            stronk7 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:
                  Fix Release Date:
                  12/Nov/12