Make WordPress Core

Opened 12 years ago

Closed 9 years ago

Last modified 8 years ago

#19471 closed defect (bug) (maybelater)

Support use of custom post types in category and tag archives

Reported by: chrisjean's profile chrisjean Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.0
Component: Taxonomy Keywords: needs-patch
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 (3)

add-custom-post-types-to-core-taxonomy-archives.diff (421 bytes) - added by chrisbliss18 12 years ago.
test-core-archive-page-with-custom-post-type.php (636 bytes) - added by chrisbliss18 12 years ago.
19471.diff (486 bytes) - added by mdgl 9 years ago.
Refreshed/updated patch

Download all attachments as: .zip

Change History (46)

#1 @nacin
12 years ago

Automatically support custom post types in category and tag archives

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

#2 @nacin
12 years ago

  • Version changed from 3.3 to 3.0

#3 @chrisbliss18
12 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.

#4 follow-ups: @scribu
12 years ago

Isn't this what register_taxonomy_for_object_type() is for?

#5 in reply to: ↑ 4 @nacin
12 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 12 years ago by scribu (previous) (diff)

#6 in reply to: ↑ 4 @chrisbliss18
12 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.

#7 @scribu
12 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 12 years ago by scribu (previous) (diff)

#8 follow-up: @DrewAPicture
12 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 12 years ago by DrewAPicture (previous) (diff)

#9 in reply to: ↑ 8 ; follow-up: @chrisbliss18
12 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.

#10 in reply to: ↑ 9 ; follow-up: @helenyhou
12 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/

#11 in reply to: ↑ 10 @chrisbliss18
12 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.

#12 @DrewAPicture
12 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.

#13 @wycks
12 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.

#14 @sirzooro
12 years ago

  • Cc sirzooro added

#15 follow-up: @scribu
12 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

#16 in reply to: ↑ 15 ; follow-up: @chrisbliss18
12 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 more 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.

Last edited 12 years ago by chrisbliss18 (previous) (diff)

#17 in reply to: ↑ 16 ; follow-up: @scribu
12 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().

#18 in reply to: ↑ 17 @chrisbliss18
12 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.

#19 @scribu
12 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.

#20 @scribu
12 years ago

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

#21 follow-up: @wonderboymusic
12 years 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.

#22 in reply to: ↑ 21 @mdgl
11 years 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.

#23 follow-up: @wonderboymusic
11 years 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.

#24 in reply to: ↑ 23 @mdgl
11 years 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...

#25 @knutsp
11 years ago

  • Cc knut@… added

#26 follow-up: @wonderboymusic
11 years 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

#27 in reply to: ↑ 26 @mdgl
11 years 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.

#28 @knutsp
11 years 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.

#29 @nacin
11 years 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.

#30 @knutsp
11 years 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?

#31 @helen
11 years ago

  • Resolution set to maybelater
  • Status changed from reopened to closed

@mdgl
9 years ago

Refreshed/updated patch

#32 @mdgl
9 years ago

  • Resolution maybelater deleted
  • Status changed from closed to reopened

I think this is worth another look. There have been many changes here in the past couple of years, in particular by Andrew Nacin in response to #21290 and released in WordPress 3.5, which takes the fix approach suggested by Silviu-Cristian Burcă (see comment:7).

Accordingly, I have updated Chris Jean's patch and done a fair amount of testing and code review. Function get_posts() is critical and consists of 1263 lines of code, so we need to be careful!

Patch Details

With this patch, category and tag queries are treated in the same way as other taxonomies. If a post type is not specified explicitly, we default to a list of all of the searchable post types registered against the taxonomy.

As far as I can see, the only behaviour change is that for category and tag queries that have no post type specified, the post type is now set to the default explicitly in the block dealing with taxonomies at around line 2705 instead of within the block dealing with the corresponding SQL "where" clause at around line 2933. Apart from the conversion of a singular array to a string at line 2900, there are no other uses of the post type variables between these two locations. The generated SQL both before and after the patch appears to be identical.

Benefits

This patch means that custom post types can now make use of the standard category and tag taxonomies if they wish, including appearing in the corresponding archives. I suspect this is quite a common use case where custom post types are used to handle different kinds of content but taxonomies such as category and tag are used to organise all site content.

Note that the developer/user must explicitly declare that the custom post type make use of the category or post_tag taxonomies, either through the taxonomies parameter to register_post_type() or through register_taxonomy_for_object_type(). Furthermore, the developer/user must also declare that the custom post type is searchable by setting the exclude_from_search flag to false on registration (see below for concerns about the overloading of this flag).

If the developer/user does not explicitly take these actions, their custom post types will not be included within category or tag archives. The use of other taxonomies is not affected. The behaviour of custom post types within search queries is also not affected.

The behaviour of custom post types is now consistent with that specified in the Codex (http://codex.wordpress.org/Function_Reference/register_post_type) which does mention the use of categories and tags.

Concerns/Issues

Some concerns were raised earlier by WPCOM about a different but related fix (see comment:15), as follows:

[15649] breaks existing behavior by setting post_type to 'any' instead of defaulting to 'post' when a taxonomy is specified but post_type is not. We're already being bitten by this on VIP sites on .com.

This is now no longer the case. If taxonomy is specified but post type is not, we default to whatever searchable post types are registered for that taxonomy. In the case of categories and tags this is (by default) just 'post'.

The use case mentioned by Scott Taylor (comment:21) seems best addressed by proper use of the exclude_from_search flag when registering the custom post type.

The outstanding issue about the multiple different uses of the exclude_from_search flag and the need for a more granular approach is logged in #29418.

#33 @SergeyBiryukov
9 years ago

  • Keywords reporter-feedback removed
  • Milestone set to Awaiting Review

#34 @mdgl
9 years ago

  • Summary changed from Automatically support custom post types in category and tag archives to Support use of custom post types in category and tag archives

Title/summary updated because perhaps worth clarifying that we are not proposing that custom post types be automatically included in category and tag archives, but only if the developer/user explicitly requests this behaviour by registering the post types with the appropriate taxonomies.

#35 @mdgl
9 years ago

  • Keywords has-patch added

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


9 years ago

#37 follow-up: @jorbin
9 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to maybelater
  • Status changed from reopened to closed

The above patch is still to automatic. Despite being a bug to many, this has been the default behavior for a number of years and any change to the default output would end up being a break in backwards compatibility.

The solution, as Nacin alludes to in comment:20 is to bake this into register_taxonomy_for_object_type. An additional solution would be needed for author pages.

There are work arounds available for this, so it doesn't need to be high priority. If someone can produce a quality patch that doesn't break existing sites, we can (and should) revisit this.

#38 in reply to: ↑ 37 ; follow-ups: @mdgl
9 years ago

Replying to jorbin:

The above patch is still to automatic. Despite being a bug to many, this has been the default behavior for a number of years and any change to the default output would end up being a break in backwards compatibility.

How would the proposed patch break backwards compatibility? As far as I can see, the patch will do nothing unless the developer has explicitly declared that the category or post_tag be associated with a custom post type. Why would the developer do that? What behaviour is he/she expecting?

As it stands, if you do make use of the "standard" taxonomies in this way, nothing will happen because by default queries will never find any matching custom post types. Why would the developer want that behaviour (i.e. associate the taxonomy with a post type but not have any of those post types returned on a query for that taxonomy)?

The solution, as Nacin alludes to in comment:20 is to bake this into register_taxonomy_for_object_type. An additional solution would be needed for author pages.

I don't understand. Bake what exactly into register_taxonomy_for_object_type()?

#39 in reply to: ↑ 38 @jorbin
9 years ago

comment:21 details one such use case of having a custom post type that is a part of a default taxonomy where you don't want to have it be a part of taxonomy archive pages. The behavior that developer is expecting is the behavior that has been in place for 3+ years. They aren't expecting a change in that behavior.

I would look at adding a third argument to register_taxonomy_for_object_type that takes an array and includes a paramater such as "include_object_in_default_taxonomy_archive" that could be used to trigger to the default taxonomies that additional objects should be included in the archive queries.

#40 in reply to: ↑ 38 @johnbillion
9 years ago

  • Component changed from Query to Taxonomy
  • Keywords needs-patch added; has-patch removed
  • Milestone set to Awaiting Review

Replying to mdgl:

As it stands, if you do make use of the "standard" taxonomies in this way, nothing will happen because by default queries will never find any matching custom post types. Why would the developer want that behaviour (i.e. associate the taxonomy with a post type but not have any of those post types returned on a query for that taxonomy)?

Incidentally, I'm working on a client project currently which requires exactly this. Several custom post types which use categories and tags but which, by default, do not show up on the categories and tags archive pages. A change like this would break the behaviour of the site.

Replying to jorbin:

I would look at adding a third argument to register_taxonomy_for_object_type that takes an array and includes a paramater such as "include_object_in_default_taxonomy_archive" that could be used to trigger to the default taxonomies that additional objects should be included in the archive queries.

This is probably the best idea. It could be added as an argument in register_taxonomy() too.

If someone wants to patch this, please re-open.

#41 @johnbillion
9 years ago

  • Milestone Awaiting Review deleted

#42 @DrewAPicture
9 years ago

#32701 was marked as a duplicate.

#43 @swissspidy
8 years ago

#36192 was marked as a duplicate.

Note: See TracTickets for help on using tickets.