Make WordPress Core

Opened 5 years ago

Last modified 5 years ago

#48284 new defect (bug)

MIsleading behavior and documentation in include argument of WP_Term_Query

Reported by: leogermani's profile leogermani Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Taxonomy Keywords:
Focuses: docs Cc:

Description

The documentation for the include argument of the WP_Term_Query class states:

Array or comma/space-separated string of term ids to include.

(https://core.trac.wordpress.org/browser/tags/5.2/src/wp-includes/class-wp-term-query.php#L111)

This, and also the argument name, gives the impression that these are IDs that will be included in the results, adding terms on the top of the list retrieved by other query arguments.

However, what include actually does is to restrict the results to terms with the informed IDs.

We can see how the class parses this argument here:

                        $inclusions = '';
                        if ( ! empty( $include ) ) {
                                $exclude      = '';
                                $exclude_tree = '';
                                $inclusions   = implode( ',', wp_parse_id_list( $include ) );
                        }
        
                        if ( ! empty( $inclusions ) ) {
                                $this->sql_clauses['where']['inclusions'] = 't.term_id IN ( ' . $inclusions . ' )';
                        }

https://core.trac.wordpress.org/browser/tags/5.2/src/wp-includes/class-wp-term-query.php#L412

And, later, it implodes everything using AND as glue, thus reducing the results:

                        $where = implode( ' AND ', $this->sql_clauses['where'] );

https://core.trac.wordpress.org/browser/tags/5.2/src/wp-includes/class-wp-term-query.php#L643

This can also be tested with a simple test like this:

        function test_include() {
                
                $term1 = wp_insert_term('black', 'category');
                $term2 = wp_insert_term('blue', 'category');
                $term3 = wp_insert_term('green', 'category');
                
                $query = new \WP_Term_Query(['search' => 'bl', 'hide_empty' => false]);
                $found_ids = array_map(function($el) { return $el->term_id; }, $query->terms);
                
                $this->assertEquals(2, sizeof($query->terms));
                $this->assertContains($term1['term_id'], $found_ids);
                $this->assertContains($term2['term_id'], $found_ids);
                
                $new_query = new \WP_Term_Query(['search' => 'bl', 'hide_empty' => false, 'include' => $term3['term_id']]); // returns empty results
                $found_ids = is_array($new_query->terms) ? array_map(function($el) { return $el->term_id; }, $new_query->terms): [];
                
                // All tests fail
                $this->assertEquals(3, sizeof($query->terms)); 
                $this->assertContains($term1['term_id'], $found_ids);
                $this->assertContains($term2['term_id'], $found_ids);
                $this->assertContains($term3['term_id'], $found_ids); 
        }

Proposal

Change this behavior is not recommended because there might be many people relying on this, especially because there is no other argument to get terms by term_id (!!!), so this is the one to use in these cases.

However, we should:

  • Change the documentation, and explain it better what this argument does

Suggestion

We could also:

  • Change the argument name for something more straight forward, such as include_only (or simply term_ids), deprecating but keeping the old include for backward compatibility
  • Implement another argument that does this include

Change History (1)

#1 @SergeyBiryukov
5 years ago

  • Focuses docs added
Note: See TracTickets for help on using tickets.