Skip to content

Commit

Permalink
MDL-9506 Issue 1: adjusting a gradeitem's value would result in a gra…
Browse files Browse the repository at this point in the history 
…de_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.
  • Loading branch information
nicolasconnault committed May 8, 2007
1 parent 3504e07 commit 0aa3227
Show file tree
Hide file tree
Showing 13 changed files with 189 additions and 51 deletions.
3 changes: 2 additions & 1 deletion lib/grade/grade_calculation.php
View file
Expand Up @@ -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
}

Expand Down
50 changes: 49 additions & 1 deletion lib/grade/grade_category.php
View file
Expand Up @@ -210,7 +210,55 @@ function insert() {

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.
Expand Down
4 changes: 2 additions & 2 deletions lib/grade/grade_grades_raw.php
View file
Expand Up @@ -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.
Expand Down
76 changes: 65 additions & 11 deletions lib/grade/grade_item.php
View file
Expand Up @@ -328,12 +328,47 @@ function get_final($userid=NULL) {
*/
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)
Expand Down Expand Up @@ -522,19 +557,36 @@ function update_final_grade($userid=NULL) {
$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;
Expand All @@ -558,19 +610,21 @@ function upgrade_calculation_to_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;
}
Expand Down Expand Up @@ -601,11 +655,11 @@ function adjust_grade($grade_raw, $gradevalue=NULL) {
$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);
}

Expand Down
1 change: 0 additions & 1 deletion lib/simpletest/grade/simpletest/testgradecalculation.php
View file
Expand Up @@ -61,7 +61,6 @@ function test_grade_calculation_insert() {
$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;

}

Expand Down
16 changes: 6 additions & 10 deletions lib/simpletest/grade/simpletest/testgradecategory.php
View file
Expand Up @@ -45,8 +45,6 @@ function test_grade_category_construct() {
$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);
Expand All @@ -58,8 +56,6 @@ function test_grade_category_construct() {
$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;
Expand All @@ -69,8 +65,6 @@ function test_grade_category_construct() {
$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);
}
Expand All @@ -90,12 +84,14 @@ function test_grade_category_insert() {
$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() {
Expand Down
2 changes: 0 additions & 2 deletions lib/simpletest/grade/simpletest/testgradefinal.php
View file
Expand Up @@ -63,8 +63,6 @@ function test_grade_grades_final_insert() {
$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() {
Expand Down
2 changes: 0 additions & 2 deletions lib/simpletest/grade/simpletest/testgradehistory.php
View file
Expand Up @@ -69,8 +69,6 @@ function test_grade_history_insert() {
$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() {
Expand Down

0 comments on commit 0aa3227

Please sign in to comment.