-
-
Notifications
You must be signed in to change notification settings - Fork 927
feat(elasticsearch): filtering on nested fields #5820
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
Conversation
80af82e
to
da31df4
Compare
Thanks @jonnyeom |
I think that this actually breaks elasticsearch test suite my bad. |
@@ -69,7 +69,7 @@ private function getNestedFieldPath(string $resourceClass, string $property): ?s | |||
) { | |||
$nestedPath = $this->getNestedFieldPath($nextResourceClass, implode('.', $properties)); | |||
|
|||
return null === $nestedPath ? $nestedPath : "$currentProperty.$nestedPath"; | |||
return null === $nestedPath ? $currentProperty : "$currentProperty.$nestedPath"; |
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.
@jonnyeom I have to revert this to make https://github.com/api-platform/core/blob/main/features/elasticsearch/match_filter.feature#L120 green any idea? more at #5823
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.
interesting.
Let me take a deeper look again.
Thanks for heads up!
This is a resurrection of #5643
This PR implements the following changes to support filtering on paths like foo.bar.baz:
foo
is an object,isNestedField()
now returnstrue
instead offalse
foo
is an object,getNestedFieldPath()
now returns"foo.bar"
instead ofnull
foo
is a collection,getNestedFieldPath()
now returns"foo.bar"
instead of just"foo"
(matching the behavior iffoo
is an object)As a result, filtering on deeply-nested paths (with 3 or more levels) now produces the following query: