Skip to content

Commit

Permalink
merged branch bschussek/issue1540 (PR #3239)
Browse files Browse the repository at this point in the history
Commits
-------

9b0245b [Form] Made prefix of adder and remover method configurable. Adders and removers are not called if "by_reference" is disabled.
49d1464 [Form] Implemented MergeCollectionListener which calls addXxx() and removeXxx() in your model if found
7837f50 [Form] Added FormUtil::singularify()

Discussion
----------

[Form] Forms now call addXxx() and removeXxx() in your model

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #1540
Todo: adapt documentation

![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue1540)

Adds functionality for calling `addXxx` and `removeXxx` method in your model. All types returning a collection of values are affected: "collection", "choice" (with multiple selection) and "entity" (with multiple selection).

Example:

    class Article
    {
        public function addTag($tag) { ... }
        public function removeTag($tag) { ... }
        public function getTags($tag) { ... }
    }

And the controller:

    $form = $this->createFormBuilder($article)
        ->add('tags')
        ->getForm();

Upon modifying the form, addTag() and removeTag() are now called appropiately.

---------------------------------------------------------------------------

by stof at 2012-02-01T18:23:49Z

Really great

---------------------------------------------------------------------------

by vicb at 2012-02-01T18:24:04Z

Great !!

Two suggestions:

* make "add" and "remove" configurable,
* introduce a base class for the remove listeners with (final?) `::getSubscribedEvents()` and `::getEventPriorities()`

---------------------------------------------------------------------------

by haswalt at 2012-02-01T18:57:46Z

+1 this

---------------------------------------------------------------------------

by daFish at 2012-02-01T19:54:46Z

+1

---------------------------------------------------------------------------

by michelsalib at 2012-02-01T20:55:37Z

Can wait to have it!
It will save lots of time trying to solve WTF effects and making workarounds.

---------------------------------------------------------------------------

by bschussek at 2012-02-02T09:37:12Z

@vicb: Your first point is done. The second, I don't understand.

---------------------------------------------------------------------------

by stof at 2012-02-02T09:40:50Z

@bschussek your branch conflicts with master according to github

---------------------------------------------------------------------------

by vicb at 2012-02-02T09:52:40Z

@bschussek my point is that I can stand hard-coded priorities which are error prone. A better solution might be to introduce constants (in `FormEvents` / `FormEventPriorities` ?) with meaningful names.

---------------------------------------------------------------------------

by bschussek at 2012-02-02T10:21:52Z

@stof Rebased

@vicb I know, but who is responsible for managing priorities? There is no central entitty that can do this. (btw this is a general problem of the priority system of the EventDispatcher)

@fabpot Ready to merge.

---------------------------------------------------------------------------

by vicb at 2012-02-02T10:23:28Z

@bschussek doesn't each form has is own dispatcher so there is no need for a global registry here, something local to the form could be good enough.
  • Loading branch information
fabpot committed Feb 2, 2012
2 parents 3c8d300 + 9b0245b commit baa63b8
Show file tree
Hide file tree
Showing 15 changed files with 1,093 additions and 40 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG-2.1.md
Expand Up @@ -195,6 +195,11 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c
* choice fields now throw a FormException if neither the "choices" nor the
"choice_list" option is set
* the radio type is now a child of the checkbox type
* the collection, choice (with multiple selection) and entity (with multiple
selection) types now make use of addXxx() and removeXxx() methods in your
model
* added options "adder_prefix" and "remover_prefix" to collection and choice
type

### HttpFoundation

Expand Down
Expand Up @@ -24,37 +24,24 @@
*
* @see Doctrine\Common\Collections\Collection
*/
class MergeCollectionListener implements EventSubscriberInterface
class MergeDoctrineCollectionListener implements EventSubscriberInterface
{
static public function getSubscribedEvents()
{
return array(FormEvents::BIND_NORM_DATA => 'onBindNormData');
// Higher priority than core MergeCollectionListener so that this one
// is called before
return array(FormEvents::BIND_NORM_DATA => array('onBindNormData', 10));
}

public function onBindNormData(FilterDataEvent $event)
{
$collection = $event->getForm()->getData();
$data = $event->getData();

if (!$collection) {
$collection = $data;
} elseif (count($data) === 0) {
// If all items were removed, call clear which has a higher
// performance on persistent collections
if ($collection && count($data) === 0) {
$collection->clear();
} else {
// merge $data into $collection
foreach ($collection as $entity) {
if (!$data->contains($entity)) {
$collection->removeElement($entity);
} else {
$data->removeElement($entity);
}
}

foreach ($data as $entity) {
$collection->add($entity);
}
}

$event->setData($collection);
}
}
4 changes: 2 additions & 2 deletions src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php
Expand Up @@ -16,7 +16,7 @@
use Symfony\Component\Form\FormBuilder;
use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityChoiceList;
use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityLoaderInterface;
use Symfony\Bridge\Doctrine\Form\EventListener\MergeCollectionListener;
use Symfony\Bridge\Doctrine\Form\EventListener\MergeDoctrineCollectionListener;
use Symfony\Bridge\Doctrine\Form\DataTransformer\CollectionToArrayTransformer;
use Symfony\Component\Form\AbstractType;

Expand All @@ -36,7 +36,7 @@ public function buildForm(FormBuilder $builder, array $options)
{
if ($options['multiple']) {
$builder
->addEventSubscriber(new MergeCollectionListener())
->addEventSubscriber(new MergeDoctrineCollectionListener())
->prependClientTransformer(new CollectionToArrayTransformer())
;
}
Expand Down
@@ -0,0 +1,179 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Form\Extension\Core\EventListener;

use Symfony\Component\Form\Util\FormUtil;

use Symfony\Component\Form\FormEvents;
use Symfony\Component\Form\Event\FilterDataEvent;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\Form\Exception\UnexpectedTypeException;

/**
* @author Bernhard Schussek <bschussek@gmail.com>
*/
class MergeCollectionListener implements EventSubscriberInterface
{
/**
* Whether elements may be added to the collection
* @var Boolean
*/
private $allowAdd;

/**
* Whether elements may be removed from the collection
* @var Boolean
*/
private $allowDelete;

/**
* Whether to search for and use adder and remover methods
* @var Boolean
*/
private $useAccessors;

/**
* The prefix of the adder method to look for
* @var string
*/
private $adderPrefix;

/**
* The prefix of the remover method to look for
* @var string
*/
private $removerPrefix;

public function __construct($allowAdd = false, $allowDelete = false, $useAccessors = true, $adderPrefix = 'add', $removerPrefix = 'remove')
{
$this->allowAdd = $allowAdd;
$this->allowDelete = $allowDelete;
$this->useAccessors = $useAccessors;
$this->adderPrefix = $adderPrefix;
$this->removerPrefix = $removerPrefix;
}

static public function getSubscribedEvents()
{
return array(FormEvents::BIND_NORM_DATA => 'onBindNormData');
}

public function onBindNormData(FilterDataEvent $event)
{
$originalData = $event->getForm()->getData();
$form = $event->getForm();
$data = $event->getData();
$parentData = $form->hasParent() ? $form->getParent()->getData() : null;
$adder = null;
$remover = null;

if (null === $data) {
$data = array();
}

if (!is_array($data) && !($data instanceof \Traversable && $data instanceof \ArrayAccess)) {
throw new UnexpectedTypeException($data, 'array or (\Traversable and \ArrayAccess)');
}

if (null !== $originalData && !is_array($originalData) && !($originalData instanceof \Traversable && $originalData instanceof \ArrayAccess)) {
throw new UnexpectedTypeException($originalData, 'array or (\Traversable and \ArrayAccess)');
}

// Check if the parent has matching methods to add/remove items
if ($this->useAccessors && is_object($parentData)) {
$plural = ucfirst($form->getName());
$singulars = (array) FormUtil::singularify($plural);
$reflClass = new \ReflectionClass($parentData);

foreach ($singulars as $singular) {
$adderName = $this->adderPrefix . $singular;
$removerName = $this->removerPrefix . $singular;

if ($reflClass->hasMethod($adderName) && $reflClass->hasMethod($removerName)) {
$adder = $reflClass->getMethod($adderName);
$remover = $reflClass->getMethod($removerName);

if ($adder->isPublic() && $adder->getNumberOfRequiredParameters() === 1
&& $remover->isPublic() && $remover->getNumberOfRequiredParameters() === 1) {

// We found a public, one-parameter add and remove method
break;
}

// False alert
$adder = null;
$remover = null;
}
}
}

// Check which items are in $data that are not in $originalData and
// vice versa
$itemsToDelete = array();
$itemsToAdd = is_object($data) ? clone $data : $data;

if ($originalData) {
foreach ($originalData as $originalKey => $originalItem) {
foreach ($data as $key => $item) {
if ($item === $originalItem) {
// Item found, next original item
unset($itemsToAdd[$key]);
continue 2;
}
}

// Item not found, remember for deletion
$itemsToDelete[$originalKey] = $originalItem;
}
}

if ($adder && $remover) {
// If methods to add and to remove exist, call them now, if allowed
if ($this->allowDelete) {
foreach ($itemsToDelete as $item) {
$remover->invoke($parentData, $item);
}
}

if ($this->allowAdd) {
foreach ($itemsToAdd as $item) {
$adder->invoke($parentData, $item);
}
}
} elseif (!$originalData) {
// No original data was set. Set it if allowed
if ($this->allowAdd) {
$originalData = $data;
}
} else {
// Original data is an array-like structure
// Add and remove items in the original variable
if ($this->allowDelete) {
foreach ($itemsToDelete as $key => $item) {
unset($originalData[$key]);
}
}

if ($this->allowAdd) {
foreach ($itemsToAdd as $key => $item) {
if (!isset($originalData[$key])) {
$originalData[$key] = $item;
} else {
$originalData[] = $item;
}
}
}
}

$event->setData($originalData);
}
}
Expand Up @@ -66,7 +66,8 @@ static public function getSubscribedEvents()
return array(
FormEvents::PRE_SET_DATA => 'preSetData',
FormEvents::PRE_BIND => 'preBind',
FormEvents::BIND_NORM_DATA => 'onBindNormData',
// (MergeCollectionListener, MergeDoctrineCollectionListener)
FormEvents::BIND_NORM_DATA => array('onBindNormData', 50),
);
}

Expand Down
24 changes: 21 additions & 3 deletions src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php
Expand Up @@ -19,6 +19,7 @@
use Symfony\Component\Form\Extension\Core\ChoiceList\SimpleChoiceList;
use Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface;
use Symfony\Component\Form\Extension\Core\EventListener\FixRadioInputListener;
use Symfony\Component\Form\Extension\Core\EventListener\MergeCollectionListener;
use Symfony\Component\Form\FormView;
use Symfony\Component\Form\Extension\Core\DataTransformer\ChoiceToValueTransformer;
use Symfony\Component\Form\Extension\Core\DataTransformer\ChoiceToBooleanArrayTransformer;
Expand Down Expand Up @@ -80,7 +81,10 @@ public function buildForm(FormBuilder $builder, array $options)

if ($options['expanded']) {
if ($options['multiple']) {
$builder->appendClientTransformer(new ChoicesToBooleanArrayTransformer($options['choice_list']));
$builder
->appendClientTransformer(new ChoicesToBooleanArrayTransformer($options['choice_list']))
->addEventSubscriber(new MergeCollectionListener(true, true))
;
} else {
$builder
->appendClientTransformer(new ChoiceToBooleanArrayTransformer($options['choice_list']))
Expand All @@ -89,12 +93,24 @@ public function buildForm(FormBuilder $builder, array $options)
}
} else {
if ($options['multiple']) {
$builder->appendClientTransformer(new ChoicesToValuesTransformer($options['choice_list']));
$builder
->appendClientTransformer(new ChoicesToValuesTransformer($options['choice_list']))
->addEventSubscriber(new MergeCollectionListener(
true,
true,
// If "by_reference" is disabled (explicit calling of
// the setter is desired), disable support for
// adders/removers
// Same as in CollectionType
$options['by_reference'],
$options['adder_prefix'],
$options['remover_prefix']
))
;
} else {
$builder->appendClientTransformer(new ChoiceToValueTransformer($options['choice_list']));
}
}

}

/**
Expand Down Expand Up @@ -140,6 +156,8 @@ public function getDefaultOptions(array $options)
'empty_data' => $multiple || $expanded ? array() : '',
'empty_value' => $multiple || $expanded || !isset($options['empty_value']) ? null : '',
'error_bubbling' => false,
'adder_prefix' => 'add',
'remover_prefix' => 'remove',
);
}

Expand Down
19 changes: 17 additions & 2 deletions src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php
Expand Up @@ -16,6 +16,7 @@
use Symfony\Component\Form\FormView;
use Symfony\Component\Form\FormInterface;
use Symfony\Component\Form\Extension\Core\EventListener\ResizeFormListener;
use Symfony\Component\Form\Extension\Core\EventListener\MergeCollectionListener;

class CollectionType extends AbstractType
{
Expand All @@ -29,16 +30,28 @@ public function buildForm(FormBuilder $builder, array $options)
$builder->setAttribute('prototype', $prototype->getForm());
}

$listener = new ResizeFormListener(
$resizeListener = new ResizeFormListener(
$builder->getFormFactory(),
$options['type'],
$options['options'],
$options['allow_add'],
$options['allow_delete']
);

$mergeListener = new MergeCollectionListener(
$options['allow_add'],
$options['allow_delete'],
// If "by_reference" is disabled (explicit calling of the setter
// is desired), disable support for adders/removers
// Same as in ChoiceType
$options['by_reference'],
$options['adder_prefix'],
$options['remover_prefix']
);

$builder
->addEventSubscriber($listener)
->addEventSubscriber($resizeListener)
->addEventSubscriber($mergeListener)
->setAttribute('allow_add', $options['allow_add'])
->setAttribute('allow_delete', $options['allow_delete'])
;
Expand Down Expand Up @@ -77,6 +90,8 @@ public function getDefaultOptions(array $options)
return array(
'allow_add' => false,
'allow_delete' => false,
'adder_prefix' => 'add',
'remover_prefix' => 'remove',
'prototype' => true,
'prototype_name' => '__name__',
'type' => 'text',
Expand Down

0 comments on commit baa63b8

Please sign in to comment.