From d7781a2edde55c9d37f30a593f6059122ec2c4ba Mon Sep 17 00:00:00 2001 From: Steve Boyd Date: Mon, 11 Nov 2024 15:51:13 +1300 Subject: [PATCH] ENH Use FieldValidator for FormFields --- code/Form/UserFormsRequiredFields.php | 4 +- code/FormField/UserFormsCheckboxSetField.php | 37 +++++-------------- .../Control/UserDefinedFormControllerTest.php | 2 +- .../UserFormsCheckboxSetFieldTest.php | 16 ++++---- 4 files changed, 21 insertions(+), 38 deletions(-) diff --git a/code/Form/UserFormsRequiredFields.php b/code/Form/UserFormsRequiredFields.php index 268a1d273..711dd2739 100644 --- a/code/Form/UserFormsRequiredFields.php +++ b/code/Form/UserFormsRequiredFields.php @@ -36,7 +36,9 @@ public function php($data) $fields = $this->form->Fields(); foreach ($fields as $field) { - $valid = ($field->validate($this) && $valid); + $result = $field->validate(); + $valid = $result->isValid() && $valid; + $this->result->combineAnd($result); } if (empty($this->required)) { diff --git a/code/FormField/UserFormsCheckboxSetField.php b/code/FormField/UserFormsCheckboxSetField.php index 377814757..293702a7e 100644 --- a/code/FormField/UserFormsCheckboxSetField.php +++ b/code/FormField/UserFormsCheckboxSetField.php @@ -2,8 +2,8 @@ namespace SilverStripe\UserForms\FormField; +use SilverStripe\Core\Validation\ValidationResult; use SilverStripe\Forms\CheckboxSetField; -use SilverStripe\UserForms\Model\EditableFormField; /** * @package userforms @@ -45,33 +45,16 @@ public function getOptions() return $options; } - /** - * @inheritdoc - * - * @param Validator $validator - * - * @return bool - */ - public function validate($validator) + public function getValueForValidation(): mixed { - // get the previous values (could contain comma-delimited list) - - $previous = $value = $this->Value(); - - if (is_string($value) && strstr($value ?? '', ",")) { - $value = explode(",", $value ?? ''); + $value = $this->Value(); + if (is_array($value)) { + return $value; } - - // set the value as an array for parent validation - - $this->setValue($value); - - $validated = parent::validate($validator); - - // restore previous value after validation - - $this->setValue($previous); - - return $validated; + // Value may contain a comma-delimited list of values + if (is_string($value) && strstr($value, ",")) { + return explode(',', $value); + } + return [$value]; } } diff --git a/tests/php/Control/UserDefinedFormControllerTest.php b/tests/php/Control/UserDefinedFormControllerTest.php index 36dccbf9a..11f38b943 100644 --- a/tests/php/Control/UserDefinedFormControllerTest.php +++ b/tests/php/Control/UserDefinedFormControllerTest.php @@ -155,7 +155,7 @@ public function testValidation() 'required-email' => 'invalid', 'required-text' => 'bob' ]); - $this->assertStringContainsString('Please enter an email address', $response->getBody()); + $this->assertStringContainsString('Invalid email address', $response->getBody()); // Post with only required $this->get($form->URLSegment); diff --git a/tests/php/FormField/UserFormsCheckboxSetFieldTest.php b/tests/php/FormField/UserFormsCheckboxSetFieldTest.php index 85738e677..a45801bec 100644 --- a/tests/php/FormField/UserFormsCheckboxSetFieldTest.php +++ b/tests/php/FormField/UserFormsCheckboxSetFieldTest.php @@ -3,7 +3,6 @@ namespace SilverStripe\UserForms\Tests\FormField; use SilverStripe\Dev\SapphireTest; -use SilverStripe\UserForms\Form\UserFormsRequiredFields; use SilverStripe\UserForms\FormField\UserFormsCheckboxSetField; use SilverStripe\UserForms\Model\EditableFormField\EditableCheckboxGroupField; @@ -14,27 +13,26 @@ class UserFormsCheckboxSetFieldTest extends SapphireTest public function testValidate() { $field = new UserFormsCheckboxSetField('Field', 'My field', ['One' => 'One', 'Two' => 'Two']); - $validator = new UserFormsRequiredFields(); // String values $field->setValue('One'); - $this->assertTrue($field->validate($validator)); + $this->assertTrue($field->validate()->isValid()); $field->setValue('One,Two'); - $this->assertTrue($field->validate($validator)); + $this->assertTrue($field->validate()->isValid()); $field->setValue('Three,Four'); - $this->assertFalse($field->validate($validator)); + $this->assertFalse($field->validate()->isValid()); // Array values $field->setValue(array('One')); - $this->assertTrue($field->validate($validator)); + $this->assertTrue($field->validate()->isValid()); $field->setValue(array('One', 'Two')); - $this->assertTrue($field->validate($validator)); + $this->assertTrue($field->validate()->isValid()); // Invalid $field->setValue('Three'); - $this->assertFalse($field->validate($validator)); + $this->assertFalse($field->validate()->isValid()); $field->setValue(array('Three', 'Four')); - $this->assertFalse($field->validate($validator)); + $this->assertFalse($field->validate()->isValid()); } public function testCustomErrorMessageValidationAttributesHTML()