From 946025adb97a6ac2cde63272ab0f4fee16f17eb2 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Fri, 17 Jul 2009 02:09:27 +0000 Subject: [PATCH] API CHANGE Removed TableField->FieldSet() and TableField->SubmittedFieldSet(), please use Items() and TableField_Item->Fields() instead (merged branches/2.3-nzct) BUGFIX Fixed re-loading of unsaved TableField form data (e.g. after a validation error). The (now removed) method SubmittedFieldSet() was setting incremental temporary identifiers ("new1", "new2", etc), which wasn't picked up by Items() (merged from branches/2.3-nzct) BUGFIX Using $this->value instead of $_POST to process submitted data in TableField (merged from branches/2.3-nzct) BUGFIX TableField validation logic iterates over TableField_Item instances to get all formfields, rather than creating their own set in SubmittedFieldSet() MINOR Removed repitition of temporary Form generation in TableField by generateTableFieldItem() method (merged from branches/2.3-nzct) git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@82089 467b73ca-7a2a-4603-9d3b-597d59a354a9 --- forms/TableField.php | 185 ++++++++++++++++----------------- tests/forms/TableFieldTest.php | 152 ++++++++++++++++++++++++--- tests/forms/TableFieldTest.yml | 14 ++- 3 files changed, 240 insertions(+), 111 deletions(-) diff --git a/forms/TableField.php b/forms/TableField.php index 8b3c53147..36e85a4bc 100644 --- a/forms/TableField.php +++ b/forms/TableField.php @@ -181,88 +181,75 @@ class TableField extends TableListField { * @return DataObjectSet Collection of {@link TableField_Item} */ function Items() { - $output = new DataObjectSet(); - if($items = $this->sourceItems()) { - foreach ($items as $item) { - // Load the data in to a temporary form (for correct field types) - $fieldset = $this->FieldSetForRow(); - if($fieldset){ - $form = new Form($this, null, $fieldset, new FieldSet()); - $form->loadDataFrom($item); - // Add the item to our new DataObjectSet, with a wrapper class. - $output->push(new TableField_Item($item, $this, $form, $this->fieldTypes)); - } - } - } - // Create a temporary DataObject - if($this->Can('add')) { - if($this->showAddRow){ - $output->push(new TableField_Item(null, $this, null, $this->fieldTypes, true)); - } - } - return $output; - } - - /** - * Get all fields for each row contained in the TableField. - * Does not include the empty row. - * - * @return array - */ - function FieldSet() { - $fields = array (); - if($items = $this->sourceItems()) { - foreach($items as $item) { - // Load the data in to a temporary form (for correct field types) - $fieldset = $this->FieldSetForRow(); - if ($fieldset) - { - // TODO Needs to be attached to a form existing in the DOM-tree - $form = new Form($this, 'EditForm', $fieldset, new FieldSet()); - $form->loadDataFrom($item); - $row = new TableField_Item($item, $this, $form, $this->fieldTypes); - $fields = array_merge($fields, $row->Fields()->toArray()); - } - } - } + // holds TableField_Item instances + $items = new DataObjectSet(); - return $fields; - } - - function SubmittedFieldSet(&$sourceItems){ - $fields = array (); - if(isset($_POST[$this->name])&&$rows = $_POST[$this->name]){ - if(count($rows)){ - foreach($rows as $idx => $row){ - if($idx == 'new'){ - $newitems = ArrayLib::invert($row); - if(count($newitems)){ - $sourceItems = new DataObjectSet(); - foreach($newitems as $k => $newitem){ - $fieldset = $this->FieldSetForRow(); - if($fieldset){ - $newitem['ID'] = "new".$k; - foreach($newitem as $k => $v){ - if($this->extraData && array_key_exists($k, $this->extraData)){ - unset($newitem[$k]); - } - } - $sourceItem = new DataObject($newitem); - if(!$sourceItem->isEmpty()){ - $sourceItems->push($sourceItem); - $form = new Form($this, "EditForm", $fieldset, new FieldSet()); - $form->loadDataFrom($sourceItem); - $item = new TableField_Item($sourceItem, $this, $form, $this->fieldTypes); - $fields = array_merge($fields, $item->Fields()->toArray()); - } - } - } + $sourceItems = $this->sourceItems(); + + // either load all rows from the field value, + // (e.g. when validation failed), or from sourceItems() + if($this->value) { + if(!$sourceItems) $sourceItems = new DataObjectSet(); + + // get an array keyed by rows, rather than values + $rows = $this->sortData(ArrayLib::invert($this->value)); + // ignore all rows which are already saved + if(isset($rows['new'])) { + $newRows = $this->sortData($rows['new']); + // iterate over each value (not each row) + $i = 0; + foreach($newRows as $idx => $newRow){ + // set a pseudo-ID + $newRow['ID'] = "new"; + + // unset any extradata + foreach($newRow as $k => $v){ + if($this->extraData && array_key_exists($k, $this->extraData)){ + unset($newRow[$k]); } } + + // generate a temporary DataObject container (not saved in the database) + $sourceClass = $this->sourceClass; + $sourceItems->push(new $sourceClass($newRow)); + + $i++; } } + } + + // generate a new TableField_Item instance from each collected item + if($sourceItems) foreach($sourceItems as $sourceItem) { + $items->push($this->generateTableFieldItem($sourceItem)); } - return $fields; + + // add an empty TableField_Item for a single "add row" + if($this->showAddRow && $this->Can('add')) { + $items->push(new TableField_Item(null, $this, null, $this->fieldTypes, true)); + } + + return $items; + } + + /** + * Generates a new {@link TableField} instance + * by loading a fieldset for this row into a temporary form. + * + * @param DataObject $dataObj + * @return TableField_Item + */ + protected function generateTableFieldItem($dataObj) { + // Load the data in to a temporary form (for correct field types) + $form = new Form( + $this, + null, + $this->FieldSetForRow(), + new FieldSet() + ); + $form->loadDataFrom($dataObj); + + // Add the item to our new DataObjectSet, with a wrapper class. + return new TableField_Item($dataObj, $this, $form, $this->fieldTypes); } /** @@ -537,21 +524,18 @@ class TableField extends TableListField { function jsValidation() { $js = ""; - $fields = $this->FieldSet(); - $fields = new FieldSet($fields); - // TODO doesn't automatically update validation when adding a row - foreach($fields as $field) { - //if the field type has some special specific specification for validation of itself - $js .= $field->jsValidation($this->form->class."_".$this->form->Name()); + $items = $this->Items(); + if($items) foreach($items as $item) { + foreach($item->Fields() as $field) { + //if the field type has some special specific specification for validation of itself + $js .= $field->jsValidation($this->form->class."_".$this->form->Name()); + } } // TODO Implement custom requiredFields $items = $this->sourceItems(); if($items && $this->requiredFields && $items->count()) { foreach ($this->requiredFields as $field) { - /*if($fields->dataFieldByName($field)) { - $js .= "\t\t\t\t\trequire('$field');\n"; - }*/ foreach($items as $item){ $cellName = $this->Name().'['.$item->ID.']['.$field.']'; $js .= "\n"; @@ -575,14 +559,16 @@ JS; function php($data) { $valid = true; - if($data['methodName'] != 'delete'){ - $fields = $this->FieldSet(); - $fields = new FieldSet($fields); - foreach($fields as $field){ - $valid = $field->validate($this) && $valid; + if($data['methodName'] != 'delete') { + $items = $this->Items(); + if($items) foreach($items as $item) { + foreach($item->Fields() as $field) { + $valid = $field->validate($this) && $valid; + } } + return $valid; - }else{ + } else { return $valid; } } @@ -590,10 +576,13 @@ JS; function validate($validator) { $errorMessage = ''; $valid = true; - $fields = $this->SubmittedFieldSet($sourceItemsNew); - $fields = new FieldSet($fields); - foreach($fields as $field){ - $valid = $field->validate($validator)&&$valid; + + // @todo should only contain new elements + $items = $this->Items(); + if($items) foreach($items as $item) { + foreach($item->Fields() as $field) { + $valid = $field->validate($validator) && $valid; + } } //debug::show($this->form->Message()); @@ -702,8 +691,10 @@ class TableField_Item extends TableListField_Item { $origFieldName = $field->Name(); // set unique fieldname with id - $combinedFieldName = $this->parent->Name() . "[" . $this->ID . "][" . $origFieldName . "]"; - if($this->isAddRow) $combinedFieldName .= '[]'; + $combinedFieldName = $this->parent->Name() . "[" . $this->ID() . "][" . $origFieldName . "]"; + // ensure to set field to nested array notation + // if its an unsaved row, or the "add row" which is present by default + if($this->isAddRow || $this->ID() == 'new') $combinedFieldName .= '[]'; // get value if(strpos($origFieldName,'.') === false) { diff --git a/tests/forms/TableFieldTest.php b/tests/forms/TableFieldTest.php index 94be2344c..dd9d744df 100644 --- a/tests/forms/TableFieldTest.php +++ b/tests/forms/TableFieldTest.php @@ -2,28 +2,35 @@ class TableFieldTest extends SapphireTest { static $fixture_file = 'sapphire/tests/forms/TableFieldTest.yml'; - - - function testTableFieldSaving() { - $group = $this->objFromFixture('Group','a'); + + function testAdd() { + $group = $this->objFromFixture('Group','group1_no_perms'); $tableField = new TableField( "Permissions", "Permission", array( - "Code" => _t('SecurityAdmin.CODE', 'Code'), - "Arg" => _t('SecurityAdmin.OPTIONALID', 'Optional ID'), + "Code" => 'Code', + "Arg" => 'Arg', ), array( - "Code" => "PermissionDropdownField", + "Code" => "TextField", "Arg" => "TextField", ), "GroupID", $group->ID ); - $form = new Form(new TableFieldTest_Controller(), "Form", new FieldSet($tableField), new FieldSet()); + $form = new Form( + new TableFieldTest_Controller(), + "Form", + new FieldSet($tableField), + new FieldSet() + ); - /* The field starts emppty. Save some new data. We have replicated the array structure that the specific layout of the form generates. */ + // Test Insert + + // The field starts emppty. Save some new data. + // We have replicated the array structure that the specific layout of the form generates. $tableField->setValue(array( 'new' => array( 'Code' => array( @@ -38,7 +45,7 @@ class TableFieldTest extends SapphireTest { )); $tableField->saveInto($group); - /* Let's check that the 2 permissions entries have been saved */ + // Let's check that the 2 permissions entries have been saved $permissions = $group->Permissions()->toDropdownMap('Arg', 'Code'); $this->assertEquals(array( 1 => 'CMS_ACCESS_CMSMain', @@ -46,7 +53,7 @@ class TableFieldTest extends SapphireTest { ), $permissions); - /* Now let's perform an update query */ + // Test repeated insert $value = array(); foreach($group->Permissions() as $permission) { $value[$permission->ID] = array("Code" => $permission->Code, "Arg" => $permission->Arg); @@ -62,7 +69,7 @@ class TableFieldTest extends SapphireTest { $tableField->setValue($value); $tableField->saveInto($group); - /* Let's check that the 2 existing permissions entries, and the 1 new one, have been saved */ + // Let's check that the 2 existing permissions entries, and the 1 new one, have been saved $permissions = $group->Permissions()->toDropdownMap('Arg', 'Code'); $this->assertEquals(array( 1 => 'CMS_ACCESS_CMSMain', @@ -72,6 +79,88 @@ class TableFieldTest extends SapphireTest { } + function testEdit() { + $group = $this->objFromFixture('Group','group2_existing_perms'); + $perm1 = $this->objFromFixture('Permission', 'perm1'); + $perm2 = $this->objFromFixture('Permission', 'perm2'); + + $tableField = new TableField( + "Permissions", + "Permission", + array( + "Code" => 'Code', + "Arg" => 'Arg', + ), + array( + "Code" => "TextField", + "Arg" => "TextField", + ), + "GroupID", + $group->ID + ); + $form = new Form( + new TableFieldTest_Controller(), + "Form", + new FieldSet($tableField), + new FieldSet() + ); + + $this->assertEquals($tableField->sourceItems()->Count(), 2); + + // We have replicated the array structure that the specific layout of the form generates. + $tableField->setValue(array( + $perm1->ID => array( + 'Code' => 'Perm1 Modified', + 'Arg' => '101' + ), + $perm2->ID => array( + 'Code' => 'Perm2 Modified', + 'Arg' => '102' + ) + )); + $tableField->saveInto($group); + + // Let's check that the 2 permissions entries have been saved + $permissions = $group->Permissions()->toDropdownMap('Arg', 'Code'); + $this->assertEquals(array( + 101 => 'Perm1 Modified', + 102 => 'Perm2 Modified', + ), $permissions); + } + + function testDelete() { + $group = $this->objFromFixture('Group','group2_existing_perms'); + $perm1 = $this->objFromFixture('Permission', 'perm1'); + $perm2 = $this->objFromFixture('Permission', 'perm2'); + + $tableField = new TableField( + "Permissions", + "Permission", + array( + "Code" => 'Code', + "Arg" => 'Arg', + ), + array( + "Code" => "TextField", + "Arg" => "TextField", + ), + "GroupID", + $group->ID + ); + $form = new Form( + new TableFieldTest_Controller(), + "Form", + new FieldSet($tableField), + new FieldSet() + ); + + $this->assertContains($perm1->ID, $tableField->sourceItems()->column('ID')); + + $response = $tableField->Items()->find('ID', $perm1->ID)->delete(); + + $this->assertNotContains($perm1->ID, $tableField->sourceItems()->column('ID')); + } + function testAutoRelationSettingOn() { $tf = new TableField( 'HasManyRelations', @@ -150,6 +239,45 @@ class TableFieldTest extends SapphireTest { //$this->assertEquals($data['TestTableField']['new']['Currency'][0], 1234.56); //$this->assertEquals($data['TestTableField']['new']['Currency'][1], 1234.57); } + + function testHasItemsWhenSetAsArray() { + $tf = new TableField( + 'TestTableField', + 'TableFieldTest_HasManyRelation', + array( + 'Value' => 'Value' + ), + array( + 'Value' => 'TextField' + ) + ); + $tf->setValue(array( + 'new' => array( + 'Value' => array( + 'One', + 'Two', + ) + ) + )); + $items = $tf->Items(); + $itemsArr = $items->toArray(); + + // includes the two values and an "add" row + $this->assertEquals($items->Count(), 3); + + // first row + $this->assertEquals( + $itemsArr[0]->Fields()->fieldByName('TestTableField[new][Value][]')->Value(), + 'One' + ); + + // second row + $this->assertEquals( + $itemsArr[1]->Fields()->fieldByName('TestTableField[new][Value][]')->Value(), + 'Two' + ); + } + } /** diff --git a/tests/forms/TableFieldTest.yml b/tests/forms/TableFieldTest.yml index b6261874b..b8822d23c 100644 --- a/tests/forms/TableFieldTest.yml +++ b/tests/forms/TableFieldTest.yml @@ -1,3 +1,13 @@ +Permission: + perm1: + Code: Perm1 + Arg: 1 + perm2: + Code: Perm2 + Arg: 2 Group: - a: - Title: Group A \ No newline at end of file + group1_no_perms: + Title: Group A + group2_existing_perms: + Title: Group B + Permissions: =>Permission.perm1,=>Permission.perm2 \ No newline at end of file