Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[DoctrineBridge] indexBy could reference to association columns#38628
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
| $associationMapping =$subMetadata->getAssociationMapping($fieldName); | ||
| /** @var ClassMetadataInfo $subMetadata */ | ||
| $indexProperty =$subMetadata->getSingleAssociationReferencedJoinColumnName($fieldName); |
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.
Shouldn't we add some kind of check that the column name used here is in fact the join column?
nicolas-grekas commentedOct 28, 2020
Please check#38861 also. |
juanmiguelbesada commentedOct 28, 2020
I will take a look |
jderusse left a comment
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.
People at Doctrine says it could/shoud be a property (doctrine/orm#8018 (comment))
Which leads to issue in Symfony:#38861
Given this is not clear, I suggest to supports both cases:
- IndexBy point to a property
- IndexBy point to a column_name
bwach commentedOct 30, 2020 • 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.
This is also reflected in description for ClassMetadataInfo which we use here (depending on how you understand the whole paragraph) describing "indexBy" as:
So it should be one of the fields on the entity, not a column. |
xabbuh commentedNov 2, 2020
How does Doctrine behave for one or the other? Does it ignore the attribute? Does it try to map the column to a property or vice versa? |
6b58fa4 toa0b6778Comparejuanmiguelbesada commentedNov 5, 2020
PR updated fixing#38861 (I hope) and adding tests |
xabbuh left a comment
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.
LGTM
nicolas-grekas commentedNov 9, 2020
Thank you@juanmiguelbesada. |
This is my approach to solve#37982. It partials reverts@xabbuh PR#38604
This is my first Symfony contribution, so please, tell me if I need to do something more or something is wrong.
Also, this bug affects 4.x and 5.x versions. I think merging in this branches is done automatically. If not, please tell me.
Thanks you