MDL-10226 regrading of final grades improved, calculation cleanup before MDL-10231
This commit is contained in:
@@ -304,15 +304,20 @@ class grade_category extends grade_object {
|
||||
return true;
|
||||
}
|
||||
|
||||
$result = true;
|
||||
|
||||
$this->load_grade_item();
|
||||
|
||||
if ($this->grade_item->needsupdate) {
|
||||
// this grade_item (and category) already needs update, no need to set it again here or in parent categories
|
||||
return true;
|
||||
}
|
||||
|
||||
$paths = explode('/', $this->path);
|
||||
|
||||
// Remove the first index, which is always empty
|
||||
unset($paths[0]);
|
||||
|
||||
$result = true;
|
||||
|
||||
if (!empty($paths)) {
|
||||
$wheresql = '';
|
||||
|
||||
@@ -325,11 +330,6 @@ class grade_category extends grade_object {
|
||||
|
||||
}
|
||||
|
||||
if (count($paths) == 1) {
|
||||
// we are the top category - force recalculation of all formulas in course
|
||||
$this->grade_item->force_recalculation();
|
||||
}
|
||||
|
||||
return $result;
|
||||
}
|
||||
|
||||
|
||||
+46
-59
@@ -521,12 +521,15 @@ class grade_item extends grade_object {
|
||||
|
||||
if ($this->is_locked()) {
|
||||
// locked grade items already have correct final grades
|
||||
$this->needsupdate = false;
|
||||
$this->update();
|
||||
return true;
|
||||
}
|
||||
|
||||
if ($calculation = $this->get_calculation()) {
|
||||
if ($calculation->compute()) {
|
||||
$this->force_regrade();
|
||||
if ($this->is_calculated()) {
|
||||
if ($this->calculation->compute()) {
|
||||
$this->needsupdate = false;
|
||||
$this->update();
|
||||
return true;
|
||||
} else {
|
||||
return array("Could not calculate grades for grade item id:".$this->id); // TODO: improve and localize
|
||||
@@ -691,31 +694,11 @@ class grade_item extends grade_object {
|
||||
if ($category = $this->get_category()) {
|
||||
$category->force_regrading(); // we can ignore the result
|
||||
|
||||
} else {
|
||||
$this->force_recalculation(); // recalculate all formulas - we do not know if any of them depends on this item
|
||||
}
|
||||
|
||||
|
||||
return $result;
|
||||
}
|
||||
|
||||
/**
|
||||
* Force recalculation of all calculated grades in course.
|
||||
*/
|
||||
function force_recalculation($courseid=null) {
|
||||
if (empty($courseid)) {
|
||||
$courseid = $this->courseid;
|
||||
}
|
||||
$grade_item = new grade_item(array('courseid'=>$courseid), false);
|
||||
$grade_items = $grade_item->fetch_all_using_this();
|
||||
foreach($grade_items as $gi) {
|
||||
if ($gi->get_calculation()) {
|
||||
$gi->needsupdate = true;
|
||||
$gi->update();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Disassociates this item from its category parent(s). The object is then updated in DB.
|
||||
* @return boolean Success or Failure
|
||||
@@ -791,67 +774,71 @@ class grade_item extends grade_object {
|
||||
}
|
||||
|
||||
/**
|
||||
* 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)
|
||||
* @return mixed $calculation Object if found, false otherwise.
|
||||
* Checks if grade calculated. Returns this object's calculation.
|
||||
* @return mixed $calculation Calculation object if exists, false otherwise.
|
||||
*/
|
||||
function get_calculation($nocache = false) {
|
||||
function is_calculated() {
|
||||
if (is_null($this->calculation)) {
|
||||
$nocache = true;
|
||||
}
|
||||
|
||||
if ($nocache) {
|
||||
$this->calculation = grade_calculation::fetch('itemid', $this->id);
|
||||
}
|
||||
|
||||
return $this->calculation;
|
||||
return !empty($this->calculation);
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns calculation string if grade calculated.
|
||||
* @return mixed string if calculation used, null if not
|
||||
*/
|
||||
function get_calculation() {
|
||||
if ($this->is_calculated()) {
|
||||
return $this->calculation->calculation;
|
||||
} else {
|
||||
return NULL;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Sets this item's calculation (creates it) if not yet set, or
|
||||
* updates it if already set (in the DB). If no calculation is given,
|
||||
* the calculation is removed.
|
||||
* @param string $formula
|
||||
* @return boolean
|
||||
* @param string $formula string representation of formula used for calculation
|
||||
* @return boolean success
|
||||
*/
|
||||
function set_calculation($formula) {
|
||||
// remove cached calculation object
|
||||
// refresh cached calculation object
|
||||
$this->calculation = null;
|
||||
|
||||
$result = true;
|
||||
|
||||
if (empty($formula)) { // We are removing this calculation
|
||||
if (!empty($this->id)) {
|
||||
if ($grade_calculation = $this->get_calculation(true)) {
|
||||
$grade_calculation->delete();
|
||||
}
|
||||
if (!empty($this->id) and $this->is_calculated()) {
|
||||
$this->calculation->delete();
|
||||
$this->calculation = null; // remove cache
|
||||
}
|
||||
$this->calculation = false; // cache no calculation present
|
||||
$this->force_regrading();
|
||||
return true;
|
||||
|
||||
} else { // We are updating or creating the calculation entry in the DB
|
||||
if ($grade_calculation = $this->get_calculation(true)) {
|
||||
$grade_calculation->calculation = $formula;
|
||||
if ($grade_calculation->update()) {
|
||||
$this->force_regrading();
|
||||
return true;
|
||||
} else {
|
||||
if ($this->is_calculated()) {
|
||||
$this->calculation->calculation = $formula;
|
||||
if (!$this->calculation->update()) {
|
||||
$this->calculation = null; // remove cache
|
||||
$result = false;
|
||||
debugging("Could not save the calculation in the database for this grade_item.");
|
||||
return false;
|
||||
}
|
||||
|
||||
} else {
|
||||
$grade_calculation = new grade_calculation();
|
||||
$grade_calculation->calculation = $formula;
|
||||
$grade_calculation->itemid = $this->id;
|
||||
|
||||
if ($grade_calculation->insert()) {
|
||||
return true;
|
||||
} else {
|
||||
$grade_calculation = new grade_calculation(array('calculation'=>$formula, 'itemid'=>$this->id), false);
|
||||
if (!$grade_calculation->insert()) {
|
||||
$this->calculation = null; // remove cache
|
||||
$result = false;
|
||||
debugging("Could not save the calculation in the database for this grade_item.");
|
||||
return false;
|
||||
}
|
||||
$this->calculation = $grade_calculation;
|
||||
}
|
||||
}
|
||||
|
||||
$this->force_regrading();
|
||||
|
||||
return $result;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -979,7 +966,7 @@ class grade_item extends grade_object {
|
||||
return array();
|
||||
}
|
||||
|
||||
if ($calculation = $this->get_calculation()) {
|
||||
if ($calculation = $this->is_calculated()) {
|
||||
return $calculation->dependson();
|
||||
|
||||
} else if ($this->itemtype == 'category') {
|
||||
@@ -1030,7 +1017,7 @@ class grade_item extends grade_object {
|
||||
function update_raw_grade($userid, $rawgrade=false, $howmodified='manual', $note=NULL, $feedback=false, $feedbackformat=FORMAT_MOODLE) {
|
||||
|
||||
// calculated grades can not be updated
|
||||
if ($this->get_calculation()) {
|
||||
if ($this->is_calculated()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
|
||||
+57
-59
@@ -116,6 +116,15 @@ function grade_update($source, $courseid, $itemtype, $itemmodule, $iteminstance,
|
||||
$params = compact('courseid', 'itemtype', 'itemmodule', 'iteminstance', 'itemnumber');
|
||||
if ($itemdetails) {
|
||||
$itemdetails = (array)$itemdetails;
|
||||
|
||||
// grademin and grademax ignored when scale specified
|
||||
if (array_key_exists('scaleid', $itemdetails)) {
|
||||
if ($itemdetails['scaleid']) {
|
||||
unset($itemdetails['grademin']);
|
||||
unset($itemdetails['grademax']);
|
||||
}
|
||||
}
|
||||
|
||||
foreach ($itemdetails as $k=>$v) {
|
||||
if (!in_array($k, $allowed)) {
|
||||
// ignore it
|
||||
@@ -320,52 +329,66 @@ function grade_get_items($courseid, $itemtype=NULL, $itemmodule=NULL, $iteminsta
|
||||
* Updates all final grades in course.
|
||||
*
|
||||
* @param int $courseid
|
||||
* @param boolean $regradeall force regrading of all items
|
||||
*
|
||||
* @return boolean true if ok, array of errors if problems found
|
||||
*/
|
||||
function grade_update_final_grades($courseid) {
|
||||
$errors = array();
|
||||
$grade_item = new grade_item();
|
||||
$grade_item->courseid = $courseid;
|
||||
function grade_update_final_grades($courseid, $regradeall=false) {
|
||||
|
||||
if ($regradeall) {
|
||||
set_field('grade_items', 'needsupdate', 1, 'courseid', $courseid);
|
||||
}
|
||||
|
||||
$grade_item = new grade_item(array('courseid'=>$courseid), false);
|
||||
|
||||
if (!$grade_items = $grade_item->fetch_all_using_this()) {
|
||||
return true;
|
||||
}
|
||||
|
||||
$needsupdate = false;
|
||||
$calculated = false;
|
||||
foreach ($grade_items as $gid=>$gitem) {
|
||||
$grade_item =& $grade_items[$gid];
|
||||
if ($grade_item->needsupdate) {
|
||||
$needsupdate = true;
|
||||
}
|
||||
if ($grade_item->get_calculation()) {
|
||||
$calculated = true;
|
||||
}
|
||||
}
|
||||
|
||||
// no update needed
|
||||
if (!$needsupdate) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// the easy way
|
||||
if (!$calculated) {
|
||||
if (!$regradeall) {
|
||||
$needsupdate = false;
|
||||
$calculated = false;
|
||||
foreach ($grade_items as $gid=>$gitem) {
|
||||
$grade_item =& $grade_items[$gid];
|
||||
if ($grade_item->needsupdate) {
|
||||
$result = $grade_item->update_final_grades();
|
||||
if ($result !== true) {
|
||||
$errors = array_merge($errors, $result);
|
||||
}
|
||||
$needsupdate = true;
|
||||
}
|
||||
if ($grade_item->is_calculated()) {
|
||||
$calculated = true;
|
||||
}
|
||||
}
|
||||
|
||||
if (count($errors) == 0) {
|
||||
|
||||
if (!$needsupdate) {
|
||||
// no update needed
|
||||
return true;
|
||||
} else {
|
||||
return $errors;
|
||||
|
||||
} else if ($calculated) {
|
||||
// flag all calculated grade items with needsupdate
|
||||
// we want to make sure all are ok, this can be improved later with proper dependency calculation
|
||||
foreach ($grade_items as $gid=>$gitem) {
|
||||
$grade_item =& $grade_items[$gid];
|
||||
if (!$grade_item->is_calculated()) {
|
||||
continue;
|
||||
}
|
||||
$grade_item->update_from_db(); // make sure we have current data, it might have been updated in this loop already
|
||||
if (!$grade_item->needsupdate) {
|
||||
//force recalculation and forced update of all parents
|
||||
$grade_item->force_regrading();
|
||||
}
|
||||
}
|
||||
|
||||
// again make sure all date is up-to-date - the needsupdate flag might have changed
|
||||
foreach ($grade_items as $gid=>$gitem) {
|
||||
$grade_item =& $grade_items[$gid];
|
||||
$grade_item->update_from_db();
|
||||
unset($grade_item->category);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
$errors = array();
|
||||
|
||||
// now the hard way with calculated grade_items or categories
|
||||
$finalitems = array();
|
||||
$finalids = array();
|
||||
@@ -373,41 +396,16 @@ function grade_update_final_grades($courseid) {
|
||||
$count = 0;
|
||||
foreach ($grade_items as $gid=>$gitem) {
|
||||
$grade_item =& $grade_items[$gid];
|
||||
if (!$grade_item->needsupdate and $grade_item->itemtype!='category' and !$grade_item->get_calculation()) {
|
||||
if (!$grade_item->needsupdate) {
|
||||
$finalitems[$gid] = $grade_item;
|
||||
$finalids[] = $gid;
|
||||
unset($grade_items[$gid]);
|
||||
continue;
|
||||
}
|
||||
|
||||
//do we have all data for finalizing of this item?
|
||||
$dependson = $grade_item->dependson();
|
||||
|
||||
//are we dealing with category with no calculated items?
|
||||
// we can not trust the needsupdate flag because category might contain calculated items
|
||||
if ($grade_item->itemtype=='category' and !$grade_item->needsupdate) {
|
||||
$forceupdate = false;
|
||||
foreach ($dependson as $childid) {
|
||||
if (in_array($childid, $finalids)) {
|
||||
$child = $finalitems[$childid];
|
||||
} else {
|
||||
$child = $grade_items[$childid];
|
||||
}
|
||||
if ($child->itemtype == 'category' or $child->get_calculation()) {
|
||||
$forceupdate = true;
|
||||
}
|
||||
}
|
||||
|
||||
if ($forceupdate) {
|
||||
$grade_item->force_regrading();
|
||||
} else {
|
||||
$finalitems[$gid] = $grade_item;
|
||||
$finalids[] = $gid;
|
||||
unset($grade_items[$gid]);
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
//do we have all data for this item?
|
||||
$doupdate = true;
|
||||
foreach ($dependson as $did) {
|
||||
if (!in_array($did, $finalids)) {
|
||||
@@ -430,7 +428,7 @@ function grade_update_final_grades($courseid) {
|
||||
|
||||
if ($count == 0) {
|
||||
foreach($grade_items as $grade_item) {
|
||||
$errors[] = 'Probably circular reference in grade_item id:'.$grade_item->id; // TODO: localize
|
||||
$errors[] = 'Probably circular reference or broken calculation formula in grade_item id:'.$grade_item->id; // TODO: localize
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
||||
@@ -232,22 +232,20 @@ class grade_item_test extends grade_test {
|
||||
$this->assertEqual($this->grade_grades[0]->finalgrade, $final_grade->finalgrade);
|
||||
}
|
||||
|
||||
function test_grade_item_get_calculation() {
|
||||
function test_grade_item_is_calculated() {
|
||||
$grade_item = new grade_item($this->grade_items[1]);
|
||||
$this->assertTrue(method_exists($grade_item, 'get_calculation'));
|
||||
$grade_calculation = $grade_item->get_calculation();
|
||||
|
||||
$this->assertEqual($this->grade_calculations[0]->id, $grade_calculation->id);
|
||||
$this->assertTrue(method_exists($grade_item, 'is_calculated'));
|
||||
$this->assertTrue($grade_item->is_calculated());
|
||||
}
|
||||
|
||||
function test_grade_item_set_calculation() {
|
||||
/* $grade_item = new grade_item($this->grade_items[1]);
|
||||
$this->assertTrue(method_exists($grade_item, 'set_calculation'));
|
||||
$this->assertTrue(method_exists($grade_item, 'get_calculation'));
|
||||
$this->assertTrue(method_exists($grade_item, 'is_calculated'));
|
||||
|
||||
$calculation = '=SUM([unittestgradeitem1], [unittestgradeitem3])';
|
||||
$grade_item->set_calculation($calculation);
|
||||
$new_calculation = $grade_item->get_calculation();
|
||||
$new_calculation = $grade_item->is_calculated();
|
||||
|
||||
$this->assertEqual($calculation, $new_calculation->calculation);
|
||||
*/ }
|
||||
|
||||
Reference in New Issue
Block a user