8000 merged branch bschussek/issue1540 (PR #3239) · Olajide/symfony@baa63b8 · GitHub 8000
[go: up one dir, main page]
Skip to content

Commit baa63b8

Browse files
fabpotfabpot
committed
merged branch bschussek/issue1540 (PR symfony#3239)
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: symfony#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.
2 parents 3c8d300 + 9b0245b commit baa63b8

File tree

15 files changed

+1093
-40
lines changed

15 files changed

+1093
-40
lines changed

‎CHANGELOG-2.1.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,11 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c
195195
* choice fields now throw a FormException if neither the "choices" nor the
196196
"choice_list" option is set
197197
* the radio type is now a child of the checkbox type
198+
* the collection, choice (with multiple selection) and entity (with multiple
199+
selection) types now make use of addXxx() and removeXxx() methods in your
200+
model
201+
* added options "adder_prefix" and "remover_prefix" to collection and choice
202+
type
198203

199204
### HttpFoundation
200205

‎src/Symfony/Bridge/Doctrine/Form/EventListener/MergeCollectionListener.php renamed to ‎src/Symfony/Bridge/Doctrine/Form/EventListener/MergeDoctrineCollectionListener.php

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -24,37 +24,24 @@
2424
*
2525
* @see Doctrine\Common\Collections\Collection
2626
*/
27-
class MergeCollectionListener implements EventSubscriberInterface
27+
class MergeDoctrineCollectionListener implements EventSubscriberInterface
2828
{
2929
static public function getSubscribedEvents()
3030
{
31-
return array(FormEvents::BIND_NORM_DATA => 'onBindNormData');
31+
// Higher priority than core MergeCollectionListener so that this one
32+
// is called before
33+
return array(FormEvents::BIND_NORM_DATA => array('onBindNormData', 10));
3234
}
3335

3436
public function onBindNormData(FilterDataEvent $event)
3537
{
3638
$collection = $event->getForm()->getData();
3739
$data = $event->getData();
3840

39-
if (!$collection) {
40-
$collection = $data;
41-
} elseif (count($data) === 0) {
41+
// If all items were removed, call clear which has a higher
42+
// performance on persistent collections
43+
if ($collection && count($data) === 0) {
4244
$collection->clear();
43-
} else {
44-
// merge $data into $collection
45-
foreach ($collection as $entity) {
46-
if (!$data->contains($entity)) {
47-
$collection->removeElement($entity);
48-
} else {
49-
$data->removeElement($entity);
50-
}
51-
}
52-
53-
foreach ($data as $entity) {
54-
$collection->add($entity);
55-
}
5645
}
57-
58-
$event->setData($collection);
5946
}
6047
}

‎src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
use Symfony\Component\Form\FormBuilder;
1717
use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityChoiceList;
1818
use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityLoaderInterface;
19-
use Symfony\Bridge\Doctrine\Form\EventListener\MergeCollectionListener;
19+
use Symfony\Bridge\Doctrine\Form\EventListener\MergeDoctrineCollectionListener;
2020
use Symfony\Bridge\Doctrine\Form\DataTransformer\CollectionToArrayTransformer;
2121
use Symfony\Component\Form\AbstractType;
2222

@@ -36,7 +36,7 @@ public function buildForm(FormBuilder $builder, array $options)
3636
{
3737
if ($options['multiple']) {
3838
$builder
39-
->addEventSubscriber(new MergeCollectionListener())
39+
->addEventSubscriber(new MergeDoctrineCollectionListener())
4040
->prependClientTransformer(new CollectionToArrayTransformer())
4141
;
4242
}

‎src/Symfony/Component/Form/Extension/Core/EventListener/MergeCollectionListener.php

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Form\Extension\Core\EventListener;
13+
14+
use Symfony\Component\Form\Util\FormUtil;
15+
16+
use Symfony\Component\Form\FormEvents;
17+
use Symfony\Component\Form\Event\FilterDataEvent;
18+
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
19+
use Symfony\Component\Form\Exception\UnexpectedTypeException;
20+
21+
/**
22+
* @author Bernhard Schussek <bschussek@gmail.com>
23+
*/
24+
class MergeCollectionListener implements EventSubscriberInterface
25+
{
26+
/**
27+
* Whether elements may be added to the collection
28+
* @var Boolean
29+
*/
30+
private $allowAdd;
31+
32+
/**
33+
* Whether elements may be removed from the collection
34+
* @var Boolean
35+
*/
36+
private $allowDelete;
37+
38+
/**
39+
* Whether to search for and use adder and remover methods
40+
* @var Boolean
41+
*/
42+
private $useAccessors;
43+
44+
/**
45+
* The prefix of the adder method to look for
46+
* @var string
47+
*/
48+
private $adderPrefix;
49+
50+
/**
51+
* The prefix of the remover method to look for
52+
* @var string
53+
*/
54+
private $removerPrefix;
55+
56+
public function __construct($allowAdd = false, $allowDelete = false, $useAccessors = true, $adderPrefix = 'add', $removerPrefix = 'remove')
57+
{
58+
$this->allowAdd = $allowAdd;
59+
$this->allowDelete = $allowDelete;
60+
$this->useAccessors = $useAccessors;
61+
$this->adderPrefix = $adderPrefix;
62+
$this->removerPrefix = $removerPrefix;
63+
}
64+
65+
static public function getSubscribedEvents()
66+
{
67+
return array(FormEvents::BIND_NORM_DATA => 'onBindNormData');
68+
}
69+
70+
public function onBindNormData(FilterDataEvent $event)
71+
{
72+
$originalData = $event->getForm()->getData();
73+
$form = $event->getForm();
74+
$data = $event->getData();
75+
$parentData = $form->hasParent() ? $form->getParent()->getData() : null;
76+
$adder = null;
77+
$remover = null;
78+
79+
if (null === $data) {
80+
$data = array();
81+
}
82+
83+
if (!is_array($data) && !($data instanceof \Traversable && $data instanceof \ArrayAccess)) {
84+
throw new UnexpectedTypeException($data, 'array or (\Traversable and \ArrayAccess)');
85+
}
86+
87+
if (null !== $originalData && !is_array($originalData) && !($originalData instanceof \Traversable && $originalData instanceof \ArrayAccess)) {
88+
throw new UnexpectedTypeException($originalData, 'array or (\Traversable and \ArrayAccess)');
89+
}
90+
91+
// Check if the parent has matching methods to add/remove items
92+
if ($this->useAccessors && is_object($parentData)) {
93+
$plural = ucfirst($form->getName());
94+
$singulars = (array) FormUtil::singularify($plural);
95+
$reflClass = new \ReflectionClass($parentData);
96+
97+
foreach ($singulars as $singular) {
98+
$adderName = $this->adderPrefix . $singular;
99+
$removerName = $this->removerPrefix . $singular;
100+
101+
if ($reflClass->hasMethod($adderName) && $reflClass->hasMethod($removerName)) {
102+
$adder = $reflClass->getMethod($adderName);
103+
$remover = $reflClass->getMethod($removerName);
104+
105+
if ($adder->isPublic() && $adder->getNumberOfRequiredParameters() === 1
106+
&& $remover->isPublic() && $remover->getNumberOfRequiredParameters() === 1) {
107+
108+
// We found a public, one-parameter add and remove method
109+
break;
110+
}
111+
112+
// False alert
113+
$adder = null;
114+
$remover = null;
115+
}
116+
}
117+
}
118+
119+
// Check which items are in $data that are not in $originalData and
120+
// vice versa
121+
$itemsToDelete = array();
122+
$itemsToAdd = is_object($data) ? clone $data : $data;
123+
124+
if ($originalData) {
125+
foreach ($originalData as $originalKey => $originalItem) {
126+
foreach ($data as $key => $item) {
127+
if ($item === $originalItem) {
128+
// Item found, next original item
129+
unset($itemsToAdd[$key]);
130+
continue 2;
131+
}
132+
}
133+
134+
// Item not found, remember for deletion
135+
$itemsToDelete[$originalKey] = $originalItem;
136+
}
137+
}
138+
139+
if ($adder && $remover) {
140+
// If methods to add and to remove exist, call them now, if allowed
141+
if ($this->allowDelete) {
142+
foreach ($itemsToDelete as $item) {
143+
$remover->invoke($parentData, $item);
144+
}
145+
}
146+
147+
if ($this->allowAdd) {
148+
foreach ($itemsToAdd as $item) {
149+
$adder->invoke($parentData, $item);
150+
}
151+
}
152+
} elseif (!$originalData) {
153+
// No original data was set. Set it if allowed
154+
if ($this->allowAdd) {
155+
$originalData = $data;
156+
}
157+
} else {
158+
// Original data is an array-like structure
159+
// Add and remove items in the original variable
160+
if ($this->allowDelete) {
161+
foreach ($itemsToDelete as $key => $item) {
162+
unset($originalData[$key]);
163+
}
164+
}
165+
166+
if ($this->allowAdd) {
167+
foreach ($itemsToAdd as $key => $item) {
168+
if (!isset($originalData[$key])) {
169+
$originalData[$key] = $item;
170+
} else {
171+
$originalData[] = $item;
172+
}
173+
}
174+
}
175+
}
176+
177+
$event->setData($originalData);
178+
}
179+
}

‎src/Symfony/Component/Form/Extension/Core/EventListener/ResizeFormListener.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ static public function getSubscribedEvents()
6666
return array(
6767
FormEvents::PRE_SET_DATA => 'preSetData',
6868
FormEvents::PRE_BIND => 'preBind',
69-
FormEvents::BIND_NORM_DATA => 'onBindNormData',
69+
// (MergeCollectionListener, MergeDoctrineCollectionListener)
70+
FormEvents::BIND_NORM_DATA => array('onBindNormData', 50),
7071
);
7172
}
7273

‎src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use Symfony\Component\Form\Extension\Core\ChoiceList\SimpleChoiceList;
2020
use Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface;
2121
use Symfony\Component\Form\Extension\Core\EventListener\FixRadioInputListener;
22+
use Symfony\Component\Form\Extension\Core\EventListener\MergeCollectionListener;
2223
use Symfony\Component\Form\FormView;
2324
use Symfony\Component\Form\Extension\Core\DataTransformer\ChoiceToValueTransformer;
2425
use Symfony\Component\Form\Extension\Core\DataTransformer\ChoiceToBooleanArrayTransformer;
@@ -80,7 +81,10 @@ public function buildForm(FormBuilder $builder, array $options)
8081

8182
if ($options['expanded']) {
8283
if ($options['multiple']) {
83-
$builder->appendClientTransformer(new ChoicesToBooleanArrayTransformer($options['choice_list']));
84+
$builder
85+
->appendClientTransformer(new ChoicesToBooleanArrayTransformer($options['choice_list']))
86+
10000 ->addEventSubscriber(new MergeCollectionListener(true, true))
87+
;
8488
} else {
8589
$builder
8690
->appendClientTransformer(new ChoiceToBooleanArrayTransformer($options['choice_list']))
@@ -89,12 +93,24 @@ public function buildForm(FormBuilder $builder, array $options)
8993
}
9094
} else {
9195
if ($options['multiple']) {
92-
$builder->appendClientTransformer(new ChoicesToValuesTransformer($options['choice_list']));
96+
$builder
97+
->appendClientTransformer(new ChoicesToValuesTransformer($options['choice_list']))
98+
->addEventSubscriber(new MergeCollectionListener(
99+
true,
100+
true,
101+
// If "by_reference" is disabled (explicit calling of
102+
// the setter is desired), disable support for
103+
// adders/removers
104+
// Same as in CollectionType
105+
$options['by_reference'],
106+
$options['adder_prefix'],
107+
$options['remover_prefix']
108+
))
109+
;
93110
} else {
94111
$builder->appendClientTransformer(new ChoiceToValueTransformer($options['choice_list']));
95112
}
96113
}
97-
98114
}
99115

100116
/**
@@ -140,6 +156,8 @@ public function getDefaultOptions(array $options)
140156
'empty_data' => $multiple || $expanded ? array() : '',
141157
'empty_value' => $multiple || $expanded || !isset($options['empty_value']) ? null : '',
142158
'error_bubbling' => false,
159+
'adder_prefix' => 'add',
160+
'remover_prefix' => 'remove',
143161
);
144162
}
145163

‎src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Symfony\Component\Form\FormView;
1717
use Symfony\Component\Form\FormInterface;
1818
use Symfony\Component\Form\Extension\Core\EventListener\ResizeFormListener;
19+
use Symfony\Component\Form\Extension\Core\EventListener\MergeCollectionListener;
1920

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

32-
$listener = new ResizeFormListener(
33+
$resizeListener = new ResizeFormListener(
3334
$builder->getFormFactory(),
3435
$options['type'],
3536
$options['options'],
3637
$options['allow_add'],
3738
$options['allow_delete']
3839
);
3940

41+
$mergeListener = new MergeCollectionListener(
42+
$options['allow_add'],
43+
$options['allow_delete'],
44+
// If "by_reference" is disabled (explicit calling of the setter
45+
// is desired), disable support for adders/removers
46+
// Same as in ChoiceType
47+
$options['by_reference'],
48+
$options['adder_prefix'],
49+
$options['remover_prefix']
50+
);
51+
4052
$builder
41-
->addEventSubscriber($listener)
53+
->addEventSubscriber($resizeListener)
54+
->addEventSubscriber($mergeListener)
4255
->setAttribute('allow_add', $options['allow_add'])
4356
->setAttribute('allow_delete', $options['allow_delete'])
4457
;
@@ -77,6 +90,8 @@ public function getDefaultOptions(array $options)
7790
return array(
7891
'allow_add' => false,
7992
'allow_delete' => false,
93+
'adder_prefix' => 'add',
94+
'remover_prefix' => 'remove',
8095
'prototype' => true,
8196
'prototype_name' => '__name__',
8297
'type' => 'text',

0 commit comments

Comments
 (0)
0