Browse files

MDL-11718 aggregated grades which depend on hidden grades are now hid…

…den too + cleanup in grader report
  • Loading branch information...
1 parent 93b2ed9 commit b89a70ce8ad10499be55c049e04457fbf0cf9afc skodak committed Oct 17, 2007
Showing with 108 additions and 87 deletions.
  1. +7 −1 grade/lib.php
  2. +48 −65 grade/report/grader/lib.php
  3. +21 −11 grade/report/user/lib.php
  4. +2 −2 lib/grade/grade_grade.php
  5. +30 −8 lib/grade/grade_item.php
View
8 grade/lib.php
@@ -735,7 +735,7 @@ function grade_seq($courseid, $category_grade_last=false, $nooutcomes=false) {
$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'];
}
}
@@ -873,6 +873,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.
* @param int $courseid
@@ -1011,6 +1016,7 @@ function fill_levels(&$levels, &$element, $depth) {
$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;
View
113 grade/report/grader/lib.php
@@ -38,15 +38,9 @@
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 @@ function load_final_grades() {
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,16 +579,13 @@ function get_headerhtml() {
$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') .'"/>';
}
$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 @@ function get_studentshtml() {
// 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 @@ function get_studentshtml() {
$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,47 +647,37 @@ function get_studentshtml() {
. '<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>';
}
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');
$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 @@ function get_studentshtml() {
$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 @@ function get_studentshtml() {
} 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 @@ function get_avghtml($grouponly=false) {
$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 @@ function get_rangehtml() {
. '<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]) {
View
32 grade/report/user/lib.php
@@ -127,7 +127,7 @@ function fill_table() {
$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,16 +136,18 @@ function fill_table() {
$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];
$grade_grade =& $grades[$key];
$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 @@ function fill_table() {
$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,23 +186,28 @@ function fill_table() {
}
/// 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 {
$data[] = grade_format_gradevalue($grade_grade->finalgrade, $grade_item, true, GRADE_DISPLAY_TYPE_PERCENTAGE);
}
/// 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 @@ function fill_table() {
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 {
View
4 lib/grade/grade_grade.php
@@ -476,11 +476,11 @@ function standardise_score($rawgrade, $source_min, $source_max, $target_min, $ta
* 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()!");
}
View
38 lib/grade/grade_item.php
@@ -239,12 +239,20 @@ 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
* @param string $source from where was the object inserted (mod/forum, manual, etc.)
* @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 @@ function set_parent($parentid) {
/**
* 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 @@ function depends_on() {
}
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;
}
}

0 comments on commit b89a70c

Please sign in to comment.