get_terms with parent set incorrectly hides non-empty terms

[smerriman]

In #26903, it was reported that there was a bug when get_terms is called with hierarchical=true and parent=0 - it would automatically reset hierarchical to false, and therefore hide parent terms which contained non-empty children.

This was fixed by hardcoding a check for parent=0.

However, this should apply for every other value of parent as well. For example, let's say I had:

• One (0)
  • Two (0)
    • Three (2)
    • Four (3)
  • Five (0)
    • Six (2)

Calling get_terms with a parent set to category 'One' will return no terms, while it should.

[landakram]

If I understand correctly, then for any value of parent, hierarchical should not be automatically reset, since the cases that were already being excluded were when parent == '' and when parent == 0.

I attached a patch which removes the value checks for parent there entirely. I also added a test for the setup described above, which verifies that the patch has the intended behavior.

Apologies for uploading the patch with the wrong extension — I'm not sure how to remove it!

[boonebgorges]

landakram and I worked on this for a while at WCSF contributor day. First off, I am a bozo for marking it good-first-bug. Second, I think we have the proper fix in 29185.2.patch​, but we need to have some more unit tests that describe the current behavior of 'child_of', 'parent', and 'hierarchical', especially in cases where one overrides another.

[landakram]

I wrote some additional unit tests that describe the interplay between child_of, parent, and hierarchical. I used this hierarchy for the tests:

• One
  • Two
    • Three (1)
    • Four
  • Five
    • Six (1)
  • Seven

Here's what I found:

1. When hierarchical == true and child_of is set, both children and non-empty descendants are returned. In other words, in the hierarchy described, Two, Three, Five, and Six are returned.
2. When hierarchical == false and child_of is set, only direct children are returned. In the above hierarchy, the returned array is empty. I wrote another test which the hierarchy so that a post was associated with Seven, and the returned array contained only Seven.
3. When both child_of and parent are set, parent takes precedent over child_of.

4. When hierarchical == true and parent is set, the returned array is empty. This is the bug — it should return Two and Five.
5. When hierarchical == false and parent is set, the returned array is empty.
6. When hierarchical == true and both child_of and parent are set, the returned array is empty. This is the bug — to be consistent with the behavior of 3, it should return Two and Five.
7. When hierarchical == false and both child_of and parent are set, the returned array is empty.

The attached patch includes the unit tests and the fix that boonebgorges and I came up with. When patched, 4 and 5 are fixed (test_hierarchical_parent_get_terms and test_hierarchical_parent_child_of_get_terms respectively).

Now, I can't say whether the existing behaviors make 'sense', but the proposed patch leaves those behaviors intact while fixing this bug :)

[boonebgorges]

Thanks for the additional tests, landakram. The behavior of these parameters is pretty confusing, and it's extremely valuable to have these tests to demonstrate a bit more clearly how they work together.

[jeremyfelt]

Fixed in [30107], commit missed the ticket.