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

Syntax Error - Enrolling Users into a course with locked quiz grade items

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.1, 2.3.6, 2.4.3
    • Fix Version/s: 2.3.7, 2.4.4
    • Component/s: Enrolments, Quiz
    • Labels:
    • Testing Instructions:
      Hide
      1. Create a course with a quiz.
      2. Lock the quiz in the gradebook.
      3. Manually enroll a user who is not already in the course, using the AJAX interface, and with the option "Recover user's old grades if possible".

      The enrollment should be performed, and there should be no javascript error message.

      Show
      Create a course with a quiz. Lock the quiz in the gradebook. Manually enroll a user who is not already in the course, using the AJAX interface, and with the option "Recover user's old grades if possible". The enrollment should be performed, and there should be no javascript error message.
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull Master Branch:

      Description

      Syntax error message received from javascript when enrolling user in course

      To replicate:

      1. Create a course with a quiz.
      2. Lock the quiz in the gradebook.
      3. Manually enroll a user who is not already in the course, and with the
        option "Recover user's old grades if possible".

      The enrollment will be performed, but javascript will display a "syntax error" box.

        Gliffy Diagrams

          Activity

          Hide
          moodlecvqo CLAIRE BROWNE added a comment -

          I can however if I click 'OK' carry on with enrolments as normal.

          Show
          moodlecvqo CLAIRE BROWNE added a comment - I can however if I click 'OK' carry on with enrolments as normal.
          Hide
          skodak Petr Skoda added a comment -

          thanks for the report

          Show
          skodak Petr Skoda added a comment - thanks for the report
          Hide
          moodlecvqo CLAIRE BROWNE added a comment -

          no problem.

          Show
          moodlecvqo CLAIRE BROWNE added a comment - no problem.
          Hide
          jimevans54 Jim Evans added a comment -

          I am having the same issue with 2.2.4. The error appears when a change is made to a user's enrollment, is added and even when I simply change the view from All to a particular enrollment view. We use both flatfill and manual enrollments methods.

          Show
          jimevans54 Jim Evans added a comment - I am having the same issue with 2.2.4. The error appears when a change is made to a user's enrollment, is added and even when I simply change the view from All to a particular enrollment view. We use both flatfill and manual enrollments methods.
          Hide
          andrewnormore Andrew Normore added a comment -

          I'm having the same issue. Any developer taken note of this error yet?

          Show
          andrewnormore Andrew Normore added a comment - I'm having the same issue. Any developer taken note of this error yet?
          Hide
          andrewnormore Andrew Normore added a comment -

          /theme/yui_combo.php?
          3.5.1/build/querystring-stringify-simple/querystring-stringify-simple-min.js
          &3.5.1/build/io-base/io-base-min.js
          &3.5.1/build/json-parse/json-parse-min.js

          Line 19

          No stack trace

          Show
          andrewnormore Andrew Normore added a comment - /theme/yui_combo.php? 3.5.1/build/querystring-stringify-simple/querystring-stringify-simple-min.js &3.5.1/build/io-base/io-base-min.js &3.5.1/build/json-parse/json-parse-min.js Line 19 No stack trace
          Hide
          andrewnormore Andrew Normore added a comment -

          Further analysis is Json Parse @ 216

          Show
          andrewnormore Andrew Normore added a comment - Further analysis is Json Parse @ 216
          Hide
          andrewnormore Andrew Normore added a comment -

          Even further...

          /theme/yui_combo.php?moodle/1354561227/enrol_manual/quickenrolment/quickenrolment.js

          Line 404

          No stack trace

          Show
          andrewnormore Andrew Normore added a comment - Even further... /theme/yui_combo.php?moodle/1354561227/enrol_manual/quickenrolment/quickenrolment.js Line 404 No stack trace
          Hide
          andrewnormore Andrew Normore added a comment - - edited

          So, this is where the problem is coming from:

          on: {
          start : this.displayLoading,
          complete : function(tid, outcome, args) {
          try {
          var result = Y.JSON.parse(outcome.responseText);
          if (result.error)

          { // PROBLEM IS HERE return new M.core.ajaxException(result); // PROBLEM IS HERE }

          else

          { args.userNode.addClass(CSS.ENROLLED); args.userNode.one('.'+CSS.ENROL).remove(); this.set(UEP.REQUIREREFRESH, true); }

          } catch (e)

          { new M.core.exception(e); }

          },
          end : this.removeLoading
          },

          Show
          andrewnormore Andrew Normore added a comment - - edited So, this is where the problem is coming from: on: { start : this.displayLoading, complete : function(tid, outcome, args) { try { var result = Y.JSON.parse(outcome.responseText); if (result.error) { // PROBLEM IS HERE return new M.core.ajaxException(result); // PROBLEM IS HERE } else { args.userNode.addClass(CSS.ENROLLED); args.userNode.one('.'+CSS.ENROL).remove(); this.set(UEP.REQUIREREFRESH, true); } } catch (e) { new M.core.exception(e); } }, end : this.removeLoading },
          Hide
          andrewnormore Andrew Normore added a comment - - edited

          TEMPORARY FIX
          We will disable the function from running, forcing the user to use the non-ajax version of manual enrollment.

          edit: enrol\manual\yui\quickenrolment\quickenrolment.js

          Find:
          Line 1: YUI.add('moodle-enrol_manual-quickenrolment', function(Y) {

          Becomes:
          Line 1: YUI.add('moodle-enrol_manual-quickenrolment', function(Y) {
          Line 2: return;

          Good luck every one.

          Show
          andrewnormore Andrew Normore added a comment - - edited TEMPORARY FIX We will disable the function from running, forcing the user to use the non-ajax version of manual enrollment. edit: enrol\manual\yui\quickenrolment\quickenrolment.js Find: Line 1: YUI.add('moodle-enrol_manual-quickenrolment', function(Y) { Becomes: Line 1: YUI.add('moodle-enrol_manual-quickenrolment', function(Y) { Line 2: return; Good luck every one.
          Hide
          mpetrowi Matt Petro added a comment -

          We just ran into this on a 2.3.6 site. Here's the circumstances for us:

          1. The course has a locked grade item.
          2. The enrollment is performed for a new user in the course, and "Recover user's old grades if possible" is enabled

          The syntax error results because moodle prints the following message in the response from enrol/manual/ajax.php

          "This activity is locked in the gradebook. Changes that are made to grades in this activity will not be copied to the gradebook until it is unlocked"

          Show
          mpetrowi Matt Petro added a comment - We just ran into this on a 2.3.6 site. Here's the circumstances for us: 1. The course has a locked grade item. 2. The enrollment is performed for a new user in the course, and "Recover user's old grades if possible" is enabled The syntax error results because moodle prints the following message in the response from enrol/manual/ajax.php "This activity is locked in the gradebook. Changes that are made to grades in this activity will not be copied to the gradebook until it is unlocked"
          Hide
          mpetrowi Matt Petro added a comment -

          I've confirmed that the same bug exists in 2.4.3 and master.

          Show
          mpetrowi Matt Petro added a comment - I've confirmed that the same bug exists in 2.4.3 and master.
          Hide
          mpetrowi Matt Petro added a comment -

          There could be many reasons for such a syntax error. I hope it's okay that I changed this bug report to reflect the circumstances we've found for the error.

          Show
          mpetrowi Matt Petro added a comment - There could be many reasons for such a syntax error. I hope it's okay that I changed this bug report to reflect the circumstances we've found for the error.
          Hide
          mpetrowi Matt Petro added a comment -

          Actually the error only occurs for locked quiz items in the gradebook. The code which causes the problem is in mod/quiz/lib.php and prefaced by:

          // NOTE: this is an extremely nasty hack! It is not a bug if this confirmation fails badly. --skodak.

          Show
          mpetrowi Matt Petro added a comment - Actually the error only occurs for locked quiz items in the gradebook. The code which causes the problem is in mod/quiz/lib.php and prefaced by: // NOTE: this is an extremely nasty hack! It is not a bug if this confirmation fails badly. --skodak.
          Hide
          mpetrowi Matt Petro added a comment -

          I was considering fixing this, but I think I'll wait for comments from someone who understands why that code is there before Hacking the hack Hopefully Petr will notice this.

          Show
          mpetrowi Matt Petro added a comment - I was considering fixing this, but I think I'll wait for comments from someone who understands why that code is there before Hacking the hack Hopefully Petr will notice this.
          Hide
          mnoorenberghe Matthew N added a comment -

          Matt Petro, I don't think the original investigation in this bug was related to quizzes and locked grade items but I could be wrong. It may have been better to file a new bug.

          Show
          mnoorenberghe Matthew N added a comment - Matt Petro, I don't think the original investigation in this bug was related to quizzes and locked grade items but I could be wrong. It may have been better to file a new bug.
          Hide
          mpetrowi Matt Petro added a comment -

          Hi Matthew N, I didn't see anything in the issue that lead me to believe it wasn't the same bug. Ultimately, it comes down to either making a new issue and resolving this as probably related, or else amending this issue. This way seemed cleaner at the time. I could be wrong.

          Show
          mpetrowi Matt Petro added a comment - Hi Matthew N, I didn't see anything in the issue that lead me to believe it wasn't the same bug. Ultimately, it comes down to either making a new issue and resolving this as probably related, or else amending this issue. This way seemed cleaner at the time. I could be wrong.
          Hide
          mpetrowi Matt Petro added a comment -

          Hi Tim,

          Can you review this minor change? It suppresses HTML output to AJAX scripts when attempting to regrade a locked quiz grade item. This was coming up during course enrollment when restoring grades.

          As an aside, I have no idea when that 'confirm_regrade' hack in quiz_grade_item_update() is ever used. From my poking around, all the routes in the UI to refresh grades are disabled when the grade item locked. It could be the whole hack could be removed?

          Thanks,
          -matt

          Show
          mpetrowi Matt Petro added a comment - Hi Tim, Can you review this minor change? It suppresses HTML output to AJAX scripts when attempting to regrade a locked quiz grade item. This was coming up during course enrollment when restoring grades. As an aside, I have no idea when that 'confirm_regrade' hack in quiz_grade_item_update() is ever used. From my poking around, all the routes in the UI to refresh grades are disabled when the grade item locked. It could be the whole hack could be removed? Thanks, -matt
          Hide
          timhunt Tim Hunt added a comment -

          That code is evil. It should never have been added in the first place, but it was before we had integration review.

          For now, I guess this is a blocker for your, so we need to integrate this horrible ugly hack. MDL-13978 is the issue for a proper fix.

          Show
          timhunt Tim Hunt added a comment - That code is evil. It should never have been added in the first place, but it was before we had integration review. For now, I guess this is a blocker for your, so we need to integrate this horrible ugly hack. MDL-13978 is the issue for a proper fix.
          Hide
          timhunt Tim Hunt added a comment -

          Fixing workflow state.

          Show
          timhunt Tim Hunt added a comment - Fixing workflow state.
          Hide
          timhunt Tim Hunt added a comment -

          So, please back-port and then submit for integration.

          Show
          timhunt Tim Hunt added a comment - So, please back-port and then submit for integration.
          Hide
          mpetrowi Matt Petro added a comment -

          I added the backported fixes.

          Tim, is it possible for the integrators to cherry pick changes from master for simple fixes like this, or is it always necessary to create the 2.x branches? Not a big deal, just curious.

          Show
          mpetrowi Matt Petro added a comment - I added the backported fixes. Tim, is it possible for the integrators to cherry pick changes from master for simple fixes like this, or is it always necessary to create the 2.x branches? Not a big deal, just curious.
          Hide
          timhunt Tim Hunt added a comment -

          Thanks. Submitted for integration.

          The integrators strongly prefer that the developer does the back-porting. It confirms that there are no merge conflicts, and increases the chance that you will test the stable versions. http://tjhunt.blogspot.co.uk/2012/08/automating-git.html has some useful shortcuts. https://github.com/FMCorz/mdk is also available.

          Show
          timhunt Tim Hunt added a comment - Thanks. Submitted for integration. The integrators strongly prefer that the developer does the back-porting. It confirms that there are no merge conflicts, and increases the chance that you will test the stable versions. http://tjhunt.blogspot.co.uk/2012/08/automating-git.html has some useful shortcuts. https://github.com/FMCorz/mdk is also available.
          Hide
          poltawski Dan Poltawski added a comment -

          Integrated to master, 24 and 23 - thanks Matt!

          Show
          poltawski Dan Poltawski added a comment - Integrated to master, 24 and 23 - thanks Matt!
          Hide
          poltawski Dan Poltawski added a comment -

          Tested succesfully on master, 24 and 23 during integration. Thanks!

          Show
          poltawski Dan Poltawski added a comment - Tested succesfully on master, 24 and 23 during integration. Thanks!
          Hide
          poltawski Dan Poltawski added a comment -

          Thanks! You're changes are now spread to the world through this git and our source control repositories.

          No time to rest though, we've got days to make 2.5 the best yet!

          ciao

          Show
          poltawski Dan Poltawski added a comment - Thanks! You're changes are now spread to the world through this git and our source control repositories. No time to rest though, we've got days to make 2.5 the best yet! ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                13/May/13