Opened 5 years ago
Last modified 4 years ago
#47719 reviewing defect (bug)
Consistency issue with `include` parameter set to "0" in `WP_Term_Query`
Reported by: | audrasjb | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | |
Component: | Query | Keywords: | has-patch has-unit-tests dev-feedback |
Focuses: | Cc: |
Description (last modified by )
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.
Attachments (3)
Change History (14)
#1
@
5 years ago
- Keywords has-patch needs-unit-tests added; needs-patch removed
47719.diff
is fixing the issue on my side.
Credits: @loicblascos for the initial bug report on WP French Slack Team.
#3
@
5 years ago
@audrasjb Patch 47719.1.diff working fine for me in the latest version
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
4 years ago
#5
@
4 years ago
- Milestone changed from Awaiting Review to 5.6
- Owner set to SergeyBiryukov
- Status changed from new to reviewing
This ticket was mentioned in PR #679 on WordPress/wordpress-develop by hellofromtonya.
4 years ago
#6
- Keywords has-unit-tests added; needs-unit-tests removed
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
#7
@
4 years ago
- Keywords commit added; dev-feedback removed
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.
#8
@
4 years ago
- Keywords dev-feedback added; commit removed
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?
This ticket was mentioned in Slack in #core by pbiron. View the logs.
4 years ago
#10
@
4 years ago
- Milestone changed from 5.6 to 5.7
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.
Fix 47719 consistency issue with include parameter