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

error enrolling users if scorm modules present

    Details

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

      I couldn't reproduce this problem as it arises in some courses but not in other courses. It seems that the "recover grades" option has something to do with it.

      Show
      I couldn't reproduce this problem as it arises in some courses but not in other courses. It seems that the "recover grades" option has something to do with it.
    • Workaround:
      Hide

      The AJAX form calls enrol/manual/ajax.php, which (line 126) calls lib/gradelib/grade_recover_history_grades() -> lib/gradelib/grade_grab_course_grades() -> lib/gradelib/grade_update_mod_grades(). There, at last, if a SCORM module is present, the mod/scorm/lib/scorm_grade_item_update() is called and tries to create a New completion_info() without including completionlib.php.
      Workaround: use patch displayed in the Description of this issue.

      Show
      The AJAX form calls enrol/manual/ajax.php, which (line 126) calls lib/gradelib/grade_recover_history_grades() -> lib/gradelib/grade_grab_course_grades() -> lib/gradelib/grade_update_mod_grades(). There, at last, if a SCORM module is present, the mod/scorm/lib/scorm_grade_item_update() is called and tries to create a New completion_info() without including completionlib.php. Workaround: use patch displayed in the Description of this issue.
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      master_MDL-35227

      Description

      In a course containing one or several SCORM module instances, if one tries to enrol users manually (from the Settings Block -> Course administration -> Users -> Enrolled users), we get a "Syntax error" Javascript modal. The error logged in the Apache error log is: PHP Fatal error: Class 'completion_info' not found in /***/mod/scorm/lib.php on line 691, referer: http://***/enrol/users.php?id=514
      The following hack fixes the issue :

      --- a/mod/scorm/lib.php
      +++ b/mod/scorm/lib.php
      @@ -688,6 +688,9 @@ function scorm_grade_item_update($scorm, $grades=null, $updatecompletion=true) {
       
               $cm = get_coursemodule_from_instance('scorm', $scorm->id, $course->id);
               if (!empty($cm)) {
      +            if (!class_exists('completion_info')) { //workaround for buggy PHP versions
      +                require_once($CFG->libdir.'/completionlib.php');
      +            }
                   $completion = new completion_info($course);
                   $completion->update_state($cm, COMPLETION_COMPLETE);
               }
       

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              danmarsden Dan Marsden added a comment -

              I think this makes sense to add in course/ajax.php - bouncing up for peer review. We could add it in SCORM if the integrator/peer reviewer can think of a reason why it should go there instead?

              Show
              danmarsden Dan Marsden added a comment - I think this makes sense to add in course/ajax.php - bouncing up for peer review. We could add it in SCORM if the integrator/peer reviewer can think of a reason why it should go there instead?
              Hide
              poltawski Dan Poltawski added a comment -

              I agree with your assessment, +1

              Show
              poltawski Dan Poltawski added a comment - I agree with your assessment, +1
              Hide
              danmarsden Dan Marsden added a comment -

              thanks Dan, rebased and pushing for integration.

              Show
              danmarsden Dan Marsden added a comment - thanks Dan, rebased and pushing for integration.
              Hide
              stronk7 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
              stronk7 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
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Uhm, the question is why is SCORM the only module doing completion tasks at scorm_grade_item_update(). I've looked to all the modules and none does that.

              Also, my feelings are about to think that the require_once() call must be as closer as possible to the code needing it. Just imagine I create any code calling scorm_grade_item_update(). It's that function responsibility (and not the caller) to include the needed libs.

              So, apart from the question about why SCORM is the only one doing that with completion... that perhaps will lead us to some new issue... I think the correct solution is to require_once(completionlib), immediately after the require_once(gradelib) and done.

              Reopening... ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Uhm, the question is why is SCORM the only module doing completion tasks at scorm_grade_item_update(). I've looked to all the modules and none does that. Also, my feelings are about to think that the require_once() call must be as closer as possible to the code needing it. Just imagine I create any code calling scorm_grade_item_update(). It's that function responsibility (and not the caller) to include the needed libs. So, apart from the question about why SCORM is the only one doing that with completion... that perhaps will lead us to some new issue... I think the correct solution is to require_once(completionlib), immediately after the require_once(gradelib) and done. Reopening... ciao
              Hide
              danmarsden Dan Marsden added a comment -

              yeah - good call - looking back at the code again myself... looks like there's a bigger issue there - enrolling a user could potentially end up flagging the module as complete? - need to look go and look back at where that was added to grade_item_update.

              Show
              danmarsden Dan Marsden added a comment - yeah - good call - looking back at the code again myself... looks like there's a bigger issue there - enrolling a user could potentially end up flagging the module as complete? - need to look go and look back at where that was added to grade_item_update.
              Hide
              danmarsden Dan Marsden added a comment -

              in fact... I think this might have already been fixed in MDL-33978 - so it may not be an issue? - Nicolas what Moodle version were you using when you saw this?

              Show
              danmarsden Dan Marsden added a comment - in fact... I think this might have already been fixed in MDL-33978 - so it may not be an issue? - Nicolas what Moodle version were you using when you saw this?
              Hide
              danmarsden Dan Marsden added a comment -

              nope - that fixes a similar issue but not the same.

              Show
              danmarsden Dan Marsden added a comment - nope - that fixes a similar issue but not the same.
              Hide
              danmarsden Dan Marsden added a comment -

              have updated the patch (not ready for complete peer review yet - needs phpdocs for starters) - have added Aaron/Simon to this patch as I'm keen for feedback from them before I submit this - Aaron wrote the completion code for SCORM for Totara.

              Show
              danmarsden Dan Marsden added a comment - have updated the patch (not ready for complete peer review yet - needs phpdocs for starters) - have added Aaron/Simon to this patch as I'm keen for feedback from them before I submit this - Aaron wrote the completion code for SCORM for Totara.
              Hide
              cibot CiBoT added a comment -

              Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

              Show
              cibot CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
              Hide
              monidu Nicolas Dunand added a comment -

              Hi Dan, We noticed this on Moodle 2.3.1+ (Build: 20120726)

              Show
              monidu Nicolas Dunand added a comment - Hi Dan, We noticed this on Moodle 2.3.1+ (Build: 20120726)
              Hide
              sry_not4sale Aaron Barnes added a comment -

              Hi Dan,

              Are you able to tidy up the $updatecompletion parameter of scorm_grade_item_update() function which is no longer used?

              Also, in scorm_get_completion() could you use completion_info::is_enabled() rather than the $CFG check.

              Cheers,
              Aaron

              Show
              sry_not4sale Aaron Barnes added a comment - Hi Dan, Are you able to tidy up the $updatecompletion parameter of scorm_grade_item_update() function which is no longer used? Also, in scorm_get_completion() could you use completion_info::is_enabled() rather than the $CFG check. Cheers, Aaron
              Hide
              danmarsden Dan Marsden added a comment -

              Thanks Aaron - bouncing this up for peer review again.

              Show
              danmarsden Dan Marsden added a comment - Thanks Aaron - bouncing this up for peer review again.
              Hide
              sry_not4sale Aaron Barnes added a comment -

              Looks good to me!

              Show
              sry_not4sale Aaron Barnes added a comment - Looks good to me!
              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
              danmarsden Dan Marsden added a comment -

              rebased

              Show
              danmarsden Dan Marsden added a comment - rebased
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Integrated (23 & master), thanks!

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Integrated (23 & master), thanks!
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              I trust you. Passing.

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - I trust you. Passing.
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Closing as fixed, many thanks for your awesome collaboration.

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Closing as fixed, many thanks for your awesome collaboration.
              Hide
              jk3us Jay Knight added a comment -

              It looks like the changes from this issue are causing this bug: MDL-36107

              Show
              jk3us Jay Knight added a comment - It looks like the changes from this issue are causing this bug: MDL-36107

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    12/Nov/12