From dfb9b76b95fd33e1ad804e3579ac56f7a11503a0 Mon Sep 17 00:00:00 2001 From: Loz Calver Date: Fri, 22 May 2015 20:55:54 +0100 Subject: [PATCH 1/2] Add validation for GroupedDropdownField (closes #3923) --- forms/GroupedDropdownField.php | 35 +++++++++++++++++-- tests/forms/GroupedDropdownFieldTest.php | 44 ++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 tests/forms/GroupedDropdownFieldTest.php diff --git a/forms/GroupedDropdownField.php b/forms/GroupedDropdownField.php index 500ecfa9e..f128d94ac 100644 --- a/forms/GroupedDropdownField.php +++ b/forms/GroupedDropdownField.php @@ -89,11 +89,42 @@ class GroupedDropdownField extends DropdownField { } /** - * @todo Implement DropdownField::validate() with group validation support + * Validate this field + * + * @param Validator $validator + * @return bool */ public function validate($validator) { + $valid = false; + $source = $this->getSourceAsArray(); + + if ($this->value) { + foreach ($source as $value => $title) { + // Check if value matches an option, or is in a nested array of grouped options + if ((is_array($title) && array_key_exists($this->value, $title)) + || ($this->value == $value) + ) { + $valid = true; + } + } + } elseif ($this->getHasEmptyDefault()) { + $valid = true; + } + + if (!$valid) { + $validator->validationError( + $this->name, + _t( + 'DropdownField.SOURCE_VALIDATION', + "Please select a value within the list provided. {value} is not a valid option", + array('value' => $this->value) + ), + "validation" + ); + return false; + } + return true; } } - diff --git a/tests/forms/GroupedDropdownFieldTest.php b/tests/forms/GroupedDropdownFieldTest.php new file mode 100644 index 000000000..772684f91 --- /dev/null +++ b/tests/forms/GroupedDropdownFieldTest.php @@ -0,0 +1,44 @@ + "One", + "Group One" => array( + "2" => "Two", + "3" => "Three" + ), + "Group Two" => array( + "4" => "Four" + ) + )); + + $validator = new RequiredFields(); + + $field->setValue("1"); + $this->assertTrue($field->validate($validator)); + + //test grouped values + $field->setValue("3"); + $this->assertTrue($field->validate($validator)); + + //non-existent value should make the field invalid + $field->setValue("Over 9000"); + $this->assertFalse($field->validate($validator)); + + //empty string shouldn't validate + $field->setValue(''); + $this->assertFalse($field->validate($validator)); + + //empty field should validate after being set + $field->setEmptyString('Empty String'); + $field->setValue(''); + $this->assertTrue($field->validate($validator)); + } + +} From 68d8df4e04556b3f9ad63a6d8cff84e63f972d66 Mon Sep 17 00:00:00 2001 From: Loz Calver Date: Fri, 22 May 2015 21:11:01 +0100 Subject: [PATCH 2/2] FIX: DropdownField didn't consider disabled items --- forms/DropdownField.php | 4 +++- forms/GroupedDropdownField.php | 12 ++++++++---- tests/forms/DropdownFieldTest.php | 7 ++++++- tests/forms/GroupedDropdownFieldTest.php | 10 ++++++++++ 4 files changed, 27 insertions(+), 6 deletions(-) diff --git a/forms/DropdownField.php b/forms/DropdownField.php index d3f990a6c..c58aeb211 100644 --- a/forms/DropdownField.php +++ b/forms/DropdownField.php @@ -314,7 +314,9 @@ class DropdownField extends FormField { */ public function validate($validator) { $source = $this->getSourceAsArray(); - if (!array_key_exists($this->value, $source)) { + $disabled = $this->getDisabledItems(); + + if (!array_key_exists($this->value, $source) || in_array($this->value, $disabled)) { if ($this->getHasEmptyDefault() && !$this->value) { return true; } diff --git a/forms/GroupedDropdownField.php b/forms/GroupedDropdownField.php index f128d94ac..6e24a5012 100644 --- a/forms/GroupedDropdownField.php +++ b/forms/GroupedDropdownField.php @@ -97,13 +97,17 @@ class GroupedDropdownField extends DropdownField { public function validate($validator) { $valid = false; $source = $this->getSourceAsArray(); + $disabled = $this->getDisabledItems(); if ($this->value) { foreach ($source as $value => $title) { - // Check if value matches an option, or is in a nested array of grouped options - if ((is_array($title) && array_key_exists($this->value, $title)) - || ($this->value == $value) - ) { + if (is_array($title) && array_key_exists($this->value, $title)) { + // Check that the set value is not in the list of disabled items + if (!isset($disabled[$value]) || !in_array($this->value, $disabled[$value])) { + $valid = true; + } + // Check that the value matches and is not disabled + } elseif($this->value == $value && !in_array($this->value, $disabled)) { $valid = true; } } diff --git a/tests/forms/DropdownFieldTest.php b/tests/forms/DropdownFieldTest.php index ef131e893..481da561b 100644 --- a/tests/forms/DropdownFieldTest.php +++ b/tests/forms/DropdownFieldTest.php @@ -313,7 +313,8 @@ class DropdownFieldTest extends SapphireTest { public function testValidation() { $field = DropdownField::create('Test', 'Testing', array( "One" => "One", - "Two" => "Two" + "Two" => "Two", + "Five" => "Five" )); $validator = new RequiredFields(); $form = new Form($this, 'Form', new FieldList($field), new FieldList(), $validator); @@ -331,6 +332,10 @@ class DropdownFieldTest extends SapphireTest { $field->setEmptyString('Empty String'); $field->setValue(''); $this->assertTrue($field->validate($validator)); + //disabled items shouldn't validate + $field->setDisabledItems(array('Five')); + $field->setValue('Five'); + $this->assertFalse($field->validate($validator)); } } diff --git a/tests/forms/GroupedDropdownFieldTest.php b/tests/forms/GroupedDropdownFieldTest.php index 772684f91..e15b623b4 100644 --- a/tests/forms/GroupedDropdownFieldTest.php +++ b/tests/forms/GroupedDropdownFieldTest.php @@ -39,6 +39,16 @@ class GroupedDropdownFieldTest extends SapphireTest { $field->setEmptyString('Empty String'); $field->setValue(''); $this->assertTrue($field->validate($validator)); + + //disabled items shouldn't validate + $field->setDisabledItems(array('1')); + $field->setValue('1'); + $this->assertFalse($field->validate($validator)); + + //grouped disabled items shouldn't validate + $field->setDisabledItems(array("Group One" => array("2"))); + $field->setValue('2'); + $this->assertFalse($field->validate($validator)); } }