Consistency issue with `include` parameter set to "0" in `WP_Term_Query`

[audrasjb]

There is a consistency issue with include parameter when used in WP_Term_query.

If include is set to/contains [0], the query returns all terms.

This is not consistent with other query classes, like WP_query (using corresponding post_in parameter) and even more like WP_User_query (using the same include parameter).

Those doesn't return any result.

See reproductible examples below:

WP_Query

$args = array(
        'post_type' => 'post',
        'post__in' => [0],
);
$wp_query = new WP_Query( $args );
if ( $wp_query->have_posts() ) :
        while ( $wp_query->have_posts() ) :
                $wp_query->the_post();
                echo $post->ID;
        endwhile;
endif;

=> returns no post

WP_user_query

$args = array(
        'role' => '',
        'include' => [0],
);
$wp_user_query = new WP_User_Query( $args );
$users = $wp_user_query->get_results();
if ( ! empty( $users ) ) :
        foreach ( $users as $user ) :
                echo $user->ID;
        endforeach;
endif;

=> returns no user

WP_Term_Query

$args = array(
        'taxonomy' => 'category',
        'include' => [0],
);
$wp_term_query = new WP_Term_query( $args );
if ( ! empty( $wp_term_query->terms ) ) :
        foreach ( $wp_term_query->terms as $term ) :
                echo $term->term_id;
        endforeach;
endif;

=> returns all terms

Credits @loicblascos for the initial bug report.

[audrasjb]

Fix 47719 consistency issue with include parameter

[audrasjb]

47719.diff is fixing the issue on my side.

Credits: @loicblascos for the initial bug report on WP French Slack Team.

[audrasjb]

Removing useless $inclusions var

[mukesh27]

@audrasjb Patch 47719.1.diff working fine for me in the latest version

[Hareesh Pillai]

Patch refreshed against trunk

Trac ticket: https://core.trac.wordpress.org/ticket/47719

• Adds unit test
• Validated issue, ie returns all terms when 'include' => array( 0 ),
• Applies patch `47719.2.diff`
• Tests pass, ie returns null

[hellofromTonya]

​PR 679 adds unit test for the patch.

Step 1: Validate issue

Before applying patch 47719.2.diff, added the unit test. Confirmed the query returns all of the terms when 'include' => [0]. Failing test results:

<?php
There was 1 failure:

1) Tests_Term_Query::test_should_return_null_when_include_0
Failed asserting that Array &0 (
    0 => WP_Term Object &000000003eeb693c000000003803654e (
        'term_id' => 2
        'name' => 'Term 0000018'
        'slug' => 'term-0000018'
        'term_group' => 0
        'term_taxonomy_id' => 2
        'taxonomy' => 'wptests_tax'
        'description' => 'Term description 0000018'
        'parent' => 0
        'count' => 0
        'filter' => 'raw'
    )
    1 => WP_Term Object &000000003eeb693d000000003803654e (
        'term_id' => 3
        'name' => 'Term 0000019'
        'slug' => 'term-0000019'
        'term_group' => 0
        'term_taxonomy_id' => 3
        'taxonomy' => 'wptests_tax'
        'description' => 'Term description 0000019'
        'parent' => 0
        'count' => 0
        'filter' => 'raw'
    )
    2 => WP_Term Object &000000003eeb693e000000003803654e (
        'term_id' => 4
        'name' => 'Term 0000020'
        'slug' => 'term-0000020'
        'term_group' => 0
        'term_taxonomy_id' => 4
        'taxonomy' => 'wptests_tax'
        'description' => 'Term description 0000020'
        'parent' => 0
        'count' => 0
        'filter' => 'raw'
    )
) is null.

/var/www/tests/phpunit/tests/term/query.php:867

Step 2: Apply patch. Revalidate.

Confirmed. Test passes, i.e. no terms are returned.

[hellofromTonya]

When applying the patch, the $terms property's value and data type changes: was array, now null. This will be a breaking change, especially when using the terms property in an iterator, such as foreach.

When no terms are found, ​this commit changes the terms property to an empty array before bailing out.

<?php
                if ( empty( $terms ) ) {
                        wp_cache_add( $cache_key, array(), 'terms', DAY_IN_SECONDS );
                        $this->terms = array();
                        return $this->terms;
                }

@audrasjb @SergeyBiryukov What do you foresee any side effects or impacts from this change?

[hellofromTonya]

With 5.6 RC1 tomorrow, it's too late in the cycle for a WP_Query change. Punting this ticket.

If any maintainer or committer feels this can be resolved in time, or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

[lukecarbis]

As per @hellofromTonya's comment above, we're too late into the 5.7 cycle to include this change. I'll update to Future Release for now.

[johnjamesjacoby]

Patch refreshed against trunk, based on Tonya's PR, with renamed tests and some added whitespace

[johnjamesjacoby]

cc @boonebgorges for 2nd opinion, as a current maintainer of the Taxonomy & Query components

[johnjamesjacoby]

In 61048:

Query: Prevent querying for all terms in WP_Term_Query when include is set to [0].

This change brings the include parameter of WP_Term_Query up-to-speed with the include and __in parameters of other _Query classes, so that sending a value of [0] will not unintentionally perform an unbound query that returns all Terms.

It also introduces 3 new unit tests (for the Post, Term, and User query classes) to compare this behavior between them and ensure they remain consistent going forward.

Props audrasjb, hareesh-pillai, hellofromTonya, johnjamesjacoby, mukesh27.

Fixes #47719.