Moodle
  1. Moodle
  2. MDL-33537

Rubric grading inconsistencies due to gradingform_rubric_controller get_or_create_instance() method

    Details

    • Testing Instructions:
      Hide
      1. Delete mod_assign and mod_assignment modules and install them again (it will be easier doing this, is to make both modules use the same grading instance itemid, this way will be 1)
      2. Enable the assignment 2.2
      3. Create a course and assign a student
      4. Create a new assignment instance with rubrics as a grading method
      5. Create an old assignment instance with rubrics as a grading method
      6. Create a rubric for the new assignment with 1 criteria and 4 levels with values 1,2,3,4
      7. Create a rubric for the old assignment with 1 criteria and 4 levels with values 10,20,30,40
      8. Go to the new assignment; grade the student, add a "AAAA" text on the right box of the criteria and save changes
      9. Go to the old assignment; grade the student, add a "BBBB" text on the right box of the criteria and save changes
      10. Return to the student grading page of the new assignment, you SHOULD see the previously marked grade and the "AAAA", and you SHOULD NOT see a message "NOTE: The last attempt to grade this person was not saved properly so draft grades have been restored. If you want to cancel these changes use the 'Cancel' button below." Edit the grading and save changes.
      11. Return to the student grading page of the old assignment, you SHOULD see the previously marked grade and the "BBBB", and you SHOULD NOT see a message "NOTE: The last attempt to grade this person was not saved properly so draft grades have been restored. If you want to cancel these changes use the 'Cancel' button below."
      Show
      Delete mod_assign and mod_assignment modules and install them again (it will be easier doing this, is to make both modules use the same grading instance itemid, this way will be 1) Enable the assignment 2.2 Create a course and assign a student Create a new assignment instance with rubrics as a grading method Create an old assignment instance with rubrics as a grading method Create a rubric for the new assignment with 1 criteria and 4 levels with values 1,2,3,4 Create a rubric for the old assignment with 1 criteria and 4 levels with values 10,20,30,40 Go to the new assignment; grade the student, add a "AAAA" text on the right box of the criteria and save changes Go to the old assignment; grade the student, add a "BBBB" text on the right box of the criteria and save changes Return to the student grading page of the new assignment, you SHOULD see the previously marked grade and the "AAAA", and you SHOULD NOT see a message "NOTE: The last attempt to grade this person was not saved properly so draft grades have been restored. If you want to cancel these changes use the 'Cancel' button below." Edit the grading and save changes. Return to the student grading page of the old assignment, you SHOULD see the previously marked grade and the "BBBB", and you SHOULD NOT see a message "NOTE: The last attempt to grade this person was not saved properly so draft grades have been restored. If you want to cancel these changes use the 'Cancel' button below."
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-33537_master
    • Rank:
      41462

      Description

      While working on using advanced grading methods for another activity I discovered some inconsistencies when grading with rubric. Sometimes activities for user that had been graded wouldn't properly load the graded rubric (it would load a blank rubric) and sometimes activities that hadn't been graded would load a partially filled rubric.

      I tracked it down to the gradingform_rubric_controller's get_or_create_instance($instanceid, $raterid, $itemid) method. When $instanceid is 0 and $raterid and $itemid are not, this query gets executed:

      if ($rs = $DB->get_records('grading_instances', array('raterid' => $raterid, 'itemid' => $itemid), 'timemodified DESC', '*', 0, 1)) {
      

      With only one type of activity being used with advanced grading (assignment submissions) this is not a problem because $itemid is guaranteed to be unique because it is an assignment submission id. But with multiple activities being used there may be multiple $itemids that are the same, but do not represent the same activity.

      I believe this issue is simply addressed by adding the definitionid as a parameter to the query:

      if ($rs = $DB->get_records('grading_instances', array('definitionid' => $this->definition->id, 'raterid' => $raterid, 'itemid' => $itemid), 'timemodified DESC', '*', 0, 1)) {
      

        Issue Links

          Activity

          Hide
          Sam Chaffee added a comment -

          Attached diff for proposed fix.

          Show
          Sam Chaffee added a comment - Attached diff for proposed fix.
          Hide
          Michael de Raadt added a comment -

          Thanks for reporting that and providing a solution.

          Show
          Michael de Raadt added a comment - Thanks for reporting that and providing a solution.
          Hide
          David Monllaó added a comment -

          Thanks for the patch Sam.

          I've reproduced the problem with the new and the old assignment modules and added testing instructions. Requesting peer review

          Show
          David Monllaó added a comment - Thanks for the patch Sam. I've reproduced the problem with the new and the old assignment modules and added testing instructions. Requesting peer review
          Hide
          Andrew Davis added a comment -

          It seems sensible. Put this up for integration whenever you are ready.

          Show
          Andrew Davis added a comment - It seems sensible. Put this up for integration whenever you are ready.
          Hide
          Dan Poltawski added a comment -

          Thanks David/Sam, this has been integrated now

          Show
          Dan Poltawski added a comment - Thanks David/Sam, this has been integrated now
          Hide
          Rossiani Wijaya added a comment -

          This looks good.

          Test passed.

          Show
          Rossiani Wijaya added a comment - This looks good. Test passed.
          Hide
          Dan Poltawski added a comment -

          asko, Дзякуй, ধন্যবাদ, Благодаря, Gràcies, 感谢, 謝謝, Hvala, Díky, Tak, Bedankt, Tänan, متشکریم, Salamat, Kiitokset, Merci, Grazas, Danke, Ευχαριστώ, આભાર, תודה, धन्यवाद, Köszönjük, Takk fyrir, Terima Kasih, Grazie, ありがとうございます, Рахмет, សូមអរគុណ, 감사합니다, gratiās, Pateicamies, Ačiū, Благодарам, Tēnā koa, Kia Ora Rawa Atu, आभारी आहोत, Талархал, Takk, Dziękuję, Obrigado, Mulţumesc, Engraziel, Спасибо, Fa'afetai, Хвала, Hvala, ස්තූතියි, Vďaka, Hvala, Mahadsanid, Thanks, Gracias, Tack, Salamat, நன்றி, నెనరులు, ขอบคุณค่ะ!

          Your work has made it into this weeks Moodle release! There are no gold medals available this week - but millions around the world will benefit. Thank you!

          Show
          Dan Poltawski added a comment - asko, Дзякуй, ধন্যবাদ, Благодаря, Gràcies, 感谢, 謝謝, Hvala, Díky, Tak, Bedankt, Tänan, متشکریم, Salamat, Kiitokset, Merci, Grazas, Danke, Ευχαριστώ, આભાર, תודה, धन्यवाद, Köszönjük, Takk fyrir, Terima Kasih, Grazie, ありがとうございます, Рахмет, សូមអរគុណ, 감사합니다, gratiās, Pateicamies, Ačiū, Благодарам, Tēnā koa, Kia Ora Rawa Atu, आभारी आहोत, Талархал, Takk, Dziękuję, Obrigado, Mulţumesc, Engraziel, Спасибо, Fa'afetai, Хвала, Hvala, ස්තූතියි, Vďaka, Hvala, Mahadsanid, Thanks, Gracias, Tack, Salamat, நன்றி, నెనరులు, ขอบคุณค่ะ! Your work has made it into this weeks Moodle release! There are no gold medals available this week - but millions around the world will benefit. Thank you!

            People

            • Votes:
              2 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: