Opened 8 years ago
Closed 8 years ago
#37059 closed enhancement (fixed)
Allow post to be passed to get_the_modified_time & get_the_modified_date
Reported by: | peterwilsoncc | Owned by: | peterwilsoncc |
---|---|---|---|
Milestone: | 4.6 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Posts, Post Types | Keywords: | good-first-bug has-patch has-unit-tests |
Focuses: | Cc: |
Description (last modified by )
The signatures of get_the_time and get_the_modified_time differ. Let's make them match
The current signatures are:
get_the_time ( string $d = '', int|WP_Post $post = null )
get_the_modified_time ( string $d = '' )
get_the_date ( string $d = '', int|WP_Post $post = null )
get_the_modified_date ( string $d = '' )
~
Note: added the date functions per @Soean's comment below.
Attachments (9)
Change History (30)
#1
@
8 years ago
- Component changed from General to Posts, Post Types
- Description modified (diff)
- Keywords good-first-bug needs-patch needs-unit-tests added
- Milestone changed from Awaiting Review to Future Release
#4
@
8 years ago
- Keywords has-patch added; needs-patch removed
Same difference in the date functions.
get_the_date( $d = '', $post = null ) get_the_modified_date($d = '')
#5
@
8 years ago
The patch added in the documentation that was added, as well as changing the null value to be false.
#6
@
8 years ago
- Keywords has-unit-tests added; needs-unit-tests removed
- Owner set to peterwilsoncc
- Status changed from new to reviewing
You have your patch, sir :)
#7
@
8 years ago
- Description modified (diff)
- Summary changed from Allow post to be passed to get_the_modified_time to Allow post to be passed to get_the_modified_time & get_the_modified_date
#8
@
8 years ago
- Keywords needs-refresh added
Thanks for the patches @Soean and @lukecavanagh.
Just a few notes to get the patch commit ready. I'm basing them off the latest patch, 37059.3.diff
This looks like a lot but it's all really minor (and I'm verbose).
In the source code:
- In the docs:
- add a second
@since
tag noting the new argument (similar to thelanguage_attributes
function in the same file). - align the
@param
variables and descriptions.
- add a second
- Add the
get_post
& check for a valid post similar to the check inget_the_time
- The filters in the functions we're aiming to match also accept the post as an argument, let's match them too (noting the update in the docs).
- There's a couple of coding standards (whitespace) fixes required in
get_the_modified_date
's first call toget_post_modified_time
- [edit] all the code inside the if...else statements is changing, so it's a good opportunity to add braces to meet the coding standards.
In the tests:
- Coding standards (whitespace) the closing brace of the first new function onwards are incorrectly indented.
- Add some documentation for each function, including:
- explanation
- since tags
- this ticket number
...with_post_id
is clearer for people scanning the code.
As I say, all quite minor.
Pete
#9
@
8 years ago
Thanks @peterwilsoncc for your great review.
This is my first patch and your feedback helps me a lot.
So I hope I get all your points in 37059.4.diff .
#11
@
8 years ago
- Milestone changed from Future Release to 4.6
@Soean: Thanks for the update. Here are some further notes on 37059.4.diff:
- Let's add backticks around
$post
in the changelog entries for both functions. The should also be structured as sentences, albeit short:* @since 4.6.0 The `$post` parameter was added.
- We also need changelog entries for the new parameters added to the two filters
- Also, at the point where
$post
is passed to the filters, it's only aWP_Post
object
#12
@
8 years ago
Thanks @DrewAPicture for your notes, I updated the patch and uploaded 37059.5.diff
#14
@
8 years ago
Thanks for the refreshes @Soean, I'll commit this Friday Australian time and props you and @lukecavanagh.
This is my first patch and your feedback helps me a lot.
Looking forward to many more, I'm glad Drew and I could help :)
#16
follow-up:
↓ 20
@
8 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
I think that in get_the_modified_date() this code
$post = get_post( $post ); if ( ! $post ) { return false; }
is unnecessary because in get_post_modified_time check valid $variable and function get_post_modified_time() is called always
This ticket was mentioned in Slack in #core by peterwilsoncc. View the logs.
8 years ago
#18
@
8 years ago
- Keywords commit removed
Post commit [37738] @azaozz and I had a brief chat in Slack re backcompat.
Previously, if called before the global $post
object was set, the value false
would be run through the filter. Now, if the post is called before the global $post
object is set, the function returns false and bypasses the filter.
37059.2.diff restores the old behavior of passing a failure to get the post though the filter.
I'm genuinely undecided on how much it matters, the old behavior could only have occurred before the posts_selection
action.
#19
@
8 years ago
In 37059.6.diff:
- maintains back-compat by passing failures to get a post through filters for both
get_the_modified_date
andget_the_modified_time
- adds unit test to ensure failures are filtered
- added tests for
get_modified_date
to match those forget_modified_time
.
#20
in reply to:
↑ 16
@
8 years ago
Replying to sebastian.pisula:
I think that in get_the_modified_date() this code
$post = get_post( $post ); if ( ! $post ) { return false; }is unnecessary because in get_post_modified_time check valid $variable and function get_post_modified_time() is called always
I'm going to keep the ! $post
check in both get_the_modified_date
and get_the_modified_time
. You're correct, get_post_modified_time
does the same check but when the outcome of a function call is easily premepted it can be bypassed.
Let me work on a patch.