REST API: Support search_columns argument in the user endpoint #67330
+68
−1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
youknowriad
added
[Type] Bug
youknowriad
commented
Nov 26, 2024
| @@ -5,6 +5,6 @@ export const BASE_QUERY = { | |||
|
|
|||
| export const AUTHORS_QUERY = { | |||
| who: 'authors', | |||
| per_page: 50, | |||
| per_page: 100, | |||
There was a problem hiding this comment.
I don't see any reason to limit this to 50, I just put 100 which I believe is the max.
There was a problem hiding this comment.
I don't remember the exact reason; it was probably just an arbitrary number we chose. I'll have a better look tomorrow.
There was a problem hiding this comment.
Confirmed. The 50 was just an arbitrary number. It might affect initial loading for the post editor on sites with a large user sets, but it's hard to measure locally.
There was a problem hiding this comment.
Happy to revert that if you think it's better for now, it's kind of outside the scope of the current PR.
There was a problem hiding this comment.
We can keep it. The query only fetches the id,name fields; the response size difference between 50 and 100 should be minimal.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +11 B (0%) Total Size: 1.83 MB
ℹ️ View Unchanged
|
|
A couple of surface notes:
|
Mamaduka
requested review from
TimothyBJacobs
and removed request for
Mamaduka
I think this has no impact at all since it's the "search_columns" for WP_Query is already used in the search endpoint. It's just using the default value when the user has the "list_users" capability, otherwise it's using
My guess is that it was just not requested. Even the posts endpoint one is a recent addition https://core.trac.wordpress.org/ticket/43867 |
Mamaduka
reviewed
Nov 27, 2024
Comment on lines
+40
to
+46
| $search_columns_mapping = array( | ||
| 'id' => 'ID', | ||
| 'username' => 'user_login', | ||
| 'slug' => 'user_nicename', | ||
| 'email' => 'user_email', | ||
| 'name' => 'display_name', | ||
| ); |
There was a problem hiding this comment.
Question (non-blocking): The post controllers don't map these values; should we follow the same example?
There was a problem hiding this comment.
Personally, I think it's a mistake that they're not mapped in the posts controller. I think the arguments should match the "fields" that are present in the schema and also the _fields argument.
There was a problem hiding this comment.
I think the arguments should match the "fields" that are present in the schema and also the _fields argument.
Agreed. Just double checking for the record :)
lib/compat/wordpress-6.8/class-gutenberg-rest-user-controller.php
Outdated
Show resolved
Hide resolved
|
Flaky tests detected in 05433a6. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12048455541
|
|
I've updated the testing instructions. Props again to @claudiulodro for originally reporting the bug. |
|
While I get the use case described in the PR, someone could also search by email. This is subjective, but to be honest personally I usually go by emails at similar cases, as they are unique. Having said that, I think the By the way, you can test way easier by adjusting this value here. I just set it to 1 and had to create a single user. |
For me it doesn't make sense to search by email unless you actually show the emails in the results which we don't. |
ntsekouras
reviewed
Nov 27, 2024
lib/compat/wordpress-6.8/class-gutenberg-rest-user-controller.php
Outdated
Show resolved
Hide resolved
ntsekouras
reviewed
Nov 27, 2024
ntsekouras
approved these changes
Nov 27, 2024
Fair enough. Ok. |
|
Re-updated search columns: The |
Mamaduka
approved these changes
Nov 28, 2024
Co-authored-by: George Mamadashvili <[email protected]>
youknowriad
force-pushed
the
add/support-search-columns-argument
branch
from
05433a6 to
289a0d0
Compare
Open
1 task
im3dabasia
pushed a commit
to im3dabasia/gutenberg
that referenced
this pull request
Dec 4, 2024
…ress#67330) Co-authored-by: youknowriad <[email protected]> Co-authored-by: Mamaduka <[email protected]> Co-authored-by: ntsekouras <[email protected]> Co-authored-by: mreishus <[email protected]> Co-authored-by claudiulodro < [email protected]>
michalczaplinski
pushed a commit
that referenced
this pull request
Dec 5, 2024
Co-authored-by: youknowriad <[email protected]> Co-authored-by: Mamaduka <[email protected]> Co-authored-by: ntsekouras <[email protected]> Co-authored-by: mreishus <[email protected]> Co-authored-by claudiulodro < [email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #67331
What?
There's a weird bug in the "post author" selector in the editor. Say you have a site with thousands of users, and imagine these users all have emails that end with
@automattic.com:). This means that if you search for an author named "Matt" in the post author selector, you'll never be able to assign the post to that person. The reason is the following:What a funny 🐛 right :)
This PR solves this by adding support for a
search_columnsargument to the user endpoint. A similar argument is already supported for the posts endpoints.Testing Instructions
Based on #38791, props to @claudiulodro!
example.comemail address.User Example.exampleauthor.A snippet for user generation via WP CLI
Screenshot