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.

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

Terms: Respect order specified by register_taxonomy() #67154

Merged
merged 5 commits into from
Nov 25, 2024

Conversation

ockham
Copy link
Contributor

@ockham ockham commented Nov 20, 2024

Fixes #65052.
Supersedes #64471.

What?

It is possible to supply a default set of args to register_taxonomy() which will be used when querying a list of terms -- for example, orderby in order to specify how the resulting list of terms should be sorted. This PR makes it so that the list of terms returned by the Terms REST API controller respects that order.

Why?

To reflect the order in which "flat terms" (such as tags) are supposed to be sorted. For example, see the actors taxonomy in the code sample (and screencasts) below: It uses array( 'orderby' => 'term_order' ) to make sure that terms -- i.e. actor names -- are sorted in the order that they are stored (rather than in alphabetical order).

How?

By adding a filter to the rest_{$this->taxonomy}_query hook for all registered taxonomies, which then merges any args registered for a given taxonomy into the arguments given by the network request.

Note that this mimics behavior already present in wp_get_object_terms(). In the Terms Controller, the latter function is used for requests that include a post ID. However, we cannot provide a post ID for reasons first explained here:

The problem is that when a new term is entered by the user, it is created (via a POST request), but not yet associated with the current post. This is by design: We only want that to happen once the user clicks the 'Save' button in order to persist their changes. (Otherwise, the new term would show up on that post, even if the user ends up never saving the post.)

Instead, our network requests go through a code branch in the controller that uses get_terms() (rather than wp_get_object_terms()) to query terms. To make that function respect the default args registered for a given taxonomy, we use the rest_{$this->taxonomy}_query hook.

Testing Instructions

Add the following code (e.g. to your theme's functions.php) and use the "Actors" panel in the block inspector to assign a number of actor names to a given post. Note that their order is kept (unlike previously, when they were re-ordered in alphabetical order). Save the post, and verify that the term order is still kept. Reload the post, and verify that the order still persists.

Finally, verify that tags entered into the "Tags" panel are still sorted alphabetically across the same set of operations (tag creation, saving the post, reloading it). This demonstrates that the input field truly reflects the sort order that a taxonomy is registered with.

function register_taxonomies() {
	/**
	 * Taxonomy: Actors.
	 */

	 $labels = array(
		'name'          => __( 'Actors' ),
		'singular_name' => __( 'Actor' ),
		'add_new_item'  => __( 'Add Actor' ),
		'new_item_name' => __( 'New Actor' ),
	);

	$args = array(
		'label'                 => __( 'Cast' ),
		'labels'                => $labels,
		'public'                => true,
		'publicly_queryable'    => true,
		'hierarchical'          => false,
		'show_ui'               => true,
		'show_in_menu'          => true,
		'show_in_nav_menus'     => true,
		'query_var'             => true,
		'show_admin_column'     => false,
		'show_in_rest'          => true,
		'show_tagcloud'         => false,
		'rest_base'             => 'actors',
		'rest_controller_class' => 'WP_REST_Terms_Controller',
		'rest_namespace'        => 'wp/v2',
		'show_in_quick_edit'    => false,
		'show_in_graphql'       => false,
		'sort'                  => true, // Note this.
		'args'                  => array( 'orderby' => 'term_order' ), // Note this!
	);

	register_taxonomy( 'actors', array( 'post' ), $args );
}

add_action( 'init', 'register_taxonomies', 0 );

Screenshots or screencast

Before After
flat-terms-before flat-terms-after

@ockham ockham added the [Type] Enhancement A suggestion for improvement. label Nov 20, 2024
@ockham ockham self-assigned this Nov 20, 2024
@ockham ockham force-pushed the fix/keep-term-order-according-to-taxonomy-args branch from 82608e6 to 1d6db39 Compare November 20, 2024 11:40
@ockham ockham marked this pull request as ready for review November 20, 2024 11:41
@ockham ockham requested a review from spacedmonkey as a code owner November 20, 2024 11:41
@ockham ockham requested a review from jsnajdr November 20, 2024 11:41
Copy link

github-actions bot commented Nov 20, 2024

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ockham <[email protected]>
Co-authored-by: jsnajdr <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@ockham ockham force-pushed the fix/keep-term-order-according-to-taxonomy-args branch from 69972b0 to 5e22ffa Compare November 20, 2024 11:53
Copy link

Flaky tests detected in b525718.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11953506179
📝 Reported issues:

Copy link
Member

@jsnajdr jsnajdr left a comment

Choose a reason for hiding this comment

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

Yes, this is a very nice patch 👍 And the Core backport in WordPress/wordpress-develop#7848 also makes perfect sense. The REST controller should apply the taxonomy's default args.

@jsnajdr
Copy link
Member

jsnajdr commented Nov 22, 2024

I was actually wondering if 'sort' => true should set 'args' => [ 'order_by' => 'term_order' ] by default

My answer is yes and no 🙂 I found the original ticket where sort was added in 2008: https://core.trac.wordpress.org/ticket/5857 And one of the comments has this code snippet:

function plugin_get_the_ordered_terms ( $terms, $id, $taxonomy ) {
  if ( 'post_tag' != $taxonomy ) // only ordering tags for now but could add other taxonomies here.
    return $terms;

  return wp_get_object_terms( $id, $taxonomy, array( 'orderby' => 'term_order' ) );
}

add_filter( 'get_the_terms', 'plugin_get_the_ordered_terms' , 10, 4 );

/* Adds sorting by term_order to post_tag by doing a partial register replacing the default */
function plugin_register_sorted_post_tag () {
  register_taxonomy( 'post_tag', 'post', array( 'sort' => true, 'args' => array( 'orderby' => 'term_order' ) ) );
}

add_action( 'init', 'plugin_register_sorted_post_tag' );

It adds order_by => term_order to two places:

  1. to the taxonomy's default args, the same thing that you're considering
  2. to the get_the_terms function, which retrieves the terms for the frontend template and apparently doesn't use the default args.

The biggest problem I see here is that ordering by term_order makes sense only when listing terms for a particular post. And each post can have different order! It's a property of the post<->taxonomy relationship, not of the taxonomy terms themselves.

The get_the_terms function always retrieves terms for a particular post, so term_order makes sense here.

But orderby in default taxonomy args is also used to list all the terms that exist, for example on the wp-admin/edit-tags.php page. Here term_order makes no sense at all, because there is no such order for terms. The get_terms function will change term_order to term_id when it detects there is no post ID specified:

https://github.com/WordPress/wordpress-develop/blob/8cd0ec0234cf09b275031073274283cc860334e6/src/wp-includes/class-wp-term-query.php#L445

So, adding orderby => term_order to default args means that on the wp-admin/edit-tags.php page, and at other places where all terms are listed, the terms would be sorted by term_id. Which is a somewhat random number.

@ockham ockham merged commit a6a04be into trunk Nov 25, 2024
65 checks passed
@ockham ockham deleted the fix/keep-term-order-according-to-taxonomy-args branch November 25, 2024 10:20
@github-actions github-actions bot added this to the Gutenberg 19.8 milestone Nov 25, 2024
@bph bph added the REST API Interaction Related to REST API label Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
REST API Interaction Related to REST API [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Taxonomies: Term order not retained (if sort is true)
3 participants