#33045 closed enhancement (fixed)
New conditional tags for child/parent pages
Reported by: | ramiy | Owned by: | johnbillion |
---|---|---|---|
Milestone: | 5.7 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Posts, Post Types | Keywords: | has-patch has-unit-tests commit |
Focuses: | template | Cc: |
Description
New enhancements for theme developers with new conditional tags:
- has_parent() - return boolean result using
$post->post_parent
. - parent_page() - return the parent page id using
$post->post_parent
. - is_child_of( $id ) - return boolean result using
$post->post_parent
.
We can change the names, but we need the functionality.
Attachments (10)
Change History (66)
#2
@
9 years ago
For consistency, the patch is based on the code of has_excerpt()
and get_the_excerpt()
functions.
This ticket was mentioned in Slack in #core by chriscct7. View the logs.
9 years ago
#5
follow-up:
↓ 7
@
9 years ago
- Component changed from General to Posts, Post Types
- Keywords reporter-feedback 2nd-opinion added
@ramiy What's the use case for filtering the return value of get_parent()
?
Also these functions need post
in their name to avoid ambiguity. eg. wp_get_post_parent()
.
#7
in reply to:
↑ 5
@
9 years ago
- Keywords reporter-feedback removed
Replying to johnbillion:
@ramiy What's the use case for filtering the return value of
get_parent()
?
Also these functions need
post
in their name to avoid ambiguity. eg.wp_get_post_parent()
.
You are right, there is no reason to filter the returned value. @sebastian.pisula revised my patch, he removed the filter and added the required post
to the function name.
#9
follow-up:
↓ 10
@
9 years ago
Actually,
I don't agree with the new function name wp_get_post_parent()
,
I prefer it would be get_post_parent()
, without the wp_
prefix.
We should stick with the naming conventions in post-template.php
, non of the get
functions use the wp_
prefix:
- get_the_ID()
- get_the_title()
- get_the_guid()
- get_the_content()
- get_the_excerpt()
- get_post_class()
- get_body_class()
#10
in reply to:
↑ 9
@
9 years ago
Love these functions! Great idea.
I actually just wrapped up a project where these functions could have been super helpful if existed.
Also, I agree with @ramiy on the naming convention. IMHO, I think there shouldn't be a wp_
prefix for the proposed conditional tags.
#11
@
9 years ago
Same logic can be applied for the boolean function, rename back from has_post_parent()
to has_parent()
.
The other has_()
functions don't use the post
in their name:
- has_excerpt()
- has_tag()
- has_term()
- has_nav_menu()
Only one function use post
in the name:
- has_post_thumbnail()
This ticket was mentioned in Slack in #core by ramiy. View the logs.
9 years ago
This ticket was mentioned in Slack in #core by ramiy. View the logs.
7 years ago
#15
@
7 years ago
- Keywords 2nd-opinion removed
@SergeyBiryukov Let's add this enhancement to 4.9. The ticket has a patch, it was tested and got a very good feedback from @johnbillion and @sebastian.pisula.
The only issue here was the function names. In the last patch I applied the naming conventions set in post-template.php
to match similar template functions.
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
7 years ago
#18
@
7 years ago
I've decided these functions should be named get_parent_post()
and has_parent_post()
. The "parent post" is the object being gotten.
This also allows functions such as get_parent_term()
to be introduced in the future in order to fetch a term's parent.
#19
@
7 years ago
And just as I submitted that I questioned it. Maybe it needs to be get_parent_post_id()
. Naming things is hard.
#21
@
7 years ago
For reference, comment:28:ticket:24164 has a good overview of existing get_*()
and the_*()
template functions.
Until a new function is added, get_post_field( 'post_parent', $post )
could be used for the same purpose.
This ticket was mentioned in Slack in #core by desrosj. View the logs.
5 years ago
#26
@
5 years ago
In 33045.6.patch for the has_post_parent()
docs, there's currently:
* @return int
but it should be:
* @return bool
Regarding the naming, I would assume beforehand that get_post_parent()
would return the post parent object, just as get_post()
.
But beside from that, we already have this function in core:
/** * Returns the ID of the post's parent. * * @since 3.1.0 * * @param int|WP_Post $post Post ID or post object. Defaults to global $post. * @return int|false Post parent ID (which can be 0 if there is no parent), or false if the post does not exist. */ function wp_get_post_parent_id( $post ) { $post = get_post( $post ); if ( ! $post || is_wp_error( $post ) ) { return false; } return (int) $post->post_parent; }
See https://developer.wordpress.org/reference/functions/wp_get_post_parent_id/
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
5 years ago
#29
@
4 years ago
- Keywords needs-testing dev-feedback added
- Milestone changed from Future Release to 5.7
- Owner set to audrasjb
Moving for 5.7 consideration
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
4 years ago
#31
@
4 years ago
- Keywords 2nd-opinion added; needs-testing dev-feedback removed
- Status changed from reviewing to accepted
Hi there,
33045.7.diff
is a new iteration for this ticket:
@since
docs updateget_post_parent()
now returns parent post object- few DocBlock enhancements
#33
@
4 years ago
- Keywords dev-feedback added; 2nd-opinion removed
As Core Tech Lead for 5.7, @SergeyBiryukov, what's your feeling about those changes? I think it's pretty neat but feel free to take ownership on the ticket for proper review before the end of the alpha cycle ;)
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
4 years ago
#35
@
4 years ago
- Keywords needs-unit-tests added; dev-feedback removed
These need tests. The functions in the latest patch do not work as expected.
#36
follow-up:
↓ 37
@
4 years ago
I don't think the function names should have 'post' in them, because the standard Post is not hierarchical.
These functions don't seem very useful either, as we've gone more than 15 years without them just fine.
#37
in reply to:
↑ 36
@
4 years ago
Thank you @xkon, love the new checks you added, looks much better.
#38
@
4 years ago
Hey @ramiy, thanks but I've re-uploaded 33045.8.diff. It seems that they where not really needed as get_post()
takes care of it as it seems on it's own either way, so I reverted to the original :) with the minor fix mentioned below.
@audrasjb I took a look on 33045.7.diff and instead of returning $post->post_parent
it was returning $post
again in get_post_parent()
so it wasn't working as expected.
I'll try to work on unit tests as well to get this ready :). It should be good now though.
#39
@
4 years ago
- Keywords has-unit-tests added; needs-unit-tests removed
33045.9.diff adds unit tests, also renames the functions into has_parent_post()
& get_parent_post()
as mentioned by @johnbillion in comment 18 as it makes more sense.
I believe this is ready for a review if I didn't miss anything extra from the previous comments.
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
4 years ago
#41
@
4 years ago
- Keywords commit added
Thanks for the new patch and for adding unit tests.
I think we're ready for 5.7 beta 1 👌
#42
@
4 years ago
This looks good. 33045.diff just tweaks the docs and switches to using the post factory in the tests.
One remaining concern that I haven't seen mentioned yet is whether any existing plugins or themes already define either a get_parent_post()
or has_parent_post()
function. If they do, a fatal error will occur in 5.7.
I checked the plugin and theme directories and (amazingly?) only one plugin is affected,Revisionize with 6,000 active installations, and it defines the get_parent_post()
function. No themes are affected.
I think we can reach out to the plugin author and ask them to update the plugin with a function_exists()
check. If it's absolutely necessary we can ask the plugin team to push out an update with that change before 5.7.
Unless anybody disagrees, I'll commit this this evening.
#43
@
4 years ago
Good point @johnbillion. I just opened a pull request on their GitHub repository and a thread on their support forum :)
#46
follow-up:
↓ 49
@
4 years ago
Hey @johnbillion!
These functions are named weird. 😕
Your commit message uses post_parent()
but the functions that were committed use parent_post()
.
I’ve read all of the comments in this ticket, and see it’s gone back and forth until you put your foot down 3 years ago, but then your commit message hilariously inverted them, and my intuition tells me that post-parent is more natural than parent-post.
If left as-is, I think it is highly likely that everyone until the end of time is going to get these function names wrong in their code-editor auto-completes.
I.E. I would never think to type get_status_post
and I’ll never remember to type get_parent_post
.
Thoughts?
#47
@
4 years ago
- Keywords 2nd-opinion added; has-patch needs-dev-note needs-docs has-unit-tests commit removed
- Resolution fixed deleted
- Status changed from closed to reopened
Naming things is hard.
This ticket was mentioned in Slack in #core by sergey. View the logs.
4 years ago
#49
in reply to:
↑ 46
@
4 years ago
Replying to johnjamesjacoby:
I’ve read all of the comments in this ticket, and see it’s gone back and forth until you put your foot down 3 years ago, but then your commit message hilariously inverted them, and my intuition tells me that post-parent is more natural than parent-post.
I also went back and forth on this when reviewing the commit :) I guess get_post_parent()
, for consistency with the post_parent
field, would indeed be preferable here.
#50
@
4 years ago
This is a really hard call, but I also like the consistency with the post_parent
field. I'm on board with get_post_parent()
and has_post_parent()
.
This ticket was mentioned in PR #1017 on WordPress/wordpress-develop by peterwilsoncc.
4 years ago
#51
- Keywords has-patch has-unit-tests added
#52
@
4 years ago
I've set up a pull request renaming these to *_post_parent(). I'm happy with those as names too, click approve/request changes on the PR to express an opinion either way.
#53
@
4 years ago
- Keywords commit added; 2nd-opinion removed
- Status changed from reopened to reviewing
johnbillion commented on PR #1017:
4 years ago
#55
#56
@
4 years ago
Dev note has been updated: https://make.wordpress.org/core/2021/02/10/introducing-new-post-parent-related-functions-in-wordpress-5-7/
See the patch.