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

          Attachments

            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