Skip to content

Commit cf8d43e

Browse 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

File tree

3 files changed

+79
-6
lines changed

3 files changed

+79
-6
lines changed

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

Lines 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

Lines 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

Lines 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)