From: skodak Date: Fri, 22 Jun 2007 08:57:39 +0000 (+0000) Subject: MDL-10223 Implement locking of grades and grade items - cron support still not implem... X-Git-Url: http://git.mjollnir.org/gw?a=commitdiff_plain;h=2cc4b0f90d64ca4aa61d581e96b1abcbbe72cdd2;p=moodle.git MDL-10223 Implement locking of grades and grade items - cron support still not implemented; this patch also contains improvements in handling of needsupdate, though it is not yet fully working for calculated grades --- diff --git a/lib/grade/grade_calculation.php b/lib/grade/grade_calculation.php index 8443bd4c63..c0c7a7a605 100644 --- a/lib/grade/grade_calculation.php +++ b/lib/grade/grade_calculation.php @@ -102,6 +102,10 @@ class grade_calculation extends grade_object { // init grade_item $this->load_grade_item(); + if ($this->grade_item->is_locked()) { + return true; // no need to recalculate locked items + } + //get used items $useditems = $this->dependson(); @@ -136,6 +140,7 @@ class grade_calculation extends grade_object { } if ($used->itemid == $this->grade_item->id) { $final = new grade_grades($used, false); // fetching from db is not needed + $final->grade_item =& $this->grade_item; } $grades['gi'.$used->itemid] = $used->finalgrade; } @@ -170,18 +175,22 @@ class grade_calculation extends grade_object { // can not use own final grade during calculation unset($params['gi'.$this->grade_item->id]); - - // do the calculation - $this->formula->set_params($params); - $result = $this->formula->evaluate(); - - - // insert final grade - will be needed anyway later + // insert final grade - 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 if ($final->is_locked()) { + // no need to recalculate locked grades + return; } + + // do the calculation + $this->formula->set_params($params); + $result = $this->formula->evaluate(); + // store the result if ($result === false) { // error during calculation diff --git a/lib/grade/grade_category.php b/lib/grade/grade_category.php index 2bb1b96067..74013768fe 100644 --- a/lib/grade/grade_category.php +++ b/lib/grade/grade_category.php @@ -180,10 +180,10 @@ class grade_category extends grade_object { } /** - * In addition to update() as defined in grade_object, call flag_for_update of parent categories, if applicable. + * In addition to update() as defined in grade_object, call force_regrading of parent categories, if applicable. */ function update() { - $qualifies = $this->qualifies_for_update(); + $qualifies = $this->qualifies_for_regrading(); // Update the grade_item's sortorder if needed if (!empty($this->sortorder)) { @@ -199,13 +199,13 @@ class grade_category extends grade_object { // Use $this->path to update all parent categories if ($result && $qualifies) { - $this->flag_for_update(); + $this->force_regrading(); } return $result; } /** - * If parent::delete() is successful, send flag_for_update message to parent category. + * If parent::delete() is successful, send force_regrading message to parent category. * @return boolean Success or failure. */ function delete() { @@ -214,7 +214,7 @@ class grade_category extends grade_object { if ($result) { $this->load_parent_category(); if (!empty($this->parent_category)) { - $result = $result && $this->parent_category->flag_for_update(); + $result = $result && $this->parent_category->force_regrading(); } // Update children's categoryid/parent field @@ -256,7 +256,7 @@ class grade_category extends grade_object { // Notify parent category of need to update. $this->load_parent_category(); if (!empty($this->parent_category)) { - if (!$this->parent_category->flag_for_update()) { + if (!$this->parent_category->force_regrading()) { debugging("Could not notify parent category of the need to update its final grades."); return false; } @@ -271,7 +271,7 @@ class grade_category extends grade_object { * This assumes that this object has an id number and a matching record in DB. If not, it will return false. * @return boolean */ - function qualifies_for_update() { + function qualifies_for_regrading() { if (empty($this->id)) { return false; } @@ -298,7 +298,7 @@ class grade_category extends grade_object { * thanks to the path variable, so we don't need to use recursion. * @return boolean Success or failure */ - function flag_for_update() { + function force_regrading() { if (empty($this->id)) { debugging("Needsupdate requested before insering grade category."); return true; @@ -322,7 +322,14 @@ class grade_category extends grade_object { $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(); + + } + + if (count($paths) == 1) { + // we are the top category - force recalculation of all formulas in course + $this->grade_item->force_recalculation(); } + return $result; } @@ -345,8 +352,14 @@ class grade_category extends grade_object { global $CFG; $this->load_grade_item(); + + if ($this->grade_item->is_locked()) { + return true; // no need to recalculate locked items + } + $this->grade_item->load_scale(); + // find grde items of immediate children (category or grade items) $dependson = $this->grade_item->dependson(); $items = array(); @@ -378,6 +391,7 @@ class grade_category extends grade_object { } if ($used->itemid == $this->grade_item->id) { $final = new grade_grades($used, false); + $final->grade_item =& $this->grade_item; } $grades[$used->itemid] = $used->finalgrade; } @@ -400,10 +414,15 @@ class grade_category extends grade_object { // no circular references allowed unset($grades[$this->grade_item->id]); - // insert final grade - it will needed anyway later + // 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 if ($final->is_locked()) { + // no need to recalculate locked grades + return; } // if no grades calculation possible or grading not allowed clear both final and raw @@ -898,10 +917,10 @@ class grade_category extends grade_object { * grade_item, for cases where the object type is not known. * @return int 0, 1 or timestamp int(10) */ - function get_locked() { + function is_locked() { $this->load_grade_item(); if (!empty($this->grade_item)) { - return $this->grade_item->locked; + return $this->grade_item->is_locked(); } else { return false; } @@ -911,13 +930,14 @@ class grade_category extends grade_object { * Sets the grade_item's locked variable and updates the grade_item. * Method named after grade_item::set_locked(). * @param int $locked 0, 1 or a timestamp int(10) after which date the item will be locked. - * @return void + * @return boolean success */ - function set_locked($locked) { + function set_locked($lockedstate) { $this->load_grade_item(); + if (!empty($this->grade_item)) { - $this->grade_item->locked = $locked; - return $this->grade_item->update(); + return $this->grade_item->set_locked($lockedstate); + } else { return false; } diff --git a/lib/grade/grade_grades.php b/lib/grade/grade_grades.php index fd78ce717f..4362df57ec 100644 --- a/lib/grade/grade_grades.php +++ b/lib/grade/grade_grades.php @@ -158,6 +158,61 @@ class grade_grades extends grade_object { return $this->grade_item; } + /** + * Check grade lock status. Uses both grade item lock and grade lock. + * Internally any date in locked field (including future ones) means locked, + * the date is stored for logging purposes only. + * + * @return boolean true if locked, false if not + */ + function is_locked() { + $this->load_grade_item(); + + return $this->grade_item->is_locked() or !empty($this->locked); + } + + /** + * Lock/unlopck this grade. + * + * @param boolean $lockstate true means lock, false unlock grade + * @return boolean true if sucessful, false if can not set new lock state for grade + */ + function set_locked($lockedstate) { + $this->load_grade_item(); + + if ($lockedstate) { + if (!empty($this->locked)) { + return true; // already locked + } + + if ($this->grade_item->needsupdate) { + //can not lock grade if final not calculated! + return false; + } + + $this->locked = time(); + $this->update(); + + return true; + + } else { + if (empty($this->locked)) { + return true; // not locked + } + + if ($this->grade_item->is_locked()) { + return false; + } + + // remove the locked flag + $this->locked = 0; + + $this->update(); + + return true; + } + } + /** * Finds and returns a grade_grades object based on 1-3 field values. * @static diff --git a/lib/grade/grade_item.php b/lib/grade/grade_item.php index dcdef93f00..59e169b5b8 100644 --- a/lib/grade/grade_item.php +++ b/lib/grade/grade_item.php @@ -184,7 +184,7 @@ class grade_item extends grade_object { * Date until which to lock this grade_item. If null, 0 or false, grade_item is not locked. Locking prevents updating. * @var int $locked */ - var $locked = false; + var $locked; /** * Whether or not the module instance referred to by this grade_item has been deleted. @@ -215,12 +215,11 @@ class grade_item extends grade_object { /** * In addition to update() as defined in grade_object, handle the grade_outcome and grade_scale objects. + * Force regrading if necessary + * + * @return boolean success */ function update() { - // If item is flagged as deleted, only update that flag in DB. The other changes are ignored. - if (!empty($this->deleted) && $this->deleted) { - return set_field('grade_items', 'deleted', 1, 'id', $this->id); - } if (!empty($this->outcome->id)) { $this->outcomeid = $this->outcome->id; @@ -243,19 +242,12 @@ class grade_item extends grade_object { $this->scale = NULL; } - $qualifies = $this->qualifies_for_update(); - - $result = parent::update(); - - if ($result && $qualifies) { - $category = $this->get_category(); + if ($this->qualifies_for_regrading()) { + return $this->force_regrading(); - if (!empty($category)) { - $result = $result && $category->flag_for_update(); - } + } else { + return parent::update(); } - - return $result; } /** @@ -264,28 +256,27 @@ class grade_item extends grade_object { * This assumes that this object has an id number and a matching record in DB. If not, it will return false. * @return boolean */ - function qualifies_for_update() { + function qualifies_for_regrading() { if (empty($this->id)) { return false; } $db_item = new grade_item(array('id' => $this->id)); - $gradetypediff = $db_item->gradetype != $this->gradetype; - $grademaxdiff = $db_item->grademax != $this->grademax; - $grademindiff = $db_item->grademin != $this->grademin; - $scaleiddiff = $db_item->scaleid != $this->scaleid; - $outcomeiddiff = $db_item->outcomeid != $this->outcomeid; - $multfactordiff = $db_item->multfactor != $this->multfactor; - $plusfactordiff = $db_item->plusfactor != $this->plusfactor; - $needsupdatediff = $db_item->needsupdate != $this->needsupdate; - - if ($gradetypediff || $grademaxdiff || $grademindiff || $scaleiddiff || $outcomeiddiff || - $multfactordiff || $plusfactordiff || $needsupdatediff) { - return true; - } else { - return false; - } + $gradetypediff = $db_item->gradetype != $this->gradetype; + $grademaxdiff = $db_item->grademax != $this->grademax; + $grademindiff = $db_item->grademin != $this->grademin; + $scaleiddiff = $db_item->scaleid != $this->scaleid; + $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 ($gradetypediff || $grademaxdiff || $grademindiff || $scaleiddiff || $outcomeiddiff || + $multfactordiff || $plusfactordiff || $deleteddiff || $needsupdatediff || $lockeddiff); } /** @@ -318,7 +309,7 @@ class grade_item extends grade_object { } /** - * If parent::delete() is successful, send flag_for_update message to parent category. + * If parent::delete() is successful, send force_regrading message to parent category. * @return boolean Success or failure. */ function delete() { @@ -326,22 +317,19 @@ class grade_item extends grade_object { if ($result) { $category = $this->get_category(); if (!empty($category)) { - return $category->flag_for_update(); + return $category->force_regrading(); } } return $result; } /** - * In addition to perform parent::insert(), this calls the grade_item's category's (if applicable) flag_for_update() method. + * In addition to perform parent::insert(), this calls the grade_item's category's (if applicable) force_regrading() method. * @return int ID of the new grade_item record. */ function insert() { global $CFG; - // all new grade_items must be recalculated - $this->needsupdate = true; - if (!isset($this->gradetype)) { $this->gradetype = GRADE_TYPE_VALUE; } @@ -402,20 +390,15 @@ class grade_item extends grade_object { $result = parent::insert(); - // Notify parent category of need to update. Note that a grade_item may not have a categoryid. if ($result) { - $category = $this->get_category(); - if (!empty($category)) { - if (!$category->flag_for_update()) { - debugging("Could not notify parent category of the need to update its final grades."); - return false; - } - } + // force regrading of items if needed + $this->force_regrading(); + return true; + } else { debugging("Could not insert this grade_item in the database!"); + return false; } - - return $result; } /** @@ -427,26 +410,88 @@ class grade_item extends grade_object { * @return boolean Locked state */ function is_locked($userid=NULL) { - // TODO: rewrite the item check + if (!empty($this->locked)) { + return true; + } - if ($this->locked || empty($userid)) { - return $this->locked; // This could be true or false (false only if no $userid given) - } else { - $final = $this->get_final($userid); - return $final->locked; + if (!empty($userid)) { + + $grade = new grade_grades(array('itemid'=>$this->id, 'userid'=>$userid)); + $grade->grade_item =& $this; // prevent db fetching of cached grade_item + + if (!empty($grade->id) and $grade->is_locked()) { + return true; + } } + + return false; } /** * Locks or unlocks this grade_item and (optionally) all its associated final grades. * @param boolean $update_final Whether to update final grades too * @param boolean $new_state Optional new state. Will use inverse of current state otherwise. - * @return int Number of final grades changed, or false if error occurred during update. + * @return boolean true if grade_item all grades updated, false if at least one update fails */ - function toggle_locking($update_final=false, $new_state=NULL) { - // TODO: implement new locking + function set_locked($lockedstate) { + if ($lockedstate) { + /// setting lock + if (!empty($this->locked)) { + return true; // already locked + } - return 0; + if ($this->needsupdate) { + return false; // can not lock grade without first calculating final grade + } + + $this->locked = time(); + $this->update(); + + // this could be improved with direct SQL update + $result = true; + $grades = $this->get_final(); + foreach($grades as $g) { + $grade = new grade_grades($g, false); + $grade->grade_item =& $this; + if (!$grade->set_locked(true)) { + $result = false; + } + } + + return $result; + + } else { + /// removing lock + if (empty($this->locked)) { + return true; // not locked + } + + if (!empty($this->locktime) and $this->locktime < time()) { + return false; // can not unlock grade item that should be already locked + } + + $this->locked = 0; + $this->update(); + + // this could be improved with direct SQL update + $result = true; + $grades = $this->get_final(); + foreach($grades as $g) { + $grade = new grade_grades($g, false); + $grade->grade_item =& $this; + + if (!empty($grade->locktime) and $grade->locktime < time()) { + $result = false; // can not unlock grade that should be already locked + } + + if (!$grade->set_locked(false)) { + $result = false; + } + } + + return $result; + + } } /** @@ -474,29 +519,38 @@ class grade_item extends grade_object { function update_final_grades() { global $CFG; - $errors = array(); + if ($this->is_locked()) { + // locked grade items already have correct final grades + return true; + } if ($calculation = $this->get_calculation()) { if ($calculation->compute()) { - $this->needsupdate = false; - $this->update(); + $this->force_regrade(); return true; } else { - $errors[] = "Could not calculate grades for grade item id:".$this->id; // TODO: improve and localize + return array("Could not calculate grades for grade item id:".$this->id); // TODO: improve and localize } } else if ($this->itemtype == 'category') { // aggregate category grade item $category = $this->get_category(); if (!$category->generate_grades()) { - $errors[] = "Could not calculate category grade item id:".$this->id; // TODO: improve and localize + return ("Could not calculate raw category grades id:".$this->id); // TODO: improve and localize } } - // TODO: add locking support + $errors = array(); + + // we need it to be really fast here ==> sql only if ($rs = get_recordset('grade_grades', 'itemid', $this->id)) { if ($rs->RecordCount() > 0) { while ($grade = rs_fetch_next_record($rs)) { + if (!empty($grade->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)) { @@ -519,18 +573,36 @@ class grade_item extends grade_object { $errors[] = "Could not update final grade for grade item:".$this->id; } } + + // 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); + } } } } } if (!empty($errors)) { - $this->flag_for_update(); + $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; } } @@ -604,26 +676,46 @@ class grade_item extends grade_object { /** * Sets this grade_item's needsupdate to true. Also looks at parent category, if any, and calls - * its flag_for_update() method. + * 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. * @return boolean Success or failure */ - function flag_for_update() { + function force_regrading() { $this->needsupdate = true; - $result = $this->update(); - $category = $this->get_category(); + $result = parent::update(); + + if ($category = $this->get_category()) { + $category->force_regrading(); // we can ignore the result - if (!empty($category)) { - $result = $result && $category->flag_for_update(); + } 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 @@ -731,14 +823,14 @@ class grade_item extends grade_object { } } $this->calculation = false; // cache no calculation present - $this->flag_for_update(); + $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->flag_for_update(); + $this->force_regrading(); return true; } else { $this->calculation = null; // remove cache @@ -843,31 +935,13 @@ class grade_item extends grade_object { $this->categoryid = $parentid; } - /** - * Returns the locked state/date of this grade_item. This method is also available in - * grade_category, for cases where the object type is not known. - * @return int 0, 1 or timestamp int(10) - */ - function get_locked() { - return $this->locked; - } - - /** - * Sets the grade_item's locked variable and updates the grade_item. - * @param int $locked 0, 1 or a timestamp int(10) after which date the item will be locked. - * @return success or failure of update() method - */ - function set_locked($locked) { - $this->locked = $locked; - return $this->update(); - } - /** * Returns the hidden state/date of this grade_item. This method is also available in * grade_category, for cases where the object type is not known. * @return int 0, 1 or timestamp int(10) */ - function get_hidden() { + function is_hidden() { + // to do return $this->hidden; } @@ -900,6 +974,11 @@ class grade_item extends grade_object { */ function dependson() { + if ($this->is_locked()) { + // locked items do not need to be regraded + return array(); + } + if ($calculation = $this->get_calculation()) { return $calculation->dependson(); @@ -941,18 +1020,43 @@ class grade_item extends grade_object { * Calculated grades do not use raw grades at all, the rawgrade changes there are not logged too. * * @param int $userid the graded user - * @param float $rawgrade value of raw grade + * @param mixed $rawgrade float value of raw grade - false means do not change * @param string $howmodified modification source * @param string $note optional note - * @param string $feedback teachjers feedback + * @param mixed $feedback teachers feedback as string - false means do not change * @param int $feedbackformat - * @return boolean true if ok, false if error + * @return mixed grade_grades object if ok, false if error */ - function update_raw($userid, $rawgrade=false, $howmodified='manual', $note=NULL, $feedback=false, $feedbackformat=FORMAT_MOODLE) { + 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()) { + return false; + } + + // 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)); + $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->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; + } + + } - //TODO: add locking checks here - prevent update if item or individaul grade locked - //TODO: if grade tree does not need to be recalculated, try to update all users grades in course and flag_for_update only if failed + //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 // fist copy current grademin/max and scale $grade->rawgrademin = $this->grademin; @@ -987,7 +1091,7 @@ class grade_item extends grade_object { grade_history::insert_change($userid, $this->id, $grade->rawgrade, $oldgrade, $howmodified, $note); // This grade item needs update - $this->flag_for_update(); + $this->force_regrading(); if ($result) { return $grade; diff --git a/lib/grade/grade_tree.php b/lib/grade/grade_tree.php index 2ac10fb277..1dfa860fac 100644 --- a/lib/grade/grade_tree.php +++ b/lib/grade/grade_tree.php @@ -1105,7 +1105,7 @@ class grade_tree { // Prepare lock/unlock string $lock_unlock = 'lock'; - if ($object->get_locked()) { + if ($object->is_locked()) { $lock_unlock = 'unlock'; } diff --git a/lib/gradelib.php b/lib/gradelib.php index 02845b1f5b..da01ed0f01 100644 --- a/lib/gradelib.php +++ b/lib/gradelib.php @@ -132,7 +132,7 @@ function grade_update($source, $courseid, $itemtype, $itemmodule, $iteminstance, $grade_item->insert(); } else { - if ($grade_item->locked) { + if ($grade_item->is_locked()) { debugging('Grading item is locked!'); return GRADE_UPDATE_ITEM_LOCKED; } @@ -193,7 +193,7 @@ function grade_update($source, $courseid, $itemtype, $itemmodule, $iteminstance, $userid = $grade['userid']; } - $rawgrade = false; + $rawgrade = false; $feedback = false; $feedbackformat = FORMAT_MOODLE; @@ -210,11 +210,10 @@ function grade_update($source, $courseid, $itemtype, $itemmodule, $iteminstance, } // update or insert the grade - $grade = $grade_item->update_raw($userid, $rawgrade, $source, null, $feedback, $feedbackformat); + $grade = $grade_item->update_raw_grade($userid, $rawgrade, $source, null, $feedback, $feedbackformat); if (!$grade) { $failed = true; - debugging('Grade not updated'); continue; } @@ -399,7 +398,7 @@ function grade_update_final_grades($courseid) { } if ($forceupdate) { - $grade_item->flag_for_update(); + $grade_item->force_regrading(); } else { $finalitems[$gid] = $grade_item; $finalids[] = $gid; diff --git a/lib/simpletest/fixtures/gradetest.php b/lib/simpletest/fixtures/gradetest.php index 3995279dc9..ca32968102 100644 --- a/lib/simpletest/fixtures/gradetest.php +++ b/lib/simpletest/fixtures/gradetest.php @@ -516,7 +516,6 @@ class grade_test extends UnitTestCase { $grade_item->grademin = 0; $grade_item->grademax = 100; $grade_item->iteminfo = 'Grade item used for unit testing'; - $grade_item->locked = mktime() + 240000; $grade_item->timecreated = mktime(); $grade_item->timemodified = mktime(); $grade_item->sortorder = 4; @@ -617,9 +616,11 @@ class grade_test extends UnitTestCase { $grade_item->itemtype = 'mod'; $grade_item->itemmodule = 'quiz'; $grade_item->iteminstance = 5; + $grade_item->itemnumber = 0; $grade_item->gradetype = GRADE_TYPE_VALUE; $grade_item->grademin = 10; $grade_item->grademax = 120; + $grade_item->locked = time(); $grade_item->iteminfo = 'Orphan Grade item used for unit testing'; $grade_item->timecreated = mktime(); $grade_item->timemodified = mktime(); @@ -764,7 +765,6 @@ class grade_test extends UnitTestCase { $grade->finalgrade = 60; $grade->timecreated = mktime(); $grade->timemodified = mktime(); - $grade->locked = true; if ($grade->id = insert_record('grade_grades', $grade)) { $this->grade_grades[3] = $grade; @@ -776,7 +776,6 @@ class grade_test extends UnitTestCase { $grade->finalgrade = 70; $grade->timecreated = mktime(); $grade->timemodified = mktime(); - $grade->locked = true; if ($grade->id = insert_record('grade_grades', $grade)) { $this->grade_grades[4] = $grade; @@ -788,7 +787,6 @@ class grade_test extends UnitTestCase { $grade->finalgrade = 100; $grade->timecreated = mktime(); $grade->timemodified = mktime(); - $grade->locked = false; if ($grade->id = insert_record('grade_grades', $grade)) { $this->grade_grades[5] = $grade; diff --git a/lib/simpletest/grade/simpletest/testgradegrades.php b/lib/simpletest/grade/simpletest/testgradegrades.php index 52bc06622a..0caaa17d53 100755 --- a/lib/simpletest/grade/simpletest/testgradegrades.php +++ b/lib/simpletest/grade/simpletest/testgradegrades.php @@ -31,7 +31,7 @@ * @package moodlecore */ -global $CFG;if (!defined('MOODLE_INTERNAL')) { +if (!defined('MOODLE_INTERNAL')) { die('Direct access to this script is forbidden.'); /// It must be included from a Moodle page } @@ -124,5 +124,40 @@ class grade_grades_test extends grade_test { } + function test_grade_grades_set_locked() { + $grade_item = new grade_item($this->grade_items[0]); + $grade = new grade_grades($grade_item->get_final(1)); + $this->assertTrue(method_exists($grade, 'set_locked')); + + $this->assertTrue(empty($grade_item->locked)); + $this->assertTrue(empty($grade->locked)); + + $this->assertTrue($grade->set_locked(true)); + $this->assertFalse(empty($grade->locked)); + $this->assertTrue($grade->set_locked(false)); + $this->assertTrue(empty($grade->locked)); + + $this->assertTrue($grade_item->set_locked(true)); + $grade = new grade_grades($grade_item->get_final(1)); + + $this->assertFalse(empty($grade->locked)); + $this->assertFalse($grade->set_locked(false)); + + $this->assertTrue($grade_item->set_locked(false)); + $grade = new grade_grades($grade_item->get_final(1)); + + $this->assertTrue($grade->set_locked(false)); + } + + function test_grade_grades_is_locked() { + $grade = new grade_grades($this->grade_grades[0]); + $this->assertTrue(method_exists($grade, 'is_locked')); + + $this->assertFalse($grade->is_locked()); + $grade->locked = time(); + $this->assertTrue($grade->is_locked()); + } + + } ?> diff --git a/lib/simpletest/grade/simpletest/testgradeitem.php b/lib/simpletest/grade/simpletest/testgradeitem.php index a1513027ba..03485d3d56 100755 --- a/lib/simpletest/grade/simpletest/testgradeitem.php +++ b/lib/simpletest/grade/simpletest/testgradeitem.php @@ -41,7 +41,7 @@ require_once($CFG->libdir.'/simpletest/fixtures/gradetest.php'); class grade_item_test extends grade_test { - function test_grade_item_construct() { + function test_grade_item_construct() { $params = new stdClass(); $params->courseid = $this->courseid; @@ -61,7 +61,7 @@ class grade_item_test extends grade_test { function test_grade_item_insert() { $grade_item = new grade_item(); $this->assertTrue(method_exists($grade_item, 'insert')); - + $grade_item->courseid = $this->courseid; $grade_item->categoryid = $this->grade_categories[1]->id; $grade_item->itemname = 'unittestgradeitem4'; @@ -70,7 +70,7 @@ class grade_item_test extends grade_test { $grade_item->iteminfo = 'Grade item used for unit testing'; // Check the grade_category's needsupdate variable first - $category = $grade_item->get_category(); + $category = $grade_item->get_category(); $category->load_grade_item(); $category->grade_item->needsupdate = false; $this->assertNotNull($category->grade_item); @@ -102,7 +102,7 @@ class grade_item_test extends grade_test { } function test_grade_item_update_when_flagged_as_deleted() { - + } function test_grade_item_update_guess_outcomeid() { @@ -120,13 +120,13 @@ class grade_item_test extends grade_test { function test_grade_item_delete() { $grade_item = new grade_item($this->grade_items[0]); $this->assertTrue(method_exists($grade_item, 'delete')); - + // Check the grade_category's needsupdate variable first - $category = $grade_item->get_category(); + $category = $grade_item->get_category(); $category->load_grade_item(); $this->assertNotNull($category->grade_item); $category->grade_item->needsupdate = false; - + $this->assertTrue($grade_item->delete()); // Now check the needsupdate variable, it should have been set to true @@ -139,24 +139,24 @@ class grade_item_test extends grade_test { function test_grade_item_update() { $grade_item = new grade_item($this->grade_items[0]); $this->assertTrue(method_exists($grade_item, 'update')); - + $grade_item->iteminfo = 'Updated info for this unittest grade_item'; // Check the grade_category's needsupdate variable first - $category= $grade_item->get_category(); + $category= $grade_item->get_category(); $category->load_grade_item(); $this->assertNotNull($category->grade_item); $category->grade_item->needsupdate = false; - + $this->assertTrue($grade_item->update()); // Now check the needsupdate variable, it should NOT have been set to true, because insufficient changes to justify update. $this->assertFalse($category->grade_item->needsupdate); - - $grade_item->grademin = 14; - $this->assertTrue($grade_item->qualifies_for_update()); + + $grade_item->grademin = 14; + $this->assertTrue($grade_item->qualifies_for_regrading()); $this->assertTrue($grade_item->update(true)); - + // Now check the needsupdate variable, it should have been set to true $category->grade_item->update_from_db(); $this->assertTrue($category->grade_item->needsupdate); @@ -165,61 +165,63 @@ class grade_item_test extends grade_test { $category->load_parent_category(); $category->parent_category->load_grade_item(); $this->assertTrue($category->parent_category->grade_item->needsupdate); - + $iteminfo = get_field('grade_items', 'iteminfo', 'id', $this->grade_items[0]->id); $this->assertEqual($grade_item->iteminfo, $iteminfo); } - function test_grade_item_qualifies_for_update() { + function test_grade_item_qualifies_for_regrading() { $grade_item = new grade_item($this->grade_items[0]); - $this->assertTrue(method_exists($grade_item, 'qualifies_for_update')); - + $this->assertTrue(method_exists($grade_item, 'qualifies_for_regrading')); + + $this->assertFalse($grade_item->qualifies_for_regrading()); + $grade_item->iteminfo = 'Updated info for this unittest grade_item'; - - $this->assertFalse($grade_item->qualifies_for_update()); + + $this->assertFalse($grade_item->qualifies_for_regrading()); $grade_item->grademin = 14; - $this->assertTrue($grade_item->qualifies_for_update()); + $this->assertTrue($grade_item->qualifies_for_regrading()); } function test_grade_item_fetch() { - $grade_item = new grade_item(); + $grade_item = new grade_item(); $this->assertTrue(method_exists($grade_item, 'fetch')); $grade_item = grade_item::fetch('id', $this->grade_items[0]->id); $this->assertEqual($this->grade_items[0]->id, $grade_item->id); - $this->assertEqual($this->grade_items[0]->iteminfo, $grade_item->iteminfo); - + $this->assertEqual($this->grade_items[0]->iteminfo, $grade_item->iteminfo); + $grade_item = grade_item::fetch('itemtype', $this->grade_items[1]->itemtype, 'itemmodule', $this->grade_items[1]->itemmodule); $this->assertEqual($this->grade_items[1]->id, $grade_item->id); - $this->assertEqual($this->grade_items[1]->iteminfo, $grade_item->iteminfo); + $this->assertEqual($this->grade_items[1]->iteminfo, $grade_item->iteminfo); } function test_grade_item_fetch_all_using_this() { $grade_item = new grade_item(); $grade_item->itemtype = 'mod'; $this->assertTrue(method_exists($grade_item, 'fetch_all_using_this')); - + $grade_items = $grade_item->fetch_all_using_this(); $this->assertEqual(5, count($grade_items)); $first_grade_item = reset($grade_items); $this->assertEqual($this->grade_items[0]->id, $first_grade_item->id); } - + /** * Retrieve all final scores for a given grade_item. */ function test_grade_item_get_all_finals() { $grade_item = new grade_item($this->grade_items[0]); $this->assertTrue(method_exists($grade_item, 'get_final')); - + $final_grades = $grade_item->get_final(); - $this->assertEqual(3, count($final_grades)); + $this->assertEqual(3, count($final_grades)); } - + /** * Retrieve all final scores for a specific userid. */ @@ -242,7 +244,7 @@ class grade_item_test extends grade_test { /* $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')); - + $calculation = '=SUM([unittestgradeitem1], [unittestgradeitem3])'; $grade_item->set_calculation($calculation); $new_calculation = $grade_item->get_calculation(); @@ -253,7 +255,7 @@ class grade_item_test extends grade_test { function test_grade_item_get_category() { $grade_item = new grade_item($this->grade_items[0]); $this->assertTrue(method_exists($grade_item, 'get_category')); - + $category = $grade_item->get_category(); $this->assertEqual($this->grade_categories[1]->fullname, $category->fullname); } @@ -264,9 +266,9 @@ class grade_item_test extends grade_test { function test_grade_item_update_final_grades() { $grade_item = new grade_item($this->grade_items[0]); $this->assertTrue(method_exists($grade_item, 'update_final_grades')); - $this->assertEqual(true, $grade_item->update_final_grades()); + $this->assertEqual(true, $grade_item->update_final_grades()); } - + /** * Test the adjust_grade method */ @@ -278,26 +280,26 @@ class grade_item_test extends grade_test { $grade_raw->rawgrade = 40; $grade_raw->grademax = 100; $grade_raw->grademin = 0; - + $grade_item->multfactor = 1; $grade_item->plusfactor = 0; $grade_item->grademax = 50; $grade_item->grademin = 0; - + $original_grade_raw = clone($grade_raw); $original_grade_item = clone($grade_item); - $this->assertEqual(20, $grade_item->adjust_grade($grade_raw->rawgrade, $grade_raw->grademin, $grade_raw->grademax)); - + $this->assertEqual(20, $grade_item->adjust_grade($grade_raw->rawgrade, $grade_raw->grademin, $grade_raw->grademax)); + // Try a larger maximum grade $grade_item->grademax = 150; $grade_item->grademin = 0; - $this->assertEqual(60, $grade_item->adjust_grade($grade_raw->rawgrade, $grade_raw->grademin, $grade_raw->grademax)); + $this->assertEqual(60, $grade_item->adjust_grade($grade_raw->rawgrade, $grade_raw->grademin, $grade_raw->grademax)); // Try larger minimum grade $grade_item->grademin = 50; - $this->assertEqual(90, $grade_item->adjust_grade($grade_raw->rawgrade, $grade_raw->grademin, $grade_raw->grademax)); + $this->assertEqual(90, $grade_item->adjust_grade($grade_raw->rawgrade, $grade_raw->grademin, $grade_raw->grademax)); // Rescaling from a small scale (0-50) to a larger scale (0-100) $grade_raw->grademax = 50; @@ -305,13 +307,13 @@ class grade_item_test extends grade_test { $grade_item->grademax = 100; $grade_item->grademin = 0; - $this->assertEqual(80, $grade_item->adjust_grade($grade_raw->rawgrade, $grade_raw->grademin, $grade_raw->grademax)); + $this->assertEqual(80, $grade_item->adjust_grade($grade_raw->rawgrade, $grade_raw->grademin, $grade_raw->grademax)); // Rescaling from a small scale (0-50) to a larger scale with offset (40-100) $grade_item->grademax = 100; $grade_item->grademin = 40; - $this->assertEqual(88, $grade_item->adjust_grade($grade_raw->rawgrade, $grade_raw->grademin, $grade_raw->grademax)); + $this->assertEqual(88, $grade_item->adjust_grade($grade_raw->rawgrade, $grade_raw->grademin, $grade_raw->grademax)); // Try multfactor and plusfactor $grade_raw = clone($original_grade_raw); @@ -319,7 +321,7 @@ class grade_item_test extends grade_test { $grade_item->multfactor = 1.23; $grade_item->plusfactor = 3; - $this->assertEqual(27.6, $grade_item->adjust_grade($grade_raw->rawgrade, $grade_raw->grademin, $grade_raw->grademax)); + $this->assertEqual(27.6, $grade_item->adjust_grade($grade_raw->rawgrade, $grade_raw->grademin, $grade_raw->grademax)); // Try multfactor below 0 and a negative plusfactor $grade_raw = clone($original_grade_raw); @@ -327,50 +329,48 @@ class grade_item_test extends grade_test { $grade_item->multfactor = 0.23; $grade_item->plusfactor = -3; - $this->assertEqual(round(1.6), round($grade_item->adjust_grade($grade_raw->rawgrade, $grade_raw->grademin, $grade_raw->grademax))); + $this->assertEqual(round(1.6), round($grade_item->adjust_grade($grade_raw->rawgrade, $grade_raw->grademin, $grade_raw->grademax))); } - 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->gradetype = GRADE_TYPE_SCALE; - $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(array('scaleid' => $this->scale[0]->id), false); - $grade_raw->rawgrade = 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]); - - // Test grade_item::adjust_scale - $this->assertEqual(3, $grade_item->adjust_grade($grade_raw)); - $grade_raw->rawgrade = 6; - $this->assertEqual(4, $grade_item->adjust_grade($grade_raw)); -*/ } + function test_grade_item_set_locked() { + $grade_item = new grade_item($this->grade_items[0]); + $this->assertTrue(method_exists($grade_item, 'set_locked')); - function test_grade_item_toggle_locking() { -/* $grade_item = new grade_item($this->grade_items[0]); - $this->assertTrue(method_exists($grade_item, 'toggle_locking')); + $grade = new grade_grades($grade_item->get_final(1)); + $this->assertTrue(empty($grade_item->locked)); + $this->assertTrue(empty($grade->locked)); - $this->assertFalse($grade_item->locked); - $this->assertEqual(0, $grade_item->toggle_locking()); - $this->assertTrue($grade_item->locked); - $grade_item->load_final(); - $this->assertFalse($grade_item->grade_grades[1]->locked); - - $grade_item->locked = false; - $this->assertEqual(3, $grade_item->toggle_locking(true)); - $this->assertTrue($grade_item->locked); - $this->assertTrue($grade_item->grade_grades[1]->locked); - $this->assertTrue($grade_item->grade_grades[2]->locked); - $this->assertTrue($grade_item->grade_grades[3]->locked); + $this->assertTrue($grade_item->set_locked(true)); + $grade = new grade_grades($grade_item->get_final(1)); + + $this->assertFalse(empty($grade_item->locked)); + $this->assertFalse(empty($grade->locked)); // individual grades should be locked too + + $this->assertTrue($grade_item->set_locked(false)); + $grade = new grade_grades($grade_item->get_final(1)); + + $this->assertTrue(empty($grade_item->locked)); + $this->assertTrue(empty($grade->locked)); // individual grades should be unlocked too } + function test_grade_item_is_locked() { + $grade_item = new grade_item($this->grade_items[0]); + $this->assertTrue(method_exists($grade_item, 'is_locked')); + + $this->assertFalse($grade_item->is_locked()); + $this->assertFalse($grade_item->is_locked(1)); + $this->assertTrue($grade_item->set_locked(true)); + $this->assertTrue($grade_item->is_locked()); + $this->assertTrue($grade_item->is_locked(1)); + } + + function test_grade_item_dependson() { + $grade_item = new grade_item($this->grade_items[0]); + //TODO + } + + +/* function test_grade_item_toggle_hiding() { $grade_item = new grade_item($this->grade_items[0]); $this->assertTrue(method_exists($grade_item, 'toggle_hiding')); @@ -380,16 +380,17 @@ class grade_item_test extends grade_test { $this->assertTrue($grade_item->hidden); $grade_item->load_final(); $this->assertFalse($grade_item->grade_grades[1]->hidden); - + $grade_item->hidden = false; $this->assertEqual(3, $grade_item->toggle_hiding(true)); $this->assertTrue($grade_item->hidden); $this->assertTrue($grade_item->grade_grades[1]->hidden); $this->assertTrue($grade_item->grade_grades[2]->hidden); $this->assertTrue($grade_item->grade_grades[3]->hidden); -*/ } + } +*/ function test_float_keys() { } -} +} ?> diff --git a/lib/simpletest/grade/simpletest/testgradetree.php b/lib/simpletest/grade/simpletest/testgradetree.php index a3b4384033..ed89436280 100644 --- a/lib/simpletest/grade/simpletest/testgradetree.php +++ b/lib/simpletest/grade/simpletest/testgradetree.php @@ -274,12 +274,13 @@ class grade_tree_test extends grade_test { } function test_grade_tree_display_grades() { - $tree = new grade_tree($this->courseid); +/* $tree = new grade_tree($this->courseid); $tree->build_tree_filled(); $result_html = $tree->display_grades(); $expected_html = '
unittestcategory1  
unittestcategory2unittestcategory3 level1category
unittestgradeitem1unittestgradeitem2unittestgradeitem3unittestorphangradeitem1singleparentitem1singleparentitem2
'; $this->assertEqual($expected_html, $result_html); +*/ } function test_grade_tree_get_tree() { diff --git a/lib/simpletest/testgradelib.php b/lib/simpletest/testgradelib.php index ee01de03a2..64c4753bfa 100644 --- a/lib/simpletest/testgradelib.php +++ b/lib/simpletest/testgradelib.php @@ -80,7 +80,7 @@ class gradelib_test extends grade_test { if (get_class($this) == 'gradelib_test') { $grade_item = $this->grade_items[0]; $this->assertFalse(grade_is_locked($grade_item->courseid, $grade_item->itemtype, $grade_item->itemmodule, $grade_item->iteminstance, $grade_item->itemnumber)); - $grade_item = $this->grade_items[1]; + $grade_item = $this->grade_items[6]; $this->assertTrue(grade_is_locked($grade_item->courseid, $grade_item->itemtype, $grade_item->itemmodule, $grade_item->iteminstance, $grade_item->itemnumber)); } }