get_terms on hierarchical taxonomy fails when 'fields' arg contains 'id=>name' OR 'id=>slug'

[technical_mastermind]

When using get_terms() and passing in 'id=>name' or 'id=>slug' as the value for the 'fields' argument, it fails to return the expected array if you are working with a hierarchical taxonomy.

For example, this code block works:

$terms = get_terms( 'post_tag', array( 'fields' => 'id=>name' ) );

This code block does not work:

$terms = get_terms( 'category', array( 'fields' => 'id=>name' ) );

The difference being that 'post_tag' is a non-hierarchical taxonomy while 'category' is a hierarchical taxonomy. I have tested this in several versions of WordPress, it is still present in 4.0. I have created a patch for it against trunk as well as written more in-depth unit tests to test the functionality of the get_terms function. Note that 4 of the unit tests I wrote fail before the get_terms patch is applied, none of the unit tests I wrote fail after the patch is applied.

[technical_mastermind]

Bug patch

[technical_mastermind]

Unit tests that I added.

[markjaquith]

Looks like a simple fix and has unit tests. @boonebgorges, you wanna take this one?

[boonebgorges]

Fix looks good at a glance - thanks, technical_mastermind. I'll review the patch in more detail in the upcoming days.

[boonebgorges]

29859.patch​ breaks up technical_mastermind's tests so that only one assertion happens per test. (The duplicated setup code required by the split tests is a bit annoying, but it's important to ensure that the assertions are actually independent of one another. Separating them out also means that one test can fail while the rest continue to run.)

Before going ahead with this fix: technical_mastermind, in your original post you mentioned that *4* of the tests fail without the fix to get_terms(). I'm only seeing two failures: test_get_terms_hierarchical_tax_hide_empty_true_fields_idslug() and test_get_terms_hierarchical_tax_hide_empty_true_fields_idname(). Your diagnosis of the problem - that the 'tt.count' column wasn't being selected when 'hierarchical=true' on hierarchical taxonomies - suggests that there should only be failures when 'hierarchical=true' *and* 'hide_empty=true', since when 'hide_empty=false' the count value is never used. So, 2 seems like the correct number of failing tests. Can you clarify where your 4 is coming from?

[SergeyBiryukov]

Replying to boonebgorges:

The duplicated setup code required by the split tests is a bit annoying

Can we move it into a separate private function?

[boonebgorges]

Replying to SergeyBiryukov:

Replying to boonebgorges:

The duplicated setup code required by the split tests is a bit annoying

Can we move it into a separate private function?

Yes. 29859.2.patch​

[technical_mastermind]

Just looked at this again boonebgorges, you are correct that only 2 fail. That was an error on my part, I incorrectly stated the number of tests that fail. Sorry about the confusion. That's what I get for learning PHPUnit the same night I submit a patch.

[boonebgorges]

Replying to technical_mastermind:

Just looked at this again boonebgorges, you are correct that only 2 fail. That was an error on my part, I incorrectly stated the number of tests that fail. Sorry about the confusion. That's what I get for learning PHPUnit the same night I submit a patch.

Great - thanks for verifying. And really, thank you so much for these unit tests - I appreciate them more than the fix itself :)

[boonebgorges]

In 29845:

In get_terms(), select term taxonomy count for all values of 'fields'.

Not having the count caused queries with 'fields' values of 'id=>name' and
'id=>slug' to return incorrect results when querying a hierarchical taxonomy
with 'hide_empty=true'.

Includes unit tests for get_terms() when using various combinations of 'fields',
'hide_empty', and 'hierarchical' arguments.

Props technical_mastermind.
Fixes #29859.