From 0aa32279b723adb6121ab17333db9e94094376f0 Mon Sep 17 00:00:00 2001 From: nicolasconnault Date: Tue, 8 May 2007 02:20:26 +0000 Subject: [PATCH] MDL-9506 Issue 1: adjusting a gradeitem's value would result in a grade_final object with gradevalue assigned, even when a gradescale should have been assigned. Issue 2: double grade_final entries when calling grade_item->load_final() after grade_item->generate_final(). Issue 3: Calling grade_item->update_final_grade() without first calling grade_item->generate_final() would result in fatal error. generate_final() is now called if the raw and final arrays' sizes don't match. --- lib/grade/grade_calculation.php | 3 +- lib/grade/grade_category.php | 50 +++++++++++- lib/grade/grade_grades_raw.php | 4 +- lib/grade/grade_item.php | 76 +++++++++++++++--- .../grade/simpletest/testgradecalculation.php | 1 - .../grade/simpletest/testgradecategory.php | 16 ++-- .../grade/simpletest/testgradefinal.php | 2 - .../grade/simpletest/testgradehistory.php | 2 - .../grade/simpletest/testgradeitem.php | 79 +++++++++++++++---- .../grade/simpletest/testgradeoutcome.php | 1 - .../grade/simpletest/testgraderaw.php | 2 - .../grade/simpletest/testgradescale.php | 2 - .../grade/simpletest/testgradetext.php | 2 - 13 files changed, 189 insertions(+), 51 deletions(-) diff --git a/lib/grade/grade_calculation.php b/lib/grade/grade_calculation.php index 1508dc89a2..8fec2e23d3 100644 --- a/lib/grade/grade_calculation.php +++ b/lib/grade/grade_calculation.php @@ -70,9 +70,10 @@ class grade_calculation extends grade_object { /** * Applies the formula represented by this object to the value given, and returns the result. * @param float $oldvalue + * @param string $valuetype Either 'gradevalue' or 'gradescale' * @return float result */ - function compute($oldvalue) { + function compute($oldvalue, $valuetype = 'gradevalue') { return $oldvalue; // TODO implement computation using parser } diff --git a/lib/grade/grade_category.php b/lib/grade/grade_category.php index d3458dd9d0..e05f223e1b 100644 --- a/lib/grade/grade_category.php +++ b/lib/grade/grade_category.php @@ -210,7 +210,55 @@ class grade_category extends grade_object { return $result; } - + + /** + * Generates and saves raw_grades, based on this category's immediate children, then uses the + * associated grade_item to generate matching final grades. These immediate children must first have their own + * raw and final grades, which means that ultimately we must get grade_items as children. The category's aggregation + * method is used to generate these raw grades, which can then be used by the category's associated grade_item + * to apply calculations to and generate final grades. + */ + function generate_grades() { + // Check that the children have final grades. If not, call their generate_raw_grades method (recursion) + if (empty($this->children)) { + $this->children = $this->get_children(1, 'flat'); + } + + $category_raw_grades = array(); + $aggregated_grades = array(); + + foreach ($this->children as $child) { + if (get_class($child) == 'grade_item') { + $category_raw_grades[$child->id] = $child->load_final(); + } elseif ($get_class($child) == 'grade_category') { + $category_raw_grades[$child->id] = $child->load_final(); + if (empty($category_raw_grades)) { + $category_raw_grades[$child->id] = $child->generate_grades(); + } + } + } + + if (empty($category_raw_grades)) { + return null; + } else { + $aggregated_grades = $this->aggregate_grades($category_raw_grades); + foreach ($aggregated_grades as $raw_grade) { + $raw_grade->insert(); + } + $this->grade_item->generate_final(); + } + } + + /** + * Given an array of arrays of grade objects (raw or final), uses this category's aggregation method to + * compute and return a single array of grade_raw objects with the aggregated gradevalue. + * @param array $raw_grade_sets + * @return array Raw grade objects + */ + function aggregate_grades($raw_grade_sets) { + + } + /** * Looks at a path string (e.g. /2/45/56) and returns the depth level represented by this path (in this example, 3). * If no string is given, it looks at the obect's path and assigns the resulting depth to its $depth variable. diff --git a/lib/grade/grade_grades_raw.php b/lib/grade/grade_grades_raw.php index dd62c4f5b3..57ba565670 100644 --- a/lib/grade/grade_grades_raw.php +++ b/lib/grade/grade_grades_raw.php @@ -59,9 +59,9 @@ class grade_grades_raw extends grade_object { /** * The scale value of this raw grade, if such was provided by the module. - * @var int $scalevalue + * @var int $gradescale */ - var $scalevalue; + var $gradescale; /** * The maximum allowable grade when this grade was created. diff --git a/lib/grade/grade_item.php b/lib/grade/grade_item.php index 6fcac6265a..c559469c5d 100644 --- a/lib/grade/grade_item.php +++ b/lib/grade/grade_item.php @@ -328,12 +328,47 @@ class grade_item extends grade_object { */ function load_final() { $grade_final_array = get_records('grade_grades_final', 'itemid', $this->id); + + if (empty($grade_final_array)) { + $this->generate_final(); + $grade_final_array = get_records('grade_grades_final', 'itemid', $this->id); + } + + if (empty($grade_final_array)) { + return false; + } + foreach ($grade_final_array as $f) { $this->grade_grades_final[$f->userid] = new grade_grades_final($f); } return $this->grade_grades_final; } + /** + * Once the raw_grades are imported or entered, this method uses the grade_item's calculation and rules to + * generate final grade entries in the DB. + * @return array final grade objects (grade_grades_final). + */ + function generate_final() { + if (empty($this->grade_grades_raw)) { + $this->load_raw(); + } + + $success = true; + + foreach ($this->grade_grades_raw as $raw_grade) { + $final_grade = new grade_grades_final(); + $final_grade->gradevalue = $this->adjust_grade($raw_grade, null, 'gradevalue'); + $final_grade->gradescale = $this->adjust_grade($raw_grade, null, 'gradescale'); + $final_grade->itemid = $this->id; + $final_grade->userid = $raw_grade->userid; + $success = $success & $final_grade->insert(); + $this->grade_grades_final[$final_grade->userid] = $final_grade; + } + + return $success; + } + /** * Returns this object's calculation. * @param boolean $fetch Whether to fetch the value from the DB or not (false == just use the object's value) @@ -522,19 +557,36 @@ class grade_item extends grade_object { $grade_raw_array = $this->grade_grades_raw; } + // The following code assumes that there is a grade_final object in DB for every + // grade_raw object. This assumption depends on the correct creation of grade_final entries. + // This also assumes that the two arrays $this->grade_grades_raw and its final counterpart are + // indexed by userid, not sequentially or by grade_id + if (count($this->grade_grades_final) != count($this->grade_grades_raw)) { + $this->generate_final(); + } + foreach ($grade_raw_array as $userid => $raw) { - $newgradevalue = $raw->gradevalue; + // the value could be gradevalue or gradescale + $valuetype = null; + + if (!empty($raw->gradevalue)) { + $valuetype = 'gradevalue'; + } elseif (!empty($raw->gradescale)) { + $valuetype = 'gradescale'; + } + + $newgradevalue = $raw->$valuetype; if (!empty($this->calculation)) { $this->upgrade_calculation_to_object(); - $newgradevalue = $this->calculation->compute($raw->gradevalue); + $newgradevalue = $this->calculation->compute($raw->$valuetype, $valuetype); } $final = $this->grade_grades_final[$userid]; - $final->gradevalue = $this->adjust_grade($raw, $newgradevalue); + $final->$valuetype = $this->adjust_grade($raw, $newgradevalue, $valuetype); - if ($final->update($newgradevalue)) { + if ($final->update()) { $count++; } else { return false; @@ -558,19 +610,21 @@ class grade_item extends grade_object { * Given a float grade value or integer grade scale, applies a number of adjustment based on * grade_item variables and returns the result. * @param object $grade_raw The raw object to compare with this grade_item's rules - * @param mixed $gradevalue The new gradevalue (after calculations are performed) + * @param mixed $gradevalue The new gradevalue (after calculations are performed). + * If null, the raw_grade's gradevalue or gradescale will be used. + * @param string $valuetype Either 'gradevalue' or 'gradescale' * @return mixed */ - function adjust_grade($grade_raw, $gradevalue=NULL) { + function adjust_grade($grade_raw, $gradevalue=NULL, $valuetype='gradevalue') { $raw_offset = 0; $item_offset = 0; - - if (!empty($grade_raw->gradevalue)) { // Dealing with numerical grade + + if ($valuetype == 'gradevalue') { // Dealing with numerical grade if (empty($gradevalue)) { $gradevalue = $grade_raw->gradevalue; } - } elseif(!empty($grade_raw->gradescale)) { // Dealing with a scale value + } elseif($valuetype == 'gradescale') { // Dealing with a scale value if (empty($gradevalue)) { $gradevalue = $grade_raw->gradescale; } @@ -601,11 +655,11 @@ class grade_item extends grade_object { $gradevalue = $factor * $diff + $this->grademin; // Apply rounding or factors, depending on whether it's a scale or value - if (!empty($grade_raw->gradevalue)) { + if ($valuetype == 'gradevalue') { // Apply other grade_item factors $gradevalue *= $this->multfactor; $gradevalue += $this->plusfactor; - } elseif (!empty($grade_raw->gradescale)) { + } elseif ($valuetype == 'gradescale') { $gradevalue = (int) round($gradevalue); } diff --git a/lib/simpletest/grade/simpletest/testgradecalculation.php b/lib/simpletest/grade/simpletest/testgradecalculation.php index b92a0ad1f8..b4631d237d 100644 --- a/lib/simpletest/grade/simpletest/testgradecalculation.php +++ b/lib/simpletest/grade/simpletest/testgradecalculation.php @@ -61,7 +61,6 @@ class grade_calculation_test extends gradelib_test { $this->assertEqual($grade_calculation->id, $last_grade_calculation->id + 1); $this->assertFalse(empty($grade_calculation->timecreated)); $this->assertFalse(empty($grade_calculation->timemodified)); - $this->grade_calculations[] = $grade_calculation; } diff --git a/lib/simpletest/grade/simpletest/testgradecategory.php b/lib/simpletest/grade/simpletest/testgradecategory.php index 65ab826980..a89a56e660 100755 --- a/lib/simpletest/grade/simpletest/testgradecategory.php +++ b/lib/simpletest/grade/simpletest/testgradecategory.php @@ -45,8 +45,6 @@ class grade_category_test extends gradelib_test { $grade_category = new grade_category($params, false); $grade_category->insert(); - $this->grade_categories[] = $grade_category; - $this->grade_items[] = $grade_category->grade_item; $this->assertEqual($params->courseid, $grade_category->courseid); $this->assertEqual($params->fullname, $grade_category->fullname); $this->assertEqual(1, $grade_category->depth); @@ -58,8 +56,6 @@ class grade_category_test extends gradelib_test { $params->fullname = 'unittestcategory5'; $grade_category = new grade_category($params, false); $grade_category->insert(); - $this->grade_categories[] = $grade_category; - $this->grade_items[] = $grade_category->grade_item; $this->assertEqual(2, $grade_category->depth); $this->assertEqual("$parentpath/$grade_category->id", $grade_category->path); $parentpath = $grade_category->path; @@ -69,8 +65,6 @@ class grade_category_test extends gradelib_test { $params->fullname = 'unittestcategory6'; $grade_category = new grade_category($params, false); $grade_category->insert(); - $this->grade_categories[] = $grade_category; - $this->grade_items[] = $grade_category->grade_item; $this->assertEqual(3, $grade_category->depth); $this->assertEqual("$parentpath/$grade_category->id", $grade_category->path); } @@ -90,12 +84,14 @@ class grade_category_test extends gradelib_test { $grade_category->insert(); $last_grade_category = end($this->grade_categories); + + $this->assertFalse(empty($grade_category->grade_item)); + $this->assertEqual($grade_category->id, $grade_category->grade_item->iteminstance); + $this->assertEqual('category', $grade_category->grade_item->itemtype); $this->assertEqual($grade_category->id, $last_grade_category->id + 1); - $this->assertTrue(!empty($grade_category->timecreated)); - $this->assertTrue(!empty($grade_category->timemodified)); - $this->grade_categories[] = $grade_category; - $this->grade_items[] = $grade_category->grade_item; + $this->assertFalse(empty($grade_category->timecreated)); + $this->assertFalse(empty($grade_category->timemodified)); } function test_grade_category_update() { diff --git a/lib/simpletest/grade/simpletest/testgradefinal.php b/lib/simpletest/grade/simpletest/testgradefinal.php index e822bf6058..ff90461cc5 100644 --- a/lib/simpletest/grade/simpletest/testgradefinal.php +++ b/lib/simpletest/grade/simpletest/testgradefinal.php @@ -63,8 +63,6 @@ class grade_final_test extends gradelib_test { $this->assertEqual($grade_grades_final->id, $last_grade_grades_final->id + 1); $this->assertFalse(empty($grade_grades_final->timecreated)); $this->assertFalse(empty($grade_grades_final->timemodified)); - $this->grade_grades_final[] = $grade_grades_final; - } function test_grade_grades_final_update() { diff --git a/lib/simpletest/grade/simpletest/testgradehistory.php b/lib/simpletest/grade/simpletest/testgradehistory.php index 2943da14f6..e54922373d 100644 --- a/lib/simpletest/grade/simpletest/testgradehistory.php +++ b/lib/simpletest/grade/simpletest/testgradehistory.php @@ -69,8 +69,6 @@ class grade_history_test extends gradelib_test { $this->assertEqual($grade_history->id, $last_grade_history->id + 1); $this->assertFalse(empty($grade_history->timecreated)); $this->assertFalse(empty($grade_history->timemodified)); - $this->grade_history[] = $grade_history; - } function test_grade_history_update() { diff --git a/lib/simpletest/grade/simpletest/testgradeitem.php b/lib/simpletest/grade/simpletest/testgradeitem.php index faa81e0bf8..c6cf4a102c 100755 --- a/lib/simpletest/grade/simpletest/testgradeitem.php +++ b/lib/simpletest/grade/simpletest/testgradeitem.php @@ -40,11 +40,10 @@ class grade_item_test extends gradelib_test { $params = new stdClass(); $params->courseid = $this->courseid; - $params->categoryid = $this->grade_categories[0]->id; + $params->categoryid = $this->grade_categories[1]->id; $params->itemname = 'unittestgradeitem4'; $params->itemtype = 'mod'; $params->itemmodule = 'database'; - $params->iteminstance = 4; $params->iteminfo = 'Grade item used for unit testing'; $grade_item = new grade_item($params, false); @@ -59,11 +58,10 @@ class grade_item_test extends gradelib_test { $this->assertTrue(method_exists($grade_item, 'insert')); $grade_item->courseid = $this->courseid; - $grade_item->categoryid = $this->grade_categories[0]->id; + $grade_item->categoryid = $this->grade_categories[1]->id; $grade_item->itemname = 'unittestgradeitem4'; $grade_item->itemtype = 'mod'; $grade_item->itemmodule = 'quiz'; - $grade_item->iteminstance = 1; $grade_item->iteminfo = 'Grade item used for unit testing'; $grade_item->insert(); @@ -71,7 +69,6 @@ class grade_item_test extends gradelib_test { $last_grade_item = end($this->grade_items); $this->assertEqual($grade_item->id, $last_grade_item->id + 1); - $this->grade_items[] = $grade_item; } function test_grade_item_delete() { @@ -191,7 +188,6 @@ class grade_item_test extends gradelib_test { $calculation = 'SUM([unittestgradeitem1], [unittestgradeitem3])'; $grade_item->set_calculation($calculation); $new_calculation = $grade_item->get_calculation(); - $this->grade_calculations[] = $new_calculation; $this->assertEqual($calculation, $new_calculation->calculation); } @@ -301,21 +297,33 @@ class grade_item_test extends gradelib_test { } function test_grade_item_adjust_scale_grade() { + // Load grade item and its scale + $grade_item = new grade_item(array('scaleid' => $this->scale[1]->id), false); + $grade_item->insert(); + $grade_item->load_scale(); + $this->assertEqual('Very Good', $grade_item->scale->scale_items[1]); + // Load raw grade and its scale - $grade_raw = new grade_grades_raw(array('scaleid' => $this->scale[0]->id)); + $grade_raw = new grade_grades_raw(array('scaleid' => $this->scale[0]->id), false); $grade_raw->gradescale = 4; + $grade_raw->itemid = $grade_item->id; + $grade_raw->userid = 1; + $grade_raw->insert(); $grade_raw->load_scale(); $this->assertEqual('Fairly neutral', $grade_raw->scale->scale_items[2]); - - // Load grade item and its scale - $grade_item = new grade_item(array('scaleid' => $this->scale[1]->id)); - $grade_item->load_scale(); - $this->assertEqual('Very Good', $grade_item->scale->scale_items[1]); // Test grade_item::adjust_scale - $this->assertEqual(3, $grade_item->adjust_grade($grade_raw)); + $this->assertEqual(3, $grade_item->adjust_grade($grade_raw, null, 'gradescale')); $grade_raw->gradescale = 6; - $this->assertEqual(4, $grade_item->adjust_grade($grade_raw)); + $this->assertEqual(4, $grade_item->adjust_grade($grade_raw, null, 'gradescale')); + + // Check that the final grades have the correct values now + $grade_item->load_raw(); + $grade_item->update_final_grade(); + $this->assertFalse(empty($grade_item->grade_grades_final)); + $this->assertEqual($grade_item->id, $grade_item->grade_grades_final[1]->itemid); + $this->assertEqual(3, $grade_item->grade_grades_final[1]->gradescale); + $this->assertEqual(1, $grade_item->grade_grades_final[1]->userid); } function test_grade_item_toggle_locking() { @@ -353,5 +361,48 @@ class grade_item_test extends gradelib_test { $this->assertTrue($grade_item->grade_grades_final[2]->hidden); $this->assertTrue($grade_item->grade_grades_final[3]->hidden); } + + function test_grade_item_generate_final() { + $grade_item = new grade_item(); + $grade_item->courseid = $this->courseid; + $grade_item->categoryid = $this->grade_categories[1]->id; + $grade_item->itemname = 'unittestgradeitem4'; + $grade_item->itemtype = 'mod'; + $grade_item->itemmodule = 'quiz'; + $grade_item->iteminfo = 'Grade item used for unit testing'; + + $grade_item->insert(); + + $grade_grades_raw = new grade_grades_raw(); + $grade_grades_raw->itemid = $grade_item->id; + $grade_grades_raw->userid = 1; + $grade_grades_raw->gradevalue = 88; + $grade_grades_raw->grademax = 110; + $grade_grades_raw->grademin = 18; + $grade_grades_raw->insert(); + + $grade_grades_raw = new grade_grades_raw(); + $grade_grades_raw->itemid = $grade_item->id; + $grade_grades_raw->userid = 2; + $grade_grades_raw->gradevalue = 68; + $grade_grades_raw->grademax = 110; + $grade_grades_raw->grademin = 18; + $grade_grades_raw->insert(); + + $grade_grades_raw = new grade_grades_raw(); + $grade_grades_raw->itemid = $grade_item->id; + $grade_grades_raw->userid = 3; + $grade_grades_raw->gradevalue = 81; + $grade_grades_raw->grademax = 110; + $grade_grades_raw->grademin = 18; + $grade_grades_raw->insert(); + + $grade_item->load_raw(); + $this->assertEqual(3, count($grade_item->grade_grades_raw)); + + $grade_item->generate_final(); + $this->assertEqual(3, count($grade_item->grade_grades_final)); + + } } ?> diff --git a/lib/simpletest/grade/simpletest/testgradeoutcome.php b/lib/simpletest/grade/simpletest/testgradeoutcome.php index c5d113fbf8..23f61a0897 100644 --- a/lib/simpletest/grade/simpletest/testgradeoutcome.php +++ b/lib/simpletest/grade/simpletest/testgradeoutcome.php @@ -61,7 +61,6 @@ class grade_outcome_test extends gradelib_test { $this->assertEqual($grade_outcome->id, $last_grade_outcome->id + 1); $this->assertFalse(empty($grade_outcome->timecreated)); $this->assertFalse(empty($grade_outcome->timemodified)); - $this->grade_outcomes[] = $grade_outcome; } function test_grade_outcome_update() { diff --git a/lib/simpletest/grade/simpletest/testgraderaw.php b/lib/simpletest/grade/simpletest/testgraderaw.php index d67490605a..d69a70f879 100755 --- a/lib/simpletest/grade/simpletest/testgraderaw.php +++ b/lib/simpletest/grade/simpletest/testgraderaw.php @@ -67,8 +67,6 @@ class grade_raw_test extends gradelib_test { $this->assertEqual($grade_grades_raw->id, $last_grade_grades_raw->id + 1); $this->assertFalse(empty($grade_grades_raw->timecreated)); $this->assertFalse(empty($grade_grades_raw->timemodified)); - $this->grade_grades_raw[] = $grade_grades_raw; - } function test_grade_grades_raw_update() { diff --git a/lib/simpletest/grade/simpletest/testgradescale.php b/lib/simpletest/grade/simpletest/testgradescale.php index e4e75a67ca..c4bf9e2771 100755 --- a/lib/simpletest/grade/simpletest/testgradescale.php +++ b/lib/simpletest/grade/simpletest/testgradescale.php @@ -71,8 +71,6 @@ class grade_scale_test extends gradelib_test { $this->assertEqual($grade_scale->id, $last_grade_scale->id + 1); $this->assertTrue(!empty($grade_scale->timecreated)); $this->assertTrue(!empty($grade_scale->timemodified)); - $this->scale[] = $grade_scale; - } function test_grade_scale_update() { diff --git a/lib/simpletest/grade/simpletest/testgradetext.php b/lib/simpletest/grade/simpletest/testgradetext.php index fa29636e94..e4b0d02f64 100755 --- a/lib/simpletest/grade/simpletest/testgradetext.php +++ b/lib/simpletest/grade/simpletest/testgradetext.php @@ -73,8 +73,6 @@ class grade_text_test extends gradelib_test { $this->assertFalse(empty($grade_grades_text->timecreated)); $this->assertFalse(empty($grade_grades_text->timemodified)); $this->assertEqual($USER->id, $grade_grades_text->usermodified); - $this->grade_grades_text[] = $grade_grades_text; - } function test_grade_grades_text_update() { -- 2.39.5