Repository navigation
Fields: Update author field's query - #75373
Closed
ntsekouras wants to merge 1 commit into
Closed
ntsekouras wants to merge 1 commit into
ntsekouras wants to merge 1 commit into
Conversation
ntsekouras
added
[Type] Bug
|
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: +83 B (0%) Total Size: 3 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 9ece0f6. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/21860099083
|
Mamaduka
reviewed
Feb 10, 2026
Mamaduka
left a comment
Mamaduka
left a comment
Member
There was a problem hiding this comment.
Thanks, Nik!
What would be the best way to test this for low-cap users?
Comment on lines
+35
to
+38
| resolveSelect( coreDataStore ).getEntityRecords( 'root', 'user', { | ||
| ...query, | ||
| has_published_posts: true, | ||
| } ), |
Member
There was a problem hiding this comment.
I think this query will fail for low-cap users. who: 'author' has special treatment in the controller. See: https://github.com/WordPress/wordpress-develop/blob/9d9a0f11106d80507e4d4c95864ee507b5ba3083/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php#L236-L251
Contributor
Author
There was a problem hiding this comment.
Hm.. How could that affect us though? A low-cap user wouldn't have access to places where the authorField is used, would they?
Can you share an example of what you have in mind? I logged in with author, editor, subscriber roles and they don't have access to site editor. Post editor worked fine, although it doesn't use that field, but if it did contributors cannot update that field.
Member
There was a problem hiding this comment.
Yeah, I was also just double-checking. I remember hitting this problem a couple of months ago, but can't reproduce it at the moment.
Hm.. How could that affect us though? A low-cap user wouldn't have access to places where the authorField is used, would they?
Maybe at the moment, but we don't know how or by whom these fields will be used in the future. These are meant to be reusable by third parties, correct?
Contributor
Author
There was a problem hiding this comment.
Maybe at the moment, but we don't know how or by whom these fields will be used in the future. These are meant to be reusable by third parties, correct?
Of course. Usually though low-cap users shouldn't be able to view the posts of others etc.. Not sure what can we do in that case.
Member
There was a problem hiding this comment.
Go to site editor in pages and observe that the user is still an option for filtering and editing.
Thanks for getting this PR up - I'm looking at how to steal adapt it to update the media author query as well.
has_published_posts can be an array of post types.
This will come in handy for attachments, but I was wondering if it would be helpful to narrow the query, for example, if we're looking at a data views table for pages only.
I don't know, I'll trust your judgement 😄
Setting true all public post types seems like a decent generic catch all anyway.
Contributor
There was a problem hiding this comment.
Thanks for working on this!
The tricky part about this problem is that for it to work ideally, we need to deal with two edge cases:
has_published_postsonly checks whether there are published posts, so it cannot strictly check whether the user is the author of the post. A user may have unpublished posts, such as drafts.has_published_posts=truechecks all post types, so if the user has published posts of a different post type than the current post type context, the current logic will cause unintended behavior.
At the moment, I'm not sure how to deal with these two issues. Perhaps in the end, we may have no choice but to take one of two approaches: either keep the who: 'authors' parameter or remove it 😅
Mamaduka
reviewed
Feb 10, 2026
Comment on lines
+25
to
+29
| const query = { | ||
| per_page: -1, | ||
| _fields: 'id,name', | ||
| context: 'view', | ||
| }; |
Member
There was a problem hiding this comment.
I'm worried about doing two unbound queries (-1). Depending on the installation, it can be problematic. We even try to avoid it with post editors.
Contributor
Author
There was a problem hiding this comment.
Not sure how we can avoid this.. Should we consider a core update to REST controller? Maybe something like extending the who param to accept something like authors_and_has_published_posts that would create an OR relationship?
Member
There was a problem hiding this comment.
Interesting idea, so basically a combo query with who and has_published_posts. This might require changes to WP_User_Query, I don't think it's only a REST controller limitation.
Meanwhile, do you think we can use a combobox for similar filters and do a limited per_page query initially?
Contributor
Author
There was a problem hiding this comment.
This might require changes to WP_User_Query
It will since the controller creates WP_User_Query.
Meanwhile, do you think we can use a combobox for similar filters and do a limited per_page query initially?
What do you mean? Async filtering has more nuances and is being explored here. Though even if we had that, we'd still need two queries and it would be even more complex how to combine two queries.
Member
There was a problem hiding this comment.
Though even if we had that, we'd still need two queries and it would be even more complex how to combine two queries.
I understand. Though two per_page: 20|100 queries are better than two unbound queries, for installations with a large user base.
We can get back to this when #73623 is ready.
Contributor
Author
|
Closing for now since it's been while with no activity and no reports have been made about the edge case that could create the issue, this PR tries to solve. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
What?
Follow up of: #75298
@t-hamano mentioned a scenario, where a user had published some posts, but then their role gets updated to a role that can't publish posts.
Besides that, there is another issue which involves the differentiation of the field's elements based on context (filter/edit).
filtersit seems the query in this PR is correct.editwe would need to have a query withwho: authors, and include only the current author - even if it falls into the above Aki's scenario. In this PR we still show all previous authors, so we can end up updating a page with an author, that cannot be an author anymore.Testing Instructions
authoruser and create a pagesubscriber