From 03aa22e05c6284afa3afd9c281804deb564b4f76 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Tue, 11 May 2021 15:12:58 +0700 Subject: [PATCH] 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 | 1 + lib/pear/HTML/QuickForm.php | 1 + lib/pear/HTML/QuickForm/Rule/Regex.php | 3 ++- lib/pear/README_MOODLE.txt | 3 +++ .../report/grading/gradingsettings_form.php | 1 + mod/quiz/report/grading/report.php | 25 ++++++++++++++----- .../grading/tests/behat/grading.feature | 25 +++++++++++++++++++ 7 files changed, 52 insertions(+), 7 deletions(-) diff --git a/lang/en/form.php b/lang/en/form.php index ab3ee372988..1cab461df3c 100644 --- a/lang/en/form.php +++ b/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.'; diff --git a/lib/pear/HTML/QuickForm.php b/lib/pear/HTML/QuickForm.php index c0fb5b7fa9e..c1a8473efac 100644 --- a/lib/pear/HTML/QuickForm.php +++ b/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') ); diff --git a/lib/pear/HTML/QuickForm/Rule/Regex.php b/lib/pear/HTML/QuickForm/Rule/Regex.php index 787a9572e11..3faf0640d49 100644 --- a/lib/pear/HTML/QuickForm/Rule/Regex.php +++ b/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*$/' ); /** diff --git a/lib/pear/README_MOODLE.txt b/lib/pear/README_MOODLE.txt index 8423a446d1e..3a8a9efe6a5 100644 --- a/lib/pear/README_MOODLE.txt +++ b/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 ==== diff --git a/mod/quiz/report/grading/gradingsettings_form.php b/mod/quiz/report/grading/gradingsettings_form.php index 3bb83149cc6..89400e22a2a 100644 --- a/mod/quiz/report/grading/gradingsettings_form.php +++ b/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( diff --git a/mod/quiz/report/grading/report.php b/mod/quiz/report/grading/report.php index f2d3efb7b42..6b90de87179 100644 --- a/mod/quiz/report/grading/report.php +++ b/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; } @@ -414,12 +421,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 - // redirect to a cleaned-up GET URL. - if ($mform->get_data()) { - set_user_preference('quiz_grading_pagesize', $pagesize); - set_user_preference('quiz_grading_order', $order); - redirect($this->grade_question_url($slot, $questionid, $grade, $page)); + if ($mform->is_submitted()) { + if ($mform->is_validated()) { + // If the form was submitted and validated, save the user preferences, and + // redirect to a cleaned-up GET URL. + set_user_preference('quiz_grading_pagesize', $pagesize); + 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( diff --git a/mod/quiz/report/grading/tests/behat/grading.feature b/mod/quiz/report/grading/tests/behat/grading.feature index 2a308a1db25..7eeaa012c5a 100644 --- a/mod/quiz/report/grading/tests/behat/grading.feature +++ b/mod/quiz/report/grading/tests/behat/grading.feature @@ -83,3 +83,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"