Moodle
  1. Moodle
  2. MDL-34363

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

    Details

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

      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.

        Issue Links

          Activity

          Hide
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          Michael Spall added a comment -

          Applying Eric's patch fixes this issue for us.

          Show
          Michael Spall added a comment - Applying Eric's patch fixes this issue for us.
          Hide
          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
          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
          Eric Merrill added a comment -

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

          Thanks!

          Show
          Eric Merrill added a comment - Andrew, could you please take a look at this ticket. Thanks!
          Hide
          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
          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
          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
          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
          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
          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 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 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
          Emma Richardson added a comment -

          The fix is posted above.

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

          Seeing this issue here too on 2.3.1+.

          Show
          Bret Miller added a comment - Seeing this issue here too on 2.3.1+.
          Hide
          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
          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
          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
          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
          Dan Poltawski added a comment -

          (thanks Andrew for reviewing, thats helpful)

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

          Thanks Eric,

          I've integrated this to 2.3 and master.

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

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

          Test passed.

          Show
          Rossiani Wijaya added a comment - Tested this on 2.3 and master. It is working great. Test passed.
          Hide
          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
          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: