Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 65cbf18

Browse filesBrowse files
HeahDudexabbuh
HeahDude
authored and
xabbuh
committed
[Form] Fix same choice loader with different choice values
1 parent a6f405a commit 65cbf18
Copy full SHA for 65cbf18

File tree

9 files changed

+100
-32
lines changed
Filter options

9 files changed

+100
-32
lines changed

‎src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php
+9-12Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313

1414
use Doctrine\Persistence\ObjectManager;
1515
use Symfony\Component\Form\ChoiceList\ArrayChoiceList;
16-
use Symfony\Component\Form\ChoiceList\ChoiceListInterface;
1716
use Symfony\Component\Form\ChoiceList\Loader\ChoiceLoaderInterface;
1817

1918
/**
@@ -29,9 +28,9 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface
2928
private $objectLoader;
3029

3130
/**
32-
* @var ChoiceListInterface
31+
* @var array|null
3332
*/
34-
private $choiceList;
33+
private $choices;
3534

3635
/**
3736
* Creates a new choice loader.
@@ -74,15 +73,13 @@ public function __construct(ObjectManager $manager, string $class, IdReader $idR
7473
*/
7574
public function loadChoiceList($value = null)
7675
{
77-
if ($this->choiceList) {
78-
return $this->choiceList;
76+
if (null === $this->choices) {
77+
$this->choices = $this->objectLoader
78+
? $this->objectLoader->getEntities()
79+
: $this->manager->getRepository($this->class)->findAll();
7980
}
8081

81-
$objects = $this->objectLoader
82-
? $this->objectLoader->getEntities()
83-
: $this->manager->getRepository($this->class)->findAll();
84-
85-
return $this->choiceList = new ArrayChoiceList($objects, $value);
82+
return new ArrayChoiceList($this->choices, $value);
8683
}
8784

8885
/**
@@ -100,7 +97,7 @@ public function loadValuesForChoices(array $choices, $value = null)
10097
$optimize = $this->idReader && (null === $value || \is_array($value) && $value[0] === $this->idReader);
10198

10299
// Attention: This optimization does not check choices for existence
103-
if ($optimize && !$this->choiceList && $this->idReader->isSingleId()) {
100+
if ($optimize && !$this->choices && $this->idReader->isSingleId()) {
104101
$values = [];
105102

106103
// Maintain order and indices of the given objects
@@ -136,7 +133,7 @@ public function loadChoicesForValues(array $values, $value = null)
136133
// a single-field identifier
137134
$optimize = $this->idReader && (null === $value || \is_array($value) && $this->idReader === $value[0]);
138135

139-
if ($optimize && !$this->choiceList && $this->objectLoader && $this->idReader->isSingleId()) {
136+
if ($optimize && !$this->choices && $this->objectLoader && $this->idReader->isSingleId()) {
140137
$unorderedObjects = $this->objectLoader->getEntitiesByIds($this->idReader->getIdField(), $values);
141138
$objectsById = [];
142139
$objects = [];

‎src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/DoctrineChoiceLoaderTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/DoctrineChoiceLoaderTest.php
+1-2Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,7 @@ public function testLoadChoiceListUsesObjectLoaderIfAvailable()
146146
$this->assertEquals($choiceList, $loaded = $loader->loadChoiceList());
147147

148148
// no further loads on subsequent calls
149-
150-
$this->assertSame($loaded, $loader->loadChoiceList());
149+
$this->assertEquals($loaded, $loader->loadChoiceList());
151150
}
152151

153152
public function testLoadValuesForChoices()

‎src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php
+28Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1779,4 +1779,32 @@ public function testSubmitNullMultipleUsesDefaultEmptyData()
17791779
$this->assertEquals($collection, $form->getNormData());
17801780
$this->assertEquals($collection, $form->getData());
17811781
}
1782+
1783+
public function testWithSameLoaderAndDifferentChoiceValueCallbacks()
1784+
{
1785+
$entity1 = new SingleIntIdEntity(1, 'Foo');
1786+
$entity2 = new SingleIntIdEntity(2, 'Bar');
1787+
$this->persist([$entity1, $entity2]);
1788+
1789+
$view = $this->factory->create(FormTypeTest::TESTED_TYPE)
1790+
->add('entity_one', self::TESTED_TYPE, [
1791+
'em' => 'default',
1792+
'class' => self::SINGLE_IDENT_CLASS,
1793+
])
1794+
->add('entity_two', self::TESTED_TYPE, [
1795+
'em' => 'default',
1796+
'class' => self::SINGLE_IDENT_CLASS,
1797+
'choice_value' => function ($choice) {
1798+
return $choice ? $choice->name : '';
1799+
},
1800+
])
1801+
->createView()
1802+
;
1803+
1804+
$this->assertSame('1', $view['entity_one']->vars['choices'][1]->value);
1805+
$this->assertSame('2', $view['entity_one']->vars['choices'][2]->value);
1806+
1807+
$this->assertSame('Foo', $view['entity_two']->vars['choices']['Foo']->value);
1808+
$this->assertSame('Bar', $view['entity_two']->vars['choices']['Bar']->value);
1809+
}
17821810
}

‎src/Symfony/Bridge/Doctrine/composer.json

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/composer.json
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
"symfony/stopwatch": "^3.4|^4.0|^5.0",
3030
"symfony/config": "^4.2|^5.0",
3131
"symfony/dependency-injection": "^3.4|^4.0|^5.0",
32-
"symfony/form": "^4.4.11|^5.0.11",
32+
"symfony/form": "^4.4.41|^5.0.11",
3333
"symfony/http-kernel": "^4.3.7",
3434
"symfony/messenger": "^4.4|^5.0",
3535
"symfony/property-access": "^3.4|^4.0|^5.0",

‎src/Symfony/Component/Form/ChoiceList/Loader/CallbackChoiceLoader.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/ChoiceList/Loader/CallbackChoiceLoader.php
+6-6Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,11 @@ class CallbackChoiceLoader implements ChoiceLoaderInterface
2323
private $callback;
2424

2525
/**
26-
* The loaded choice list.
26+
* The loaded choices.
2727
*
28-
* @var ArrayChoiceList
28+
* @var array|null
2929
*/
30-
private $choiceList;
30+
private $choices;
3131

3232
/**
3333
* @param callable $callback The callable returning an array of choices
@@ -42,11 +42,11 @@ public function __construct(callable $callback)
4242
*/
4343
public function loadChoiceList($value = null)
4444
{
45-
if (null !== $this->choiceList) {
46-
return $this->choiceList;
45+
if (null === $this->choices) {
46+
$this->choices = ($this->callback)();
4747
}
4848

49-
return $this->choiceList = new ArrayChoiceList(($this->callback)(), $value);
49+
return new ArrayChoiceList($this->choices, $value);
5050
}
5151

5252
/**

‎src/Symfony/Component/Form/Tests/ChoiceList/LazyChoiceListTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Tests/ChoiceList/LazyChoiceListTest.php
+5-5Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public function testGetChoiceLoadersLoadsLoadedListOnFirstCall()
3232

3333
$this->assertSame(['RESULT'], $list->getChoices());
3434
$this->assertSame(['RESULT'], $list->getChoices());
35-
$this->assertSame(1, $calls);
35+
$this->assertSame(2, $calls);
3636
}
3737

3838
public function testGetValuesLoadsLoadedListOnFirstCall()
@@ -46,7 +46,7 @@ public function testGetValuesLoadsLoadedListOnFirstCall()
4646

4747
$this->assertSame(['RESULT'], $list->getValues());
4848
$this->assertSame(['RESULT'], $list->getValues());
49-
$this->assertSame(1, $calls);
49+
$this->assertSame(2, $calls);
5050
}
5151

5252
public function testGetStructuredValuesLoadsLoadedListOnFirstCall()
@@ -60,7 +60,7 @@ public function testGetStructuredValuesLoadsLoadedListOnFirstCall()
6060

6161
$this->assertSame(['RESULT'], $list->getStructuredValues());
6262
$this->assertSame(['RESULT'], $list->getStructuredValues());
63-
$this->assertSame(1, $calls);
63+
$this->assertSame(2, $calls);
6464
}
6565

6666
public function testGetOriginalKeysLoadsLoadedListOnFirstCall()
@@ -79,7 +79,7 @@ public function testGetOriginalKeysLoadsLoadedListOnFirstCall()
7979

8080
$this->assertSame(['foo' => 'a', 'bar' => 'b', 'baz' => 'c'], $list->getOriginalKeys());
8181
$this->assertSame(['foo' => 'a', 'bar' => 'b', 'baz' => 'c'], $list->getOriginalKeys());
82-
$this->assertSame(3, $calls);
82+
$this->assertSame(6, $calls);
8383
}
8484

8585
public function testGetChoicesForValuesForwardsCallIfListNotLoaded()
@@ -98,7 +98,7 @@ public function testGetChoicesForValuesForwardsCallIfListNotLoaded()
9898

9999
$this->assertSame(['foo', 'bar'], $list->getChoicesForValues(['a', 'b']));
100100
$this->assertSame(['foo', 'bar'], $list->getChoicesForValues(['a', 'b']));
101-
$this->assertSame(3, $calls);
101+
$this->assertSame(6, $calls);
102102
}
103103

104104
public function testGetChoicesForValuesUsesLoadedList()

‎src/Symfony/Component/Form/Tests/ChoiceList/Loader/CallbackChoiceLoaderTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Tests/ChoiceList/Loader/CallbackChoiceLoaderTest.php
+10-3Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,18 @@ public function testLoadChoiceList()
6767
$this->assertInstanceOf(ChoiceListInterface::class, self::$loader->loadChoiceList(self::$value));
6868
}
6969

70-
public function testLoadChoiceListOnlyOnce()
70+
public function testLoadChoicesOnlyOnce()
7171
{
72-
$loadedChoiceList = self::$loader->loadChoiceList(self::$value);
72+
$calls = 0;
73+
$loader = new CallbackChoiceLoader(function () use (&$calls) {
74+
++$calls;
7375

74-
$this->assertSame($loadedChoiceList, self::$loader->loadChoiceList(self::$value));
76+
return [1];
77+
});
78+
$loadedChoiceList = $loader->loadChoiceList();
79+
80+
$this->assertNotSame($loadedChoiceList, $loader->loadChoiceList());
81+
$this->assertSame(1, $calls);
7582
}
7683

7784
public function testLoadChoicesForValuesLoadsChoiceListOnFirstCall()

‎src/Symfony/Component/Form/Tests/ChoiceList/Loader/IntlCallbackChoiceLoaderTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Tests/ChoiceList/Loader/IntlCallbackChoiceLoaderTest.php
+11-3Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,19 @@ public function testLoadChoiceList()
6868
$this->assertInstanceOf(ChoiceListInterface::class, self::$loader->loadChoiceList(self::$value));
6969
}
7070

71-
public function testLoadChoiceListOnlyOnce()
71+
public function testLoadChoicesOnlyOnce()
7272
{
73-
$loadedChoiceList = self::$loader->loadChoiceList(self::$value);
73+
$calls = 0;
74+
$loader = new IntlCallbackChoiceLoader(function () use (&$calls) {
75+
++$calls;
7476

75-
$this->assertSame($loadedChoiceList, self::$loader->loadChoiceList(self::$value));
77+
return self::$choices;
78+
});
79+
80+
$loadedChoiceList = $loader->loadChoiceList(self::$value);
81+
82+
$this->assertNotSame($loadedChoiceList, $loader->loadChoiceList(self::$value));
83+
$this->assertSame(1, $calls);
7684
}
7785

7886
public function testLoadChoicesForValuesLoadsChoiceListOnFirstCall()

‎src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php
+29Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\Form\Tests\Extension\Core\Type;
1313

14+
use Symfony\Component\Form\ChoiceList\Loader\CallbackChoiceLoader;
1415
use Symfony\Component\Form\ChoiceList\View\ChoiceGroupView;
1516
use Symfony\Component\Form\ChoiceList\View\ChoiceView;
1617
use Symfony\Component\Form\Extension\Validator\ValidatorExtension;
@@ -2165,4 +2166,32 @@ public function expandedIsEmptyWhenNoRealChoiceIsSelectedProvider()
21652166
'Placeholder submitted / single / not required / with a placeholder -> should not be empty' => [false, '', false, false, 'ccc'], // The placeholder is a selected value
21662167
];
21672168
}
2169+
2170+
public function testWithSameLoaderAndDifferentChoiceValueCallbacks()
2171+
{
2172+
$choiceLoader = new CallbackChoiceLoader(function () {
2173+
return [1, 2, 3];
2174+
});
2175+
2176+
$view = $this->factory->create(FormTypeTest::TESTED_TYPE)
2177+
->add('choice_one', self::TESTED_TYPE, [
2178+
'choice_loader' => $choiceLoader,
2179+
])
2180+
->add('choice_two', self::TESTED_TYPE, [
2181+
'choice_loader' => $choiceLoader,
2182+
'choice_value' => function ($choice) {
2183+
return $choice ? (string) $choice * 10 : '';
2184+
},
2185+
])
2186+
->createView()
2187+
;
2188+
2189+
$this->assertSame('1', $view['choice_one']->vars['choices'][0]->value);
2190+
$this->assertSame('2', $view['choice_one']->vars['choices'][1]->value);
2191+
$this->assertSame('3', $view['choice_one']->vars['choices'][2]->value);
2192+
2193+
$this->assertSame('10', $view['choice_two']->vars['choices'][0]->value);
2194+
$this->assertSame('20', $view['choice_two']->vars['choices'][1]->value);
2195+
$this->assertSame('30', $view['choice_two']->vars['choices'][2]->value);
2196+
}
21682197
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.