Make WordPress Core

Opened 5 years ago

Closed 3 years ago

Last modified 3 years ago

#48358 closed defect (bug) (fixed)

wp_get_post_parent_id() does not default to global $post

Reported by: danielpost's profile danielpost Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.9 Priority: normal
Severity: normal Version: 3.1
Component: Posts, Post Types Keywords: has-patch has-unit-tests commit has-dev-note
Focuses: Cc:

Description

https://core.trac.wordpress.org/browser/tags/5.2.4/src/wp-includes/post.php#L6595

Because wp_get_post_parent_id() does not declare a default value for the $post parameter, calling it without explicitly passing the $post parameter results in an error. This means that it will never default to the global $post. I believe the fix here is to declare $post = null in the function declaration and specify in the PHPDoc that this parameter is optional, similar to how other functions handle this.

Attachments (4)

48358.diff (728 bytes) - added by danielpost 5 years ago.
48358.1.diff (1.6 KB) - added by danielpost 5 years ago.
Adds test
48358.2.diff (1.6 KB) - added by audrasjb 3 years ago.
Posts, Post Types: Default wp_get_post_parent_id() to global $post
48358.3.diff (1.7 KB) - added by peterwilsoncc 3 years ago.

Download all attachments as: .zip

Change History (26)

@danielpost
5 years ago

#1 @danielpost
5 years ago

  • Version changed from 5.2.3 to 3.1

#2 @davidbaumwald
5 years ago

  • Keywords has-patch needs-unit-tests added

#3 @SergeyBiryukov
5 years ago

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

@danielpost
5 years ago

Adds test

#4 @danielpost
5 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

#5 @davidbaumwald
5 years ago

  • Keywords needs-dev-note added

This doesn't need a full dev-note, but it should get a small call-out in the miscellaneous changes note in 5.4.

#6 @birgire
5 years ago

The test in 48358.1.diff might need the annotation @ticket 48358. I also wonder if the function's doc would need the @since annotation after the signature change.

#7 @danielpost
5 years ago

I'm happy to make those changes if the consensus is that they're necessary—I don't have enough experience with core contributing to make that call myself.

#8 @davidbaumwald
5 years ago

  • Milestone changed from 5.4 to Future Release

With 5.4 Beta 3 approaching and the Beta period reserved for bugs introduced during the cycle, this is being moved to Future Release. If any maintainer or committer feels this should be included or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

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


3 years ago

#10 @SergeyBiryukov
3 years ago

  • Milestone changed from Future Release to 5.9

#11 @audrasjb
3 years ago

  • Keywords needs-refresh added

The patch doesn't apply against trunk anymore.

@audrasjb
3 years ago

Posts, Post Types: Default wp_get_post_parent_id() to global $post

#12 @audrasjb
3 years ago

  • Keywords needs-refresh removed

Refreshed in 48358.2.diff. I also added the ticket number in the provided tests.

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


3 years ago

audrasjb commented on PR #1898:


3 years ago
#15

The failing test looks like an unrelated issue.

hellofromtonya commented on PR #1898:


3 years ago
#16

@audrasjb It's a remote request to wp.org that has timed out. There's a ticket open to mock each of these HTTP remote requests https://core.trac.wordpress.org/ticket/54420. Adding this test to that ticket's list.

But you're right. This failing test is not associated with this PR.

#17 @hellofromTonya
3 years ago

  • Keywords commit added

PR 1898 is ready for commit.

audrasjb commented on PR #1898:


3 years ago
#18

Tests are passing now 😉

#19 @peterwilsoncc
3 years ago

In 48358.3.diff:

  • Functional code in PR as approved earlier
  • @covers and @since documentation added
  • @param ... $post description modified for consistency with the same use case elsewhere in core

I pushed the latter two items to @audrasjb's branch, presuming the tests pass I will commit momentarily.

#20 @peterwilsoncc
3 years ago

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

In 52194:

Posts, Post Types: Use global post as the default for wp_get_post_parent_id().

Convert the $post parameter of wp_get_post_parent_id() to optional, defaulting to the current global post object when called within the loop.

Props danielpost, davidbaumwald, SergeyBiryukov, birgire, audrasjb, hellofromTonya, TimothyBlynJacobs.
Fixes #48358.

#22 @audrasjb
3 years ago

  • Keywords has-dev-note added; needs-dev-note removed
Note: See TracTickets for help on using tickets.