Moodle
  1. Moodle
  2. MDL-18095

Cannot clear override flag on grades with feedback

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.3
    • Fix Version/s: 2.1.6, 2.2.3
    • Component/s: Gradebook
    • Labels:
      None
    • Testing Instructions:
      Hide

      Make sure that your site has quick feedback turned on.

      Create an assignment activity. Online text is generally easiest.

      As a student complete the assignment.

      As a teacher or admin log in and grade the assignment. Provide some feedback when you do.

      Go to the gradebook, note that the grade and feedback appears in the gradebook (ie the grader report). You may see html tags in the feedback box. This is a known issue so please ignore it while testing this.

      Turn on editing and click the edit/hand icon for the grade item.

      Tick "Overridden", alter the grade, edit the feedback and click "save changes".

      Check that the feedback and grade was changed on the grader report.

      Click the edit icon again. Untick overriden and click save changes.

      Check that the grader report now shows the original grade and feedback.

      In the grade report change the grade and feedback and click "Update".

      Check that the feedback and grade were changed.

      Click the edit icon again. Untick overriden and click save changes.

      Check that the grader report now shows the original grade and feedback.

      Show
      Make sure that your site has quick feedback turned on. Create an assignment activity. Online text is generally easiest. As a student complete the assignment. As a teacher or admin log in and grade the assignment. Provide some feedback when you do. Go to the gradebook, note that the grade and feedback appears in the gradebook (ie the grader report). You may see html tags in the feedback box. This is a known issue so please ignore it while testing this. Turn on editing and click the edit/hand icon for the grade item. Tick "Overridden", alter the grade, edit the feedback and click "save changes". Check that the feedback and grade was changed on the grader report. Click the edit icon again. Untick overriden and click save changes. Check that the grader report now shows the original grade and feedback. In the grade report change the grade and feedback and click "Update". Check that the feedback and grade were changed. Click the edit icon again. Untick overriden and click save changes. Check that the grader report now shows the original grade and feedback.
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-18095-master-2
    • Rank:
      691

      Description

      To reproduce:

      -In the grader report, turn editing on and turn quick feedback on.
      -Enter some feedback and click update. Note that the grade is now overridden.
      -Click the edit icon above the grade, clear the overridden flag, and save changes. Note that the grade still shows as overridden and the feedback is now surrounded by paragraph tags.
      -Click the edit icon again, clear the overridden flag again, and save. Note that the override appears to be cleared now.
      -Enter a grade in any other cell and click update. Note that the grade that was previously cleared is now overridden once again.

      The only way I've found to really clear the overridden flag is to completely clear the feedback. It is sometimes necessary to go to html source view in the feedback editor and clear any html that might be left behind after the text is removed.

        Issue Links

          Activity

          Hide
          Ann Adamcik added a comment -

          One more symptom that seems to be related:

          -Go into the assignment grading interface and enter feedback that contains a list or a table.
          -Go to the gradebook, change a grade in some other activity, and click update. Note that the assignment grade is now overridden.

          Show
          Ann Adamcik added a comment - One more symptom that seems to be related: -Go into the assignment grading interface and enter feedback that contains a list or a table. -Go to the gradebook, change a grade in some other activity, and click update. Note that the assignment grade is now overridden.
          Hide
          Petr Škoda added a comment -

          Yes, this is intentional. Solution is to internally move all grading from assignment into new gradebook plugin. Unfortunately this can not be done in 1.9.x branch.

          Show
          Petr Škoda added a comment - Yes, this is intentional. Solution is to internally move all grading from assignment into new gradebook plugin. Unfortunately this can not be done in 1.9.x branch.
          Hide
          Petr Škoda added a comment -

          resolving as duplicate, you can track progress in linked bug, thanks

          Show
          Petr Škoda added a comment - resolving as duplicate, you can track progress in linked bug, thanks
          Hide
          Enrique Castro added a comment - - edited

          Reopening to try a second thought for 1.9 branch.

          In 1.9 manual marking at gradebook level (and thus overrridding) is an exceptional measure, activities should be marked at moduel level. But such overridding occurs sometimes. A clear pathway to REVERT overridding and return to module-set grade is needed.

          This bug appears by a very strict condition in code requiring that feedback MUST be exactly equal between two copies. The code is in script /grade/edit/tree/grade.php, by line 187

                  if (!grade_floats_different($data->finalgrade, $old_grade_grade->finalgrade)
                    and $data->feedback === $old_grade_grade->feedback) {
                      // change overridden flag only if grade or feedback not changed
                      if (!isset($data->overridden)) {
                          $data->overridden = 0; // checkbox
                      }
                      $grade_grade->set_overridden($data->overridden);
                  }
          

          But I do not see the reason to ONLY reset overrridden in that conditions, with grade-value and feedback identical. Actually, if the teacher wants to de-overridde the grade is because module-set grade is OK. There is no need for the checking, in my opinion.

          Simply eliminating the finalgrade and feedback check for equality will eliminate the bug and allow smooth behavior for the teacher. The call to force_regrading() later in the script will ensure that the grade and feedback will be re-copied from the module instance.

          Thus, code simpler and more functional.

          Show
          Enrique Castro added a comment - - edited Reopening to try a second thought for 1.9 branch. In 1.9 manual marking at gradebook level (and thus overrridding) is an exceptional measure, activities should be marked at moduel level. But such overridding occurs sometimes. A clear pathway to REVERT overridding and return to module-set grade is needed. This bug appears by a very strict condition in code requiring that feedback MUST be exactly equal between two copies. The code is in script /grade/edit/tree/grade.php, by line 187 if (!grade_floats_different($data->finalgrade, $old_grade_grade->finalgrade) and $data->feedback === $old_grade_grade->feedback) { // change overridden flag only if grade or feedback not changed if (!isset($data->overridden)) { $data->overridden = 0; // checkbox } $grade_grade->set_overridden($data->overridden); } But I do not see the reason to ONLY reset overrridden in that conditions, with grade-value and feedback identical. Actually, if the teacher wants to de-overridde the grade is because module-set grade is OK. There is no need for the checking, in my opinion. Simply eliminating the finalgrade and feedback check for equality will eliminate the bug and allow smooth behavior for the teacher. The call to force_regrading() later in the script will ensure that the grade and feedback will be re-copied from the module instance. Thus, code simpler and more functional.
          Hide
          Andrew Davis added a comment -

          There appears to still be some feedback/override related oddness in Moodle 2.2 similar to that described in this issue report.

          Show
          Andrew Davis added a comment - There appears to still be some feedback/override related oddness in Moodle 2.2 similar to that described in this issue report.
          Hide
          Andrew Davis added a comment - - edited

          This is definitely a problem as of 2.2. If the activity provides multiline feedback the quick feedback text box compresses it down to a single line. The html editor on the grade edit page produces multiline text. When you override the grade on the grader report your feedback gets saved as a single line. When you go to the grade edit page to turn off the override the editor appears to split it out to multiple lines. This makes it look like the feedback has changed preventing you from turning off "override".

          the first string is coming from the grade edit form. The second is from the database. New lines have been removed from the second one so they don't match even they that are functionally equivalent.

          string(55) "<p>this is html feedback</p>
          <p>and its multiline.</p>"
          string(53) "<p>this is html feedback</p><p>and its multiline.</p>"
          
          Show
          Andrew Davis added a comment - - edited This is definitely a problem as of 2.2. If the activity provides multiline feedback the quick feedback text box compresses it down to a single line. The html editor on the grade edit page produces multiline text. When you override the grade on the grader report your feedback gets saved as a single line. When you go to the grade edit page to turn off the override the editor appears to split it out to multiple lines. This makes it look like the feedback has changed preventing you from turning off "override". the first string is coming from the grade edit form. The second is from the database. New lines have been removed from the second one so they don't match even they that are functionally equivalent. string(55) "<p> this is html feedback</p> <p>and its multiline.</p>" string(53) "<p> this is html feedback</p><p>and its multiline.</p>"
          Hide
          Ruslan Kabalin added a comment - - edited

          Added branch. The solution is not to check whether recorded the feedback is equal to the one in the form, just remove override flag if its removal is requested in the form. Note, that the affected snippet does not set override flag (when override is set initially), it is done earlier in update_final_grade function if grade/edit/tree/grade.php form is used.

          Show
          Ruslan Kabalin added a comment - - edited Added branch. The solution is not to check whether recorded the feedback is equal to the one in the form, just remove override flag if its removal is requested in the form. Note, that the affected snippet does not set override flag (when override is set initially), it is done earlier in update_final_grade function if grade/edit/tree/grade.php form is used.
          Hide
          Ruslan Kabalin added a comment - - edited

          Andrew Davis: unless you have a better solution, can you please review mine

          Show
          Ruslan Kabalin added a comment - - edited Andrew Davis: unless you have a better solution, can you please review mine
          Hide
          Andrew Davis added a comment - - edited

          Looks good to me. I'll add some testing instructions.

          Show
          Andrew Davis added a comment - - edited Looks good to me. I'll add some testing instructions.
          Hide
          Andrew Davis added a comment -

          Added testing instructions. Are you able to submit this for integration? Also, make sure to create versions for both 2.1 and 2.2 stable.

          Show
          Andrew Davis added a comment - Added testing instructions. Are you able to submit this for integration? Also, make sure to create versions for both 2.1 and 2.2 stable.
          Hide
          Ruslan Kabalin added a comment -

          Thanks Andrew. I have added patch for stable branches.

          Show
          Ruslan Kabalin added a comment - Thanks Andrew. I have added patch for stable branches.
          Hide
          Sam Hemelryk added a comment -

          Thanks guys, this has been integrated now

          Show
          Sam Hemelryk added a comment - Thanks guys, this has been integrated now
          Hide
          Michael de Raadt added a comment -

          Test result: Success

          Tested in master (old assignment module), 2.2 and 2.1.

          I tried testing this with the new assignment module, but ran into some errors (which I will report separately).

          Thanks for working on that.

          Show
          Michael de Raadt added a comment - Test result: Success Tested in master (old assignment module), 2.2 and 2.1. I tried testing this with the new assignment module, but ran into some errors (which I will report separately). Thanks for working on that.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          This is now part of Moodle and a few millions people around the globe will be using it soon. Isn't that awesome?

          Many, many thanks and don't forget http://youtu.be/4N7dPaP5Z8U

          Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - This is now part of Moodle and a few millions people around the globe will be using it soon. Isn't that awesome? Many, many thanks and don't forget http://youtu.be/4N7dPaP5Z8U Closing, ciao

            People

            • Votes:
              9 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: