get_categories returning duplicate child categories in 3.9

[BStofko]

Executing the get_categories function for child categories returns duplicate entries in version 3.9. This worked in version 3.8.1 so this is something new.

My call:

$terms = get_categories( array ('child_of' => $cat));

Under version 3.8.1 returns:

terms: Array (
	[0] => stdClass Object (
		[term_id] => 7
		[name] => Child Category I
		[slug] => child-category-i
		[term_group] => 0
		[term_taxonomy_id] => 7
		[taxonomy] => category
		[description] =>
		[parent] => 4
		[count] => 1
		[cat_ID] => 7
		[category_count] => 1
		[category_description] =>
		[cat_name] => Child Category I
		[category_nicename] => child-category-i
		[category_parent] => 4
	)
)

Under version 3.9 returns:

terms: Array (
	[0] => stdClass Object (
		[term_id] => 7
		[name] => Child Category I
		[slug] => child-category-i
		[term_group] => 0
		[term_taxonomy_id] => 7
		[taxonomy] => category
		[description] =>
		[parent] => 4
		[count] => 1
		[cat_ID] => 7
		[category_count] => 1
		[category_description] =>
		[cat_name] => Child Category I
		[category_nicename] => child-category-i
		[category_parent] => 4
	)
	[4] => stdClass Object (
		[term_id] => 7
		[name] => Child Category I
		[slug] => child-category-i
		[term_group] => 0
		[term_taxonomy_id] => 7
		[taxonomy] => category
		[description] =>
		[parent] => 4
		[count] => 1
		[object_id] => 1746
		[filter] => raw
		[cat_ID] => 7
		[category_count] => 1
		[category_description] =>
		[cat_name] => Child Category I
		[category_nicename] => child-category-i
		[category_parent] => 4
	)
)

[BStofko]

I have tracked this down to the function _get_term_children in wp-includes/taxonomy.php.

A new section of code has been added, which essentially duplicates the work of an old section of code that is still there. I'm not sure what is the reason for the new code, but it seems that one or the other should be removed.

/* This (new) block adds the child terms to $term_list */
if ( $term->term_id == $term_id ) {
	if ( isset( $has_children[$term_id] ) ) {
		$current_id = $term_id;
		while ( $current_id > 0 ) {
			foreach ( $has_children[$current_id] as $t_id ) {
				if ( $use_id ) {
					$term_list[] = $t_id;
				} else {
					$term_list[] = get_term( $t_id, $taxonomy );
				}
			}
			$current_id = isset( $has_children[$t_id] ) ? $t_id : 0;
		}
	}
	continue;
}

/* This (old) block adds the child terms to $term_list again! */
if ( $term->parent == $term_id ) {
	if ( $use_id )
		$term_list[] = $term->term_id;
	else
		$term_list[] = $term;

	if ( !isset($has_children[$term->term_id]) )
		continue;

	if ( $children = _get_term_children($term->term_id, $terms, $taxonomy) )
		$term_list = array_merge($term_list, $children);
}

[wonderboymusic]

This is my fault, I look forward to rectifying this situation

[SergeyBiryukov]

Confirmed, working on a patch and unit tests.

[SergeyBiryukov]

So, we have a public function get_term_children(), which returns all child IDs of an item, and a private function _get_term_children(), which only returns child items (IDs or objects) from a given subset.

It looks like the changes to _get_term_children() in [27108] and [27125] were an attempt to make it work like get_term_children(), i.e. return all child IDs, regardless of the passed subset. This leads to duplicated results when using child_of, due to the same items being added to the $term_list array twice.

Instead, I think the correct fix for both #26903 and this ticket would be to revert those changes and just use get_term_children() in get_terms() when dealing with hierarchical.

This is confirmed by unit tests, however they still intermittently fail for some unknown reason, as noted in IRC:
​http://irclogs.wordpress.org/chanlog.php?channel=wordpress-dev&day=2014-02-19&sort=asc#m794718

27123.patch​ contains the fix and the new test. It also makes assertions for #26903 a bit more accurate.

delete_option() hack looks weird, but it seems to be required for get_terms() to work properly in the test. test_get_terms_exclude_tree() has the same hack: tags/3.8.1/tests/phpunit/tests/term/getTerms.php#L168​ (added in [25933]).

[SergeyBiryukov]

Replying to SergeyBiryukov:

This is confirmed by unit tests, however they still intermittently fail for some unknown reason, as noted in IRC:
​http://irclogs.wordpress.org/chanlog.php?channel=wordpress-dev&day=2014-02-19&sort=asc#m794718

See ticket:14485:95.

27123.2.patch​ uses clean_term_cache() instead of delete_option( 'category_children' ).

[SergeyBiryukov]

27123.3.patch​ removes both clean_term_cache() and delete_option( 'category_children' ) workarounds from the tests, as they are no longer needed after the changes in #14485 and #25711.

[SergeyBiryukov]

Replying to SergeyBiryukov:

This is confirmed by unit tests, however they still intermittently fail for some unknown reason, as noted in IRC:
​http://irclogs.wordpress.org/chanlog.php?channel=wordpress-dev&day=2014-02-19&sort=asc#m794718

Unit test failures are fixed in [27300].

Moved unrelated changes to separate tickets: #27312 and #27313.

27123.4.patch​ should be good to go.

[wonderboymusic]

In 27458:

In get_terms(), leverage get_term_children() over _get_term_children() when making sure to show empty terms that have children in a hierarchical taxonomy while avoiding duplicates.

Adds unit test for child_of param. Adjusts unit tests for get_terms().

See [27108] and [27125].
Props SergeyBiryukov.
Fixes #27123.

[kovshenin]

I think r27458 has the potential of an infinite recursion resulting in a fatal error.

I haven't found a way to create a term and then set its parent to itself from the UI, but clearly it was possible, at least with wp_update_term() before r15806. We have a check in _get_term_children() to avoid that loop, but the move to get_term_children() skips that check.

Probably quite an edge case, but I have come across at least a dozen blogs with the borked term parents in the database, as well as a few with the correct data but a borked category_children option.

Patch with unit tests in 27123.diff​.

[nacin]

In 27837:

Avoid infinite recursion in get_term_children() when a term is incorrectly a parent of itself.

props kovshenin.
fixes #27123.