-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow to search by related entities fields #2045
Conversation
While we can make add columns of an related entity in the "list" actions, there is no way to search by them. ```yml easy_admin: entities: Campaign: list: fields: - property: name - property: domain.name // <--- ``` ```yml easy_admin: entities: Campaign: search: fields: - domain.name // <--- throwing error ``` These changes are making that it will now be possible. What do you think about it?
Fix multiple join
if (false !== strpos($fieldName, '.')) { | ||
list($associatedEntityName, $associatedFieldName) = explode('.', $fieldName); | ||
if (!in_array($associatedEntityName, $entitiesAlreadyJoined)) { | ||
$queryBuilder->leftJoin('entity.'.$associatedEntityName, $associatedEntityName); |
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.
Why leftJoin
? If you search on a related entity field, the related entity has to be not null to match the search. A join
sound totally appropriate as it will get only the lines where the relation is available and avoid searching against null values in the where clause.
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.
IMO leftJoin
is fine, because rows not related to the joined table can match the search.
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.
leftJoin
allows to search on NULL values in the future
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.
@yceruto Indeed, I didn't think about this use case.
@ktrzos @maximecolin this feature is finally merged! Thanks a lot for your work and reviews. |
…imecolin, javiereguiluz) This PR was merged into the 1.17.x-dev branch. Discussion ---------- Allow to search by related entities fields This finishes #1991. Commits ------- 7ed99c6 Added docs for the new feature 851fce9 Added tests for the new feature f136e6f Fix tests 6248c60 Fixed tests 6f201dd Fixes db3b4b4 Code syntax fixes and tweaks b60b986 Merge pull request #1 from maximecolin/patch-3 e73afa4 Fix multiple join 984018f Searching by related entities fields
works well, great move ! |
This finishes #1991.