WordPress.org

Make WordPress Core

Opened 20 months ago

Closed 9 months ago

#21660 closed enhancement (duplicate)

'p' arg should work with custom post types

Reported by: batmoo Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Query Keywords:
Focuses: Cc:

Description

Doing a query like following, where 42 is a custom post_type, will fail:

$query = new WP_Query( array( 'p' => 42 ) );

The resulting query looks like:

SELECT wp_posts.* FROM wp_posts  WHERE 1=1  AND wp_posts.ID = 42 AND wp_posts.post_type = 'post'  ORDER BY wp_posts.post_date DESC

This is because WP_Query forces the post_type arg to 'post' when it's not set. We should not force it when the p arg is set.

Change History (22)

comment:1 batmoo20 months ago

The workaround is to add 'post_type' => 'any' to the query, but WP_Query should handle this for us.

comment:2 follow-up: scribu20 months ago

If you do that, then pages will also become accessible through an URL like ?p=123.

comment:3 in reply to: ↑ 2 batmoo20 months ago

Replying to scribu:

If you do that, then pages will also become accessible through an URL like ?p=123.

We can canonically redirect those? Or maybe we should introduce a new arg: post_id?

comment:4 dd3220 months ago

I would argue that 'p' is a query for the Post post_type, same as 'page_id' is a query for Pages. We also have post__in but that accepts an array of Post post_id's only. Adding a specific post_id query var (a private one I assume for use only from within code) seems to be suggesting one should be using get_post( $post_id );.

I'd expect that example to be written something like this, which is a lot more portable, and should work exactly as you're intending

$query = new WP_Query( array( 'cpt_query_var' => 'cpt_post_slug' ) );

comment:5 joshkadis20 months ago

As for the original question, neither p nor page_id will work for a custom post type, correct?

I'd argue that if you're looking for one specific post object and know its ID, you should be able to find it without having to specify any other parameters like post_type or page_id (which is essentially the same as having to specify p=[ID]&post_type=page)

comment:6 wonderboymusic20 months ago

  • Keywords dev-feedback added

comment:7 follow-up: wonderboymusic18 months ago

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

when you don't specify post_type, 'post' has long been the default, and that's how it's documented everywhere, so I don't think this is a great idea

comment:8 danielbachhuber18 months ago

  • Resolution wontfix deleted
  • Status changed from closed to reopened
  • Type changed from defect (bug) to enhancement

I think it's a valid enhancement. From my perspective, creating a new WP_Query object from an explicit post ID is more like using get_post() than get_posts(). The former accepts any post ID, regardless of post type. Posts, Pages, and Custom Post Types are all posts in the grand scheme of things.

comment:9 SergeyBiryukov18 months ago

  • Milestone set to Awaiting Review

comment:10 in reply to: ↑ 7 ; follow-up: joshkadis18 months ago

Replying to wonderboymusic:

when you don't specify post_type, 'post' has long been the default,

Sure, but doesn't that date back to WordPress as a "traditional" blog CMS? Does it reflect how users and developers are using WordPress today? Personally, the themes that I use and write tend to make use of custom post types as a core part of functionality.

and that's how it's documented everywhere, so I don't think this is a great idea


Even if some documentation escaped without being updated, a query like p=[ID]&post_type=custom_post_type would still work, no?

comment:11 in reply to: ↑ 10 scribu18 months ago

  • Keywords dev-feedback removed

Replying to joshkadis:

Personally, the themes that I use and write tend to make use of custom post types as a core part of functionality.

Yes, and those CPTs have pretty permalinks, so why do you need ?p=123 ?

Last edited 18 months ago by scribu (previous) (diff)

comment:12 follow-up: MikeSchinkel18 months ago

  • Cc mike@… added

What is the downside of supporting this which seems to be a pretty reasonable request?

Are they examples where it will be backward incompatible? If not, why is "because we've always done it that way" a good justification for not enabling a behaviour that someone not knowing the past would simple assume would work?

comment:13 in reply to: ↑ 12 ; follow-up: scribu18 months ago

Replying to MikeSchinkel:

Are they [sic] examples where it will be backward incompatible?

Yes: #comment:2

comment:14 in reply to: ↑ 13 MikeSchinkel18 months ago

Replying to scribu:

Replying to MikeSchinkel:

Are they [sic] examples where it will be backward incompatible?

Yes: #comment:2

That's does not appear to be [sic] backward incompatible in terms of breaking a common use-case, which is what my question implied.

Your example makes something work that previously did not, and since it previously did not work there won't be an example of someone using it where a new behavior will break their expected behavior, right?

comment:15 follow-up: scribu18 months ago

The expected behaviour is that ?p=<id of page> will return a 404. If we want to avoid a SEO pitchfork mob, we have to implement canonical redirects, make sure it doesn't return revisions etc. so it's not such a trivial change.

At which point, you have to ask if the effort is worth it. So, use cases and patches please.

comment:16 in reply to: ↑ 15 ; follow-up: MikeSchinkel18 months ago

Replying to scribu:

The expected behaviour is that ?p=<id of page> will return a 404. If we want to avoid a SEO pitchfork mob, we have to implement canonical redirects, make sure it doesn't return revisions etc. so it's not such a trivial change.

Under what cases are URLs of that form going to exist on the web? Give me a good example and I'll acquiesce.

At which point, you have to ask if the effort is worth it. So, use cases and patches please.

I didn't propose it nor do I need it. It just seems to me there's frequently push back from selected people on what seems very reasonable requests.

comment:17 in reply to: ↑ 16 ; follow-up: scribu18 months ago

Replying to MikeSchinkel:

Replying to scribu:

The expected behaviour is that ?p=<id of page> will return a 404.

Under what cases are URLs of that form going to exist on the web? Give me a good example and I'll acquiesce.

Ok, it's not a back-compat problem, but it is a problem.

I didn't propose it nor do I need it. It just seems to me there's frequently push back from selected people on what seems very reasonable requests.

Yes, the argument "it's been this way for a long time, so let's not change it" does come up sometimes and it is itself a reasonable reaction, given the lack of unit tests. If you're not confident that nothing breaks, you're going to be afraid of making changes.

comment:18 in reply to: ↑ 17 MikeSchinkel18 months ago

Replying to scribu:

Ok, it's not a back-compat problem, but it is a problem.

I'm still not seeing where this would be a problem except in very edge cases.

Yes, the argument "it's been this way for a long time, so let's not change it" does come up sometimes and it is itself a reasonable reaction, given the lack of unit tests. If you're not confident that nothing breaks, you're going to be afraid of making changes.

Agreed that is it a consideration, and depending on the nature of the change it can be an overriding concern. Moving to an MVC model, for example would be a non-starter. But I too often see inertia being given as the reason not to do something when it appears there are not other reasons where there may otherwise be good reasons to consider the improvement.

comment:19 follow-up: batmoo18 months ago

One possible use case for this: /%post_id%/%name%/ for all permalink URLs (custom post type or not)? (e.g. on http://qz.com)

comment:20 in reply to: ↑ 19 ; follow-up: MikeSchinkel18 months ago

Replying to batmoo:

One possible use case for this: /%post_id%/%name%/ for all permalink URLs (custom post type or not)? (e.g. on http://qz.com)

Argh! I argued on behalf of this ticket only to now realize it will enable what I think is a really bad practice from a site usability perspective.

I guess no good deeds go unpunished. :(

comment:21 in reply to: ↑ 20 joshkadis18 months ago

Replying to MikeSchinkel:

Argh! I argued on behalf of this ticket only to now realize it will enable what I think is a really bad practice from a site usability perspective.

Emphasis mine. Do tell.

Regardless, what constitutes good or bad UX design should be up to individual designers and devs to decide what's best for their own readers. If WordPress didn't let you build whatever kind of crazy, crappy website you want, it wouldn't be WordPress. :)

Last edited 18 months ago by joshkadis (previous) (diff)

comment:22 wonderboymusic9 months ago

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

We should probably do this - but dupe of #17551

Note: See TracTickets for help on using tickets.