[Form] [DoctrineBridge] added a failing test for EntityType #15328
Conversation
|
Hi @stof, I'd like to bring this pull request to your attention. It isn't passing the tests because it isn't intended to -- it consists of a new test whose failure exposes what I believe is a BC-breaking bug. Is this an okay thing to do? Update: I've just committed a possible fix. |
|
Hi @stof, @webmozart, could I trouble one of you to look at this pull request? I believe it is a BC break introduced in 2.7. I'm pretty confident in that the tests I added are correctly exposing the problem, but I'm not at all confident that my proposed fix is the best solution. Your feedback would be much appreciated. |
5b0f992 to
587e409
Compare
|
Update: it seems that someone else committed a fix for the same problem, so I've removed my conflicting code. This PR now amounts to just an added test case. |
|
👍 |
|
|
||
| $this->persist(array($single, $assoc)); | ||
|
|
||
| $field = $this->factory->createNamed('name', 'entity', null, array( |
There was a problem hiding this comment.
"entity" needs to be replaced by the entity form type FQCN
There was a problem hiding this comment.
hm, actually that only needs to be changed in 2.8 where this has been deprecated. So the merger would need to make the changes.
|
Please rebase on current 2.7 to see if tests really pass after the above changes. |
The new test uses an entity whose primary key is a foreign key to another entity (an "association key"), as described here: http://doctrine-orm.readthedocs.org/en/latest/tutorials/composite-primary-keys.html#identity-through-foreign-entities
587e409 to
046f1e8
Compare
|
@ianfp |
|
status: reviewed |
|
@ianfp #15251 also contains a test. Please let me know if you still disagree. :) |
|
@xabbuh Oops! You're right. This one can stay closed. |
*This is a new test for
EntityTypethat uses an entity whose primary key is a foreign key toanother entity (an "association key"), as described here:
http://doctrine-orm.readthedocs.org/en/latest/tutorials/composite-primary-keys.html#identity-through-foreign-entities
Using the
entityform type with an association-key entity used to work before 2.7, so I believe this is a BC break.Update: I've added a possible fix to the
IdReaderclass in the latest commit. I don't know if it is the right fix, but...*I say the tests pass "sometimes" becauseSymfony\Component\HttpKernel\Tests\HttpCache\HttpCacheTestis failing intermittently, and I don't think it is related to my change.