Moodle
  1. Moodle
  2. MDL-34403

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

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor 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:
    • Rank:
      42800

      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.

        Activity

        Hide
        CLAIRE BROWNE added a comment -

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

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

        thanks for the report

        Show
        Petr Škoda added a comment - thanks for the report
        Hide
        CLAIRE BROWNE added a comment -

        no problem.

        Show
        CLAIRE BROWNE added a comment - no problem.
        Hide
        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
        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
        Andrew Normore added a comment -

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

        Show
        Andrew Normore added a comment - I'm having the same issue. Any developer taken note of this error yet?
        Hide
        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
        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
        Andrew Normore added a comment -

        Further analysis is Json Parse @ 216

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

        Even further...

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

        Line 404

        No stack trace

        Show
        Andrew Normore added a comment - Even further... /theme/yui_combo.php?moodle/1354561227/enrol_manual/quickenrolment/quickenrolment.js Line 404 No stack trace
        Hide
        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
        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
        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
        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
        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
        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
        Matt Petro added a comment -

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

        Show
        Matt Petro added a comment - I've confirmed that the same bug exists in 2.4.3 and master.
        Hide
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        Tim Hunt added a comment -

        Fixing workflow state.

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

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

        Show
        Tim Hunt added a comment - So, please back-port and then submit for integration.
        Hide
        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
        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
        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
        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
        Dan Poltawski added a comment -

        Integrated to master, 24 and 23 - thanks Matt!

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

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

        Show
        Dan Poltawski added a comment - Tested succesfully on master, 24 and 23 during integration. Thanks!
        Hide
        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
        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: