From 9c8d38fa97d21f9050f43286917c897a5dcd82b2 Mon Sep 17 00:00:00 2001 From: skodak Date: Fri, 31 Aug 2007 22:42:05 +0000 Subject: [PATCH] MDL-11092 events are not used for changed raw grades anymore + minor cleanup in handling of raw grades (now used only for modules or other external plugins that produce grades) --- grade/edit/tree/item_form.php | 14 ++-- lib/grade/grade_category.php | 30 ++++--- lib/grade/grade_item.php | 105 +++++++++++-------------- lib/grade/simpletest/testgradeitem.php | 20 ++--- lib/gradelib.php | 2 +- mod/assignment/db/events.php | 42 ---------- mod/assignment/lib.php | 52 +++++------- mod/assignment/version.php | 4 +- 8 files changed, 106 insertions(+), 163 deletions(-) delete mode 100644 mod/assignment/db/events.php diff --git a/grade/edit/tree/item_form.php b/grade/edit/tree/item_form.php index c76f6eeb00..4d1c4d0e6c 100644 --- a/grade/edit/tree/item_form.php +++ b/grade/edit/tree/item_form.php @@ -64,12 +64,14 @@ class edit_item_form extends moodleform { $mform->addElement('text', 'multfactor', get_string('multfactor', 'grades')); $mform->setHelpButton('multfactor', array(false, get_string('multfactor', 'grades'), false, true, false, get_string('multfactorhelp', 'grades'))); + $mform->setAdvanced('multfactor'); $mform->disabledIf('multfactor', 'gradetype', 'eq', GRADE_TYPE_NONE); $mform->disabledIf('multfactor', 'gradetype', 'eq', GRADE_TYPE_TEXT); $mform->addElement('text', 'plusfactor', get_string('plusfactor', 'grades')); $mform->setHelpButton('plusfactor', array(false, get_string('plusfactor', 'grades'), false, true, false, get_string('plusfactorhelp', 'grades'))); + $mform->setAdvanced('plusfactor'); $mform->disabledIf('plusfactor', 'gradetype', 'eq', GRADE_TYPE_NONE); $mform->disabledIf('plusfactor', 'gradetype', 'eq', GRADE_TYPE_TEXT); @@ -143,10 +145,13 @@ class edit_item_form extends moodleform { if ($id = $mform->getElementValue('id')) { $grade_item = grade_item::fetch(array('id'=>$id)); - if ($grade_item->is_outcome_item()) { - // we have to prevent incompatible modifications of outcomes + if (!$grade_item->is_raw_used()) { $mform->removeElement('plusfactor'); $mform->removeElement('multfactor'); + } + + if ($grade_item->is_outcome_item()) { + // we have to prevent incompatible modifications of outcomes $mform->removeElement('grademax'); $mform->removeElement('grademin'); $mform->removeElement('gradetype'); @@ -157,11 +162,6 @@ class edit_item_form extends moodleform { // following items are set up from modules and should not be overrided by user $mform->hardFreeze('itemname,idnumber,gradetype,grademax,grademin,scaleid'); //$mform->removeElement('calculation'); - - } else if ($grade_item->is_manual_item()) { - // manual grade item does not use these - uses only final grades - $mform->removeElement('plusfactor'); - $mform->removeElement('multfactor'); } } //remove the aggregation coef element if not needed diff --git a/lib/grade/grade_category.php b/lib/grade/grade_category.php index 70cc6fc9f6..a0b315dc10 100644 --- a/lib/grade/grade_category.php +++ b/lib/grade/grade_category.php @@ -513,20 +513,20 @@ class grade_category extends grade_object { $num = count($grade_values); $grades = array_values($grade_values); if ($num % 2 == 0) { - $rawgrade = ($grades[intval($num/2)-1] + $grades[intval($num/2)]) / 2; + $agg_grade = ($grades[intval($num/2)-1] + $grades[intval($num/2)]) / 2; } else { - $rawgrade = $grades[intval(($num/2)-0.5)]; + $agg_grade = $grades[intval(($num/2)-0.5)]; } break; case GRADE_AGGREGATE_MIN_ALL: case GRADE_AGGREGATE_MIN_GRADED: - $rawgrade = reset($grade_values); + $agg_grade = reset($grade_values); break; case GRADE_AGGREGATE_MAX_ALL: case GRADE_AGGREGATE_MAX_GRADED: - $rawgrade = array_pop($grade_values); + $agg_grade = array_pop($grade_values); break; case GRADE_AGGREGATE_MODE_ALL: // the most common value, average used if multimode @@ -536,7 +536,7 @@ class grade_category extends grade_object { $top = reset($freq); // highest frequency count $modes = array_keys($freq, $top); // search for all modes (have the same highest count) rsort($modes, SORT_NUMERIC); // get highes mode - $rawgrade = reset($modes); + $agg_grade = reset($modes); break; case GRADE_AGGREGATE_WEIGHTED_MEAN_GRADED: // Weighted average of all existing final grades @@ -551,9 +551,9 @@ class grade_category extends grade_object { $sum += $items[$itemid]->aggregationcoef * $grade_value; } if ($weightsum == 0) { - $rawgrade = null; + $agg_grade = null; } else { - $rawgrade = $sum / $weightsum; + $agg_grade = $sum / $weightsum; } break; @@ -570,9 +570,9 @@ class grade_category extends grade_object { } } if ($num == 0) { - $rawgrade = $sum; // only extra credits or wrong coefs + $agg_grade = $sum; // only extra credits or wrong coefs } else { - $rawgrade = $sum / $num; + $agg_grade = $sum / $num; } break; @@ -581,7 +581,7 @@ class grade_category extends grade_object { default: $num = count($grade_values); $sum = array_sum($grade_values); - $rawgrade = $sum / $num; + $agg_grade = $sum / $num; break; } @@ -589,12 +589,16 @@ class grade_category extends grade_object { $grade->rawgrademin = $this->grade_item->grademin; $grade->rawgrademax = $this->grade_item->grademax; $grade->rawscaleid = $this->grade_item->scaleid; + $grade->rawgrade = null; // categories do not use raw grades // recalculate the rawgrade back to requested range - $grade->rawgrade = grade_grade::standardise_score($rawgrade, 0, 1, $grade->rawgrademin, $grade->rawgrademax); + $finalgrade = grade_grade::standardise_score($agg_grade, 0, 1, $this->grade_item->grademin, $this->grade_item->grademax); - // calculate final grade - $grade->finalgrade = $this->grade_item->adjust_grade($grade->rawgrade, $grade->rawgrademin, $grade->rawgrademax); + if (!is_null($finalgrade)) { + $grade->finalgrade = bounded_number($this->grade_item->grademin, $finalgrade, $this->grade_item->grademax); + } else { + $grade->finalgrade = $finalgrade; + } // update in db if changed if ( $grade->finalgrade !== $oldgrade->finalgrade diff --git a/lib/grade/grade_item.php b/lib/grade/grade_item.php index 385d12e748..038f96b195 100644 --- a/lib/grade/grade_item.php +++ b/lib/grade/grade_item.php @@ -597,9 +597,14 @@ class grade_item extends grade_object { } else { return "Could not aggregate final grades for category:".$this->id; // TODO: improve and localize } + } else if ($this->is_manual_item()) { // manual items track only final grades, no raw grades return true; + + } else if (!$this->is_raw_used()) { + // hmm - raw grades are not used- nothing to regrade + return true; } // normal grade item - just new final grades @@ -619,7 +624,7 @@ class grade_item extends grade_object { continue; } - $grade->finalgrade = $this->adjust_grade($grade->rawgrade, $grade->rawgrademin, $grade->rawgrademax); + $grade->finalgrade = $this->adjust_raw_grade($grade->rawgrade, $grade->rawgrademin, $grade->rawgrademax); if ($grade_record->finalgrade !== $grade->finalgrade) { if (!$grade->update('system')) { @@ -640,7 +645,7 @@ class grade_item extends grade_object { * @param object $rawgrade The raw grade value. * @return mixed */ - function adjust_grade($rawgrade, $rawmin, $rawmax) { + function adjust_raw_grade($rawgrade, $rawmin, $rawmax) { if (is_null($rawgrade)) { return null; } @@ -667,7 +672,7 @@ class grade_item extends grade_object { return bounded_number($this->grademin, $rawgrade, $this->grademax); - } else if($this->gradetype == GRADE_TYPE_SCALE) { // Dealing with a scale value + } else if ($this->gradetype == GRADE_TYPE_SCALE) { // Dealing with a scale value if (empty($this->scale)) { $this->load_scale(); } @@ -842,6 +847,14 @@ class grade_item extends grade_object { return ($this->itemtype != 'course' and $this->itemtype != 'category' and $this->itemtype != 'manual'); } + /** + * Returns true if grade items uses raw grades + * @return boolean + */ + function is_raw_used() { + return ($this->is_normal_item() and !$this->is_calculated() and !$this->is_outcome_item()); + } + /** * Returns grade item associated with the course * @param int $courseid @@ -1209,7 +1222,7 @@ class grade_item extends grade_object { * TODO Allow for a change of feedback without a change of finalgrade. Currently I get notice about uninitialised $result */ function update_final_grade($userid, $finalgrade=false, $source=NULL, $note=NULL, $feedback=false, $feedbackformat=FORMAT_MOODLE, $usermodified=null) { - global $USER; + global $USER, $CFG; if (empty($usermodified)) { $usermodified = $USER->id; } @@ -1244,27 +1257,42 @@ class grade_item extends grade_object { $oldgrade->rawscaleid = $grade->rawscaleid; $oldgrade->overridden = $grade->overridden; - if ($finalgrade !== false) { - if (!is_null($finalgrade)) { - $grade->finalgrade = bounded_number($this->grademin, $finalgrade, $this->grademax); + $override = false; + $functionname = null; + + if ($finalgrade !== false or $feedback !== false) { + if (($this->is_outcome_item() or $this->is_manual_item()) and !$this->is_calculated()) { + // final grades updated only by user - no need for overriding + + } else if ($this->itemtype == 'mod' and $this->plusfactor == 0 and $this->multfactor == 1) { + // do not override grade if module can update own raw grade + require_once($CFG->dirroot.'/mod/'.$this->itemmodule.'/lib.php'); + $functionname = $this->itemmodule.'_grade_updated'; + if (!function_exists($functionname)) { + $override = true; + $functionname = null; + } + } else { - $grade->finalgrade = $finalgrade; + $override = true; } + } - if ($this->is_manual_item() and !$this->is_calculated()) { - // no overriding on manual grades - raw not used - - } else if ($this->is_outcome_item() and !$this->is_calculated()) { - // no updates of raw grades for outcomes - raw grades not used + if ($finalgrade !== false) { + if (!is_null($finalgrade)) { + $finalgrade = bounded_number($this->grademin, $finalgrade, $this->grademax); + } else { + $finalgrade = $finalgrade; + } + $grade->finalgrade = $finalgrade; - } else if (!$this->is_normal_item() or $this->plusfactor != 0 or $this->multfactor != 1 - or !events_is_registered('grade_updated', $this->itemtype.'/'.$this->itemmodule)) { - // we can not update the raw grade - flag it as overridden + if ($override) { if (!$grade->overridden) { $grade->overridden = time(); } - } else { + } else if ($this->is_raw_used()) { + // module which is not overridden - no factors used $grade->rawgrade = $finalgrade; // copy current grademin/max and scale $grade->rawgrademin = $this->grademin; @@ -1310,9 +1338,9 @@ class grade_item extends grade_object { } } - // no events for overridden items and outcomes - if ($result and !$grade->overridden and $this->itemnumber < 1000) { - $this->trigger_raw_updated($grade, $source); + // inform modules, etc. if needed + if ($result and $functionname) { + $functionname($this->iteminstance, $this->itemnumber, $userid, $finalgrade, $feedback, $feedbackformat, $usermodified); } return $result; @@ -1409,46 +1437,9 @@ class grade_item extends grade_object { } } - // no events for outcomes - if ($result and $this->itemnumber < 1000) { - $this->trigger_raw_updated($grade, $source); - } - return $result; } - /** - * Internal function used by update_final/raw_grade() only. - */ - function trigger_raw_updated($grade, $source) { - global $CFG; - require_once($CFG->libdir.'/eventslib.php'); - - // trigger grade_updated event notification - $eventdata = new object(); - - $eventdata->source = $source; - $eventdata->itemid = $this->id; - $eventdata->courseid = $this->courseid; - $eventdata->itemtype = $this->itemtype; - $eventdata->itemmodule = $this->itemmodule; - $eventdata->iteminstance = $this->iteminstance; - $eventdata->itemnumber = $this->itemnumber; - $eventdata->idnumber = $this->idnumber; - $eventdata->userid = $grade->userid; - $eventdata->rawgrade = $grade->rawgrade; - - // load existing text annotation - if ($grade_text = $grade->load_text()) { - $eventdata->feedback = $grade_text->feedback; - $eventdata->feedbackformat = $grade_text->feedbackformat; - $eventdata->information = $grade_text->information; - $eventdata->informationformat = $grade_text->informationformat; - } - - events_trigger('grade_updated', $eventdata); - } - /** * Calculates final grade values using the formula in calculation property. * The parameters are taken from final grades of grade items in current course only. diff --git a/lib/grade/simpletest/testgradeitem.php b/lib/grade/simpletest/testgradeitem.php index bef65f7013..97c9dccab9 100755 --- a/lib/grade/simpletest/testgradeitem.php +++ b/lib/grade/simpletest/testgradeitem.php @@ -295,11 +295,11 @@ class grade_item_test extends grade_test { } /** - * Test the adjust_grade method + * Test the adjust_raw_grade method */ - function test_grade_item_adjust_grade() { + function test_grade_item_adjust_raw_grade() { $grade_item = new grade_item($this->grade_items[0]); - $this->assertTrue(method_exists($grade_item, 'adjust_grade')); + $this->assertTrue(method_exists($grade_item, 'adjust_raw_grade')); $grade_raw = new stdClass(); $grade_raw->rawgrade = 40; @@ -314,17 +314,17 @@ class grade_item_test extends grade_test { $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_raw_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_raw_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_raw_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; @@ -332,13 +332,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_raw_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_raw_grade($grade_raw->rawgrade, $grade_raw->grademin, $grade_raw->grademax)); // Try multfactor and plusfactor $grade_raw = clone($original_grade_raw); @@ -346,7 +346,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_raw_grade($grade_raw->rawgrade, $grade_raw->grademin, $grade_raw->grademax)); // Try multfactor below 0 and a negative plusfactor $grade_raw = clone($original_grade_raw); @@ -354,7 +354,7 @@ 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_raw_grade($grade_raw->rawgrade, $grade_raw->grademin, $grade_raw->grademax))); } /** diff --git a/lib/gradelib.php b/lib/gradelib.php index 2dc76a7085..d3acbcb92c 100644 --- a/lib/gradelib.php +++ b/lib/gradelib.php @@ -95,7 +95,7 @@ require_once($CFG->libdir . '/grade/grade_grade_text.php'); * Only following grade item properties can be changed 'itemname', 'idnumber', 'gradetype', 'grademax', * 'grademin', 'scaleid', 'multfactor', 'plusfactor', 'deleted'. * - * @param string $source source of the grade such as 'mod/assignment', often used to prevent infinite loops when processing grade_updated events + * @param string $source source of the grade such as 'mod/assignment' * @param int $courseid id of course * @param string $itemtype type of grade item - mod, block * @param string $itemmodule more specific then $itemtype - assignment, forum, etc.; maybe NULL for some item types diff --git a/mod/assignment/db/events.php b/mod/assignment/db/events.php deleted file mode 100644 index 2fdb74ea94..0000000000 --- a/mod/assignment/db/events.php +++ /dev/null @@ -1,42 +0,0 @@ - array ( - 'handlerfile' => '/mod/assignment/lib.php', - 'handlerfunction' => 'assignment_grade_handler', - 'schedule' => 'instant' - ) -); - -?> diff --git a/mod/assignment/lib.php b/mod/assignment/lib.php index 71889395ae..8fb913f5a9 100644 --- a/mod/assignment/lib.php +++ b/mod/assignment/lib.php @@ -2125,23 +2125,13 @@ function assignment_grade_item_delete($assignment) { } /** - * Something wants to change the grade from outside using "grade_updated" event. + * Gradebook informs this module about new grade or feedback. * */ -function assignment_grade_handler($eventdata) { +function assignment_grade_updated($instance, $itemnumber, $userid, $gradevalue, $feedback, $feedbackformat, $usermodified) { global $CFG, $USER; - if ($eventdata->source == 'mod/assignment') { - // event from assignment itself - return true; - } - - if ($eventdata->itemtype != 'mod' or $eventdata->itemmodule != 'assignment') { - //not for us - ignore it - return true; - } - - if (!$assignment = get_record('assignment', 'id', $eventdata->iteminstance)) { + if (!$assignment = get_record('assignment', 'id', $instance)) { return true; } if (! $course = get_record('course', 'id', $assignment->course)) { @@ -2156,36 +2146,36 @@ function assignment_grade_handler($eventdata) { $assignmentclass = 'assignment_'.$assignment->assignmenttype; $assignmentinstance = new $assignmentclass($cm->id, $assignment, $cm, $course); - $old = $assignmentinstance->get_submission($eventdata->userid, true); // Get or make one + $old = $assignmentinstance->get_submission($userid, true); // Get or make one $submission = new object(); $submission->id = $old->id; $submission->userid = $old->userid; - $submission->teacher = $USER->id; - $submission->timemarked = time(); + $submission->teacher = $usermodified; - if (is_null($eventdata->rawgrade)) { + if ($gradevalue === false) { + $submission->grade = $old->grade; + } else if (is_null($gradevalue)) { $submission->grade = -1; } else { - $submission->grade = (int)$eventdata->rawgrade; // round it for now - if ($old->grade != $submission->grade) { - $submission->mailed = 0; // Make sure mail goes out (again, even) - } + $submission->grade = (int)$gradevalue; // round it for now + $submission->timemarked = time(); } - if (isset($eventdata->feedback)) { - $submission->submissioncomment = addslashes($eventdata->feedback); + if ($feedback === false) { + $submission->submissioncomment = addslashes($old->submissioncomment); + $submission->format = $old->format; + } else { + $submission->submissioncomment = addslashes($feedback); + $submission->format = (int)$feedbackformat; } - if (isset($eventdata->feedbackformat)) { - $submission->format = (int)$eventdata->feedbackformat; - } + if ($old->submissioncomment != $submission->submissioncomment or $old->grade != $submission->grade) { - if (isset($eventdata->feedback) && ($old->submissioncomment != $eventdata->feedback or $old->format != $submission->format)) { - $submission->mailed = 0; // Make sure mail goes out (again, even) - } + $submission->mailed = 0; // Make sure mail goes out (again, even) - if (!update_record('assignment_submissions', $submission)) { - //return false; + if (!update_record('assignment_submissions', $submission)) { + return false; + } } // TODO: add proper logging diff --git a/mod/assignment/version.php b/mod/assignment/version.php index 60d7cfdfcb..0ac76a128f 100644 --- a/mod/assignment/version.php +++ b/mod/assignment/version.php @@ -5,8 +5,8 @@ // This fragment is called by /admin/index.php //////////////////////////////////////////////////////////////////////////////// -$module->version = 2007072200; -$module->requires = 2007072200; // Requires this Moodle version +$module->version = 2007090100; +$module->requires = 2007083101; // Requires this Moodle version $module->cron = 60; ?> -- 2.39.5