MDL-53279 grades: Recalculate only required items
authorDavid Monllao <davidm@moodle.com>
Mon, 29 Feb 2016 02:31:56 +0000 (10:31 +0800)
committerDavid Monllao <davidm@moodle.com>
Tue, 15 Mar 2016 04:18:23 +0000 (12:18 +0800)
Skip grade categories and calculated grade items that do not depend
on .

grade/report/grader/tests/behat/ajax_grader.feature
lib/gradelib.php

index 880a341..d341ffd 100644 (file)
@@ -25,22 +25,25 @@ Feature: Using the AJAX grading feature of Grader report to update grades and fe
       | name       | scale                                  |
       | Test Scale | Disappointing,Good,Very good,Excellent |
     And the following "grade categories" exist:
-      | fullname | course |
-      | Grade Cat | C1 |
+      | fullname  | course |
+      | Grade Cat | C1     |
+    And the following "grade categories" exist:
+      | fullname  | course | gradecategory |
+      | Grade Sub Cat  | C1 | Grade Cat |
     And the following "grade items" exist:
       | itemname | course | locked | gradetype | gradecategory |
-      | Item 1 | C1 | 0 | value | Grade Cat |
+      | Item 1  | C1 | 0 | value | Grade Cat |
       | Item VU | C1 | 0 | value | Grade Cat |
       | Item VL | C1 | 1 | value | Grade Cat |
-      | Item TU | C1 | 0 | text | Grade Cat |
-      | Item TL | C1 | 1 | text | Grade Cat |
-    And the following "grade items" exist:
-      | itemname | course | locked | gradetype | scale | gradecategory |
-      | Item SU | C1 | 0 | scale | Test Scale | Grade Cat |
-      | Item SL | C1 | 1 | scale | Test Scale | Grade Cat |
+      | Item TU | C1 | 0 | text  | Grade Cat |
+      | Item TL | C1 | 1 | text  | Grade Cat |
+      | Item 3  | C1 | 0 | value | Grade Cat |
+      | Calc Item  | C1 | 0 | value | Grade Cat     |
+      | Item VUSub | C1 | 0 | value | Grade Sub Cat |
     And the following "grade items" exist:
-      | itemname | course | locked | gradetype | gradecategory |
-      | Item 3 | C1 | 0 | value | Grade Cat |
+      | itemname   | course | locked | gradetype | scale | gradecategory |
+      | Item SU    | C1 | 0 | scale | Test Scale | Grade Cat |
+      | Item SL    | C1 | 1 | scale | Test Scale | Grade Cat |
     And the following config values are set as admin:
       | grade_report_showaverages | 0 |
       | grade_report_enableajax | 1 |
@@ -71,7 +74,7 @@ Feature: Using the AJAX grading feature of Grader report to update grades and fe
     And I set the field "ajaxgrade" to "Very good"
     And I press key "13" in the field "ajaxgrade"
     And the following should exist in the "user-grades" table:
-      | -1-                | -4-      | -5-      | -9-       | -13-         |
+      | -1-                | -6-      | -7-      | -13-      | -16-         |
       | Student 2          | -        | 33.00    | -         | 33.00        |
       | Student 3          | 80.00    | 50.00    | Very good | 133.00       |
     And I click on student "Student 3" for grade item "Item VL"
@@ -89,14 +92,14 @@ Feature: Using the AJAX grading feature of Grader report to update grades and fe
     And I set the field "ajaxgrade" to "90"
     And I press key "13" in the field "ajaxgrade"
     And the following should exist in the "user-grades" table:
-      | -1-                | -13-      |
+      | -1-                | -16-      |
       | Student 1          | 90.00     |
     And I navigate to "Grader report" node in "Grade administration"
     And the following should exist in the "user-grades" table:
-      | -1-                | -4-      | -5-      | -9-       | -13-         |
-      | Student 1          | -        | -        | -         | 90.00        |
-      | Student 2          | -        | 33.00    | -         | 33.00        |
-      | Student 3          | 80.00    | 50.00    | Very good | 133.00       |
+      | -1-                | -6-   | -7-   | -13-      | -16-      |
+      | Student 1          | -     | -     | -         | 90.00     |
+      | Student 2          | -     | 33.00 | -         | 33.00     |
+      | Student 3          | 80.00 | 50.00 | Very good | 133.00    |
 
   @javascript
   Scenario: Use the grader report without editing, with AJAX and quick feedback on
@@ -126,8 +129,8 @@ Feature: Using the AJAX grading feature of Grader report to update grades and fe
     And I press key "13" in the field "ajaxfeedback"
     And I navigate to "Grader report" node in "Grade administration"
     And the following should exist in the "user-grades" table:
-      | -1-                | -5-      | -9-       | -13-         |
-      | Student 2          | 33.00    | Very good | 36.00        |
+      | -1-       | -7-   | -13-      | -16-  |
+      | Student 2 | 33.00 | Very good | 36.00 |
     And I click on student "Student 3" for grade item "Item TU"
     And the field "ajaxfeedback" matches value "Student 3 TU feedback"
     And I click on student "Student 2" for grade item "Item SU"
@@ -150,8 +153,8 @@ Feature: Using the AJAX grading feature of Grader report to update grades and fe
     And I should not see a grade field for "Student 3" and grade item "Course total"
     And I should not see a feedback field for "Student 3" and grade item "Course total"
     And the following should exist in the "user-grades" table:
-      | -1-                | -5-      | -13-    |
-      | Student 2          | 33.00    | 33.00   |
+      | -1-         | -7-      | -16-    |
+      | Student 2   | 33.00    | 33.00   |
 
   @javascript
   Scenario: Use the grader report with editing, with AJAX and quick feedback on, with category override
@@ -179,8 +182,8 @@ Feature: Using the AJAX grading feature of Grader report to update grades and fe
     And the grade for "Student 2" in grade item "Course total" should match "53.00"
     And I turn editing mode off
     And the following should exist in the "user-grades" table:
-      | -1-                | -4-      | -5-     | -9-       | -12-     | -13-    |
-      | Student 2          | 30.00    | 20.00   | Very good | 53.00    | 53.00   |
+      | -1-        | -6-      | -7-     | -13-      | -15-     | -16-    |
+      | Student 2  | 30.00    | 20.00   | Very good | 53.00    | 53.00   |
     And I click on student "Student 2" for grade item "Item 1"
     And the field "ajaxfeedback" matches value "Some feedback"
 
@@ -193,20 +196,41 @@ Feature: Using the AJAX grading feature of Grader report to update grades and fe
     And I follow "Course 1"
     And I navigate to "Grades" node in "Course administration"
     And I turn editing mode on
+    And I change window size to "large"
+    And I set "=[[i1]] + [[i3]] + [[gsc]]" calculation for grade item "Calc Item" with idnumbers:
+      | Item 1        | i1  |
+      | Item 3        | i3  |
+      | Grade Sub Cat | gsc |
     Then I should not see a grade field for "Student 2" and grade item "Course total"
     And I should not see a feedback field for "Student 2" and grade item "Course total"
     And I give the grade "20.00" to the user "Student 2" for the grade item "Item VU"
     And I click away from student "Student 2" and grade item "Item VU" value
+    And the following should exist in the "user-grades" table:
+      | -1-        | -15-   | -16-  |
+      | Student 2  | 20.00  | 20.00 |
     And I give the grade "30.00" to the user "Student 2" for the grade item "Item 1"
     And I click away from student "Student 2" and grade item "Item 1" value
+    And the following should exist in the "user-grades" table:
+      | -1-        | -15-  | -16-  |
+      | Student 2  | 80.00 | 80.00 |
+    And the field "Student 2 Calc Item grade" matches value "30.00"
+    And I give the grade "5.00" to the user "Student 2" for the grade item "Item 3"
+    And I click away from student "Student 2" and grade item "Item 3" value
+    And the following should exist in the "user-grades" table:
+      | -1-        | -15-  | -16- |
+      | Student 2  | 90.00 | 90.00 |
+    And the field "Student 2 Calc Item grade" matches value "35.00"
+    And I give the grade "10.00" to the user "Student 2" for the grade item "Item VUSub"
+    And I click away from student "Student 2" and grade item "Item VUSub" value
+    And the following should exist in the "user-grades" table:
+      | -1-        | -5-   | -15-   | -16-   |
+      | Student 2  | 10.00 | 110.00 | 110.00 |
+    And the field "Student 2 Calc Item grade" matches value "45.00"
     And I give the feedback "Some feedback" to the user "Student 2" for the grade item "Item 1"
     And I click away from student "Student 2" and grade item "Item 1" feedback
-    And the following should exist in the "user-grades" table:
-      | -1-                | -13-     |
-      | Student 2          | 50.00    |
     And I turn editing mode off
     And the following should exist in the "user-grades" table:
-      | -1-                | -4-      | -5-      | -13-         |
-      | Student 2          | 30.00    | 20.00    | 50.00        |
+      | -1-        | -4-   | -6-   | -7-   | -11- | -12-  | -15-   | -16-   |
+      | Student 2  | 10.00 | 30.00 | 20.00 | 5.00 | 45.00 | 110.00 | 110.00 |
     And I click on student "Student 2" for grade item "Item 1"
     And the field "ajaxfeedback" matches value "Some feedback"
index c50ecd9..5eaa301 100644 (file)
@@ -1140,37 +1140,30 @@ function grade_regrade_final_grades($courseid, $userid=null, $updated_item=null,
         }
     }
 
+    $progresstotal = 0;
+    $progresscurrent = 0;
+
     $grade_items = grade_item::fetch_all(array('courseid'=>$courseid));
     $depends_on = array();
 
-    // first mark all category and calculated items as needing regrading
-    // this is slower, but 100% accurate
     foreach ($grade_items as $gid=>$gitem) {
-        if (!empty($updated_item) and $updated_item->id == $gid) {
-            $grade_items[$gid]->needsupdate = 1;
-
-        } else if ($gitem->is_course_item() or $gitem->is_category_item() or $gitem->is_calculated()) {
+        if ((!empty($updated_item) and $updated_item->id == $gid) ||
+                $gitem->is_course_item() || $gitem->is_category_item() || $gitem->is_calculated()) {
             $grade_items[$gid]->needsupdate = 1;
         }
 
-        // construct depends_on lookup array
-        $depends_on[$gid] = $grade_items[$gid]->depends_on();
-    }
-
-    $progresstotal = 0;
-    $progresscurrent = 0;
-
-    // This progress total might not be 100% accurate, because more things might get marked as needsupdate
-    // during the process.
-    foreach ($grade_items as $item) {
-        if ($item->needsupdate) {
+        // We load all dependencies of these items later we can discard some grade_items based on this.
+        if ($grade_items[$gid]->needsupdate) {
+            $depends_on[$gid] = $grade_items[$gid]->depends_on();
             $progresstotal++;
         }
     }
+
     $progress->start_progress('regrade_course', $progresstotal);
 
     $errors = array();
     $finalids = array();
+    $updatedids = array();
     $gids     = array_keys($grade_items);
     $failed = 0;
 
@@ -1196,28 +1189,51 @@ function grade_regrade_final_grades($courseid, $userid=null, $updated_item=null,
             $progress->progress($thisprogress);
             $progresscurrent = $thisprogress;
 
-            $doupdate = true;
             foreach ($depends_on[$gid] as $did) {
                 if (!in_array($did, $finalids)) {
-                    $doupdate = false;
-                    continue; // this item depends on something that is not yet in finals array
+                    // This item depends on something that is not yet in finals array.
+                    continue 2;
                 }
             }
 
-            //oki - let's update, calculate or aggregate :-)
-            if ($doupdate) {
-                $result = $grade_items[$gid]->regrade_final_grades($userid);
+            // If $updated_item was specified we discard the grade items that do not depend on it or on any grade item that
+            // depend on $updated_item.
+            if (!empty($updated_item) && $gid != $updated_item->id && !in_array($updated_item->id, $depends_on[$gid])) {
+                // We need to ensure that non of this item dependencies has been updated.
 
-                if ($result === true) {
-                    $grade_items[$gid]->regrading_finished();
-                    $grade_items[$gid]->check_locktime(); // do the locktime item locking
-                    $count++;
+                $updateddependencies = false;
+                foreach ($depends_on[$gid] as $dependency) {
+                    if (in_array($dependency, $updatedids)) {
+                        $updateddependencies = true;
+                    }
+                }
+                if ($updateddependencies === false) {
+                    // No need to regrade it.
                     $finalids[] = $gid;
+                    continue;
+                }
+            }
+
+            //oki - let's update, calculate or aggregate :-)
+            $result = $grade_items[$gid]->regrade_final_grades($userid);
+
+            if ($result === true) {
 
+                // We should only update the database if we regraded all users.
+                if (empty($userid)) {
+                    $grade_items[$gid]->regrading_finished();
+                    // Do the locktime item locking.
+                    $grade_items[$gid]->check_locktime();
                 } else {
-                    $grade_items[$gid]->force_regrading();
-                    $errors[$gid] = $result;
+                    $grade_items[$gid]->needsupdate = 0;
                 }
+                $count++;
+                $finalids[] = $gid;
+                $updatedids[] = $gid;
+
+            } else {
+                $grade_items[$gid]->force_regrading();
+                $errors[$gid] = $result;
             }
         }