get_the_terms() doesn't respect register_taxonomy()'s 'orderby' => 'term_order'

[danielbachhuber]

The following test passes in 4.6 and fails in 4.7:

public function test_get_the_terms_should_respect_taxonomy_orderby() {
	register_taxonomy( 'wptests_tax', 'post', array(
		'sort' => true,
		'args' => array(
			'orderby' => 'term_order',
		),
	) );
	$term_ids = self::factory()->term->create_many( 2, array(
		'taxonomy' => 'wptests_tax',
	) );
	$post_id = self::factory()->post->create();
	wp_set_object_terms( $post_id, array( $term_ids[0], $term_ids[1] ), 'wptests_tax' );
	$terms = get_the_terms( $post_id, 'wptests_tax' );
	$this->assertEquals( array( $term_ids[0], $term_ids[1] ), wp_list_pluck( $terms, 'term_id' ) );
	// Flip the order
	wp_set_object_terms( $post_id, array( $term_ids[1], $term_ids[0] ), 'wptests_tax' );
	$terms = get_the_terms( $post_id, 'wptests_tax' );
	$this->assertEquals( array( $term_ids[1], $term_ids[0] ), wp_list_pluck( $terms, 'term_id' ) );
}

I haven't tracked down the source of the break though.

[johnbillion]

Broken in [38667].

cc @boonebgorges

[boonebgorges]

Yes, totally my fault. This argument merging was omitted when wp_get_object_terms() was refactored to use WP_Term_Query.

In my (lame) defense, I had no idea that this weird feature existed - it was introduced in [7520], doesn't appear to be documented anywhere, and is a pattern I've never seen used elsewhere in WordPress. It's not the worst idea to have taxonomy-specific query arguments (or post types, for that matter!), but perhaps we should advertise it better.

40496.diff​ appears to resolve the problem by restoring the argument-merging from pre-4.7. @danielbachhuber Care to have a look?

[danielbachhuber]

In my (lame) defense, I had no idea that this weird feature existed - it was introduced in [7520], doesn't appear to be documented anywhere, and is a pattern I've never seen used elsewhere in WordPress.

Undocumented features are the best features.

40496.diff​ appears to resolve the problem by restoring the argument-merging from pre-4.7. @danielbachhuber Care to have a look?

Good thing I tested! It fixes the problem for querying one taxonomy, but breaks for querying two taxonomies with wp_get_object_terms().

Here's the test that fails:

public function test_wp_get_object_terms_should_respect_taxonomy_orderby() {
	register_taxonomy( 'wptests_tax', 'post', array(
		'sort' => true,
		'args' => array(
			'orderby' => 'term_order',
		),
	) );
	$term_ids = self::factory()->term->create_many( 2, array(
		'taxonomy' => 'wptests_tax',
	) );
	$post_id = self::factory()->post->create();
	wp_set_object_terms( $post_id, array( $term_ids[0], $term_ids[1] ), 'wptests_tax' );
	$terms = wp_get_object_terms( $post_id, array( 'category', 'wptests_tax' ) );
	$this->assertEquals( array( $term_ids[0], $term_ids[1] ), wp_list_pluck( $terms, 'term_id' ) );
	// Flip the order
	wp_set_object_terms( $post_id, array( $term_ids[1], $term_ids[0] ), 'wptests_tax' );
	$terms = wp_get_object_terms( $post_id, array( 'category', 'wptests_tax' ) );
	$this->assertEquals( array( $term_ids[1], $term_ids[0] ), wp_list_pluck( $terms, 'term_id' ) );
}

The original code set $terms = array() early, then called wp_get_object_terms() for each taxonomy and merged the terms in.

The code in 4.7 sets $terms = get_terms( $args ); after the code you've ported over, so the handling for two taxonomies is discarded.

[danielbachhuber]

40496.2.diff includes the unit test for querying two taxonomies, and fixes the unit test by:

1. Parsing $args and setting a default $terms = array(); before taxonomy args are processed.
2. Merging $terms = array_merge( $terms, get_terms( $args ) ); after taxonomy args are processed, to ensure already queried terms aren't discarded.

[boonebgorges]

In 40513:

Restore support for taxonomy 'args' override when querying object terms.

[7520] introduced an undocumented feature whereby developers could
register a custom taxonomy with an 'args' parameter, consisting of
an array of config params that, when present, override corresponding
params in the $args array passed to wp_get_object_terms() when
using that function to query for terms in the specified taxonomy.

The wp_get_object_terms() refactor in [38667] failed to respect
this secret covenant, and the current changeset atones for the
transgression.

Props danielbachhuber.
Fixes #40496.

[boonebgorges]

In 40514:

Restore support for taxonomy 'args' override when querying object terms.

[7520] introduced an undocumented feature whereby developers could
register a custom taxonomy with an 'args' parameter, consisting of
an array of config params that, when present, override corresponding
params in the $args array passed to wp_get_object_terms() when
using that function to query for terms in the specified taxonomy.

The wp_get_object_terms() refactor in [38667] failed to respect
this secret covenant, and the current changeset atones for the
transgression.

Ports [40513] to the 4.7 branch.

Props danielbachhuber.
Fixes #40496.

[GhostToast]

@boonebgorges @danielbachhuber - I stumbled upon this trac ticket, when working with WP 4.8 I noticed I was getting duplicate terms (double the amount) when using get_the_terms() on a custom post type with custom taxonomies. I followed it along to the line where it does the array_merge() with a (seemingly) redundant call to get_terms():

$terms = array_merge( $terms, get_terms( $args ) );

This perplexed me for a while until I experimented enough to discover that if my custom post type had at least 1 taxonomy registered to it which _did not_ provide this in its setup:

'args' => [
	'orderby' => 'term_order',
],

then the problem was resolved!

I'm not sure if this is a new bug or a re-introduced one, but wanted to point it out.

To reproduce, try to view a custom post type's terms, fetched via get_the_terms(), where the custom post type has only taxonomies registered to it which include this manually-declared orderby. You should end up seeing twice as many terms (even if there is only 1 term to view).

[boonebgorges]

@GhostToast - Thanks for reporting this. I'm unable to reproduce, though it's likely that I'm not understanding the setup correctly. Here's the automated test I was using to try reproducing:

		register_post_type( 'wptests_pt' );
		register_taxonomy( 'wptests_tax2', 'wptests_pt', array(
			'args' => array(
				'orderby' => 'term_order',
			),
		) );

		$p = self::factory()->post->create( array(
			'post_type' => 'wptests_pt',
		) );

		$t = self::factory()->term->create( array(
			'taxonomy' => 'wptests_tax2',
		) );

		wp_set_object_terms( $p, $t, 'wptests_tax2' );

		$found = get_the_terms( $p, 'wptests_tax2' );
		print_r( $found );

If you're still having the issue, could you please open a new ticket that references this one, and contains more detailed instructions (ideally, with some code examples) to reproduce the issue? The current ticket was closed against 4.7.5, so it'd be nice to start fresh. Thanks!