WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#30970 closed enhancement (fixed)

setup_postdata should be able to accept a Post ID, currently requires full WP Post object

Reported by: sc0ttkclark Owned by: boonebgorges
Milestone: 4.4 Priority: normal
Severity: normal Version: 1.5
Component: Posts, Post Types Keywords: needs-unit-tests has-patch
Focuses: Cc:

Description

You should have to get_post before calling setup_postdata, the function (and by extension $wp_query->setup_postdata) should be able to accept an ID.

Currently, setup_postdata() expects a full WP_Post (or faux standard post) object.

Something as easy as this could allow for this to function:

	public function setup_postdata( $post ) {
		global $id, $authordata, $currentday, $currentmonth, $page, $pages, $multipage, $more, $numpages;

		if ( ! is_object( $post ) ) {
			$post = get_post( $post );

			if ( ! $post ) {
				return false;
			}
		}

		$id = (int) $post->ID;

This also has a fallback with a return false, which sticks with the bool typed return already documented.

It solves cases where you may want to use WP template tags but all you have is a Post ID from another variable or from an option / transient / cached object.

This can allow for 3 additional tests to be added to test setup_postdata( $post_id ) will return true and setup the post, setup_postdata( $null_or_anything_invalid ) will return false, and setup_postdata( $post ) will return true and setup the post as normal with little to no overhead or ill effects.

Attachments (4)

30970.patch (835 bytes) - added by mordauk 3 years ago.
30970.2.patch (678 bytes) - added by mordauk 3 years ago.
30970-tests.patch (813 bytes) - added by mordauk 3 years ago.
30970.diff (2.1 KB) - added by wonderboymusic 3 years ago.

Download all attachments as: .zip

Change History (16)

#1 @sc0ttkclark
3 years ago

  • Summary changed from setup_postdata( $post_id ) should still work to setup_postdata should be able to accept a Post ID, currently requires full WP Post object

#2 @sc0ttkclark
3 years ago

I have found only one gotcha, which would be that get_post() will return the current global post object, if $post is empty.

This can be alleviated by:

	public function setup_postdata( $post ) {
		global $id, $authordata, $currentday, $currentmonth, $page, $pages, $multipage, $more, $numpages;

		if ( empty( $post ) ) {
			return false;
		} elseif ( ! is_object( $post ) ) {
			$post = get_post( $post );

			if ( ! $post ) {
				return false;
			}
		}

		$id = (int) $post->ID;

#3 @boonebgorges
3 years ago

  • Keywords needs-patch needs-unit-tests added

This seems reasonable. We should be able to simplify your suggested logic a bit: if ( ! ( $post instanceof WP_Post ) ) { $post = get_post( $post ); } will cover either an int or a faux-WP_Post object.

I'd like to see some unit tests for the various scenarios you've laid out (int, faux post, WP_Post; $post global populated vs not populated).

#4 @sc0ttkclark
3 years ago

I'll work up the unit tests, etc.

I think at the same time, what should be happening is the global $post be set from setup_postdata too, since setup_postdata sets all other globals, but leaves out the $post global itself.

I further propose we add a global $post set from the $post variable sent to setup_postdata at the end of the local $post setup in if ( ! ( $post instanceof WP_Post )

Last edited 3 years ago by sc0ttkclark (previous) (diff)

#5 @wonderboymusic
3 years ago

  • Milestone changed from Awaiting Review to Future Release

@mordauk
3 years ago

#6 @mordauk
3 years ago

  • Keywords has-patch added; needs-patch removed

30970.patch modifies setup_postdata() to support passing a post ID or post object.

#7 @mordauk
3 years ago

Oh, 30970.patch also changes it to setup the global $post var.

@mordauk
3 years ago

@mordauk
3 years ago

#8 @mordauk
3 years ago

Modifying the global $post was breaking tests and, in hindsight, is probably really dangerous, so 30970.2.patch is an updated version that does not modify $post.

30970-tests.patch includes some simple unit tests.

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


3 years ago

@wonderboymusic
3 years ago

#10 @wonderboymusic
3 years ago

30970.diff combines and cleans up the patches. I'm not really sure what the point of Tests_Query_SetupPostdata::test_fake_post() is...?

#11 @wonderboymusic
3 years ago

  • Milestone changed from Future Release to 4.4
  • Owner set to boonebgorges
  • Status changed from new to assigned

#12 @boonebgorges
3 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 34089:

Allow setup_postdata() to accept a post ID.

Previously, it accepted only a full post object.

Props sc0ttclark, mordauk, wonderboymusic.
Fixes #30970.

Note: See TracTickets for help on using tickets.