MDL-74516 activities: Better handling of floats for gradepass fields

Current code was relying on grade_floatval() that is not a validation
function. Instead, the gradepass field must be defined as proper float
and then perform the needed validations using unformat_float().

Note that the float element form has some particularities, see
MDL-73994 for more information and that makes us to have to check
for some values (null, zero) manually. Once that form element type
gets its behaviour fixed, the code will need to be revisited, hence
we have annotated it as comments for easier finding it in the future
(the same changes already were applied to other gradebook forms).

Also, remove an unreachable line of code (comparing with empty string)
within the grade_floatval() function because it's not possible anymore
to pass any string to it, as far as it's "?float" typed.

Covered as many cases with quiz completion as have been able to imagine.
This commit is contained in:
Eloy Lafuente (stronk7) 2022-06-21 19:28:29 +02:00
parent 9ccda59e00
commit b3b1006201
3 changed files with 47 additions and 8 deletions

View File

@ -492,8 +492,16 @@ abstract class moodleform_mod extends moodleform {
$validategradepass = true;
}
// Confirm gradepass is a valid non-zero value.
if ($validategradepass && (!isset($data[$gradepassfieldname]) || grade_floatval($data[$gradepassfieldname]) == 0)) {
// We need to make all the validations related with $gradepassfieldname
// 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.
$gradepass = isset($data[$gradepassfieldname]) ? unformat_float($data[$gradepassfieldname]) : null;
// Confirm gradepass is a valid non-empty (null or zero) value.
if ($validategradepass && (is_null($gradepass) || $gradepass == 0)) {
$errors['completionpassgrade'] = get_string(
'activitygradetopassnotset',
'completion'
@ -959,10 +967,9 @@ abstract class moodleform_mod extends moodleform {
}
// Grade to pass.
$mform->addElement('text', $gradepassfieldname, get_string('gradepass', 'grades'));
$mform->addElement('float', $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');
}
@ -1136,10 +1143,9 @@ abstract class moodleform_mod extends moodleform {
}
// Grade to pass.
$mform->addElement('text', $gradepassfieldname, get_string($gradepassfieldname, 'grades'));
$mform->addElement('float', $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');
}
}

View File

@ -1586,14 +1586,14 @@ function grade_course_reset($courseid) {
}
/**
* Convert a number to 5 decimal point float, an empty string or a null db compatible format
* Convert a number to 5 decimal point float, null db compatible format
* (we need this to decide if db value changed)
*
* @param float|null $number The number to convert
* @return float|null float or null
*/
function grade_floatval(?float $number) {
if (is_null($number) or $number === '') {
if (is_null($number)) {
return null;
}
// we must round to 5 digits to get the same precision as in 10,5 db fields

View File

@ -54,3 +54,36 @@ Feature: Set a quiz to be marked complete when the student passes
And I navigate to "Reports" in current page administration
And I click on "Activity completion" "link"
And "Completed" "icon" should exist in the "Student 1" "table_row"
Scenario Outline: Verify that gradepass, together with completionpassgrade are validated correctly
Given the following "language customisations" exist:
| component | stringid | value |
| core_langconfig | decsep | <decsep> |
And the following "activity" exist:
| activity | name | course | idnumber | gradepass | completion | completionpassgrade |
| quiz | Oh, grades, passgrades and floats| C1 | ohgrades | <gradepass>| 2 | <completionpassgrade> |
When I am on the "ohgrades" "quiz activity editing" page logged in as "teacher1"
And I expand all fieldsets
And I set the field "Grade to pass" to "<gradepass>"
And I set the field "completionusegrade" to "1"
And I set the field "completionpassgrade" to "<completionpassgrade>"
And I press "Save and display"
Then I should see "<seen>"
And I should not see "<notseen>"
Examples:
| gradepass | completionpassgrade | decsep | seen | notseen | outcome |
| | 0 | . | method: Highest | Save and display | ok |
| | 1 | . | does not have a valid | method: Highest | completion-err |
| 0 | 0 | . | method: Highest | Save and display | ok |
| 0 | 1 | . | does not have a valid | method: Highest | completion-err |
| aaa | 0 | . | must enter a number | method: Highest | number-err |
| aaa | 1 | . | must enter a number | method: Highest | number-err |
| 200 | 0 | . | can not be greater | method: Highest | grade-big-err |
| 200 | 1 | . | can not be greater | method: Highest | grade-big-err |
| 5.55 | 0 | . | 5.55 out of 100 | Save and display | ok |
| 5.55 | 1 | . | 5.55 out of 100 | Save and display | ok |
| 5#55 | 0 | . | must enter a number | method: Highest | number-err |
| 5#55 | 1 | . | must enter a number | method: Highest | number-err |
| 5#55 | 0 | # | 5#55 out of 100 | Save and display | ok |
| 5#55 | 1 | # | 5#55 out of 100 | Save and display | ok |