Link categories filter doesn't show all categories

[ocean90]

From #18983.

• Add a new category
• Add a new link to this new category
• Go to all links
  • You will see the link, but you can't filter the list to show only links from the new category

[ocean90]

Replying to SergeyBiryukov:

Replying to SergeyBiryukov:

On my install the dropdown is currently empty though, which is weird.

And on Link Categories screen, all link counts are zero. Even though there are still 7 default links in the Blogroll category.

Screenshot: ​http://cl.ly/B3w8

[ocean90]

Seems like the count won't be updated.

Related ticket #17548 ?

[Ipstenu]

See also

Of note: BOTH 'family' and 'my other sites' existed before any of this started. I added in a new category 'this is a test link cat' and applied it to 'Ask Ipstenu' and it would show up in the Categories column, but not the dropdown.

[SergeyBiryukov]

Broken in [18932]. Links are not posts (yet).

[nacin]

As it is possible to attach taxonomies elswhere, this should probably be:

• if ( attachment )
• elseif ( get_post_type_object( $object_type) )
• else this generic query

[ryan]

In [19025]:

Fix term counting for links. Props SergeyBiryukov. fixes #18986

[SergeyBiryukov]

Since $taxonomy->object_type can be an array, is it possible for a taxonomy to be attached to both a registered post type and some other object? Do we need to take that into account here (perhaps looping over $object_types)? 18986.3.patch​ illustrates what I'm thinking of.

[duck_]

Replying to SergeyBiryukov:

Since $taxonomy->object_type can be an array, is it possible for a taxonomy to be attached to both a registered post type and some other object? Do we need to take that into account here (perhaps looping over $object_types)? 18986.3.patch​ illustrates what I'm thinking of.

I think there would be a problem with 18986.3.patch​. The term count is updated on every loop iteration so after the function call then it would only equal the number of relationships for the last object type.

I do think there might be a problem here, but haven't tested it yet and not super familiar with the problem being solved. In [19025] we check post_type_exists( $object_type ) and then seem to assume all the objects are post types and implode them for post_type IN. I assume that $object_type is left over from the loop through $object_types and so is the last object type. What if one of the other ones is a link?

[SergeyBiryukov]

Replying to duck_:

In [19025] we check post_type_exists( $object_type ) and then seem to assume all the objects are post types and implode them for post_type IN. I assume that $object_type is left over from the loop through $object_types and so is the last object type. What if one of the other ones is a link?

Yeah, that's the exact problem I meant. Need to test this more thoroughly.

[ryan]

In [19031]:

Typo fix. Props SergeyBiryukov. see #18986

[nacin]

Perhaps we can loop through the object types to sort them into one of the three buckets (links, attachments, other post types), then run whatever queries are necessary.

[nacin]

18986.diff​ takes a step back and approaches this from a different perspective.

Essentially, link_category should never be using _update_post_term_count(). In 3.2, it was using the generic handler, which was inline. In 3.3, all taxonomies by default now use _update_post_term_count(). This includes not only link_category, but all custom taxonomies. This will change how custom taxonomies attached to posts are counted, assuming they don't specify an update_count_callback. I always forget to, so perhaps this would normally be for the better for developers in general, but it is technically a backwards incompatible change that will make counts drop pretty quickly.

(Reference thus far: [18783/trunk/wp-includes/taxonomy.php], via #17548.)

So I think what we need to do is create a new generic handler, _update_generic_term_count(). This does restore the original behavior that was changed in #17548, but it doesn't do so at the expense of all other taxonomies.

Note that the patch makes this handler the default callback. I don't know if that's the best idea, given the comments in #17548. So it sounds like maybe we need to do some detection on $object_types when choosing our default callback. If it's nothing but post types, then use _update_post_term_count(), otherwise, we'll need to use the generic handler.

[nacin]

18986.2.diff​ implements a conditional when choosing which "generic" handler to use. It carefully deals with attachment:* object types (1) and then checks to see if *all* object types are indeed a post type. If so, it goes with _update_post_term_count(). If not, it goes with the generic handler.

(Very rarely will a taxonomy include object types of both posts and non-posts. In this case the developer is going to certainly implement their own callback.)

(1) - Note, I think the simpler handling of object types with a colon in _update_post_term_count() is fine, given that these are only supposed to be post types.

[nacin]

duck_ pointed out that, of course, array_search() needs to be checked against false in case the attachments index is 0. Slight tweak removes the boolean cast and does a cheeky $check_attachments = false !== $check_attachments; to cast it to a boolean.

[nacin]

s/generic/object/ would also be a good idea.

[nacin]

Sergey, I thought about the approach in 18986.4.patch​ as well, but the moment that else gets triggered along with either the if or the elseif, the counts get out of whack, as it isn't bound like the previous queries and a lot gets double-counted.

Granted, this situation will occur infrequently (as I argued above), but I think it would be better if we split these into two callbacks. (Especially since someone's custom taxonomy may wish to leverage the existing generic callback.)

[nacin]

Final thought, I think.

The handler-choosing logic in [18986.3.diff] fails to account for the backwards compatibility implications that all custom taxonomies were previously used to counting every post, regardless of publish status.

Should the handler-choosing logic only decide to use the post_term_count callback if the only object types are pages, posts, and attachments, and defer to the generic handler the moment a custom post type is involved? That may be the safest approach, as who knows how the CPT is implemented -- the CT counting all items, rather than just published ones, might make sense in many situations.

I think the answer to my question is that custom taxonomies should use the generic_term_count callback (as had occurred in 3.2) except for when the taxonomy is only attached to one or more of posts/pages/attachments and no custom post types, in which case post_term_count is a safe bet.

[nacin]

18986.5.diff​ implements comment 17. Now, custom taxonomies tied to custom post types fall back to the generic handler. They use the post_term handler only when attached to post/page/attachment. This is for compatibility reasons.

It also fixes a parse error, as well as logic and parameter inversions in the use of array_filter() (by replacing that line all together).

[SergeyBiryukov]

Replying to nacin:

Granted, this situation will occur infrequently (as I argued above), but I think it would be better if we split these into two callbacks.

Yeah, you're right. I've attached it just in case, to show what I had in mind and failed to implement in 18986.3.patch.

[nacin]

18986.6.diff​ takes a step back from .5 and provides for post status checking for custom taxonomies tied to custom post types.

[nacin]

In [19035]:

Separate non-post term counting from _update_post_term_count(). Introduce _update_generic_term_count(). The generic handler will be the default whenever the taxonomy is attached to an object type other than a post type (link, user). Otherwise the _update_post_term_count() handler will be the default. fixes #18986. see #17548.

[nacin]

In [19036]:

No need to specify an update_count_callback for categories or tags, as the default handler for them will now be _update_post_term_count(). see #18986, [19035].

[nacin]

In [19037]:

Update the update_count_callback docs. see #18986.

[SergeyBiryukov]

Replying to nacin:

In [19037]:

Update the update_count_callback docs. see #18986.

Probably don't need a second "then" (see 18986.minor-typo-fix.2.patch​).

[nacin]

In [19065]:

Remove duplicate word in docs. props SergeyBiryukov, fixes #18986.