MDL-10524 fixed aggregation for course item + other minor improvements

diff --git a/grade/report/grader/index.php b/grade/report/grader/index.php
index 0388e8f177296ffebe66cafada3821d5046dbe17..01026c45b629117d6dfe322e1ddcf5f04a470721 100644 (file)
--- a/grade/report/grader/index.php
+++ b/grade/report/grader/index.php
@@ -20,6 +20,9 @@ if (!is_null($toggle) && !empty($toggle_type)) {
     set_user_preferences(array('grade_report_show' . $toggle_type => $toggle));
 }
 
+//first make sure we have proper final grades - this must be done before constructing of the grade tree
+grade_regrade_final_grades($courseid);
+
 // Initialise the grader report object
 $report = new grade_report_grader($courseid, $gpr, $context, $page, $sortitemid);
 
@@ -38,10 +41,6 @@ if (!empty($target) && !empty($action) && confirm_sesskey()) {
     $report->process_action($target, $action);
 }
 
-// first make sure we have all final grades
-// TODO: check that no grade_item has needsupdate set
-grade_regrade_final_grades($courseid);
-
 $report->load_users();
 $numusers = $report->get_numusers();
 $report->load_final_grades();

diff --git a/grade/report/grader/lib.php b/grade/report/grader/lib.php
index f4339a88b13c095876ccb15f7bf3bfb7c8a0dcd4..a6c8c7754874c739f573dbdd26131bf59400ae0f 100644 (file)
--- a/grade/report/grader/lib.php
+++ b/grade/report/grader/lib.php
@@ -600,7 +600,7 @@ class grade_report_grader extends grade_report {
 
                 // Do not show any icons if no grade (no record in DB to match)
                 // TODO: change edit/hide/etc. links to use itemid and userid to allow creating of new grade objects
-                if (!empty($grade->id) && $USER->gradeediting) {
+                if (!$item->needsupdate and !empty($grade->id) and $USER->gradeediting) {
                     $states = array('is_hidden' => $item->hidden,
                                     'is_locked' => $item->locked,
                                     'is_editable' => $item->gradetype != GRADE_TYPE_NONE && !$grade->locked && !$item->locked);
@@ -610,7 +610,10 @@ class grade_report_grader extends grade_report {
                 // if in editting mode, we need to print either a text box
                 // or a drop down (for scales)
                 // grades in item of type grade category or course are not directly editable
-                if ($USER->gradeediting) {
+                if ($item->needsupdate) {
+                    $studentshtml .= '<span class="gradingerror">'.get_string('error').'</span>';
+
+                } else if ($USER->gradeediting) {
                     // We need to retrieve each grade_grade object from DB in order to
                     // know if they are hidden/locked
 
@@ -692,7 +695,10 @@ class grade_report_grader extends grade_report {
                         $studentshtml .= '<span onmouseover="' . $overlib . '" onmouseout="return nd();">';
                     }
 
-                    if ($gradedisplaytype == GRADE_REPORT_GRADE_DISPLAY_TYPE_LETTER) {
+                    if ($item->needsupdate) {
+                        $studentshtml .= '<span class="gradingerror">'.get_string('error').'</span>';
+
+                    } else if ($gradedisplaytype == GRADE_REPORT_GRADE_DISPLAY_TYPE_LETTER) {
                         $letters = grade_report::get_grade_letters();
                         if (!is_null($gradeval)) {
                             $studentshtml .= grade_grade::get_letter($letters, $gradeval, $grademin, $grademax);

diff --git a/lib/grade/grade_item.php b/lib/grade/grade_item.php
index cc688811be227f72c8d3e3900ea5fec523bad764..4dab7e6303ba9a879f6190893701bb6f4f6ad26b 100644 (file)
--- a/lib/grade/grade_item.php
+++ b/lib/grade/grade_item.php
@@ -1194,7 +1194,12 @@ class grade_item extends grade_object {
             $result = $grade->update_feedback($feedback, $feedbackformat, $usermodified);
         }
 
-        if (!$this->needsupdate) {
+        if ($this->is_course_item() and !$this->needsupdate) {
+            if (!grade_regrade_final_grades($this->courseid, $userid, $this)) {
+                $this->force_regrading();
+            }
+
+        } else if (!$this->needsupdate) {
             $course_item = grade_item::fetch_course_item($this->courseid);
             if (!$course_item->needsupdate) {
                 if (!grade_regrade_final_grades($this->courseid, $userid, $this)) {

diff --git a/lib/gradelib.php b/lib/gradelib.php
index e0176899324d5d60eb0f80e348ad89f916a6a14d..e387db67ba76292df67e7208dbd04724d134dd0c 100644 (file)
--- a/lib/gradelib.php
+++ b/lib/gradelib.php
@@ -351,25 +351,27 @@ function grade_regrade_final_grades($courseid, $userid=null, $updated_item=null)
         if (!empty($updated_item) and $updated_item->id == $gid) {
             $grade_items[$gid]->needsupdate = 1;
 
-        } else if ($grade_items[$gid]->is_category_item() or $grade_items[$gid]->is_calculated()) {
+        } else if ($grade_items[$gid]->is_course_item() or $grade_items[$gid]->is_category_item() or $grade_items[$gid]->is_calculated()) {
             $grade_items[$gid]->needsupdate = 1;
         }
 
         // construct depends_on lookup array
         $depends_on[$gid] = $grade_items[$gid]->depends_on();
     }
-    $errors = array();
 
-    $finalitems = array();
+    $errors = array();
     $finalids = array();
-    while (count($grade_items) > 0) {
-        $count = 0; // count how many items were updated in this cycle
-        foreach ($grade_items as $gid=>$gitem) {
-            $grade_item =& $grade_items[$gid];
-            if (!$grade_item->needsupdate) {
-                $finalitems[$gid] = $grade_item;
-                $finalids[] = $gid;
-                unset($grade_items[$gid]);
+    $gids     = array_keys($grade_items);
+
+    while (count($finalids) < count($gids)) { // work until all grades are final or error found
+        $count = 0;
+        foreach ($gids as $gid) {
+            if (in_array($gid, $finalids)) {
+                continue; // already final
+            }
+
+            if (!$grade_items[$gid]->needsupdate) {
+                $finalids[] = $gid; // we can make it final - does not need update
                 continue;
             }
 
@@ -377,33 +379,34 @@ function grade_regrade_final_grades($courseid, $userid=null, $updated_item=null)
             foreach ($depends_on[$gid] as $did) {
                 if (!in_array($did, $finalids)) {
                     $doupdate = false;
-                    break;
+                    continue; // this item depends on something that is not yet in finals array
                 }
             }
 
             //oki - let's update, calculate or aggregate :-)
             if ($doupdate) {
-                $result = $grade_item->regrade_final_grades($userid);
+                $result = $grade_items[$gid]->regrade_final_grades($userid);
 
                 if ($result === true) {
-                    $grade_item->regrading_finished();
+                    $grade_items[$gid]->regrading_finished();
                     $count++;
-                    $finalitems[$gid] = $grade_item;
                     $finalids[] = $gid;
-                    unset($grade_items[$gid]);
                 } else {
-                    $grade_item->force_regrading();
+                    $grade_items[$gid]->force_regrading();
                     $errors[$gid] = $result;
                 }
             }
         }
 
         if ($count == 0) {
-            foreach($grade_items as $grade_item) {
-                $grade_item->force_regrading();
-                $errors[$grade_item->id] = 'Probably circular reference or broken calculation formula'; // TODO: localize
+            foreach($gids as $gid) {
+                if (in_array($gid, $finalids)) {
+                    continue; // this one is ok
+                }
+                $grade_items[$gid]->force_regrading();
+                $errors[$grade_items[$gid]->id] = 'Probably circular reference or broken calculation formula'; // TODO: localize
             }
-            break;
+            break; // oki, found error
         }
     }