WordPress.org

Make WordPress Core

#27123 closed defect (bug) (fixed)

get_categories returning duplicate child categories in 3.9

Reported by: BStofko Owned by: wonderboymusic
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.9
Component: Taxonomy Keywords: has-patch
Focuses: Cc:

Description (last modified by SergeyBiryukov)

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

Attachments (5)

27123.patch (3.5 KB) - added by SergeyBiryukov 17 months ago.
27123.2.patch (3.4 KB) - added by SergeyBiryukov 17 months ago.
27123.3.patch (3.8 KB) - added by SergeyBiryukov 16 months ago.
27123.4.patch (3.0 KB) - added by SergeyBiryukov 16 months ago.
27123.diff (1.6 KB) - added by kovshenin 15 months ago.

Download all attachments as: .zip

Change History (15)

comment:1 @BStofko17 months ago

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

comment:2 @wonderboymusic17 months ago

  • Keywords needs-patch needs-unit-tests added
  • Milestone changed from Awaiting Review to 3.9

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

comment:3 @SergeyBiryukov17 months ago

  • Component changed from General to Taxonomy
  • Description modified (diff)

Confirmed, working on a patch and unit tests.

@SergeyBiryukov17 months ago

comment:4 follow-ups: @SergeyBiryukov17 months ago

  • Keywords has-patch added; needs-patch removed

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]).

@SergeyBiryukov17 months ago

comment:5 in reply to: ↑ 4 ; follow-up: @SergeyBiryukov17 months ago

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

@SergeyBiryukov16 months ago

comment:6 in reply to: ↑ 5 @SergeyBiryukov16 months ago

  • Keywords commit added

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.

@SergeyBiryukov16 months ago

comment:7 in reply to: ↑ 4 @SergeyBiryukov16 months ago

  • Keywords needs-unit-tests removed

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.

comment:8 @wonderboymusic16 months ago

  • Owner set to wonderboymusic
  • Resolution set to fixed
  • Status changed from new to closed

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.

@kovshenin15 months ago

comment:9 @kovshenin15 months ago

  • Keywords commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

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.

comment:10 @nacin15 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 27837:

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

props kovshenin.
fixes #27123.

Note: See TracTickets for help on using tickets.