From b89a70ce8ad10499be55c049e04457fbf0cf9afc Mon Sep 17 00:00:00 2001 From: skodak Date: Wed, 17 Oct 2007 20:38:52 +0000 Subject: [PATCH] MDL-11718 aggregated grades which depend on hidden grades are now hidden too + cleanup in grader report --- grade/lib.php | 8 ++- grade/report/grader/lib.php | 113 +++++++++++++++--------------------- grade/report/user/lib.php | 32 ++++++---- lib/grade/grade_grade.php | 4 +- lib/grade/grade_item.php | 38 +++++++++--- 5 files changed, 108 insertions(+), 87 deletions(-) diff --git a/grade/lib.php b/grade/lib.php index 13254d1746b..d60ef42bd87 100644 --- 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 4fe96a66e45..f4bd9557985 100644 --- 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 = '' .$this->get_lang_string('modulename', $object->itemmodule).''; } else if ($object->itemtype == 'manual') { - //TODO: add manual grading icon $icon = '' .$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 .= ''. $headerlink . $arrow; $headerhtml .= $this->get_icons($element) . ''; - - $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 { . '' . fullname($user) . ''; - 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 .= ''.userdate($grade->timecreated,get_string('strftimedatetimeshort')).''; } else { $studentshtml .= '-'; @@ -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 .= ''.get_string('error').''; } 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 .= ''; + $feedback = addslashes_js(trim(format_string($grade->feedback, $grade->feedbackformat))); + $overlib = "return overlib('$feedback', BORDER, 0, FGCLASS, 'feedback', " + ."CAPTIONFONTCLASS, 'caption', CAPTION, '$strfeedback');"; + $studentshtml .= ''; } if ($item->needsupdate) { $studentshtml .= ''.get_string('error').''; - } 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 { . ''.$this->get_lang_string('range','grades').''; $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 339ebdc522a..4d7933a9168 100644 --- 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[] = '
'.$grade_item->get_name().'
'; @@ -164,10 +166,13 @@ class grade_report_user extends grade_report { $excluded = ''; } - if (is_null($grade_grade->finalgrade)) { + if ($grade_item->needsupdate) { + $data[] = ''.get_string('error').''; + + } 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[] = ''.get_string('error').''; - 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[] = ''.get_string('error').''; + + } 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[] = ' '; - } 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 { diff --git a/lib/grade/grade_grade.php b/lib/grade/grade_grade.php index 2d35722aa32..3c02bf9f633 100644 --- 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 0882c71d684..4140ad2191a 100644 --- 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; } }