diff --git a/forms/FieldSet.php b/forms/FieldSet.php index 2f6fac0c2..f99f0457e 100755 --- a/forms/FieldSet.php +++ b/forms/FieldSet.php @@ -163,6 +163,10 @@ class FieldSet extends DataObjectSet { * be left as-is. */ public function removeByName($fieldName, $dataFieldOnly = false) { + if(!$fieldName) { + user_error('FieldSet::removeByName() was called with a blank field name.', E_USER_WARNING); + } + 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(); @@ -385,7 +389,7 @@ class FieldSet extends DataObjectSet { */ function beforeInsert($item) { if($this->sequentialSet) $this->sequentialSet = null; - $this->rootFieldSet()->removeByName($item->Name(), true); + if($item->Name()) $this->rootFieldSet()->removeByName($item->Name(), true); } @@ -524,4 +528,4 @@ class FieldSet extends DataObjectSet { } -?> \ No newline at end of file +?> diff --git a/tests/forms/FieldSetTest.php b/tests/forms/FieldSetTest.php index bbb4374ef..aa104f306 100644 --- a/tests/forms/FieldSetTest.php +++ b/tests/forms/FieldSetTest.php @@ -281,6 +281,14 @@ class FieldSetTest extends SapphireTest { /* There are now 2 fields in the set */ $this->assertEquals(2, $fields->Count()); + + // Test that pushing a composite field without a name onto the set works + // See ticket #2932 + $fields->push(new CompositeField( + new TextField('Test1'), + new TextField('Test2') + )); + $this->assertEquals(3, $fields->Count()); } /** @@ -439,4 +447,4 @@ class FieldSetTest extends SapphireTest { } -?> \ No newline at end of file +?>