L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'MDL-82687' of https://github.com/timhunt/moodle

Browse Source
This commit is contained in:
Sara Arjona
2024-08-13 12:40:44 +02:00
parent 3370f7e107 fc19af4ce5
commit 8f2a665e26
5 changed files with 98 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.upgradenotes/MDL-82687-2024080211282638.yml
+10
View File
@@ -0,0 +1,10 @@
issueNumber: MDL-82687
notes:
core_form:
- message:
Previously, the 'duration' form field type would allow users to input positive or negative durations.
However looking at all the uses, everyone was expecting this input type to only accept times >= 0 seconds,
and almost no-one was bothering to write manual form validation, leading to subtle bugs.
So now, by default this field type will validate the input value is not negative. If you need the
previous behaviour, there is a new option 'allownegative' which you can set to true. (The default is false.)
type: improved
lang/en/form.php
+1
View File
@@ -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_positiveduration'] = 'This duration cannot be negative.';
$string['err_positiveint'] = 'You must enter a whole number that is greater than 0.';
$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.';
lib/form/duration.php
+15 -1
View File
@@ -36,6 +36,9 @@ require_once($CFG->libdir . '/form/text.php');
* HTML class for a length of time. For example, 30 minutes of 4 days. The
* values returned to PHP is the duration in seconds (an int rounded to the nearest second).
*
* By default, only durations >= 0 can be input. If you want to allow negative
* durations set the option allownegative.
*
* @package core_form
* @category form
* @copyright 2009 Tim Hunt
@@ -46,9 +49,10 @@ class MoodleQuickForm_duration extends MoodleQuickForm_group {
* Control the field names for form elements
* optional => if true, show a checkbox beside the element to turn it on (or off)
* defaultunit => which unit is default when the form is blank (default Minutes).
* allownegative => are durations < 0 allowed? (default false)
* @var array
*/
protected $_options = ['optional' => false, 'defaultunit' => MINSECS];
protected $_options = ['optional' => false, 'defaultunit' => MINSECS, 'allownegative' => false];
/** @var array associative array of time units (days, hours, minutes, seconds) */
private $_units = null;
@@ -64,6 +68,7 @@ class MoodleQuickForm_duration extends MoodleQuickForm_group {
* the time is blank. If not specified, minutes is used.
* 'units' => array containing some or all of 1, MINSECS, HOURSECS, DAYSECS and WEEKSECS
* which unit choices to offer.
* 'allownegative' => true/false - are durations < 0 allowed? (default false)
* @param mixed $attributes Either a typical HTML attribute string or an associative array
*/
public function __construct($elementName = null, $elementLabel = null,
@@ -78,6 +83,7 @@ class MoodleQuickForm_duration extends MoodleQuickForm_group {
$options = [];
}
$this->_options['optional'] = !empty($options['optional']);
$this->_options['allownegative'] = !empty($options['allownegative']);
if (isset($options['defaultunit'])) {
if (!array_key_exists($options['defaultunit'], $this->get_units())) {
throw new coding_exception($options['defaultunit'] .
@@ -248,6 +254,14 @@ class MoodleQuickForm_duration extends MoodleQuickForm_group {
}
}
#[\Override]
public function validateSubmitValue($values) {
if ($this->exportValue($values) < 0 && !$this->_options['allownegative']) {
return get_string('err_positiveduration', 'core_form');
}
return null;
}
/**
* Returns HTML for advchecbox form element.
*
lib/form/tests/duration_test.php
+44 -13
View File
@@ -14,17 +14,6 @@
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
/**
* Unit tests for MoodleQuickForm_duration
*
* Contains test cases for testing MoodleQuickForm_duration
*
* @package core_form
* @category test
* @copyright 2009 Tim Hunt
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
namespace core_form;
use moodleform;
@@ -44,6 +33,7 @@ require_once($CFG->libdir . '/form/duration.php');
* @category test
* @copyright 2009 Tim Hunt
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
* @covers \MoodleQuickForm_duration
*/
class duration_test extends \basic_testcase {
@@ -54,7 +44,7 @@ class duration_test extends \basic_testcase {
*/
protected function get_test_form(): MoodleQuickForm {
$form = new temp_form_duration();
return $form->getform();
return $form->get_form();
}
/**
@@ -192,6 +182,47 @@ class duration_test extends \basic_testcase {
$this->assertEquals(['testel' => $expected], $el->exportValue($values, true));
$this->assertEquals($expected, $el->exportValue($values));
}
/**
* Test cases for {@see test_validate_submit_value_negative_blocked()}.
* @return array[] test cases.
*/
public static function validate_submit_value_cases(): array {
return [
[false, -10, MINSECS, false],
[false, 10, MINSECS, true],
[false, 0, MINSECS, true],
[true, -10, MINSECS, true],
[true, 10, MINSECS, true],
[true, 0, MINSECS, true],
];
}
/**
* Test for {@see MoodleQuickForm_duration::validateSubmitValue()}.
*
* @dataProvider validate_submit_value_cases
* @param bool $allownegative whether the element should be created to allow negative values.
* @param int $number the number submitted.
* @param int $unit the unit submitted.
* @param bool $isvalid whether this submission is valid.
*/
public function test_validate_submit_value(bool $allownegative, int $number, int $unit, bool $isvalid): void {
$form = new temp_form_duration(null, null, 'post', '', null, true);
/** @var \MoodleQuickForm_duration $element */
$element = $form->get_form()->addElement('duration', 'testel', '', ['allownegative' => $allownegative]);
$values = ['testel' => ['number' => $number, 'timeunit' => $unit]];
if ($isvalid) {
$this->assertNull($element->validateSubmitValue($values));
} else {
$this->assertEquals(
get_string('err_positiveduration', 'core_form'),
$element->validateSubmitValue($values),
);
}
}
}
/**
@@ -209,7 +240,7 @@ class temp_form_duration extends moodleform {
* Returns form reference
* @return MoodleQuickForm
*/
public function getform() {
public function get_form() {
$mform = $this->_form;
// Set submitted flag, to simulate submission.
$mform->_flagSubmitted = true;
mod/quiz/tests/behat/settings_form_validation.feature
+28
View File
@@ -0,0 +1,28 @@
@mod @mod_quiz
Feature: Settings form fields are validated
To help me avoid mistakes
As a teacher
I need the quiz settings to be validated
Background:
Given the following "users" exist:
| username | firstname |
| teacher | Teach |
And the following "courses" exist:
| fullname | shortname | category |
| Course 1 | C1 | 0 |
And the following "course enrolments" exist:
| user | course | role |
| teacher | C1 | editingteacher |
And the following "activities" exist:
| activity | course | section | name |
| quiz | C1 | 1 | Test quiz 1 |
Scenario: Negative time limits are not allowed
When I am on the "Test quiz 1" "quiz activity editing" page logged in as teacher
And I expand all fieldsets
And I set the following fields to these values:
| id_timelimit_enabled | 1 |
| id_timelimit_number | -10 |
And I press "Save and display"
Then I should see "This duration cannot be negative"