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
Make sure the order of nested order filters is preserved #4171
Conversation
15c9e9b
to
6dcaf8b
Compare
Order filters are being written with underscores in GraphQL but are sent along with dots to access nested entities. The previous solution appended the dot version to the order array. This leads to an issue where every order filter that accesses a nested property will always be added to the query after local properties.
6dcaf8b
to
2fa12b6
Compare
$filters = | ||
\array_slice($filters, 0, $index + 1) + | ||
[str_replace($this->nestingSeparator, '.', $name) => $value] + | ||
\array_slice($filters, $index); |
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 starting at index?
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.
It does not make a difference in the output as the duplicated key gets merged so I did not notice that this code adds it twice. Will add +1
there as well.
CHANGELOG.md
Outdated
@@ -13,6 +13,7 @@ | |||
* Swagger UI: Remove Google fonts (#4112) | |||
* Doctrine: Revert #3774 support for binary UUID in search filter (#4134) | |||
* Do not override Vary headers already set in the Response | |||
* GraphQL: Make sure the order of order filters is preserved if nested resources are used |
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.
* GraphQL: Make sure the order of order filters is preserved if nested resources are used | |
* GraphQL: Make sure the order of order filters is preserved if nested resources are used (#4171) |
/** | ||
* Test a regression where the order in which order fitlers were applied | ||
* was wrong when the data had been ordered by a nested field. | ||
*/ |
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.
I think this comment is unnecessary.
Hello! A tricky one 🙂 Thanks for spotting it and fixing it. |
95d17eb
to
0994e5a
Compare
It's probably not a super common use case to sort on a child entity first... |
Co-authored-by: Alan Poulain <contact@alanpoulain.eu>
Thank you @jnugh. |
Order filters are being written with underscores in GraphQL but are sent along with dots to access nested entities.
The previous solution appended the dot version to the order array. This leads to an issue where every order filter that accesses a nested property will always be added to the query after local properties.
Looking at the test case: The first order that WAS applied was
localField
. It should have beensome.field
which is now the case.