From e7c71c189bb906741951e69c8c178191bf81bf3c Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Thu, 13 Aug 2015 12:16:21 +0800 Subject: [PATCH] MDL-48634 grades: Make the rescaling option required if maxgrade changes --- course/modlib.php | 4 ++-- grade/edit/tree/item.php | 2 +- grade/edit/tree/item_form.php | 14 +++++++++++++- lang/en/grades.php | 1 + lib/form/modgrade.php | 30 ++++++++++++++++++++++++++++-- 5 files changed, 45 insertions(+), 6 deletions(-) diff --git a/course/modlib.php b/course/modlib.php index 93452c12774..b878ca10250 100644 --- a/course/modlib.php +++ b/course/modlib.php @@ -531,7 +531,7 @@ function update_moduleinfo($cm, $moduleinfo, $course, $mform = null) { // Get the a copy of the grade_item before it is modified incase we need to scale the grades. $oldgradeitem = null; $newgradeitem = null; - if (!empty($data->grade_rescalegrades)) { + if (!empty($data->grade_rescalegrades) && $data->grade_rescalegrades == 'yes') { // Fetch the grade item before it is updated. $oldgradeitem = grade_item::fetch(array('itemtype' => 'mod', 'itemmodule' => $moduleinfo->modulename, @@ -546,7 +546,7 @@ function update_moduleinfo($cm, $moduleinfo, $course, $mform = null) { } // This needs to happen AFTER the grademin/grademax have already been updated. - if (!empty($data->grade_rescalegrades)) { + if (!empty($data->grade_rescalegrades) && $data->grade_rescalegrades == 'yes') { // Get the grade_item after the update call the activity to scale the grades. $newgradeitem = grade_item::fetch(array('itemtype' => 'mod', 'itemmodule' => $moduleinfo->modulename, diff --git a/grade/edit/tree/item.php b/grade/edit/tree/item.php index 920cab6cc7e..7ee5cbebd31 100644 --- a/grade/edit/tree/item.php +++ b/grade/edit/tree/item.php @@ -178,7 +178,7 @@ if ($mform->is_cancelled()) { } else { $grade_item->update(); - if (!empty($data->rescalegrades)) { + if (!empty($data->rescalegrades) && $data->rescalegrades == 'yes') { $newmin = $grade_item->grademin; $newmax = $grade_item->grademax; $grade_item->rescale_grades_keep_percentage($oldmin, $oldmax, $newmin, $newmax, 'gradebook'); diff --git a/grade/edit/tree/item_form.php b/grade/edit/tree/item_form.php index f07d9401296..c17480e98f2 100644 --- a/grade/edit/tree/item_form.php +++ b/grade/edit/tree/item_form.php @@ -97,7 +97,11 @@ class edit_item_form extends moodleform { $mform->setType('grademin', PARAM_RAW); } - $mform->addElement('selectyesno', 'rescalegrades', get_string('modgraderescalegrades', 'grades')); + $choices = array(); + $choices[''] = get_string('choose'); + $choices['no'] = get_string('no'); + $choices['yes'] = get_string('yes'); + $mform->addElement('select', 'rescalegrades', get_string('modgraderescalegrades', 'grades'), $choices); $mform->addHelpButton('rescalegrades', 'modgraderescalegrades', 'grades'); $mform->disabledIf('rescalegrades', 'gradetype', 'noteq', GRADE_TYPE_VALUE); @@ -394,6 +398,14 @@ class edit_item_form extends moodleform { $errors['grademax'] = get_string('incorrectminmax', 'grades'); } } + if ($grade_item) { + if (grade_floats_different($data['grademin'], $grade_item->grademin) || + grade_floats_different($data['grademax'], $grade_item->grademax)) { + if ($grade_item->has_grades() && empty($data['rescalegrades'])) { + $errors['rescalegrades'] = get_string('mustchooserescaleyesorno', 'grades'); + } + } + } return $errors; } diff --git a/lang/en/grades.php b/lang/en/grades.php index 4689fcd0a30..4eb6b3b2fe6 100644 --- a/lang/en/grades.php +++ b/lang/en/grades.php @@ -501,6 +501,7 @@ $string['mygrades'] = 'User menu grades link'; $string['mygrades_desc'] = 'This setting allows for the option of linking to an external gradebook from the user menu.'; $string['mypreferences'] = 'My preferences'; $string['myreportpreferences'] = 'My report preferences'; +$string['mustchooserescaleyesorno'] = 'You must choose whether to rescale existing grades or not.'; $string['navmethod'] = 'Navigation method'; $string['neverdeletehistory'] = 'Never delete history'; $string['newcategory'] = 'New category'; diff --git a/lib/form/modgrade.php b/lib/form/modgrade.php index fa608a8e3d0..d7145e9166a 100644 --- a/lib/form/modgrade.php +++ b/lib/form/modgrade.php @@ -66,6 +66,7 @@ class MoodleQuickForm_modgrade extends MoodleQuickForm_group { * 'isupdate' - is this a new module or are we editing an existing one? * 'currentgrade' - the current grademax in the database for this gradeitem * 'hasgrades' - whether or not the grade_item has existing grade_grades + * 'canrescale' - whether or not the activity supports rescaling grades * @param mixed $attributes Either a typical HTML attribute string or an associative array */ public function __construct($elementname = null, $elementlabel = null, $options = array(), $attributes = null) { @@ -138,9 +139,14 @@ class MoodleQuickForm_modgrade extends MoodleQuickForm_group { // Check box for options for processing existing grades. if ($this->isupdate && $this->hasgrades && $this->canrescale) { $langrescalegrades = get_string('modgraderescalegrades', 'grades'); - $rescalegradesselect = @MoodleQuickForm::createElement('selectyesno', + $choices = array(); + $choices[''] = get_string('choose'); + $choices['no'] = get_string('no'); + $choices['yes'] = get_string('yes'); + $rescalegradesselect = @MoodleQuickForm::createElement('select', 'modgrade_rescalegrades', - $langrescalegrades); + $langrescalegrades, + $choices); $rescalegradesselect->_generateId(); $rescalegradesid = $rescalegradesselect->getAttribute('id'); } @@ -300,13 +306,33 @@ class MoodleQuickForm_modgrade extends MoodleQuickForm_group { return true; }; + $checkrescale = function($val) { + // Nothing is affected by changes to grademax if there are no grades yet. + if (!$this->isupdate || !$this->hasgrades || !$this->canrescale) { + return true; + } + // Closure to validate a scale value. See the note above about scope if this confuses you. + if (isset($val['modgrade_type']) && $val['modgrade_type'] === 'point') { + // Work out if the value was actually changed in the form. + if (grade_floats_different($this->currentgrade, $val['modgrade_point'])) { + if (empty($val['modgrade_rescalegrades'])) { + // This was an "edit", the grademax was changed and the process existing setting was not set. + return false; + } + } + } + return true; + }; + $maxgradeexceeded = get_string('modgradeerrorbadpoint', 'grades', get_config('core', 'gradepointmax')); $invalidscale = get_string('modgradeerrorbadscale', 'grades'); + $mustchooserescale = get_string('mustchooserescaleyesorno', 'grades'); // When creating the rules the sixth arg is $force, we set it to true because otherwise the form // will attempt to validate the existence of the element, we don't want this because the element // is being created right now and doesn't actually exist as a registered element yet. $caller->addRule($name, $maxgradeexceeded, 'callback', $checkmaxgrade, 'server', false, true); $caller->addRule($name, $invalidscale, 'callback', $checkvalidscale, 'server', false, true); + $caller->addRule($name, $mustchooserescale, 'callback', $checkrescale, 'server', false, true); break;