From e5d3b28a4d10cb4d960897d37071246532ab8ebc Mon Sep 17 00:00:00 2001 From: Sam Minnee Date: Sat, 6 Oct 2018 11:53:17 +1300 Subject: [PATCH] =?UTF-8?q?FIX:=20Don=E2=80=99t=20break=20validation=20on?= =?UTF-8?q?=20selects=20without=20a=20source.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes cases where there are no valid values or an empty-string value is manually added rather than using setEmptyString() Fixes #4849 Fixes #7159 --- src/Forms/SingleSelectField.php | 6 ++++-- tests/php/Forms/DropdownFieldTest.php | 31 ++++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/Forms/SingleSelectField.php b/src/Forms/SingleSelectField.php index 7fad88036..8c60a84cf 100644 --- a/src/Forms/SingleSelectField.php +++ b/src/Forms/SingleSelectField.php @@ -124,15 +124,17 @@ abstract class SingleSelectField extends SelectField { // Check if valid value is given $selected = $this->Value(); + $validValues = $this->getValidValues(); + if (strlen($selected)) { // Use selection rules to check which are valid - foreach ($this->getValidValues() as $formValue) { + foreach ($validValues as $formValue) { if ($this->isSelectedValue($formValue, $selected)) { return true; } } } else { - if ($this->getHasEmptyDefault()) { + if ($this->getHasEmptyDefault() || !$validValues || in_array('', $validValues)) { // Check empty value return true; } diff --git a/tests/php/Forms/DropdownFieldTest.php b/tests/php/Forms/DropdownFieldTest.php index 585287a11..8aaa8fed4 100644 --- a/tests/php/Forms/DropdownFieldTest.php +++ b/tests/php/Forms/DropdownFieldTest.php @@ -505,7 +505,7 @@ class DropdownFieldTest extends SapphireTest */ public function testRequiredDropdownHasEmptyDefault() { - $field = new \SilverStripe\Forms\DropdownField("RequiredField", "dropdown", ["item 1", "item 2"]); + $field = new DropdownField("RequiredField", "dropdown", ["item 1", "item 2"]); $form = new Form( Controller::curr(), @@ -517,4 +517,33 @@ class DropdownFieldTest extends SapphireTest $this->assertTrue($field->getHasEmptyDefault()); } + + public function testEmptySourceDoesntBlockValidation() + { + // Empty source + $field = new DropdownField("EmptySource", "", []); + $v = new RequiredFields(); + $field->validate($v); + $this->assertTrue($v->getResult()->isValid()); + + // Source with a setEmptyString + $field = new DropdownField("EmptySource", "", []); + $field->setEmptyString('(Select one)'); + $v = new RequiredFields(); + $field->validate($v); + $this->assertTrue($v->getResult()->isValid()); + + // Source with an empty value + $field = new DropdownField("SourceWithBlankVal", "", [ "" => "(Choose)" ]); + $v = new RequiredFields(); + $field->validate($v); + $this->assertTrue($v->getResult()->isValid()); + + // Source with all items disabled + $field = new DropdownField("SourceWithBlankVal", "", [ "A" => "A", "B" => "B" ]); + $field->setDisabledItems([ 'A', 'B' ]); + $v = new RequiredFields(); + $field->validate($v); + $this->assertTrue($v->getResult()->isValid()); + } }