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 cf8d43e

Browse filesBrowse files
fabpotfabpot
committed
bug #41534 [Form] Fix ChoiceType to effectively set and use translator (marek-binkowski-sim)
This PR was merged into the 5.3 branch. Discussion ---------- [Form] Fix ChoiceType to effectively set and use translator [Component][Form][ChoiceType] constructor line order changed to set translator before early return; [Component][Form][CoreExtension] a second argument $translator has been added for instantiation of [Component][Form][ChoiceType] | Q | A | ------------- | --- | Branch? | 5.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #41497 | License | MIT | Doc PR | - [Component][Form][ChoiceType] The bug probably came to life after a merge of two commits from different branches: 5.1 and 4.4. The commit in branch 5.1 contained a conditional early return related with the choiceListFactory: ```php if ($this->choiceListFactory instanceof CachingFactoryDecorator) { return; } ``` The code from the branch 4.4 was there to set the translator, but it wasn't effective when the constructor early returned in the line above. ```php $this->translator = $translator; ``` The fix was to switch [Component][Form][ChoiceType] constructor lines related with the translator above the early return related with choiceListFactory. As a second parameter $translator has been added to the [Component][Form][ChoiceType] constructor, the translator needs to be injected by the CoreExtension so that ChoiceType could set and use it. This is why a second argument is added now in [Component][Form][CoreExtension] loadTypes for Type\ChoiceType: ```php new Type\ChoiceType($this->choiceListFactory, $this->translator), ``` Commits ------- 2fdec3e [Form] Fix ChoiceType Extension to effectively set and use the translator
2 parents e5be65f + 2fdec3e commit cf8d43e
Copy full SHA for cf8d43e

File tree

3 files changed

+79
-6
lines changed
Filter options

3 files changed

+79
-6
lines changed

‎src/Symfony/Component/Form/Extension/Core/CoreExtension.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Extension/Core/CoreExtension.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ protected function loadTypes()
4545
new Type\FormType($this->propertyAccessor),
4646
new Type\BirthdayType(),
4747
new Type\CheckboxType(),
48-
new Type\ChoiceType($this->choiceListFactory),
48+
new Type\ChoiceType($this->choiceListFactory, $this->translator),
4949
new Type\CollectionType(),
5050
new Type\CountryType(),
5151
new Type\DateIntervalType(),

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php
+5-5Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,11 @@ public function __construct(ChoiceListFactoryInterface $choiceListFactory = null
6363
)
6464
);
6565

66+
if (null !== $translator && !$translator instanceof TranslatorInterface) {
67+
throw new \TypeError(sprintf('Argument 2 passed to "%s()" must be han instance of "%s", "%s" given.', __METHOD__, TranslatorInterface::class, \is_object($translator) ? \get_class($translator) : \gettype($translator)));
68+
}
69+
$this->translator = $translator;
70+
6671
// BC, to be removed in 6.0
6772
if ($this->choiceListFactory instanceof CachingFactoryDecorator) {
6873
return;
@@ -73,11 +78,6 @@ public function __construct(ChoiceListFactoryInterface $choiceListFactory = null
7378
if ($ref->getNumberOfParameters() < 3) {
7479
trigger_deprecation('symfony/form', '5.1', 'Not defining a third parameter "callable|null $filter" in "%s::%s()" is deprecated.', $ref->class, $ref->name);
7580
}
76-
77-
if (null !== $translator && !$translator instanceof TranslatorInterface) {
78-
throw new \TypeError(sprintf('Argument 2 passed to "%s()" must be han instance of "%s", "%s" given.', __METHOD__, TranslatorInterface::class, \is_object($translator) ? \get_class($translator) : \gettype($translator)));
79-
}
80-
$this->translator = $translator;
8181
}
8282

8383
/**

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

Copy file name to clipboard
+73Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
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\Tests\Extension\Core\Type;
13+
14+
use Symfony\Component\Form\Extension\Core\CoreExtension;
15+
use Symfony\Component\Form\Test\TypeTestCase;
16+
use Symfony\Contracts\Translation\TranslatorInterface;
17+
18+
class ChoiceTypeTranslationTest extends TypeTestCase
19+
{
20+
public const TESTED_TYPE = 'Symfony\Component\Form\Extension\Core\Type\ChoiceType';
21+
22+
private $choices = [
23+
'Bernhard' => 'a',
24+
'Fabien' => 'b',
25+
'Kris' => 'c',
26+
'Jon' => 'd',
27+
'Roman' => 'e',
28+
];
29+
30+
protected function getExtensions()
31+
{
32+
$translator = $this->createMock(TranslatorInterface::class);
33+
$translator->expects($this->any())->method('trans')
34+
->willReturnCallback(function ($key, $params) {
35+
return strtr(sprintf('Translation of: %s', $key), $params);
36+
}
37+
);
38+
39+
return array_merge(parent::getExtensions(), [new CoreExtension(null, null, $translator)]);
40+
}
41+
42+
public function testInvalidMessageAwarenessForMultiple()
43+
{
44+
$form = $this->factory->create(static::TESTED_TYPE, null, [
45+
'multiple' => true,
46+
'expanded' => false,
47+
'choices' => $this->choices,
48+
'invalid_message' => 'You are not able to use value "{{ value }}"',
49+
]);
50+
51+
$form->submit(['My invalid choice']);
52+
$this->assertEquals(
53+
"ERROR: Translation of: You are not able to use value \"My invalid choice\"\n",
54+
(string) $form->getErrors(true)
55+
);
56+
}
57+
58+
public function testInvalidMessageAwarenessForMultipleWithoutScalarOrArrayViewData()
59+
{
60+
$form = $this->factory->create(static::TESTED_TYPE, null, [
61+
'multiple' => true,
62+
'expanded' => false,
63+
'choices' => $this->choices,
64+
'invalid_message' => 'You are not able to use value "{{ value }}"',
65+
]);
66+
67+
$form->submit(new \stdClass());
68+
$this->assertEquals(
69+
"ERROR: Translation of: You are not able to use value \"stdClass\"\n",
70+
(string) $form->getErrors(true)
71+
);
72+
}
73+
}

0 commit comments

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