Repository navigation
Abilities API: Add core/users-query ability - #10775
jorgefilipecosta wants to merge 56 commits into
Conversation
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
|
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 Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
(Sorry I missed this one. Wish there was a way we could actually tag other #core-ai folks for review. Will take it for a spin hopefully tomorrow or wednesday) |
0756a9a to
37e01db
Compare
|
This PR was rebases and the conflicts were fixed. cc: @justlevine |
|
My biggest concern with this PR is that it's not automatically scalable or extendable. It's not obvious (to me or an LLM) why these are the particular set of user data we're returning, we're not exposing user meta that's registered with |
Hi @justlevine the plan is to expose user meta. The only reason we are not doing it yet is because I learned the lesson on #10867 😅 and PR's rapidly become very huge and hard to review if we include the full features from the beginning. So the plan is to merge a simple user ability first, and then follow up right away with the meta expansion. |
100% agree. The nuance here is that "simple" means something a bit different for abilities than traditional progressive programming, since it's the schema that needs to scale, and we have a lot more leeway with the internals. My focus isn't why not metadata, but why yes to e.g. Like @/aaronjorbin notes in https://core.trac.wordpress.org/ticket/64596#comment:9 , once things are in core back and forward compat take on a mind on its own, so we need to make sure we can iterate the schema in the holistic direction we want, like how we intentionally prepped for the possibility of nested namespaces |
29c36cc to
4312b3f
Compare
|
This question came up tangentially elsewhere - I couldn't find a trac ticket for this ability , so sharing it here for completeness: what are we doing with the existing
|
9963eaa to
a8218c3
Compare
a8218c3 to
e846a9c
Compare
Start the label with a verb, like the other core abilities, such as "Get Site Information" and "Get User Information".
…query`. Without `per_page`, an `include` request used the default page size of 10, so a caller loading a known set of users silently received only the first 10. Page such a request to the number of included IDs instead, and cap `include` at the maximum page size so the IDs always fit on one page. An explicit `per_page` still wins. This matches `core/content-query`.
The note said that leaving the include order out of `orderby` lets WP_User_Query share cached results with other queries. It does not: WP_User_Query keys its cache on the SQL, and `ID IN (...)` lists the IDs in the order given, so lists in a different order never share an entry either way. Say what the code does instead: like the REST users controller, the include list filters the results without ordering them. Also say so in the `include` description, and fix the test docblock that repeated the note.
…s-query`. Together, rest_is_boolean() and rest_sanitize_boolean() match exactly the forms of `true` that the hand-written check matched: `true`, `1`, and the strings `'1'` and `'true'` in any case. Schema validation reads booleans with the same helpers.
…ry`. Parse the value with wp_parse_list() and keep only its strings, like the list parser of `core/content-query`. The loop and array_unique() that dropped empty and duplicate items are not needed, because schema validation has already rejected them.
Treat `fields: []` like an omitted `fields` argument and return the default fields, as `normalize_fields()` already did, instead of rejecting the list in the input schema. `core/content-query` accepts an empty list too, so clients can use one convention for both abilities. Also say in the `fields` description that `id` is always included, and that fields the current user cannot view are omitted rather than causing an error.
A single-user lookup that the execute callback could not resolve returned `ability_invalid_permissions` without an HTTP status. Return `users_not_found` with a 404 instead, like the `content_not_found` error of `core/content-query`. Gated transports still never reach it, because the permission callback denies the same lookups first. A user the current user cannot read is reported like a missing one. Report a page past the last one as `users_invalid_page_number` with a 400, like `core/content-query` and the REST posts controller, instead of returning an empty list. An empty result set still returns zero totals on any page.
On multisite, a super admin can publish posts on a site without being a member of it. The posts show them as an author on the front end, and `core/content-query` returns their `author_slug`, but a lookup of that slug was refused, because every lookup of another user required membership of the site. Only require membership for the capability checks, which should reveal users of the site, not of the whole network. Lookups by ID or slug now find public authors who are not members, with the fields of a public author. Lookups by email or username still only find users of the site.
`core/content-query` lets a user who can edit others' posts resolve any user of the site as an `author_slug`, since they can make any of them the author of a post. A lookup of that slug here was refused when the user had no public posts, such as an author whose only post is pending review. Let a caller who can edit others' posts of a post type that supports authors look up any user of the site by ID or slug, and return only the fields that identify the user: `id`, `name`, `link`, `slug`, and `avatar_urls`. The profile fields, `description` and `url`, and the sensitive fields are left out, and their descriptions now say they are present when the current user can view them. Lookups by email or username still require permission to list or edit users, and collections still only list public authors to callers who cannot list users. The permission and execute callbacks share the decision through `get_readable_fields()`, which replaces `can_read_user_for_lookup()`.
- Report a page past the last one as not found (404) instead of as a caller error. - Fail closed on collection filters that cannot be honored instead of dropping them, which silently widened the query: an `include` list with no valid ID, an empty `roles` list, and a `has_published_posts` value that is neither true nor a list of post types are rejected, and a role filter is refused to a caller who cannot list users. - Read `id`, `page`, and `per_page` with the content query's integer parser, so a fraction or a value beyond 2 ** 53 cannot be cast onto another user, and paging that cannot be parsed falls back to the defaults. - Default the input schema to an empty array, like the other core abilities, and say in the description that the ability requires an authenticated user. - Tests: set up the fixtures in wpSetUpBeforeClass() and clean up in wpTearDownAfterClass(), and cover the rejected filters and the pagination fallback.
…-info`. `core/get-user-info` only returns the current user's own profile, while `core/users-query` reads any user the current user is allowed to see. Note this on `register_users_query()`, so the two abilities are not mistaken for duplicates.
Only `register()` stays public. The permission and execute callbacks are now private methods, registered through closures defined in the class, so callers run the ability through the Abilities API, such as `wp_get_ability( 'core/users-query' )->execute()`, which validates the input and checks permissions first. This replaces the class docblock's note that the class is not part of the public API. Public methods are covered by Core's backward compatibility commitment whatever the docblock says. Tests that call the callbacks directly now capture them from the registration arguments, through the `wp_register_ability_args` filter.
…nt query - Map the filter to the message under `params` in the `users_invalid_filter` error data, as the REST API's `rest_invalid_param` errors do, so callers can tell which filter failed without parsing the translated message. - Mark `id`, which is always returned, as required in the user output schema.
The `WP_Abilities_Users` and `WP_Abilities_Settings` docblocks said the other core abilities are self-contained closures registered directly in `wp_register_core_abilities()`. With both classes in Core, that is no longer true for either, so both now contrast the class with the core abilities that are registered there as closures.
…uery`. Use `empty()` instead of comparing with `array()`. Every value checked is always an array, so the behavior does not change.
Use `assertSameSets()` for the checks of which fields, wrapper keys, required properties and avatar sizes are returned, since their order does not matter. The include order checks keep `assertSame()`, because the order is what they test.
…ted fields. The self lookup by ID requests some of the sensitive fields, so also check that exactly those fields are returned.
…` collection test. Run the check that a collection only lists public authors for callers who cannot list users for editors, authors, contributors and subscribers, through a data provider. Also check that the current user, who has no published posts, is not listed.
…y` tests. `wp_update_user()` already clears the user cache, through `wp_insert_user()`, so the tests that store an empty or invalid email no longer clear it again.
…gination`. Drop the top-level `oneOf` from the input and output schemas, since some AI providers, such as Anthropic, reject a tool whose input schema has a combinator at the top level. The input now takes `where`, `fields` and `pagination`. `where` holds either exactly one lookup key (`id`, `email`, `username` or `slug`) or any of the collection filters (`include`, `roles` and `has_published_posts`), as an `anyOf` of two branches without titles. `pagination` holds `page` and `per_page`. Every result now has the same shape: the users in `items`, and the totals in `pagination`. A lookup is a page of one user, so a later page is an error, as it is for a collection. The invalid filter errors name the filter by its input path, such as `where[include]`, and a `where` that is not an object is rejected instead of being read as no filter.
…ation` input. Pass the filters in `where` and the page values in `pagination`, and read the users from `items` and the totals from `pagination`. The schema test now checks the whole input and output schemas, including that neither has a combinator at its root. Also cover a lookup past the first page, two lookup keys in `where`, filters and page values at the root, `where` and `pagination` objects, and a `where` that is not an object.
… lists. Drop the `anyOf` in `where`, along with the single-user lookups and their access rules. `where` is now one object whose filters combine with AND: `ids` replaces `id` and `include`, `slugs` and `usernames` take lists, and `email` is removed. Every request is now one user query with the same rules: callers who can list users see the users of the site, and other callers see public authors. A user the caller can't see is left out of `items` instead of being refused, and callers read their own profile through `core/get-user-info`. Filtering by username needs permission to list users, like filtering by role. On calls that skip schema validation, an ID that isn't a whole number of at least 1 is dropped instead of truncated, and a list with no valid value left is rejected as an invalid filter.
Filter with `ids`, `slugs` and `usernames` in `where`, and check the single set of access rules: callers who can't list users only see public authors, even when they ask for their own account, and users they can't see are left out of the results. Also cover the lists returning several users ordered by login, filters combining with AND, the removed `where` keys failing validation, the username filter needing permission to list users, and how calls that skip validation read each list.
0f7d4b3 to
232479b
Compare
gziolo
left a comment
There was a problem hiding this comment.
Approving based on my testing of #92. The implementation and tests are unchanged after merging. Thanks for confirming the behavior changes, and to @JasonTheAdams for approving the updated design.
My earlier provider results still apply: all eight cases passed with Anthropic and OpenAI. Gemini required parametersJsonSchema. Enabling strict mode required extra changes for Anthropic and OpenAI, supporting the need for provider-specific schema adjustments.
I also tested @dugyen’s observations on the current revision:
- Missing users now return an empty collection with HTTP 200, following the new query behavior.
- Invalid fields now return HTTP 400 with a clear message identifying the field.
Adds a read-only `core/users-query` ability that returns a page of users, optionally filtered by IDs, slugs, usernames, roles, or published-post authorship. Until now, the only user ability was `core/get-user-info`, which returns the current user's own profile. The code lives in the new private `WP_Abilities_Users` class. The input has three members: `where` selects the users, `fields` the fields returned for each one, and `pagination` the page. The output always holds the users in `items`, ordered by username, and the totals in `pagination`. Developed in WordPress/ai#774, and #10775. Props jorgefilipecosta, gziolo, justlevine, peterwilsoncc, dkotter, jason_the_adams, ugyensupport, iamadisingh, sangu3105. Fixes #64657. git-svn-id: https://develop.svn.wordpress.org/trunk@64346 602fd350-edb4-49c9-b593-d223f7449a82
Adds a read-only `core/users-query` ability that returns a page of users, optionally filtered by IDs, slugs, usernames, roles, or published-post authorship. Until now, the only user ability was `core/get-user-info`, which returns the current user's own profile. The code lives in the new private `WP_Abilities_Users` class. The input has three members: `where` selects the users, `fields` the fields returned for each one, and `pagination` the page. The output always holds the users in `items`, ordered by username, and the totals in `pagination`. Developed in WordPress/ai#774, and WordPress/wordpress-develop#10775. Props jorgefilipecosta, gziolo, justlevine, peterwilsoncc, dkotter, jason_the_adams, ugyensupport, iamadisingh, sangu3105. Fixes #64657. Built from https://develop.svn.wordpress.org/trunk@64346 git-svn-id: http://core.svn.wordpress.org/trunk@63496 1a063a9b-81f0-0310-95a4-ce76da25c4cd
What?
core/users-queryability registered through Core ability registration.core/users-queryinput intowhereandpaginationjorgefilipecosta/wordpress-develop#92 then reshaped the input and output and removed the top-leveloneOf, which some AI providers, such as Anthropic, reject.where,fields, andpagination.wherefilters byids,slugs,usernames,roles, andhas_published_posts. Every given filter must match, and a list matches users with any of its values.{ "items": [ ... ], "pagination": { "total": 1, "total_pages": 1 } }, ordered by username. Values that match no readable user are left out rather than causing an error, and a page past the last one returnsusers_invalid_page_number.usernamesorroles.core/get-user-inforeturns the caller's own profile.id,name,description,url,link,slug,avatar_urls,username,email,first_name,last_name,nickname,locale,registered_date, androles.id, and omits fields that the caller cannot view. Sensitive fields are only returned for the caller's own account or users they can edit.{ "where": { "has_published_posts": true, "roles": [ "author" ] }, "fields": [ "id", "name", "email" ], "pagination": { "page": 1, "per_page": 10 } }Testing
Verify unit tests are passing:
The reshape from jorgefilipecosta#92 was drafted with AI assistance (Claude Code) and reviewed by me.
Trac ticket: https://core.trac.wordpress.org/ticket/64657