moodle/moodle
3
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-08-05 08:56:36 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

MDL-47064 Grades: Peer review cleanups

Browse source 
Changes include:
 * Search for existing items to reduce DB queries in grade_category::aggregate_grades
 * Comments improvements
 * Move brackets to be part of lang string
 * Convert aggregationhints to be a class variable instead of passing it around

Part of: MDL-46576
This commit is contained in:
Damyon Wiese 2014-09-24 16:21:30 +08:00 • committed by Adrian Greeve
parent 6070e53313
commit ee07a54be0
4 changed files with 67 additions and 53 deletions
Download patch file Download diff file Expand all files Collapse all files

10
grade/report/lib.php
View file

@ -489,19 +489,19 @@ abstract class grade_report {
$finalgrade = null; $finalgrade = null;
} else { } else {
//use reprocessed marks that exclude hidden items //use reprocessed marks that exclude hidden items
if (!empty($hiding_affected['altered'][$course_item->id])) { if (isset($hiding_affected['altered'][$course_item->id])) {
$finalgrade = $hiding_affected['altered'][$course_item->id]; $finalgrade = $hiding_affected['altered'][$course_item->id];
} }
if (!empty($hiding_affected['alteredgrademin'][$course_item->id])) { if (isset($hiding_affected['alteredgrademin'][$course_item->id])) {
$grademin = $hiding_affected['alteredgrademin'][$course_item->id]; $grademin = $hiding_affected['alteredgrademin'][$course_item->id];
} }
if (!empty($hiding_affected['alteredgrademax'][$course_item->id])) { if (isset($hiding_affected['alteredgrademax'][$course_item->id])) {
$grademax = $hiding_affected['alteredgrademax'][$course_item->id]; $grademax = $hiding_affected['alteredgrademax'][$course_item->id];
} }
if (!empty($hiding_affected['alteredaggregationstatus'][$course_item->id])) { if (isset($hiding_affected['alteredaggregationstatus'][$course_item->id])) {
$aggregationstatus = $hiding_affected['alteredaggregationstatus'][$course_item->id]; $aggregationstatus = $hiding_affected['alteredaggregationstatus'][$course_item->id];
} }
if (!empty($hiding_affected['alteredaggregationweight'][$course_item->id])) { if (isset($hiding_affected['alteredaggregationweight'][$course_item->id])) {
$aggregationweight = $hiding_affected['alteredaggregationweight'][$course_item->id]; $aggregationweight = $hiding_affected['alteredaggregationweight'][$course_item->id];
} }
} }

55
grade/report/user/lib.php
View file

@ -176,6 +176,15 @@ class grade_report_user extends grade_report {
*/ */
protected $viewasuser = false; protected $viewasuser = false;
/**
* An array that collects the aggregationhints for every
* grade_item. The hints contain grade, grademin, grademax
* status, weight and parent.
*
* @var array
*/
protected $aggregationhints = array();
/** /**
* Constructor. Sets local copies of user preferences and initialises grade_tree. * Constructor. Sets local copies of user preferences and initialises grade_tree.
* @param int $courseid * @param int $courseid
@ -354,11 +363,8 @@ class grade_report_user extends grade_report {
* Fill the table with data. * Fill the table with data.
* *
* @param $element - An array containing the table data for the current row. * @param $element - An array containing the table data for the current row.
* @param $aggregationhints - An array that collects the aggregationhints for every
* grade_item. The hints contain grade, grademin, grademax
* status, weight and parent.
*/ */
private function fill_table_recursive(&$element, &$aggregationhints = array()) { private function fill_table_recursive(&$element) {
global $DB, $CFG; global $DB, $CFG;
$type = $element['type']; $type = $element['type'];
@ -494,7 +500,7 @@ class grade_report_user extends grade_report {
$data['weight']['content'] = format_float($hint['weight'] * 100.0, 2) . ' %'; $data['weight']['content'] = format_float($hint['weight'] * 100.0, 2) . ' %';
} }
if ($hint['status'] != 'used' && $hint['status'] != 'unknown') { if ($hint['status'] != 'used' && $hint['status'] != 'unknown') {
$data['weight']['content'] .= '<br/>( ' . get_string('aggregationhint' . $hint['status'], 'grades') . ' )'; $data['weight']['content'] .= '<br>' . get_string('aggregationhint' . $hint['status'], 'grades');
} }
} }
@ -637,7 +643,7 @@ class grade_report_user extends grade_report {
$parent = $parent->load_parent_category(); $parent = $parent->load_parent_category();
} }
$hint['parent'] = $parent->load_grade_item()->id; $hint['parent'] = $parent->load_grade_item()->id;
$aggregationhints[$grade_grade->itemid] = $hint; $this->aggregationhints[$grade_grade->itemid] = $hint;
} }
} }
} }
@ -664,7 +670,7 @@ class grade_report_user extends grade_report {
/// Recursively iterate through all child elements /// Recursively iterate through all child elements
if (isset($element['children'])) { if (isset($element['children'])) {
foreach ($element['children'] as $key=>$child) { foreach ($element['children'] as $key=>$child) {
$this->fill_table_recursive($element['children'][$key], $aggregationhints); $this->fill_table_recursive($element['children'][$key]);
} }
} }
@ -673,7 +679,7 @@ class grade_report_user extends grade_report {
if ($this->showcontributiontocoursetotal && ($type == 'category' && $depth == 1)) { if ($this->showcontributiontocoursetotal && ($type == 'category' && $depth == 1)) {
// We should have collected all the hints by now - walk the tree again and build the contributions column. // We should have collected all the hints by now - walk the tree again and build the contributions column.
$this->fill_contributions_column($element, $aggregationhints); $this->fill_contributions_column($element);
} }
} }
@ -683,16 +689,13 @@ class grade_report_user extends grade_report {
* grade_item. * grade_item.
* *
* @param $element - An array containing the table data for the current row. * @param $element - An array containing the table data for the current row.
* @param $aggregationhints - An array that collects the aggregationhints for every
* grade_item. The hints contain grade, grademin, grademax
* status, weight and parent.
*/ */
public function fill_contributions_column($element, $aggregationhints) { public function fill_contributions_column($element) {
/// Recursively iterate through all child elements // Recursively iterate through all child elements.
if (isset($element['children'])) { if (isset($element['children'])) {
foreach ($element['children'] as $key=>$child) { foreach ($element['children'] as $key=>$child) {
$this->fill_contributions_column($element['children'][$key], $aggregationhints); $this->fill_contributions_column($element['children'][$key]);
} }
} else if ($element['type'] == 'item') { } else if ($element['type'] == 'item') {
// This is a grade item (We don't do this for categories or we would double count). // This is a grade item (We don't do this for categories or we would double count).
@ -700,40 +703,38 @@ class grade_report_user extends grade_report {
$itemid = $grade_object->id; $itemid = $grade_object->id;
// Ignore anything with no hint - e.g. a hidden row. // Ignore anything with no hint - e.g. a hidden row.
if (isset($aggregationhints[$itemid])) { if (isset($this->aggregationhints[$itemid])) {
// Normalise the gradeval. // Normalise the gradeval.
$graderange = $aggregationhints[$itemid]['grademax'] - $aggregationhints[$itemid]['grademin']; $graderange = $this->aggregationhints[$itemid]['grademax'] - $this->aggregationhints[$itemid]['grademin'];
$gradeval = ($aggregationhints[$itemid]['grade'] - $aggregationhints[$itemid]['grademin']) / $graderange; $gradeval = ($this->aggregationhints[$itemid]['grade'] - $this->aggregationhints[$itemid]['grademin']) / $graderange;
// Multiply the normalised value by the weight // Multiply the normalised value by the weight
// of all the categories higher in the tree. // of all the categories higher in the tree.
$parent = $aggregationhints[$itemid]['parent'];
do { do {
if (!is_null($aggregationhints[$itemid]['weight'])) { if (!is_null($this->aggregationhints[$itemid]['weight'])) {
$gradeval *= $aggregationhints[$itemid]['weight']; $gradeval *= $this->aggregationhints[$itemid]['weight'];
} }
// The second part of this if is to prevent infinite loops // The second part of this if is to prevent infinite loops
// in case of crazy data. // in case of crazy data.
if (isset($aggregationhints[$itemid]['parent']) && if (isset($this->aggregationhints[$itemid]['parent']) &&
$aggregationhints[$itemid]['parent'] != $itemid) { $this->aggregationhints[$itemid]['parent'] != $itemid) {
$parent = $aggregationhints[$itemid]['parent']; $parent = $this->aggregationhints[$itemid]['parent'];
$itemid = $parent; $itemid = $parent;
} else { } else {
// We are at the top of the tree // We are at the top of the tree.
$parent = false; $parent = false;
} }
} while ($parent); } while ($parent);
// Finally multiply by the course grademax. // Finally multiply by the course grademax.
$gradeval *= $aggregationhints[$itemid]['grademax']; $gradeval *= $this->aggregationhints[$itemid]['grademax'];
// Now we need to loop through the "built" table data and update the // Now we need to loop through the "built" table data and update the
// contributions column for the current row. // contributions column for the current row.
$header_row = "row_{$grade_object->id}_{$this->user->id}"; $header_row = "row_{$grade_object->id}_{$this->user->id}";
foreach ($this->tabledata as $key => $row) { foreach ($this->tabledata as $key => $row) {
if (isset($row['itemname']) && if (isset($row['itemname']) && ($row['itemname']['id'] == $header_row)) {
($row['itemname']['id'] == $header_row)) {
// Found it - update the column. // Found it - update the column.
$decimals = $grade_object->get_decimals(); $decimals = $grade_object->get_decimals();
$this->tabledata[$key]['contributiontocoursetotal']['content'] = format_float($gradeval, $decimals, true); $this->tabledata[$key]['contributiontocoursetotal']['content'] = format_float($gradeval, $decimals, true);

8
lang/en/grades.php
View file

@ -64,10 +64,10 @@ $string['aggregation_help'] = 'The aggregation determines how grades in a catego
* Highest grade * Highest grade
* Mode of grades - The grade that occurs the most frequently * Mode of grades - The grade that occurs the most frequently
* Natural - The sum of all grade values scaled by weight'; * Natural - The sum of all grade values scaled by weight';
$string['aggregationhintnovalue'] = 'Empty'; $string['aggregationhintnovalue'] = '( Empty )';
$string['aggregationhintdropped'] = 'Dropped'; $string['aggregationhintdropped'] = '( Dropped )';
$string['aggregationhintexcluded'] = 'Excluded'; $string['aggregationhintexcluded'] = '( Excluded )';
$string['aggregationhintextra'] = 'Extra credit'; $string['aggregationhintextra'] = '( Extra credit )';
$string['aggregation_link'] = 'grade/aggregation'; $string['aggregation_link'] = 'grade/aggregation';
$string['aggregationcoef'] = 'Aggregation coefficient'; $string['aggregationcoef'] = 'Aggregation coefficient';
$string['aggregationcoefextra'] = 'Extra credit'; // for the header of the table at Edit categories and items page $string['aggregationcoefextra'] = 'Extra credit'; // for the header of the table at Edit categories and items page

33
lib/grade/grade_category.php
View file

@ -564,7 +564,7 @@ class grade_category extends grade_object {
$excluded, $excluded,
$grademinoverrides, $grademinoverrides,
$grademaxoverrides) { $grademaxoverrides) {
global $CFG; global $CFG, $DB;
// Remember these so we can set flags on them to describe how they were used in the aggregation. // Remember these so we can set flags on them to describe how they were used in the aggregation.
$novalue = array(); $novalue = array();
@ -601,16 +601,26 @@ class grade_category extends grade_object {
// Make sure a grade_grade exists for every grade_item. // Make sure a grade_grade exists for every grade_item.
// We need to do this so we can set the aggregationstatus // We need to do this so we can set the aggregationstatus
// with a set_field call instead of checking if each one exists and creating/updating. // with a set_field call instead of checking if each one exists and creating/updating.
foreach ($items as $itemid => $gradeitem) { if (count($items) > 0) {
$gradegrade = new grade_grade(array('itemid' => $gradeitem->id, list($ggsql, $params) = $DB->get_in_or_equal(array_keys($items), SQL_PARAMS_NAMED, 'g');
$params['userid'] = $userid;
$sql = "SELECT itemid
FROM {grade_grades}
WHERE itemid $ggsql AND userid = :userid";
$existingitems = $DB->get_records_sql($sql, $params);
$notexisting = array_diff(array_keys($items), array_keys($existingitems));
foreach ($notexisting as $itemid) {
$gradeitem = $this->grade_item;
$gradegrade = new grade_grade(array('itemid' => $itemid,
'userid' => $userid, 'userid' => $userid,
'rawgrademin' => $gradeitem->grademin, 'rawgrademin' => $gradeitem->grademin,
'rawgrademax' => $gradeitem->grademax), false); 'rawgrademax' => $gradeitem->grademax), false);
$gradegrade->grade_item = $this->grade_item; $gradegrade->grade_item = $gradeitem;
if (!$gradegrade->fetch(array('itemid'=>$gradeitem->id, 'userid'=>$userid))) {
$gradegrade->insert('system'); $gradegrade->insert('system');
} }
} }
// if no grades calculation possible or grading not allowed clear final grade // if no grades calculation possible or grading not allowed clear final grade
@ -632,7 +642,7 @@ class grade_category extends grade_object {
// If null, it means no grade. // If null, it means no grade.
if ($this->aggregateonlygraded) { if ($this->aggregateonlygraded) {
unset($grade_values[$itemid]); unset($grade_values[$itemid]);
// Mark this item as dropped because it has no grade. // Mark this item as "excluded empty" because it has no grade.
$novalue[$itemid] = 0; $novalue[$itemid] = 0;
continue; continue;
} }
@ -654,14 +664,13 @@ class grade_category extends grade_object {
$grade_values[$itemid] = grade_grade::standardise_score($v, $usergrademin, $usergrademax, 0, 1); $grade_values[$itemid] = grade_grade::standardise_score($v, $usergrademin, $usergrademax, 0, 1);
} }
// Use min grade if grade missing for these types. // For items with no value, and not excluded - either set their grade to 0 or exclude them.
foreach ($items as $itemid=>$value) { foreach ($items as $itemid=>$value) {
if (!isset($grade_values[$itemid]) and !in_array($itemid, $excluded)) { if (!isset($grade_values[$itemid]) and !in_array($itemid, $excluded)) {
if (!$this->aggregateonlygraded) { if (!$this->aggregateonlygraded) {
$grade_values[$itemid] = 0; $grade_values[$itemid] = 0;
} else { } else {
// We are specifically marking these items that get dropped // We are specifically marking these items as "excluded empty".
// because they are empty.
$novalue[$itemid] = 0; $novalue[$itemid] = 0;
} }
} }
@ -811,6 +820,10 @@ class grade_category extends grade_object {
* @since Moodle 2.6.5, 2.7.2 * @since Moodle 2.6.5, 2.7.2
* @param array & $weights If provided, will be filled with the normalized weights * @param array & $weights If provided, will be filled with the normalized weights
* for each grade_item as used in the aggregation. * for each grade_item as used in the aggregation.
* Some rules for the weights are:
* 1. The weights must add up to 1 (unless there are extra credit)
* 2. The contributed points column must add up to the course
* final grade and this column is calculated from these weights.
* @param array $grademinoverrides User specific grademin values if different to the grade_item grademin (key is itemid) * @param array $grademinoverrides User specific grademin values if different to the grade_item grademin (key is itemid)
* @param array $grademaxoverrides User specific grademax values if different to the grade_item grademax (key is itemid) * @param array $grademaxoverrides User specific grademax values if different to the grade_item grademax (key is itemid)
* @return array containing values for: * @return array containing values for: