From: nicolasconnault Date: Fri, 28 Sep 2007 07:55:51 +0000 (+0000) Subject: MDL-11480 Reviewed, tested, and pplied Petr's patch. X-Git-Url: http://git.mjollnir.org/gw?a=commitdiff_plain;h=e9096dc2cac4a7989bdecf8ddca5aff73354389f;p=moodle.git MDL-11480 Reviewed, tested, and pplied Petr's patch. --- diff --git a/grade/export/lib.php b/grade/export/lib.php index 4e0538e264..e8d9995f49 100755 --- a/grade/export/lib.php +++ b/grade/export/lib.php @@ -40,13 +40,10 @@ class grade_export { var $columns; // array of grade_items selected for export var $previewrows; // number of rows in preview - var $export_letters; // export letters - TODO: finish implementation + var $export_letters; // export letters var $export_feedback; // export feedback var $userkey; // export using private user key - var $letters; // internal - var $report; // internal - /** * Constructor should set up all the private variables ready to be pulled * @param object $course @@ -143,43 +140,18 @@ class grade_export { } } - /** - * internal - */ - function _init_letters() { - global $CFG; - - if (!isset($this->letters)) { - if ($this->export_letters) { - require_once($CFG->dirroot . '/grade/report/lib.php'); - $this->report = new grade_report($this->course->id, null, null); - $this->letters = $this->report->get_grade_letters(); - } else { - $this->letters = false; // false prevents another fetching of grade letters - } - } - } - /** * Returns string representation of final grade * @param $object $grade instance of grade_grade class * @return string */ function format_grade($grade) { - $this->_init_letters(); - - //TODO: rewrite the letters handling code - this is slow - if ($this->letters) { - $grade_item = $this->grade_items[$grade->itemid]; - $grade_item_displaytype = $this->report->get_pref('gradedisplaytype', $grade_item->id); - - if ($grade_item_displaytype == GRADE_DISPLAY_TYPE_LETTER) { - return grade_grade::get_letter($this->letters, $grade->finalgrade, $grade_item->grademin, $grade_item->grademax); - } + $displaytype = null; + if ($this->export_letters) { + $displaytype = GRADE_DISPLAY_TYPE_LETTER; } - //TODO: format it somehow - scale/letter/number/etc. - return $grade->finalgrade; + return grade_format_gradevalue($grade->finalgrade, $this->grade_items[$grade->itemid], false, $displaytype, null); } /** @@ -299,7 +271,7 @@ class grade_export { echo '
'; if (!$this->userkey) { // this button should trigger a download prompt - print_single_button($CFG->wwwroot.'/grade/export/'.$this->plugin.'/export.php', + print_single_button($CFG->wwwroot.'/grade/export/'.$this->plugin.'/export.php', $params, get_string('download', 'admin')); } else { diff --git a/grade/report/grader/lib.php b/grade/report/grader/lib.php index 078b211608..98fd2dae7d 100644 --- a/grade/report/grader/lib.php +++ b/grade/report/grader/lib.php @@ -757,9 +757,8 @@ class grade_report_grader extends grade_report { $studentshtml .= ''.get_string('error').''; } else if ($gradedisplaytype == GRADE_DISPLAY_TYPE_LETTER) { - $letters = grade_report::get_grade_letters(); if (!is_null($gradeval)) { - $studentshtml .= grade_grade::get_letter($letters, $gradeval, $grademin, $grademax); + $studentshtml .= grade_format_gradevalue($gradeval, $item, false, GRADE_DISPLAY_TYPE_LETTER, null); } } else if ($item->scaleid && !empty($scales_array[$item->scaleid]) && $gradedisplaytype == GRADE_DISPLAY_TYPE_REAL) { @@ -946,12 +945,8 @@ class grade_report_grader extends grade_report { $gradehtml = $gradeval; } - if ($displaytype == GRADE_DISPLAY_TYPE_PERCENTAGE) { - $gradeval = grade_to_percentage($rawgradeval, $item->grademin, $item->grademax); - $gradehtml = format_float($gradeval, $decimalpoints). '%'; - } elseif ($displaytype == GRADE_DISPLAY_TYPE_LETTER) { - $letters = grade_report::get_grade_letters(); - $gradehtml = grade_grade::get_letter($letters, $rawgradeval, $item->grademin, $item->grademax); + if ($displaytype == GRADE_DISPLAY_TYPE_PERCENTAGE or $displaytype == GRADE_DISPLAY_TYPE_LETTER) { + $gradeshtml = grade_format_gradevalue($rawgradeval, $item, true, $displaytype, null); } $numberofgrades = ''; diff --git a/lib/grade/constants.php b/lib/grade/constants.php index 5569476e67..3ca326b414 100644 --- a/lib/grade/constants.php +++ b/lib/grade/constants.php @@ -59,16 +59,18 @@ define('GRADE_HISTORY_INSERT', 1); define('GRADE_HISTORY_UPDATE', 2); define('GRADE_HISTORY_DELETE', 3); -define('GRADE_REPORT_AGGREGATION_POSITION_LEFT', 0); -define('GRADE_REPORT_AGGREGATION_POSITION_RIGHT', 1); -define('GRADE_REPORT_AGGREGATION_VIEW_FULL', 0); -define('GRADE_REPORT_AGGREGATION_VIEW_AGGREGATES_ONLY', 1); -define('GRADE_REPORT_AGGREGATION_VIEW_GRADES_ONLY', 2); +// Display style constants define('GRADE_DISPLAY_TYPE_DEFAULT', 0); define('GRADE_DISPLAY_TYPE_REAL', 1); define('GRADE_DISPLAY_TYPE_PERCENTAGE', 2); define('GRADE_DISPLAY_TYPE_LETTER', 3); define('GRADE_DECIMALS_DEFAULT', null); + +define('GRADE_REPORT_AGGREGATION_POSITION_LEFT', 0); +define('GRADE_REPORT_AGGREGATION_POSITION_RIGHT', 1); +define('GRADE_REPORT_AGGREGATION_VIEW_FULL', 0); +define('GRADE_REPORT_AGGREGATION_VIEW_AGGREGATES_ONLY', 1); +define('GRADE_REPORT_AGGREGATION_VIEW_GRADES_ONLY', 2); define('GRADE_REPORT_PREFERENCE_DEFAULT', 'default'); define('GRADE_REPORT_PREFERENCE_INHERIT', 'inherit'); define('GRADE_REPORT_PREFERENCE_UNUSED', -1); diff --git a/lib/grade/grade_grade.php b/lib/grade/grade_grade.php index 36d4437732..9f1e4cc21a 100644 --- a/lib/grade/grade_grade.php +++ b/lib/grade/grade_grade.php @@ -473,45 +473,5 @@ class grade_grade extends grade_object { $standardised_value = $factor * $diff + $target_min; return $standardised_value; } - - /** - * Returns the grade letter this grade falls under, as they are set up in the given array. - * @param array $letters An array of grade boundaries with associated letters - * @param float $gradevalue The value to convert. If not given, will use instantiated object - * @param float $grademin If not given, will look up the grade_item's grademin - * @param float $grademax If not given, will look up the grade_item's grademax - * @return string Grade letter - */ - function get_letter($letters, $gradevalue=null, $grademin=null, $grademax=null) { - if (is_null($grademin) || is_null($grademax)) { - if (!isset($this)) { - debugging("Tried to call grade_grade::get_letter statically without giving an explicit grademin or grademax!"); - return false; - } - $this->load_grade_item(); - $grademin = $this->grade_item->grademin; - $grademax = $this->grade_item->grademax; - } - - if (is_null($gradevalue)) { - if (!isset($this)) { - debugging("Tried to call grade_grade::get_letter statically without giving an explicit gradevalue!"); - return false; - } - $gradevalue = $this->finalgrade; - } - // Standardise grade first - $grade = grade_grade::standardise_score($gradevalue, $grademin, $grademax, 0, 100); - - // Sort the letters by descending boundaries (100-0) - krsort($letters); - foreach ($letters as $boundary => $letter) { - if ($grade >= $boundary) { - return $letter; - } - } - return '-'; - } - } ?> diff --git a/lib/grade/grade_item.php b/lib/grade/grade_item.php index febffe8ca8..26b20e908e 100644 --- a/lib/grade/grade_item.php +++ b/lib/grade/grade_item.php @@ -1693,17 +1693,25 @@ class grade_item extends grade_object { */ function get_displaytype() { global $CFG; - $course_gradedisplaytype = get_field('grade_items', 'display', 'courseid', $this->courseid, 'itemtype', 'course'); - $site_gradedisplaytype = $CFG->grade_report_gradedisplaytype; - $default_gradedisplaytype = $this->display; + static $cache = array(); if ($this->display == GRADE_DISPLAY_TYPE_DEFAULT) { - $default_gradedisplaytype = $course_gradedisplaytype; - if ($course_gradedisplaytype == GRADE_DISPLAY_TYPE_DEFAULT) { - $default_gradedisplaytype = $site_gradedisplaytype; + if (array_key_exists($this->courseid, $cache)) { + return $cache[$this->courseid]; + } else if (count($cache) > 100) { + $cache = array(); // cache size limit } + + $gradedisplaytype = get_field('grade_items', 'display', 'courseid', $this->courseid, 'itemtype', 'course'); + if ($gradedisplaytype == GRADE_DISPLAY_TYPE_DEFAULT) { + $gradedisplaytype = $CFG->grade_report_gradedisplaytype; + } + $cache[$this->courseid] = $gradedisplaytype; + return $gradedisplaytype; + + } else { + return $this->display; } - return $default_gradedisplaytype; } /** @@ -1712,17 +1720,24 @@ class grade_item extends grade_object { */ function get_decimals() { global $CFG; - $course_gradedecimals = get_field('grade_items', 'decimals', 'courseid', $this->courseid, 'itemtype', 'course'); - $site_gradedecimals = $CFG->grade_report_decimalpoints; - $item_gradedecimals = $this->decimals; + static $cache = array(); if ($this->decimals == GRADE_DECIMALS_DEFAULT) { - $item_gradedecimals = $course_gradedecimals; - if ($course_gradedecimals == GRADE_DECIMALS_DEFAULT) { - $item_gradedecimals = $site_gradedecimals; + if (array_key_exists($this->courseid, $cache)) { + return $cache[$this->courseid]; + } else if (count($cache) > 100) { + $cache = array(); // cache size limit } + $gradedecimals = get_field('grade_items', 'decimals', 'courseid', $this->courseid, 'itemtype', 'course'); + if ($gradedecimals == GRADE_DECIMALS_DEFAULT) { + $gradedecimals = $CFG->grade_report_decimalpoints; + } + $cache[$this->courseid] = $gradedecimals; + return $gradedecimals; + + } else { + return $this->decimals; } - return $item_gradedecimals; } } ?> diff --git a/lib/gradelib.php b/lib/gradelib.php index f50a9d223f..055fb93a99 100644 --- a/lib/gradelib.php +++ b/lib/gradelib.php @@ -334,25 +334,7 @@ function grade_get_grades($courseid, $itemtype, $itemmodule, $iteminstance, $use $grade->str_grade = '-'; } else { - switch ($grade_item->gradetype) { - case GRADE_TYPE_VALUE: - if (!isset($decimalpoints)) { - require_once($CFG->dirroot.'/grade/report/user/lib.php');//TODO: which setting to use? - $decimalpoints = grade_report_user::get_pref('decimalpoints', $grade_item->id); - } - $grade->str_grade = format_float($grade->grade, $decimalpoints); - break; - - case GRADE_TYPE_SCALE: - $scale = $grade_item->load_scale(); - $grade->grade = (int)bounded_number($item->grademin, $grade->grade, $item->grademax); - $grade->str_grade = format_string($scale->scale_items[$grade->grade-1]); - break; - - case GRADE_TYPE_TEXT: - default: - $grade->str_grade = ''; - } + $grade->str_grade = grade_format_gradevalue($grade->grade, $grade_item); } // create html representation of feedback @@ -442,6 +424,138 @@ function grade_get_grades($courseid, $itemtype, $itemmodule, $iteminstance, $use /***** END OF PUBLIC API *****/ +/** + * Returns string representation of grade value + * @param float $value grade value + * @param object $grade_item - by reference to prevent scale reloading + * @param bool $localized use localised decimal separator + * @param int $display type of display - raw, letter, percentage + * @param int $decimalplaces number of decimal places when displaying float values + * @return string + */ +function grade_format_gradevalue($value, &$grade_item, $localized=true, $displaytype=null, $decimals=null) { + if ($grade_item->gradetype == GRADE_TYPE_NONE or $grade_item->gradetype == GRADE_TYPE_TEXT) { + return ''; + } + + // no grade yet? + if (is_null($value)) { + return '-'; + } + + if ($grade_item->gradetype != GRADE_TYPE_VALUE and $grade_item->gradetype != GRADE_TYPE_SCALE) { + //unknown type?? + return ''; + } + + if (is_null($displaytype)) { + $displaytype = $grade_item->get_displaytype(); + } + + if (is_null($decimals)) { + $decimals = $grade_item->get_decimals(); + } + + switch ($displaytype) { + case GRADE_DISPLAY_TYPE_REAL: + if ($grade_item->gradetype == GRADE_TYPE_SCALE) { + $scale = $grade_item->load_scale(); + $value = (int)bounded_number($grade_item->grademin, $value, $grade_item->grademax); + return format_string($scale->scale_items[$value-1]); + + } else { + return format_float($value, $decimals, $localized); + } + + case GRADE_DISPLAY_TYPE_PERCENTAGE: + $min = $grade_item->grademin; + $max = $grade_item->grademax; + if ($min == $max) { + return ''; + } + $value = bounded_number($min, $value, $max); + $percentage = (($value-$min)*100)/($max-$min); + return format_float($percentage, $decimals, $localized).' %'; + + case GRADE_DISPLAY_TYPE_LETTER: + $context = get_context_instance(CONTEXT_COURSE, $grade_item->courseid); + if (!$letters = grade_get_letters($context)) { + return ''; // no letters?? + } + + $value = grade_grade::standardise_score($value, $grade_item->grademin, $grade_item->grademax, 0, 100); + $value = bounded_number(0, $value, 100); // just in case + foreach ($letters as $boundary => $letter) { + if ($value >= $boundary) { + return format_string($letter); + } + } + return '-'; // no match? maybe '' would be more correct + + default: + return ''; + } +} + +/** + * Returns grade letters array used in context + * @param object $context object or null for defaults + * @return array of grade_boundary=>letter_string + */ +function grade_get_letters($context=null) { + if (empty($context)) { + // defaults + // TODO: maybe we should hardcode defaults here and remove them from admin tree + // it seems a bit less than optional to use report preferences for this + // when letters are used in other types of plugins too + global $CFG; + require_once($CFG->dirroot.'/grade/report/lib.php'); + + for ($i = 1; $i <= 10; $i++) { + $boundary = grade_report::get_pref('gradeboundary' . $i); + $letter = grade_report::get_pref('gradeletter' . $i); + if (!is_null($boundary) && $boundary != -1 && !empty($letter)) { + $letters[$boundary] = $letter; + } + } + return $letters; + } + + static $cache = array(); + + if (array_key_exists($context->id, $cache)) { + return $cache[$context->id]; + } + + if (count($cache) > 100) { + $cache = array(); // cache size limit + } + + $letters = array(); + + $contexts = get_parent_contexts($context); + array_unshift($contexts, $context->id); + + foreach ($contexts as $ctxid) { + if ($records = get_records('grade_letters', 'contextid', $ctxid, 'lowerboundary DESC')) { //TODO: add index? + foreach ($records as $record) { + if (!is_null($record->lowerboundary) && !empty($record->letter)) { + $letters[$record->lowerboundary] = $record->letter; + } + } + } + + if (!empty($letters)) { + $cache[$context->id] = $letters; + return $letters; + } + } + + $letters = grade_get_letters(null); + $cache[$context->id] = $letters; + return $letters; +} + /** * Verify new value of idnumber - checks for uniqueness of new idnumbers, old are kept intact