MDL-11718 aggregated grades which depend on hidden grades are now hidden too + cleanu...

diff --git a/grade/lib.php b/grade/lib.php
index 13254d1746b8d9c54ed42e26a50467a1b43b8aea..d60ef42bd87d14b444b416c4f277e0123385720c 100644 (file)
--- a/grade/lib.php
+++ b/grade/lib.php
@@ -735,7 +735,7 @@ class grade_seq {
         $this->elements = grade_seq::flatten($top_element, $category_grade_last, $nooutcomes);
 
         foreach ($this->elements as $key=>$unused) {
-            $this->items[$key] =& $this->elements[$key]['object'];
+            $this->items[$this->elements[$key]['object']->id] =& $this->elements[$key]['object'];
         }
     }
 
@@ -872,6 +872,11 @@ class grade_tree {
      */
     var $context;
 
+    /**
+     * Grade items
+     */
+    var $items;
+
     /**
      * Constructor, retrieves and stores a hierarchical array of all grade_category and grade_item
      * objects for the given courseid. Full objects are instantiated. Ordering sequence is fixed if needed.
@@ -1011,6 +1016,7 @@ class grade_tree {
             $element['eid'] = 'c'.$element['object']->id;
         } else if (in_array($element['type'], array('item', 'courseitem', 'categoryitem'))) {
             $element['eid'] = 'i'.$element['object']->id;
+            $this->items[$element['object']->id] =& $element['object'];
         }
 
         $levels[$depth][] =& $element;

diff --git a/grade/report/grader/lib.php b/grade/report/grader/lib.php
index 4fe96a66e4595bcc85dbaa9784092ba5436624fb..f4bd9557985bdc2160ed43eb81c77e79cda7965b 100644 (file)
--- a/grade/report/grader/lib.php
+++ b/grade/report/grader/lib.php
@@ -38,15 +38,9 @@ require_once($CFG->libdir.'/tablelib.php');
 class grade_report_grader extends grade_report {
     /**
      * The final grades.
-     * @var array $finalgrades
+     * @var array $grades
      */
-    var $finalgrades;
-
-    /**
-     * The grade items.
-     * @var array $items
-     */
-    var $items;
+    var $grades;
 
     /**
      * Array of errors for bulk grades updating.
@@ -341,14 +335,29 @@ class grade_report_grader extends grade_report {
         global $CFG;
 
         // please note that we must fetch all grade_grades fields if we want to contruct grade_grade object from it!
-        $sql = "SELECT g.*, gi.grademin, gi.grademax
+        $sql = "SELECT g.*
                   FROM {$CFG->prefix}grade_items gi,
                        {$CFG->prefix}grade_grades g
-                 WHERE g.itemid = gi.id AND gi.courseid = $this->courseid $this->userselect";
+                 WHERE g.itemid = gi.id AND gi.courseid = {$this->courseid} {$this->userselect}";
+
+        $userids = array_keys($this->users);
 
         if ($grades = get_records_sql($sql)) {
-            foreach ($grades as $grade) {
-                $this->finalgrades[$grade->userid][$grade->itemid] = $grade;
+            foreach ($grades as $graderec) {
+                if (in_array($graderec->userid, $userids)) {
+                    $this->grades[$graderec->userid][$graderec->itemid] = new grade_grade($graderec, false);
+                    $this->grades[$graderec->userid][$graderec->itemid]->grade_item =& $this->gtree->items[$graderec->itemid]; // db caching
+                }
+            }
+        }
+
+        // prefil grades that do not exist yet
+        foreach ($userids as $userid) {
+            foreach ($this->gtree->items as $itemid=>$unused) {
+                if (!isset($this->grades[$userid][$itemid])) {
+                    $this->grades[$userid][$itemid] = new grade_grade();
+                    $this->grades[$userid][$itemid]->grade_item =& $this->gtree->items[$itemid]; // db caching
+                }
             }
         }
     }
@@ -570,7 +579,6 @@ class grade_report_grader extends grade_report {
                         $icon = '<img src="'.$CFG->modpixpath.'/'.$object->itemmodule.'/icon.gif" class="icon" alt="'
                               .$this->get_lang_string('modulename', $object->itemmodule).'"/>';
                     } else if ($object->itemtype == 'manual') {
-                        //TODO: add manual grading icon
                         $icon = '<img src="'.$CFG->pixpath.'/t/edit.gif" class="icon" alt="'
                                 .$this->get_lang_string('manualgrade', 'grades') .'"/>';
                     }
@@ -578,8 +586,6 @@ class grade_report_grader extends grade_report {
                     $headerlink = $this->get_module_link($element['object']->get_name(), $itemmodule, $iteminstance, $element['object']->is_hidden());
                     $headerhtml .= '<th class="header '.$columnclass.' '.$type.$catlevel.$dimmed.'" scope="col">'. $headerlink . $arrow;
                     $headerhtml .= $this->get_icons($element) . '</th>';
-
-                    $this->items[$element['object']->sortorder] =& $element['object'];
                 }
 
             }
@@ -605,7 +611,7 @@ class grade_report_grader extends grade_report {
         // Preload scale objects for items with a scaleid
         $scales_list = '';
         $tabindices = array();
-        foreach ($this->items as $item) {
+        foreach ($this->gtree->items as $item) {
             if (!empty($item->scaleid)) {
                 $scales_list .= "$item->scaleid,";
             }
@@ -623,6 +629,13 @@ class grade_report_grader extends grade_report {
         $canviewhidden = has_capability('moodle/grade:viewhidden', get_context_instance(CONTEXT_COURSE, $this->course->id));
 
         foreach ($this->users as $userid => $user) {
+
+            if ($canviewhidden) {
+                $hiding_affected = array();
+            } else {
+                $hiding_affected = grade_grade::get_hiding_affected($this->grades[$userid], $this->gtree->items);
+            }
+
             $columncount = 0;
             // Student name and link
             $user_pic = null;
@@ -634,26 +647,19 @@ class grade_report_grader extends grade_report {
                           . '<a href="' . $CFG->wwwroot . '/user/view.php?id='
                           . $user->id . '">' . fullname($user) . '</a></th>';
 
-            foreach ($this->items as $itemid=>$item) {
+            foreach ($this->gtree->items as $itemid=>$unused) {
+                $item =& $this->gtree->items[$itemid];
+
                 // Get the decimal points preference for this item
                 $decimalpoints = $item->get_decimals();
 
-                if (isset($this->finalgrades[$userid][$item->id])) {
-                    $gradeval = $this->finalgrades[$userid][$item->id]->finalgrade;
-                    $grade = new grade_grade($this->finalgrades[$userid][$item->id], false);
-                    $grade->feedback = stripslashes_safe($this->finalgrades[$userid][$item->id]->feedback);
-                    $grade->feedbackformat = $this->finalgrades[$userid][$item->id]->feedbackformat;
-
-                } else {
-                    $gradeval = null;
-                    $grade = new grade_grade(array('userid'=>$userid, 'itemid'=>$item->id), false);
-                    $grade->feedback = '';
-                }
+                $grade = $this->grades[$userid][$item->id];
+                $gradeval = $grade->finalgrade;
 
                 // MDL-11274
                 // Hide grades in the grader report if the current grader doesn't have 'moodle/grade:viewhidden'
-                if ($grade->is_hidden() && !$canviewhidden) {
-                    if (isset($grade->finalgrade)) {
+                if (!$canviewhidden and ($grade->is_hidden() or in_array($itemid, $hiding_affected))) {
+                    if (!is_null($gradeval) and $grade->is_hidden()) {
                         $studentshtml .= '<td class="cell c'.$columncount++.'">'.userdate($grade->timecreated,get_string('strftimedatetimeshort')).'</td>';
                     } else {
                         $studentshtml .= '<td class="cell c'.$columncount++.'">-</td>';
@@ -661,9 +667,6 @@ class grade_report_grader extends grade_report {
                     continue;
                 }
 
-                $grade->courseid = $this->courseid;
-                $grade->grade_item =& $this->items[$itemid]; // this speedsup is_hidden() and other grade_grade methods
-
                 // emulate grade element
                 $eid = $this->gtree->get_grade_eid($grade);
                 $element = array('eid'=>$eid, 'object'=>$grade, 'type'=>'grade');
@@ -671,10 +674,10 @@ class grade_report_grader extends grade_report {
                 $cellclasses = 'cell c'.$columncount++;
                 if ($item->is_category_item()) {
                     $cellclasses .= ' cat';
-                }  
+                }
                 if ($item->is_course_item()) {
                     $cellclasses .= ' course';
-                }  
+                }
                 if ($grade->is_overridden()) {
                     $cellclasses .= ' overridden';
                 }
@@ -697,8 +700,6 @@ class grade_report_grader extends grade_report {
                     $studentshtml .= '<span class="gradingerror">'.get_string('error').'</span>';
 
                 } else if ($USER->gradeediting[$this->courseid]) {
-                    // We need to retrieve each grade_grade object from DB in order to
-                    // know if they are hidden/locked
 
                     if ($item->scaleid && !empty($scales_array[$item->scaleid])) {
                         $scale = $scales_array[$item->scaleid];
@@ -765,41 +766,20 @@ class grade_report_grader extends grade_report {
                 } else { // Not editing
                     $gradedisplaytype = $item->get_displaytype();
 
-                    $percentsign = '';
-                    $grademin = $item->grademin;
-                    $grademax = $item->grademax;
-
                     // If feedback present, surround grade with feedback tooltip: Open span here
                     if (!empty($grade->feedback)) {
                         $overlib = '';
-                        if ($grade->feedbackformat == 1) {
-                            $overlib = "return overlib('" . s(ltrim($grade->feedback)) . "', FULLHTML);";
-                        } else {
-                            $overlib = "return overlib('" . s($grade->feedback) . "', BORDER, 0, FGCLASS, 'feedback', "
-                                     . "CAPTIONFONTCLASS, 'caption', CAPTION, '$strfeedback');";
-                        }
-
-                        $studentshtml .= '<span onmouseover="' . $overlib . '" onmouseout="return nd();">';
+                        $feedback = addslashes_js(trim(format_string($grade->feedback, $grade->feedbackformat)));
+                        $overlib = "return overlib('$feedback', BORDER, 0, FGCLASS, 'feedback', "
+                                  ."CAPTIONFONTCLASS, 'caption', CAPTION, '$strfeedback');";
+                        $studentshtml .= '<span onmouseover="'.s($overlib).'" onmouseout="return nd();">';
                     }
 
                     if ($item->needsupdate) {
                         $studentshtml .= '<span class="gradingerror">'.get_string('error').'</span>';
-                    } elseif ($item->scaleid && !empty($scales_array[$item->scaleid])) {
-                        $scale = $scales_array[$item->scaleid];
-                        $scales = explode(",", $scale->scale);
 
-                        // invalid grade if gradeval < 1
-                        if ((int) $gradeval < 1) {
-                            $studentshtml .= '-';
-                        } else {
-                            $studentshtml .= $scales[$gradeval-1];
-                        }
                     } else {
-                        if (is_null($gradeval)) {
-                            $studentshtml .= '-';
-                        } else {
-                           $studentshtml .= grade_format_gradevalue($gradeval, $item, true, $gradedisplaytype, null);
-                        }
+                        $studentshtml .= grade_format_gradevalue($gradeval, $item, true, $gradedisplaytype, null);
                     }
 
                     // Close feedback span
@@ -896,7 +876,9 @@ class grade_report_grader extends grade_report {
 
             $columncount=1;
 
-            foreach ($this->items as $item) {
+            foreach ($this->gtree->items as $itemid=>$unused) {
+                $item =& $this->gtree->items[$itemid];
+
                 // If the user shouldn't see this grade_item, hide the average as well
                 // MDL-11576 If any of the grades are hidden and the user doesn't have permission to view them, hide average as well
                 if (!$canviewhidden and $item->is_hidden()) {
@@ -994,7 +976,8 @@ class grade_report_grader extends grade_report {
                        . '<th class="header c0 range" scope="row">'.$this->get_lang_string('range','grades').'</th>';
 
             $columncount = 1;
-            foreach ($this->items as $item) {
+            foreach ($this->gtree->items as $itemid=>$unused) {
+                $item =& $this->gtree->items[$itemid];
 
                 // Determine which display type to use for this average
                 if ($USER->gradeediting[$this->courseid]) {

diff --git a/grade/report/user/lib.php b/grade/report/user/lib.php
index 339ebdc522abd696be47409eead0402c41ee1116..4d7933a91680f9add7fd2a3519601f7e735bf20d 100644 (file)
--- a/grade/report/user/lib.php
+++ b/grade/report/user/lib.php
@@ -127,7 +127,7 @@ class grade_report_user extends grade_report {
         $items =& $this->gseq->items;
         $grades = array();
 
-        $viewhidden = has_capability('moodle/grade:viewhidden', get_context_instance(CONTEXT_COURSE, $this->courseid));
+        $canviewhidden = has_capability('moodle/grade:viewhidden', get_context_instance(CONTEXT_COURSE, $this->courseid));
 
         foreach ($items as $key=>$unused) {
             $grade_item =& $items[$key];
@@ -136,7 +136,11 @@ class grade_report_user extends grade_report {
             $grades[$key]->grade_item =& $grade_item;
         }
 
-        $hiding_affected = grade_grade::get_hiding_affected($grades, $items);
+        if ($canviewhidden) {
+            $hiding_affected = array();
+        } else {
+            $hiding_affected = grade_grade::get_hiding_affected($grades, $items);
+        }
 
         foreach ($items as $key=>$unused) {
             $grade_item  =& $items[$key];
@@ -144,8 +148,6 @@ class grade_report_user extends grade_report {
 
             $data = array();
 
-            // TODO: indicate items that "needsupdate" - missing final calculation
-
             /// prints grade item name
             if ($grade_item->is_course_item() or $grade_item->is_category_item()) {
                 $data[] = '<div class="catname">'.$grade_item->get_name().'</div>';
@@ -164,10 +166,13 @@ class grade_report_user extends grade_report {
                 $excluded = '';
             }
 
-            if (is_null($grade_grade->finalgrade)) {
+            if ($grade_item->needsupdate) {
+                $data[] = '<span class="gradingerror">'.get_string('error').'</span>';
+
+            } else if (is_null($grade_grade->finalgrade)) {
                 $data[] = $excluded . '-';
 
-            } else if (($grade_grade->is_hidden() or in_array($grade_item->id, $hiding_affected)) and !$viewhidden) {
+            } else if (!$canviewhidden and ($grade_grade->is_hidden() or in_array($grade_item->id, $hiding_affected))) {
                 // TODO: optinally do not show anything for hidden grades
                 // $data[] = '-';
                 if ($grade_grade->is_hidden()) {
@@ -181,11 +186,13 @@ class grade_report_user extends grade_report {
             }
 
             /// prints percentage
+            if ($grade_item->needsupdate) {
+                $data[] = '<span class="gradingerror">'.get_string('error').'</span>';
 
-            if (is_null($grade_grade->finalgrade)) {
+            } else if (is_null($grade_grade->finalgrade)) {
                 $data[] = '-';
 
-            } else if (($grade_grade->is_hidden() or in_array($grade_item->id, $hiding_affected)) and !$viewhidden) {
+            } else if (!$canviewhidden and ($grade_grade->is_hidden() or in_array($grade_item->id, $hiding_affected))) {
                 $data[] = '-';
 
             } else {
@@ -193,11 +200,14 @@ class grade_report_user extends grade_report {
             }
 
             /// prints rank
-            if (is_null($grade_grade->finalgrade)) {
+            if ($grade_item->needsupdate) {
+                $data[] = '<span class="gradingerror">'.get_string('error').'</span>';
+
+            } else if (is_null($grade_grade->finalgrade)) {
                 // no grade, no rank
                 $data[] = '-';
 
-            } else if (($grade_grade->is_hidden() or in_array($grade_item->id, $hiding_affected)) and !$viewhidden) {
+            } else if (!$canviewhidden and ($grade_grade->is_hidden() or in_array($grade_item->id, $hiding_affected))) {
                 $data[] = '-';
 
             } else {
@@ -215,7 +225,7 @@ class grade_report_user extends grade_report {
             if (empty($grade_grade->feedback)) {
                 $data[] = '&nbsp;';
 
-            } else if (($grade_grade->is_hidden() or in_array($grade_item->id, $hiding_affected)) and !$viewhidden) {
+            } else if (!$canviewhidden and ($grade_grade->is_hidden() or in_array($grade_item->id, $hiding_affected))) {
                 $data[] = '&nbsp;';
 
             } else {

diff --git a/lib/grade/grade_grade.php b/lib/grade/grade_grade.php
index 2d35722aa3265da127f6ce11277f2c888693ebfe..3c02bf9f633a96facf904828f7ee5ba2af0c5fec 100644 (file)
--- a/lib/grade/grade_grade.php
+++ b/lib/grade/grade_grade.php
@@ -476,11 +476,11 @@ class grade_grade extends grade_object {
      * Return array of grade item ids that are either hidden or indirectly depend
      * on hidden grades, excluded grades are not returned.
      * @static
-     * @param array $grades all course grades of one user
+     * @param array $grades all course grades of one user, & used for better internal caching
      * @param array $items $grade_items array of grade items, & used for better internal caching
      * @return array
      */
-    function get_hiding_affected($grade_grades, &$grade_items) {
+    function get_hiding_affected(&$grade_grades, &$grade_items) {
         if (count($grade_grades) !== count($grade_items)) {
             error("Incorrent size of arrays in params of grade_grade::get_hiding_affected()!");
         }

diff --git a/lib/grade/grade_item.php b/lib/grade/grade_item.php
index 0882c71d684e08b10ea2f52adc31c11c8a11fc9c..4140ad2191a86977bbec67b6dab418b3d6f8bf23 100644 (file)
--- a/lib/grade/grade_item.php
+++ b/lib/grade/grade_item.php
@@ -238,6 +238,11 @@ class grade_item extends grade_object {
      */
     var $needsupdate = 1;
 
+    /**
+     * Cached dependson array
+     */
+    var $dependson_cache = null;
+
     /**
      * In addition to update() as defined in grade_object, handle the grade_outcome and grade_scale objects.
      * Force regrading if necessary
@@ -245,6 +250,9 @@ class grade_item extends grade_object {
      * @return boolean success
      */
     function update($source=null) {
+        // reset caches
+        $this->dependson_cache = null;
+
         // Retrieve scale and infer grademax/min from it if needed
         $this->load_scale();
 
@@ -1149,27 +1157,38 @@ class grade_item extends grade_object {
 
     /**
      * Finds out on which other items does this depend directly when doing calculation or category agregation
+     * @param bool $reset_cache
      * @return array of grade_item ids this one depends on
      */
-    function depends_on() {
+    function depends_on($reset_cache=false) {
         global $CFG;
 
+        if ($reset_cache) {
+            $this->dependson_cache = null;
+        } else if (isset($this->dependson_cache)) {
+            return $this->dependson_cache;
+        }
+
         if ($this->is_locked()) {
             // locked items do not need to be regraded
-            return array();
+            $this->dependson_cache = array();
+            return $this->dependson_cache;
         }
 
         if ($this->is_calculated()) {
             if (preg_match_all('/##gi(\d+)##/', $this->calculation, $matches)) {
-                return array_unique($matches[1]); // remove duplicates
+                $this->dependson_cache = array_unique($matches[1]); // remove duplicates
+                return $this->dependson_cache;
             } else {
-                return array();
+                $this->dependson_cache = array();
+                return $this->dependson_cache;
             }
 
         } else if ($grade_category = $this->load_item_category()) {
             //only items with numeric or scale values can be aggregated
             if ($this->gradetype != GRADE_TYPE_VALUE and $this->gradetype != GRADE_TYPE_SCALE) {
-                return array();
+                $this->dependson_cache = array();
+                return $this->dependson_cache;
             }
 
             // If global aggregateoutcomes is set, override category value
@@ -1217,13 +1236,16 @@ class grade_item extends grade_object {
             }
 
             if ($children = get_records_sql($sql)) {
-                return array_keys($children);
+                $this->dependson_cache = array_keys($children);
+                return $this->dependson_cache;
             } else {
-                return array();
+                $this->dependson_cache = array();
+                return $this->dependson_cache;
             }
 
         } else {
-            return array();
+            $this->dependson_cache = array();
+            return $this->dependson_cache;
         }
     }