WordPress.org

Make WordPress Core

Opened 2 months ago

Closed 5 weeks ago

#52187 closed enhancement (fixed)

Create function to resolve a post date from post_date and post_date_gmt strings

Reported by: jmdodd Owned by: pento
Milestone: 5.7 Priority: normal
Severity: normal Version:
Component: Posts, Post Types Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

This change breaks out the code to resolve a post date from passed post_date and post_date_gmt strings. This will allow use of the new function in other post type-specific inserts, such as found in wp_update_nav_menu_item.

By doing so, imported nav menu items can start to retain the same post date as found in their export file, improving performance of post_exists checks on existing nav menu items.

Attachments (6)

52187.diff (2.6 KB) - added by jmdodd 2 months ago.
52187.1.diff (1.8 KB) - added by jmdodd 2 months ago.
Unit tests demonstrating current wp_insert_post post_date behavior in core.
52187.2.diff (5.2 KB) - added by jmdodd 2 months ago.
Updated patch with requested changes and unit tests.
52187.3.diff (9.5 KB) - added by jmdodd 8 weeks ago.
Updated patch with improved test coverage of wp_insert_post.
52187.4.diff (10.9 KB) - added by jmdodd 7 weeks ago.
Updated patch with improved test coverage of wp_insert_post and complete coverage of wp_resolve_post_date.
52187.5.diff (11.2 KB) - added by pento 5 weeks ago.

Download all attachments as: .zip

Change History (16)

@jmdodd
2 months ago

#1 @jorbin
2 months ago

  • Keywords needs-unit-tests added

Love this idea. I think automated tests for the new resolve_post_date function would be very helpful. It's also not obvious at first glance what is is meant by Resolve the mysql-formatted post date from provided date and date GMT strings. I wonder if a longer description might help. Perhaps:

"Uses wp_checkdate to verify that the date passed in is a valid Gregorian-calendar date string. If no date is given, first attempt to use date_gmt otherwise fallback to useing the current time."

#2 @jmdodd
2 months ago

I'll work on the function description and add unit tests. Thank you!

#3 @SergeyBiryukov
2 months ago

Should the new function have a wp_ prefix, e.g. wp_resolve_post_date(), to avoid any potential conflicts?

#4 @jmdodd
2 months ago

While testing this, I found an interesting behavior in wp_insert_post -- an empty post_date and an invalid post_date_gmt (ex. "2020-12-41 10:11:41") will result in an inserted post with a post_date of "1970-01-01 00:00:00" rather than a failure because get_date_from_gmt will always return a date.

resolve_post_date (soon to be wp_resolve_post_date) doesn't change this behavior, but unit tests surfaced it as one that was already present in 5.6.

$ wp shell
wp> wp_insert_post( array( 'post_date' => '', 'post_date_gmt' => '2020-12-41 10:10:10', 'post_title' => 'Test', 'post_content' => 'Test content.', 'post_status' => 'publish' ) );
=> int(11)
wp> 
$ wp post get 11
+-----------------------+--------------------------------------+
| Field                 | Value                                |
+-----------------------+--------------------------------------+
| ID                    | 11                                   |
| post_author           | 0                                    |
| post_date             | 1970-01-01 00:00:00                  |
| post_date_gmt         | 0000-00-00 00:00:00                  |
| post_content          | Test content.                        |
| post_title            | Test                                 |
| post_excerpt          |                                      |
| post_status           | publish                              |
| comment_status        | open                                 |
| ping_status           | open                                 |
| post_password         |                                      |
| post_name             | test                                 |
| to_ping               |                                      |
| pinged                |                                      |
| post_modified         | 1970-01-01 00:00:00                  |
| post_modified_gmt     | 0000-00-00 00:00:00                  |
| post_content_filtered |                                      |
| post_parent           | 0                                    |
| guid                  | http://wp.localhost/1970/01/01/test/ |
| menu_order            | 0                                    |
| post_type             | post                                 |
| post_mime_type        |                                      |
| comment_count         | 0                                    |
+-----------------------+--------------------------------------+
$ wp core version
5.6

Fixing this behavior would be a breaking change, as posts that would previously have been inserted with post_date "1970-01-01 00:00:00" would instead fail because of the invalid passed post_date_gmt.

@jmdodd
2 months ago

Unit tests demonstrating current wp_insert_post post_date behavior in core.

#5 @SergeyBiryukov
2 months ago

  • Milestone changed from Awaiting Review to 5.7

Related: #52189

@jmdodd
2 months ago

Updated patch with requested changes and unit tests.

@jmdodd
8 weeks ago

Updated patch with improved test coverage of wp_insert_post.

#6 @jmdodd
8 weeks ago

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

@jmdodd
7 weeks ago

Updated patch with improved test coverage of wp_insert_post and complete coverage of wp_resolve_post_date.

#7 @jmdodd
7 weeks ago

In 82187.4.diff, I've included unit tests to cover existing behavior in wp_insert_post, as well as the output of wp_resolve_post_date. The new wp_insert_post tests should work without the patched file wp-includes/post.php to demonstrate existing behavior that should remain unaffected by the introduction of wp_resolve_post_date.

#8 @pento
5 weeks ago

  • Owner set to pento
  • Status changed from new to reviewing

@pento
5 weeks ago

#9 @pento
5 weeks ago

  • Status changed from reviewing to accepted
  • Version trunk deleted

Thank you for the masses of unit tests, @jmdodd, that's always something I love to see on patches. 🙂

This is a fairly straightforward change, I'm happy with it as is. attachment:52187.5.diff has a few minor formatting tweaks, but is otherwise good to go.

#10 @pento
5 weeks ago

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

In 50012:

Posts: Create a new function for resolving the post date.

wp_insert_post() has a few checks using post_date and post_date_gmt, to determine the correct post date. This functionality is now extracted out into a new wp_resolve_post_date() function, allowing the checks to be reused elsewhere.

Props jmdodd.
Fixes #52187.

Note: See TracTickets for help on using tickets.