diff --git a/admin/tool/behat/tests/behat_form_text_test.php b/admin/tool/behat/tests/behat_form_text_test.php new file mode 100644 index 00000000000..0823cbac758 --- /dev/null +++ b/admin/tool/behat/tests/behat_form_text_test.php @@ -0,0 +1,191 @@ +. + +/** + * Tests for behat_form_text class + * + * @copyright 2022 onwards Eloy Lafuente (stronk7) {@link https://stronk7.com} + * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace tool_behat; + +use behat_form_text; +use Behat\Mink\Session; +use Behat\Mink\Element\NodeElement; +use core_string_manager_standard; + +defined('MOODLE_INTERNAL') || die; + +global $CFG; +require_once($CFG->libdir . '/behat/classes/behat_session_interface.php'); +require_once($CFG->libdir . '/behat/classes/behat_session_trait.php'); +require_once($CFG->libdir . '/behat/form_field/behat_form_text.php'); + +/** + * Tests for the behat_form_text class + * + * @package tool_behat + * @category test + * @copyright 2022 onwards Eloy Lafuente (stronk7) {@link https://stronk7.com} + * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * + * @covers \behat_form_text + * @covers \behat_form_field + */ +class behat_form_text_test extends \basic_testcase { + + /** + * Data provider for the test_set_get_value() method. + * + * @return array of value and expectation pairs to be tested. + */ + public function provider_test_set_get_value() { + return [ + 'null' => [null, null], + 'int' => [3, 3], + 'float' => [3.14, 3.14], + 'string' => ['hello', 'hello'], + 'utf8' => ['你好', '你好'], + ]; + } + + /** + * Test the set_value() and get_value() methods. + * + * @param mixed $value value to be set. + * @param mixed $expectation value to be checked. + * @dataProvider provider_test_set_get_value() + */ + public function test_set_get_value($value, $expectation) { + $session = $this->createMock(Session::class); + $node = $this->createMock(NodeElement::class); + $node->method('getValue')->willReturn($value); + $field = new behat_form_text($session, $node); + + $field->set_value($value); + $this->assertEquals($expectation, $field->get_value()); + } + + /** + * Data provider for the test_text_matches() method. + * + * @return array of decsep, value, match and result pairs to be tested. + */ + public function provider_test_matches() { + return [ + 'lazy true' => ['.', 'hello', 'hello', true], + 'lazy false' => ['.', 'hello', 'bye', false], + 'float true' => ['.', '3.14', '3.1400', true], + 'float false' => ['.', '3.14', '3.1401', false], + 'float and float string true' => ['.', 3.14, '3.1400', true], + 'float and unrelated string false' => ['.', 3.14, 'hello', false], + 'float hash decsep true' => ['#', '3#14', '3#1400', true], + 'float hash decsep false' => ['#', '3#14', '3#1401', false], + 'float and float string hash decsep true' => ['#', 3.14, '3.1400', true], + 'float and unrelated string hash decsep false' => ['#', 3.14, 'hello', false], + 'float custom-default decsep mix1 true' => ['#', '3#14', '3.1400', true], + 'float custom-default decsep mix2 true' => ['#', '3.14', '3#1400', true], + 'float 2-custom decsep mix1 false' => ['#', '3#14', '3,1400', false], + 'float 2-custom decsep mix2 false' => [',', '3#14', '3,1400', false], + 'float default-custom decsep mix1 false' => ['.', '3#14', '3.1400', false], + 'float default-custom decsep mix2 false' => ['.', '3.14', '3#1400', false], + ]; + } + + /** + * Test the matches() method. + * + * @param string $decsep decimal separator to use. + * @param mixed $value value to be set. + * @param mixed $match value to be matched. + * @param bool $result expected return status of the function. + * @dataProvider provider_test_matches() + */ + public function test_matches($decsep, $value, $match, $result) { + global $CFG; + + // Switch of string manager to avoid having to (slow) customise the lang file. + $origcustom = $CFG->config_php_settings['customstringmanager'] ?? null; + $CFG->config_php_settings['customstringmanager'] = '\tool_behat\phpunit_string_manager'; + $manager = get_string_manager(true); + $manager->set_string('decsep', 'langconfig', $decsep); + + $session = $this->createMock(Session::class); + $node = $this->createMock(NodeElement::class); + $node->method('getValue')->willReturn($value); + + $field = new behat_form_text($session, $node); + + $field->set_value($value); + $this->assertSame($result, $field->matches($match)); + + // Switch back to the original string manager. + if (is_null($origcustom)) { + unset($CFG->config_php_settings['customstringmanager']); + } else { + $CFG->config_php_settings['customstringmanager'] = $origcustom; + } + $manager = get_string_manager(true); + } +} + +/** + * Customised values that will be used instead of standard manager one. + * + * If an existing component/identifier is found, return it instead of the real + * one from language files. Note this doesn't support place holders or another niceties. + * + * @package tool_behat + * @category test + * @copyright 2022 onwards Eloy Lafuente (stronk7) {@link https://stronk7.com} + * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class phpunit_string_manager extends core_string_manager_standard { + + /** @var array language customisations provided by the manager without asking for real contents */ + protected $customstrings = []; + + /** + * Get String returns a requested string + * + * @param string $identifier The identifier of the string to search for + * @param string $component The module the string is associated with + * @param string|object|array $a An object, string or number that can be used + * within translation strings + * @param string $lang moodle translation language, null means use current + * @return string The String ! + */ + public function get_string($identifier, $component = '', $a = null, $lang = null) { + $key = trim($component) . '/' . trim($identifier); + if (isset($this->customstrings[$key])) { + return $this->customstrings[$key]; + } + return parent::get_string($identifier, $component, $a, $lang); + } + + /** + * Sets a custom string to be returned by the string manager instead of the language file one. + * + * @param string $identifier The identifier of the string to search for + * @param string $component The module the string is associated with + * @param string $value the contents of the language string to be returned by get_string() + */ + public function set_string($identifier, $component, $value) { + $key = trim($component) . '/' . trim($identifier); + $this->customstrings[$key] = $value; + } +} diff --git a/grade/classes/component_gradeitem.php b/grade/classes/component_gradeitem.php index 060e5b89776..8aea26bf02a 100644 --- a/grade/classes/component_gradeitem.php +++ b/grade/classes/component_gradeitem.php @@ -405,6 +405,7 @@ abstract class component_gradeitem { if ($grade = $this->get_grade_for_user($gradeduser, $grader)) { $gradeitem = $this->get_grade_item(); if (!$this->is_using_scale()) { + $grade->grade = !is_null($grade->grade) ? (float)$grade->grade : null; // Cast non-null values, keeping nulls. $grade->usergrade = grade_format_gradevalue($grade->grade, $gradeitem); $grade->maxgrade = format_float($gradeitem->grademax, $gradeitem->get_decimals()); // If displaying the raw grade, also display the total value. diff --git a/grade/edit/letter/edit_form.php b/grade/edit/letter/edit_form.php index 6c823da116b..48254ba17be 100644 --- a/grade/edit/letter/edit_form.php +++ b/grade/edit/letter/edit_form.php @@ -59,12 +59,10 @@ class edit_letter_form extends moodleform { } $entry[] = $mform->createElement('static', '', '', '≥'); - $entry[] = $mform->createElement('text', $gradeboundaryname, $gradeboundary." $i"); + $entry[] = $mform->createElement('float', $gradeboundaryname, $gradeboundary." $i"); $entry[] = $mform->createElement('static', '', '', '%'); $mform->addGroup($entry, 'gradeentry'.$i, $gradeletter." $i", array(' '), false); - $mform->setType($gradeboundaryname, PARAM_FLOAT); - if (!$admin) { $mform->disabledIf($gradeboundaryname, 'override', 'notchecked'); } diff --git a/grade/edit/tree/category.php b/grade/edit/tree/category.php index 699e02c2868..f9f1453c1af 100644 --- a/grade/edit/tree/category.php +++ b/grade/edit/tree/category.php @@ -133,7 +133,7 @@ $mform = new edit_category_form(null, array('current'=>$category, 'gpr'=>$gpr)); if ($mform->is_cancelled()) { redirect($returnurl); -} else if ($data = $mform->get_data(false)) { +} else if ($data = $mform->get_data()) { grade_edit_tree::update_gradecategory($grade_category, $data); redirect($returnurl); } diff --git a/grade/edit/tree/category_form.php b/grade/edit/tree/category_form.php index ef6ada4b10b..f9705d272c0 100644 --- a/grade/edit/tree/category_form.php +++ b/grade/edit/tree/category_form.php @@ -167,22 +167,19 @@ class edit_category_form extends moodleform { $mform->addHelpButton('grade_item_rescalegrades', 'modgradecategoryrescalegrades', 'grades'); $mform->disabledIf('grade_item_rescalegrades', 'grade_item_gradetype', 'noteq', GRADE_TYPE_VALUE); - $mform->addElement('text', 'grade_item_grademax', get_string('grademax', 'grades')); - $mform->setType('grade_item_grademax', PARAM_RAW); + $mform->addElement('float', 'grade_item_grademax', get_string('grademax', 'grades')); $mform->addHelpButton('grade_item_grademax', 'grademax', 'grades'); $mform->disabledIf('grade_item_grademax', 'grade_item_gradetype', 'noteq', GRADE_TYPE_VALUE); $mform->disabledIf('grade_item_grademax', 'aggregation', 'eq', GRADE_AGGREGATE_SUM); if ((bool) get_config('moodle', 'grade_report_showmin')) { - $mform->addElement('text', 'grade_item_grademin', get_string('grademin', 'grades')); - $mform->setType('grade_item_grademin', PARAM_RAW); + $mform->addElement('float', 'grade_item_grademin', get_string('grademin', 'grades')); $mform->addHelpButton('grade_item_grademin', 'grademin', 'grades'); $mform->disabledIf('grade_item_grademin', 'grade_item_gradetype', 'noteq', GRADE_TYPE_VALUE); $mform->disabledIf('grade_item_grademin', 'aggregation', 'eq', GRADE_AGGREGATE_SUM); } - $mform->addElement('text', 'grade_item_gradepass', get_string('gradepass', 'grades')); - $mform->setType('grade_item_gradepass', PARAM_RAW); + $mform->addElement('float', 'grade_item_gradepass', get_string('gradepass', 'grades')); $mform->addHelpButton('grade_item_gradepass', 'gradepass', 'grades'); $mform->disabledIf('grade_item_gradepass', 'grade_item_gradetype', 'eq', GRADE_TYPE_NONE); $mform->disabledIf('grade_item_gradepass', 'grade_item_gradetype', 'eq', GRADE_TYPE_TEXT); @@ -247,9 +244,8 @@ class edit_category_form extends moodleform { $mform->addElement('advcheckbox', 'grade_item_weightoverride', get_string('adjustedweight', 'grades')); $mform->addHelpButton('grade_item_weightoverride', 'weightoverride', 'grades'); - $mform->addElement('text', 'grade_item_aggregationcoef2', get_string('weight', 'grades')); + $mform->addElement('float', 'grade_item_aggregationcoef2', get_string('weight', 'grades')); $mform->addHelpButton('grade_item_aggregationcoef2', 'weight', 'grades'); - $mform->setType('grade_item_aggregationcoef2', PARAM_RAW); $mform->disabledIf('grade_item_aggregationcoef2', 'grade_item_weightoverride'); $options = array(); @@ -549,19 +545,28 @@ class edit_category_form extends moodleform { $errors['grade_item_scaleid'] = get_string('missingscale', 'grades'); } } - if (array_key_exists('grade_item_grademin', $data) and array_key_exists('grade_item_grademax', $data)) { - if (($data['grade_item_grademax'] != 0 OR $data['grade_item_grademin'] != 0) AND - ($data['grade_item_grademax'] == $data['grade_item_grademin'] OR - $data['grade_item_grademax'] < $data['grade_item_grademin'])) { - $errors['grade_item_grademin'] = get_string('incorrectminmax', 'grades'); - $errors['grade_item_grademax'] = get_string('incorrectminmax', 'grades'); - } + + // We need to make all the validations related with grademax and grademin + // with them being correct floats, keeping the originals unmodified for + // later validations / showing the form back... + // TODO: Note that once MDL-73994 is fixed we'll have to re-visit this and + // adapt the code below to the new values arriving here, without forgetting + // the special case of empties and nulls. + $grademax = isset($data['grade_item_grademax']) ? unformat_float($data['grade_item_grademax']) : null; + $grademin = isset($data['grade_item_grademin']) ? unformat_float($data['grade_item_grademin']) : null; + + if (!is_null($grademin) and !is_null($grademax)) { + if (($grademax != 0 OR $grademin != 0) AND + ($grademax == $grademin OR $grademax < $grademin)) { + $errors['grade_item_grademin'] = get_string('incorrectminmax', 'grades'); + $errors['grade_item_grademax'] = get_string('incorrectminmax', 'grades'); + } } if ($data['id'] && $gradeitem->has_overridden_grades()) { if ($gradeitem->gradetype == GRADE_TYPE_VALUE) { - if (grade_floats_different($data['grade_item_grademin'], $gradeitem->grademin) || - grade_floats_different($data['grade_item_grademax'], $gradeitem->grademax)) { + if (grade_floats_different($grademin, $gradeitem->grademin) || + grade_floats_different($grademax, $gradeitem->grademax)) { if (empty($data['grade_item_rescalegrades'])) { $errors['grade_item_rescalegrades'] = get_string('mustchooserescaleyesorno', 'grades'); } @@ -571,5 +576,3 @@ class edit_category_form extends moodleform { return $errors; } } - - diff --git a/grade/edit/tree/item.php b/grade/edit/tree/item.php index 45757d25377..3f6ec64ffc5 100644 --- a/grade/edit/tree/item.php +++ b/grade/edit/tree/item.php @@ -109,7 +109,7 @@ $mform = new edit_item_form(null, array('current'=>$item, 'gpr'=>$gpr)); if ($mform->is_cancelled()) { redirect($returnurl); -} else if ($data = $mform->get_data(false)) { +} else if ($data = $mform->get_data()) { // This is a new item, and the category chosen is different than the default category. if (empty($grade_item->id) && isset($data->parentcategory) && $parent_category->id != $data->parentcategory) { diff --git a/grade/edit/tree/item_form.php b/grade/edit/tree/item_form.php index fe5f58aadd0..4483c4d8d9a 100644 --- a/grade/edit/tree/item_form.php +++ b/grade/edit/tree/item_form.php @@ -110,37 +110,32 @@ class edit_item_form extends moodleform { $mform->addHelpButton('rescalegrades', 'modgraderescalegrades', 'grades'); $mform->disabledIf('rescalegrades', 'gradetype', 'noteq', GRADE_TYPE_VALUE); - $mform->addElement('text', 'grademax', get_string('grademax', 'grades')); + $mform->addElement('float', 'grademax', get_string('grademax', 'grades')); $mform->addHelpButton('grademax', 'grademax', 'grades'); $mform->disabledIf('grademax', 'gradetype', 'noteq', GRADE_TYPE_VALUE); - $mform->setType('grademax', PARAM_RAW); if ((bool) get_config('moodle', 'grade_report_showmin')) { - $mform->addElement('text', 'grademin', get_string('grademin', 'grades')); + $mform->addElement('float', 'grademin', get_string('grademin', 'grades')); $mform->addHelpButton('grademin', 'grademin', 'grades'); $mform->disabledIf('grademin', 'gradetype', 'noteq', GRADE_TYPE_VALUE); - $mform->setType('grademin', PARAM_RAW); } - $mform->addElement('text', 'gradepass', get_string('gradepass', 'grades')); + $mform->addElement('float', 'gradepass', get_string('gradepass', 'grades')); $mform->addHelpButton('gradepass', 'gradepass', 'grades'); $mform->disabledIf('gradepass', 'gradetype', 'eq', GRADE_TYPE_NONE); $mform->disabledIf('gradepass', 'gradetype', 'eq', GRADE_TYPE_TEXT); - $mform->setType('gradepass', PARAM_RAW); - $mform->addElement('text', 'multfactor', get_string('multfactor', 'grades')); + $mform->addElement('float', 'multfactor', get_string('multfactor', 'grades')); $mform->addHelpButton('multfactor', 'multfactor', 'grades'); $mform->setAdvanced('multfactor'); $mform->disabledIf('multfactor', 'gradetype', 'eq', GRADE_TYPE_NONE); $mform->disabledIf('multfactor', 'gradetype', 'eq', GRADE_TYPE_TEXT); - $mform->setType('multfactor', PARAM_RAW); - $mform->addElement('text', 'plusfactor', get_string('plusfactor', 'grades')); + $mform->addElement('float', 'plusfactor', get_string('plusfactor', 'grades')); $mform->addHelpButton('plusfactor', 'plusfactor', 'grades'); $mform->setAdvanced('plusfactor'); $mform->disabledIf('plusfactor', 'gradetype', 'eq', GRADE_TYPE_NONE); $mform->disabledIf('plusfactor', 'gradetype', 'eq', GRADE_TYPE_TEXT); - $mform->setType('plusfactor', PARAM_RAW); /// grade display prefs $default_gradedisplaytype = grade_get_setting($COURSE->id, 'displaytype', $CFG->grade_displaytype); @@ -207,9 +202,8 @@ class edit_item_form extends moodleform { $mform->disabledIf('weightoverride', 'gradetype', 'eq', GRADE_TYPE_NONE); $mform->disabledIf('weightoverride', 'gradetype', 'eq', GRADE_TYPE_TEXT); - $mform->addElement('text', 'aggregationcoef2', get_string('weight', 'grades')); + $mform->addElement('float', 'aggregationcoef2', get_string('weight', 'grades')); $mform->addHelpButton('aggregationcoef2', 'weight', 'grades'); - $mform->setType('aggregationcoef2', PARAM_RAW); $mform->disabledIf('aggregationcoef2', 'weightoverride'); $mform->disabledIf('aggregationcoef2', 'gradetype', 'eq', GRADE_TYPE_NONE); $mform->disabledIf('aggregationcoef2', 'gradetype', 'eq', GRADE_TYPE_TEXT); @@ -442,8 +436,17 @@ class edit_item_form extends moodleform { } } - if (array_key_exists('grademin', $data) and array_key_exists('grademax', $data)) { - if ($data['grademax'] == $data['grademin'] or $data['grademax'] < $data['grademin']) { + // We need to make all the validations related with grademax and grademin + // with them being correct floats, keeping the originals unmodified for + // later validations / showing the form back... + // TODO: Note that once MDL-73994 is fixed we'll have to re-visit this and + // adapt the code below to the new values arriving here, without forgetting + // the special case of empties and nulls. + $grademax = isset($data['grademax']) ? unformat_float($data['grademax']) : null; + $grademin = isset($data['grademin']) ? unformat_float($data['grademin']) : null; + + if (!is_null($grademin) and !is_null($grademax)) { + if ($grademax == $grademin or $grademax < $grademin) { $errors['grademin'] = get_string('incorrectminmax', 'grades'); $errors['grademax'] = get_string('incorrectminmax', 'grades'); } @@ -466,8 +469,8 @@ class edit_item_form extends moodleform { if ($grade_item) { if ($grade_item->gradetype == GRADE_TYPE_VALUE) { if ((((bool) get_config('moodle', 'grade_report_showmin')) && - grade_floats_different($data['grademin'], $grade_item->grademin)) || - grade_floats_different($data['grademax'], $grade_item->grademax)) { + grade_floats_different($grademin, $grade_item->grademin)) || + grade_floats_different($grademax, $grade_item->grademax)) { if ($grade_item->has_grades() && empty($data['rescalegrades'])) { $errors['rescalegrades'] = get_string('mustchooserescaleyesorno', 'grades'); } diff --git a/grade/report/singleview/classes/local/ui/finalgrade.php b/grade/report/singleview/classes/local/ui/finalgrade.php index a3e19e1416e..9b9e9b3e9ea 100644 --- a/grade/report/singleview/classes/local/ui/finalgrade.php +++ b/grade/report/singleview/classes/local/ui/finalgrade.php @@ -141,6 +141,7 @@ class finalgrade extends grade_attribute_format implements unique_value, be_disa $feedback = false; $feedbackformat = false; if ($gradeitem->gradetype == GRADE_TYPE_SCALE) { + $value = (int)unformat_float($value); if ($value == -1) { $finalgrade = null; } else { diff --git a/grade/report/singleview/tests/behat/singleview.feature b/grade/report/singleview/tests/behat/singleview.feature index d266c61f07a..a4388a58a4b 100644 --- a/grade/report/singleview/tests/behat/singleview.feature +++ b/grade/report/singleview/tests/behat/singleview.feature @@ -117,6 +117,24 @@ Feature: We can use Single view And I press "Save" Then I should see "Grades were set for 6 items" + Scenario: I can bulk update grades with custom decimal separator + Given the following "language customisations" exist: + | component | stringid | value | + | core_langconfig | decsep | # | + And I follow "Single view for Ann, Jill, Grainne, Beauchamp" + And I should see "Gronya,Beecham" + When I set the field "For" to "All grades" + And I set the field "Insert value" to "1#25" + And I set the field "Perform bulk insert" to "1" + And I press "Save" + Then I should see "Grades were set for 6 items" + # Custome scale, cast to int + And the field "Grade for new grade item 1" matches value "Disappointing" + # Value grade, float with custom decsep. + And the field "Grade for Test assignment one" matches value "1#25" + # Numerical scale, cast to int, showing as float with custom decsep. + And the field "Grade for Test grade item" matches value "1#00" + Scenario: Navigation works in the Single view. Given I follow "Single view for Ann, Jill, Grainne, Beauchamp" Then I should see "Gronya,Beecham" diff --git a/grade/tests/behat/grade_category_validation.feature b/grade/tests/behat/grade_category_validation.feature index b7da9640ed9..b11e533b862 100644 --- a/grade/tests/behat/grade_category_validation.feature +++ b/grade/tests/behat/grade_category_validation.feature @@ -101,3 +101,21 @@ Feature: Editing a grade item And I set the field "Maximum grade" to "50" When I press "Save changes" Then I should see "You must choose whether to rescale existing grades or not." + + Scenario: Perform changes to a grade category with custom decimal separator + Given the following "language customisations" exist: + | component | stringid | value | + | core_langconfig | decsep | # | + And I navigate to "View > Grader report" in the course gradebook + And I turn editing mode on + And I give the grade "20#00" to the user "Student 1" for the grade item "EN Cat 1 total" + And I press "Save changes" + And I navigate to "Setup > Gradebook setup" in the course gradebook + And I click on "Edit settings" "link" in the "EN Cat 1" "table_row" + And I set the field "Rescale overridden grades" to "Yes" + And I set the field "Maximum grade" to "87#50" + When I press "Save changes" + And I navigate to "View > Grader report" in the course gradebook + And I follow "Single view for Student 1" + Then I should see "Student 1" + And the field "Grade for Category total" matches value "17#50" diff --git a/grade/tests/behat/grade_override_letter.feature b/grade/tests/behat/grade_override_letter.feature index 55a91c9e3d8..d21ae91edb6 100644 --- a/grade/tests/behat/grade_override_letter.feature +++ b/grade/tests/behat/grade_override_letter.feature @@ -69,6 +69,47 @@ Feature: Grade letters can be overridden | | | | | | | A | 95.25 | B | 76.75 | C | 50.01 | D | 40 | F | 0.01 | F- | 0 | 100.00 % | 95.25 % | A | 95.24 % | 76.75 % | B | 76.74 % | 50.01 % | C | 50.00 % | 40.00 % | D | 39.99 % | 0.01 % | F | 0.00 % | 0.00 % | F- | | | | A | 95.25 | B | 76.75 | C | 50.01 | | | | | D | 40 | F | 0.01 | F- | 0 | 100.00 % | 95.25 % | A | 95.24 % | 76.75 % | B | 76.74 % | 50.01 % | C | 50.00 % | 40.00 % | D | 39.99 % | 0.01 % | F | 0.00 % | 0.00 % | F- | + Scenario Outline: Define grade letters with custom decimal separator + Given the following "language customisations" exist: + | component | stringid | value | + | core_langconfig | decsep | | + When I set the following fields to these values: + | override | 1 | + | Grade letter 1 | | + | gradeboundary1 | | + | Grade letter 2 | | + | gradeboundary2 | | + | Grade letter 3 | | + | gradeboundary3 | | + | Grade letter 4 | | + | gradeboundary4 | | + | Grade letter 5 | | + | gradeboundary5 | | + | Grade letter 6 | | + | gradeboundary6 | | + | Grade letter 7 | | + | gradeboundary7 | | + | Grade letter 8 | | + | gradeboundary8 | | + | Grade letter 9 | | + | gradeboundary9 | | + | Grade letter 10 | | + | gradeboundary10 | | + | Grade letter 11 | | + | gradeboundary11 | | + And I press "Save changes" + Then I should see "The default grade letters are currently overridden." + And the following should exist in the "grade-letters-view" table: + | Highest | Lowest | Letter | + | | | | + | | | | + | | | | + + Examples: + | decsep | l1 | b1 | l2 | b2 | l3 | b3 | high1 | low1 | high2 | low2 | high3 | low3 | + | . | A | 88.88 | B | 50.00 | C | 0.00 | 100.00 % | 88.88 % | 88.87 % | 50.00 % | 49.99 % | 0.00 % | + | # | A | 88#88 | B | 50#00 | C | 0#00 | 100#00 % | 88#88 % | 88#87 % | 50#00 % | 49#99 % | 0#00 % | + Scenario: I delete a grade letter Given I set the following fields to these values: | override | 1 | diff --git a/grade/tests/behat/grade_to_pass.feature b/grade/tests/behat/grade_to_pass.feature index 79acf830ced..46d8f65d0aa 100644 --- a/grade/tests/behat/grade_to_pass.feature +++ b/grade/tests/behat/grade_to_pass.feature @@ -199,6 +199,30 @@ Feature: We can set the grade to pass value And I follow "Settings" And the field "Grade to pass" matches value "80" + Scenario: Set a valid grade to pass for lesson activity with custom decimal separator + Given the following "activities" exist: + | activity | name | intro | course | section | idnumber | + | lesson | Test Lesson 1 | Test | C1 | 1 | lesson1 | + And the following "language customisations" exist: + | component | stringid | value | + | core_langconfig | decsep | # | + And I am on "Course 1" course homepage with editing mode on + And I follow "Test Lesson 1" + And I navigate to "Settings" in current page administration + And I set the following fields to these values: + | Grade to pass | 90#50 | + And I press "Save and return to course" + And I navigate to "View > Grader report" in the course gradebook + And I click on "Edit lesson Test Lesson 1" "link" + And I expand all fieldsets + Then the field "Grade to pass" matches value "90#50" + And I set the field "Grade to pass" to "80" + And I press "Save changes" + And I am on "Course 1" course homepage + And I follow "Test Lesson 1" + And I follow "Settings" + And the field "Grade to pass" matches value "80#00" + Scenario: Set a valid grade to pass for database activity Given the following "activities" exist: | activity | name | intro | course | section | idnumber | diff --git a/lib/behat/classes/behat_core_generator.php b/lib/behat/classes/behat_core_generator.php index c39bf93099a..02ad43e3730 100644 --- a/lib/behat/classes/behat_core_generator.php +++ b/lib/behat/classes/behat_core_generator.php @@ -362,6 +362,22 @@ class behat_core_generator extends behat_generator_base { $data['categoryid'] = $cat->id; } + // We need to ensure that all these attributes coming from data are not-localised floats. + $attrs = [ + 'grademax', + 'grademin', + 'gradepass', + 'multfactor', + 'plusfactor', + 'aggregationcoef', + 'aggregationcoef2', + ]; + foreach ($attrs as $attr) { + if (array_key_exists($attr, $data)) { + $data[$attr] = unformat_float($data[$attr]); + } + } + return $data; } diff --git a/lib/behat/form_field/behat_form_field.php b/lib/behat/form_field/behat_form_field.php index 620b238b212..c834b7bdb35 100644 --- a/lib/behat/form_field/behat_form_field.php +++ b/lib/behat/form_field/behat_form_field.php @@ -247,10 +247,23 @@ class behat_form_field implements behat_session_interface { * @return bool */ protected function text_matches($expectedvalue) { - if (trim($expectedvalue) != trim($this->get_value())) { - return false; + // Non strict string comparison. + if (trim($expectedvalue) == trim($this->get_value())) { + return true; } - return true; + + // Do one more matching attempt for floats that are valid with current decsep in use + // (let's continue non strict comparing them as strings, but once unformatted). + $expectedfloat = unformat_float(trim($expectedvalue), true); + $actualfloat = unformat_float(trim($this->get_value()), true); + // If they aren't null or false, then we are good to be compared (basically is_numeric()). + $goodfloats = !is_null($expectedfloat) && ($expectedfloat !== false) && + !is_null($actualfloat) && ($actualfloat !== false); + if ($goodfloats && ((string)$expectedfloat == (string)$actualfloat)) { + return true; + } + + return false; } /** diff --git a/lib/gradelib.php b/lib/gradelib.php index daea2b4754f..91c3f680c61 100644 --- a/lib/gradelib.php +++ b/lib/gradelib.php @@ -759,14 +759,14 @@ function grade_set_setting($courseid, $name, $value) { /** * Returns string representation of grade value * - * @param float $value The grade value + * @param float|null $value The grade value * @param object $grade_item Grade item object passed by reference to prevent scale reloading * @param bool $localized use localised decimal separator * @param int $displaytype type of display. For example GRADE_DISPLAY_TYPE_REAL, GRADE_DISPLAY_TYPE_PERCENTAGE, GRADE_DISPLAY_TYPE_LETTER * @param int $decimals The number of decimal places when displaying float values * @return string */ -function grade_format_gradevalue($value, &$grade_item, $localized=true, $displaytype=null, $decimals=null) { +function grade_format_gradevalue(?float $value, &$grade_item, $localized=true, $displaytype=null, $decimals=null) { if ($grade_item->gradetype == GRADE_TYPE_NONE or $grade_item->gradetype == GRADE_TYPE_TEXT) { return ''; } @@ -830,13 +830,13 @@ function grade_format_gradevalue($value, &$grade_item, $localized=true, $display /** * Returns a float representation of a grade value * - * @param float $value The grade value + * @param float|null $value The grade value * @param object $grade_item Grade item object * @param int $decimals The number of decimal places * @param bool $localized use localised decimal separator * @return string */ -function grade_format_gradevalue_real($value, $grade_item, $decimals, $localized) { +function grade_format_gradevalue_real(?float $value, $grade_item, $decimals, $localized) { if ($grade_item->gradetype == GRADE_TYPE_SCALE) { if (!$scale = $grade_item->load_scale()) { return get_string('error'); @@ -853,13 +853,13 @@ function grade_format_gradevalue_real($value, $grade_item, $decimals, $localized /** * Returns a percentage representation of a grade value * - * @param float $value The grade value + * @param float|null $value The grade value * @param object $grade_item Grade item object * @param int $decimals The number of decimal places * @param bool $localized use localised decimal separator * @return string */ -function grade_format_gradevalue_percentage($value, $grade_item, $decimals, $localized) { +function grade_format_gradevalue_percentage(?float $value, $grade_item, $decimals, $localized) { $min = $grade_item->grademin; $max = $grade_item->grademax; if ($min == $max) { @@ -874,11 +874,11 @@ function grade_format_gradevalue_percentage($value, $grade_item, $decimals, $loc * Returns a letter grade representation of a grade value * The array of grade letters used is produced by {@link grade_get_letters()} using the course context * - * @param float $value The grade value + * @param float|null $value The grade value * @param object $grade_item Grade item object * @return string */ -function grade_format_gradevalue_letter($value, $grade_item) { +function grade_format_gradevalue_letter(?float $value, $grade_item) { global $CFG; $context = context_course::instance($grade_item->courseid, IGNORE_MISSING); if (!$letters = grade_get_letters($context)) { @@ -1587,10 +1587,10 @@ function grade_course_reset($courseid) { * Convert a number to 5 decimal point float, an empty string or a null db compatible format * (we need this to decide if db value changed) * - * @param mixed $number The number to convert - * @return mixed float or null + * @param float|null $number The number to convert + * @return float|null float or null */ -function grade_floatval($number) { +function grade_floatval(?float $number) { if (is_null($number) or $number === '') { return null; } @@ -1603,11 +1603,11 @@ function grade_floatval($number) { * Compare two float numbers safely. Uses 5 decimals php precision using {@link grade_floatval()}. Nulls accepted too. * Used for determining if a database update is required * - * @param float $f1 Float one to compare - * @param float $f2 Float two to compare + * @param float|null $f1 Float one to compare + * @param float|null $f2 Float two to compare * @return bool True if the supplied values are different */ -function grade_floats_different($f1, $f2) { +function grade_floats_different(?float $f1, ?float $f2): bool { // note: db rounding for 10,5 is different from php round() function return (grade_floatval($f1) !== grade_floatval($f2)); } @@ -1619,11 +1619,11 @@ function grade_floats_different($f1, $f2) { * different from php round() function. * * @since Moodle 2.0 - * @param float $f1 Float one to compare - * @param float $f2 Float two to compare + * @param float|null $f1 Float one to compare + * @param float|null $f2 Float two to compare * @return bool True if the values should be considered as the same grades */ -function grade_floats_equal($f1, $f2) { +function grade_floats_equal(?float $f1, ?float $f2): bool { return (grade_floatval($f1) === grade_floatval($f2)); } diff --git a/lib/upgrade.txt b/lib/upgrade.txt index fbdce39d287..70050362a18 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -2,6 +2,16 @@ This files describes API changes in core libraries and APIs, information provided here is intended especially for developers. === 4.0 === +* To better detect wrong floats (like, for example, unformatted, using local-dependent separators ones) a number of + gradebook functions now have stricter float type checking. All them will require now the "float" being passed to be + a correct float value (numeric or string). Usually, that's achieved by using unformat_float() or + PARAM_LOCALISEDFLOAT for all the user-entered grades before any processing on them. Functions affected are: + - grade_format_gradevalue(), $value param (keeping it as optional/nullable). + - grade_format_gradevalue_real(), $value param (keeping it as optional/nullable). + - grade_format_gradevalue_percentage(), $value param (keeping it as optional/nullable). + - grade_format_gradevalue_letter(), $value param (keeping it as optional/nullable). + - grade_floats_different(), $f1 and $f2 params (keeping them as optional/nullable). + - grade_floats_equal(), $f1 and $f2 params (keeping them as optional/nullable). * The method action_menu->set_alignment() has been deprecated, please use action_menu->set_menu_left if you need a dropdown to align to the left of the dropdown button. * The $OUTPUT->should_display_main_logo() function has been deprecated and should no longer be used. diff --git a/mod/lesson/locallib.php b/mod/lesson/locallib.php index d9b7d858bd7..f3a1e15d731 100644 --- a/mod/lesson/locallib.php +++ b/mod/lesson/locallib.php @@ -3162,7 +3162,7 @@ class lesson extends lesson_base { $this->add_message(get_string("numberofcorrectanswers", "lesson", $gradeinfo->earned), 'notify'); if ($this->properties->grade != GRADE_TYPE_NONE) { $a = new stdClass; - $a->grade = number_format($gradeinfo->grade * $this->properties->grade / 100, 1); + $a->grade = format_float($gradeinfo->grade * $this->properties->grade / 100, 1); $a->total = $this->properties->grade; $this->add_message(get_string('yourcurrentgradeisoutof', 'lesson', $a), 'notify'); } @@ -3596,7 +3596,7 @@ class lesson extends lesson_base { } if ($this->properties->grade != GRADE_TYPE_NONE) { $a = new stdClass; - $a->grade = number_format($gradeinfo->grade * $this->properties->grade / 100, 1); + $a->grade = format_float($gradeinfo->grade * $this->properties->grade / 100, 1); $a->total = $this->properties->grade; $data->yourcurrentgradeisoutof = $a; } diff --git a/mod/lesson/tests/behat/lesson_informations_at_end.feature b/mod/lesson/tests/behat/lesson_informations_at_end.feature index cdbcfb3e773..abc5e7effd8 100644 --- a/mod/lesson/tests/behat/lesson_informations_at_end.feature +++ b/mod/lesson/tests/behat/lesson_informations_at_end.feature @@ -2,7 +2,7 @@ Feature: In a lesson activity, if custom scoring is not enabled, student should see some informations at the end of lesson: questions answered, correct answers, grade, score - Scenario: Informations at end of lesson if custom scoring not enabled + Background: Given the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@example.com | @@ -18,11 +18,10 @@ Feature: In a lesson activity, if custom scoring is not enabled, student should | activity | name | intro | course | section | idnumber | | lesson | Test lesson name | Test lesson description | C1 | 1 | lesson1 | And I log in as "teacher1" - And I am on "Course 1" course homepage - And I follow "Test lesson name" - And I navigate to "Settings" in current page administration + And I am on the "Test lesson name" "lesson activity editing" page And I set the following fields to these values: - | Custom scoring | No | + | Maximum grade | 75 | + | Custom scoring | No | And I press "Save and return to course" And I follow "Test lesson name" And I follow "Add a content page" @@ -47,18 +46,40 @@ Feature: In a lesson activity, if custom scoring is not enabled, student should And I press "Save page" And I log out And I log in as "student1" - And I am on "Course 1" course homepage - When I follow "Test lesson name" - Then I should see "First page contents" - And I press "Next page" + + Scenario: Informations at end of lesson if custom scoring not enabled + Given I am on "Course 1" course homepage + And I follow "Test lesson name" + And I should see "First page contents" + When I press "Next page" And I should see "1 + 1?" And I set the following fields to these values: | Your answer | 1 | And I press "Submit" And I should see "Incorrect answer" And I press "Continue" - And I should see "Congratulations - end of lesson reached" + Then I should see "Congratulations - end of lesson reached" And I should see "Number of questions answered: 1" And I should see "Number of correct answers: 0" And I should see "Your score is 0 (out of 1)." - And I should see "Your current grade is 0.0 out of 100" + And I should see "Your current grade is 0.0 out of 75" + + Scenario: Informations at end of lesson if custom scoring not enabled with custom decimal separator + Given the following "language customisations" exist: + | component | stringid | value | + | core_langconfig | decsep | # | + And I am on "Course 1" course homepage + And I follow "Test lesson name" + And I should see "First page contents" + When I press "Next page" + And I should see "1 + 1?" + And I set the following fields to these values: + | Your answer | 1 | + And I press "Submit" + And I should see "Incorrect answer" + And I press "Continue" + Then I should see "Congratulations - end of lesson reached" + And I should see "Number of questions answered: 1" + And I should see "Number of correct answers: 0" + And I should see "Your score is 0 (out of 1)." + And I should see "Your current grade is 0#0 out of 75" diff --git a/mod/lesson/tests/numeric_helper_test.php b/mod/lesson/tests/numeric_helper_test.php index da932920d5a..48ba9eb0f84 100644 --- a/mod/lesson/tests/numeric_helper_test.php +++ b/mod/lesson/tests/numeric_helper_test.php @@ -148,9 +148,7 @@ class mod_lesson_numeric_type_helper_test extends advanced_testcase { * It is not possible to directly change the result of get_string in * a unit test. Instead, we create a language pack for language 'xx' in * dataroot and make langconfig.php with the string we need to change. - * The default example separator used here is 'X'; on PHP 5.3 and before this - * must be a single byte character due to PHP bug/limitation in - * number_format, so you can't use UTF-8 characters. + * The default example separator used here is 'X'. * * @param string $decsep Separator character. Defaults to `'X'`. */ diff --git a/mod/quiz/tests/behat/behat_mod_quiz.php b/mod/quiz/tests/behat/behat_mod_quiz.php index 6201222d421..bdeb3ce5de0 100644 --- a/mod/quiz/tests/behat/behat_mod_quiz.php +++ b/mod/quiz/tests/behat/behat_mod_quiz.php @@ -239,8 +239,8 @@ class behat_mod_quiz extends behat_question_base { if (!array_key_exists('maxmark', $questiondata) || $questiondata['maxmark'] === '') { $maxmark = null; } else { - $maxmark = clean_param($questiondata['maxmark'], PARAM_FLOAT); - if (!is_numeric($questiondata['maxmark']) || $maxmark < 0) { + $maxmark = clean_param($questiondata['maxmark'], PARAM_LOCALISEDFLOAT); + if (!is_numeric($maxmark) || $maxmark < 0) { throw new ExpectationException('The max mark for question "' . $questiondata['question'] . '" must be a positive number.', $this->getSession()); diff --git a/mod/quiz/tests/behat/info_page.feature b/mod/quiz/tests/behat/info_page.feature index 96c34611c29..318f7768d99 100644 --- a/mod/quiz/tests/behat/info_page.feature +++ b/mod/quiz/tests/behat/info_page.feature @@ -31,6 +31,19 @@ Feature: Display of information before starting a quiz When I am on the "Quiz 1" "mod_quiz > View" page logged in as "student" Then I should see "Grade to pass: 60.00 out of 100.00" + Scenario: Check the pass grade is displayed with custom decimal separator + Given the following "language customisations" exist: + | component | stringid | value | + | core_langconfig | decsep | # | + And the following "activities" exist: + | activity | name | intro | course | idnumber | gradepass | + | quiz | Quiz 1 | Quiz 1 description | C1 | quiz1 | 60#00 | + And quiz "Quiz 1" contains the following questions: + | question | page | + | TF1 | 1 | + When I am on the "Quiz 1" "mod_quiz > View" page logged in as "student" + Then I should see "Grade to pass: 60#00 out of 100#00" + Scenario: Check the pass grade is not displayed if not set Given the following "activities" exist: | activity | name | intro | course | idnumber | gradepass | diff --git a/mod/quiz/tests/behat/preview.feature b/mod/quiz/tests/behat/preview.feature index f853ec91bf6..dde7286d05a 100644 --- a/mod/quiz/tests/behat/preview.feature +++ b/mod/quiz/tests/behat/preview.feature @@ -41,6 +41,19 @@ Feature: Preview a quiz as a teacher And I follow "Finish review" And "Review" "link" in the "Preview" "table_row" should be visible + @javascript + Scenario: Review the quiz attempt with custom decimal separator + Given the following "language customisations" exist: + | component | stringid | value | + | core_langconfig | decsep | # | + When I am on the "Quiz 1" "mod_quiz > View" page logged in as "teacher" + And I follow "Review" + Then I should see "1#00/4#00" + And I should see "25#00 out of 100#00" + And I should see "Mark 1#00 out of 1#00" + And I follow "Finish review" + And "Review" "link" in the "Preview" "table_row" should be visible + @javascript Scenario: Preview the quiz Given I am on the "Quiz 1" "mod_quiz > View" page logged in as "teacher" diff --git a/mod/quiz/tests/generator/lib.php b/mod/quiz/tests/generator/lib.php index e6a331688a6..ae5ca854196 100644 --- a/mod/quiz/tests/generator/lib.php +++ b/mod/quiz/tests/generator/lib.php @@ -93,6 +93,10 @@ class mod_quiz_generator extends testing_module_generator { } } + if (isset($record->gradepass)) { + $record->gradepass = unformat_float($record->gradepass); + } + return parent::create_instance($record, (array)$options); } diff --git a/question/type/ddimageortext/tests/behat/backup_and_restore.feature b/question/type/ddimageortext/tests/behat/backup_and_restore.feature index 8317809d722..ca724b4c340 100644 --- a/question/type/ddimageortext/tests/behat/backup_and_restore.feature +++ b/question/type/ddimageortext/tests/behat/backup_and_restore.feature @@ -87,7 +87,7 @@ Feature: Test duplicating a quiz containing a drag and drop onto image question | For any partially correct response | Parts, but only parts, of your response are correct. | | id_shownumcorrect | 1 | | For any incorrect response | That is not right at all. | - | Penalty for each incorrect try | 0.3333333 | + | Penalty for each incorrect try | 33.33333% | | Hint 1 | Incorrect placements will be removed. | | id_hintclearwrong_0 | 1 | | id_hintshownumcorrect_0 | 1 | diff --git a/question/type/ddimageortext/tests/behat/edit.feature b/question/type/ddimageortext/tests/behat/edit.feature index 7fecbcfe477..9bb97d495ef 100644 --- a/question/type/ddimageortext/tests/behat/edit.feature +++ b/question/type/ddimageortext/tests/behat/edit.feature @@ -31,3 +31,20 @@ Feature: Test editing a drag and drop onto image questions | Question name | Edited question name | And I press "id_submitbutton" Then I should see "Edited question name" + + Scenario: Edit a drag and drop onto image question and verify penalty works as expected + When I choose "Edit question" action for "Drag onto image" in the question bank + Then the following fields match these values: + | Question name | Drag onto image | + | Penalty for each incorrect try | 33.33333% | + | Penalty for each incorrect try | 0.3333333 | + + Scenario: Edit a drag and drop onto image question and verify penalty works as expected with custom decimal separator + When the following "language customisations" exist: + | component | stringid | value | + | core_langconfig | decsep | # | + And I choose "Edit question" action for "Drag onto image" in the question bank + Then the following fields match these values: + | Question name | Drag onto image | + | Penalty for each incorrect try | 33#33333% | + | Penalty for each incorrect try | 0.3333333 | diff --git a/question/type/ddmarker/tests/behat/backup_and_restore.feature b/question/type/ddmarker/tests/behat/backup_and_restore.feature index 5fde8360817..a5d15a262af 100644 --- a/question/type/ddmarker/tests/behat/backup_and_restore.feature +++ b/question/type/ddmarker/tests/behat/backup_and_restore.feature @@ -56,7 +56,7 @@ Feature: Test duplicating a quiz containing a drag and drop markers question | For any partially correct response | Parts, but only parts, of your response are correct. | | id_shownumcorrect | 1 | | For any incorrect response | That is not right at all. | - | Penalty for each incorrect try | 0.3333333 | + | Penalty for each incorrect try | 33.33333% | | Hint 1 | You are trying to place four markers on the map. | | id_hintshownumcorrect_0 | 1 | | id_hintclearwrong_0 | 0 | diff --git a/question/type/edit_question_form.php b/question/type/edit_question_form.php index ebd23e12537..c6f357567db 100644 --- a/question/type/edit_question_form.php +++ b/question/type/edit_question_form.php @@ -541,7 +541,7 @@ abstract class question_edit_form extends question_wizard_form { } $penaltyoptions = array(); foreach ($penalties as $penalty) { - $penaltyoptions["{$penalty}"] = (100 * $penalty) . '%'; + $penaltyoptions["{$penalty}"] = format_float(100 * $penalty, 5, true, true) . '%'; } $mform->addElement('select', 'penalty', get_string('penaltyforeachincorrecttry', 'question'), $penaltyoptions);