From 41ea83b3374d026da9575f5256a02bdcfbb82059 Mon Sep 17 00:00:00 2001 From: Stevie Mayhew Date: Wed, 12 Nov 2014 15:19:12 +1300 Subject: [PATCH] FEATURE: add validation to form field subclasses --- docs/en/changelogs/3.2.0.md | 2 + forms/CheckboxSetField.php | 42 +++++++++++ forms/CompositeField.php | 10 ++- forms/ConfirmedPasswordField.php | 7 +- forms/CreditCardField.php | 2 +- forms/CurrencyField.php | 2 +- forms/DateField.php | 6 +- forms/DatetimeField.php | 5 +- forms/DropdownField.php | 67 ++++++++++++++++-- forms/EmailField.php | 2 +- forms/FileField.php | 2 +- forms/FormField.php | 10 +-- forms/HiddenField.php | 1 + forms/ListboxField.php | 43 +++++++++++- forms/MemberDatetimeOptionsetField.php | 8 ++- forms/MoneyField.php | 10 +++ forms/NullableField.php | 1 + forms/NumericField.php | 10 ++- forms/OptionsetField.php | 1 - forms/PhoneNumberField.php | 7 +- forms/ReadonlyField.php | 1 + forms/TimeField.php | 12 ++-- forms/UploadField.php | 4 +- forms/Validator.php | 8 +-- lang/en.yml | 5 ++ tests/control/RequestHandlingTest.php | 16 ++--- tests/forms/CheckboxFieldTest.php | 28 ++++++++ tests/forms/CheckboxSetFieldTest.php | 62 ++++++++++++++++ tests/forms/CheckboxSetFieldTest.yml | 13 +--- tests/forms/CompositeFieldTest.php | 17 +++++ tests/forms/ConfirmedPasswordFieldTest.php | 15 +++- tests/forms/CurrencyFieldTest.php | 37 ++++++++-- tests/forms/DropdownFieldTest.php | 23 ++++++ tests/forms/FormFieldTest.php | 4 +- tests/forms/ListboxFieldTest.php | 70 +++++++++++++++++++ .../MemberDatetimeOptionsetFieldTest.php | 7 +- tests/forms/NumericFieldTest.php | 4 +- tests/forms/uploadfield/UploadFieldTest.php | 10 ++- 38 files changed, 490 insertions(+), 84 deletions(-) diff --git a/docs/en/changelogs/3.2.0.md b/docs/en/changelogs/3.2.0.md index 9a1fde57a..ca85ba92d 100644 --- a/docs/en/changelogs/3.2.0.md +++ b/docs/en/changelogs/3.2.0.md @@ -16,6 +16,7 @@ * `Convert::html2raw` no longer wraps text by default and can decode single quotes. * `Mailer` no longer calls `xml2raw` on all email subject line, and now must be passed in via plain text. * `ErrorControlChain` now supports reload on exceptions + * `FormField::validate` now requires an instance of `Validator` #### Deprecated classes/methods removed @@ -545,3 +546,4 @@ coding conventions. E.g. `DB::requireTable` is now `DB::require_table` * create_table_options now uses constants as API specific filters rather than strings. This is in order to promote better referencing of elements across the codebase. See `FulltextSearchable->enable` for example. + * `FormField` subclasses must now use `validate(Validator $validator)` as the interface has changed for this function \ No newline at end of file diff --git a/forms/CheckboxSetField.php b/forms/CheckboxSetField.php index eaf7316f4..7644b3f3e 100644 --- a/forms/CheckboxSetField.php +++ b/forms/CheckboxSetField.php @@ -312,4 +312,46 @@ class CheckboxSetField extends OptionsetField { return FormField::ExtraOptions(); } + /** + * Validate this field + * + * @param Validator $validator + * @return bool + */ + public function validate(Validator $validator) { + $values = $this->value; + if (!$values) { + return true; + } + $sourceArray = $this->getSourceAsArray(); + if (is_array($values)) { + if (!array_intersect_key($sourceArray, $values)) { + $validator->validationError( + $this->name, + _t( + 'CheckboxSetField.SOURCE_VALIDATION', + "Please select a value within the list provided. '{value}' is not a valid option", + array('value' => implode(' and ', array_diff($sourceArray, $values))) + ), + "validation" + ); + return false; + } + } else { + if (!in_array($this->value, $sourceArray)) { + $validator->validationError( + $this->name, + _t( + 'CheckboxSetField.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/forms/CompositeField.php b/forms/CompositeField.php index 9e9f305e7..3e86b641c 100644 --- a/forms/CompositeField.php +++ b/forms/CompositeField.php @@ -341,7 +341,7 @@ class CompositeField extends FormField { // Clear an internal cache $this->sequentialSet = null; - // A true results indicates that the field was foudn + // A true results indicates that the field was found return true; } } @@ -358,7 +358,13 @@ class CompositeField extends FormField { return $result; } - public function validate($validator) { + /** + * Validate this field + * + * @param Validator $validator + * @return bool + */ + public function validate(Validator $validator) { $valid = true; foreach($this->children as $idx => $child){ $valid = ($child && $child->validate($validator) && $valid); diff --git a/forms/ConfirmedPasswordField.php b/forms/ConfirmedPasswordField.php index 7be3eacd5..cca24ee42 100644 --- a/forms/ConfirmedPasswordField.php +++ b/forms/ConfirmedPasswordField.php @@ -314,11 +314,12 @@ class ConfirmedPasswordField extends FormField { } /** - * @param Validator $validator + * Validate this field * - * @return boolean + * @param Validator $validator + * @return bool */ - public function validate($validator) { + public function validate(Validator $validator) { $name = $this->name; // if field isn't visible, don't validate diff --git a/forms/CreditCardField.php b/forms/CreditCardField.php index bbd1c8be3..9ebc2d8d1 100644 --- a/forms/CreditCardField.php +++ b/forms/CreditCardField.php @@ -58,7 +58,7 @@ class CreditCardField extends TextField { else return $this->value; } - public function validate($validator){ + public function validate(Validator $validator){ // If the field is empty then don't return an invalidation message if(!trim(implode("", $this->value))) return true; diff --git a/forms/CurrencyField.php b/forms/CurrencyField.php index 568f5907f..9b3a8b19b 100644 --- a/forms/CurrencyField.php +++ b/forms/CurrencyField.php @@ -43,7 +43,7 @@ class CurrencyField extends TextField { return $this->castedCopy('CurrencyField_Readonly'); } - public function validate($validator) { + public function validate(Validator $validator) { if(!empty ($this->value) && !preg_match('/^\s*(\-?\$?|\$\-?)?(\d{1,3}(\,\d{3})*|(\d+))(\.\d{2})?\s*$/', $this->value)) { diff --git a/forms/DateField.php b/forms/DateField.php index 7fbf0cae8..0ceca0101 100644 --- a/forms/DateField.php +++ b/forms/DateField.php @@ -336,7 +336,7 @@ class DateField extends TextField { /** * @return Boolean */ - public function validate($validator) { + public function validate(Validator $validator) { $valid = true; // Don't validate empty fields @@ -496,10 +496,6 @@ class DateField_Disabled extends DateField { public function Type() { return "date_disabled readonly"; } - - public function validate($validator) { - return true; - } } /** diff --git a/forms/DatetimeField.php b/forms/DatetimeField.php index d72a5e01b..0340535db 100644 --- a/forms/DatetimeField.php +++ b/forms/DatetimeField.php @@ -323,7 +323,7 @@ class DatetimeField extends FormField { } } - public function validate($validator) { + public function validate(Validator $validator) { $dateValid = $this->dateField->validate($validator); $timeValid = $this->timeField->validate($validator); @@ -391,7 +391,4 @@ class DatetimeField_Readonly extends DatetimeField { return "id() . "\">$val"; } - public function validate($validator) { - return true; - } } diff --git a/forms/DropdownField.php b/forms/DropdownField.php index a425933fd..c7a1b3675 100644 --- a/forms/DropdownField.php +++ b/forms/DropdownField.php @@ -84,7 +84,7 @@ class DropdownField extends FormField { /** - * @var Array $source Associative or numeric array of all dropdown items, + * @var array|ArrayAccess $source Associative or numeric array of all dropdown items, * with array key as the submitted field value, and the array value as a * natural language description shown in the interface element. */ @@ -119,7 +119,7 @@ class DropdownField extends FormField { /** * @param string $name The field name * @param string $title The field title - * @param array $source An map of the dropdown items + * @param array|ArrayAccess $source A map of the dropdown items * @param string $value The current value * @param Form $form The parent form */ @@ -223,14 +223,14 @@ class DropdownField extends FormField { /** * Gets the source array including any empty default values. * - * @return array + * @return array|ArrayAccess */ public function getSource() { return $this->source; } /** - * @param array $source + * @param array|ArrayAccess $source */ public function setSource($source) { $this->source = $source; @@ -286,4 +286,63 @@ class DropdownField extends FormField { return $field; } + + /** + * Get the source of this field as an array + * + * @return array + */ + public function getSourceAsArray() + { + $source = $this->getSource(); + if (is_array($source)) { + return $source; + } else { + $sourceArray = array(); + foreach ($source as $key => $value) { + $sourceArray[$key] = $value; + } + } + return $sourceArray; + } + + /** + * Validate this field + * + * @param Validator $validator + * @return bool + */ + public function validate(Validator $validator) { + $source = $this->getSourceAsArray(); + if (!array_key_exists($this->value, $source)) { + if ($this->getHasEmptyDefault() && !$this->value) { + return true; + } + $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; + } + + /** + * Returns another instance of this field, but "cast" to a different class. + * + * @see FormField::castedCopy() + * + * @param String $classOrCopy + * @return FormField + */ + public function castedCopy($classOrCopy) { + $field = parent::castedCopy($classOrCopy); + $field->setHasEmptyDefault($this->getHasEmptyDefault()); + return $field; + } } diff --git a/forms/EmailField.php b/forms/EmailField.php index 8e540c0f6..f8f0ef614 100644 --- a/forms/EmailField.php +++ b/forms/EmailField.php @@ -30,7 +30,7 @@ class EmailField extends TextField { * @param Validator $validator * @return String */ - public function validate($validator) { + public function validate(Validator $validator) { $this->value = trim($this->value); $pcrePattern = '^[a-z0-9!#$%&\'*+/=?^_`{|}~-]+(?:\\.[a-z0-9!#$%&\'*+/=?^_`{|}~-]+)*' diff --git a/forms/FileField.php b/forms/FileField.php index 9294371be..52d425790 100644 --- a/forms/FileField.php +++ b/forms/FileField.php @@ -181,7 +181,7 @@ class FileField extends FormField { return ($this->folderName !== false) ? $this->folderName : Config::inst()->get('Upload', 'uploads_folder'); } - public function validate($validator) { + public function validate(Validator $validator) { if(!isset($_FILES[$this->name])) return true; $tmpFile = $_FILES[$this->name]; diff --git a/forms/FormField.php b/forms/FormField.php index c74fd1a91..bd059abb0 100644 --- a/forms/FormField.php +++ b/forms/FormField.php @@ -15,7 +15,8 @@ * format of a date or currency field. Define {@link saveInto()} to totally * customise saving. For example, data might be saved to the filesystem instead * of the data record, or saved to a component of the data record instead of - * the data record itself. + * the data record itself. Define {@link validate()} to validate the form field + * and ensure that the content provided is valid. * * @package forms * @subpackage core @@ -894,14 +895,13 @@ class FormField extends RequestHandler { } /** - * Abstract method each {@link FormField} subclass must implement, - * determines whether the field is valid or not based on the value. - * @todo Make this abstract. + * Validation method each {@link FormField} subclass should implement, + * determining whether the field is valid or not based on the value. * * @param Validator * @return boolean */ - public function validate($validator) { + public function validate(Validator $validator) { return true; } diff --git a/forms/HiddenField.php b/forms/HiddenField.php index 1aa12fcbe..48b1e4882 100644 --- a/forms/HiddenField.php +++ b/forms/HiddenField.php @@ -31,4 +31,5 @@ class HiddenField extends FormField { function SmallFieldHolder($properties = array()) { return $this->FieldHolder($properties); } + } diff --git a/forms/ListboxField.php b/forms/ListboxField.php index 0a5958906..7501a0010 100644 --- a/forms/ListboxField.php +++ b/forms/ListboxField.php @@ -203,7 +203,7 @@ class ListboxField extends DropdownField { } /** - * Load a value into this CheckboxSetField + * Load a value into this ListboxField */ public function setValue($val, $obj = null) { // If we're not passed a value directly, @@ -281,4 +281,45 @@ class ListboxField extends DropdownField { return $this->defaultItems; } + /** + * Validate this field + * + * @param Validator $validator + * @return bool + */ + public function validate(Validator $validator) { + $values = $this->value; + if (!$values) { + return true; + } + $source = $this->getSourceAsArray(); + if (is_array($values)) { + if (!array_intersect_key($source,array_flip($values))) { + $validator->validationError( + $this->name, + _t( + "Please select a value within the list provided. {value} is not a valid option", + array('value' => $this->value) + ), + "validation" + ); + return false; + } + } else { + if (!array_key_exists($this->value, $source)) { + $validator->validationError( + $this->name, + _t( + 'ListboxField.SOURCE_VALIDATION', + "Please select a value within the list provided. %s is not a valid option", + array('value' => $this->value) + ), + "validation" + ); + return false; + } + } + return true; + } + } diff --git a/forms/MemberDatetimeOptionsetField.php b/forms/MemberDatetimeOptionsetField.php index a233416b3..7219131db 100644 --- a/forms/MemberDatetimeOptionsetField.php +++ b/forms/MemberDatetimeOptionsetField.php @@ -119,7 +119,13 @@ class MemberDatetimeOptionsetField extends OptionsetField { } } - public function validate($validator) { + /** + * Validate this field + * + * @param Validator $validator + * @return bool + */ + public function validate(Validator $validator) { $value = isset($_POST[$this->name . '_custom']) ? $_POST[$this->name . '_custom'] : null; if(!$value) return true; // no custom value, don't validate diff --git a/forms/MoneyField.php b/forms/MoneyField.php index 6c0300708..9a1009bca 100644 --- a/forms/MoneyField.php +++ b/forms/MoneyField.php @@ -177,4 +177,14 @@ class MoneyField extends FormField { public function getLocale() { return $this->_locale; } + + /** + * Validate this field + * + * @param Validator $validator + * @return bool + */ + public function validate(Validator $validator) { + return !(is_null($this->fieldAmount) || is_null($this->fieldCurrency)); + } } diff --git a/forms/NullableField.php b/forms/NullableField.php index 2df2c2f7b..2eed07176 100644 --- a/forms/NullableField.php +++ b/forms/NullableField.php @@ -126,4 +126,5 @@ class NullableField extends FormField { $result .= (is_null($this->value)) ? "<>" : $this->value; return result; } + } diff --git a/forms/NumericField.php b/forms/NumericField.php index 7edcd6963..2897e18e3 100644 --- a/forms/NumericField.php +++ b/forms/NumericField.php @@ -20,8 +20,14 @@ class NumericField extends TextField { )); } - public function validate($validator) { - if(!$this->value && !$validator->fieldIsRequired($this->name)) { + /** + * Validate this field + * + * @param Validator $validator + * @return bool + */ + public function validate(Validator $validator) { + if(!$this->value) { return true; } diff --git a/forms/OptionsetField.php b/forms/OptionsetField.php index 7305060d4..d5974314d 100644 --- a/forms/OptionsetField.php +++ b/forms/OptionsetField.php @@ -120,5 +120,4 @@ class OptionsetField extends DropdownField { public function ExtraOptions() { return new ArrayList(); } - } diff --git a/forms/PhoneNumberField.php b/forms/PhoneNumberField.php index 319639865..2725806e5 100644 --- a/forms/PhoneNumberField.php +++ b/forms/PhoneNumberField.php @@ -127,9 +127,14 @@ class PhoneNumberField extends FormField { } /** + * Validate this field + * * @todo Very basic validation at the moment + * + * @param Validator $validator + * @return bool */ - public function validate($validator){ + public function validate(Validator $validator){ $valid = preg_match( '/^[0-9\+\-\(\)\s\#]*$/', $this->joinPhoneNumber($this->value) diff --git a/forms/ReadonlyField.php b/forms/ReadonlyField.php index 6000a9beb..b5a8d595f 100644 --- a/forms/ReadonlyField.php +++ b/forms/ReadonlyField.php @@ -66,4 +66,5 @@ class ReadonlyField extends FormField { public function Type() { return 'readonly'; } + } diff --git a/forms/TimeField.php b/forms/TimeField.php index a6b1fc95b..703688e31 100644 --- a/forms/TimeField.php +++ b/forms/TimeField.php @@ -160,10 +160,12 @@ class TimeField extends TextField { } /** - * @return Boolean + * Validate this field + * + * @param Validator $validator + * @return bool */ - public function validate($validator) { - $valid = true; + public function validate(Validator $validator) { // Don't validate empty fields if(empty($this->value)) return true; @@ -260,8 +262,4 @@ class TimeField_Readonly extends TimeField { return "id() . "\">$val"; } - - public function validate($validator) { - return true; - } } diff --git a/forms/UploadField.php b/forms/UploadField.php index ce1163bcb..ff4026fbc 100644 --- a/forms/UploadField.php +++ b/forms/UploadField.php @@ -985,9 +985,7 @@ class UploadField extends FileField { * @param Validator $validator * @return boolean */ - public function validate($validator) { - - // @todo Test compatibility with RequiredFields + public function validate(Validator $validator) { $name = $this->getName(); $files = $this->getItems(); diff --git a/forms/Validator.php b/forms/Validator.php index 3b48bab8c..526fdee51 100644 --- a/forms/Validator.php +++ b/forms/Validator.php @@ -42,10 +42,10 @@ abstract class Validator extends Object { /** * Callback to register an error on a field (Called from implementations of {@link FormField::validate}) * - * @param $fieldName name of the field - * @param $message error message to display - * @param $messageType optional parameter, gets loaded into the HTML class attribute in the rendered output. - * See {@link getErrors()} for details. + * @param string $fieldName name of the field + * @param string $message error message to display + * @param string $messageType optional parameter, gets loaded into the HTML class attribute in the rendered output. + * See {@link getErrors()} for details. */ public function validationError($fieldName, $message, $messageType='') { $this->errors[] = array( diff --git a/lang/en.yml b/lang/en.yml index 22ffbf69d..3923e3d1f 100644 --- a/lang/en.yml +++ b/lang/en.yml @@ -83,6 +83,8 @@ en: CheckboxField: NOANSWER: No YESANSWER: Yes + CheckboxFieldSetField: + SOURCE_VALIDATION: 'Please select a value within the list provided. {value} is not a valid option' ConfirmedPasswordField: ATLEAST: 'Passwords must be at least {min} characters long.' BETWEEN: 'Passwords must be {min} to {max} characters long.' @@ -129,6 +131,7 @@ en: DropdownField: CHOOSE: (Choose) CHOOSESEARCH: '(Choose or Search)' + SOURCE_VALIDATION: 'Please select a value within the list provided. {value} is not a valid option' EmailField: VALIDATION: 'Please enter an email address' Enum: @@ -323,6 +326,8 @@ en: LeftAndMain_Menu_ss: Hello: Hi LOGOUT: 'Log out' + ListboxField: + SOURCE_VALIDATION: 'Please select a value within the list provided. {value} is not a valid option' LoginAttempt: Email: 'Email Address' IP: 'IP Address' diff --git a/tests/control/RequestHandlingTest.php b/tests/control/RequestHandlingTest.php index 389950c0b..c56248685 100644 --- a/tests/control/RequestHandlingTest.php +++ b/tests/control/RequestHandlingTest.php @@ -358,7 +358,7 @@ class RequestHandlingTest_Controller extends Controller implements TestOnly { } } -class RequestHandlingTest_FormActionController extends Controller { +class RequestHandlingTest_FormActionController extends Controller implements TestOnly { protected $template = 'BlankPage'; @@ -417,7 +417,7 @@ class RequestHandlingTest_FormActionController extends Controller { /** * Simple extension for the test controller */ -class RequestHandlingTest_ControllerExtension extends Extension { +class RequestHandlingTest_ControllerExtension extends Extension implements TestOnly { public static $called_error = false; @@ -490,7 +490,7 @@ class RequestHandlingTest_AllowedController extends Controller implements TestOn /** * Simple extension for the test controller - with allowed_actions define */ -class RequestHandlingTest_AllowedControllerExtension extends Extension { +class RequestHandlingTest_AllowedControllerExtension extends Extension implements TestOnly { private static $allowed_actions = array( 'otherExtendedMethod' ); @@ -500,7 +500,7 @@ class RequestHandlingTest_AllowedControllerExtension extends Extension { } } -class RequestHandlingTest_ControllerFailover extends ViewableData { +class RequestHandlingTest_ControllerFailover extends ViewableData implements TestOnly { public function failoverMethod() { return "failoverMethod"; } @@ -509,7 +509,7 @@ class RequestHandlingTest_ControllerFailover extends ViewableData { /** * Form for the test */ -class RequestHandlingTest_Form extends Form { +class RequestHandlingTest_Form extends Form implements TestOnly { private static $url_handlers = array( 'fields/$FieldName' => 'handleField', "POST " => "handleSubmission", @@ -552,7 +552,7 @@ class RequestHandlingTest_ControllerFormWithAllowedActions extends Controller im } } -class RequestHandlingTest_FormWithAllowedActions extends Form { +class RequestHandlingTest_FormWithAllowedActions extends Form implements TestOnly { private static $allowed_actions = array( 'allowedformaction' => 1, @@ -571,7 +571,7 @@ class RequestHandlingTest_FormWithAllowedActions extends Form { /** * Form field for the test */ -class RequestHandlingTest_FormField extends FormField { +class RequestHandlingTest_FormField extends FormField implements TestOnly { private static $url_handlers = array( "POST " => "handleInPlaceEdit", '' => 'handleField', @@ -638,7 +638,7 @@ class RequestHandlingFieldTest_Controller extends Controller implements TestOnly /** * Form field for the test */ -class RequestHandlingTest_HandlingField extends FormField { +class RequestHandlingTest_HandlingField extends FormField implements TestOnly { private static $allowed_actions = array( 'actionOnField' diff --git a/tests/forms/CheckboxFieldTest.php b/tests/forms/CheckboxFieldTest.php index 255db3af5..de6a33635 100644 --- a/tests/forms/CheckboxFieldTest.php +++ b/tests/forms/CheckboxFieldTest.php @@ -139,6 +139,34 @@ class CheckboxFieldTest extends SapphireTest { } + public function testValidation() { + $field = CheckboxField::create('Test', 'Testing'); + $validator = new RequiredFields(); + $field->setValue(1); + $this->assertTrue( + $field->validate($validator), + 'Field correctly validates integers as allowed' + ); + //string value should validate + $field->setValue("test"); + $this->assertTrue( + $field->validate($validator), + 'Field correctly validates words as allowed' + ); + //empty string should validate + $field->setValue(''); + $this->assertTrue( + $field->validate($validator), + 'Field correctly validates empty strings as allowed' + ); + //null should validate + $field->setValue(null); + $this->assertTrue( + $field->validate($validator), + 'Field correct validates null as allowed' + ); + } + } class CheckboxFieldTest_Article extends DataObject implements TestOnly { diff --git a/tests/forms/CheckboxSetFieldTest.php b/tests/forms/CheckboxSetFieldTest.php index c2e76f492..f22f2a866 100644 --- a/tests/forms/CheckboxSetFieldTest.php +++ b/tests/forms/CheckboxSetFieldTest.php @@ -144,6 +144,68 @@ class CheckboxSetFieldTest extends SapphireTest { $this->assertEquals('Test,Another', $dbValue); } + public function testValidationWithArray() { + //test with array input + $field = CheckboxSetField::create('Test', 'Testing', array( + "One" => "One", + "Two" => "Two", + "Three" => "Three" + )); + $validator = new RequiredFields(); + $field->setValue(array("One" => "One", "Two" => "Two")); + $this->assertTrue( + $field->validate($validator), + 'Field validates values within source array' + ); + //non valid value should fail + $field->setValue(array("Four" => "Four")); + $this->assertFalse( + $field->validate($validator), + 'Field does not validate values outside of source array' + ); + //non valid value included with valid options should succeed + $field->setValue(array("One" => "One", "Two" => "Two", "Four" => "Four")); + $this->assertTrue( + $field->validate($validator), + 'Field validates when presented with mixed valid and invalid values' + ); + } + + public function testValidationWithDataList() { + //test with datalist input + $checkboxTestArticle = $this->objFromFixture('CheckboxSetFieldTest_Article', 'articlewithtags'); + $tag1 = $this->objFromFixture('CheckboxSetFieldTest_Tag', 'tag1'); + $tag2 = $this->objFromFixture('CheckboxSetFieldTest_Tag', 'tag2'); + $tag3 = $this->objFromFixture('CheckboxSetFieldTest_Tag', 'tag3'); + $field = CheckboxSetField::create('Test', 'Testing', $checkboxTestArticle->Tags() ->map()); + $validator = new RequiredFields(); + $field->setValue(array( + $tag1->ID => $tag1->ID, + $tag2->ID => $tag2->ID + )); + $this->assertTrue( + $field->validate($validator), + 'Validates values in source map' + ); + //invalid value should fail + $fakeID = CheckboxSetFieldTest_Tag::get()->max('ID') + 1; + $field->setValue(array($fakeID => $fakeID)); + $this->assertFalse( + $field->validate($validator), + 'Field does not valid values outside of source map' + ); + //non valid value included with valid options should succeed + $field->setValue(array( + $tag1->ID => $tag1->ID, + $tag2->ID => $tag2->ID, + $tag3->ID => $tag3->ID + )); + $this->assertTrue( + $field->validate($validator), + 'Validates when presented with mixed valid and invalid values' + ); + } + } /** diff --git a/tests/forms/CheckboxSetFieldTest.yml b/tests/forms/CheckboxSetFieldTest.yml index 34ea7f275..fafe8f3e9 100644 --- a/tests/forms/CheckboxSetFieldTest.yml +++ b/tests/forms/CheckboxSetFieldTest.yml @@ -3,17 +3,8 @@ CheckboxSetFieldTest_Tag: Title: Tag 1 tag2: Title: Tag 2 -CheckboxSetFieldTest_Article: - articlewithouttags: - Content: Article 1 - articlewithtags: - Content: Article 2 - Tags: =>CheckboxSetFieldTest_Tag.tag1,=>CheckboxSetFieldTest_Tag.tag2 -CheckboxSetFieldTest_Tag: - tag1: - Title: Tag 1 - tag2: - Title: Tag 2 + tag3: + Title: Tag 3 CheckboxSetFieldTest_Article: articlewithouttags: Content: Article 1 diff --git a/tests/forms/CompositeFieldTest.php b/tests/forms/CompositeFieldTest.php index edc9bc7d2..2b6431ca6 100644 --- a/tests/forms/CompositeFieldTest.php +++ b/tests/forms/CompositeFieldTest.php @@ -60,4 +60,21 @@ class CompositeFieldTest extends SapphireTest { $this->assertNotNull($legend); $this->assertEquals('My legend', (string)$legend[0]); } + + public function testValidation() { + $field = CompositeField::create( + $fieldOne = DropdownField::create('A'), + $fieldTwo = TextField::create('B') + ); + $validator = new RequiredFields(); + $this->assertFalse( + $field->validate($validator), + "Validation fails when child is invalid" + ); + $fieldOne->setEmptyString('empty'); + $this->assertTrue( + $field->validate($validator), + "Validates when children are valid" + ); + } } diff --git a/tests/forms/ConfirmedPasswordFieldTest.php b/tests/forms/ConfirmedPasswordFieldTest.php index ec22fb600..846aa4436 100644 --- a/tests/forms/ConfirmedPasswordFieldTest.php +++ b/tests/forms/ConfirmedPasswordFieldTest.php @@ -61,15 +61,24 @@ class ConfirmedPasswordFieldTest extends SapphireTest { )); $validator = new RequiredFields(); $form = new Form($this, 'Form', new FieldList($field), new FieldList(), $validator); - $this->assertTrue($field->validate($validator)); + $this->assertTrue( + $field->validate($validator), + "Validates when both passwords are the same" + ); $field->setName("TestNew"); //try changing name of field - $this->assertTrue($field->validate($validator)); + $this->assertTrue( + $field->validate($validator), + "Validates when field name is changed" + ); //non-matching password should make the field invalid $field->setValue(array( "_Password" => "abc123", "_ConfirmPassword" => "123abc" )); - $this->assertFalse($field->validate($validator)); + $this->assertFalse( + $field->validate($validator), + "Does not validate when passwords differ" + ); } } diff --git a/tests/forms/CurrencyFieldTest.php b/tests/forms/CurrencyFieldTest.php index 0c2e25ff3..ffa8d611a 100644 --- a/tests/forms/CurrencyFieldTest.php +++ b/tests/forms/CurrencyFieldTest.php @@ -8,42 +8,55 @@ class CurrencyFieldTest extends SapphireTest { public function testValidate() { $f = new CurrencyField('TestField'); + $validator = new RequiredFields(); $f->setValue('123.45'); $this->assertTrue( - $f->validate(new RequiredFields()), + $f->validate($validator), 'Validates positive decimals' ); $f->setValue('-123.45'); $this->assertTrue( - $f->validate(new RequiredFields()), + $f->validate($validator), 'Validates negative decimals' ); $f->setValue('$123.45'); $this->assertTrue( - $f->validate(new RequiredFields()), + $f->validate($validator), 'Validates positive decimals with sign' ); $f->setValue('-$123.45'); $this->assertTrue( - $f->validate(new RequiredFields()), + $f->validate($validator), 'Validates negative decimals with sign' ); $f->setValue('$-123.45'); $this->assertTrue( - $f->validate(new RequiredFields()), + $f->validate($validator), 'Validates negative decimals with sign' ); $f->setValue('324511434634'); $this->assertTrue( - $f->validate(new RequiredFields()), + $f->validate($validator), 'Validates large integers' ); + + $f->setValue('test$1.23test'); + $this->assertTrue( + $f->validate($validator), + 'Alphanumeric is valid' + ); + + $f->setValue('$test'); + $this->assertTrue( + $f->validate($validator), + 'Words are valid' + ); } public function testSetValue() { @@ -84,6 +97,18 @@ class CurrencyFieldTest extends SapphireTest { $f->value, '$2.50', 'Truncates long decimal portions' ); + + $f->setValue('test123.00test'); + $this->assertEquals( + $f->value, '$123.00', + 'Strips alpha values' + ); + + $f->setValue('test'); + $this->assertEquals( + $f->value, '$0.00', + 'Does not set alpha values' + ); } public function testDataValue() { diff --git a/tests/forms/DropdownFieldTest.php b/tests/forms/DropdownFieldTest.php index 4483660e8..ef131e893 100644 --- a/tests/forms/DropdownFieldTest.php +++ b/tests/forms/DropdownFieldTest.php @@ -310,4 +310,27 @@ class DropdownFieldTest extends SapphireTest { return $foundDisabled; } + public function testValidation() { + $field = DropdownField::create('Test', 'Testing', array( + "One" => "One", + "Two" => "Two" + )); + $validator = new RequiredFields(); + $form = new Form($this, 'Form', new FieldList($field), new FieldList(), $validator); + $field->setValue("One"); + $this->assertTrue($field->validate($validator)); + $field->setName("TestNew"); //try changing name of field + $this->assertTrue($field->validate($validator)); + //non-existent value should make the field invalid + $field->setValue("Three"); + $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)); + } + } diff --git a/tests/forms/FormFieldTest.php b/tests/forms/FormFieldTest.php index dabc44d87..7441222a4 100644 --- a/tests/forms/FormFieldTest.php +++ b/tests/forms/FormFieldTest.php @@ -3,7 +3,7 @@ * @package framework * @subpackage tests */ -class FormFieldTest extends SapphireTest { +class FormFieldTest extends SapphireTest implements TestOnly { public function testAddExtraClass() { $field = new FormField('MyField'); @@ -187,4 +187,4 @@ class FormFieldTest extends SapphireTest { } } -} +} \ No newline at end of file diff --git a/tests/forms/ListboxFieldTest.php b/tests/forms/ListboxFieldTest.php index bea37e83c..f2a7d4ad7 100644 --- a/tests/forms/ListboxFieldTest.php +++ b/tests/forms/ListboxFieldTest.php @@ -194,6 +194,76 @@ class ListboxFieldTest extends SapphireTest { $field = new ListboxField('Choices', 'Choices', $choices); } + public function testValidationWithArray() { + //test with array input + $field = ListboxField::create('Test', 'Testing', array( + 1 => "One", + 2 => "Two", + 3 => "Three" + )); + $validator = new RequiredFields(); + + $field->setValue(1); + $this->assertTrue( + $field->validate($validator), + 'Validates values in source map' + ); + $field->setMultiple(true); + $field->setValue(array(1)); + $this->assertTrue( + $field->validate($validator), + 'Validates values within source array' + ); + //non valid value should fail + $field->setValue(4); + $this->assertFalse( + $field->validate($validator), + 'Does not validates values not within source array' + ); + } + + public function testValidationWithDataList() { + //test with datalist input + $tag1 = $this->objFromFixture('ListboxFieldTest_Tag', 'tag1'); + $tag2 = $this->objFromFixture('ListboxFieldTest_Tag', 'tag2'); + $tag3 = $this->objFromFixture('ListboxFieldTest_Tag', 'tag3'); + $field = ListboxField::create('Test', 'Testing', DataObject::get("ListboxFieldTest_Tag")->map()->toArray()); + $validator = new RequiredFields(); + + $field->setValue( + $tag1->ID + ); + $this->assertTrue( + $field->validate($validator), + 'Field validates values in source map' + ); + + /** + * @todo re-enable these tests when field validation is removed from {@link ListboxField::setValue()} and moved + * to the {@link ListboxField::validate()} function + */ +// $field->setValue(4); +// $this->assertFalse( +// $field->validate($validator), +// 'Field does not validate values outside of source map' +// ); + $field->setMultiple(true); + $field->setValue(false, new ArrayData(array( + $tag1->ID => $tag1->ID, + $tag2->ID => $tag2->ID + ))); + $this->assertTrue( + $field->validate($validator), + 'Validates values in source map' + ); + //invalid value should fail + $field->setValue(4); + $this->assertFalse( + $field->validate($validator), + 'Does not validate values not within source map' + ); + } + } class ListboxFieldTest_DataObject extends DataObject implements TestOnly { diff --git a/tests/forms/MemberDatetimeOptionsetFieldTest.php b/tests/forms/MemberDatetimeOptionsetFieldTest.php index 22938737a..035efd9fe 100644 --- a/tests/forms/MemberDatetimeOptionsetFieldTest.php +++ b/tests/forms/MemberDatetimeOptionsetFieldTest.php @@ -89,11 +89,12 @@ class MemberDatetimeOptionsetFieldTest extends SapphireTest { public function testDateFormValid() { $field = new MemberDatetimeOptionsetField('DateFormat', 'DateFormat'); - $this->assertTrue($field->validate(null)); + $validator = new RequiredFields(); + $this->assertTrue($field->validate($validator)); $_POST['DateFormat_custom'] = 'dd MM yyyy'; - $this->assertTrue($field->validate(null)); + $this->assertTrue($field->validate($validator)); $_POST['DateFormat_custom'] = 'sdfdsfdfd1244'; - $this->assertFalse($field->validate(null)); + $this->assertFalse($field->validate($validator)); } } diff --git a/tests/forms/NumericFieldTest.php b/tests/forms/NumericFieldTest.php index b856cdb98..8c1f2cee7 100644 --- a/tests/forms/NumericFieldTest.php +++ b/tests/forms/NumericFieldTest.php @@ -14,7 +14,7 @@ class NumericFieldTest extends SapphireTest { $field = new NumericField('Number'); $field->setValue('12.00'); - $validator = new RequiredFields('Number'); + $validator = new RequiredFields(); $this->assertTrue($field->validate($validator)); $field->setValue('12,00'); @@ -24,7 +24,7 @@ class NumericFieldTest extends SapphireTest { $this->assertTrue($field->validate($validator)); $field->setValue(false); - $this->assertFalse($field->validate($validator)); + $this->assertTrue($field->validate($validator)); i18n::set_locale('de_DE'); $field->setValue('12,00'); diff --git a/tests/forms/uploadfield/UploadFieldTest.php b/tests/forms/uploadfield/UploadFieldTest.php index 3a801db89..1c982984d 100644 --- a/tests/forms/uploadfield/UploadFieldTest.php +++ b/tests/forms/uploadfield/UploadFieldTest.php @@ -463,7 +463,7 @@ class UploadFieldTest extends FunctionalTest { } public function testEdit() { - $this->loginWithPermission('ADMIN'); + $memberID = $this->loginWithPermission('ADMIN'); $record = $this->objFromFixture('UploadFieldTest_Record', 'record1'); $file4 = $this->objFromFixture('File', 'file4'); @@ -474,7 +474,13 @@ class UploadFieldTest extends FunctionalTest { $response = $this->get($baseUrl . '/edit'); $this->assertFalse($response->isError()); - $response = $this->post($baseUrl . '/EditForm', array('Title' => 'File 4 modified')); + $response = $this->post( + $baseUrl . '/EditForm', + array( + 'Title' => 'File 4 modified', + 'OwnerID' => $memberID + ) + ); $this->assertFalse($response->isError()); $record = DataObject::get_by_id($record->class, $record->ID, false);