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

          Attachments

            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