Skip to content

Commit

Permalink
[Form] Improved semantics of property paths and removed FieldGroup::m…
Browse files Browse the repository at this point in the history
…erge() for now

The semantics of property paths are now:

   (1) if a property path is set, it is _always_ respected (relative to the object
       of the parent field)
   (2) if no property path is set, the object of the parent field is _always_ ignored

Fact (2) allows us to set data into fields that is updated independently of the parent
field (like CSRF tokens, subforms with different objects etc.)

What is missing now is support for subfields that pass the object of the parent field
through to their own subfields. This functionality would be needed for GoogleMapFields,
DateRangeFields etc., which are compositions of individual fields that update the
parent object of the FieldGroup.

There are several alternatives for the latter functionality that should be discussed
in a RFC.
  • Loading branch information
Bernhard Schussek authored and fabpot committed Dec 16, 2010
1 parent 242be93 commit b8ef7e7
Show file tree
Hide file tree
Showing 4 changed files with 2 additions and 106 deletions.
4 changes: 1 addition & 3 deletions src/Symfony/Component/Form/Field.php
Expand Up @@ -500,11 +500,9 @@ protected function reverseTransform($value)
public function updateFromProperty(&$objectOrArray)
{
// TODO throw exception if not object or array

if ($this->propertyPath !== null) {
$this->setData($this->propertyPath->getValue($objectOrArray));
} else {
// pass object through if the property path is empty
$this->setData($objectOrArray);
}
}

Expand Down
47 changes: 1 addition & 46 deletions src/Symfony/Component/Form/FieldGroup.php
Expand Up @@ -98,58 +98,13 @@ public function add(FieldInterface $field)

// if the property "data" is NULL, getTransformedData() returns an empty
// string
if (!empty($data) && $field->getPropertyPath() !== null) {
if (!empty($data)) {
$field->updateFromProperty($data);
}

return $field;
}

/**
* Merges a field group into this group. The group must have a unique name
* within the group. Otherwise the existing field is overwritten.
*
* Contrary to added groups, merged groups operate on the same object as
* the group they are merged into.
*
* <code>
* class Entity
* {
* public $longitude;
* public $latitude;
* }
*
* $entity = new Entity();
*
* $form = new Form('entity', $entity, $validator);
*
* $locationGroup = new FieldGroup('location');
* $locationGroup->add(new TextField('longitude'));
* $locationGroup->add(new TextField('latitude'));
*
* $form->merge($locationGroup);
* </code>
*
* @param FieldGroup $group
*/
public function merge(FieldGroup $group)
{
if ($group->isBound()) {
throw new AlreadyBoundException('A bound form group cannot be merged');
}

foreach ($group as $field) {
$group->remove($field->getKey());
$this->add($field);

if (($path = $group->getPropertyPath()) !== null) {
$field->setPropertyPath($path.'.'.$field->getPropertyPath());
}
}

return $this;
}

/**
* Removes the field with the given key.
*
Expand Down
41 changes: 0 additions & 41 deletions tests/Symfony/Tests/Component/Form/FieldGroupTest.php
Expand Up @@ -395,32 +395,6 @@ public function testRemoveUnsetsFieldParent()
$group->remove('firstName');
}

public function testMergeAddsFieldsFromAnotherGroup()
{
$group1 = new TestFieldGroup('author');
$group1->add($field1 = new TestField('firstName'));

$group2 = new TestFieldGroup('publisher');
$group2->add($field2 = new TestField('lastName'));

$group1->merge($group2);

$this->assertTrue($group1->has('lastName'));
$this->assertEquals(new PropertyPath('publisher.lastName'), $group1->get('lastName')->getPropertyPath());
}

public function testMergeThrowsExceptionIfOtherGroupAlreadyBound()
{
$group1 = new TestFieldGroup('author');
$group2 = new TestFieldGroup('publisher');
$group2->add($this->createMockField('firstName'));

$group2->bind(array('firstName' => 'Bernhard'));

$this->setExpectedException('Symfony\Component\Form\Exception\AlreadyBoundException');
$group1->merge($group2);
}

public function testAddUpdatesFieldFromTransformedData()
{
$originalAuthor = new Author();
Expand Down Expand Up @@ -450,21 +424,6 @@ public function testAddUpdatesFieldFromTransformedData()
$group->add($field);
}

public function testAddDoesNotUpdateFieldsWithEmptyPropertyPath()
{
$group = new TestFieldGroup('author');
$group->setData(new Author());

$field = $this->createMockField('firstName');
$field->expects($this->any())
->method('getPropertyPath')
->will($this->returnValue(null));
$field->expects($this->never())
->method('updateFromProperty');

$group->add($field);
}

public function testAddDoesNotUpdateFieldIfTransformedDataIsEmpty()
{
$originalAuthor = new Author();
Expand Down
16 changes: 0 additions & 16 deletions tests/Symfony/Tests/Component/Form/FieldTest.php
Expand Up @@ -433,22 +433,6 @@ public function testBoundValuesAreNotTrimmedBeforeTransformingIfDisabled()
$this->assertEquals('reverse[ a ]', $field->getData());
}

/*
* The use case of this test is a field group with an empty property path.
* Even if the field group itself is not associated to a specific property,
* nested fields might be.
*/
public function testUpdateFromPropertyPassesObjectThroughIfPropertyPathIsEmpty()
{
$object = new Author();
$object->firstName = 'Bernhard';

$field = new TestField('firstName', array('property_path' => null));
$field->updateFromProperty($object);

$this->assertEquals($object, $field->getData());
}

/*
* This is important so that bind() can work even if setData() was not called
* before
Expand Down

0 comments on commit b8ef7e7

Please sign in to comment.