Skip to content
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.

Sign up for GitHub

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

REST API: Support search_columns argument in the user endpoint #67330

Merged
merged 4 commits into from
Nov 28, 2024
Merged

REST API: Support search_columns argument in the user endpoint #67330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b7fb7cd
REST API: Support search_columns argument in the user endpoint
youknowriad Nov 26, 2024
a839a16
Remove useless check
youknowriad Nov 28, 2024
289a0d0
Apply review suggestion.
youknowriad Nov 27, 2024
cbf220b
Add backport file
youknowriad Nov 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions backport-changelog/6.8/7909.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
https://github.com/WordPress/wordpress-develop/pull/7909

* https://github.com/WordPress/gutenberg/pull/67330
62 changes: 62 additions & 0 deletions lib/compat/wordpress-6.8/class-gutenberg-rest-user-controller.php
View file
Open in desktop
Mamaduka marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
<?php

/**
* Add search_columns parameter to users endpoint parameters
*
* @param array $query_params JSON Schema-formatted collection parameters.
* @return array Updated collection parameters
*/
function gutenberg_add_search_columns_param( $query_params ) {
$query_params['search_columns'] = array(
'default' => array(),
'description' => __( 'Array of column names to be searched.' ),
'type' => 'array',
'items' => array(
'enum' => array( 'email', 'name', 'id', 'username', 'slug' ),
youknowriad marked this conversation as resolved.
Show resolved Hide resolved
'type' => 'string',
),
);

return $query_params;
}

add_filter( 'rest_user_collection_params', 'gutenberg_add_search_columns_param', 10, 1 );

/**
* Modify user query based on search_columns parameter
*
* @param array $prepared_args Array of arguments for WP_User_Query.
* @param WP_REST_Request $request The REST API request.
* @return array Modified arguments
*/
function gutenberg_modify_user_query_args( $prepared_args, $request ) {
if ( $request->get_param( 'search' ) && $request->get_param( 'search_columns' ) ) {
$search_columns = $request->get_param( 'search_columns' );

// Validate search columns
$valid_columns = isset( $prepared_args['search_columns'] )
? $prepared_args['search_columns']
: array( 'ID', 'user_login', 'user_nicename', 'user_email', 'user_url', 'display_name' );
$search_columns_mapping = array(
'id' => 'ID',
'username' => 'user_login',
'slug' => 'user_nicename',
'email' => 'user_email',
'name' => 'display_name',
);
Comment on lines +40 to +46
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question (non-blocking): The post controllers don't map these values; should we follow the same example?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

$search_columns = array_map(
static function ( $column ) use ( $search_columns_mapping ) {
return $search_columns_mapping[ $column ];
},
$search_columns
);
$search_columns = array_intersect( $search_columns, $valid_columns );

if ( ! empty( $search_columns ) ) {
$prepared_args['search_columns'] = $search_columns;
}
}

return $prepared_args;
}
add_filter( 'rest_user_query', 'gutenberg_modify_user_query_args', 10, 2 );
1 change: 1 addition & 0 deletions lib/load.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ function gutenberg_is_experiment_enabled( $name ) {
require __DIR__ . '/compat/wordpress-6.8/functions.php';
require __DIR__ . '/compat/wordpress-6.8/post.php';
require __DIR__ . '/compat/wordpress-6.8/site-editor.php';
require __DIR__ . '/compat/wordpress-6.8/class-gutenberg-rest-user-controller.php';

// Experimental features.
require __DIR__ . '/experimental/block-editor-settings-mobile.php';
Expand Down
2 changes: 1 addition & 1 deletion packages/editor/src/components/post-author/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ export const BASE_QUERY = {

export const AUTHORS_QUERY = {
who: 'authors',
per_page: 50,
per_page: 100,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any reason to limit this to 50, I just put 100 which I believe is the max.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember the exact reason; it was probably just an arbitrary number we chose. I'll have a better look tomorrow.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to revert that if you think it's better for now, it's kind of outside the scope of the current PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can keep it. The query only fetches the id,name fields; the response size difference between 50 and 100 should be minimal.

...BASE_QUERY,
};
1 change: 1 addition & 0 deletions packages/editor/src/components/post-author/hook.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export function useAuthorsQuery( search ) {

if ( search ) {
query.search = search;
query.search_columns = [ 'name' ];
}

return {
Expand Down
Loading