Skip to content

Commit

Permalink
MDL-69507 duration form field: modernise coding style
Browse files Browse the repository at this point in the history
  • Loading branch information
@timhunt
timhunt committed Aug 20, 2020
1 parent a0fc902 commit e5a0f11
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 78 deletions.
37 changes: 18 additions & 19 deletions lib/form/duration.php
View file
Expand Up @@ -43,19 +43,20 @@
*/
class MoodleQuickForm_duration extends MoodleQuickForm_group {
/**
* Control the fieldnames for form elements
* 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).
* @var array
*/
protected $_options = array('optional' => false, 'defaultunit' => MINSECS);
protected $_options = ['optional' => false, 'defaultunit' => MINSECS];

/** @var array associative array of time units (days, hours, minutes, seconds) */
private $_units = null;

/**
* constructor
*
* @param string $elementName Element's name
* @param ?string $elementName Element's name
* @param mixed $elementLabel Label(s) for an element
* @param array $options Options to control the element's display. Recognised values are
* 'optional' => true/false - whether to display an 'enabled' checkbox next to the element.
Expand All @@ -66,16 +67,15 @@ class MoodleQuickForm_duration extends MoodleQuickForm_group {
* @param mixed $attributes Either a typical HTML attribute string or an associative array
*/
public function __construct($elementName = null, $elementLabel = null,
$options = array(), $attributes = null) {
// TODO MDL-52313 Replace with the call to parent::__construct().
HTML_QuickForm_element::__construct($elementName, $elementLabel, $attributes);
$options = [], $attributes = null) {
parent::__construct($elementName, $elementLabel, $attributes);
$this->_persistantFreeze = true;
$this->_appendName = true;
$this->_type = 'duration';

// Set the options, do not bother setting bogus ones
if (!is_array($options)) {
$options = array();
$options = [];
}
$this->_options['optional'] = !empty($options['optional']);
if (isset($options['defaultunit'])) {
Expand Down Expand Up @@ -111,7 +111,7 @@ public function __construct($elementName = null, $elementLabel = null,
* @deprecated since Moodle 3.1
*/
public function MoodleQuickForm_duration($elementName = null, $elementLabel = null,
$options = array(), $attributes = null) {
$options = [], $attributes = null) {
debugging('Use of class name as constructor is deprecated', DEBUG_DEVELOPER);
self::__construct($elementName, $elementLabel, $options, $attributes);
}
Expand All @@ -123,13 +123,13 @@ public function MoodleQuickForm_duration($elementName = null, $elementLabel = nu
*/
public function get_units() {
if (is_null($this->_units)) {
$this->_units = array(
$this->_units = [
WEEKSECS => get_string('weeks'),
DAYSECS => get_string('days'),
HOURSECS => get_string('hours'),
MINSECS => get_string('minutes'),
1 => get_string('seconds'),
);
];
}
return $this->_units;
}
Expand Down Expand Up @@ -158,14 +158,14 @@ protected function get_units_used() {
*/
public function seconds_to_unit($seconds) {
if ($seconds == 0) {
return array(0, $this->_options['defaultunit']);
return [0, $this->_options['defaultunit']];
}
foreach ($this->get_units_used() as $unit => $notused) {
if (fmod($seconds, $unit) == 0) {
return array($seconds / $unit, $unit);
return [$seconds / $unit, $unit];
}
}
return array($seconds, 1);
return [$seconds, 1];
}

/**
Expand All @@ -174,12 +174,12 @@ public function seconds_to_unit($seconds) {
function _createElements() {
$attributes = $this->getAttributes();
if (is_null($attributes)) {
$attributes = array();
$attributes = [];
}
if (!isset($attributes['size'])) {
$attributes['size'] = 3;
}
$this->_elements = array();
$this->_elements = [];
// E_STRICT creating elements without forms is nasty because it internally uses $this
$number = $this->createFormElement('text', 'number',
get_string('time', 'form'), $attributes, true);
Expand Down Expand Up @@ -226,7 +226,7 @@ function onQuickFormEvent($event, $arg, &$caller) {
}
if (!is_array($value)) {
list($number, $unit) = $this->seconds_to_unit($value);
$value = array('number' => $number, 'timeunit' => $unit);
$value = ['number' => $number, 'timeunit' => $unit];
// If optional, default to off, unless a date was provided
if ($this->_options['optional']) {
$value['enabled'] = $number != 0;
Expand All @@ -245,7 +245,6 @@ function onQuickFormEvent($event, $arg, &$caller) {
}
$caller->setType($arg[0] . '[number]', PARAM_FLOAT);
return parent::onQuickFormEvent($event, $arg, $caller);
break;

default:
return parent::onQuickFormEvent($event, $arg, $caller);
Expand All @@ -270,7 +269,7 @@ function toHtml() {
*
* @param HTML_QuickForm_Renderer $renderer An HTML_QuickForm_Renderer object
* @param bool $required Whether a group is required
* @param string $error An error message associated with a group
* @param ?string $error An error message associated with a group
*/
function accept(&$renderer, $required = false, $error = null) {
$renderer->renderElement($this, $required, $error);
Expand All @@ -286,7 +285,7 @@ function accept(&$renderer, $required = false, $error = null) {
*/
function exportValue(&$submitValues, $assoc = false) {
// Get the values from all the child elements.
$valuearray = array();
$valuearray = [];
foreach ($this->_elements as $element) {
$thisexport = $element->exportValue($submitValues[$this->getName()], true);
if (!is_null($thisexport)) {
Expand Down
144 changes: 85 additions & 59 deletions lib/form/tests/duration_test.php
View file
Expand Up @@ -47,7 +47,7 @@ class core_form_duration_testcase extends basic_testcase {
*
* @return MoodleQuickForm
*/
protected function get_test_form() {
protected function get_test_form(): MoodleQuickForm {
$form = new temp_form_duration();
return $form->getform();
}
Expand All @@ -57,27 +57,26 @@ protected function get_test_form() {
*
* @return array with two elements, a MoodleQuickForm and a MoodleQuickForm_duration.
*/
protected function get_test_form_and_element() {
protected function get_test_form_and_element(): array {
$mform = $this->get_test_form();
$element = $mform->addElement('duration', 'duration');
return [$mform, $element];
}

/**
* Testcase for testing contructor.
*
* @expectedException coding_exception
* Test the constructor error handling.
*/
public function test_constructor() {
public function test_constructor_rejects_invalid_unit(): void {
// Test trying to create with an invalid unit.
$mform = $this->get_test_form();
$this->expectException('coding_exception');
$mform->addElement('duration', 'testel', null, ['defaultunit' => 123, 'optional' => false]);
}

/**
* Test contructor only some units.
* Test constructor only some units.
*/
public function test_constructor_limited_units() {
public function test_constructor_limited_units(): void {
$mform = $this->get_test_form();
$mform->addElement('duration', 'testel', null, ['units' => [MINSECS, 1], 'optional' => false]);
$html = $mform->toHtml();
Expand All @@ -90,74 +89,101 @@ public function test_constructor_limited_units() {
/**
* Testcase for testing units (seconds, minutes, hours and days)
*/
public function test_get_units() {
public function test_get_units(): void {
[$mform, $element] = $this->get_test_form_and_element();
$units = $element->get_units();
$this->assertEquals($units, [1 => get_string('seconds'), 60 => get_string('minutes'),
3600 => get_string('hours'), 86400 => get_string('days'), 604800 => get_string('weeks')]);
}

/**
* Testcase for testing conversion of seconds to the best possible unit
* Data provider for {@see test_seconds_to_unit()}.
*
* @return array test cases.
*/
public function test_seconds_to_unit() {
[$mform, $element] = $this->get_test_form_and_element();
$this->assertEquals([0, MINSECS], $element->seconds_to_unit(0)); // Zero minutes, for a nice default unit.
$this->assertEquals([1, 1], $element->seconds_to_unit(1));
$this->assertEquals([3601, 1], $element->seconds_to_unit(3601));
$this->assertEquals([1, MINSECS], $element->seconds_to_unit(60));
$this->assertEquals([3, MINSECS], $element->seconds_to_unit(180));
$this->assertEquals([1, HOURSECS], $element->seconds_to_unit(3600));
$this->assertEquals([2, HOURSECS], $element->seconds_to_unit(7200));
$this->assertEquals([1, DAYSECS], $element->seconds_to_unit(86400));
$this->assertEquals([25, HOURSECS], $element->seconds_to_unit(90000));
public function seconds_to_unit_cases(): array {
return [
[[0, MINSECS], 0], // Zero minutes, for a nice default unit.
[[1, 1], 1],
[[3601, 1], 3601],
[[1, MINSECS], 60],
[[3, MINSECS], 180],
[[1, HOURSECS], 3600],
[[2, HOURSECS], 7200],
[[1, DAYSECS], 86400],
[[25, HOURSECS], 90000],
];
}

/**
* Testcase for testing conversion of seconds to the best possible unit.
*
* @dataProvider seconds_to_unit_cases
* @param array $expected expected return value from seconds_to_unit
* @param int $seconds value to pass to seconds_to_unit
*/
public function test_seconds_to_unit(array $expected, int $seconds): void {
[, $element] = $this->get_test_form_and_element();
$this->assertEquals($expected, $element->seconds_to_unit($seconds));
}

/**
* Testcase for testing conversion of seconds to the best possible unit with a non-default default unit.
*/
public function test_seconds_to_unit_different_default_unit() {
$mform = $this->get_test_form();
$element = $mform->addElement('duration', 'testel', null,
['defaultunit' => DAYSECS, 'optional' => false]);
$this->assertEquals([0, DAYSECS], $element->seconds_to_unit(0)); // Zero minutes, for a nice default unit.
$this->assertEquals([0, DAYSECS], $element->seconds_to_unit(0));
}

/**
* Data provider for {@see test_export_value()}.
*
* @return array test cases.
*/
public function export_value_cases(): array {
return [
[10, '10', 1],
[180, '3', MINSECS],
[90, '1.5', MINSECS],
[7200, '2', HOURSECS],
[86400, '1', DAYSECS],
[0, '0', HOURSECS],
[0, '10', 1, 0, true],
[20, '20', 1, 1, true],
[0, '10', 1, 0, true, ''],
[20, '20', 1, 1, true, ''],
];
}

/**
* Testcase to check generated timestamp
*
* @dataProvider export_value_cases
* @param int $expected Expected value returned by the element.
* @param string $number Number entered into the element.
* @param int $unit Unit selected in the element.
* @param int $enabled Whether the enabled checkbox on the form was selected. (Only used if $optional is true.)
* @param bool $optional Whether the element has the optional option on.
* @param string|null $label The element's label.
*/
public function test_exportValue() {
public function test_export_value(int $expected, string $number, int $unit, int $enabled = 0,
bool $optional = false, ?string $label = null): void {

// Create the test element.
$mform = $this->get_test_form();
$el = $mform->addElement('duration', 'testel');
$values = ['testel' => ['number' => 10, 'timeunit' => 1]];
$this->assertEquals(['testel' => 10], $el->exportValue($values, true));
$this->assertEquals(10, $el->exportValue($values));
$values = ['testel' => ['number' => 3, 'timeunit' => MINSECS]];
$this->assertEquals(['testel' => 180], $el->exportValue($values, true));
$this->assertEquals(180, $el->exportValue($values));
$values = ['testel' => ['number' => 1.5, 'timeunit' => MINSECS]];
$this->assertEquals(['testel' => 90], $el->exportValue($values, true));
$this->assertEquals(90, $el->exportValue($values));
$values = ['testel' => ['number' => 2, 'timeunit' => HOURSECS]];
$this->assertEquals(['testel' => 7200], $el->exportValue($values, true));
$this->assertEquals(7200, $el->exportValue($values));
$values = ['testel' => ['number' => 1, 'timeunit' => DAYSECS]];
$this->assertEquals(['testel' => 86400], $el->exportValue($values, true));
$this->assertEquals(86400, $el->exportValue($values));
$values = ['testel' => ['number' => 0, 'timeunit' => HOURSECS]];
$this->assertEquals(['testel' => 0], $el->exportValue($values, true));
$this->assertEquals(0, $el->exportValue($values));

$el = $mform->addElement('duration', 'testel', null, ['optional' => true]);
$values = ['testel' => ['number' => 10, 'timeunit' => 1]];
$this->assertEquals(['testel' => 0], $el->exportValue($values, true));
$this->assertEquals(0, $el->exportValue($values));
$values = ['testel' => ['number' => 20, 'timeunit' => 1, 'enabled' => 1]];
$this->assertEquals(['testel' => 20], $el->exportValue($values, true));
$this->assertEquals(20, $el->exportValue($values));

// Optional element.
$el2 = $mform->addElement('duration', 'testel', '', ['optional' => true]);
$values = ['testel' => ['number' => 10, 'timeunit' => 1, 'enabled' => 1]];
$this->assertEquals(['testel' => 10], $el2->exportValue($values, true));
$this->assertEquals(10, $el2->exportValue($values));
$values = ['testel' => ['number' => 10, 'timeunit' => 1, 'enabled' => 0]];
$this->assertEquals(['testel' => 0], $el2->exportValue($values, true));
$this->assertEquals(null, $el2->exportValue($values));
$el = $mform->addElement('duration', 'testel', $label, $optional ? ['optional' => true] : []);

// Prepare the submitted values.
$values = ['testel' => ['number' => $number, 'timeunit' => $unit]];
if ($optional) {
$values['testel']['enabled'] = $enabled;
}

// Test.
$this->assertEquals(['testel' => $expected], $el->exportValue($values, true));
$this->assertEquals($expected, $el->exportValue($values));
}
}

Expand Down

0 comments on commit e5a0f11

Please sign in to comment.