mirror of
https://github.com/silverstripe/silverstripe-framework
synced 2024-10-22 14:05:37 +02:00
removeRequiredField() limits field (fixes #2165)
Added tests to RequiredFields and fixed bugs that were found Now you: 1. Can't add the same field name many times 2. Can use append RequiredFields correctly without fear of duplicates I've also added a Deprecation warning to $useLabels as it's not used *anywhere* in framework
This commit is contained in:
parent
d3aa38f4b4
commit
6c943007a1
@ -19,16 +19,23 @@ class RequiredFields extends Validator {
|
||||
* to the constructor of this object. (an array of elements are ok)
|
||||
*/
|
||||
public function __construct() {
|
||||
$Required = func_get_args();
|
||||
if( isset($Required[0]) && is_array( $Required[0] ) )
|
||||
$Required = $Required[0];
|
||||
$this->required = $Required;
|
||||
$required = func_get_args();
|
||||
if(isset($required[0]) && is_array($required[0])) {
|
||||
$required = $required[0];
|
||||
}
|
||||
if(!empty($required)) {
|
||||
$this->required = ArrayLib::valuekey($required);
|
||||
} else {
|
||||
$this->required = array();
|
||||
}
|
||||
|
||||
parent::__construct();
|
||||
}
|
||||
|
||||
public function useLabels($flag) {
|
||||
Deprecation::notice('3.2', 'useLabels will be removed from 3.2, please do not use it or implement it yourself');
|
||||
$this->useLabels = $flag;
|
||||
return $this;
|
||||
}
|
||||
|
||||
/**
|
||||
@ -36,6 +43,7 @@ class RequiredFields extends Validator {
|
||||
*/
|
||||
public function removeValidation(){
|
||||
$this->required = array();
|
||||
return $this;
|
||||
}
|
||||
|
||||
/**
|
||||
@ -74,7 +82,7 @@ class RequiredFields extends Validator {
|
||||
// submitted data for file upload fields come back as an array
|
||||
$value = isset($data[$fieldName]) ? $data[$fieldName] : null;
|
||||
if(is_array($value)) {
|
||||
if ($formField instanceof FileField && isset($value['error']) && $value['error']) {
|
||||
if($formField instanceof FileField && isset($value['error']) && $value['error']) {
|
||||
$error = true;
|
||||
} else {
|
||||
$error = (count($value)) ? false : true;
|
||||
@ -115,22 +123,21 @@ class RequiredFields extends Validator {
|
||||
* Add's a single required field to requiredfields stack
|
||||
*/
|
||||
public function addRequiredField( $field ) {
|
||||
$this->required[] = $field;
|
||||
$this->required[$field] = $field;
|
||||
return $this;
|
||||
}
|
||||
|
||||
public function removeRequiredField($field) {
|
||||
foreach ($this->required as $i => $required) {
|
||||
if ($field == $required) {
|
||||
array_splice($this->required, $i);
|
||||
}
|
||||
}
|
||||
unset($this->required[$field]);
|
||||
return $this;
|
||||
}
|
||||
|
||||
/**
|
||||
* allows you too add more required fields to this object after construction.
|
||||
*/
|
||||
public function appendRequiredFields($requiredFields){
|
||||
$this->required = array_merge($this->required,$requiredFields->getRequired());
|
||||
$this->required = $this->required + ArrayLib::valuekey($requiredFields->getRequired());
|
||||
return $this;
|
||||
}
|
||||
|
||||
/**
|
||||
@ -138,14 +145,14 @@ class RequiredFields extends Validator {
|
||||
* Used by FormField to return a value for FormField::Required(), to do things like show *s on the form template.
|
||||
*/
|
||||
public function fieldIsRequired($fieldName) {
|
||||
return in_array($fieldName, $this->required);
|
||||
return isset($this->required[$fieldName]);
|
||||
}
|
||||
|
||||
/**
|
||||
* getter function for append
|
||||
*/
|
||||
public function getRequired(){
|
||||
return $this->required;
|
||||
return array_values($this->required);
|
||||
}
|
||||
}
|
||||
|
||||
|
278
tests/forms/RequiredFieldsTest.php
Normal file
278
tests/forms/RequiredFieldsTest.php
Normal file
@ -0,0 +1,278 @@
|
||||
<?php
|
||||
/**
|
||||
* @package framework
|
||||
* @subpackage tests
|
||||
*
|
||||
* @todo Test the validation method php()
|
||||
*/
|
||||
class RequiredFieldsTest extends SapphireTest {
|
||||
|
||||
public function testConstructingWithArray() {
|
||||
//can we construct with an array?
|
||||
$fields = array(
|
||||
'Title',
|
||||
'Content',
|
||||
'Image',
|
||||
'AnotherField'
|
||||
);
|
||||
$requiredFields = new RequiredFields($fields);
|
||||
//check the fields and the array match
|
||||
$this->assertEquals(
|
||||
$fields,
|
||||
$requiredFields->getRequired(),
|
||||
"Failed to set the required fields using an array"
|
||||
);
|
||||
}
|
||||
|
||||
public function testConstructingWithArguments() {
|
||||
//can we construct with arguments?
|
||||
$requiredFields = new RequiredFields(
|
||||
'Title',
|
||||
'Content',
|
||||
'Image',
|
||||
'AnotherField'
|
||||
);
|
||||
//check the fields match
|
||||
$this->assertEquals(
|
||||
array(
|
||||
'Title',
|
||||
'Content',
|
||||
'Image',
|
||||
'AnotherField'
|
||||
),
|
||||
$requiredFields->getRequired(),
|
||||
"Failed to set the required fields using arguments"
|
||||
);
|
||||
}
|
||||
|
||||
public function testRemoveValidation() {
|
||||
//can we remove all fields at once?
|
||||
$requiredFields = new RequiredFields(
|
||||
'Title',
|
||||
'Content',
|
||||
'Image',
|
||||
'AnotherField'
|
||||
);
|
||||
$requiredFields->removeValidation();
|
||||
//check there are no required fields
|
||||
$this->assertEmpty(
|
||||
$requiredFields->getRequired(),
|
||||
"Failed to remove all the required fields using 'removeValidation()'"
|
||||
);
|
||||
}
|
||||
|
||||
public function testRemoveRequiredField() {
|
||||
//set up the required fields
|
||||
$requiredFields = new RequiredFields(
|
||||
'Title',
|
||||
'Content',
|
||||
'Image',
|
||||
'AnotherField'
|
||||
);
|
||||
//remove one
|
||||
$requiredFields->removeRequiredField('Content');
|
||||
//compare the arrays
|
||||
$this->assertEquals(
|
||||
array(
|
||||
'Title',
|
||||
'Image',
|
||||
'AnotherField'
|
||||
),
|
||||
$requiredFields->getRequired(),
|
||||
"Failed to remove the 'Content' field from required list"
|
||||
);
|
||||
//let's remove another
|
||||
$requiredFields->removeRequiredField('Title');
|
||||
$this->assertEquals(
|
||||
array(
|
||||
'Image',
|
||||
'AnotherField'
|
||||
),
|
||||
$requiredFields->getRequired(),
|
||||
"Failed to remove 'Title' field from required list"
|
||||
);
|
||||
//lets try to remove one that doesn't exist
|
||||
$requiredFields->removeRequiredField('DontExists');
|
||||
$this->assertEquals(
|
||||
array(
|
||||
'Image',
|
||||
'AnotherField'
|
||||
),
|
||||
$requiredFields->getRequired(),
|
||||
"Removing a non-existant field from required list altered the list of required fields"
|
||||
);
|
||||
}
|
||||
|
||||
public function testAddRequiredField() {
|
||||
//set up the validator
|
||||
$requiredFields = new RequiredFields(
|
||||
'Title'
|
||||
);
|
||||
//add a field
|
||||
$requiredFields->addRequiredField('Content');
|
||||
//check it was added
|
||||
$this->assertEquals(
|
||||
array(
|
||||
'Title',
|
||||
'Content'
|
||||
),
|
||||
$requiredFields->getRequired(),
|
||||
"Failed to add a new field to the required list"
|
||||
);
|
||||
//add another for good measure
|
||||
$requiredFields->addRequiredField('Image');
|
||||
//check it was added
|
||||
$this->assertEquals(
|
||||
array(
|
||||
'Title',
|
||||
'Content',
|
||||
'Image'
|
||||
),
|
||||
$requiredFields->getRequired(),
|
||||
"Failed to add a second new field to the required list"
|
||||
);
|
||||
//remove a field
|
||||
$requiredFields->removeRequiredField('Title');
|
||||
//check it was removed
|
||||
$this->assertEquals(
|
||||
array(
|
||||
'Content',
|
||||
'Image'
|
||||
),
|
||||
$requiredFields->getRequired(),
|
||||
"Failed to remove 'Title' field from required list"
|
||||
);
|
||||
//add the same field back to check we can add and remove at will
|
||||
$requiredFields->addRequiredField('Title');
|
||||
//check it's in there
|
||||
$this->assertEquals(
|
||||
array(
|
||||
'Content',
|
||||
'Image',
|
||||
'Title'
|
||||
),
|
||||
$requiredFields->getRequired(),
|
||||
"Failed to add 'Title' back to the required field list"
|
||||
);
|
||||
//add a field that already exists (we can't have the same field twice, can we?)
|
||||
$requiredFields->addRequiredField('Content');
|
||||
//check the field wasn't added
|
||||
$this->assertEquals(
|
||||
array(
|
||||
'Content',
|
||||
'Image',
|
||||
'Title'
|
||||
),
|
||||
$requiredFields->getRequired(),
|
||||
"Adding a duplicate field to required field list had unexpected behaviour"
|
||||
);
|
||||
}
|
||||
|
||||
public function testAppendRequiredFields() {
|
||||
//get the validator
|
||||
$requiredFields = new RequiredFields(
|
||||
'Title',
|
||||
'Content',
|
||||
'Image',
|
||||
'AnotherField'
|
||||
);
|
||||
//create another validator with other fields
|
||||
$otherRequiredFields = new RequiredFields(array(
|
||||
'ExtraField1',
|
||||
'ExtraField2'
|
||||
));
|
||||
//append the new fields
|
||||
$requiredFields->appendRequiredFields($otherRequiredFields);
|
||||
//check they were added correctly
|
||||
$this->assertEquals(
|
||||
array(
|
||||
'Title',
|
||||
'Content',
|
||||
'Image',
|
||||
'AnotherField',
|
||||
'ExtraField1',
|
||||
'ExtraField2'
|
||||
),
|
||||
$requiredFields->getRequired(),
|
||||
"Merging of required fields failed to behave as expected"
|
||||
);
|
||||
// create the standard validator so we can check duplicates are ignored
|
||||
$otherRequiredFields = new RequiredFields(
|
||||
'Title',
|
||||
'Content',
|
||||
'Image',
|
||||
'AnotherField'
|
||||
);
|
||||
//add the new validator
|
||||
$requiredFields->appendRequiredFields($otherRequiredFields);
|
||||
//check nothing was changed
|
||||
$this->assertEquals(
|
||||
array(
|
||||
'Title',
|
||||
'Content',
|
||||
'Image',
|
||||
'AnotherField',
|
||||
'ExtraField1',
|
||||
'ExtraField2'
|
||||
),
|
||||
$requiredFields->getRequired(),
|
||||
"Merging of required fields with duplicates failed to behave as expected"
|
||||
);
|
||||
//add some new fields and some old ones in a strange order
|
||||
$otherRequiredFields = new RequiredFields(
|
||||
'ExtraField3',
|
||||
'Title',
|
||||
'ExtraField4',
|
||||
'Image',
|
||||
'Content'
|
||||
);
|
||||
//add the new validator
|
||||
$requiredFields->appendRequiredFields($otherRequiredFields);
|
||||
//check that only the new fields were added
|
||||
$this->assertEquals(
|
||||
array(
|
||||
'Title',
|
||||
'Content',
|
||||
'Image',
|
||||
'AnotherField',
|
||||
'ExtraField1',
|
||||
'ExtraField2',
|
||||
'ExtraField3',
|
||||
'ExtraField4'
|
||||
),
|
||||
$requiredFields->getRequired(),
|
||||
"Merging of required fields with some duplicates in a muddled order failed to behave as expected"
|
||||
);
|
||||
}
|
||||
|
||||
public function testFieldIsRequired() {
|
||||
//get the validator
|
||||
$requiredFields = new RequiredFields($fieldNames = array(
|
||||
'Title',
|
||||
'Content',
|
||||
'Image',
|
||||
'AnotherField'
|
||||
));
|
||||
|
||||
foreach($fieldNames as $field) {
|
||||
$this->assertTrue(
|
||||
$requiredFields->fieldIsRequired($field),
|
||||
sprintf("Failed to find '%s' field in required list", $field)
|
||||
);
|
||||
}
|
||||
|
||||
//add a new field
|
||||
$requiredFields->addRequiredField('ExtraField1');
|
||||
//check the new field is required
|
||||
$this->assertTrue(
|
||||
$requiredFields->fieldIsRequired('ExtraField1'),
|
||||
"Failed to find 'ExtraField1' field in required list after adding it to the list"
|
||||
);
|
||||
//check a non-existant field returns false
|
||||
$this->assertFalse(
|
||||
$requiredFields->fieldIsRequired('DoesntExist'),
|
||||
"Unexpectedly returned true for a non-existant field"
|
||||
);
|
||||
}
|
||||
|
||||
}
|
Loading…
Reference in New Issue
Block a user