MDL-71126 Quiz: Manual grading page size preference can get stuck at 0

Including in this change:
 - New positiveint regex rule to check if the value is a positive integer

lang/en/form.php

@@ -39,6 +39,7 @@ $string['err_minlength'] = 'You must enter at least {$a->format} characters here
 $string['err_nonzero'] = 'You must enter a number not starting with a 0 here.';
 $string['err_nopunctuation'] = 'You must enter no punctuation characters here.';
 $string['err_numeric'] = 'You must enter a number here.';
+$string['err_positiveint'] = 'You must enter a number that greater than 0 here.';
 $string['err_rangelength'] = 'You must enter between {$a->format[0]} and {$a->format[1]} characters here.';
 $string['err_required'] = 'You must supply a value here.';
 $string['err_wrappingwhitespace'] = 'The value must not start or end with whitespace.';

lib/pear/HTML/QuickForm.php

@@ -65,6 +65,7 @@ $GLOBALS['_HTML_QuickForm_registered_rules'] = array(
     'numeric'       => array('html_quickform_rule_regex',    'HTML/QuickForm/Rule/Regex.php'),
     'nopunctuation' => array('html_quickform_rule_regex',    'HTML/QuickForm/Rule/Regex.php'),
     'nonzero'       => array('html_quickform_rule_regex',    'HTML/QuickForm/Rule/Regex.php'),
+    'positiveint'   => array('html_quickform_rule_regex',    'HTML/QuickForm/Rule/Regex.php'),
     'callback'      => array('html_quickform_rule_callback', 'HTML/QuickForm/Rule/Callback.php'),
     'compare'       => array('html_quickform_rule_compare',  'HTML/QuickForm/Rule/Compare.php')
 );

lib/pear/HTML/QuickForm/Rule/Regex.php

@@ -40,7 +40,8 @@ class HTML_QuickForm_Rule_Regex extends HTML_QuickForm_Rule
                     'alphanumeric'  => '/^[a-zA-Z0-9]+$/',
                     'numeric'       => '/(^-?\d\d*\.\d*$)|(^-?\d\d*$)|(^-?\.\d\d*$)/',
                     'nopunctuation' => '/^[^().\/\*\^\?#!@$%+=,\"\'><~\[\]{}]+$/',
-                    'nonzero'       => '/^-?[1-9][0-9]*/'
+                    'nonzero'       => '/^-?[1-9][0-9]*/',
+                    'positiveint'   => '/^[1-9]\d*$/'
                     );
 
     /**

lib/pear/README_MOODLE.txt

@@ -29,6 +29,9 @@ MDL-70711 - removed unnecessary if-else conditional block in HTML_QuickForm as t
             condition always evaluates to false due to the deprecated get_magic_quotes_gpc()
             which always returns false
 MDL-70457 - PHP 7.4 curly brackets string access fix.
+MDL-71126 - Quiz: Manual grading page size preference can get stuck at 0
+            Including in this change:
+             - New positiveint regex rule to check if the value is a positive integer
 
 Pear
 ====

mod/quiz/report/grading/gradingsettings_form.php

@@ -72,6 +72,7 @@ class quiz_grading_settings_form extends moodleform {
 
         $mform->addElement('text', 'pagesize', get_string('questionsperpage', 'quiz_grading'),
                 array('size' => 3));
+        $mform->addRule('pagesize', null, 'positiveint', null, 'client');
         $mform->setType('pagesize', PARAM_INT);
 
         $orderoptions = array(

mod/quiz/report/grading/report.php

@@ -42,6 +42,9 @@ class quiz_grading_report extends quiz_default_report {
     const DEFAULT_PAGE_SIZE = 5;
     const DEFAULT_ORDER = 'random';
 
+    /** @var string Positive integer regular expression. */
+    const REGEX_POSITIVE_INT = '/^[1-9]\d*$/';
+
     /** @var array URL parameters for what is being displayed when grading. */
     protected $viewoptions = [];
 
@@ -99,6 +102,10 @@ class quiz_grading_report extends quiz_default_report {
                 $this->viewoptions[$param] = $$param;
             }
         }
+        if (!data_submitted() && !preg_match(self::REGEX_POSITIVE_INT, $pagesize)) {
+            // We only validate if the user accesses the page via a cleaned-up GET URL here.
+            throw new moodle_exception('invalidpagesize');
+        }
         if ($pagesize != self::DEFAULT_PAGE_SIZE) {
             $this->viewoptions['pagesize'] = $pagesize;
         }
@@ -415,12 +422,18 @@ class quiz_grading_report extends quiz_default_report {
         $settings->order = $order;
         $mform->set_data($settings);
 
-        // If the form was submitted, save the user preferences, and
+        if ($mform->is_submitted()) {
-        // redirect to a cleaned-up GET URL.
+            if ($mform->is_validated()) {
-        if ($mform->get_data()) {
+                // If the form was submitted and validated, save the user preferences, and
-            set_user_preference('quiz_grading_pagesize', $pagesize);
+                // redirect to a cleaned-up GET URL.
-            set_user_preference('quiz_grading_order', $order);
+                set_user_preference('quiz_grading_pagesize', $pagesize);
-            redirect($this->grade_question_url($slot, $questionid, $grade, $page));
+                set_user_preference('quiz_grading_order', $order);
+                redirect($this->grade_question_url($slot, $questionid, $grade, $page));
+            } else {
+                // Set the pagesize back to the previous value, so the report page can continue the render
+                // and the form can show the validation.
+                $pagesize = get_user_preferences('quiz_grading_pagesize', self::DEFAULT_PAGE_SIZE);
+            }
         }
 
         list($qubaids, $count) = $this->get_usage_ids_where_question_in_state(

mod/quiz/report/grading/tests/behat/grading.feature

@@ -87,3 +87,28 @@ Feature: Basic use of the Manual grading report
     Then the following fields match these values:
       | Questions per page | 42      |
       | Order attempts     | By date |
+
+  @javascript
+  Scenario: Manual grading settings are validated
+    Given user "student1" has attempted "Quiz 1" with responses:
+      | slot | response |
+      | 1    | Paris    |
+    And I am on the "Quiz 1" "mod_quiz > Manual grading report" page logged in as "teacher1"
+    And I follow "Also show questions that have been graded automatically"
+    And I click on "update grades" "link" in the "Short answer 001" "table_row"
+    When I set the following fields to these values:
+      | Questions per page | 0 |
+    And I press "Change options"
+    Then I should see "You must enter a number that greater than 0 here"
+    And I set the following fields to these values:
+      | Questions per page | -1 |
+    And I press "Change options"
+    And I should see "You must enter a number that greater than 0 here"
+    And I set the following fields to these values:
+      | Questions per page | abc |
+    And I press "Change options"
+    And I should see "You must enter a number that greater than 0 here"
+    And I set the following fields to these values:
+      | Questions per page | 1 |
+    And I press "Change options"
+    And I should not see "You must enter a number that greater than 0 here"