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

disabled ajax and 'quick feedback' in grader report causes grades to become 'overridden'

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.3.1
    • Fix Version/s: 2.3.3
    • Component/s: Gradebook
    • Labels:
    • Testing Instructions:
      Hide
      1. Have a gradebook with numerous grade items and assignments
      2. In the settings, turn AJAX off and Quick Feedback on.
      3. In the grader report, turn editing on. Hit save down at the bottom.
      4. Confirm that non of the cells have turned colors to indicate overridden.
      5. Enter various grades and feedback, save, and confirm that the settings are saved and are overridden properly.

      (Added by Andrew Davis, this shouldn't be affected at all but I want to be double sure)

      1. In the settings, turn Quick Feedback off.
      2. In the grader report, turn editing on. Hit save down at the bottom.
      3. Confirm that non of the cells have turned colors to indicate overridden.
      Show
      Have a gradebook with numerous grade items and assignments In the settings, turn AJAX off and Quick Feedback on. In the grader report, turn editing on. Hit save down at the bottom. Confirm that non of the cells have turned colors to indicate overridden. Enter various grades and feedback, save, and confirm that the settings are saved and are overridden properly. (Added by Andrew Davis, this shouldn't be affected at all but I want to be double sure) In the settings, turn Quick Feedback off. In the grader report, turn editing on. Hit save down at the bottom. Confirm that non of the cells have turned colors to indicate overridden.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:

      Description

      Might be related to: http://tracker.moodle.org/browse/MDL-33168

      When you are in the viewing the gradebook in 'grader report' and have 'show quick feedback' enabled so you have the feedback box and disable 'Enable AJAX'.

      Then go into edit mode in the grader report and click 'update'. All fields with empty comment boxes are now set to overridden.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            mulroony Patrick Mulrooney added a comment - - edited

            Better walk through:

            1)Go to any course with a couple students and a couple grade items in the gradebook
            2)Go in to the gradebook for that course
            3)Go to "My preferences -> grader report"
            4)Set "Show quick feedback" to "Yes" and "Enable ajax" to "No"
            5)Go back to the grader report
            6)'Update' the gradebook
            7)Overridden status of the grade items might not reflect immediately. If they do not, go out of the gradebook and back in and grades not changed will now be overridden.

            Show
            mulroony Patrick Mulrooney added a comment - - edited Better walk through: 1)Go to any course with a couple students and a couple grade items in the gradebook 2)Go in to the gradebook for that course 3)Go to "My preferences -> grader report" 4)Set "Show quick feedback" to "Yes" and "Enable ajax" to "No" 5)Go back to the grader report 6)'Update' the gradebook 7)Overridden status of the grade items might not reflect immediately. If they do not, go out of the gradebook and back in and grades not changed will now be overridden.
            Hide
            mulroony Patrick Mulrooney added a comment -

            Seems to be the comparison on line 228 of 'grade/report/grader.php'

                        } else if ($datatype === 'feedback') {
                            if ($oldvalue->feedback === $postedvalue) {
                                continue;
                            }
                        }

            If there is no existing feedback for the student the type of 'oldvalue->feedback' is NULL, the type of '$postedvalue' is string. So they never equal.

            Show
            mulroony Patrick Mulrooney added a comment - Seems to be the comparison on line 228 of 'grade/report/grader.php' } else if ($datatype === 'feedback') { if ($oldvalue->feedback === $postedvalue) { continue; } } If there is no existing feedback for the student the type of 'oldvalue->feedback' is NULL, the type of '$postedvalue' is string. So they never equal.
            Hide
            mulroony Patrick Mulrooney added a comment -

            grade/report/grader/lib.php line 228

                            if (($oldvalue->feedback === $postedvalue) or ($oldvalue->feedback === NULL and empty($postedvalue))) {
                                continue;
                            }

            This seems to correct the issue for me. First check if they are the same, if not but oldvalue is NULL and the new value is an empty string then move on.

            Show
            mulroony Patrick Mulrooney added a comment - grade/report/grader/lib.php line 228 if (($oldvalue->feedback === $postedvalue) or ($oldvalue->feedback === NULL and empty($postedvalue))) { continue; } This seems to correct the issue for me. First check if they are the same, if not but oldvalue is NULL and the new value is an empty string then move on.
            Hide
            salvetore Michael de Raadt added a comment -

            Thanks for spotting that and providing a solution.

            I have removed the old content from your description. It is still in the history of this issue if needed.

            Show
            salvetore Michael de Raadt added a comment - Thanks for spotting that and providing a solution. I have removed the old content from your description. It is still in the history of this issue if needed.
            Hide
            mcwoods Michael Woods added a comment -

            Yes, just had this issue reported by a staff member on our site too. Thanks, Patrick, for the patch - it has worked a charm.

            Show
            mcwoods Michael Woods added a comment - Yes, just had this issue reported by a staff member on our site too. Thanks, Patrick, for the patch - it has worked a charm.
            Hide
            emerrill Eric Merrill added a comment -

            I've made the patch in this ticket into pull branches.

            We have been hit by the same thing!

            Show
            emerrill Eric Merrill added a comment - I've made the patch in this ticket into pull branches. We have been hit by the same thing!
            Hide
            mspall Michael Spall added a comment -

            When grade items are mentioned, do you mean grade items from activities or manual grade items? In our case manual grade items are not being overridden, just activities. Also, in our case, it doesn't matter whether or not ajax is enabled in the gradebook, this bug happens regardless.

            Show
            mspall Michael Spall added a comment - When grade items are mentioned, do you mean grade items from activities or manual grade items? In our case manual grade items are not being overridden, just activities. Also, in our case, it doesn't matter whether or not ajax is enabled in the gradebook, this bug happens regardless.
            Hide
            mulroony Patrick Mulrooney added a comment -

            When grade items are mentioned, do you mean grade items from activities or manual grade items? In our case manual grade items are not being overridden, just activities. Also, in our case, it doesn't matter whether or not ajax is enabled in the gradebook, this bug happens regardless.

            I believe it was both, but we have my patch in place so I cannot test it at them moment.

            Show
            mulroony Patrick Mulrooney added a comment - When grade items are mentioned, do you mean grade items from activities or manual grade items? In our case manual grade items are not being overridden, just activities. Also, in our case, it doesn't matter whether or not ajax is enabled in the gradebook, this bug happens regardless. I believe it was both, but we have my patch in place so I cannot test it at them moment.
            Hide
            mspall Michael Spall added a comment -

            Applying Eric's patch fixes this issue for us.

            Show
            mspall Michael Spall added a comment - Applying Eric's patch fixes this issue for us.
            Hide
            emmarichardson Emma Richardson added a comment -

            Just to point out to those wanting to apply this fix that the file to change is actually /grade/report/grader/lib.php.

            Show
            emmarichardson Emma Richardson added a comment - Just to point out to those wanting to apply this fix that the file to change is actually /grade/report/grader/lib.php.
            Hide
            emerrill Eric Merrill added a comment -

            Andrew, could you please take a look at this ticket.

            Thanks!

            Show
            emerrill Eric Merrill added a comment - Andrew, could you please take a look at this ticket. Thanks!
            Hide
            bitoffish Thomas Haines added a comment -

            Could this issue be affecting the 2.2 branch? I have a user who has identified these exact symptoms, but I have been unable to reproduce the issue.

            Show
            bitoffish Thomas Haines added a comment - Could this issue be affecting the 2.2 branch? I have a user who has identified these exact symptoms, but I have been unable to reproduce the issue.
            Hide
            rogermorris roger morris added a comment -

            Just wondering, the database default value for feedback is NULL. In grade/querylib.php on line 231, the feedback on the form is set to '' if the database value is NULL.
            Could an alternate solution be to have the default feedback set to '' instead of NULL, then the original comparison on line 228 of /grade/report/grader/lib.php would work as intended.

            Show
            rogermorris roger morris added a comment - Just wondering, the database default value for feedback is NULL. In grade/querylib.php on line 231, the feedback on the form is set to '' if the database value is NULL. Could an alternate solution be to have the default feedback set to '' instead of NULL, then the original comparison on line 228 of /grade/report/grader/lib.php would work as intended.
            Hide
            kbsmith Kenton Smith added a comment - - edited

            Confirmed this is an issue within "Show quick feedback" regardless of AJAX settings in 2.3.1 and 2.3.2

            Show
            kbsmith Kenton Smith added a comment - - edited Confirmed this is an issue within "Show quick feedback" regardless of AJAX settings in 2.3.1 and 2.3.2
            Hide
            vicki.dunnam Vicki Dunnam added a comment -

            We are still having this issue. I have noticed that if I have my gradebook set to show 6 students on the page, when I enter feedback in the gradebook, all assignments and grades are all overridden just on that one page. I don't know if that helps you in searching this or not. Just thought that was interesting. We sure would like to see it resolved. I have lots of data or will be will to test if you let me know.

            Is there a patch or fix.... please direct me to correct place.

            Show
            vicki.dunnam Vicki Dunnam added a comment - We are still having this issue. I have noticed that if I have my gradebook set to show 6 students on the page, when I enter feedback in the gradebook, all assignments and grades are all overridden just on that one page. I don't know if that helps you in searching this or not. Just thought that was interesting. We sure would like to see it resolved. I have lots of data or will be will to test if you let me know. Is there a patch or fix.... please direct me to correct place.
            Hide
            emmarichardson Emma Richardson added a comment -

            The fix is posted above.

            Show
            emmarichardson Emma Richardson added a comment - The fix is posted above.
            Hide
            bret.miller Bret Miller added a comment -

            Seeing this issue here too on 2.3.1+.

            Show
            bret.miller Bret Miller added a comment - Seeing this issue here too on 2.3.1+.
            Hide
            poltawski Dan Poltawski added a comment -

            Sending all 'waiting for peer review' issues to integration before freeze, as agreed in Integrators Meeting 19/10/12. We are doing this to ensure any 'integratable issues' will not got missed before freeze..

            Show
            poltawski Dan Poltawski added a comment - Sending all 'waiting for peer review' issues to integration before freeze, as agreed in Integrators Meeting 19/10/12. We are doing this to ensure any 'integratable issues' will not got missed before freeze..
            Hide
            andyjdavis Andrew Davis added a comment -

            Although this is already up for integration Im doing a last minute peer review.

            [Y] Syntax
            [Y] Output
            [Y] Whitespace
            [NA] Language
            [NA] Databases
            [Y] Testing
            [Y] Security
            [NA] Documentation
            [Y] Git
            [Y] Sanity check

            Looks good. I'll add a few more steps to the testing instructions.

            Show
            andyjdavis Andrew Davis added a comment - Although this is already up for integration Im doing a last minute peer review. [Y] Syntax [Y] Output [Y] Whitespace [NA] Language [NA] Databases [Y] Testing [Y] Security [NA] Documentation [Y] Git [Y] Sanity check Looks good. I'll add a few more steps to the testing instructions.
            Hide
            poltawski Dan Poltawski added a comment -

            (thanks Andrew for reviewing, thats helpful)

            Show
            poltawski Dan Poltawski added a comment - (thanks Andrew for reviewing, thats helpful)
            Hide
            poltawski Dan Poltawski added a comment -

            Thanks Eric,

            I've integrated this to 2.3 and master.

            Show
            poltawski Dan Poltawski added a comment - Thanks Eric, I've integrated this to 2.3 and master.
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Tested this on 2.3 and master. It is working great.

            Test passed.

            Show
            rwijaya Rossiani Wijaya added a comment - Tested this on 2.3 and master. It is working great. Test passed.
            Hide
            poltawski Dan Poltawski added a comment -

            Hurray!

            You did it, congratulations! You have on Mojito credit to redeem after the release of Moodle 2.4

            Show
            poltawski Dan Poltawski added a comment - Hurray! You did it, congratulations! You have on Mojito credit to redeem after the release of Moodle 2.4

              People

              • Votes:
                31 Vote for this issue
                Watchers:
                24 Start watching this issue

                Dates

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