WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 7 months ago

#19471 closed defect (bug) (maybelater)

Automatically support custom post types in category and tag archives

Reported by: chrisjean Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.0
Component: Query Keywords: reporter-feedback
Focuses: Cc:

Description

I dealt with a plugin recently that had the following code in it:

<?php
add_filter('pre_get_posts', 'query_post_type');
function query_post_type($query) {
    if(is_category() || is_tag()) {
        $post_type = get_query_var('post_type');
        if($post_type)
            $post_type = $post_type;
        else
            $post_type = array('post','external-videos');
        $query->set('post_type',$post_type);
        return $query;
    }
}
?>

The goal was simply to allow their custom post type that was connected to category and tag taxonomies to have its entries rendered on category and tag archive pages. Digging around, it seems that many people are using a solution like this.

Of course, the code has problems in that it misses home pages and will mess with all other queries on the affected page (such as menus). Looking further in the thread, it seems people tried fixing up these limitations with code such as the following:

<?php
add_filter('pre_get_posts', 'query_post_type');
function query_post_type($query) {
  if(is_category() || is_tag() || is_home() && empty( $query->query_vars['suppress_filters'] ) ) {
    $post_type = get_query_var('post_type');
        if($post_type)
            $post_type = $post_type;
        else
            $post_type = array('post','articles','nav_menu_item');
    $query->set('post_type',$post_type);
        return $query;
    }
}
?>

While this may address the known issues, I don't see it as a solid solution in any way. Here are just couple of the primary issues I see with such a solution:

  • Things are very likely to change in the future (such as the possibility of adding additional core taxonomies or query modifications), causing the ever-increasing number of plugins that use custom post types to constantly have bugs.
  • Since the WP_Query->set function replaces values rather than merging them, even the "fixed" version of the code will cause plugin conflicts as multiple instances of this solution will systematically undo the modification done by each previous instance. Sure, the code could be improved to cover this, but the odds of getting everyone to fix their code are not good.

To me, this is something that was an oversight in the development of custom post types. I can think of no reason why this should not be automatically handled. The dashboard properly shows post counts for categories and tags that include the custom post types, but the query simply fails to handle this. It needs to be fixed.

Testing things out, I noticed that custom taxonomies did not have this problem at all. Looking at wp-includes/query.php, I saw the following:

<?php
if ( $this->is_tax ) { 
    if ( empty($post_type) ) { 
        $post_type = 'any';
        $post_status_join = true;
    } elseif ( in_array('attachment', (array) $post_type) ) { 
        $post_status_join = true;
    }   
}   
?>

This is what allows custom taxonomy archives to work properly without having to filter the query. I've always felt that is_tax should cover categories and tags as well, but what is done is done. So, I propose that this section is modified to include a check for is_category and is_tag. Example:

<?php
if ( $this->is_tax || $this->is_category || $this->is_tag ) { 
    if ( empty($post_type) ) { 
        $post_type = 'any';
        $post_status_join = true;
    } elseif ( in_array('attachment', (array) $post_type) ) { 
        $post_status_join = true;
    }   
}   
?>

This allows for things to "just work" as people would expect. Of course, plugins would have to remove their filter in order to avoid plugin conflicts as the post_type query arg would still be set and will not be changed automatically to "any". This is as it should be though, so people will just have to be convinced over time to remove the modification from their plugins.

Attached is the proposed patch.

Attachments (2)

add-custom-post-types-to-core-taxonomy-archives.diff (421 bytes) - added by chrisbliss18 2 years ago.
test-core-archive-page-with-custom-post-type.php (636 bytes) - added by chrisbliss18 2 years ago.

Download all attachments as: .zip

Change History (33)

comment:1 nacin2 years ago

Automatically support custom post types in category and tag archives

I was under the impression we had done this in 3.1.

comment:2 nacin2 years ago

  • Version changed from 3.3 to 3.0

comment:3 chrisbliss182 years ago

It's possible that it was fixed at some point (I haven't tested for regressions).

I'm attaching a simple plugin based on the Codex custom post type example. It creates a post type named "Product" and adds it to both the Category and Tag taxonomies.

Simply create an entry, and add it to at least one Category and at least one Tag. Then visit the used Category and Tag archive pages to see that the entry is not listed. Applying the patch addresses the issue.

comment:4 follow-ups: scribu2 years ago

Isn't this what register_taxonomy_for_object_type() is for?

comment:5 in reply to: ↑ 4 nacin2 years ago

Replying to scribu:

Isn't this what register_taxonomy_for_object_type() is for?

#14589 — It was your change (over my objections).

Last edited 2 years ago by scribu (previous) (diff)

comment:6 in reply to: ↑ 4 chrisbliss182 years ago

Replying to scribu:

Isn't this what register_taxonomy_for_object_type() is for?

register_taxonomy_for_object_type only does what the object_type arg of the register_taxonomy function does except for register_taxonomy_for_object_type is additive.

I have no doubt that this was supposed to work, but as you can see from using the attached plugin, it doesn't as the query only checks is_tax.

comment:7 scribu2 years ago

Replying to nacin:

Replying to scribu:

Isn't this what register_taxonomy_for_object_type() is for?

#14589 — It was your change (over my objections).

Yeah, I remember having something to do with it, but it was reverted along the way. There was probably a good reason for it, but neither I nor nacin can remember what it was.

Anyway, instead of using 'any', we could collect all the post types which have the current taxonomy in their 'taxonomies' property.

Since you would need to call register_taxonomy_for_object_type() explicitly, this can be safely used for 'category' and 'post_tag' as well, not just for custom taxonomies.

Last edited 2 years ago by scribu (previous) (diff)

comment:8 follow-up: DrewAPicture2 years ago

I can confirm this problem in 3.2.1. We employed a similar filter to accommodate a CPT on archives pages. We also fixed the 'breaking menus problem', by adding nav_menu_item to the $post_type array, e.g.:

	else
	    $post_type = array('post','our_cpt', 'nav_menu_item'); 
    $query->set('post_type', $post_type);
	return $query;
Last edited 2 years ago by DrewAPicture (previous) (diff)

comment:9 in reply to: ↑ 8 ; follow-up: chrisbliss182 years ago

Replying to DrewAPicture:

I can confirm this problem in 3.2.1. We employed a similar filter to accommodate a CPT on archives pages. We also fixed the 'breaking menus problem', by adding nav_menu_item to the $post_type array...

That really only avoids the main issue. The primary problem is that some queries should not be modified. Simply forcing nav_menu_item into every query is not a proper solution.

I sent the following to the plugin author as a temporary fix until WP core updates:

<?php
add_filter( 'pre_get_posts', 'sp_ev_filter_query_post_type' );
function sp_ev_filter_query_post_type( $query ) {
    if ( $query->query_vars['suppress_filters'] || ( ! is_category() && ! is_tag() ) )
        return $query;
   
    $post_type = get_query_var( 'post_type' );
   
    if ( 'any' == $post_type )
        return $query;
   
    if ( empty( $post_type ) ) {
        $post_type = 'any';
    }
    else {
        if ( ! is_array( $post_type ) )
            $post_type = array( $post_type );
       
        $post_type[] = 'external-videos';
    }
   
    $query->set( 'post_type', $post_type );
   
    return $query;
}
?>

By no means is this rock-solid, but it's much better than what was there before. It attempts to apply a fix similar to the one provided by this patch.

Key changes are respecting the suppress_filters var, using the 'any' post_type, bailing out when no modifications are needed, and adding to the post_type list rather than replacing the current value or values.

comment:10 in reply to: ↑ 9 ; follow-up: helenyhou2 years ago

Replying to chrisbliss18:

Key changes are respecting the suppress_filters var

http://wpdevel.wordpress.com/2011/12/07/new-api-in-3-3-is_main_query/

comment:11 in reply to: ↑ 10 chrisbliss182 years ago

Replying to helenyhou:

Replying to chrisbliss18:

Key changes are respecting the suppress_filters var

http://wpdevel.wordpress.com/2011/12/07/new-api-in-3-3-is_main_query/

Very nice. I don't know how I missed this. Thanks Helen.

comment:12 DrewAPicture2 years ago

@chrisbliss18 I am in no way suggesting that our hacky fix is the way to go at all. I was merely pointing out that's what we had to do to get it working at the time. I'm all for fixing this problem in core once and for all, so kudos for taking the initiative and pushing the issue to the forefront here.

comment:13 wycks2 years ago

  • Cc wycks added

I remember correctly is that it was mentioned as fixed somewhere (changelog?) around 3.01, but then reverted, or possibly it worked for a release or two and then was changed. It's still rather confusing.

comment:14 sirzooro2 years ago

  • Cc sirzooro added

comment:15 follow-up: scribu2 years ago

  • Keywords has-patch removed

Found the revert: [16505]

And the reason, as always is back-compat: http://core.trac.wordpress.org/ticket/12891#comment:138

comment:16 in reply to: ↑ 15 ; follow-up: chrisbliss182 years ago

Replying to scribu:

Found the revert: [16505]

And the reason, as always is back-compat: http://core.trac.wordpress.org/ticket/12891#comment:138

The code reverted in [16505] was different than the patch I provided and has much far-reaching consequences.

Is the solution offered by my patch appropriate or would it be better to generate a list of post types as you recommend above? I would think that the only difference between the use of "any" and a specifically-generated list of post types given the final query would be a possible query performance difference.

If neither is sufficient, what process would fix this up so that WordPress core is handling all the details?

I just don't want to see this flaw present for another year. Requiring such complex sections of code to be present in plugins to work around limitations in the API is a poor solution.

Version 0, edited 2 years ago by chrisbliss18 (next)

comment:17 in reply to: ↑ 16 ; follow-up: scribu2 years ago

Replying to chrisbliss18:

The code reverted in [16505] was different than the patch I provided and has more far-reaching consequences.

The code was different, but the intent was clear: don't set the post type to 'any' when categories and tags are involved, because this creates problems on some installs. So no, your patch is not appropriate.

I would think that the only difference between the use of "any" and a specifically-generated list of post types given the final query would be a possible query performance difference.

The other difference is that it wouldn't affect sites that expect only 'post' to be set. As I said before, you would have to explicitly enable it, either when defining your CPT or via register_taxonomy_for_object_type().

comment:18 in reply to: ↑ 17 chrisbliss182 years ago

Replying to scribu:

Replying to chrisbliss18:

The code reverted in [16505] was different than the patch I provided and has more far-reaching consequences.

The code was different, but the intent was clear: don't set the post type to 'any' when categories and tags are involved, because this creates problems on some installs. So no, your patch is not appropriate.

Can you give more details about these problems? Without knowing all the details, I don't know how it will be possible to create a patch that can avoid these landmines.

I would think that the only difference between the use of "any" and a specifically-generated list of post types given the final query would be a possible query performance difference.

The other difference is that it wouldn't affect sites that expect only 'post' to be set. As I said before, you would have to explicitly enable it, either when defining your CPT or via register_taxonomy_for_object_type().

The "any" gets parsed into an array of types. Manually creating an array of post types results in an array of types. In both scenarios, an array of types would not be equivalent to just "post". So, I don't see any difference. Again, give details please.

comment:19 scribu2 years ago

I agree that array( 'post' ) is not the same as 'post', something that most plugins don't account for.

Unfortunately, I don't have those details. I think the best course of action would be to just write the patch and then ask either ryan or chrisscott to test it out.

comment:20 scribu2 years ago

We could just be super-careful: if the resulting array is array( 'post' ), just set the post type to ''.

comment:21 follow-up: wonderboymusic18 months ago

  • Keywords reporter-feedback added

Supporting a taxonomy in a post type does not mean that the post should appear on archive / tax archive pages

I create custom post types that are really "modules" (not posts) that can be attached to tags so I know which sidebars to dynamically render them on, but I have no intention of them ever appearing in archives.

Kicking this back to the reporter to provide a patch and use case that requires this fix.

comment:22 in reply to: ↑ 21 mdgl17 months ago

Replying to wonderboymusic:

Supporting a taxonomy in a post type does not mean that the post should appear on archive / tax archive pages

Are you kidding? Surely if I declare a new *public* post type and explicitly register it with a common taxonomy such as "category" or "post_tag" then that's exactly what I am declaring! In this case, the new post type should appear in all of the registered taxonomy archives.

Your use case of "modules" sounds more like a *private* post type or your own new (and hence to some extent "private") taxonomy, perhaps both.

I think this is worth another look early in 3.6. Quite a lot has changed since the ticket was opened and as chrisbliss18 points out in comment:16 and comment:18, the reverted patch [16505] was far more risky than what I believe is actually required to fix this issue (even if WP.com are "doing it wrong" :-). We do need a brand new patch, however.

comment:23 follow-up: wonderboymusic17 months ago

Surprisingly, no, I was not kidding. I think the reason no one has looked at this ticket in 11 months may have something to do with the premise. To quote from the Bible (markjaquith.wordpress.com):

If you want it to show up in your site’s main RSS feed, then it’s probably not a custom post type.

RSS Feed and archives being interchangeable here.

comment:24 in reply to: ↑ 23 mdgl17 months ago

Replying to wonderboymusic:

Surprisingly, no, I was not kidding. I think the reason no one has looked at this ticket in 11 months may have something to do with the premise. To quote from the Bible (markjaquith.wordpress.com):

If you want it to show up in your site’s main RSS feed, then it’s probably not a custom post type.

RSS Feed and archives being interchangeable here.

Oh, I see! But the key word in Mark's post is *main*, referring to the RSS feed commonly available at example.com/feed/ and example.com/comments/feed.

Although these are commonly labelled as the "site feed" and "site comments feed", in fact they are really just the "blog feed" and "blog comments feed" or more accurately "post_type=post feed" and "post_type=post comments feed".

You can argue that this definition is too narrow in these days of custom post types, but the fact is these feeds have always been restricted to "post_type=post" as they have never (for example and to my knowledge) included pages or attachments. Hence the case can be made to not extend these feeds to include other (custom) post types.

Separately, however we have archives and feeds for all sorts of things: date, author, category, tag, taxonomy and custom post type archives as well as search results and comments on search results. It is here where things get a bit more problematic and inconsistent!

Search results (and comments on search results) for example seem to include all post types, as do taxonomy archives/feeds (I believe - for those post types against which they are registered). The archives/feeds for date, author, category and tag are different in that they only seem to apply to posts of type "post".

Maybe we can argue that date archives should only apply to the blog (post_type=post) because this forms the only explicitly time-ordered series of posts. I would suggest, however that the more obvious behaviour from the user's perspective would be to included *all* (public) content within the specified date range (an exceptional argument could be made in the case of pages :-).

But why should author archives not include custom post types that have declared themselves to support "author" (see add_post_type_support())? And why should category and tag behave differently to other taxonomies? If I explicitly register the taxonomies "category" and "post_tag" against a particular post type, why shouldn't my instructions be respected?

If you are saying that such archives should definitely *not* contain custom post types, then at the very least we should remove/disallow the "supports => author" attribute for custom post types and return an error if anybody tries to register the taxonomies "category" and "post_tag" against custom post types.

Anything else is just adding to the inconsistency and confusion.

P.S. I don't regard inactivity on a ticket as any sign of lack of interest, importance or concern. As you can see from the comments from nacin and others, this "feature" was apparently and intentionally made to work at one time. It regressed due to some unspecified concerns over at WP.com.

The sad fact is that there are hundreds if not thousands of important tickets that are in some way found to be "difficult" (often architectural in nature or having potential impact on themes/plugins) and so tend to get dropped by developers keen to work on easier items such as the front-end tools. Unfortunately, these tickets really *need* to be addressed at some point if WP is not to degenerate into a completely unmaintainable mess...

comment:25 knutsp17 months ago

  • Cc knut@… added

comment:26 follow-up: wonderboymusic8 months ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

There is code that will opt-in post types to tags and categories, turning that on automatically for all post types is not an option

comment:27 in reply to: ↑ 26 mdgl8 months ago

Replying to wonderboymusic:

I think you are misreading this ticket.

There is code that will opt-in post types to tags and categories, ...

No there isn't. Unless this has changed recently elsewhere, if you explicitly attach post_tag or category to a custom post type (e.g. via register_taxonomy(), those posts will not appear in the respective taxonomy archives.

... turning that on automatically for all post types is not an option

That is not what the ticket is suggesting.

comment:28 knutsp8 months ago

If I register cats and dogs as public custom post types, and then have them share a foo taxonomy, I would certainly expect the foo taxonomy archive to list both species. That's about the whole point of a shared taxonomy. Same with category and post_tag.

If this is not going to be the default behaviour, then please provide some robust code for how to make this happen, generally.

I saw this ticket as a way to make things consistent, predictable and intuitive, maturing WordPress as a CMS and as an application platform, perfect for the scope of 3.7.

comment:29 nacin7 months ago

I wouldn't mind revisiting this at some point in the future. Perhaps there is something we can do at the register_taxonomy() step.

comment:30 knutsp7 months ago

  • Resolution wontfix deleted
  • Status changed from closed to reopened

I will look forward to a patch that will make it possible to opt-in for taxonomies to contain custom post types, or for custom post types to be included shared taxonomies archives, making core queries more consistent and behave more common sense with respect to this issue.

This is closed as wontfix. Will it be ok to have this ticket open for Awaiting Review or Future Release, or closed as maybelater resolution for now?

comment:31 helen7 months ago

  • Resolution set to maybelater
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.