mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-01-19 06:18:28 +01:00
Code Issues Projects Releases Wiki Activity

MDL-46997 Grades: Fix aggregation when hiding is excluded and items have nested dependencies

Browse Source 
Includes a unit test for the dependency flattening function.
This commit is contained in:
Damyon Wiese 2014-08-29 09:48:44 +08:00
parent 15ace20460
commit 0db54b5ba2
4 changed files with 145 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
grade/report/user/lib.php
View File

@ -433,6 +433,18 @@ class grade_report_user extends grade_report {
/// Actual Grade
$gradeval = $grade_grade->finalgrade;
if (!$this->canviewhidden) {
/// Virtual Grade (may be calculated excluding hidden items etc).
$adjustedgrade = $this->blank_hidden_total_and_adjust_bounds($this->courseid,
$grade_grade->grade_item,
$gradeval);
$gradeval = $adjustedgrade['grade'];
// We temporarily adjust the view of this grade item - because the min and
// max are affected by the hidden values in the aggregation.
$grade_grade->grade_item->grademax = $adjustedgrade['grademax'];
$grade_grade->grade_item->grademin = $adjustedgrade['grademin'];
}
if ($this->showfeedback) {
// Copy $class before appending itemcenter as feedback should not be centered
@ -460,20 +472,17 @@ class grade_report_user extends grade_report {
$data['grade']['class'] = $class;
$data['grade']['content'] = get_string('submittedon', 'grades', userdate($grade_grade->get_datesubmitted(), get_string('strftimedatetimeshort')));
} elseif ($grade_grade->is_hidden()) {
$data['grade']['class'] = $class.' dimmed_text';
$data['grade']['content'] = '-';
} else if ($grade_grade->is_hidden()) {
$data['grade']['class'] = $class.' dimmed_text';
$data['grade']['content'] = '-';
if ($this->canviewhidden) {
$data['grade']['content'] = grade_format_gradevalue($gradeval,
$grade_grade->grade_item,
true);
}
} else {
$data['grade']['class'] = $class;
$adjustedgrade = $this->blank_hidden_total_and_adjust_bounds($this->courseid,
$grade_grade->grade_item,
$gradeval);
// We temporarily adjust the view of this grade item - because the min and
// max are affected by the hidden values in the aggregation.
$grade_grade->grade_item->grademax = $adjustedgrade['grademax'];
$grade_grade->grade_item->grademin = $adjustedgrade['grademin'];
$data['grade']['content'] = grade_format_gradevalue($adjustedgrade['grade'],
$data['grade']['content'] = grade_format_gradevalue($gradeval,
$grade_grade->grade_item,
true);
}
@ -495,6 +504,9 @@ class grade_report_user extends grade_report {
} else if ($grade_grade->is_hidden()) {
$data['percentage']['class'] = $class.' dimmed_text';
$data['percentage']['content'] = '-';
if ($this->canviewhidden) {
$data['percentage']['content'] = grade_format_gradevalue($gradeval, $grade_grade->grade_item, true, GRADE_DISPLAY_TYPE_PERCENTAGE);
}
} else {
$data['percentage']['class'] = $class;
$data['percentage']['content'] = grade_format_gradevalue($gradeval, $grade_grade->grade_item, true, GRADE_DISPLAY_TYPE_PERCENTAGE);

78
lib/grade/grade_grade.php
View File

@ -565,6 +565,46 @@ class grade_grade extends grade_object {
return $standardised_value;
}
/**
* Given an array like this:
* $a = array(1=>array(2, 3),
* 2=>array(4),
* 3=>array(1),
* 4=>array())
* this function fully resolves the dependencies so each value will be an array of
* the all items this item depends on and their dependencies (and their dependencies...).
* It should not explode if there are circular dependencies.
* The dependency depth array will list the number of branches in the tree above each leaf.
*
* @param array $dependson Array to flatten
* @param array $dependencydepth Array of itemids => depth. Initially these should be all set to 1.
* @return array Flattened array
*/
protected static function flatten_dependencies_array(&$dependson, &$dependencydepth) {
// Flatten the nested dependencies - this will handle recursion bombs because it removes duplicates.
$somethingchanged = true;
while ($somethingchanged) {
$somethingchanged = false;
foreach ($dependson as $itemid => $depends) {
// Make a copy so we can tell if it changed.
$before = $dependson[$itemid];
foreach ($depends as $subitemid => $subdepends) {
$dependson[$itemid] = array_unique(array_merge($depends, $dependson[$subdepends]));
sort($dependson[$itemid], SORT_NUMERIC);
}
if ($before != $dependson[$itemid]) {
$somethingchanged = true;
if (!isset($dependencydepth[$itemid])) {
$dependencydepth[$itemid] = 1;
} else {
$dependencydepth[$itemid]++;
}
}
}
}
}
/**
* Return array of grade item ids that are either hidden or indirectly depend
* on hidden grades, excluded grades are not returned.
@ -590,10 +630,13 @@ class grade_grade extends grade_object {
$altered = array(); // altered grades
$alteredgrademax = array(); // Altered grade max values.
$alteredgrademin = array(); // Altered grade min values.
$dependencydepth = array();
$hiddenfound = false;
foreach($grade_grades as $itemid=>$unused) {
$grade_grade =& $grade_grades[$itemid];
// We need the immediate dependencies of all every grade_item so we can calculate nested dependencies.
$dependson[$grade_grade->itemid] = $grade_items[$grade_grade->itemid]->depends_on();
if ($grade_grade->is_excluded()) {
//nothing to do, aggregation is ok
} else if ($grade_grade->is_hidden()) {
@ -602,18 +645,24 @@ class grade_grade extends grade_object {
} else if ($grade_grade->is_locked() or $grade_grade->is_overridden()) {
// no need to recalculate locked or overridden grades
} else {
$dependson[$grade_grade->itemid] = $grade_items[$grade_grade->itemid]->depends_on();
if (!empty($dependson[$grade_grade->itemid])) {
$dependencydepth[$grade_grade->itemid] = 1;
$todo[] = $grade_grade->itemid;
}
}
}
// Flatten the dependency tree and count number of branches to each leaf.
self::flatten_dependencies_array($dependson, $dependencydepth);
if (!$hiddenfound) {
return array('unknown' => array(),
'altered' => array(),
'alteredgrademax' => array(),
'alteredgrademin' => array());
}
// We need to resort the todo list by the dependency depth. This guarantees we process the leaves, then the branches.
array_multisort($dependencydepth, $todo);
$max = count($todo);
$hidden_precursors = null;
@ -641,20 +690,28 @@ class grade_grade extends grade_object {
if ($grade_items[$do]->is_calculated() or
(!$grade_items[$do]->is_category_item() and !$grade_items[$do]->is_course_item())
) {
// This is a grade item that is not a category or course and has been affected by grade hiding.
// I guess this means it is a calculation that needs to be recalculated.
$unknown[$do] = $do;
unset($todo[$key]);
$found = true;
continue;
} else {
// This is a grade category (or course).
$grade_category = $grade_items[$do]->load_item_category();
// Build a new list of the grades in this category.
$values = array();
foreach ($dependson[$do] as $itemid) {
$immediatedepends = $grade_items[$do]->depends_on();
foreach ($immediatedepends as $itemid) {
if (array_key_exists($itemid, $altered)) {
//nulling an altered precursor
$values[$itemid] = $altered[$itemid];
unset($values[$itemid]);
if (is_null($values[$itemid])) {
// This means this was a hidden grade item removed from the result.
unset($values[$itemid]);
}
} elseif (empty($values[$itemid])) {
$values[$itemid] = $grade_grades[$itemid]->finalgrade;
}
@ -665,7 +722,16 @@ class grade_grade extends grade_object {
unset($values[$itemid]);
continue;
}
$values[$itemid] = grade_grade::standardise_score($value, $grade_items[$itemid]->grademin, $grade_items[$itemid]->grademax, 0, 1);
// The grade min/max may have been altered by hiding.
$grademin = $grade_items[$itemid]->grademin;
if (isset($alteredgrademin[$itemid])) {
$grademin = $alteredgrademin[$itemid];
}
$grademax = $grade_items[$itemid]->grademax;
if (isset($alteredgrademax[$itemid])) {
$grademax = $alteredgrademax[$itemid];
}
$values[$itemid] = grade_grade::standardise_score($value, $grademin, $grademax, 0, 1);
}
if ($grade_category->aggregateonlygraded) {
@ -707,6 +773,10 @@ class grade_grade extends grade_object {
$finalgrade = $grade_items[$do]->bounded_grade($finalgrade);
$alteredgrademin[$do] = $adjustedgrade['grademin'];
$alteredgrademax[$do] = $adjustedgrade['grademax'];
// We need to muck with the "in-memory" grade_items records so
// that subsequent calculations will use the adjusted grademin and grademax.
$grade_items[$do]->grademin = $adjustedgrade['grademin'];
$grade_items[$do]->grademax = $adjustedgrade['grademax'];
$altered[$do] = $finalgrade;
unset($todo[$key]);

10
lib/grade/tests/fixtures/lib.php vendored
View File

@ -973,3 +973,13 @@ abstract class grade_base_testcase extends advanced_testcase {
$this->grade_items[17] = $grade_item;
}
}
/**
* Allow calling protected method.
*/
class test_grade_grade_flatten_dependencies_array extends grade_grade {
public static function test_flatten_dependencies_array(&$a,&$b) {
return self::flatten_dependencies_array($a, $b);
}
}

37
lib/grade/tests/grade_grade_test.php
View File

@ -193,4 +193,41 @@ class core_grade_grade_testcase extends grade_base_testcase {
$grade->hidden = time()+666;
$this->assertTrue($grade->is_hidden());
}
public function test_flatten_dependencies() {
// First test a simple normal case.
$a = array(1 => array(2, 3), 2 => array(), 3 => array(4), 4 => array());
$b = array();
$expecteda = array(1 => array(2, 3, 4), 2 => array(), 3 => array(4), 4 => array());
$expectedb = array(1 => 1);
test_grade_grade_flatten_dependencies_array::test_flatten_dependencies_array($a, $b);
$this->assertSame($expecteda, $a);
$this->assertSame($expectedb, $b);
// Edge case - empty arrays.
$a = $b = $expecteda = $expectedb = array();
test_grade_grade_flatten_dependencies_array::test_flatten_dependencies_array($a, $b);
$this->assertSame($expecteda, $a);
$this->assertSame($expectedb, $b);
// Circular dependency.
$a = array(1 => array(2), 2 => array(3), 3 => array(1));
$b = array();
$expecteda = array(1 => array(1, 2, 3), 2 => array(1, 2, 3), 3 => array(1, 2, 3));
test_grade_grade_flatten_dependencies_array::test_flatten_dependencies_array($a, $b);
$this->assertSame($expecteda, $a);
// Note - we don't test the depth when we got circular dependencies - the main thing we wanted to test was that there was
// no ka-boom. The result would be hard to understand and doesn't matter.
// Circular dependency 2.
$a = array(1 => array(2), 2 => array(3), 3 => array(4), 4 => array(2, 1));
$b = array();
$expecteda = array(1 => array(1, 2, 3, 4), 2 => array(1, 2, 3, 4), 3 => array(1, 2, 3, 4), 4 => array(1, 2, 3, 4));
test_grade_grade_flatten_dependencies_array::test_flatten_dependencies_array($a, $b);
$this->assertSame($expecteda, $a);
}
}