-
Notifications
You must be signed in to change notification settings - Fork 313
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
Fix WP_Query_User when it tries to sort the results #2226
Conversation
tests/php/indexables/TestUser.php
Outdated
/** | ||
* Check if Zoey is the first user because Elasticsearch sort | ||
* is case-sensitive | ||
* | ||
* Ref: https://www.elastic.co/guide/en/elasticsearch/guide/2.x/sorting-collations.html#case-insensitive-sorting | ||
* | ||
*/ | ||
$this->assertEquals( 'Zoey', $users_display_name_fetched[0] ); |
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.
@Rahmon it seems to be counterintuitive to rely on a case-sensitive sorting while WP's is case insensitive. Also, for fields in the posts indexable we have a "sortable" field that we could replicate here. Do you mind taking care of that?
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.
Sure. Thanks for that @felipeelia
@Rahmon did you test it with Elasticsearch 5.6.16 (the version shipped with WP Local Docker)? I'm using the following snippet in theme's header.php:
As is, I receive a 400 error saying:
If I change
Can you please check it? Thanks |
@felipeelia I've updated the mapping and updated some tests to check if the query uses the Elasticsearch to retrieve the users. |
Description of the Change
This changes updates to use keyword fields to sort in the WP_User_Query and adds tests to ensure we are using these fields.
Alternate Designs
Benefits
Elasticsearch will not fail and fall back to MySQL.
Possible Drawbacks
Verification Process
@
and try to search a user e.g.@admin
Checklist:
Applicable Issues
Closes: #2225
Changelog Entry
Fixed sort for WP_User_Query