[Form] Fix reverseTransform on multiple entity form type #3734
Conversation
| public function testReverseTransformEmptyArray() | ||
| { | ||
| $this->assertSame(array(), $this->transformer->reverseTransform(array())); | ||
| } |
There was a problem hiding this comment.
Your test does not cover the issue. In fact, the DataTransformer is only tested with the SimpleChoiceList and not all the classes implementing the ChoiceListInterface interface. If you try to execute your new test without your fix, the test suite still passes.
To test the issue reported in #3727 isn't caracterized by your unit test. To reproduce the bug, you'd need to initialize the $this->transformer with an EntityChoiceList. However, this classe belong to the DoctrineBridge, so I'm unsure on where to put this test.
|
@bschussek ping |
|
This is an alternate implementation for #3727 |
|
OK, this is another possibility to fix this issue with working tests. What do you think about this? |
| @@ -25,7 +26,7 @@ class EntityChoiceListTest extends DoctrineOrmTestCase | |||
| const SINGLE_IDENT_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIdentEntity'; | |||
|
|
|||
| const COMPOSITE_IDENT_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\CompositeIdentEntity'; | |||
|
|
|||
|
OK, just done. |
|
@beberlei @bschussek ping |
|
@fabpot 👍 |
Commits ------- a430f3d [#3446] [Form] Fix getChoicesForValues of EntityChoiceList on empty values Discussion ---------- [Form] Fix reverseTransform on multiple entity form type Bug fix: yes Feature addition: no Backwards compatibility break: no Symfony2 tests pass: yes Fixes the following tickets: #3446, #3727 Todo: - --------------------------------------------------------------------------- by stof at 2012-04-03T23:05:55Z @bschussek ping --------------------------------------------------------------------------- by stof at 2012-04-03T23:06:45Z This is an alternate implementation for #3727 --------------------------------------------------------------------------- by chmielot at 2012-04-04T13:47:27Z OK, this is another possibility to fix this issue with working tests. What do you think about this? --------------------------------------------------------------------------- by chmielot at 2012-04-04T13:51:27Z OK, just done. --------------------------------------------------------------------------- by stof at 2012-04-04T13:51:39Z @beberlei @bschussek ping --------------------------------------------------------------------------- by bschussek at 2012-04-06T18:50:37Z @fabpot 👍
|
@fabpot @bschussek is it possible to have this merged in 2.0 branch aswell? |
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #3446, #3727
Todo: -