WordPress.org

Make WordPress Core

Opened 5 weeks ago

Closed 4 weeks ago

#51630 closed defect (bug) (fixed)

Partial term counts: `_wp_prevent_term_counting()` breaks term counting for terms created on actions.

Reported by: peterwilsoncc Owned by:
Milestone: Priority: normal
Severity: normal Version: 5.6
Component: General Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

_wp_prevent_term_counting() added in #40351 breaks term counting when terms are applied to any object apart from the original post during the wp_insert_post() workflow.

An example may be a term is created and applied to another post, term, comment or user.

In this unit test a tag is created by wp_insert_post() which in turn applies a term to another object. This test passes with WordPress 5.5 but fails with 5.6 beta 1:

<?php
function test_wp_set_object_terms_hooked_to_create_term_in_wp_insert_post() {
        $args['update_count_callback'] = '_update_generic_term_count';
        register_taxonomy( 'term_taxonomy', 'term', $args );

        add_action( 'create_term', function( $term_id, $tt_id, $taxonomy ) {
                if ( 'post_tag' === $taxonomy ) {
                        $term = wp_insert_term( uniqid(), 'term_taxonomy' );
                        $lang = $term['term_id'];
                        wp_set_object_terms( $term_id, $lang, 'term_taxonomy' );
                }
        }, 10, 3 );

        $post_id = $this->factory->post->create( array( 'tags_input' => array( 'my_tag' ) ) );
        $tags = wp_get_post_tags( $post_id );
        $tag = reset( $tags );
        $terms = wp_get_object_terms( $tag->term_id, 'term_taxonomy' );
        $this->assertCount( 1, $terms ); // Check that a term has been assigned to the tag.
        $term = reset( $terms );
        $this->assertEquals( 1, $term->count ); // Check the term count, fails in WP 5.6.
}

Reported by and example test provided by @Chouby on earlier ticket ticket:40351#comment:42

Change History (7)

#1 @Chouby
5 weeks ago

Here is another test which is similar to the previous one but doesn't involve a custom taxonomy on terms.

In this test a category is assigned to a second post when a category is assigned to a first post in wp_insert_post(). The test passes in WP 5.5 and fails in WP 5.6-beta1:

<?php
function test_wp_set_object_terms_hooked_to_set_object_terms_in_wp_insert_post() {
        $p1 = $this->factory->post->create();

        $c1 = $this->factory->category->create();
        $c2 = $this->factory->category->create();

        add_action( 'set_object_terms', function() use ( $p1, $c1 ) {
                static $avoid_recursion = false;
                if ( ! $avoid_recursion ) {
                        $avoid_recursion = true;
                        wp_set_object_terms( $p1, $c1, 'category' );
                }
                $avoid_recursion = false;
        } );

        $p2 = $this->factory->post->create( array( 'post_category' => array( $c2 ) ) );
        $term = get_term( $c1 );
        $this->assertEquals( 1, $term->count ); // Check the term count, fails in WP 5.6.
}

A variant of the test above uses the same category for the 2 posts:

<?php
function test_wp_set_object_terms_hooked_to_set_same_object_terms_in_wp_insert_post() {
        $p1 = $this->factory->post->create();

        $c1 = $this->factory->category->create();

        add_action( 'set_object_terms', function() use ( $p1, $c1 ) {
                static $avoid_recursion = false;
                if ( ! $avoid_recursion ) {
                        $avoid_recursion = true;
                        wp_set_object_terms( $p1, $c1, 'category' );
                }
                $avoid_recursion = false;
        } );

        $p2 = $this->factory->post->create( array( 'post_category' => array( $c1 ) ) );
        $term = get_term( $c1 );
        $this->assertEquals( 2, $term->count ); // Check the term count, fails in WP 5.6.
}

This ticket was mentioned in PR #664 on WordPress/wordpress-develop by peterwilsoncc.


5 weeks ago

  • Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed

#3 @peterwilsoncc
5 weeks ago

@Chouby I love a report that includes clear unit tests, it's a great head start. Thank you.

I've created a pull request to work on this using tests similar to those in the comments above, I've used tags in place of categories so to take advantage of the shared fixtures.

At the moment it's tests only so, as expected, the tests are failing. If you think of any further tests, let me know and I will add them to the pull request.

#4 @prbot
4 weeks ago

Chouby commented on PR #664:

Could be a problem if a taxonomy is registered for two primitive object types, eg register_taxonomy( 'pwcc', [ 'post', 'user' ], $args ), but it's a little out of warranty as there is no way of knowing if the relationship refers to post 1, or user 1.

Indeed, we can't do that for the exact reason that you exposed. In Polylang I had this need (define a taxonomy 'language' for posts and terms). Due to the fact that we cannot tell if the relationship refers to posts or terms or anything else, I needed to register 2 taxonomies, one for posts, one for terms.

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


4 weeks ago

This ticket was mentioned in Slack in #core by peterwilsoncc. View the logs.


4 weeks ago

#7 @peterwilsoncc
4 weeks ago

  • Milestone 5.6 deleted
  • Resolution set to fixed
  • Status changed from new to closed

@Chouby I decided to revert the term counting changes as I wasn't sure enough the code in the linked pull request was problem free. At this point in the beta it's seemed smarter to stick with the existing code.

Closing this as fixed, as the revert in [49451] did the trick.

Note: See TracTickets for help on using tickets.