ENHANCEMENT Added CompositeField->fieldPosition()

ENHANCEMENT Supporting recursion into nested sets in FieldSet->insertAfter()/TabSet->insertAfter()/CompositeField->insertAfter()
API CHANGE Deprecated CompositeField->insertBeforeRecursive(), use CompositeField->insertBefore()
ENHANCEMENT Renamed CompositeField->beforeInsert() to CompositeField->onBeforeInsert() to avoid confusion with insertBefore()
ENHANCEMENT Added CompositeFieldTest
ENHANCEMENT Added unit tests for FieldSet->insertBefore()/insertAfter()

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@65581 467b73ca-7a2a-4603-9d3b-597d59a354a9

forms/CompositeField.php

@@ -156,15 +156,29 @@ class CompositeField extends FormField {
 	public function push(FormField $field) {
 		$this->children->push($field);
 	}
+	
+	/**
+	 * @uses FieldSet->insertBefore()
+	 */
 	public function insertBefore($field, $insertBefore) {
+		$ret = $this->children->insertBefore($field, $insertBefore);
+		$this->sequentialSet = null;
+		return $ret;
+	}
+
+	/**
+	 * @deprecated 2.3 Use insertBefore
+	 */
+	public function insertBeforeRecursive($field, $insertBefore) {
 		return $this->children->insertBefore($field, $insertBefore);
 	}
 
-	public function insertBeforeRecursive($field, $insertBefore, $level = 0) {
+	public function insertAfter($field, $insertAfter) {
-		return $this->children->insertBeforeRecursive($field, $insertBefore, $level+1);
+		$ret = $this->children->insertAfter($field, $insertAfter);
+		$this->sequentialSet = null;
+		return $ret;
 	}
 
-
 	/**
 	 * Remove a field from this CompositeField by Name.
 	 * The field could also be inside a CompositeField.
@@ -226,6 +240,26 @@ class CompositeField extends FormField {
 		return $this->readonly;
 	}
 	
+	/**
+	 * Find the numerical position of a field within
+	 * the children collection. Doesn't work recursively.
+	 * 
+	 * @param string|FormField
+	 * @return Position in children collection (first position starts with 0). Returns FALSE if the field can't be found.
+	 */
+	function fieldPosition($field) {
+		if(is_string($field)) $field = $this->fieldByName($field);
+		if(!$field) return false;
+		
+		$i = 0;
+		foreach($this->children as $child) {
+			if($child->Name() == $field->Name()) return $i;
+			$i++;
+		}
+		
+		return false;
+	}
+
 	function debug() {
 		$result = "$this->class ($this->name) <ul>";
 		foreach($this->children as $child) {

forms/FieldSet.php

@@ -169,7 +169,6 @@ class FieldSet extends DataObjectSet {
 		
 		foreach($this->items as $i => $child) {
 			if(is_object($child) && ($child->Name() == $fieldName || $child->Title() == $fieldName) && (!$dataFieldOnly || $child->hasData())) {
-				//if($child->class == 'Tab' && !$dataFieldOnly) Debug::backtrace();
 				array_splice( $this->items, $i, 1 );
 				break;
 			} else if($child->isComposite()) {
@@ -304,51 +303,29 @@ class FieldSet extends DataObjectSet {
 	 * @param string $name Name of the field to insert before
 	 */
 	public function insertBefore($item, $name) {
-		$this->beforeInsert($item);
+		$this->onBeforeInsert($item);
 		$item->setContainerFieldSet($this);
 		
 		$i = 0;
 		foreach($this->items as $child) {
 			if($name == $child->Name() || $name == $child->id) {
 				array_splice($this->items, $i, 0, array($item));
-			
+				return $item;
-				return;
+			} elseif($child->isComposite()) {
+				$ret = $child->insertBefore($item, $name);
+				if($ret) return $ret;
 			}
 			$i++;
 		}
-		$this->items[] = $item;
+		
+		return false;
 	}
 
 	/**
-	 * Inserts an item before the item with name $name
+	 * @deprecated 2.3 Use insertBefore()
-	 * It can be buried in a composite field
-	 * If no item with name $name is found, $item is inserted at the end of the FieldSet
-	 *
-	 * @param FormField $item The item to be inserted
-	 * @param string $name The name of the item that $item should be before
-	 * @param int $level For internal use only, should not be passed
 	 */
 	public function insertBeforeRecursive($item, $name, $level = 0) {
-		$this->beforeInsert($item);
+		return $this->insertBefore($item, $name);
-		$item->setContainerFieldSet($this);
-		
-		$i = 0;
-		foreach($this->items as $child) {
-			if($name == $child->Name() || $name == $child->id) {
-				array_splice($this->items, $i, 0, array($item));
-			
-				return $level;
-			} else if($child->isComposite()) {
-				if($level = $child->insertBeforeRecursive($item,$name,$level+1)) return $level;
-			}
-			
-			$i++;
-		}
-		if ($level === 0) {
-		$this->items[] = $item;
-			return 0;
-		}
-		return false;
 	}
 	
 	/**
@@ -358,18 +335,22 @@ class FieldSet extends DataObjectSet {
 	 * @param string $name Name of the field to insert after
 	 */
 	public function insertAfter($item, $name) {
-		$this->beforeInsert($item);
+		$this->onBeforeInsert($item);
 		$item->setContainerFieldSet($this);
 		
 		$i = 0;
 		foreach($this->items as $child) {
 			if($name == $child->Name() || $name == $child->id) {
 				array_splice($this->items, $i+1, 0, array($item));
-				return;
+				return $item;
+			} elseif($child->isComposite()) {
+				$ret = $child->insertAfter($item, $name);
+				if($ret) return $ret;
 			}
 			$i++;
 		}
-		$this->items[] = $item;
+		
+		return false;
 	}
 	
 	/**
@@ -379,7 +360,7 @@ class FieldSet extends DataObjectSet {
 	 * @param string $key An option array key (field name)
 	 */
 	public function push($item, $key = null) {
-		$this->beforeInsert($item);
+		$this->onBeforeInsert($item);
 		$item->setContainerFieldSet($this);
 		return parent::push($item, $key = null);
 	}
@@ -387,7 +368,7 @@ class FieldSet extends DataObjectSet {
 	/**
 	 * Handler method called before the FieldSet is going to be manipulated.
 	 */
-	function beforeInsert($item) {
+	protected function onBeforeInsert($item) {
 		if($this->sequentialSet) $this->sequentialSet = null;
 		if($item->Name()) $this->rootFieldSet()->removeByName($item->Name(), true);
 	}
@@ -495,7 +476,7 @@ class FieldSet extends DataObjectSet {
 	 * 
 	 * Please note that any tabs or other dataless fields will be clobbered by this operation.
 	 *
-	 * Field names can be given as an array, or just as a list of arguments.
+	 * @param array $fieldNames Field names can be given as an array, or just as a list of arguments.
 	 */
 	function changeFieldOrder($fieldNames) {
 		// Field names can be given as an array, or just as a list of arguments.
@@ -526,6 +507,25 @@ class FieldSet extends DataObjectSet {
 		$this->sequentialSet = null;
 	}
 	
+	/**
+	 * Find the numerical position of a field within
+	 * the children collection. Doesn't work recursively.
+	 * 
+	 * @param string|FormField
+	 * @return Position in children collection (first position starts with 0). Returns FALSE if the field can't be found.
+	 */
+	function fieldPosition($field) {
+		if(is_string($field)) $field = $this->fieldByName($field);
+		
+		$i = 0;
+		foreach($this->dataFields() as $child) {
+			if($child == $field) return $i;
+			$i++;
+		}
+		
+		return false;
+	}
+	
 }
 
 ?>

forms/TabSet.php

@@ -92,15 +92,23 @@ class TabSet extends CompositeField {
 		parent::push($field);
 		$field->setTabSet($this);
 	}
+	
+	/**
+	 * Inserts a field before a particular field in a FieldSet.
+	 *
+	 * @param FormField $item The form field to insert
+	 * @param string $name Name of the field to insert before
+	 */
 	public function insertBefore($field, $insertBefore) {
 		parent::insertBefore($field, $insertBefore);
-		$field->setTabSet($this);
+		if($field instanceof Tab) $field->setTabSet($this);
+		$this->sequentialSet = null;
 	}
 	
-	public function insertBeforeRecursive($field, $insertBefore, $level) {
+	public function insertAfter($field, $insertAfter) {
-		$level = parent::insertBeforeRecursive($field, $insertBefore, $level+1);
+		parent::insertAfter($field, $insertAfter);
-		if ($level === 0) $field->setTabSet($this);
+		if($field instanceof Tab) $field->setTabSet($this);
-		return $level;
+		$this->sequentialSet = null;
 	}
 	
 	public function removeByName( $tabName, $dataFieldOnly = false ) {

tests/forms/CompositeFieldTest.php

@@ -0,0 +1,33 @@
+<?php
+/**
+ * @package sapphire
+ * @subpackage tests
+ */
+class CompositeFieldTest extends SapphireTest {
+	
+	function testFieldPosition() {
+		$compositeOuter = new CompositeField(
+			new TextField('A'),
+			new TextField('B'),
+			$compositeInner = new CompositeField(
+				new TextField('C1'),
+				new TextField('C2')
+			),
+			new TextField('D')
+		);
+		
+		$this->assertEquals(0, $compositeOuter->fieldPosition('A'));
+		$this->assertEquals(1, $compositeOuter->fieldPosition('B'));
+		$this->assertEquals(3, $compositeOuter->fieldPosition('D'));
+		
+		$this->assertEquals(0, $compositeInner->fieldPosition('C1'));
+		$this->assertEquals(1, $compositeInner->fieldPosition('C2'));
+		
+		$compositeOuter->insertBefore(new TextField('AB'), 'B');
+		$this->assertEquals(0, $compositeOuter->fieldPosition('A'));
+		$this->assertEquals(1, $compositeOuter->fieldPosition('AB'));
+		$this->assertEquals(2, $compositeOuter->fieldPosition('B'));
+	}
+	
+}
+?>

tests/forms/FieldSetTest.php

@@ -6,21 +6,16 @@
  * @package sapphire
  * @subpackage tests
  * 
- * @TODO test for {@link FieldSet->insertBeforeRecursive()}.
+ * @todo test for {@link FieldSet->setValues()}. Need to check
- * 
- * @TODO test for {@link FieldSet->setValues()}. Need to check
  * 	that the values that were set are the correct ones given back.
- *
+ * @todo test for {@link FieldSet->transform()} and {@link FieldSet->makeReadonly()}.
- * @TODO test for {@link FieldSet->transform()} and {@link FieldSet->makeReadonly()}.
  *  Need to ensure that it correctly transforms the FieldSet object.
- *
+ * @todo test for {@link FieldSet->HiddenFields()}. Need to check
- * @TODO test for {@link FieldSet->HiddenFields()}. Need to check
  * 	the fields returned are the correct HiddenField objects for a
  * 	given FieldSet instance.
- * 
+ * @todo test for {@link FieldSet->dataFields()}.
- * @TODO test for {@link FieldSet->dataFields()}.
+ * @todo test for {@link FieldSet->findOrMakeTab()}.
- * 
+ * @todo the same as above with insertBefore() and insertAfter()
- * @TODO test for {@link FieldSet->findOrMakeTab()}.
  *
  */
 class FieldSetTest extends SapphireTest {
@@ -441,10 +436,239 @@ class FieldSetTest extends SapphireTest {
 		$this->assertSame($myName, $fieldSet->fieldByName('Root')->fieldByName('MyName')->Fields()->First());
 	}
 	
-	/**
+	function testInsertBeforeWithNestedCompositeFields() {
-	 * @TODO the same as above with insertBefore() and insertAfter()
+		$fieldSet = new FieldSet(
-	 */
+			new TextField('A_pre'),
+			new TextField('A'),
+			new TextField('A_post'),
+			$compositeA = new CompositeField(
+				new TextField('B_pre'),
+				new TextField('B'),
+				new TextField('B_post'),
+				$compositeB = new CompositeField(
+					new TextField('C_pre'),
+					new TextField('C'),
+					new TextField('C_post')
+				)
+			)
+		);
 		
+		$fieldSet->insertBefore(
+			$A_insertbefore = new TextField('A_insertbefore'),
+			'A'
+		);
+		$this->assertSame(
+			$A_insertbefore,
+			$fieldSet->dataFieldByName('A_insertbefore'),
+			'Field on toplevel fieldset can be inserted'
+		);
+		
+		$fieldSet->insertBefore(
+			$B_insertbefore = new TextField('B_insertbefore'),
+			'B'
+		);
+		$this->assertSame(
+			$fieldSet->dataFieldByName('B_insertbefore'),
+			$B_insertbefore,
+			'Field on one nesting level fieldset can be inserted'
+		);
+		
+		$fieldSet->insertBefore(
+			$C_insertbefore = new TextField('C_insertbefore'),
+			'C'
+		);
+		$this->assertSame(
+			$fieldSet->dataFieldByName('C_insertbefore'),
+			$C_insertbefore,
+			'Field on two nesting levels fieldset can be inserted'
+		);
 	}
 	
+	/**
+	 * @todo check actual placement of fields
+	 */
+	function testInsertBeforeWithNestedTabsets() {
+		$fieldSetA = new FieldSet(
+			$tabSetA = new TabSet('TabSet_A',
+				$tabA1 = new Tab('Tab_A1',
+					new TextField('A_pre'),
+					new TextField('A'),
+					new TextField('A_post')
+				),
+				$tabB1 = new Tab('Tab_B1',
+					new TextField('B')
+				)
+			)
+		);
+		$tabSetA->insertBefore(
+			$A_insertbefore = new TextField('A_insertbefore'),
+			'A'
+		);
+		$this->assertEquals(
+			$fieldSetA->dataFieldByName('A_insertbefore'),
+			$A_insertbefore,
+			'Field on toplevel tab can be inserted'
+		);
+		
+		$this->assertEquals(0, $tabA1->fieldPosition('A_pre'));
+		$this->assertEquals(1, $tabA1->fieldPosition('A_insertbefore'));
+		$this->assertEquals(2, $tabA1->fieldPosition('A'));
+		$this->assertEquals(3, $tabA1->fieldPosition('A_post'));
+
+		$fieldSetB = new FieldSet(
+			new TabSet('TabSet_A',
+				$tabsetB = new TabSet('TabSet_B',
+					$tabB1 = new Tab('Tab_B1',
+						new TextField('C')
+					),
+					$tabB2 = new Tab('Tab_B2',
+						new TextField('B_pre'),
+						new TextField('B'),
+						new TextField('B_post')
+					)
+				)
+			)
+		);
+		$fieldSetB->insertBefore(
+			$B_insertbefore = new TextField('B_insertbefore'),
+			'B'
+		);
+		$this->assertSame(
+			$fieldSetB->dataFieldByName('B_insertbefore'),
+			$B_insertbefore,
+			'Field on nested tab can be inserted'
+		);
+		$this->assertEquals(0, $tabB2->fieldPosition('B_pre'));
+		$this->assertEquals(1, $tabB2->fieldPosition('B_insertbefore'));
+		$this->assertEquals(2, $tabB2->fieldPosition('B'));
+		$this->assertEquals(3, $tabB2->fieldPosition('B_post'));
+	}
+	
+	function testInsertAfterWithNestedCompositeFields() {
+		$fieldSet = new FieldSet(
+			new TextField('A_pre'),
+			new TextField('A'),
+			new TextField('A_post'),
+			$compositeA = new CompositeField(
+				new TextField('B_pre'),
+				new TextField('B'),
+				new TextField('B_post'),
+				$compositeB = new CompositeField(
+					new TextField('C_pre'),
+					new TextField('C'),
+					new TextField('C_post')
+				)
+			)
+		);
+		
+		$fieldSet->insertAfter(
+			$A_insertafter = new TextField('A_insertafter'),
+			'A'
+		);
+		$this->assertSame(
+			$A_insertafter,
+			$fieldSet->dataFieldByName('A_insertafter'),
+			'Field on toplevel fieldset can be inserted after'
+		);
+		
+		$fieldSet->insertAfter(
+			$B_insertafter = new TextField('B_insertafter'),
+			'B'
+		);
+		$this->assertSame(
+			$fieldSet->dataFieldByName('B_insertafter'),
+			$B_insertafter,
+			'Field on one nesting level fieldset can be inserted after'
+		);
+		
+		$fieldSet->insertAfter(
+			$C_insertafter = new TextField('C_insertafter'),
+			'C'
+		);
+		$this->assertSame(
+			$fieldSet->dataFieldByName('C_insertafter'),
+			$C_insertafter,
+			'Field on two nesting levels fieldset can be inserted after'
+		);
+	}
+	
+	/**
+	 * @todo check actual placement of fields
+	 */
+	function testInsertAfterWithNestedTabsets() {
+		$fieldSetA = new FieldSet(
+			$tabSetA = new TabSet('TabSet_A',
+				$tabA1 = new Tab('Tab_A1',
+					new TextField('A_pre'),
+					new TextField('A'),
+					new TextField('A_post')
+				),
+				$tabB1 = new Tab('Tab_B1',
+					new TextField('B')
+				)
+			)
+		);
+		$tabSetA->insertAfter(
+			$A_insertafter = new TextField('A_insertafter'),
+			'A'
+		);
+		$this->assertEquals(
+			$fieldSetA->dataFieldByName('A_insertafter'),
+			$A_insertafter,
+			'Field on toplevel tab can be inserted after'
+		);
+		$this->assertEquals(0, $tabA1->fieldPosition('A_pre'));
+		$this->assertEquals(1, $tabA1->fieldPosition('A'));
+		$this->assertEquals(2, $tabA1->fieldPosition('A_insertafter'));
+		$this->assertEquals(3, $tabA1->fieldPosition('A_post'));
+
+		$fieldSetB = new FieldSet(
+			new TabSet('TabSet_A',
+				$tabsetB = new TabSet('TabSet_B',
+					$tabB1 = new Tab('Tab_B1',
+						new TextField('C')
+					),
+					$tabB2 = new Tab('Tab_B2',
+						new TextField('B_pre'),
+						new TextField('B'),
+						new TextField('B_post')
+					)
+				)
+			)
+		);
+		$fieldSetB->insertAfter(
+			$B_insertafter = new TextField('B_insertafter'),
+			'B'
+		);
+		$this->assertSame(
+			$fieldSetB->dataFieldByName('B_insertafter'),
+			$B_insertafter,
+			'Field on nested tab can be inserted after'
+		);
+		$this->assertEquals(0, $tabB2->fieldPosition('B_pre'));
+		$this->assertEquals(1, $tabB2->fieldPosition('B'));
+		$this->assertEquals(2, $tabB2->fieldPosition('B_insertafter'));
+		$this->assertEquals(3, $tabB2->fieldPosition('B_post'));
+	}
+	
+	function testFieldPosition() {
+		$set = new FieldSet(
+			new TextField('A'),
+			new TextField('B'),
+			new TextField('C')
+		);
+		
+		$this->assertEquals(0, $set->fieldPosition('A'));
+		$this->assertEquals(1, $set->fieldPosition('B'));
+		$this->assertEquals(2, $set->fieldPosition('C'));
+		
+		$set->insertBefore(new TextField('AB'), 'B');
+		$this->assertEquals(0, $set->fieldPosition('A'));
+		$this->assertEquals(1, $set->fieldPosition('AB'));
+		$this->assertEquals(2, $set->fieldPosition('B'));
+		$this->assertEquals(3, $set->fieldPosition('C'));
+		
+		unset($set);
+	}
+}
 ?>