Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Doctrine Bridge] fix UniqueEntityValidator for composite object primary keys#21288
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
3fd23b7 to5aadce3Compare| $identifiers =array_map(function ($identifier)use ($em) { | ||
| // identifiers can be objects (without any __toString method) if its a composite PK | ||
| if (is_object($identifier) && !method_exists($identifier,'__toString')) { | ||
| returnsprintf('(%s)',$this->buildInvalidValueString($em,$em->getClassMetadata(get_class($identifier)),$identifier)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
is it safe to use the same object manager here for the related class? Not sure if Doctrine supports relations between classes of different object managers?
| $this->validator->validate($newEntity,$constraint); | ||
| $expectedValue ='Object of class "Symfony\Bridge\Doctrine\Tests\Fixtures\CompositeObjectNoToStringIdEntity" identified by "(Object of class "Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdNoToStringEntity" identified by "1"), (Object of class "Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdNoToStringEntity" identified by "2")"'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
should we find a shorter way to build this message? 😝
dmaicher commentedJan 14, 2017
HeahDude commentedJan 16, 2017
I've discussed this PR with@dmaicher indmaicher#1, which is now merged. This looks good to me. ping@symfony/deciders |
nicolas-grekas commentedJan 16, 2017
Failure looks related to me (not the messages about getMock, but the one about CompositeObjectNoToStringIdEntity) - maybe a composer.json update needed somewhere? |
HeahDude commentedJan 16, 2017
@nicolas-grekas I've openeddmaicher#2 which seems to have fixed the tests, waiting for@dmaicher to merge it. |
nicolas-grekas commentedJan 16, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@dmaicher can you please squash the history? We can't merge (nor don't want to :) ) merge PRs with merges inside. |
nicolas-grekas commentedJan 16, 2017
(maybe keep just two commits, one for you and one for@HeahDude with rebase -i ?) |
dmaicher commentedJan 16, 2017
@nicolas-grekas sure 😉 Let's wait for the build and hope its fixed |
HeahDude commentedJan 16, 2017
Green 🎉 |
Fixed UniqueEntityValidator tests
d942a96 tob3ced86Comparedmaicher commentedJan 16, 2017
Done 😉 |
fabpot commentedJan 16, 2017
I've not checked, but don't we have the same issue in 2.7 and 2.8? |
HeahDude commentedJan 16, 2017
fabpot commentedJan 16, 2017
Thank you@dmaicher. |
…object primary keys (dmaicher, HeahDude)This PR was merged into the 3.1 branch.Discussion----------[Doctrine Bridge] fix UniqueEntityValidator for composite object primary keys| Q | A| ------------- | ---| Branch? | master / 3.1| Bug fix? | yes| New feature? |no| BC breaks? | no| Deprecations? |no| Tests pass? | yes (fail on php 7.1 unrelated?)| Fixed tickets |#21274| License | MIT| Doc PR | -This PR fixes an issue with the UniqueEntityValidator in case the entity being validated uses a composite primary key via relations to other entities whose classes do not have a `__toString` method.Commits-------b3ced86 [DoctrineBridge] Fixed invalid unique value as composite key5aadce3 [Doctrine Bridge] fix UniqueEntityValidator for composite object primary keys
Uh oh!
There was an error while loading.Please reload this page.
This PR fixes an issue with the UniqueEntityValidator in case the entity being validated uses a composite primary key via relations to other entities whose classes do not have a
__toStringmethod.