From fcc88fddba6b256d4c24f3333fe3bec13e6c5798 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 20 Aug 2019 14:44:41 +0800 Subject: [PATCH] MDL-66079 core_grades: Add support for multiple grade items in an activity Part of MDL-66074 --- course/modlib.php | 172 +++-- course/moodleform_mod.php | 408 ++++++----- course/tests/externallib_test.php | 2 +- grade/classes/component_gradeitems.php | 219 ++++++ .../gradeitem/advancedgrading_mapping.php | 43 ++ .../local/gradeitem/itemnumber_mapping.php | 43 ++ grade/grading/lib.php | 21 +- grade/tests/behat/grade_to_pass.feature | 2 +- grade/tests/component_gradeitems_test.php | 653 ++++++++++++++++++ grade/tests/coverage.php | 39 ++ mod/upgrade.txt | 3 + 11 files changed, 1361 insertions(+), 244 deletions(-) create mode 100644 grade/classes/component_gradeitems.php create mode 100644 grade/classes/local/gradeitem/advancedgrading_mapping.php create mode 100644 grade/classes/local/gradeitem/itemnumber_mapping.php create mode 100644 grade/tests/component_gradeitems_test.php create mode 100644 grade/tests/coverage.php diff --git a/course/modlib.php b/course/modlib.php index caf3868bbe1..181586dd385 100644 --- a/course/modlib.php +++ b/course/modlib.php @@ -27,6 +27,8 @@ defined('MOODLE_INTERNAL') || die; +use \core_grades\component_gradeitems; + require_once($CFG->dirroot.'/course/lib.php'); /** @@ -213,49 +215,63 @@ function edit_module_post_actions($moduleinfo, $course) { $hasgrades = plugin_supports('mod', $moduleinfo->modulename, FEATURE_GRADE_HAS_GRADE, false); $hasoutcomes = plugin_supports('mod', $moduleinfo->modulename, FEATURE_GRADE_OUTCOMES, true); - // Sync idnumber with grade_item. - if ($hasgrades && $grade_item = grade_item::fetch(array('itemtype'=>'mod', 'itemmodule'=>$moduleinfo->modulename, - 'iteminstance'=>$moduleinfo->instance, 'itemnumber'=>0, 'courseid'=>$course->id))) { - $gradeupdate = false; - if ($grade_item->idnumber != $moduleinfo->cmidnumber) { - $grade_item->idnumber = $moduleinfo->cmidnumber; - $gradeupdate = true; - } - if (isset($moduleinfo->gradepass) && $grade_item->gradepass != $moduleinfo->gradepass) { - $grade_item->gradepass = $moduleinfo->gradepass; - $gradeupdate = true; - } - if ($gradeupdate) { - $grade_item->update(); - } - } - - if ($hasgrades) { - $items = grade_item::fetch_all(array('itemtype'=>'mod', 'itemmodule'=>$moduleinfo->modulename, - 'iteminstance'=>$moduleinfo->instance, 'courseid'=>$course->id)); - } else { - $items = array(); - } + $items = grade_item::fetch_all([ + 'itemtype' => 'mod', + 'itemmodule' => $moduleinfo->modulename, + 'iteminstance' => $moduleinfo->instance, + 'courseid' => $course->id, + ]); // Create parent category if requested and move to correct parent category. - if ($items and isset($moduleinfo->gradecat)) { - if ($moduleinfo->gradecat == -1) { - $grade_category = new grade_category(); - $grade_category->courseid = $course->id; - $grade_category->fullname = $moduleinfo->name; - $grade_category->insert(); - if ($grade_item) { - $parent = $grade_item->get_parent_category(); - $grade_category->set_parent($parent->id); + $component = "mod_{$moduleinfo->modulename}"; + if ($items) { + foreach ($items as $item) { + $update = false; + + // Sync idnumber with grade_item. + // Note: This only happens for itemnumber 0 at this time. + if ($item->itemnumber == 0 && ($item->idnumber != $moduleinfo->cmidnumber)) { + $item->idnumber = $moduleinfo->cmidnumber; + $update = true; } - $moduleinfo->gradecat = $grade_category->id; - } - foreach ($items as $itemid=>$unused) { - $items[$itemid]->set_parent($moduleinfo->gradecat); - if ($itemid == $grade_item->id) { - // Use updated grade_item. - $grade_item = $items[$itemid]; + // Determine the grade category. + $gradecatfieldname = component_gradeitems::get_field_name_for_itemnumber($component, $item->itemnumber, 'gradecat'); + if (property_exists($moduleinfo, $gradecatfieldname)) { + $gradecat = $moduleinfo->$gradecatfieldname; + if ($gradecat == -1) { + $gradecategory = new grade_category(); + $gradecategory->courseid = $course->id; + $gradecategory->fullname = $moduleinfo->name; + $gradecategory->insert(); + + $parent = $item->get_parent_category(); + $gradecategory->set_parent($parent->id); + $gradecat = $gradecategory->id; + } + + $oldgradecat = null; + if ($parent = $item->get_parent_category()) { + $oldgradecat = $parent->id; + } + if ($oldgradecat != $gradecat) { + $item->set_parent($gradecat); + $update = true; + } + } + + // Determine the gradepass. + $gradepassfieldname = component_gradeitems::get_field_name_for_itemnumber($component, $item->itemnumber, 'gradepass'); + if (isset($moduleinfo->{$gradepassfieldname})) { + $gradepass = $moduleinfo->{$gradepassfieldname}; + if (null !== $gradepass && $gradepass != $item->gradepass) { + $item->gradepass = $gradepass; + $update = true; + } + } + + if ($update) { + $item->update(); } } } @@ -263,8 +279,6 @@ function edit_module_post_actions($moduleinfo, $course) { require_once($CFG->libdir.'/grade/grade_outcome.php'); // Add outcomes if requested. if ($hasoutcomes && $outcomes = grade_outcome::fetch_all_available($course->id)) { - $grade_items = array(); - // Outcome grade_item.itemnumber start at 1000, there is nothing above outcomes. $max_itemnumber = 999; if ($items) { @@ -279,7 +293,7 @@ function edit_module_post_actions($moduleinfo, $course) { $elname = 'outcome_'.$outcome->id; if (property_exists($moduleinfo, $elname) and $moduleinfo->$elname) { - // So we have a request for new outcome grade item? + // Check if this is a new outcome grade item. if ($items) { $outcomeexists = false; foreach($items as $item) { @@ -295,25 +309,25 @@ function edit_module_post_actions($moduleinfo, $course) { $max_itemnumber++; - $outcome_item = new grade_item(); - $outcome_item->courseid = $course->id; - $outcome_item->itemtype = 'mod'; - $outcome_item->itemmodule = $moduleinfo->modulename; - $outcome_item->iteminstance = $moduleinfo->instance; - $outcome_item->itemnumber = $max_itemnumber; - $outcome_item->itemname = $outcome->fullname; - $outcome_item->outcomeid = $outcome->id; - $outcome_item->gradetype = GRADE_TYPE_SCALE; - $outcome_item->scaleid = $outcome->scaleid; - $outcome_item->insert(); - - // Move the new outcome into correct category and fix sortorder if needed. - if ($grade_item) { - $outcome_item->set_parent($grade_item->categoryid); - $outcome_item->move_after_sortorder($grade_item->sortorder); + $outcomeitem = new grade_item(); + $outcomeitem->courseid = $course->id; + $outcomeitem->itemtype = 'mod'; + $outcomeitem->itemmodule = $moduleinfo->modulename; + $outcomeitem->iteminstance = $moduleinfo->instance; + $outcomeitem->itemnumber = $max_itemnumber; + $outcomeitem->itemname = $outcome->fullname; + $outcomeitem->outcomeid = $outcome->id; + $outcomeitem->gradetype = GRADE_TYPE_SCALE; + $outcomeitem->scaleid = $outcome->scaleid; + $outcomeitem->insert(); + if ($items) { + // Move the new outcome into the same category and immediately after the first grade item. + $item = reset($items); + $outcomeitem->set_parent($item->categoryid); + $outcomeitem->move_after_sortorder($item->sortorder); } else if (isset($moduleinfo->gradecat)) { - $outcome_item->set_parent($moduleinfo->gradecat); + $outcomeitem->set_parent($moduleinfo->gradecat); } } } @@ -354,7 +368,6 @@ function edit_module_post_actions($moduleinfo, $course) { return $moduleinfo; } - /** * Set module info default values for the unset module attributs. * @@ -702,34 +715,43 @@ function get_moduleinfo_data($cm, $course) { } } - if ($items = grade_item::fetch_all(array('itemtype'=>'mod', 'itemmodule'=>$data->modulename, - 'iteminstance'=>$data->instance, 'courseid'=>$course->id))) { + $component = "mod_{$data->modulename}"; + $items = grade_item::fetch_all([ + 'itemtype' => 'mod', + 'itemmodule' => $data->modulename, + 'iteminstance' => $data->instance, + 'courseid' => $course->id, + ]); + + if ($items) { // Add existing outcomes. foreach ($items as $item) { if (!empty($item->outcomeid)) { $data->{'outcome_' . $item->outcomeid} = 1; } else if (isset($item->gradepass)) { - $decimalpoints = $item->get_decimals(); - $data->gradepass = format_float($item->gradepass, $decimalpoints); + $gradepassfieldname = component_gradeitems::get_field_name_for_itemnumber($component, $item->itemnumber, 'gradepass'); + $data->{$gradepassfieldname} = format_float($item->gradepass, $item->get_decimals()); } + } // set category if present - $gradecat = false; + $gradecat = []; foreach ($items as $item) { - if ($gradecat === false) { - $gradecat = $item->categoryid; - continue; + if (!isset($gradecat[$item->itemnumber])) { + $gradecat[$item->itemnumber] = $item->categoryid; } - if ($gradecat != $item->categoryid) { - //mixed categories - $gradecat = false; - break; + if ($gradecat[$item->itemnumber] != $item->categoryid) { + // Mixed categories. + $gradecat[$item->itemnumber] = false; } } - if ($gradecat !== false) { - // do not set if mixed categories present - $data->gradecat = $gradecat; + foreach ($gradecat as $itemnumber => $cat) { + if ($cat !== false) { + $gradecatfieldname = component_gradeitems::get_field_name_for_itemnumber($component, $itemnumber, 'gradecat'); + // Do not set if mixed categories present. + $data->{$gradecatfieldname} = $cat; + } } } return array($cm, $context, $module, $data, $cw); diff --git a/course/moodleform_mod.php b/course/moodleform_mod.php index c977a148113..cc28b58f9f0 100644 --- a/course/moodleform_mod.php +++ b/course/moodleform_mod.php @@ -1,12 +1,40 @@ libdir.'/formslib.php'); +// This file is part of Moodle - http://moodle.org/ +// +// Moodle is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Moodle is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Moodle. If not, see . + +/** + * Moodleform. + * + * @package core_course + * @copyright Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +require_once($CFG->libdir.'/formslib.php'); require_once($CFG->libdir.'/completionlib.php'); require_once($CFG->libdir.'/gradelib.php'); require_once($CFG->libdir.'/plagiarismlib.php'); +use core_grades\component_gradeitems; + /** - * This class adds extra methods to form wrapper specific to be used for module - * add / update forms mod/{modname}/mod_form.php replaced deprecated mod/{modname}/mod.html + * This class adds extra methods to form wrapper specific to be used for module add / update forms + * mod/{modname}/mod_form.php replaced deprecated mod/{modname}/mod.html Moodleform. + * + * @package core_course + * @copyright Andrew Nicols */ abstract class moodleform_mod extends moodleform { /** Current data */ @@ -68,6 +96,9 @@ abstract class moodleform_mod extends moodleform { /** @var object The course format of the current course. */ protected $courseformat; + /** @var string Whether this is graded or rated. */ + private $gradedorrated = null; + public function __construct($current, $section, $cm, $course) { global $CFG; @@ -215,18 +246,19 @@ abstract class moodleform_mod extends moodleform { if ($id = $mform->getElementValue('update')) { $modulename = $mform->getElementValue('modulename'); $instance = $mform->getElementValue('instance'); + $component = "mod_{$modulename}"; if ($this->_features->gradecat) { - $gradecat = false; - if (!empty($CFG->enableoutcomes) and $this->_features->outcomes) { - $outcomes = grade_outcome::fetch_all_available($COURSE->id); - if (!empty($outcomes)) { - $gradecat = true; - } - } - $hasgradeitems = false; - $items = grade_item::fetch_all(array('itemtype'=>'mod', 'itemmodule'=>$modulename,'iteminstance'=>$instance, 'courseid'=>$COURSE->id)); + $items = grade_item::fetch_all([ + 'itemtype' => 'mod', + 'itemmodule' => $modulename, + 'iteminstance' => $instance, + 'courseid' => $COURSE->id, + ]); + + $gradecategories = []; + $removecategories = []; //will be no items if, for example, this activity supports ratings but rating aggregate type == no ratings if (!empty($items)) { foreach ($items as $item) { @@ -241,32 +273,23 @@ abstract class moodleform_mod extends moodleform { } foreach ($items as $item) { - if (is_bool($gradecat)) { - $gradecat = $item->categoryid; - continue; - } - if ($gradecat != $item->categoryid) { - //mixed categories - $gradecat = false; - break; + $gradecatfieldname = component_gradeitems::get_field_name_for_itemnumber( + $component, + $item->itemnumber, + 'gradecat' + ); + + if (!isset($gradecategories[$gradecatfieldname])) { + $gradecategories[$gradecatfieldname] = $item->categoryid; + } else if ($gradecategories[$gradecatfieldname] != $item->categoryid) { + $removecategories[$gradecatfieldname] = true; } } } - if (!$hasgradeitems && $mform->elementExists('gradepass')) { - // Remove form element 'Grade to pass' since there are no grade items (when rating not selected). - $mform->removeElement('gradepass'); - } - - if ($gradecat === false) { - // items and outcomes in different categories - remove the option - // TODO: add a "Mixed categories" text instead of removing elements with no explanation - if ($mform->elementExists('gradecat')) { - $mform->removeElement('gradecat'); - if ($this->_features->rating && !$mform->elementExists('gradepass')) { - //if supports ratings then the max grade dropdown wasnt added so the grade box can be removed entirely - $mform->removeElement('modstandardgrade'); - } + foreach ($removecategories as $toremove) { + if ($mform->elementExists($toremove)) { + $mform->removeElement($toremove); } } } @@ -274,13 +297,14 @@ abstract class moodleform_mod extends moodleform { if ($COURSE->groupmodeforce) { if ($mform->elementExists('groupmode')) { - $mform->hardFreeze('groupmode'); // groupmode can not be changed if forced from course settings + // The groupmode can not be changed if forced from course settings. + // + $mform->hardFreeze('groupmode'); } } - // Don't disable/remove groupingid if it is currently set to something, - // otherwise you cannot turn it off at same time as turning off other - // option (MDL-30764) + // Don't disable/remove groupingid if it is currently set to something, otherwise you cannot turn it off at same + // time as turning off other option (MDL-30764). if (empty($this->_cm) || !$this->_cm->groupingid) { if ($mform->elementExists('groupmode') && empty($COURSE->groupmodeforce)) { $mform->hideIf('groupingid', 'groupmode', 'eq', NOGROUPS); @@ -386,36 +410,45 @@ abstract class moodleform_mod extends moodleform { } } - // Ratings: Don't let them select an aggregate type without selecting a scale. - // If the user has selected to use ratings but has not chosen a scale or set max points then the form is - // invalid. If ratings have been selected then the user must select either a scale or max points. - // This matches (horrible) logic in data_preprocessing. - if (isset($data['assessed']) && $data['assessed'] > 0 && empty($data['scale'])) { - $errors['assessed'] = get_string('scaleselectionrequired', 'rating'); - } - - // Check that the grade pass is a valid number. - $gradepassvalid = false; - if (isset($data['gradepass'])) { - if (unformat_float($data['gradepass'], true) === false) { - $errors['gradepass'] = get_string('err_numeric', 'form'); - } else { - $gradepassvalid = true; + $component = "mod_{$this->_modname}"; + $itemnames = component_gradeitems::get_itemname_mapping_for_component($component); + foreach ($itemnames as $itemnumber => $itemname) { + $gradefieldname = component_gradeitems::get_field_name_for_itemnumber($component, $itemnumber, 'grade'); + $gradepassfieldname = component_gradeitems::get_field_name_for_itemnumber($component, $itemnumber, 'gradepass'); + $assessedfieldname = component_gradeitems::get_field_name_for_itemnumber($component, $itemnumber, 'assessed'); + $scalefieldname = component_gradeitems::get_field_name_for_itemnumber($component, $itemnumber, 'scale'); + + // Ratings: Don't let them select an aggregate type without selecting a scale. + // If the user has selected to use ratings but has not chosen a scale or set max points then the form is + // invalid. If ratings have been selected then the user must select either a scale or max points. + // This matches (horrible) logic in data_preprocessing. + if (isset($data[$assessedfieldname]) && $data[$assessedfieldname] > 0 && empty($data[$scalefieldname])) { + $errors[$assessedfieldname] = get_string('scaleselectionrequired', 'rating'); } - } - // Grade to pass: ensure that the grade to pass is valid for points and scales. - // If we are working with a scale, convert into a positive number for validation. - if ($gradepassvalid && isset($data['gradepass']) && (!empty($data['grade']) || !empty($data['scale']))) { - $scale = !empty($data['grade']) ? $data['grade'] : $data['scale']; - if ($scale < 0) { - $scalevalues = $DB->get_record('scale', array('id' => -$scale)); - $grade = count(explode(',', $scalevalues->scale)); - } else { - $grade = $scale; + // Check that the grade pass is a valid number. + $gradepassvalid = false; + if (isset($data[$gradepassfieldname])) { + if (unformat_float($data[$gradepassfieldname], true) === false) { + $errors[$gradepassfieldname] = get_string('err_numeric', 'form'); + } else { + $gradepassvalid = true; + } } - if (unformat_float($data['gradepass']) > $grade) { - $errors['gradepass'] = get_string('gradepassgreaterthangrade', 'grades', $grade); + + // Grade to pass: ensure that the grade to pass is valid for points and scales. + // If we are working with a scale, convert into a positive number for validation. + if ($gradepassvalid && isset($data[$gradepassfieldname]) && (!empty($data[$gradefieldname]) || !empty($data[$scalefieldname]))) { + $scale = !empty($data[$gradefieldname]) ? $data[$gradefieldname] : $data[$scalefieldname]; + if ($scale < 0) { + $scalevalues = $DB->get_record('scale', array('id' => -$scale)); + $grade = count(explode(',', $scalevalues->scale)); + } else { + $grade = $scale; + } + if (unformat_float($data[$gradepassfieldname]) > $grade) { + $errors[$gradepassfieldname] = get_string('gradepassgreaterthangrade', 'grades', $grade); + } } } @@ -485,7 +518,7 @@ abstract class moodleform_mod extends moodleform { /** * Adds all the standard elements to a form to edit the settings for an activity module. */ - function standard_coursemodule_elements(){ + protected function standard_coursemodule_elements() { global $COURSE, $CFG, $DB; $mform =& $this->_form; @@ -500,70 +533,10 @@ abstract class moodleform_mod extends moodleform { } } - if ($this->_features->rating) { - require_once($CFG->dirroot.'/rating/lib.php'); - $rm = new rating_manager(); - - $mform->addElement('header', 'modstandardratings', get_string('ratings', 'rating')); - - $permission=CAP_ALLOW; - $rolenamestring = null; - $isupdate = false; - if (!empty($this->_cm)) { - $isupdate = true; - $context = context_module::instance($this->_cm->id); - - $rolenames = get_role_names_with_caps_in_context($context, array('moodle/rating:rate', 'mod/'.$this->_cm->modname.':rate')); - $rolenamestring = implode(', ', $rolenames); - } else { - $rolenamestring = get_string('capabilitychecknotavailable','rating'); - } - $mform->addElement('static', 'rolewarning', get_string('rolewarning','rating'), $rolenamestring); - $mform->addHelpButton('rolewarning', 'rolewarning', 'rating'); - - $mform->addElement('select', 'assessed', get_string('aggregatetype', 'rating') , $rm->get_aggregate_types()); - $mform->setDefault('assessed', 0); - $mform->addHelpButton('assessed', 'aggregatetype', 'rating'); - - $gradeoptions = array('isupdate' => $isupdate, - 'currentgrade' => false, - 'hasgrades' => false, - 'canrescale' => $this->_features->canrescale, - 'useratings' => $this->_features->rating); - if ($isupdate) { - $gradeitem = grade_item::fetch(array('itemtype' => 'mod', - 'itemmodule' => $this->_cm->modname, - 'iteminstance' => $this->_cm->instance, - 'itemnumber' => 0, - 'courseid' => $COURSE->id)); - if ($gradeitem) { - $gradeoptions['currentgrade'] = $gradeitem->grademax; - $gradeoptions['currentgradetype'] = $gradeitem->gradetype; - $gradeoptions['currentscaleid'] = $gradeitem->scaleid; - $gradeoptions['hasgrades'] = $gradeitem->has_grades(); - } - } - $mform->addElement('modgrade', 'scale', get_string('scale'), $gradeoptions); - $mform->hideIf('scale', 'assessed', 'eq', 0); - $mform->addHelpButton('scale', 'modgrade', 'grades'); - $mform->setDefault('scale', $CFG->gradepointdefault); - - $mform->addElement('checkbox', 'ratingtime', get_string('ratingtime', 'rating')); - $mform->hideIf('ratingtime', 'assessed', 'eq', 0); - - $mform->addElement('date_time_selector', 'assesstimestart', get_string('from')); - $mform->hideIf('assesstimestart', 'assessed', 'eq', 0); - $mform->disabledIf('assesstimestart', 'ratingtime'); - - $mform->addElement('date_time_selector', 'assesstimefinish', get_string('to')); - $mform->hideIf('assesstimefinish', 'assessed', 'eq', 0); - $mform->disabledIf('assesstimefinish', 'ratingtime'); + $this->add_rating_settings($mform, 0); } - //doing this here means splitting up the grade related settings on the lesson settings page - //$this->standard_grading_coursemodule_elements(); - $mform->addElement('header', 'modstandardelshdr', get_string('modstandardels', 'form')); $section = get_fast_modinfo($COURSE)->get_section_info($this->_section); @@ -737,6 +710,110 @@ abstract class moodleform_mod extends moodleform { $this->plugin_extend_coursemodule_standard_elements(); } + /** + * Add rating settings. + * + * @param moodleform_mod $mform + * @param int $itemnumber + */ + protected function add_rating_settings($mform, int $itemnumber) { + global $CFG, $COURSE; + + if ($this->gradedorrated && $this->gradedorrated !== 'rated') { + return; + } + $this->gradedorrated = 'rated'; + + require_once("{$CFG->dirroot}/rating/lib.php"); + $rm = new rating_manager(); + + $component = "mod_{$this->_modname}"; + $gradecatfieldname = component_gradeitems::get_field_name_for_itemnumber($component, $itemnumber, 'gradecat'); + $gradepassfieldname = component_gradeitems::get_field_name_for_itemnumber($component, $itemnumber, 'gradepass'); + $assessedfieldname = component_gradeitems::get_field_name_for_itemnumber($component, $itemnumber, 'assessed'); + $scalefieldname = component_gradeitems::get_field_name_for_itemnumber($component, $itemnumber, 'scale'); + + $mform->addElement('header', 'modstandardratings', get_string('ratings', 'rating')); + + $isupdate = !empty($this->_cm); + + $rolenamestring = null; + if ($isupdate) { + $context = context_module::instance($this->_cm->id); + $capabilities = ['moodle/rating:rate', "mod/{$this->_cm->modname}:rate"]; + $rolenames = get_role_names_with_caps_in_context($context, $capabilities); + $rolenamestring = implode(', ', $rolenames); + } else { + $rolenamestring = get_string('capabilitychecknotavailable', 'rating'); + } + + $mform->addElement('static', 'rolewarning', get_string('rolewarning', 'rating'), $rolenamestring); + $mform->addHelpButton('rolewarning', 'rolewarning', 'rating'); + + $mform->addElement('select', $assessedfieldname, get_string('aggregatetype', 'rating') , $rm->get_aggregate_types()); + $mform->setDefault($assessedfieldname, 0); + $mform->addHelpButton($assessedfieldname, 'aggregatetype', 'rating'); + + $gradeoptions = [ + 'isupdate' => $isupdate, + 'currentgrade' => false, + 'hasgrades' => false, + 'canrescale' => false, + 'useratings' => true, + ]; + if ($isupdate) { + $gradeitem = grade_item::fetch([ + 'itemtype' => 'mod', + 'itemmodule' => $this->_cm->modname, + 'iteminstance' => $this->_cm->instance, + 'itemnumber' => $itemnumber, + 'courseid' => $COURSE->id, + ]); + if ($gradeitem) { + $gradeoptions['currentgrade'] = $gradeitem->grademax; + $gradeoptions['currentgradetype'] = $gradeitem->gradetype; + $gradeoptions['currentscaleid'] = $gradeitem->scaleid; + $gradeoptions['hasgrades'] = $gradeitem->has_grades(); + } + } + + $mform->addElement('modgrade', $scalefieldname, get_string('scale'), $gradeoptions); + $mform->hideIf($scalefieldname, $assessedfieldname, 'eq', 0); + $mform->addHelpButton($scalefieldname, 'modgrade', 'grades'); + $mform->setDefault($scalefieldname, $CFG->gradepointdefault); + + $mform->addElement('checkbox', 'ratingtime', get_string('ratingtime', 'rating')); + $mform->hideIf('ratingtime', $assessedfieldname, 'eq', 0); + + $mform->addElement('date_time_selector', 'assesstimestart', get_string('from')); + $mform->hideIf('assesstimestart', $assessedfieldname, 'eq', 0); + $mform->hideIf('assesstimestart', 'ratingtime'); + + $mform->addElement('date_time_selector', 'assesstimefinish', get_string('to')); + $mform->hideIf('assesstimefinish', $assessedfieldname, 'eq', 0); + $mform->hideIf('assesstimefinish', 'ratingtime'); + + if ($this->_features->gradecat) { + $mform->addElement( + 'select', + $gradecatfieldname, + get_string('gradecategoryonmodform', 'grades'), + grade_get_categories_menu($COURSE->id, $this->_outcomesused) + ); + $mform->addHelpButton($gradecatfieldname, 'gradecategoryonmodform', 'grades'); + $mform->hideIf($gradecatfieldname, $assessedfieldname, 'eq', 0); + $mform->hideIf($gradecatfieldname, "{$scalefieldname}[modgrade_type]", 'eq', 'none'); + } + + // Grade to pass. + $mform->addElement('text', $gradepassfieldname, get_string('gradepass', 'grades')); + $mform->addHelpButton($gradepassfieldname, 'gradepass', 'grades'); + $mform->setDefault($gradepassfieldname, ''); + $mform->setType($gradepassfieldname, PARAM_RAW); + $mform->hideIf($gradepassfieldname, $assessedfieldname, 'eq', '0'); + $mform->hideIf($gradepassfieldname, "{$scalefieldname}[modgrade_type]", 'eq', 'none'); + } + /** * Plugins can extend the coursemodule settings form. */ @@ -811,6 +888,21 @@ abstract class moodleform_mod extends moodleform { public function standard_grading_coursemodule_elements() { global $COURSE, $CFG; + + if ($this->gradedorrated && $this->gradedorrated !== 'graded') { + return; + } + if ($this->_features->rating) { + return; + } + $this->gradedorrated = 'graded'; + + $itemnumber = 0; + $component = "mod_{$this->_modname}"; + $gradefieldname = component_gradeitems::get_field_name_for_itemnumber($component, $itemnumber, 'grade'); + $gradecatfieldname = component_gradeitems::get_field_name_for_itemnumber($component, $itemnumber, 'gradecat'); + $gradepassfieldname = component_gradeitems::get_field_name_for_itemnumber($component, $itemnumber, 'gradepass'); + $mform =& $this->_form; $isupdate = !empty($this->_cm); $gradeoptions = array('isupdate' => $isupdate, @@ -820,31 +912,27 @@ abstract class moodleform_mod extends moodleform { 'useratings' => $this->_features->rating); if ($this->_features->hasgrades) { - - if (!$this->_features->rating || $this->_features->gradecat) { + if ($this->_features->gradecat) { $mform->addElement('header', 'modstandardgrade', get_string('grade')); } //if supports grades and grades arent being handled via ratings - if (!$this->_features->rating) { - - if ($isupdate) { - $gradeitem = grade_item::fetch(array('itemtype' => 'mod', - 'itemmodule' => $this->_cm->modname, - 'iteminstance' => $this->_cm->instance, - 'itemnumber' => 0, - 'courseid' => $COURSE->id)); - if ($gradeitem) { - $gradeoptions['currentgrade'] = $gradeitem->grademax; - $gradeoptions['currentgradetype'] = $gradeitem->gradetype; - $gradeoptions['currentscaleid'] = $gradeitem->scaleid; - $gradeoptions['hasgrades'] = $gradeitem->has_grades(); - } + if ($isupdate) { + $gradeitem = grade_item::fetch(array('itemtype' => 'mod', + 'itemmodule' => $this->_cm->modname, + 'iteminstance' => $this->_cm->instance, + 'itemnumber' => 0, + 'courseid' => $COURSE->id)); + if ($gradeitem) { + $gradeoptions['currentgrade'] = $gradeitem->grademax; + $gradeoptions['currentgradetype'] = $gradeitem->gradetype; + $gradeoptions['currentscaleid'] = $gradeitem->scaleid; + $gradeoptions['hasgrades'] = $gradeitem->has_grades(); } - $mform->addElement('modgrade', 'grade', get_string('grade'), $gradeoptions); - $mform->addHelpButton('grade', 'modgrade', 'grades'); - $mform->setDefault('grade', $CFG->gradepointdefault); } + $mform->addElement('modgrade', $gradefieldname, get_string('grade'), $gradeoptions); + $mform->addHelpButton($gradefieldname, 'modgrade', 'grades'); + $mform->setDefault($gradefieldname, $CFG->gradepointdefault); if ($this->_features->advancedgrading and !empty($this->current->_advancedgradingdata['methods']) @@ -858,9 +946,7 @@ abstract class moodleform_mod extends moodleform { $mform->addElement('select', 'advancedgradingmethod_'.$areaname, get_string('gradingmethod', 'core_grading'), $this->current->_advancedgradingdata['methods']); $mform->addHelpButton('advancedgradingmethod_'.$areaname, 'gradingmethod', 'core_grading'); - if (!$this->_features->rating) { - $mform->hideIf('advancedgradingmethod_'.$areaname, 'grade[modgrade_type]', 'eq', 'none'); - } + $mform->hideIf('advancedgradingmethod_'.$areaname, "{$gradefieldname}[modgrade_type]", 'eq', 'none'); } else { // the module defines multiple gradable areas, display a selector @@ -877,25 +963,19 @@ abstract class moodleform_mod extends moodleform { } if ($this->_features->gradecat) { - $mform->addElement('select', 'gradecat', + $mform->addElement('select', $gradecatfieldname, get_string('gradecategoryonmodform', 'grades'), grade_get_categories_menu($COURSE->id, $this->_outcomesused)); - $mform->addHelpButton('gradecat', 'gradecategoryonmodform', 'grades'); - if (!$this->_features->rating) { - $mform->hideIf('gradecat', 'grade[modgrade_type]', 'eq', 'none'); - } + $mform->addHelpButton($gradecatfieldname, 'gradecategoryonmodform', 'grades'); + $mform->hideIf($gradecatfieldname, "{$gradefieldname}[modgrade_type]", 'eq', 'none'); } // Grade to pass. - $mform->addElement('text', 'gradepass', get_string('gradepass', 'grades')); - $mform->addHelpButton('gradepass', 'gradepass', 'grades'); - $mform->setDefault('gradepass', ''); - $mform->setType('gradepass', PARAM_RAW); - if (!$this->_features->rating) { - $mform->hideIf('gradepass', 'grade[modgrade_type]', 'eq', 'none'); - } else { - $mform->hideIf('gradepass', 'assessed', 'eq', '0'); - } + $mform->addElement('text', $gradepassfieldname, get_string($gradepassfieldname, 'grades')); + $mform->addHelpButton($gradepassfieldname, $gradepassfieldname, 'grades'); + $mform->setDefault($gradepassfieldname, ''); + $mform->setType($gradepassfieldname, PARAM_RAW); + $mform->hideIf($gradepassfieldname, "{$gradefieldname}[modgrade_type]", 'eq', 'none'); } } @@ -1111,5 +1191,3 @@ abstract class moodleform_mod extends moodleform { return $data; } } - - diff --git a/course/tests/externallib_test.php b/course/tests/externallib_test.php index 242b02cade1..dd752c0f86e 100644 --- a/course/tests/externallib_test.php +++ b/course/tests/externallib_test.php @@ -2065,7 +2065,7 @@ class core_course_externallib_testcase extends externallib_advanced_testcase { $outcomegradeitem->cmid = 0; $outcomegradeitem->courseid = $course->id; $outcomegradeitem->aggregationcoef = 0; - $outcomegradeitem->itemnumber = 1; // The activity's original grade item will be 0. + $outcomegradeitem->itemnumber = 1000; // Outcomes start at 1000. $outcomegradeitem->gradetype = GRADE_TYPE_SCALE; $outcomegradeitem->scaleid = $outcome->scaleid; $outcomegradeitem->insert(); diff --git a/grade/classes/component_gradeitems.php b/grade/classes/component_gradeitems.php new file mode 100644 index 00000000000..daa260d3d9a --- /dev/null +++ b/grade/classes/component_gradeitems.php @@ -0,0 +1,219 @@ +. + +/** + * Helper class to fetch information about component grade items. + * + * @package core_grades + * @copyright Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +declare(strict_types = 1); + +namespace core_grades; + +use code_grades\local\gradeitem\itemnumber_mapping; +use code_grades\local\gradeitem\advancedgrading_mapping; + +/** + * Helper class to fetch information about component grade items. + * + * @package core_grades + * @copyright Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class component_gradeitems { + + /** + * Get the gradeitems classname for the specific component. + * + * @param string $component The component to fetch the classname for + * @return string The composed classname + */ + protected static function get_component_classname(string $component): string { + return "{$component}\\grades\gradeitems"; + } + + /** + * Get the grade itemnumber mapping for a component. + * + * @param string $component The component that the grade item belongs to + * @return array + */ + public static function get_itemname_mapping_for_component(string $component): array { + $classname = "{$component}\\grades\gradeitems"; + + if (!class_exists($classname)) { + return [ + 0 => '', + ]; + } + + if (!is_subclass_of($classname, 'core_grades\local\gradeitem\itemnumber_mapping')) { + throw new \coding_exception("The {$classname} class does not implement " . itemnumber_mapping::class); + } + + return $classname::get_itemname_mapping_for_component(); + } + + /** + * Whether the named grading item exists. + * + * @param string $component + * @param string $itemname + * @return bool + */ + public static function is_valid_itemname(string $component, string $itemname): bool { + $items = self::get_itemname_mapping_for_component($component); + + return array_search($itemname, $items) !== false; + } + + /** + * Check whether the component class defines the advanced grading items. + * + * @param string $component The component to check + * @return bool + */ + public static function defines_advancedgrading_itemnames_for_component(string $component): bool { + return is_subclass_of(self::get_component_classname($component), 'core_grades\local\gradeitem\advancedgrading_mapping'); + } + + /** + * Get the list of advanced grading item names for the named component. + * + * @param string $component + * @return array + */ + public static function get_advancedgrading_itemnames_for_component(string $component): array { + $classname = self::get_component_classname($component); + if (!self::defines_advancedgrading_itemnames_for_component($component)) { + throw new \coding_exception("The {$classname} class does not implement " . advancedgrading_mapping::class); + } + + return $classname::get_advancedgrading_itemnames(); + } + + /** + * Whether the named grading item name supports advanced grading. + * + * @param string $component + * @param string $itemname + * @return bool + */ + public static function is_advancedgrading_itemname(string $component, string $itemname): bool { + $gradingareas = self::get_advancedgrading_itemnames_for_component($component); + + return array_search($itemname, $gradingareas) !== false; + } + + /** + * Get the suffixed field name for an activity field mapped from its itemnumber. + * + * For legacy reasons, the first itemnumber has no suffix on field names. + * + * @param string $component The component that the grade item belongs to + * @param int $itemnumber The grade itemnumber + * @param string $fieldname The name of the field to be rewritten + * @return string The translated field name + */ + public static function get_field_name_for_itemnumber(string $component, int $itemnumber, string $fieldname): string { + $itemname = static::get_itemname_from_itemnumber($component, $itemnumber); + + if ($itemname) { + return "{$fieldname}_{$itemname}"; + } + + return $fieldname; + } + + /** + * Get the suffixed field name for an activity field mapped from its itemnumber. + * + * For legacy reasons, the first itemnumber has no suffix on field names. + * + * @param string $component The component that the grade item belongs to + * @param string $itemname The grade itemname + * @param string $fieldname The name of the field to be rewritten + * @return string The translated field name + */ + public static function get_field_name_for_itemname(string $component, string $itemname, string $fieldname): string { + if (empty($itemname)) { + return $fieldname; + } + + $itemnumber = static::get_itemnumber_from_itemname($component, $itemname); + + if ($itemnumber > 0) { + return "{$fieldname}_{$itemname}"; + } + + return $fieldname; + } + + /** + * Get the itemname for an itemnumber. + * + * For legacy compatability when the itemnumber is 0, the itemname will always be empty. + * + * @param string $component The component that the grade item belongs to + * @param int $itemnumber The grade itemnumber + * @return int The grade itemnumber of the itemname + */ + public static function get_itemname_from_itemnumber(string $component, int $itemnumber): string { + if ($itemnumber === 0) { + return ''; + } + + $mappings = self::get_itemname_mapping_for_component($component); + + if (isset($mappings[$itemnumber])) { + return $mappings[$itemnumber]; + } + + if ($itemnumber >= 1000) { + // An itemnumber >= 1000 belongs to an outcome. + return ''; + } + + throw new \coding_exception("Unknown itemnumber mapping for {$itemnumber} in {$component}"); + } + + /** + * Get the itemnumber for a item name. + * + * For legacy compatability when the itemname is empty, the itemnumber will always be 0. + * + * @param string $component The component that the grade item belongs to + * @param string $itemname The grade itemname + * @return int The grade itemname of the itemnumber + */ + public static function get_itemnumber_from_itemname(string $component, string $itemname): int { + if (empty($itemname)) { + return 0; + } + + $mappings = self::get_itemname_mapping_for_component($component); + + $flipped = array_flip($mappings); + if (isset($flipped[$itemname])) { + return $flipped[$itemname]; + } + + throw new \coding_exception("Unknown itemnumber mapping for {$itemname} in {$component}"); + } +} diff --git a/grade/classes/local/gradeitem/advancedgrading_mapping.php b/grade/classes/local/gradeitem/advancedgrading_mapping.php new file mode 100644 index 00000000000..104ccfb5ad1 --- /dev/null +++ b/grade/classes/local/gradeitem/advancedgrading_mapping.php @@ -0,0 +1,43 @@ +. + +/** + * Grade item, itemnumber mapping. + * + * @package core_grades + * @copyright Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +declare(strict_types = 1); + +namespace core_grades\local\gradeitem; + +/** + * Grade item, itemnumber mapping. + * + * @package core_grades + * @copyright Andrew Nicols + */ +interface advancedgrading_mapping { + + /** + * Get the list of advanced grading item names for this component. + * + * @return array + */ + public static function get_advancedgrading_itemnames(): array; +} diff --git a/grade/classes/local/gradeitem/itemnumber_mapping.php b/grade/classes/local/gradeitem/itemnumber_mapping.php new file mode 100644 index 00000000000..5db9726a24a --- /dev/null +++ b/grade/classes/local/gradeitem/itemnumber_mapping.php @@ -0,0 +1,43 @@ +. + +/** + * Grade item, itemnumber mapping. + * + * @package core_grades + * @copyright Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +declare(strict_types = 1); + +namespace core_grades\local\gradeitem; + +/** + * Grade item, itemnumber mapping. + * + * @package core_grades + * @copyright Andrew Nicols + */ +interface itemnumber_mapping { + + /** + * Get the grade item mapping of item number to item name. + * + * @return array + */ + public static function get_itemname_mapping_for_component(): array; +} diff --git a/grade/grading/lib.php b/grade/grading/lib.php index d2f57e9f63b..d4954489cd4 100644 --- a/grade/grading/lib.php +++ b/grade/grading/lib.php @@ -24,6 +24,8 @@ defined('MOODLE_INTERNAL') || die(); +use core_grades\component_gradeitems; + /** * Factory method returning an instance of the grading manager * @@ -288,14 +290,29 @@ class grading_manager { public static function available_areas($component) { global $CFG; + if (component_gradeitems::defines_advancedgrading_itemnames_for_component($component)) { + $result = []; + foreach (component_gradeitems::get_advancedgrading_itemnames_for_component($component) as $itemnumber => $itemname) { + $result[$itemname] = get_string("gradeitem:{$itemname}", $component); + } + + return $result; + } + list($plugintype, $pluginname) = core_component::normalize_component($component); if ($component === 'core_grading') { return array(); } else if ($plugintype === 'mod') { - return plugin_callback('mod', $pluginname, 'grading', 'areas_list', null, array()); - + $callbackfunction = "grading_areas_list"; + if (component_callback_exists($component, $callbackfunction)) { + debugging( + "Components supporting advanced grading should be updated to implement the component_gradeitems class", + DEBUG_DEVELOPER + ); + return component_callback($component, $callbackfunction, [], []); + } } else { throw new coding_exception('Unsupported area location'); } diff --git a/grade/tests/behat/grade_to_pass.feature b/grade/tests/behat/grade_to_pass.feature index e257fd95206..a0e00d79263 100644 --- a/grade/tests/behat/grade_to_pass.feature +++ b/grade/tests/behat/grade_to_pass.feature @@ -217,7 +217,7 @@ Feature: We can set the grade to pass value And I am on "Course 1" course homepage And I follow "Test Forum 1" And I follow "Edit settings" - And the field "Grade to pass" matches value "80" + And the field "Ratings > Grade to pass" matches value "80" Scenario: Set a valid grade to pass for glossary activity When I turn editing mode on diff --git a/grade/tests/component_gradeitems_test.php b/grade/tests/component_gradeitems_test.php new file mode 100644 index 00000000000..a84e5774f52 --- /dev/null +++ b/grade/tests/component_gradeitems_test.php @@ -0,0 +1,653 @@ +. + +/** + * Unit tests for core_grades\component_gradeitems; + * + * @package core_grades + * @category test + * @copyright 2019 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU Public License + */ + +declare(strict_types = 1); + +namespace tests\core_grades { + + use advanced_testcase; + use core_grades\component_gradeitems; + use coding_exception; + + /** + * Unit tests for core_grades\component_gradeitems; + * + * @package core_grades + * @category test + * @copyright 2019 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + class component_gradeitems_test extends advanced_testcase { + + /** + * Ensure that a component which does not implement the mapping class excepts. + */ + public function test_get_itemname_mapping_for_component_does_not_exist(): void { + $mappings = component_gradeitems::get_itemname_mapping_for_component('invalid_component'); + $this->assertIsArray($mappings); + $this->assertCount(1, $mappings); + $this->assertArrayHasKey(0, $mappings); + } + + /** + * Ensure that a component which does not implement the mapping class correctly excepts. + */ + public function test_get_itemname_mapping_for_valid_component_invalid_mapping(): void { + $this->expectException(coding_exception::class); + component_gradeitems::get_itemname_mapping_for_component('tests\core_grades\component_gradeitems\invalid'); + } + + /** + * Ensure that a component which implements the mapping class correctly eets the correct set of mappings. + */ + public function test_get_itemname_mapping_for_valid_component_valid_mapping(): void { + $mapping = component_gradeitems::get_itemname_mapping_for_component('tests\core_grades\component_gradeitems\valid'); + $this->assertIsArray($mapping); + $this->assertEquals([ + 0 => 'rating', + 1 => 'someother', + ], $mapping); + } + + /** + * Data provider for is_valid_itemname tests. + * + * @return array + */ + public function is_valid_itemname_provider(): array { + return [ + 'valid' => [ + 'someother', + true, + ], + 'validnotadvanced' => [ + 'rating', + true, + ], + 'invalid' => [ + 'doesnotexist', + false, + ], + ]; + } + + /** + * Ensure that a component implementing advanced grading returns the correct areas. + * + * @dataProvider is_valid_itemname_provider + * @param string $itemname + * @param bool $isadvanced + */ + public function test_is_valid_itemname(string $itemname, bool $isadvanced): void { + $this->assertEquals( + $isadvanced, + component_gradeitems::is_valid_itemname('tests\core_grades\component_gradeitems\valid_and_advanced', $itemname) + ); + } + + + /** + * Ensure that a component which does not implement the advancedgrading interface returns this. + */ + public function test_defines_advancedgrading_itemnames_for_component_does_not_exist(): void { + $this->assertFalse(component_gradeitems::defines_advancedgrading_itemnames_for_component('invalid_component')); + } + + /** + * Ensure that a component which does not implement the advancedgrading interface returns this. + */ + public function test_defines_advancedgrading_itemnames_for_component_no_interfaces(): void { + $this->assertFalse(component_gradeitems::defines_advancedgrading_itemnames_for_component('tests\core_grades\component_gradeitems\invalid')); + } + + /** + * Ensure that a component which implements the item mapping but not implement the advancedgrading interface returns this. + */ + public function test_defines_advancedgrading_itemnames_for_component_grading_no_interface(): void { + $this->assertFalse(component_gradeitems::defines_advancedgrading_itemnames_for_component('tests\core_grades\component_gradeitems\valid')); + } + + /** + * Ensure that a component which implements the item mapping but not implement the advancedgrading interface returns this. + */ + public function test_defines_advancedgrading_itemnames_for_component_grading_has_interface(): void { + $this->assertTrue(component_gradeitems::defines_advancedgrading_itemnames_for_component('tests\core_grades\component_gradeitems\valid_and_advanced')); + } + + /** + * Ensure that a component which does not implement the advancedgrading interface returns this. + */ + public function test_get_advancedgrading_itemnames_for_component_does_not_exist(): void { + $this->expectException(coding_exception::class); + component_gradeitems::get_advancedgrading_itemnames_for_component('invalid_component'); + } + + /** + * Ensure that a component which does not implement the advancedgrading interface returns this. + */ + public function test_get_advancedgrading_itemnames_for_component_no_interfaces(): void { + $this->expectException(coding_exception::class); + component_gradeitems::get_advancedgrading_itemnames_for_component('tests\core_grades\component_gradeitems\invalid'); + } + + /** + * Ensure that a component which implements the item mapping but not implement the advancedgrading interface returns this. + */ + public function test_get_advancedgrading_itemnames_for_component_grading_no_interface(): void { + $this->expectException(coding_exception::class); + component_gradeitems::get_advancedgrading_itemnames_for_component('tests\core_grades\component_gradeitems\valid'); + } + + /** + * Ensure that a component implementing advanced grading returns the correct areas. + */ + public function test_get_advancedgrading_itemnames_for_component(): void { + $areas = component_gradeitems::get_advancedgrading_itemnames_for_component('tests\core_grades\component_gradeitems\valid_and_advanced'); + $this->assertEquals(['someother'], $areas); + } + + /** + * Data provider for is_advancedgrading_itemname tests. + * + * @return array + */ + public function is_advancedgrading_itemname_provider(): array { + return [ + 'valid' => [ + 'someother', + true, + ], + 'validnotadvanced' => [ + 'rating', + false, + ], + 'invalid' => [ + 'doesnotexist', + false, + ], + ]; + } + + /** + * Ensure that a component implementing advanced grading returns the correct areas. + * + * @dataProvider is_advancedgrading_itemname_provider + * @param string $itemname + * @param bool $isadvanced + */ + public function test_is_advancedgrading_itemname(string $itemname, bool $isadvanced): void { + $this->assertEquals( + $isadvanced, + component_gradeitems::is_advancedgrading_itemname('tests\core_grades\component_gradeitems\valid_and_advanced', $itemname) + ); + } + + /** + * Data provider for get_field_name_for_itemnumber. + * + * @return array + */ + public function get_field_name_for_itemnumber_provider(): array { + return [ + 'Valid itemnumber 0 case 1' => [ + 0, + 'gradecat', + 'gradecat', + ], + 'Valid itemnumber 0 case 2' => [ + 0, + 'melon', + 'melon', + ], + 'Valid itemnumber 1 case 1' => [ + 1, + 'gradecat', + 'gradecat_someother', + ], + 'Valid itemnumber 1 case 2' => [ + 1, + 'melon', + 'melon_someother', + ], + ]; + } + + /** + * Ensure that valid field names are correctly mapped for a valid component. + * + * @dataProvider get_field_name_for_itemnumber_provider + * @param int $itemnumber The item itemnumber to test + * @param string $fieldname The field name being translated + * @param string $expected The expected value + */ + public function test_get_field_name_for_itemnumber(int $itemnumber, string $fieldname, string $expected): void { + $component = 'tests\core_grades\component_gradeitems\valid'; + $this->assertEquals($expected, component_gradeitems::get_field_name_for_itemnumber($component, $itemnumber, $fieldname)); + } + + /** + * Ensure that an invalid itemnumber does not provide any field name. + */ + public function test_get_field_name_for_itemnumber_invalid_itemnumber(): void { + $component = 'tests\core_grades\component_gradeitems\valid'; + + $this->expectException(coding_exception::class); + component_gradeitems::get_field_name_for_itemnumber($component, 100, 'gradecat'); + } + + /** + * Ensure that a component which does not define a mapping can still get a mapping for itemnumber 0. + */ + public function test_get_field_name_for_itemnumber_component_not_defining_mapping_itemnumber_zero(): void { + $component = 'tests\core_grades\othervalid'; + + $this->assertEquals('gradecat', component_gradeitems::get_field_name_for_itemnumber($component, 0, 'gradecat')); + } + + /** + * Ensure that a component which does not define a mapping cannot get a mapping for itemnumber 1+. + */ + public function test_get_field_name_for_itemnumber_component_not_defining_mapping_itemnumber_nonzero(): void { + $component = 'tests\core_grades\othervalid'; + + $this->expectException(coding_exception::class); + component_gradeitems::get_field_name_for_itemnumber($component, 100, 'gradecat'); + } + + /** + * Ensure that a component which incorrectly defines a mapping cannot get a mapping for itemnumber 1+. + */ + public function test_get_field_name_for_itemnumber_component_invalid_mapping_itemnumber_nonzero(): void { + $component = 'tests\core_grades\component_gradeitems\invalid'; + + $this->expectException(coding_exception::class); + component_gradeitems::get_field_name_for_itemnumber($component, 100, 'gradecat'); + } + + /** + * Data provider for get_field_name_for_itemname. + * + * @return array + */ + public function get_field_name_for_itemname_provider(): array { + return [ + 'Empty itemname empty case 1' => [ + '', + 'gradecat', + 'gradecat', + ], + 'Empty itemname empty case 2' => [ + '', + 'melon', + 'melon', + ], + 'First itemname empty case 1' => [ + 'rating', + 'gradecat', + 'gradecat', + ], + 'First itemname empty case 2' => [ + 'rating', + 'melon', + 'melon', + ], + 'Other itemname empty case 1' => [ + 'someother', + 'gradecat', + 'gradecat_someother', + ], + 'Other itemname empty case 2' => [ + 'someother', + 'melon', + 'melon_someother', + ], + ]; + } + + /** + * Ensure that valid field names are correctly mapped for a valid component. + * + * @dataProvider get_field_name_for_itemname_provider + * @param string $itemname The item itemname to test + * @param string $fieldname The field name being translated + * @param string $expected The expected value + */ + public function test_get_field_name_for_itemname(string $itemname, string $fieldname, string $expected): void { + $component = 'tests\core_grades\component_gradeitems\valid'; + $this->assertEquals($expected, component_gradeitems::get_field_name_for_itemname($component, $itemname, $fieldname)); + } + + /** + * Ensure that an invalid itemname does not provide any field name. + */ + public function test_get_field_name_for_itemname_invalid_itemname(): void { + $component = 'tests\core_grades\component_gradeitems\valid'; + + $this->expectException(coding_exception::class); + component_gradeitems::get_field_name_for_itemname($component, 'typo', 'gradecat'); + } + + /** + * Ensure that an empty itemname provides a matching fieldname regardless of whether the component exists or + * not. + */ + public function test_get_field_name_for_itemname_not_defining_mapping_empty_name(): void { + $component = 'tests\core_grades\othervalid'; + + $this->assertEquals('gradecat', component_gradeitems::get_field_name_for_itemname($component, '', 'gradecat')); + } + + /** + * Ensure that an valid component with some itemname excepts. + */ + public function test_get_field_name_for_itemname_not_defining_mapping_with_name(): void { + $component = 'tests\core_grades\othervalid'; + + $this->expectException(coding_exception::class); + component_gradeitems::get_field_name_for_itemname($component, 'example', 'gradecat'); + } + + /** + * Ensure that an empty itemname provides a matching fieldname even if the mapping is invalid. + */ + public function test_get_field_name_for_itemname_invalid_mapping_empty_name(): void { + $component = 'tests\core_grades\component_gradeitems\invalid'; + + $this->assertEquals('gradecat', component_gradeitems::get_field_name_for_itemname($component, '', 'gradecat')); + } + + /** + * Ensure that an invalid mapping with some itemname excepts. + */ + public function test_get_field_name_for_itemname_invalid_mapping_with_name(): void { + $component = 'tests\core_grades\component_gradeitems\invalid'; + + $this->expectException(coding_exception::class); + component_gradeitems::get_field_name_for_itemname($component, 'example', 'gradecat'); + } + + /** + * Data provider for get_itemname_from_itemnumber. + * + * @return array + */ + public function get_itemname_from_itemnumber_provider(): array { + return [ + 'Valid itemnumber 0' => [ + 0, + '', + ], + 'Valid itemnumber 1' => [ + 1, + 'someother', + ], + ]; + } + + /** + * Ensure that item names are correctly mapped for a valid component. + * + * @dataProvider get_itemname_from_itemnumber_provider + * @param int $itemnumber The item itemnumber to test + * @param string $expected The expected value + */ + public function test_get_itemname_from_itemnumber(int $itemnumber, string $expected): void { + $component = 'tests\core_grades\component_gradeitems\valid'; + $this->assertEquals($expected, component_gradeitems::get_itemname_from_itemnumber($component, $itemnumber)); + } + + /** + * Ensure that an itemnumber over 1000 is treated as itemnumber 0 for the purpose of outcomes. + */ + public function test_get_itemname_from_itemnumber_outcome_itemnumber(): void { + $component = 'tests\core_grades\component_gradeitems\valid'; + + $this->assertEquals('', component_gradeitems::get_itemname_from_itemnumber($component, 1000)); + } + + /** + * Ensure that an invalid itemnumber does not provide any field name. + */ + public function test_get_itemname_from_itemnumber_invalid_itemnumber(): void { + $component = 'tests\core_grades\component_gradeitems\valid'; + + $this->expectException(coding_exception::class); + component_gradeitems::get_itemname_from_itemnumber($component, 100); + } + + /** + * Ensure that a component which does not define a mapping can still get a mapping for itemnumber 0. + */ + public function test_get_itemname_from_itemnumber_component_not_defining_mapping_itemnumber_zero(): void { + $component = 'tests\core_grades\othervalid'; + + $this->assertEquals('', component_gradeitems::get_itemname_from_itemnumber($component, 0)); + } + + /** + * Ensure that a component which does not define a mapping cannot get a mapping for itemnumber 1+. + */ + public function test_get_itemname_from_itemnumber_component_not_defining_mapping_itemnumber_nonzero(): void { + $component = 'tests\core_grades\othervalid'; + + $this->expectException(coding_exception::class); + component_gradeitems::get_itemname_from_itemnumber($component, 100); + } + + /** + * Ensure that a component which incorrectly defines a mapping cannot get a mapping for itemnumber 1+. + */ + public function test_get_itemname_from_itemnumber_component_invalid_mapping_itemnumber_nonzero(): void { + $component = 'tests\core_grades\component_gradeitems\invalid'; + + $this->expectException(coding_exception::class); + component_gradeitems::get_itemname_from_itemnumber($component, 100); + } + + /** + * Data provider for get_itemname_from_itemnumber. + * + * @return array + */ + public function get_itemnumber_from_itemname_provider(): array { + return [ + 'Empty itemname empty' => [ + '', + 0, + ], + 'First itemname empty' => [ + 'rating', + 0, + ], + 'Other itemname empty' => [ + 'someother', + 1, + ], + ]; + } + + /** + * Ensure that valid item names are correctly mapped for a valid component. + * + * @dataProvider get_itemnumber_from_itemname_provider + * @param string $itemname The item itemname to test + * @param int $expected The expected value + */ + public function test_get_itemnumber_from_itemname(string $itemname, int $expected): void { + $component = 'tests\core_grades\component_gradeitems\valid'; + $this->assertEquals($expected, component_gradeitems::get_itemnumber_from_itemname($component, $itemname)); + } + + /** + * Ensure that an invalid itemname excepts. + */ + public function test_get_itemnumber_from_itemname_invalid_itemname(): void { + $component = 'tests\core_grades\component_gradeitems\valid'; + + $this->expectException(coding_exception::class); + component_gradeitems::get_itemnumber_from_itemname($component, 'typo'); + } + + /** + * Ensure that an empty itemname provides a correct itemnumber regardless of whether the component exists or + * not. + */ + public function test_get_itemnumber_from_itemname_not_defining_mapping_empty_name(): void { + $component = 'tests\core_grades\component_gradeitems\othervalid'; + + $this->assertEquals(0, component_gradeitems::get_itemnumber_from_itemname($component, '')); + } + + /** + * Ensure that an valid component with some itemname excepts. + */ + public function test_get_itemnumber_from_itemname_not_defining_mapping_with_name(): void { + $component = 'tests\core_grades\component_gradeitems\othervalid'; + + $this->expectException(coding_exception::class); + component_gradeitems::get_itemnumber_from_itemname($component, 'example'); + } + + /** + * Ensure that an empty itemname provides a matching fieldname even if the mapping is invalid. + */ + public function test_get_itemnumber_from_itemname_invalid_mapping_empty_name(): void { + $component = 'tests\core_grades\component_gradeitems\invalid'; + + $this->assertEquals(0, component_gradeitems::get_itemnumber_from_itemname($component, '')); + } + + /** + * Ensure that an invalid mapping with some itemname excepts. + */ + public function test_get_itemnumber_from_itemname_invalid_mapping_with_name(): void { + $component = 'tests\core_grades\component_gradeitems\invalid'; + + $this->expectException(coding_exception::class); + component_gradeitems::get_itemnumber_from_itemname($component, 'example'); + } + } +} + +namespace tests\core_grades\component_gradeitems\valid\grades { + use core_grades\local\gradeitem\itemnumber_mapping; + + /** + * Valid class for testing mappings. + * + * @package core_grades + * @category test + * @copyright 2019 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + class gradeitems implements itemnumber_mapping { + /** + * Get the grade item mapping of item number to item name. + * + * @return array + */ + public static function get_itemname_mapping_for_component(): array { + return [ + 0 => 'rating', + 1 => 'someother', + ]; + } + } +} + +namespace tests\core_grades\component_gradeitems\valid_and_advanced\grades { + use core_grades\local\gradeitem\itemnumber_mapping; + use core_grades\local\gradeitem\advancedgrading_mapping; + + /** + * Valid class for testing mappings. + * + * @package core_grades + * @category test + * @copyright 2019 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + class gradeitems implements itemnumber_mapping, advancedgrading_mapping { + /** + * Get the grade item mapping of item number to item name. + * + * @return array + */ + public static function get_itemname_mapping_for_component(): array { + return [ + 0 => 'rating', + 1 => 'someother', + ]; + } + + /** + * Get the list of items which define advanced grading. + * + * @return array + */ + public static function get_advancedgrading_itemnames(): array { + return [ + 'someother', + ]; + } + } +} + +namespace tests\core_grades\component_gradeitems\invalid\grades { + use core_grades\local\gradeitem\itemnumber_mapping; + + /** + * Invalid class for testing mappings. + * + * @package core_grades + * @category test + * @copyright 2019 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + class gradeitems { + /** + * Get the grade item mapping of item number to item name. + * + * @return array + */ + public static function get_itemname_mapping_for_component(): array { + return [ + 0 => 'rating', + 1 => 'someother', + ]; + } + + /** + * Get the list of items which define advanced grading. + * + * @return array + */ + public static function get_advancedgrading_itemnames(): array { + return [ + 1 => 'someother', + ]; + } + } +} diff --git a/grade/tests/coverage.php b/grade/tests/coverage.php new file mode 100644 index 00000000000..fa3128d7130 --- /dev/null +++ b/grade/tests/coverage.php @@ -0,0 +1,39 @@ +. + +defined('MOODLE_INTERNAL') || die(); + +/** + * Coverage information for the grades component. + * + * @package grades + * @category phpunit + * @copyright 2019 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +/** + * Coverage information for the core subsystem. + * + * @copyright 2019 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +return new class extends phpunit_coverage_info { + /** @var array The list of folders relative to the plugin root to whitelist in coverage generation. */ + protected $whitelistfolders = [ + 'classes/local', + ]; +}; diff --git a/mod/upgrade.txt b/mod/upgrade.txt index ea4dbb28e54..f81cbfe1848 100644 --- a/mod/upgrade.txt +++ b/mod/upgrade.txt @@ -4,6 +4,9 @@ information provided here is intended especially for developers. === 3.8 === * The final deprecation of xxx_print_overview() callback means that this function will no longer be called. +* Activities which define multiple grade items must now describe the mapping of the gradeitem's itemnumber to a + meaningful name in a class implementing \core_grades\local\gradeitem\itemnumber_mapping located in + \mod_name\grades\gradeitems (located in mod/[mod_name]/classes/grades/gradeitems.php). === 3.6 === -- 2.17.1