WordPress.org

Make WordPress Core

Opened 6 years ago

Last modified 5 months ago

#29418 reviewing defect (bug)

Taxonomy archive query not including all of its post types.

Reported by: msaggiorato Owned by: SergeyBiryukov
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.9.2
Component: Taxonomy Keywords: needs-testing dev-feedback
Focuses: Cc:

Description

I've noticed this in a project i'm working on.

I did some digging in wp-includes/query.php file, and found that this is the issue.

In 3.9.2 files, in line 2501 there's this

foreach ( get_post_types( array( 'exclude_from_search' => false ) ) as $pt ) {

Not sure if this is intended or not, maybe yes, but the query var name is missleading in this case, which is also excluding the post type from its taxonomies archives too.

In any case, if this is to be fixed, and in fact is not supposed to be here, just changing the line for this should work, as we should expect.

foreach ( get_post_types( array() ) as $pt ) {

Hope this helps, thanks.

Attachments (1)

29418.diff (758 bytes) - added by Howdy_McGee 9 months ago.
Exclude Search Removed

Download all attachments as: .zip

Change History (14)

#1 @SergeyBiryukov
6 years ago

  • Summary changed from Taxonomy archive query not incluiding all of its post types. to Taxonomy archive query not including all of its post types.

#2 @boonebgorges
6 years ago

  • Keywords 2nd-opinion added
  • Milestone changed from Awaiting Review to Future Release

The behavior was introduced in [21855]. See #21290.

exclude_from_search is, as you'd guess, intended to exclude items from showing up in search results. However, it's also used in a number of places through WP_Query in a more general way to prevent posts from a given post type to appear in results, and this is one of those instances. The naming convention here is quite poor, and it points toward the need for a different parameter. (I would've thought this was what 'publicly_queryable' was for, but that's not actually used in WP_Query at all.)

In any case, unfortunately I don't think we can pull up all post types here as you suggest. I think that'll cause problems for sites that are depending on the current behavior. Better would be to introduce a new param that inherits its value from 'public' or even from 'exclude_from_search' and means something like "show in archives".

#3 @msaggiorato
6 years ago

I like the "show in archives" approach, could be a good "fix" for this.

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


6 years ago

#6 @helgatheviking
5 years ago

However, it's also used in a number of places through WP_Query in a more general way to prevent posts from a given post type to appear in results, and this is one of those instances.

Seems like a mistake that exclude_from_search would mean anything other than excluding from search. Couldn't the other places that use it be converted to publically_queryable, since that sounds like it means the CPT should show up in any public queries? or failing that a new parameter... show_in_archives seems alright, if redundant.

After spending the whole morning where the heck my CPTs were in my taxonomy archive, I'd take anything.

#7 @boonebgorges
5 years ago

  • Keywords needs-patch added; 2nd-opinion removed

#8 @knutsp
5 years ago

I agree with @helgatheviking. This is a mess, and it has to change.

#9 @DoodleDogCody
5 years ago

One thing not mentioned in a few of these "exclude_from_search" templates that I have stumbled upon is that if we set it to true and then try a custom loop for the CPT with a taxonomy term in a tax query. We can only get the 1st page of results. So using something like the following only returns results for the first page, while the rest of the pages return a 404. So if at the end of our loop we have some pagination, the pagination knows how many pages there are but when going to those pages it just stops.

I agree with the previous posters that something here really needs to change. 'exclude_from_search' should do just that and exclude_from_search while another parameter 'publically_queryable' or some new parameter is used here. Not wanting search results to return this CPT but wanting to have taxonomy archives pages seems like something that might happen a lot and all the answers to accomplishing this seem like big workarounds.

    $paged = ( get_query_var('paged') ) ? get_query_var('paged') : 1;
    $taxonomy = get_query_var( 'taxonomy' );
    $term = get_query_var( 'term' );

    $args = array(
      'post_type' 		=> 'ddc-products',
      'posts_per_page' 	=> 5,
      'paged'				=> $paged,
      'tax_query' => array(
        array(
          'taxonomy' => $taxonomy,
          'field'    => 'slug',
          'terms'    => $term,
        ),
      ),
    );

@Howdy_McGee
9 months ago

Exclude Search Removed

#10 @Howdy_McGee
9 months ago

  • Keywords needs-testing dev-feedback added; needs-patch removed

Here's how I see it. At this point (LN 2135) in the get_posts method we're simply looking for post types matching the current taxonomy, that's all. We should be including all post types not just searchable ones. It doesn't really matter for searchable types anyway. Whenever a search query is added it sets the post_type to 'any' which then goes out and grabs all searchable post type as seen in lines 1847 -> 1854 -> 2409.

Removing the exclude_from_search flag solves this issue, continues to keep the post type unsearchable, and doesn't undo the original patches work.

Adding additional flags to fix this bug would end up causing more confusion than necessary. Maybe this change would be easier to apply with a major update so there's plenty of notice prior and testing to be done.

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


6 months ago

#12 @SergeyBiryukov
6 months ago

  • Milestone set to Awaiting Review
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

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


5 months ago

Note: See TracTickets for help on using tickets.