Make WordPress Core

Opened 15 years ago

Last modified 4 months ago

#9547 assigned enhancement

Taxonomy - interesting 'unused' term_order column in table term_relationships.

Reported by: michelwppi's profile michelwppi Owned by:
Milestone: Future Release Priority: high
Severity: normal Version: 2.8
Component: Taxonomy Keywords: has-patch dev-feedback
Focuses: Cc:

Description

During development of plugin xili-language, and to sort term by term list of languages in a taxonomy, I discover unused column term_order in term_relationships table and lack of functions in core about this column. Like medias in post, here the user can define languages list with first, second, third,... languages for his website (and xml header). Taxonomy tools are here very powerful without adding tables or annoying coding.

(see code here line 1309-1370).

Before to complete these very basic functions,…

Is it forecast to have more basic / generic functions using term_order in taxonomy.php ?

Related ticket

Attachments (9)

#9547-sw1.diff (2.9 KB) - added by simonwheatley 12 years ago.
Add ordering to term template tags
ordered-post-tags.php (2.2 KB) - added by simonwheatley 12 years ago.
Non-production plugin to turn post_tag into a sorted taxonomy
#9547-sw2.diff (2.9 KB) - added by simonwheatley 12 years ago.
Add ordering to term template tags through $args array type param (removed error_log call)
#9547-sw2.2.diff (2.7 KB) - added by simonwheatley 12 years ago.
Add ordering to term template tags through $args array type param (removed error_log call, var_dump, corrected the_terms args param)
9547.diff (3.1 KB) - added by wonderboymusic 11 years ago.
9547-tests.diff (8.4 KB) - added by tollmanz 11 years ago.
9547.2.diff (3.4 KB) - added by wonderboymusic 11 years ago.
9547.3.diff (11.3 KB) - added by simonwheatley 10 years ago.
Refresh the patch, add in the tests patch, fix typos
9547.4.diff (11.3 KB) - added by simonwheatley 10 years ago.
Amended end assertion of test_get_the_terms_with_args

Download all attachments as: .zip

Change History (51)

#1 @ryan
15 years ago

Right now galleries are the only user of term_order. No one is currently working on a generic API for using term_order. A patch adding some API would be welcome though.

#2 @Denis-de-Bernardy
15 years ago

  • Milestone changed from Unassigned to 2.9
  • Version set to 2.8

there are several duplicates of this bug around. most were closed as wontfix, on grounds of all sorts of fishy looking reasons. to find them, search for page order, or category order.

#3 @Denis-de-Bernardy
15 years ago

  • Milestone changed from 2.9 to 2.8
  • Resolution set to fixed
  • Status changed from new to closed

please re-open if r11250 isn't enough.

#4 @Denis-de-Bernardy
15 years ago

  • Milestone changed from 2.8 to Future Release
  • Resolution fixed deleted
  • Status changed from closed to reopened

sorry, mixed up with a separate ticket I remembered about.

#5 @kevinB
14 years ago

  • Cc kevinB added

#6 follow-up: @lgedeon
13 years ago

  • Cc luke.gedeon@… added

I propose adding an object_order column for this purpose. Term_order was intended for prioritizing multiple terms as applied to a single object. (see #5857)

Object_order would be a logical place for storing the order in the opposite direction - multiple objects assigned to the same term. Both are very helpful concepts.

I will be attempting to write functions for the term_order concept to submit as a patch. If the object_order concept is blessed, you may be able to use some of my code for that purpose too.

Last edited 11 years ago by SergeyBiryukov (previous) (diff)

#7 @lgedeon
13 years ago

No patch needed, actually. Here is the code to use term_order to sort tags.

/**
 * Sort post_tags by term_order
 *
 * @param array $terms array of objects to be replaced with sorted list
 * @param integer $id post id
 * @param string $taxonomy only 'post_tag' is changed.
 * @return array of objects
 */
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;

	$terms = wp_cache_get($id, "{$taxonomy}_relationships_sorted");
	if ( false === $terms ) {
		$terms = wp_get_object_terms( $id, $taxonomy, array( 'orderby' => 'term_order' ) );
		wp_cache_add($id, $terms, $taxonomy . '_relationships_sorted');
	}

	return $terms;
}

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' );

?>

To use this, you will need to add tags in the order you want them to be and if you get them out of order you may have to remove some and add them back correctly. I might write a better UI later, but for now this at least works.

I guess next actions would be to write a patch for object ordering, and to come up with a UI(s) for both tag-style and category-style term_ordering.

#8 @jeremyfelt
12 years ago

Closed #20200 as duplicate.

#9 @lkraav
12 years ago

  • Cc lkraav added

#10 follow-ups: @simonwheatley
12 years ago

  • Cc simon@… added
  • Keywords has-patch dev-feedback added

I've created a patch which adds the ability to request terms ordered by term_order when using get_the_terms or get_the_term_list. Points to note:

  • I've added “simple” function arguments, rather than using an $args style array as a final parameter as this seems to be the pattern for things nearer template tags than API functions, happy to change the patch if this isn't the right approach.
  • This has affected the caching in get_the_terms in that it no longer simply uses the cache controlled by get_object_term_cache, because this cache does not respect ordering. It is not currently possible to pull the terms from the cache and order in PHP within the get_the_terms function, as the term objects do not contain the term_order property, even if it was deemed appropriate to sort terms in PHP at this point. I'd argue that the cache will still fill, even if there's more keys in it, and when it's full the system is just as cached as it was before, i.e. we've still got the function covered by a cache.

@simonwheatley
12 years ago

Add ordering to term template tags

#11 @simonwheatley
12 years ago

Almost forgot. I'll attach a proof of concept plugin which converts post_tag into a sorted taxonomy…

@simonwheatley
12 years ago

Non-production plugin to turn post_tag into a sorted taxonomy

#12 in reply to: ↑ 10 @westi
12 years ago

Replying to simonwheatley:

I've created a patch which adds the ability to request terms ordered by term_order when using get_the_terms or get_the_term_list. Points to note:

  • I've added “simple” function arguments, rather than using an $args style array as a final parameter as this seems to be the pattern for things nearer template tags than API functions, happy to change the patch if this isn't the right approach.
  • This has affected the caching in get_the_terms in that it no longer simply uses the cache controlled by get_object_term_cache, because this cache does not respect ordering. It is not currently possible to pull the terms from the cache and order in PHP within the get_the_terms function, as the term objects do not contain the term_order property, even if it was deemed appropriate to sort terms in PHP at this point. I'd argue that the cache will still fill, even if there's more keys in it, and when it's full the system is just as cached as it was before, i.e. we've still got the function covered by a cache.

I've taken a quick look and it looks good.

Do you fancy creating some unit tests for this new functionality too?

#14 in reply to: ↑ 10 @SergeyBiryukov
12 years ago

  • Keywords taxonomy term_order relationships removed

Replying to simonwheatley:

I've added “simple” function arguments, rather than using an $args style array as a final parameter as this seems to be the pattern for things nearer template tags than API functions, happy to change the patch if this isn't the right approach.

#14634 suggests adding $args.

@simonwheatley
12 years ago

Add ordering to term template tags through $args array type param (removed error_log call)

#15 @simonwheatley
12 years ago

Replying to SergeyBiryukov:

#14634 suggests adding $args.

Thanks Sergey. I can see this being more flexible, though it does increase complexity and I wasn't sure if this kind of parameter was in the spirit of the WordPress template tags. I've added it as a separate patch to make things easier to compare (diffing diffs… fun).

#9547-sw2.diff demonstratesget_the_terms and get_the_term_list with the $args parameter.

A further caching caveat with this patch: If you pass an empty array in $args (as is the default) it will cache under a different key to a call where the $args param contains values equivalent to the defaults in wp_get_object_terms. This is because the patch is MD5 hashing the serialised $args array. I can't think of a way around this without duplicating the defaults from wp_get_object_terms in get_the_terms (and I suspect in most cases the problem is not serious).

Last edited 12 years ago by simonwheatley (previous) (diff)

#16 follow-up: @SergeyBiryukov
12 years ago

#9547-sw2.diff has a var_dump() left in line 1077 and still passes two separate arguments to get_the_term_list() in the_terms().

@simonwheatley
12 years ago

Add ordering to term template tags through $args array type param (removed error_log call, var_dump, corrected the_terms args param)

#17 in reply to: ↑ 16 @simonwheatley
12 years ago

Replying to SergeyBiryukov:

#9547-sw2.diff has a var_dump() left in line 1077 and still passes two separate arguments to get_the_term_list() in the_terms().

Thanks for spotting the the_terms param missing, apologies for the var_dump. Attaching a new diff.

@wonderboymusic
11 years ago

#18 @wonderboymusic
11 years ago

  • Keywords needs-unit-tests added; dev-feedback removed
  • Milestone changed from Future Release to 3.6

Patch no longer applied cleanly, updated and changed the cache key / group to be more friendly with others

#19 @SergeyBiryukov
11 years ago

#14634 was marked as a duplicate.

#20 in reply to: ↑ 6 @husobj
11 years ago

  • Cc ben@… added

Replying to lgedeon:

I propose adding an object_order column for this purpose. Term_order was intended for prioritizing multiple terms as applied to a single object. (see #5857)

Object_order would be a logical place for storing the order in the opposite direction - multiple objects assigned to the same term. Both are very helpful concepts.

If term_order is intended to be used to prioritise the order of terms associated with posts, I second the idea of a object_order column for prioritising the order of posts within terms - essentially the equivalent of menu_order for posts, but for taxonomies.

@tollmanz
11 years ago

#21 follow-up: @tollmanz
11 years ago

  • Cc tollmanz@… added

Added unit tests that attempt to test the latest patch as well as do a few other things:

  • Ordering using wp_get_post_terms is tested. The latest patch adds the ability to order by "term_group" and previously no ordering has been tested with wp_get_post_terms. I added those tests.
  • get_terms, get_the_term_list, and the_terms are tested when the $args parameter is used, which was added with the latest patch for this ticket. In all of these tests, I tested to make sure that the cache key was handled correctly so that different $args values would generate different cache objects.
  • The latest patch broke the "test_object_term_cache" test. This has been fixed. The issue was due to the change of the cache key in the latest patch.

#22 @ryan
11 years ago

  • Milestone changed from 3.6 to Future Release

#23 @emzo
11 years ago

  • Cc wordpress@… added

#24 @wonderboymusic
11 years ago

  • Milestone changed from Future Release to 3.7

9547.2.diff​ refreshes so applying doesn't blow up.

#25 @greenshady
11 years ago

  • Cc justin@… added

#26 follow-up: @c3mdigital
11 years ago

Just adding another use case for the term_order column. I would love to see an additional WP_Query orderby argument 'term_order' to sort the order or posts.

Use Case:
Many enterprise publishers using WordPress require a way to set the precise order of posts on the home page as well as other areas. menu_order is great for the home page but when a post is in multiple categories or more than one sort_order is required for a post depending on where it is being displayed. The term_order column is perfect for this because anytime you do a tax_query the term_relationships table is already INNER JOIN 'd with the posts table. This works just like menu_order except only when the tables are joined via a term archive or new WP_Query

The flexibility of the current term_relationships table allows us to use the term_order column for any object_id the term is assigned to.

WP_Query extension to allow for ordering posts by term_order:

/**
 * Extends WP_Query with a posts_orderby filter that allows you to order posts by the term_relationships.term_order column
 *
 * @class XTeam_Lineup_Query
 */
class XTeam_Lineup_Query extends WP_Query {

	var $lineup_query;

	function __construct( $args = array() ) {
		add_filter( 'posts_orderby', array( &$this, 'posts_orderby' ), 10, 2 );
		$this->lineup_query = true;

		parent::query( $args );
	}

	function posts_orderby( $orderby, $query ) {
		if ( isset( $query->lineup_query ) ) {
			global $wpdb;
			return "$wpdb->term_relationships.term_order ASC, post_date DESC";
		}
		return $orderby;
	}
}

Resulting SQL Query:
`SELECT wpomni_posts.ID FROM wpomni_posts INNER JOIN wpomni_term_relationships ON (wpomni_posts.ID = wpomni_term_relationships.object_id) WHERE 1=1 AND ( wpomni_term_relationships.term_taxonomy_id IN (9) ) AND wpomni_posts.post_type = 'post' AND (wpomni_posts.post_status = 'publish' OR wpomni_posts.post_status = 'private') GROUP BY wpomni_posts.ID ORDER BY wpomni_term_relationships.term_order ASC, post_date DESC LIMIT 0, 10
`

UI for setting the order of posts
Not suggesting a ui for this but wanted to give an example of how the posts could be ordered. Using UI-Sortable we allow the posts to be ordered directly on edit.php when the list table is filtered by term.
Ajax callback bits that set the order:

		$str = str_replace( 'post-', '', $posts );
		$order = explode( ',', $str );
		$count = 1;
		foreach ( $order as $item_id ) {
			$wpdb->query( $wpdb->prepare(
					"UPDATE $wpdb->term_relationships SET term_order = %d WHERE object_id = %d AND term_taxonomy_id = %d",
					$count,
					$item_id,
					$term_tax_id
					)
			);
			$count ++;

I can work on patch or roll this into a plugin if it's something that has any traction.

#27 in reply to: ↑ 26 @husobj
11 years ago

Replying to c3mdigital:

Just adding another use case for the term_order column. I would love to see an additional WP_Query orderby argument 'term_order' to sort the order or posts.

I agree, being able to order posts belonging to a term would be great functionality.

The above discussion seems to indicate that the term_order column was intended for ordering terms associated with a post, not posts associate with a term though.
http://core.trac.wordpress.org/ticket/9547#comment:20

I already use this column for ordering posts in a term which is fine for now unless WordPress core implements a UI that would interfere but allowing people to use it for ordering terms instead.

#28 follow-up: @nacin
10 years ago

  • Type changed from feature request to enhancement

Should get_the_term_list() and the_terms() be converted from $before = '', $sep = '', $after = '', $args = array() to just $args = array() that contains 'before', 'sep', and 'after'?

#29 in reply to: ↑ 28 ; follow-ups: @helen
10 years ago

Replying to nacin:

Should get_the_term_list() and the_terms() be converted from $before = '', $sep = '', $after = '', $args = array() to just $args = array() that contains 'before', 'sep', and 'after'?

My vote would be yes.

#30 @nacin
10 years ago

  • Milestone changed from 3.7 to Future Release

Can someone change the title of this ticket to better reflect what is actually being proposed? It seems we're just trying to expose lower level API higher up in the stack (and crossing the line between uncached API and cached API). Because of that, this is a safe punt.

@simonwheatley
10 years ago

Refresh the patch, add in the tests patch, fix typos

#31 @simonwheatley
10 years ago

Refreshed the patch:

  • Combined the tests and code changes into one patch
  • Now applies against current trunk
  • Changed "Verify get by …" to "Verify order by …" comments in tests
  • Added ticket reference to test_get_the_terms_by_order

#32 in reply to: ↑ 21 @simonwheatley
10 years ago

Replying to tollmanz:

Added unit tests that attempt to test the latest patch as well as do a few other things:

Reading through the tests, I noticed test_get_the_terms_with_args ended with $this->assertFalse( in_array( $orderby_asc, $orderby_desc ) );, which seems like a copy/paste error to me. I think this should be asserting that the two arrays are not equal instead? (Amended patch coming up.)

Otherwise the tests make sense to me.

@simonwheatley
10 years ago

Amended end assertion of test_get_the_terms_with_args

#33 in reply to: ↑ 29 @simonwheatley
10 years ago

Replying to helen:

Replying to nacin:

Should get_the_term_list() and the_terms() be converted from $before = '', $sep = '', $after = '', $args = array() to just $args = array() that contains 'before', 'sep', and 'after'?

My vote would be yes.

Refreshing and so forth aside: how would you like to tackle this? I can see either:

Option 1. Backwards compatibility: Check the third argument in get_the_term_list() and the_terms() to see if it's a string (treat the arguments as "legacy format", with the sixth argument as the $args array) and if the third argument is an array, assume the author is new school and interpret this as the $args argument. Something like:

/**
 * Retrieve a post's terms as a list with specified format.
 *
 * The following information has to do with additional arguments in the $args parameter:
 * 
 * 'before' string Optional. Before list.
 * 'sep'    string Optional. Separate items using this.
 * 'after'  string Optional. After list.
 *
 * @since 2.5.0
 *
 * @see wp_get_object_terms() for reference on the $args param
 *
 * @param int $id Post ID.
 * @param string $taxonomy Taxonomy name.
 * @param array $args Accepts the same arguments as the $args param for wp_get_object_terms, 
                      as well as some additions listed above.
 * @return string|bool|WP_Error A list of terms on success, false or WP_Error on failure.
 */
function get_the_term_list( $id, $taxonomy, $args = array(), $deprecated_1 = '', $deprecated_2 = '', $deprecated_3 = array() ) {
	if ( is_string( $args ) ) {
		_doing_it_wrong( __FUNCTION__, sprintf( __( 'get_the_term_list expects the third argument to be a $args array, you can specify $before, $sep, and $after in the array.' ), '4.0' );
		$before = $args;
		$sep    = $deprecated_1;
		$after  = $deprecated_2;
		$args   = $deprecated_3;
	} else {
		$before = isset( $args[ 'before' ] ) ? $args[ 'before' ] : '';
		$sep    = isset( $args[ 'sep' ]    ) ? $args[ 'sep' ]    : '';
		$after  = isset( $args[ 'after' ]  ) ? $args[ 'after' ]  : '';
	}

Option 2. New way or the highway: Check if the third argument is a string, and if it is then call _doing_it_wrong but otherwise just drop the argument. Something like:

/**
 * Display the terms in a list.
 *
 * The following information has to do with additional arguments in the $args parameter:
 * 
 * 'before' string Optional. Before list.
 * 'sep'    string Optional. Separate items using this.
 * 'after'  string Optional. After list.
 *
 * @since 2.5.0
 *
 * @param int $id Post ID.
 * @param string $taxonomy Taxonomy name.
 * @param array $args Accepts the same arguments as the $args param for wp_get_object_terms, 
                      as well as some additions listed above.
 * @return null|bool False on WordPress error. Returns null when displaying.
 */
function the_terms( $id, $taxonomy, $args = array() ) {
	if ( is_string( $args ) ) {
		_doing_it_wrong( __FUNCTION__, sprintf( __( 'the_terms expects the third argument to be a $args array, you can specify $before, $sep, and $after in the array.' ), '4.0' );
	}

Having coded them out in outline, I vote for not making the change or going for Option 2, to keep the code simpler.

Last edited 10 years ago by simonwheatley (previous) (diff)

This ticket was mentioned in IRC in #wordpress-dev by simonwheatley. View the logs.


10 years ago

#35 in reply to: ↑ 29 @simonwheatley
10 years ago

  • Keywords dev-feedback added; needs-unit-tests removed

Replying to helen:

Replying to nacin:

Should get_the_term_list() and the_terms() be converted from $before = '', $sep = '', $after = '', $args = array() to just $args = array() that contains 'before', 'sep', and 'after'?

My vote would be yes.

Having given this some thought, I don't think it's a good idea to change the parameters. If we change the third parameter from $before to $args I cannot see how we can reliably detect when the value being passed was intended as a WordPress $args (which can be a URL encoded string or an associative array) without code which becomes quite involved and is still not reliable.

(In detecting whether a value might be intended as $args string (detecting associative arrays is fairly straightforward) we could look for a = character, but this might be present in an attribute definition of some HTML in a valid $before value.)

I recommend we leave the parameters as they are.

#36 @ryan
10 years ago

  • Owner ryan deleted
  • Status changed from reopened to assigned

#38 @SergeyBiryukov
8 years ago

#23747 was marked as a duplicate.

#39 @SergeyBiryukov
8 years ago

#30244 was marked as a duplicate.

#40 @boonebgorges
6 years ago

#44505 was marked as a duplicate.

This ticket was mentioned in Slack in #core by sergey. View the logs.


4 years ago

This ticket was mentioned in Slack in #core by abhanonstopnews. View the logs.


4 months ago

Note: See TracTickets for help on using tickets.