BUGFIX: Do not join select property paths to embedded objects #1404
Conversation
Instead of assuming that every property path with a dot is a path to an other entity check if the property path is a mapped field which is also true for embedded object properties. Resolves neos#989
|
Woah, that looks too nice to be true :D Thanks for digging into this! A few things:
|
|
When i wrote the fix i had the following (and misquoted btw 😅 ) saying in mind:
But that one seems to to do the job. Well, we should test that extensively before merging, but i can't invest time in test at this point so i'm counting on you guys 😉
The classMetaData is already fully built (and hold available throughout the request) and the hasField is just an
i haven't checked that yet but as it seems any path (e.g. if you embedd an embeddable in an embeddable in your object) is existent in the mappedFields array.
Yes, tests are required and especially tests that check that no other unwanted side effects will pop up. Your help is very appreciated here |
|
Sorry, I totally lost track of this one. Currently a bit too loaded to get into it unfortunately :( Would like to hear @kdambekalns - should we maybe merge this even without a test in place? |
|
TBH I'd rather have a test… |
|
I'm with @kdambekalns - this is too critical not to be tested (but unfortunately I currently lack the resources and insight to add those, sorry) |
|
Assigning to myself for now to hopefully find a bit of time to add a test and get this in sometime soon. |
How else should it return both elements...
|
Finally found the inspiration and will to do it. I'm so sorry it took so absurdly long for this pretty trivial test... Edit: and it's so late, that I make the stupidest mistakes in this trivial test, like having a limit(1) and expecting 2 results... 🤦♂ |
|
Green 🎉 |
|
/rebase Hm, apparently the github action only works if the branch targeted already contains the action workflow I guess :/ |
Instead of assuming that every property path with a dot is a path
to an other entity check if the property path is a mapped field which
is also true for embedded object properties.
Resolves #989
What I did
We'll i suppose i fixed it 😅
How I did it
I searched the existing class schema for hints about embedded properties and found it in the entityManager. When the path exists in the fieldMappings it is a field embedded in the object's table. Since we're using doctrine in this kind of query anyway i think we're safe to go whit this solution.
How to verify it
The description of the original bug should be suffice.
Checklist