diff --git a/forms/CompositeField.php b/forms/CompositeField.php index 8257b1dfe..829e7e903 100755 --- a/forms/CompositeField.php +++ b/forms/CompositeField.php @@ -32,6 +32,7 @@ class CompositeField extends FormField { $children = is_array(func_get_args()) ? func_get_args() : array(); $this->children = new FieldSet($children); } + $this->children->setContainerField($this); Object::__construct(); } @@ -148,13 +149,16 @@ class CompositeField extends FormField { * Add a new child field to the end of the set. */ public function push(FormField $field) { + //$this->rootFieldSet()->removeByName($field->Name()); $this->children->push($field); } public function insertBefore($field, $insertBefore) { + //$this->rootFieldSet()->removeByName($field->Name()); return $this->children->insertBefore($field, $insertBefore); } public function insertBeforeRecursive($field, $insertBefore, $level = 0) { + //$this->rootFieldSet()->removeByName($field->Name()); return $this->children->insertBeforeRecursive($field, $insertBefore, $level+1); } public function removeByName($fieldName) { diff --git a/forms/FieldSet.php b/forms/FieldSet.php index 41dc8dd40..c0777657d 100755 --- a/forms/FieldSet.php +++ b/forms/FieldSet.php @@ -17,6 +17,18 @@ class FieldSet extends DataObjectSet { protected $sequentialSet; protected $sequentialSaveableSet; + public function __construct($items = null) { + // if the first parameter is not an array, or we have more than one parameter, collate all parameters to an array + // otherwise use the passed array + $itemsArr = (!is_array($items) || count(func_get_args()) > 1) ? func_get_args() : $items; + parent::__construct($itemsArr); + + foreach($this->items as $item) { + $item->setContainerFieldSet($this); + } + + } + /** * Return a sequential set of all fields that have data. This excludes wrapper composite fields * as well as heading / help text fields. @@ -56,10 +68,11 @@ class FieldSet extends DataObjectSet { /** * Add an extra field to a tab within this fieldset. * This is most commonly used when overloading getCMSFields() - * @param tabName The name of the tab or tabset. Subtabs can be referred to as TabSet.Tab or TabSet.Tab.Subtab. + * + * @param string $tabName The name of the tab or tabset. Subtabs can be referred to as TabSet.Tab or TabSet.Tab.Subtab. * This function will create any missing tabs. - * @param field The {@link FormField} object to add to the end of that tab. - * @param insertBefore The name of the field to insert before. Optional. + * @param FormField $field The {@link FormField} object to add to the end of that tab. + * @param string $insertBefore The name of the field to insert before. Optional. */ public function addFieldToTab($tabName, $field, $insertBefore = null) { // This is a cache that must be flushed @@ -67,14 +80,43 @@ class FieldSet extends DataObjectSet { // Find the tab $tab = $this->findOrMakeTab($tabName); - + // Add the field to the end of this set if($insertBefore) $tab->insertBefore($field, $insertBefore); else $tab->push($field); } + + /** + * Add a number of extra fields to a tab within this fieldset. + * This is most commonly used when overloading getCMSFields() + * + * @param string $tabName The name of the tab or tabset. Subtabs can be referred to as TabSet.Tab or TabSet.Tab.Subtab. + * This function will create any missing tabs. + * @param array $fields An array of {@link FormField} objects. + */ + public function addFieldsToTab($tabName, $fields) { + $this->sequentialSet = null; + + // Find the tab + $tab = $this->findOrMakeTab($tabName); + + // Add the fields to the end of this set + foreach($fields as $field) { + // Check if a field by the same name exists in this tab + if($tab->fieldByName($field->Name())) { + // It exists, so we need to replace the old one + $this->replaceField($field->Name(), $field); + } else { + $tab->push($field); + } + } + } /** * Remove the given field from the given tab in the field. + * + * @param string $tabName The name of the tab + * @param string $fieldName The name of the field */ public function removeFieldFromTab($tabName, $fieldName) { // This is a cache that must be flushed @@ -103,14 +145,14 @@ class FieldSet extends DataObjectSet { } /** - * Remove a field from this fieldset by name. - * It musn't be buried in a composite field.--- changed - * It could be buried in a composite field now. --- 5/09/2006 + * Remove a field from this FieldSet by Name. + * The field could also be inside a CompositeField. + * + * @param string $fieldName The name of the field */ public function removeByName($fieldName) { foreach($this->items as $i => $child) { if(is_object($child) && ($child->Name() == $fieldName || $child->Title() == $fieldName)) { - // unset($this->items[$i]); array_splice( $this->items, $i, 1 ); break; } else if($child->isComposite()) $child->removeByName($fieldName); @@ -118,23 +160,6 @@ class FieldSet extends DataObjectSet { } } - /** - * Add a number of extra fields to a tab within this fieldset. - * This is most commonly used when overloading getCMSFields() - * @param tabName The name of the tab or tabset. Subtabs can be referred to as TabSet.Tab or TabSet.Tab.Subtab. - * This function will create any missing tabs. - * @param fields An array of {@link FormField} objects. - */ - public function addFieldsToTab($tabName, $fields) { - $this->sequentialSet = null; - - // Find the tab - $tab = $this->findOrMakeTab($tabName); - - // Add the fields to the end of this set - foreach($fields as $field) $tab->push($field); - } - /** * Replace a single field with another. * @@ -159,9 +184,9 @@ class FieldSet extends DataObjectSet { return false; } - /** * Returns the specified tab object, creating it if necessary. + * * @param tabName The tab to return, in the form "Tab.Subtab.Subsubtab" */ protected function findOrMakeTab($tabName) { @@ -174,7 +199,7 @@ class FieldSet extends DataObjectSet { $currentPointer = $currentPointer->fieldByName($part); // Create any missing tabs if(!$currentPointer) { - if(is_a($parentPointer,'TabSet')) { + if(is_a($parentPointer, 'TabSet')) { $currentPointer = new Tab($part); $parentPointer->push($currentPointer); } else { @@ -184,7 +209,6 @@ class FieldSet extends DataObjectSet { } return $currentPointer; - } /** @@ -220,7 +244,8 @@ class FieldSet extends DataObjectSet { * @param string $name Name of the field to insert before */ public function insertBefore($item, $name) { - if($this->sequentialSet) $this->sequentialSet = null; + $this->beforeInsert($item); + $item->setContainerFieldSet($this); $i = 0; foreach($this->items as $child) { @@ -244,7 +269,9 @@ class FieldSet extends DataObjectSet { * @param int $level For internal use only, should not be passed */ public function insertBeforeRecursive($item, $name, $level = 0) { - if($this->sequentialSet) $this->sequentialSet = null; + $this->beforeInsert($item); + $item->setContainerFieldSet($this); + $i = 0; foreach($this->items as $child) { if($name == $child->Name() || $name == $child->id) { @@ -271,7 +298,8 @@ class FieldSet extends DataObjectSet { * @param string $name Name of the field to insert after */ public function insertAfter($item, $name) { - if($this->sequentialSet) $this->sequentialSet = null; + $this->beforeInsert($item); + $item->setContainerFieldSet($this); $i = 0; foreach($this->items as $child) { @@ -291,14 +319,24 @@ class FieldSet extends DataObjectSet { * @param string $key An option array key (field name) */ public function push($item, $key = null) { - if($this->sequentialSet) $this->sequentialSet = null; + $this->beforeInsert($item); + $item->setContainerFieldSet($this); return parent::push($item, $key = null); } + /** + * Handler method called before the FieldSet is going to be manipulated. + */ + function beforeInsert($item) { + if($this->sequentialSet) $this->sequentialSet = null; + $this->rootFieldSet()->removeByName($item->Name()); + } + + /** * Set the Form instance for this FieldSet. * - * @param Form $form + * @param Form $form The form to set this FieldSet to */ public function setForm($form) { foreach($this as $field) $field->setForm($form); @@ -306,7 +344,8 @@ class FieldSet extends DataObjectSet { /** * Load the given data into this form. - * @param data An map of data to load into the FieldSet. + * + * @param data An map of data to load into the FieldSet */ public function setValues($data) { foreach($this->dataFields() as $field) { @@ -347,6 +386,18 @@ class FieldSet extends DataObjectSet { } return $newFields; } + + /** + * Returns the root field set that this belongs to + */ + function rootFieldSet() { + if($this->containerField) return $this->containerField->rootFieldSet(); + else return $this; + } + + function setContainerField($field) { + $this->containerField = $field; + } /** * Transforms this FieldSet instance to readonly. diff --git a/forms/FormField.php b/forms/FormField.php index ce104a0b4..3bfe38846 100644 --- a/forms/FormField.php +++ b/forms/FormField.php @@ -41,6 +41,12 @@ class FormField extends RequestHandlingData { * @var int */ protected $tabIndex; + + /** + * Stores a reference to the FieldSet that contains this object. + * @var FieldSet + */ + protected $containerFieldSet; /** * @var $readonly boolean @@ -515,6 +521,21 @@ HTML; return $label; } + /** + * Set the fieldset that contains this field. + * + * @param FieldSet $containerFieldSet + */ + function setContainerFieldSet($containerFieldSet) { + $this->containerFieldSet = $containerFieldSet; + } + + function rootFieldSet() { + return $this->containerFieldSet->rootFieldSet(); + if(is_object($this->containerFieldSet)) return $this->containerFieldSet->rootFieldSet(); + else user_error("rootFieldSet() called on $this->class object without a containerFieldSet", E_USER_ERROR); + } + // ################### // DEPRECATED // ################### diff --git a/tests/forms/FieldSetTest.php b/tests/forms/FieldSetTest.php index 899b21e9f..a72ce5beb 100644 --- a/tests/forms/FieldSetTest.php +++ b/tests/forms/FieldSetTest.php @@ -143,6 +143,26 @@ class FieldSetTest extends SapphireTest { /* We have 1 field inside our tab */ $this->assertEquals(1, $tab->Fields()->Count()); } + + function testReplaceAFieldInADifferentTab() { + /* A FieldSet gets created with a TabSet and some field objects */ + $fieldSet = new FieldSet( + new TabSet('Root', $main = new Tab('Main', + new TextField('A'), + new TextField('B') + ), $other = new Tab('Other', + new TextField('C'), + new TextField('D') + )) + ); + + /* The field "A" gets added to the FieldSet we just created created */ + $fieldSet->addFieldToTab('Root.Other', $newA = new TextField('A', 'New Title')); + + /* The field named "A" has been removed from the Main tab to make way for our new field named "A" in Other tab. */ + $this->assertEquals(1, $main->Fields()->Count()); + $this->assertEquals(3, $other->Fields()->Count()); + } /** * Test finding a field that's inside a tabset, within another tab. @@ -252,9 +272,71 @@ class FieldSetTest extends SapphireTest { /* The position of the Title field should be at number 2 */ $this->assertEquals(2, $fields->fieldByName('Title')->Pos()); } + + function testRootFieldSet() { + /* Given a nested set of FormField, CompositeField, and FieldSet objects */ + $fieldSet = new FieldSet( + $root = new TabSet("Root", + $main = new Tab("Main", + $a = new TextField("A"), + $b = new TextField("B") + ) + ) + ); + + /* rootFieldSet() should always evaluate to the same object: the topmost fieldset */ + $this->assertSame($fieldSet, $fieldSet->rootFieldSet()); + $this->assertSame($fieldSet, $root->rootFieldSet()); + $this->assertSame($fieldSet, $main->rootFieldSet()); + $this->assertSame($fieldSet, $a->rootFieldSet()); + $this->assertSame($fieldSet, $b->rootFieldSet()); + + /* If we push additional fields, they should also have the same rootFieldSet() */ + $root->push($other = new Tab("Other")); + $other->push($c = new TextField("C")); + $root->push($third = new Tab("Third", $d = new TextField("D"))); + + $this->assertSame($fieldSet, $other->rootFieldSet()); + $this->assertSame($fieldSet, $third->rootFieldSet()); + $this->assertSame($fieldSet, $c->rootFieldSet()); + $this->assertSame($fieldSet, $d->rootFieldSet()); + } + + function testAddingDuplicateReplacesOldField() { + /* Given a nested set of FormField, CompositeField, and FieldSet objects */ + $fieldSet = new FieldSet( + $root = new TabSet("Root", + $main = new Tab("Main", + $a = new TextField("A"), + $b = new TextField("B") + ) + ) + ); + + /* Adding new fields of the same names should replace the original fields */ + $newA = new TextField("A", "New A"); + $newB = new TextField("B", "New B"); + + $fieldSet->addFieldToTab("Root.Main", $newA); + $fieldSet->addFieldToTab("Root.Other", $newB); + + $this->assertSame($newA, $fieldSet->dataFieldByName("A")); + $this->assertSame($newB, $fieldSet->dataFieldByName("B")); + $this->assertEquals(1, $main->Fields()->Count()); + + /* Pushing fields on the end of the field set should remove them from the tab */ + $thirdA = new TextField("A", "Third A"); + $thirdB = new TextField("B", "Third B"); + $fieldSet->push($thirdA); + $fieldSet->push($thirdB); + + $this->assertSame($thirdA, $fieldSet->fieldByName("A")); + $this->assertSame($thirdB, $fieldSet->fieldByName("B")); + + $this->assertEquals(0, $main->Fields()->Count()); + } /** - * @TODO test pushing a field replacing an existing one. (duplicate) * @TODO the same as above with insertBefore() and insertAfter() */