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

          Attachments

            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