From: skodak Date: Sun, 8 Jul 2007 14:57:19 +0000 (+0000) Subject: MDL-10226 improved regrading of final grades - optimised db access, partial regrading... X-Git-Url: http://git.mjollnir.org/gw?a=commitdiff_plain;h=f8e6e4dbeaabbbecc7ea43c00e6b3ca9483f3abf;p=moodle.git MDL-10226 improved regrading of final grades - optimised db access, partial regrading when raw grade updated --- diff --git a/lib/grade/grade_category.php b/lib/grade/grade_category.php index 781fd44a0f..36869df2e3 100644 --- a/lib/grade/grade_category.php +++ b/lib/grade/grade_category.php @@ -177,15 +177,10 @@ class grade_category extends grade_object { // Recalculate grades if needed if ($this->qualifies_for_regrading()) { - if (!parent::update($source)) { - return false; - } - $this->grade_item->force_regrading($source); - return true; - - } else { - return parent::update($source); + $this->force_regrading(); } + + return parent::update($source); } /** @@ -199,6 +194,8 @@ class grade_category extends grade_object { return false; } + $this->force_regrading(); + $grade_item = $this->load_grade_item(); $parent = $this->load_parent_category(); @@ -248,6 +245,8 @@ class grade_category extends grade_object { return false; } + $this->force_regrading(); + // build path and depth $this->update($source); @@ -293,48 +292,12 @@ class grade_category extends grade_object { } /** - * Sets this category's and its parent's grade_item.needsupdate to true. - * This is triggered whenever any change in any lower level may cause grade_finals - * for this category to require an update. The flag needs to be propagated up all - * levels until it reaches the top category. This is then used to determine whether or not - * to regenerate the raw and final grades for each category grade_item. This is accomplished - * thanks to the path variable, so we don't need to use recursion. - * @param string $source from where was the object updated (mod/forum, manual, etc.) - * @return boolean Success or failure + * Marks the category and course item as needing update - categories are always regraded. + * @return void */ - function force_regrading($source=null) { - if (empty($this->id)) { - debugging("Needsupdate requested before inserting grade category."); - return 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 = ''; - - foreach ($paths as $categoryid) { - $wheresql .= "iteminstance = $categoryid OR "; - } - $wheresql = substr($wheresql, 0, strrpos($wheresql, 'OR')); - $grade_items = set_field_select('grade_items', 'needsupdate', '1', $wheresql . ' AND courseid = ' . $this->courseid); - $this->grade_item->update_from_db(); - - } - - return $result; + function force_regrading() { + $grade_item = $this->load_grade_item(); + $grade_item->force_regrading(); } /** @@ -352,7 +315,7 @@ class grade_category extends grade_object { * 3. Aggregate these grades * 4. Save them in raw grades of associated category grade item */ - function generate_grades() { + function generate_grades($userid=null) { global $CFG; $this->load_grade_item(); @@ -363,44 +326,51 @@ class grade_category extends grade_object { $this->grade_item->load_scale(); - // find grade items of immediate children (category or grade items) $depends_on = $this->grade_item->depends_on(); - $items = array(); + if (empty($depends_on)) { + $items = false; + } else { + $gis = implode(',', $depends_on); + $sql = "SELECT * + FROM {$CFG->prefix}grade_items + WHERE id IN ($gis)"; + $items = get_records_sql($sql); + } - foreach($depends_on as $dep) { - $items[$dep] = grade_item::fetch(array('id'=>$dep)); + if ($userid) { + $usersql = "AND g.userid=$userid"; + } else { + $usersql = ""; } - // where to look for final grades - include or grade item too + // where to look for final grades - include grade of this item too, we will store the results there $gis = implode(',', array_merge($depends_on, array($this->grade_item->id))); - $sql = "SELECT g.* FROM {$CFG->prefix}grade_grades g, {$CFG->prefix}grade_items gi - WHERE gi.id = g.itemid AND gi.courseid={$this->grade_item->courseid} AND gi.id IN ($gis) + WHERE gi.id = g.itemid AND gi.id IN ($gis) $usersql ORDER BY g.userid"; // group the results by userid and aggregate the grades in this group if ($rs = get_recordset_sql($sql)) { if ($rs->RecordCount() > 0) { $prevuser = 0; - $grades = array(); - $final = null; + $grade_records = array(); + $oldgrade = null; while ($used = rs_fetch_next_record($rs)) { if ($used->userid != $prevuser) { - $this->aggregate_grades($prevuser, $items, $grades, $depends_on, $final); + $this->aggregate_grades($prevuser, $items, $grade_records, $oldgrade); $prevuser = $used->userid; - $grades = array(); - $final = null; + $grade_records = array(); + $oldgrade = null; } - if ($used->itemid == $this->grade_item->id) { - $final = new grade_grades($used, false); - $final->grade_item =& $this->grade_item; + $grade_records[$used->itemid] = $used->finalgrade; + if ($this->grade_item->id == $used->itemid) { + $oldgrade = $used; } - $grades[$used->itemid] = $used->finalgrade; } - $this->aggregate_grades($prevuser, $items, $grades, $depends_on, $final); + $this->aggregate_grades($prevuser, $items, $grade_records, $oldgrade);//the last one } } @@ -410,88 +380,103 @@ class grade_category extends grade_object { /** * internal function for category grades aggregation */ - function aggregate_grades($userid, $items, $grades, $depends_on, $final) { + function aggregate_grades($userid, $items, $grade_records, $oldgrade) { if (empty($userid)) { - //ignore first run + //ignore first call return; } - // no circular references allowed - unset($grades[$this->grade_item->id]); + if ($oldgrade) { + $grade = new grade_grades($oldgrade, false); + $grade->grade_item =& $this->grade_item; - // insert final grade - it will be needed later anyway - if (empty($final)) { - $final = new grade_grades(array('itemid'=>$this->grade_item->id, 'userid'=>$userid), false); - $final->insert(); - $final->grade_item =& $this->grade_item; + } else { + // insert final grade - it will be needed later anyway + $grade = new grade_grades(array('itemid'=>$this->grade_item->id, 'userid'=>$userid), false); + $grade->insert('system'); + $grade->grade_item =& $this->grade_item; + + $oldgrade = new object(); + $oldgrade->finalgrade = $grade->finalgrade; + $oldgrade->rawgrade = $grade->rawgrade; + $oldgrade->rawgrademin = $grade->rawgrademin; + $oldgrade->rawgrademax = $grade->rawgrademax; + $oldgrade->rawscaleid = $grade->rawscaleid; + } - } else if ($final->is_locked()) { - // no need to recalculate locked grades + // locked grades are not regraded + if ($grade->is_locked()) { return; } + // can not use own final category grade in calculation + unset($grade_records[$this->grade_item->id]); + // if no grades calculation possible or grading not allowed clear both final and raw - if (empty($grades) or empty($items) or ($this->grade_item->gradetype != GRADE_TYPE_VALUE and $this->grade_item->gradetype != GRADE_TYPE_SCALE)) { - $final->finalgrade = null; - $final->rawgrade = null; - $final->update(); + if (empty($grade_records) or empty($items) or ($this->grade_item->gradetype != GRADE_TYPE_VALUE and $this->grade_item->gradetype != GRADE_TYPE_SCALE)) { + $grade->finalgrade = null; + $grade->rawgrade = null; + if ($grade->finalgrade !== $oldgrade->finalgrade or $grade->rawgrade !== $oldgrade->rawgrade) { + $grade->update('system'); + } return; } - // normalize the grades first - all will have value 0...1 + /// normalize the grades first - all will have value 0...1 // ungraded items are not used in aggreagation - foreach ($grades as $k=>$v) { + foreach ($grade_records as $k=>$v) { if (is_null($v)) { // null means no grade - unset($grades[$k]); + unset($grade_records[$k]); continue; } - $grades[$k] = grade_grades::standardise_score($v, $items[$k]->grademin, $items[$k]->grademax, 0, 1); + $grade_records[$k] = grade_grades::standardise_score($v, $items[$k]->grademin, $items[$k]->grademax, 0, 1); } //limit and sort - $this->apply_limit_rules($grades); - sort($grades, SORT_NUMERIC); + $this->apply_limit_rules($grade_records); + sort($grade_records, SORT_NUMERIC); // let's see we have still enough grades to do any statisctics - if (count($grades) == 0) { + if (count($grade_records) == 0) { // not enough attempts yet - if (!is_null($final->finalgrade) or !is_null($final->rawgrade)) { - $final->finalgrade = null; - $final->rawgrade = null; - $final->update(); + $grade->finalgrade = null; + $grade->rawgrade = null; + if ($grade->finalgrade !== $oldgrade->finalgrade or $grade->rawgrade !== $oldgrade->rawgrade) { + $grade->update('system'); } return; } + /// start the aggregation switch ($this->aggregation) { case GRADE_AGGREGATE_MEDIAN: // Middle point value in the set: ignores frequencies - $num = count($grades); + $num = count($grade_records); $halfpoint = intval($num / 2); if($num % 2 == 0) { - $rawgrade = ($grades[ceil($halfpoint)] + $grades[floor($halfpoint)]) / 2; + $rawgrade = ($grade_records[ceil($halfpoint)] + $grade_records[floor($halfpoint)]) / 2; } else { - $rawgrade = $grades[$halfpoint]; + $rawgrade = $grade_records[$halfpoint]; } break; case GRADE_AGGREGATE_MIN: - $rawgrade = reset($grades); + $rawgrade = reset($grade_records); break; case GRADE_AGGREGATE_MAX: - $rawgrade = array_pop($grades); + $rawgrade = array_pop($grade_records); break; case GRADE_AGGREGATE_MEAN_ALL: // Arithmetic average of all grade items including even NULLs; NULL grade caunted as minimum - $num = count($depends_on); // you can calculate sum from this one if you multiply it with count($this->depends_on() ;-) - $sum = array_sum($grades); + $num = count($items); // you can calculate sum from this one if you multiply it with count($this->depends_on() ;-) + $sum = array_sum($grade_records); $rawgrade = $sum / $num; break; case GRADE_AGGREGATE_MODE: // the most common value, the highest one if multimode - $freq = array_count_values($grades); + $freq = array_count_values($grade_records); arsort($freq); // sort by frequency keeping keys $top = reset($freq); // highest frequency count $modes = array_keys($freq, $top); // search for all modes (have the same highest count) @@ -500,24 +485,34 @@ class grade_category extends grade_object { case GRADE_AGGREGATE_MEAN_GRADED: // Arithmetic average of all final grades, unfinished are not calculated default: - $num = count($grades); - $sum = array_sum($grades); + $num = count($grade_records); + $sum = array_sum($grade_records); $rawgrade = $sum / $num; break; } + /// prepare update of new raw grade + $grade->rawgrademin = $this->grade_item->grademin; + $grade->rawgrademax = $this->grade_item->grademax; + $grade->rawscaleid = $this->grade_item->scaleid; + // recalculate the rawgrade back to requested range - $rawgrade = $this->grade_item->adjust_grade($rawgrade, 0, 1); + $grade->rawgrade = grade_grades::standardise_score($rawgrade, 0, 1, $grade->rawgrademin, $grade->rawgrademax); - // prepare update of new raw grade - $final->rawgrade = $rawgrade; - $final->finalgrade = null; - $final->rawgrademin = $this->grade_item->grademin; - $final->rawgrademax = $this->grade_item->grademax; - $final->rawscaleid = $this->grade_item->scaleid; + // calculate final grade + $grade->finalgrade = $this->grade_item->adjust_grade($grade->rawgrade, $grade->rawgrademin, $grade->rawgrademax); - // TODO - add some checks to prevent updates when not needed - $final->update(); + // update in db if changed + if ( $grade->finalgrade !== $oldgrade->finalgrade + or $grade->rawgrade !== $oldgrade->rawgrade + or $grade->rawgrademin !== $oldgrade->rawgrademin + or $grade->rawgrademax !== $oldgrade->rawgrademax + or $grade->rawscaleid !== $oldgrade->rawscaleid) { + + $grade->update('system'); + } + + return; } /** @@ -739,7 +734,7 @@ class grade_category extends grade_object { // create a new one $grade_item = new grade_item($params, false); $grade_item->gradetype = GRADE_TYPE_VALUE; - $grade_item->insert(); + $grade_item->insert('system'); } else if (count($grade_items) == 1){ // found existing one @@ -797,7 +792,7 @@ class grade_category extends grade_object { * @param int parentid * @return boolean success */ - function set_parent($parentid) { + function set_parent($parentid, $source=null) { if ($this->parent == $parentid) { return true; } @@ -815,17 +810,16 @@ class grade_category extends grade_object { return false; } - $this->force_regrading(); // mark old parent as needing regrading + $this->force_regrading(); // set new parent category - $this->parent = $parentid; + $this->parent = $parent_category->id; + $this->parent_category =& $parent_category; $this->path = null; // remove old path and depth - will be recalculated in update() - $this->parent_category = null; - $this->update(); + $this->depth = null; // remove old path and depth - will be recalculated in update() + $this->update($source); - $grade_item = $this->load_grade_item(); - $grade_item->parent_category = null; - return $grade_item->update(); // marks new parent as needing regrading too + return $grade_item->update($source); } /** diff --git a/lib/grade/grade_item.php b/lib/grade/grade_item.php index 75d4722c77..802781b4b6 100644 --- a/lib/grade/grade_item.php +++ b/lib/grade/grade_item.php @@ -210,12 +210,6 @@ class grade_item extends grade_object { */ var $locktime = 0; - /** - * Whether or not the module instance referred to by this grade_item has been deleted. - * @var int $deleted - */ - var $deleted = 0; - /** * If set, the whole column will be recalculated, then this flag will be switched off. * @var boolean $needsupdate @@ -233,11 +227,10 @@ class grade_item extends grade_object { $this->load_scale(); if ($this->qualifies_for_regrading()) { - return $this->force_regrading(); - - } else { - return parent::update($source); + $this->force_regrading(); } + + return parent::update($source); } /** @@ -262,13 +255,12 @@ class grade_item extends grade_object { $outcomeiddiff = $db_item->outcomeid != $this->outcomeid; $multfactordiff = $db_item->multfactor != $this->multfactor; $plusfactordiff = $db_item->plusfactor != $this->plusfactor; - $deleteddiff = $db_item->deleted != $this->deleted; $needsupdatediff = !$db_item->needsupdate && $this->needsupdate; // force regrading only if setting the flag first time $lockeddiff = !empty($db_item->locked) && empty($this->locked); // force regrading only when unlocking return ($calculationdiff || $categorydiff || $gradetypediff || $grademaxdiff || $grademindiff || $scaleiddiff - || $outcomeiddiff || $multfactordiff || $plusfactordiff || $deleteddiff || $needsupdatediff + || $outcomeiddiff || $multfactordiff || $plusfactordiff || $needsupdatediff || $lockeddiff); } @@ -305,9 +297,7 @@ class grade_item extends grade_object { return false; } - if (!$this->is_category_item() and $category = $this->get_parent_category()) { - $category->force_regrading($source); - } + $this->force_regrading(); if ($grades = grade_grades::fetch_all(array('itemid'=>$this->id))) { foreach ($grades as $grade) { @@ -319,7 +309,7 @@ class grade_item extends grade_object { } /** - * In addition to perform parent::insert(), this calls the grade_item's category's (if applicable) force_regrading() method. + * In addition to perform parent::insert(), calls force_regrading() method too. * @param string $source from where was the object inserted (mod/forum, manual, etc.) * @return int PK ID if successful, false otherwise */ @@ -333,7 +323,7 @@ class grade_item extends grade_object { // load scale if needed $this->load_scale(); - // add parent categroy if needed + // add parent category if needed if (empty($this->categoryid) and !$this->is_course_item() and !$this->is_category_item()) { $course_category = grade_category::fetch_course_category($this->courseid); $this->categoryid = $course_category->id; @@ -508,107 +498,92 @@ class grade_item extends grade_object { } } + /** + * Mark regrading as finished successfully. + */ + function regrading_finished() { + $this->needsupdate = 0; + //do not use $this->update() because we do not want this logged in grade_item_history + set_field('grade_items', 'needsupdate', 0, 'id', $this->id); + + if (!empty($this->locktime) and empty($this->locked) and $this->locktime < time()) { + // time to lock this grade_item + $this->set_locked(true); + } + } + /** * Performs the necessary calculations on the grades_final referenced by this grade_item. * Also resets the needsupdate flag once successfully performed. * - * This function must be use ONLY from lib/gradeslib.php/grade_update_final_grades(), - * because the calculation must be done in correct order!! + * This function must be used ONLY from lib/gradeslib.php/grade_update_final_grades(), + * because the regrading must be done in correct order!! * - * @return boolean true if ok, array of errors otherwise + * @return boolean true if ok, error string otherwise */ - function update_final_grades() { + function update_final_grades($userid=null) { global $CFG; + // locked grade items already have correct final grades if ($this->is_locked()) { - // locked grade items already have correct final grades - $this->needsupdate = false; - $this->update(); return true; } + // calculation produces final value using formula from other final values if ($this->is_calculated()) { - if ($this->compute()) { - $this->needsupdate = false; - $this->update(); + if ($this->compute($userid)) { return true; } else { - return array("Could not calculate grades for grade item id:".$this->id); // TODO: improve and localize + return "Could not calculate grades for grade item"; // TODO: improve and localize } + // aggregate the category grade } else if ($this->is_category_item() or $this->is_course_item()) { // aggregate category grade item $category = $this->get_item_category(); - if (!$category->generate_grades()) { - return ("Could not calculate raw category grades id:".$this->id); // TODO: improve and localize + $category->grade_item =& $this; + if ($category->generate_grades($userid)) { + return true; + } else { + return "Could not aggregate final grades for category:".$this->id; // TODO: improve and localize } } - $errors = array(); - - // we need it to be really fast here ==> sql only - if ($rs = get_recordset('grade_grades', 'itemid', $this->id)) { + // normal grade item - just new final grades + $result = true; + if ($userid) { + $rs = get_recordset_select('grade_grades', "itemid={$this->id} AND userid=$userid"); + } else { + $rs = get_recordset('grade_grades', 'itemid', $this->id); + } + if ($rs) { if ($rs->RecordCount() > 0) { - while ($grade = rs_fetch_next_record($rs)) { - if (!empty($grade->locked)) { + while ($grade_record = rs_fetch_next_record($rs)) { + if (!empty($grade_record->locked)) { // this grade is locked - final grade must be ok continue; } - if (!empty($errors) or is_null($grade->rawgrade)) { - // unset existing final grade when no raw present or error - if (!is_null($grade->finalgrade)) { - $g = new object(); - $g->id = $grade->id; - $g->finalgrade = null; - if (!update_record('grade_grades', $g)) { - $errors[] = "Could not remove final grade for grade item:".$this->id; - } - } - - } else { - $finalgrade = $this->adjust_grade($grade->rawgrade, $grade->rawgrademin, $grade->rawgrademax); + $grade = new grade_grades($grade_record, false); + $grade->finalgrade = $this->adjust_grade($grade->rawgrade, $grade->rawgrademin, $grade->rawgrademax); - if ($finalgrade != $grade->finalgrade) { - $g = new object(); - $g->id = $grade->id; - $g->finalgrade = $finalgrade; - if (!update_record('grade_grades', $g)) { - $errors[] = "Could not update final grade for grade item:".$this->id; - } + if ($grade_record->finalgrade !== $grade->finalgrade) { + if (!$grade->update('system')) { + $result = "Internal error updating final grade"; } + } - // do not use $grade->is_locked() bacause item may be still locked! - if (!empty($grade->locktime) and empty($grade->locked) and $grade->locktime < time()) { - // time to lock this grade - $g = new object(); - $g->id = $grade->id; - $g->locked = time(); - update_record('grade_grades', $g); - } + // time to lock this grade? + if (!empty($grade->locktime) and empty($grade->locked) and $grade->locktime < time()) { + $grade->locked = time(); + $grade->grade_item =& $this; + $grade->set_locked(true); } } } } - if (!empty($errors)) { - $this->force_regrading(); - return $errors; - - } else { - // reset the regrading flag - $this->needsupdate = false; - $this->update(); - - // recheck the needsupdate just to make sure ;-) - if (empty($this->needsupdate) and !empty($this->locktime) - and empty($this->locked) and $this->locktime < time()) { - // time to lock this grade_item - $this->set_locked(true); - } - - return true; - } + return $result; } /** @@ -679,32 +654,14 @@ class grade_item extends grade_object { } /** - * Sets this grade_item's needsupdate to true. Also looks at parent category, if any, and calls - * its force_regrading() method. - * This is triggered whenever any change in any raw grade may cause grade_finals - * for this grade_item to require an update. The flag needs to be propagated up all - * levels until it reaches the top category. This is then used to determine whether or not - * to regenerate the raw and final grades for each category grade_item. - * @param string $source from where was the object updated (mod/forum, manual, etc.) - * @return boolean Success or failure + * Sets this grade_item's needsupdate to true. Also marks the course item as needing update. + * @return void */ - function force_regrading($source=null) { - $this->needsupdate = true; - - if (!parent::update($source)) { - return false; - } - - if ($this->is_course_item()) { - // no parent - - } else { - $parent = $this->load_parent_category(); - $parent->force_regrading($source); - - } - - return true; + function force_regrading() { + $this->needsupdate = 1; + //mark this item and course item only - categories and calculated items are always regraded + $wheresql = "(itemtype='course' OR id={$this->id}) AND courseid={$this->courseid}"; + set_field_select('grade_items', 'needsupdate', 1, $wheresql); } /** @@ -799,20 +756,41 @@ class grade_item extends grade_object { return $this->item_category; } + /** + * Is the grade item associated with category? + * @return boolean + */ function is_category_item() { return ($this->itemtype == 'category'); } + /** + * Is the grade item associated with course? + * @return boolean + */ function is_course_item() { return ($this->itemtype == 'course'); } + /** + * Is the grade item normal - associated with module, plugin or something else? + * @return boolean + */ + function is_normal_item() { + return ($this->itemtype != 'course' and $this->itemtype != 'category'); + } + + /** + * Returns grade item associated with the course + * @param int $courseid + * @return course item object + */ function fetch_course_item($courseid) { if ($course_item = grade_item::fetch(array('courseid'=>$courseid, 'itemtype'=>'course'))) { return $course_item; } - // first call - let category insert one + // first get category - it creates the associated grade item $course_category = grade_category::fetch_course_category($courseid); return grade_item::fetch(array('courseid'=>$courseid, 'itemtype'=>'course')); @@ -998,13 +976,13 @@ class grade_item extends grade_object { return false; } - $this->force_regrading(); // mark old parent as needing regrading + $this->force_regrading(); // set new parent - $this->categoryid = $parentid; - $this->parent_category = null; + $this->categoryid = $parent_category->id; + $this->parent_category =& $parent_category; - return $this->update(); // mark new parent as needing regrading too + return $this->update(); } /** @@ -1078,29 +1056,36 @@ class grade_item extends grade_object { return false; } + // TODO: we should IMO prevent modification of raw grades for course and categroy item too because + // there is no way to prevent overriding of it + // do not allow grade updates when item locked - this prevents fetching of grade from db if ($this->is_locked()) { return false; } - $grade = new grade_grades(array('itemid'=>$this->id, 'userid'=>$userid, 'usermodified'=>$usermodified)); - $grade->grade_item =& $this; // prevent db fetching of cached grade_item - - if (!empty($grade->id)) { - if ($grade->is_locked()) { - // do not update locked grades at all - return false; - } + if (!$grade = grade_grades::fetch(array('itemid'=>$this->id, 'userid'=>$userid))) { + $grade = new grade_grades(array('itemid'=>$this->id, 'userid'=>$userid), false); + } - if (!empty($grade->locktime) and $grade->locktime < time()) { - // do not update grades that should be already locked - // this does not solve all problems, cron is still needed to recalculate the final grades periodically - return false; - } + $grade->grade_item =& $this; // prevent db fetching of this grade_item + $oldgrade = new object(); + $oldgrade->finalgrade = $grade->finalgrade; + $oldgrade->rawgrade = $grade->rawgrade; + $oldgrade->rawgrademin = $grade->rawgrademin; + $oldgrade->rawgrademax = $grade->rawgrademax; + $oldgrade->rawscaleid = $grade->rawscaleid; + if ($grade->is_locked()) { + // do not update locked grades at all + return false; } - //TODO: if grade tree does not need to be recalculated, try to update grades of all users in course and force_regrading only if failed + if (!empty($grade->locktime) and $grade->locktime < time()) { + // do not update grades that should be already locked + // this does not solve all problems, cron is still needed to recalculate the final grades periodically + return false; + } // fist copy current grademin/max and scale $grade->rawgrademin = $this->grademin; @@ -1108,33 +1093,36 @@ class grade_item extends grade_object { $grade->rawscaleid = $this->scaleid; if ($rawgrade !== false) { - // change of grade value requested - if (empty($grade->id)) { - $oldgrade = null; - $grade->rawgrade = $rawgrade; - $result = $grade->insert($source); + $grade->rawgrade = $rawgrade; + } - } else { - $oldgrade = $grade->rawgrade; - $grade->rawgrade = $rawgrade; - $result = $grade->update($source); - } + if (empty($grade->id)) { + $result = (boolean)$grade->insert($source); + + } else if ($grade->finalgrade !== $oldgrade->finalgrade + or $grade->rawgrade !== $oldgrade->rawgrade + or $grade->rawgrademin !== $oldgrade->rawgrademin + or $grade->rawgrademax !== $oldgrade->rawgrademax + or $grade->rawscaleid !== $oldgrade->rawscaleid) { + + $result = $grade->update($source); } // do we have comment from teacher? if ($result and $feedback !== false) { - if (empty($grade->id)) { - // create new grade - $oldgrade = null; - $result = $grade->insert($source); - } - $result = $result && $grade->update_feedback($feedback, $feedbackformat, $usermodified); + $result = $grade->update_feedback($feedback, $feedbackformat, $usermodified); } - // TODO Handle history recording error, such as displaying a notice, but still return true - - // This grade item needs update - $this->force_regrading(); + if (!$this->needsupdate) { + $course_item = grade_item::fetch_course_item($this->courseid); + if (!$course_item->needsupdate) { + if (!grade_update_final_grades($this->courseid, $userid, $this)) { + $this->force_regrading(); + } + } else { + $this->force_regrading(); + } + } if ($result) { @@ -1174,7 +1162,7 @@ class grade_item extends grade_object { * The parameteres are taken from final grades of grade items in current course only. * @return boolean false if error */ - function compute() { + function compute($userid=null) { global $CFG; if (!$this->is_calculated()) { @@ -1198,9 +1186,15 @@ class grade_item extends grade_object { // this itemid is added so that we use only one query for source and final grades $gis = implode(',', array_merge($useditems, array($this->id))); + if ($userid) { + $usersql = "AND g.userid=$userid"; + } else { + $usersql = ""; + } + $sql = "SELECT g.* FROM {$CFG->prefix}grade_grades g, {$CFG->prefix}grade_items gi - WHERE gi.id = g.itemid AND gi.courseid={$this->courseid} AND gi.id IN ($gis) + WHERE gi.id = g.itemid AND gi.courseid={$this->courseid} AND gi.id IN ($gis) $usersql ORDER BY g.userid"; $return = true; @@ -1209,37 +1203,35 @@ class grade_item extends grade_object { if ($rs = get_recordset_sql($sql)) { if ($rs->RecordCount() > 0) { $prevuser = 0; - $grades = array(); - $final = null; + $grade_records = array(); + $oldgrade = null; while ($used = rs_fetch_next_record($rs)) { if ($used->userid != $prevuser) { - if (!$this->use_formula($prevuser, $grades, $useditems, $final)) { + if (!$this->use_formula($prevuser, $grade_records, $useditems, $oldgrade)) { $return = false; } $prevuser = $used->userid; - $grades = array(); - $final = null; + $grade_records = array(); + $oldgrade = null; } if ($used->itemid == $this->id) { - $final = new grade_grades($used, false); // fetching from db is not needed - $final->grade_item =& $this; + $oldgrade = $used; } - $grades['gi'.$used->itemid] = $used->finalgrade; + $grade_records['gi'.$used->itemid] = $used->finalgrade; } - if (!$this->use_formula($prevuser, $grades, $useditems, $final)) { + if (!$this->use_formula($prevuser, $grade_records, $useditems, $oldgrade)) { $return = false; } } } - //TODO: we could return array of errors here return $return; } /** * internal function - does the final grade calculation */ - function use_formula($userid, $params, $useditems, $final) { + function use_formula($userid, $params, $useditems, $oldgrade) { if (empty($userid)) { return true; } @@ -1258,30 +1250,41 @@ class grade_item extends grade_object { unset($params['gi'.$this->id]); // insert final grade - will be needed later anyway - if (empty($final)) { - $final = new grade_grades(array('itemid'=>$this->id, 'userid'=>$userid), false); - $final->insert(); - $final->grade_item =& $this; + if ($oldgrade) { + $grade = new grade_grades($oldgrade, false); // fetching from db is not needed + $grade->grade_item =& $this; - } else if ($final->is_locked()) { - // no need to recalculate locked grades - return; + } else { + $grade = new grade_grades(array('itemid'=>$this->id, 'userid'=>$userid, 'rawgrademin'=>null, 'rawgrademax'=>null, 'rawscaledi'=>null), false); + $grade->insert('system'); + $grade->grade_item =& $this; + + $oldgrade = new object(); + $oldgrade->finalgrade = $grade->finalgrade; + $oldgrade->rawgrade = $grade->rawgrade; + $oldgrade->rawgrademin = $grade->rawgrademin; + $oldgrade->rawgrademax = $grade->rawgrademax; + $oldgrade->rawscaleid = $grade->rawscaleid; } + // no need to recalculate locked grades + if ($grade->is_locked()) { + return; + } // do the calculation $this->formula->set_params($params); $result = $this->formula->evaluate(); - // store the result + // no raw grade for calculated grades - only final + $grade->rawgrademin = null; + $grade->rawgrademax = null; + $grade->rawscaleid = null; + $grade->rawgrade = null; + + if ($result === false) { - // error during calculation - if (!is_null($final->finalgrade) or !is_null($final->rawgrade)) { - $final->finalgrade = null; - $final->rawgrade = null; - $final->update(); - } - return false; + $grade->finalgrade = null; } else { // normalize @@ -1289,15 +1292,25 @@ class grade_item extends grade_object { if ($this->gradetype == GRADE_TYPE_SCALE) { $result = round($result+0.00001); // round scales upwards } + $grade->finalgrade = $result; + } - // store only if final grade changed, remove raw grade because we do not need it - if ($final->finalgrade != $result or !is_null($final->rawgrade)) { - $final->finalgrade = $result; - $final->rawgrade = null; - $final->update(); - } + // update in db if changed + if ( $grade->finalgrade !== $oldgrade->finalgrade + or $grade->rawgrade !== $oldgrade->rawgrade + or $grade->rawgrademin !== $oldgrade->rawgrademin + or $grade->rawgrademax !== $oldgrade->rawgrademax + or $grade->rawscaleid !== $oldgrade->rawscaleid) { + + $grade->update('system'); + } + + if ($result === false) { + return false; + } else { return true; } + } } diff --git a/lib/gradelib.php b/lib/gradelib.php index 84a722ea42..702bdfca22 100644 --- a/lib/gradelib.php +++ b/lib/gradelib.php @@ -299,74 +299,62 @@ function grade_is_locked($courseid, $itemtype, $itemmodule, $iteminstance, $item /***** END OF PUBLIC API *****/ +function grade_force_full_regrading($courseid) { + set_field('grade_items', 'needsupdate', 1, 'courseid', $courseid); +} /** * 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 + * @param int $userid if specified, try to do a quick regrading of grades of this user only + * @param object $updated_item the item in which + * @return boolean true if ok, array of errors if problems found (item id is used as key) */ -function grade_update_final_grades($courseid, $regradeall=false) { - - if ($regradeall) { - set_field('grade_items', 'needsupdate', 1, 'courseid', $courseid); - } +function grade_update_final_grades($courseid, $userid=null, $updated_item=null) { - if (!$grade_items = grade_item::fetch_all(array('courseid'=>$courseid))) { - return true; - } + $course_item = grade_item::fetch_course_item($courseid); - if (!$regradeall) { - $needsupdate = false; - $calculated = false; - foreach ($grade_items as $gid=>$gitem) { - $grade_item =& $grade_items[$gid]; - if ($grade_item->needsupdate) { - $needsupdate = true; - } - if ($grade_item->is_calculated()) { - $calculated = true; - } + if ($userid) { + // one raw grade updated for one user + if (empty($updated_item)) { + error("updated_item_id can not be null!"); + } + if ($course_item->needsupdate) { + $updated_item->force_regrading(); + return 'Can not do fast regrading after updating of raw grades'; } - if (!$needsupdate) { - // no update needed + } else { + if (!$course_item->needsupdate) { + // nothing to do :-) return true; - - } 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); - } } } + $grade_items = grade_item::fetch_all(array('courseid'=>$courseid)); + $depends_on = array(); + + // first mark all category and calculated items as needing regrading + // this is slower, but 100% accurate - this function is called only when there is + // a change in grading setup, update of individual grade does not trigger this function + foreach ($grade_items as $gid=>$gitem) { + if (!empty($updated_item) and $updated_item->id = $gid) { + $grade_items[$gid]->needsupdate = 1; + + } else if ($grade_items[$gid]->is_category_item() or $grade_items[$gid]->is_calculated()) { + $grade_items[$gid]->needsupdate = 1; + } + // construct depends_on lookup array + $depends_on[$gid] = $grade_items[$gid]->depends_on(); + } $errors = array(); - // now the hard way with calculated grade_items or categories $finalitems = array(); $finalids = array(); while (count($grade_items) > 0) { - $count = 0; + $count = 0; // count how many items were updated in this cycle foreach ($grade_items as $gid=>$gitem) { $grade_item =& $grade_items[$gid]; if (!$grade_item->needsupdate) { @@ -376,32 +364,35 @@ function grade_update_final_grades($courseid, $regradeall=false) { continue; } - //do we have all data for finalizing of this item? - $depends_on = $grade_item->depends_on(); - $doupdate = true; - foreach ($depends_on as $did) { + foreach ($depends_on[$gid] as $did) { if (!in_array($did, $finalids)) { $doupdate = false; + break; } } //oki - let's update, calculate or aggregate :-) if ($doupdate) { - $result = $grade_item->update_final_grades(); - if ($result !== true) { - $errors = array_merge($errors, $result); - } else { + $result = $grade_item->update_final_grades($userid); + + if ($result === true) { + $grade_item->regrading_finished(); + $count++; $finalitems[$gid] = $grade_item; $finalids[] = $gid; unset($grade_items[$gid]); + } else { + $grade_item->force_regrading(); + $errors[$gid] = $result; } } } if ($count == 0) { foreach($grade_items as $grade_item) { - $errors[] = 'Probably circular reference or broken calculation formula in grade_item id:'.$grade_item->id; // TODO: localize + $grade_item->force_regrading(); + $errors[$grade_item->id] = 'Probably circular reference or broken calculation formula'; // TODO: localize } break; } diff --git a/lib/weblib.php b/lib/weblib.php index bd45ca10dd..a21a35ee86 100644 --- a/lib/weblib.php +++ b/lib/weblib.php @@ -548,7 +548,7 @@ function addslashes_recursive($var) { } else if (is_string($var)) { $new_var = addslashes($var); - } else { + } else { // nulls, integers, etc. $new_var = $var; }