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

Marking Guide's get_or_create_instance() returns instances of other AGM's

    XMLWordPrintable

    Details

    • Testing Instructions:
      Hide

      Steps to reproduce:
      1. Add print_object($rs); to grade/grading/form/guide/lib.php after if ($rs = $DB->get_record... (it should be 568 line approximately).
      2. Install AGM Test module.
      3. Create an Assignment instance and set the grading method to Marking Guide.
      4. Define a form with any criteria and publish it as template.
      5. Create an AGM Test instance and set the grading method to Marking Guide.
      6. Create a form from published template.
      7. Go to Assignment and grade anyone.
      8. Open the grade page for the same student. Look at the print_object() output and remember the definitionid and itemid values.
      9. Go to AGM Test, fill the Item ID field with value shown in Assignment and submit the form.
      10. Look at the print_object() output. There is Assignment's definitionid.

      Expected result:
      On step 10 no print_object() output should be shown. The execution should go by other branch (where new instance is created because previous instance not found).

      You could also reproduce the issue using unit-tests:
      1. Run vendor/bin/phpunit gradingform_guide_testcase grade/grading/form/guide/tests/guide_test.php
      2. Test will fail

      Testing instructions:
      1. Run vendor/bin/phpunit gradingform_guide_testcase grade/grading/form/guide/tests/guide_test.php
      2. Make sure test passes.

      Show
      Steps to reproduce: 1. Add print_object($rs); to grade/grading/form/guide/lib.php after if ($rs = $DB->get_record... (it should be 568 line approximately). 2. Install AGM Test module. 3. Create an Assignment instance and set the grading method to Marking Guide. 4. Define a form with any criteria and publish it as template. 5. Create an AGM Test instance and set the grading method to Marking Guide. 6. Create a form from published template. 7. Go to Assignment and grade anyone. 8. Open the grade page for the same student. Look at the print_object() output and remember the definitionid and itemid values. 9. Go to AGM Test, fill the Item ID field with value shown in Assignment and submit the form. 10. Look at the print_object() output. There is Assignment's definitionid. Expected result: On step 10 no print_object() output should be shown. The execution should go by other branch (where new instance is created because previous instance not found). You could also reproduce the issue using unit-tests: 1. Run vendor/bin/phpunit gradingform_guide_testcase grade/grading/form/guide/tests/guide_test.php 2. Test will fail Testing instructions: 1. Run vendor/bin/phpunit gradingform_guide_testcase grade/grading/form/guide/tests/guide_test.php 2. Make sure test passes.
    • Affected Branches:
      MOODLE_27_STABLE, MOODLE_28_STABLE
    • Fixed Branches:
      MOODLE_27_STABLE, MOODLE_28_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      wip_MDL-49327_2.9_guide_getinstance

      Description

      When $instanceid is zero, the get_or_create_instance() function tries to find an existing marking guide instance for the specified $raterid and $itemid in DB, but it doesn't check the definitionid (note: the definition is attached to gradeable area which is attached to a context).

      Unfortunately there is no way to reproduce this bug, because only Assignment supports AGM for now. In order you are need at least two activity modules with AGM support and define the guide in both of it. Furthermore, these modules must use the same $itemid during grading. After that, you are need to grade the submission in first module. Then, during grading the submission in second module, the guide form will be filled with the values of first module form. It happens because the get_or_create_instance() function just took the first record it found ignoring the definitionid though the definitionid is only field which represents the context there (see note above).

      How it's done in guide (works incorrectly):

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

      How it's done in rubric (works correctly):

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

        Attachments

          Issue Links

            Activity

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  11/May/15