BUGFIX Consistenly returning cloned instances for all FormField classes when calling performReadonlyTransformation() or performDisabledTransformation(). Making sure that these instances are actually flagged as readyonly/disabled. Addd unit tests to dynamically instanciate most FormField classes to check for this behaviour. Originally, this bugfix was necessary to avoid changed FormField state when recursively calling replaceField() on FieldSet->dataFields() in Translatable->updateCMSFields()

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/branches/2.3@66505 467b73ca-7a2a-4603-9d3b-597d59a354a9
This commit is contained in:
Ingo Schommer 2008-11-24 07:25:43 +00:00 committed by Sam Minnee
parent 3431e9815b
commit 916df369d8
19 changed files with 150 additions and 52 deletions

View File

@ -82,8 +82,9 @@ HTML;
}
function performDisabledTransformation() {
$this->disabled = true;
return $this;
$clone = clone $this;
$clone->setDisabled(true);
return $clone;
}
}
@ -94,7 +95,7 @@ HTML;
*/
class CheckboxField_Readonly extends ReadonlyField {
function performReadonlyTransformation() {
return $this;
return clone $this;
}
function setValue($val) {

View File

@ -159,8 +159,9 @@ class CheckboxSetField extends OptionsetField {
}
function performDisabledTransformation() {
$this->setDisabled(true);
return $this;
$clone = clone $this;
$clone->setDisabled(true);
return $clone;
}
/**

View File

@ -65,6 +65,13 @@ class CompositeField extends FormField {
return $this->children;
}
/**
* @param FieldSet $children
*/
public function setChildren($children) {
$this->children = $children;
}
/**
* Returns the fields nested inside another DIV
*/
@ -207,14 +214,15 @@ class CompositeField extends FormField {
*/
public function performReadonlyTransformation() {
$newChildren = new FieldSet();
foreach($this->children as $idx => $child) {
$clone = clone $this;
foreach($clone->getChildren() as $idx => $child) {
if(is_object($child)) $child = $child->transform(new ReadonlyTransformation());
$newChildren->push($child, $idx);
}
$this->children = $newChildren;
$this->readonly = true;
return $this;
$clone->children = $newChildren;
$clone->readonly = true;
return $clone;
}
/**
@ -223,17 +231,18 @@ class CompositeField extends FormField {
*/
public function performDisabledTransformation($trans) {
$newChildren = new FieldSet();
if($this->children) foreach($this->children as $idx => $child) {
$clone = clone $this;
if($clone->getChildren()) foreach($clone->getChildren() as $idx => $child) {
if(is_object($child)) {
$child = $child->transform($trans);
}
$newChildren->push($child, $idx);
}
$this->children = $newChildren;
$this->readonly = true;
$clone->children = $newChildren;
$clone->readonly = true;
return $this;
return $clone;
}
function IsReadonly() {

View File

@ -101,7 +101,7 @@ class CurrencyField_Readonly extends ReadonlyField{
* This already is a readonly field.
*/
function performReadonlyTransformation() {
return $this;
return clone $this;
}
}
@ -113,6 +113,8 @@ class CurrencyField_Readonly extends ReadonlyField{
*/
class CurrencyField_Disabled extends CurrencyField{
protected $disabled = true;
/**
* overloaded to display the correctly formated value for this datatype
*/

View File

@ -53,7 +53,9 @@ class DatalessField extends FormField {
* Returns a readonly version of this field
*/
function performReadonlyTransformation() {
return $this;
$clone = clone $this;
$clone->setReadonly(true);
return $clone;
}
/**

View File

@ -91,8 +91,9 @@ class FormAction extends FormField {
* Globally disabled buttons would break the CMS.
*/
function performReadonlyTransformation() {
$this->setDisabled(true);
return $this;
$clone = clone $this;
$clone->setReadonly(true);
return $clone;
}
function readonlyField() {

View File

@ -432,12 +432,13 @@ HTML;
* Return a disabled version of this field
*/
function performDisabledTransformation() {
$disabledClassName = $this->class . '_Disabled';
$clone = clone $this;
$disabledClassName = $clone->class . '_Disabled';
if( ClassInfo::exists( $disabledClassName ) )
return new $disabledClassName( $this->name, $this->title, $this->value );
elseif($this->hasMethod('setDisabled')){
$this->setDisabled(true);
return $this;
elseif($clone->hasMethod('setDisabled')){
$clone->setDisabled(true);
return $clone;
}else{
return $this->performReadonlyTransformation();
}

View File

@ -16,8 +16,9 @@ class HiddenField extends FormField {
return $this->Field();
}
function performReadonlyTransformation() {
$this->setReadonly(true);
return $this;
$clone = clone $this;
$clone->setReadonly(true);
return $clone;
}
function IsHidden() {

View File

@ -53,8 +53,7 @@ class ImageField extends FileField {
* Returns a readonly version of this field
*/
function performReadonlyTransformation() {
$field = new SimpleImageField_Disabled($this->name, $this->title,
$this->value);
$field = new SimpleImageField_Disabled($this->name, $this->title, $this->value);
$field->setForm($this->form);
return $field;
}

View File

@ -42,8 +42,9 @@ class LiteralField extends DatalessField {
}
function performReadonlyTransformation() {
$this->setReadonly(true);
return $this;
$clone = clone $this;
$clone->setReadonly(true);
return $clone;
}
}

View File

@ -42,7 +42,8 @@ class LookupField extends DropdownField {
}
function performReadonlyTransformation() {
return $this;
$clone = clone $this;
return $clone;
}
function Type() {

View File

@ -9,7 +9,7 @@ class ReadonlyField extends FormField {
protected $readonly = true;
function performReadonlyTransformation() {
return $this;
return clone $this;
}
}
?>

View File

@ -11,7 +11,7 @@ class RestrictedTextField extends TextField {
function __construct($name, $title = null, $value = "", $restrictedChars = "", $maxLength = null){
$this->restrictedChars = $restrictedChars;
parent::__construct($name, $title, $value, $form);
parent::__construct($name, $title, $value);
}
function Field() {

View File

@ -31,16 +31,17 @@ class SelectionGroup extends CompositeField {
*/
public function performDisabledTransformation($trans) {
$newChildren = array();
if($this->children) foreach($this->children as $idx => $child) {
$clone = clone $this;
if($clone->children) foreach($clone->getChildren() as $idx => $child) {
if(is_object($child)) {
$child = $child->transform($trans);
}
$newChildren[$idx] = $child;
}
$this->children = new FieldSet($newChildren);
$this->readonly = true;
return $this;
$clone->setChildren(new FieldSet($newChildren));
$clone->setReadonly(true);
return $clone;
}
function FieldSet() {

View File

@ -81,6 +81,10 @@ class SimpleImageField extends FileField {
*/
class SimpleImageField_Disabled extends FormField {
protected $disabled = true;
protected $readonly = true;
function Field() {
$record = $this->form->getRecord();
$fieldName = $this->name;

View File

@ -321,15 +321,17 @@ class TableField extends TableListField {
}
function performReadonlyTransformation() {
$this->permissions = array('show');
$this->setReadonly(true);
return $this;
$clone = clone $this;
$clone->permissions = array('show');
$clone->setReadonly(true);
return $clone;
}
function performDisabledTransformation() {
$this->permissions = array('show');
$this->setDisabled(true);
return $this;
$clone = clone $this;
$clone->setPermissions(array('show'));
$clone->setDisabled(true);
return $clone;
}
/**

View File

@ -512,11 +512,12 @@ JS
}
function performReadonlyTransformation() {
$this->setShowPagination(false);
$this->setPermissions(array('show'));
$this->addExtraClass( 'readonly' );
$this->setReadonly(true);
return $this;
$clone = clone $this;
$clone->setShowPagination(false);
$clone->setPermissions(array('show'));
$clone->addExtraClass( 'readonly' );
$clone->setReadonly(true);
return $clone;
}
/**

View File

@ -71,9 +71,10 @@ class TextareaField extends FormField {
* The element shouldn't be both disabled and readonly at the same time.
*/
function performReadonlyTransformation() {
$this->readonly = true;
$this->disabled = false;
return $this;
$clone = clone $this;
$clone->setReadonly(true);
$clone->setDisabled(false);
return $clone;
}
/**
@ -83,9 +84,10 @@ class TextareaField extends FormField {
* The element shouldn't be both disabled and readonly at the same time.
*/
function performDisabledTransformation() {
$this->disabled = true;
$this->readonly = false;
return $this;
$clone = clone $this;
$clone->setDisabled(true);
$clone->setReadonly(false);
return $clone;
}
function Type() {

View File

@ -0,0 +1,69 @@
<?php
/**
* @package sapphire
* @subpackage tests
*/
class FormFieldTest extends SapphireTest {
function testEveryFieldTransformsReadonlyAsClone() {
$fieldClasses = ClassInfo::subclassesFor('FormField');
foreach($fieldClasses as $fieldClass) {
$reflectionClass = new ReflectionClass($fieldClass);
if(!$reflectionClass->isInstantiable()) continue;
$constructor = $reflectionClass->getMethod('__construct');
if($constructor->getNumberOfRequiredParameters() > 1) continue;
if($fieldClass == 'CompositeField' || is_subclass_of($fieldClass, 'CompositeField')) continue;
$instance = new $fieldClass("{$fieldClass}_instance");
$isReadonlyBefore = $instance->isReadonly();
$readonlyInstance = $instance->performReadonlyTransformation();
$this->assertEquals(
$isReadonlyBefore,
$instance->isReadonly(),
"FormField class '{$fieldClass} retains its readonly state after calling performReadonlyTransformation()"
);
$this->assertTrue(
$readonlyInstance->isReadonly(),
"FormField class '{$fieldClass} returns a valid readonly representation as of isReadonly()"
);
$this->assertNotSame(
$readonlyInstance,
$instance,
"FormField class '{$fieldClass} returns a valid cloned readonly representation"
);
}
}
function testEveryFieldTransformsDisabledAsClone() {
$fieldClasses = ClassInfo::subclassesFor('FormField');
foreach($fieldClasses as $fieldClass) {
$reflectionClass = new ReflectionClass($fieldClass);
if(!$reflectionClass->isInstantiable()) continue;
$constructor = $reflectionClass->getMethod('__construct');
if($constructor->getNumberOfRequiredParameters() > 1) continue;
if($fieldClass == 'CompositeField' || is_subclass_of($fieldClass, 'CompositeField')) continue;
$instance = new $fieldClass("{$fieldClass}_instance");
$isDisabledBefore = $instance->isDisabled();
$disabledInstance = $instance->performDisabledTransformation();
$this->assertEquals(
$isDisabledBefore,
$instance->isDisabled(),
"FormField class '{$fieldClass} retains its disabled state after calling performDisabledTransformation()"
);
$this->assertTrue(
$disabledInstance->isDisabled(),
"FormField class '{$fieldClass} returns a valid disabled representation as of isDisabled()"
);
$this->assertNotSame(
$disabledInstance,
$instance,
"FormField class '{$fieldClass} returns a valid cloned disabled representation"
);
}
}
}
?>