MDL-73824 gradebook: Add support for localised floats where missing

It has been detected, thanks to php80 specially, that there are
various places in core where support for localised floats is
missing. Before php80, some locale-dependent conversions were
performed by PHP, allowing things to work. But with php80 all
those comparisons are now locale-independent. See:

https://wiki.php.net/rfc/locale_independent_float_to_string

That implies that we now need to, always, unformat floats to
be internally the correct (decimal point as separator) in
order to compare it.

While this was visited in the php80 epic (MDL-70745), nothing
was found, all automated tests were passing ok. Problem is that
we run behat tests with en-AU laguage that has the decimal point
separator.

So, in this issue we are fixing all the problems detected by
running those Behat tests using localised (comma) decimal
separator.

Note that there may be other places still causing problems, but
it's really hard to find them programmatically, so we'll have to
wait for real use reports / issues and go fixing them while they
happen.

Back to this commit, this is the list of changes performed (note that
in the next commit, we'll be adding scenarios explicitly using
a localised decimal separator to ensure that they work ok).

A. Changes to various grade forms to ensure that, on their validation
  floats are unformatted properly. Also, changed the corresponding
  form element from current text/PARAM_RAW to proper float ones that
  take care of the conversion in a number of places (but when disabled,
  that's the reason we still have to unformat in validation.
  This includes the following forms:
    - edit_category_form
    - edit_item_form
  (this is the original problem reported that cause all the research
  to be performed against full behat runs)

B. Changes to edit_letter_form, so it uses a proper PARAM_LOCALISEDFLOAT
  (note this is the type of change that surely should be used for all
  the rest of /grade/edit/tree form, including those in the previous
  point).

C. Changes to the grade_item behat generator, so it's able to work with
  localised floats, un-formatting them when needed.
  At lib/behat/classes/behat_core_generator.php

D. Fix problem passing localised floats to scales, not displaying
  properly. At grade/report/singleview/classes/local/ui/finalgrade.php

E. Change the behat text matcher in order to allow comparison of
  localised floats when they are the current ones. Before this change
  the matches was using soft/lazy comparison, so '50' and '50.0000'
  match. Now, when the comma (for example) is used (and only then),
  '50' and '50,000' will also match. This comparison is in use in a
  bunch of tests and makes sense to make it localisation-aware.
  At grade/report/singleview/classes/local/ui/finalgrade.php

F. Fix a couple of number_format() uses in lesson, because they are
  not localised-aware. Switched to format_float(). At mod/lesson/locallib.php

G. Change the quiz_contains_the_following_questions() step to accept
  localised maxmark expectations. At mod/quiz/tests/behat/behat_mod_quiz.php

H. Change the quiz generator so it accepts localised gradepass.
  At mod/quiz/tests/generator/lib.php

I. Change the edit question form to show proper localised penalties,
  previously it was always showing point-decimal ones. Of course,
  leaving the values of the select element unmodified (internal floats).
  Related, also change a couple of tests to, instead of try to match the
  value (always internal floats), match the description (now localised),
  so we can test them with different separators. At:
    - question/type/ddimageortext/tests/behat/backup_and_restore.feature
    - question/type/ddmarker/tests/behat/backup_and_restore.feature
    - question/type/edit_question_form.php

grade/edit/letter/edit_form.php

@@ -59,12 +59,10 @@ public function definition() {
             }
 
             $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');
             }

grade/edit/tree/category.php

@@ -133,7 +133,7 @@
 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);
 }

grade/edit/tree/category_form.php

@@ -167,22 +167,19 @@ function definition() {
         $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 @@ function definition() {
         $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 @@ function validation($data, $files) {
                 $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 @@ function validation($data, $files) {
         return $errors;
     }
 }
-
-

grade/edit/tree/item.php

@@ -109,7 +109,7 @@
 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) {

grade/edit/tree/item_form.php

@@ -110,37 +110,32 @@ function definition() {
         $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 @@ function definition() {
         $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 @@ function validation($data, $files) {
             }
         }
 
-        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 @@ function validation($data, $files) {
         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');
                     }

grade/report/singleview/classes/local/ui/finalgrade.php

@@ -141,6 +141,7 @@ public function set($value) {
         $feedback = false;
         $feedbackformat = false;
         if ($gradeitem->gradetype == GRADE_TYPE_SCALE) {
+            $value = (int)unformat_float($value);
             if ($value == -1) {
                 $finalgrade = null;
             } else {

lib/behat/classes/behat_core_generator.php

@@ -362,6 +362,22 @@ protected function preprocess_grade_item($data) {
             $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;
     }
 

lib/behat/form_field/behat_form_field.php

@@ -247,10 +247,23 @@ protected function get_internal_field_id() {
      * @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;
+        }
+
+        // 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 true;
+
+        return false;
     }
 
     /**

mod/lesson/locallib.php

@@ -3157,7 +3157,7 @@ public function add_messages_on_page_view(lesson_page $page, $reviewmode) {
                         $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');
                         }
@@ -3591,7 +3591,7 @@ public function process_eol_page($outoftime) {
                     }
                     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;
                     }

mod/lesson/tests/numeric_helper_test.php

@@ -148,9 +148,7 @@ public function lesson_format_dataprovider() {
      * 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'`.
      */

mod/quiz/tests/behat/behat_mod_quiz.php

@@ -233,8 +233,8 @@ public function quiz_contains_the_following_questions($quizname, TableNode $data
             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());

mod/quiz/tests/generator/lib.php

@@ -93,6 +93,10 @@ public function create_instance($record = null, array $options = null) {
             }
         }
 
+        if (isset($record->gradepass)) {
+            $record->gradepass = unformat_float($record->gradepass);
+        }
+
         return parent::create_instance($record, (array)$options);
     }
 

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                                                    |

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                                                    |

question/type/edit_question_form.php

@@ -504,7 +504,7 @@ protected function add_interactive_settings($withclearwrong = false,
         }
         $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);