8000 [Form] Made prefix of adder and remover method configurable. Adders a… · symfony/symfony@9b0245b · GitHub
[go: up one dir, main page]

Skip to content

Commit 9b0245b

Browse files
committed
[Form] Made prefix of adder and remover method configurable. Adders and removers are not called if "by_reference" is disabled.
1 parent 49d1464 commit 9b0245b

File tree

5 files changed

+170
-19
lines changed

5 files changed

+170
-19
lines changed

CHANGELOG-2.1.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,9 +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
198+
* the collection, choice (with multiple selection) and entity (with multiple
199199
selection) types now make use of addXxx() and removeXxx() methods in your
200200
model
201+
* added options "adder_prefix" and "remover_prefix" to collection and choice
202+
type
201203

202204
### HttpFoundation
203205

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

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,31 @@ class MergeCollectionListener implements EventSubscriberInterface
3535
*/
3636
private $allowDelete;
3737

38-
public function __construct($allowAdd = false, $allowDelete = false)
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')
3957
{
4058
$this->allowAdd = $allowAdd;
4159
$this->allowDelete = $allowDelete;
60+
$this->useAccessors = $useAccessors;
61+
$this->adderPrefix = $adderPrefix;
62+
$this->removerPrefix = $removerPrefix;
4263
}
4364

4465
static public function getSubscribedEvents()
@@ -68,14 +89,14 @@ public function onBindNormData(FilterDataEvent $event)
6889
}
6990

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

7697
foreach ($singulars as $singular) {
77-
$adderName = 'add' . $singular;
78-
$removerName = 'remove' . $singular;
98+
$adderName = $this->adderPrefix . $singular;
99+
$removerName = $this->removerPrefix . $singular;
79100

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

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,17 @@ public function buildForm(FormBuilder $builder, array $options)
9595
if ($options['multiple']) {
9696
$builder
9797
->appendClientTransformer(new ChoicesToValuesTransformer($options['choice_list']))
98-
->addEventSubscriber(new MergeCollectionListener(true, true))
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+
))
99109
;
100110
} else {
101111
$builder->appendClientTransformer(new ChoiceToValueTransformer($options['choice_list']));
@@ -146,6 +156,8 @@ public function getDefaultOptions(array $options)
146156
'empty_data' => $multiple || $expanded ? array() : '',
147157
'empty_value' => $multiple || $expanded || !isset($options['empty_value']) ? null : '',
148158
'error_bubbling' => false,
159+
'adder_prefix' => 'add',
160+
'remover_prefix' => 'remove',
149161
);
150162
}
151163

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,13 @@ public function buildForm(FormBuilder $builder, array $options)
4040

4141
$mergeListener = new MergeCollectionListener(
4242
$options['allow_add'],
43-
$options['allow_delete']
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']
4450
);
4551

4652
$builder
@@ -84,6 +90,8 @@ public function getDefaultOptions(array $options)
8490
return array(
8591
'allow_add' => false,
8692
'allow_delete' => false,
93+
'adder_prefix' => 'add',
94+
'remover_prefix' => 'remove',
8795
'prototype' => true,
8896
'prototype_name' => '__name__',
8997
'type' => 'text',

tests/Symfony/Tests/Component/Form/Extension/Core/EventListener/MergeCollectionListenerTest.php

Lines changed: 120 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,13 @@ public function addAxis($axis) {}
2424
public function removeAxis($axis) {}
2525
}
2626

27+
class MergeCollectionListenerTest_CarCustomPrefix
28+
{
29+
public function fooAxis($axis) {}
30+
31+
public function barAxis($axis) {}
32+
}
33+
2734
abstract class MergeCollectionListenerTest extends \PHPUnit_Framework_TestCase
2835
{
2936
private $dispatcher;
@@ -219,11 +226,12 @@ public function testDontDealWithNullOriginalDataIfNotAllowAdd()
219226
public function testCallAdderIfAllowAdd()
220227
{
221228
$parentData = $this->getMock(__CLASS__ . '_Car');
222-
$parentForm = $this->getForm('article');
229+
$parentForm = $this->getForm('car');
223230
$parentForm->setData($parentData);
224231
$parentForm->add($this->form);
225232

226-
$originalData = $this->getData(array(1 => 'second'));
233+
$originalDataArray = array(1 => 'second');
234+
$originalData = $this->getData($originalDataArray);
227235
$newData = $this->getData(array(0 => 'first', 1 => 'seco F42D nd', 2 => 'third'));
228236

229237
$this->form->setData($originalData);
@@ -239,20 +247,24 @@ public function testCallAdderIfAllowAdd()
239247
$listener = new MergeCollectionListener(true, false);
240248
$listener->onBindNormData($event);
241249

242-
// The original object was modified
243250
if (is_object($originalData)) {
244251
$this->assertSame($originalData, $event->getData());
245252
}
253+
254+
// The data was not modified directly
255+
// Thus it should not be written back into the parent data!
256+
$this->assertEquals($this->getData($originalDataArray), $event->getData());
246257
}
247258

248259
public function testDontCallAdderIfNotAllowAdd()
249260
{
250261
$parentData = $this->getMock(__CLASS__ . '_Car');
251-
$parentForm = $this->getForm('article');
262+
$parentForm = $this->getForm('car');
252263
$parentForm->setData($parentData);
253264
$parentForm->add($this->form);
254265

255-
$originalData = $this->getData(array(1 => 'second'));
266+
$originalDataArray = array(1 => 'second');
267+
$originalData = $this->getData($originalDataArray);
256268
$newData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
257269

258270
$this->form->setData($originalData);
@@ -264,20 +276,50 @@ public function testDontCallAdderIfNotAllowAdd()
264276
$listener = new MergeCollectionListener(false, false);
265277
$listener->onBindNormData($event);
266278

267-
// The original object was modified
268279
if (is_object($originalData)) {
269280
$this->assertSame($originalData, $event->getData());
270281
}
282+
283+
// The data was not modified
284+
$this->assertEquals($this->getData($originalDataArray), $event->getData());
285+
}
286+
287+
public function testDontCallAdderIfNotUseAccessors()
288+
{
289+
$parentData = $this->getMock(__CLASS__ . '_Car');
290+
$parentForm = $this->getForm('car');
291+
$parentForm->setData($parentData);
292+
$parentForm->add($this->form);
293+
294+
$originalData = $this->getData(array(1 => 'second'));
295+
$newData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
296+
297+
$this->form->setData($originalData);
298+
299+
$parentData->expects($this->never())
300+
->method('addAxis');
301+
302+
$event = new FilterDataEvent($this->form, $newData);
303+
$listener = new MergeCollectionListener(true, false, false);
304+
$listener->onBindNormData($event);
305+
306+
if (is_object($originalData)) {
307+
$this->assertSame($originalData, $event->getData());
308+
}
309+
310+
// The data was modified without accessors
311+
$this->assertEquals($newData, $event->getData());
271312
}
272313

273314
public function testCallRemoverIfAllowDelete()
274315
{
275316
$parentData = $this->getMock(__CLASS__ . '_Car');
276-
$parentForm = $this->getForm('article');
317+
$parentForm = $this->getForm('car');
277318
$parentForm->setData($parentData);
278319
$parentForm->add($this->form);
279320

280-
$originalData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
321+
$originalDataArray = array(0 => 'first', 1 => 'second', 2 => 'third');
322+
$originalData = $this->getData($originalDataArray);
281323
$newData = $this->getData(array(1 => 'second'));
282324

283325
$this->form->setData($originalData);
@@ -293,20 +335,24 @@ public function testCallRemoverIfAllowDelete()
293335
$listener = new MergeCollectionListener(false, true);
294336
$listener->onBindNormData($event);
295337

296-
// The original object was modified
297338
if (is_object($originalData)) {
298339
$this->assertSame($originalData, $event->getData());
299340
}
341+
342+
// The data was not modified directly
343+
// Thus it should not be written back into the parent data!
344+
$this->assertEquals($this->getData($originalDataArray), $event->getData());
300345
}
301346

302347
public function testDontCallRemoverIfNotAllowDelete()
303348
{
304349
$parentData = $this->getMock(__CLASS__ . '_Car');
305-
$parentForm = $this->getForm('article');
350+
$parentForm = $this->getForm('car');
306351
$parentForm->setData($parentData);
307352
$parentForm->add($this->form);
308353

309-
$originalData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
354+
$originalDataArray = array(0 => 'first', 1 => 'second', 2 => 'third');
355+
$originalData = $this->getData($originalDataArray);
310356
$newData = $this->getData(array(1 => 'second'));
311357

312358
$this->form->setData($originalData);
@@ -318,9 +364,71 @@ public function testDontCallRemoverIfNotAllowDelete()
318364
$listener = new MergeCollectionListener(false, false);
319365
$listener->onBindNormData($event);
320366

321-
// The original object was modified
322367
if (is_object($originalData)) {
323368
$this->assertSame($originalData, $event->getData());
324369
}
370+
371+
// The data was not modified
372+
$this->assertEquals($this->getData($originalDataArray), $event->getData());
373+
}
374+
375+
public function testDontCallRemoverIfNotUseAccessors()
376+
{
377+
$parentData = $this->getMock(__CLASS__ . '_Car');
378+
$parentForm = $this->getForm('car');
379+
$parentForm->setData($parentData);
380+
$parentForm->add($this->form);
381+
382+
$originalData = $this->getData(array(0 => 'first', 1 => 'second', 2 => 'third'));
383+
$newData = $this->getData(array(1 => 'second'));
384+
385+
$this->form->setData($originalData);
386+
387+
$parentData->expects($this->never())
388+
->method('removeAxis');
389+
390+
$event = new FilterDataEvent($this->form, $newData);
391+
$listener = new MergeCollectionListener(false, true, false);
392+
$listener->onBindNormData($event);
393+
394+
if (is_object($originalData)) {
395+
$this->assertSame($originalData, $event->getData());
396+
}
397+
398+
// The data was modified directly
399+
$this->assertEquals($newData, $event->getData());
400+
}
401+
402+
public function testCallAccessorsWithCustomPrefixes()
403+
{
404+
$parentData = $this->getMock(__CLASS__ . '_CarCustomPrefix');
405+
$parentForm = $this->getForm('car');
406+
$parentForm->setData($parentData);
407+
$parentForm->add($this->form);
408+
409+
$originalDataArray = array(1 => 'second');
410+
$originalData = $this->getData($originalDataArray);
411+
$newData = $this->getData(array(0 => 'first'));
412+
413+
$this->form->setData($originalData);
414+
415+
$parentData->expects($this->once())
416+
->method('fooAxis')
417+
->with('first');
418+
$parentData->expects($this->once())
419+
->method('barAxis')
420+
->with('second');
421+
422+
$event = new FilterDataEvent($this->form, $newData);
423+
$listener = new MergeCollectionListener(true, true, true, 'foo', 'bar');
424+
$listener->onBindNormData($event);
425+
426+
if (is_object($originalData)) {
427+
$this->assertSame($originalData, $event->getData());
428+
}
429+
430+
// The data was not modified directly
431+
// Thus it should not be written back into the parent data!
432+
$this->assertEquals($this->getData($originalDataArray), $event->getData());
325433
}
326434
}

0 commit comments

Comments
 (0)
0