Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Prevent unexpected missed schedules when generating new posts #418

Merged
merged 11 commits into from Sep 18, 2023
Merged

Prevent unexpected missed schedules when generating new posts #418

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
6596e92
Use default wp_insert_post behavior on post_date
rafaelzaleski Aug 24, 2023
5e2aea5
Add test case
rafaelzaleski Aug 24, 2023
c28bedf
Fix hyphenated dates
rafaelzaleski Aug 24, 2023
e529043
Fix coding style
rafaelzaleski Aug 24, 2023
f6d53df
Fix cs errors
rafaelzaleski Aug 25, 2023
83b9db9
Assign a value to the post date if it is empty.
rafaelzaleski Sep 14, 2023
26e64cb
Replace isset for empty in value checks
rafaelzaleski Sep 14, 2023
2446ecd
Add fix for only date.
rafaelzaleski Sep 14, 2023
0040988
Add time to date if empty
rafaelzaleski Sep 14, 2023
5a61aeb
Add comments and fix PHP CS
rafaelzaleski Sep 14, 2023
0c1a185
Remove whitespace EOL
rafaelzaleski Sep 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 14 additions & 0 deletions features/post-generate.feature
View file
Open in desktop
Expand Up @@ -172,3 +172,17 @@ Feature: Generate new WordPress posts
"""
2000-01-01 02:11:00
"""

Scenario: Generating posts when the site timezone is ahead of UTC
When I run `wp option update timezone_string "Europe/Helsinki"`
And I run `wp post delete 1 --force`

When I run `wp post list --field=post_status`
Then STDOUT should be empty

When I run `wp post generate --count=1`
And I run `wp post list --field=post_status`
Then STDOUT should be:
"""
publish
"""
37 changes: 24 additions & 13 deletions src/Post_Command.php
View file
Open in desktop
Expand Up @@ -702,10 +702,10 @@ function( $post ) {
* ---
*
* [--post_date=<yyyy-mm-dd-hh-ii-ss>]
* : The date of the generated posts. Default: current date
* : The date of the post. Default is the current time.
*
* [--post_date_gmt=<yyyy-mm-dd-hh-ii-ss>]
* : The GMT date of the generated posts. Default: value of post_date (or current date if it's not set)
* : The date of the post in the GMT timezone. Default is the value of --post_date.
*
* [--post_content]
* : If set, the command reads the post_content from STDIN.
Expand Down Expand Up @@ -753,23 +753,16 @@ public function generate( $args, $assoc_args ) {
'post_type' => 'post',
'post_status' => 'publish',
'post_author' => false,
'post_date' => false,
'post_date_gmt' => false,
'post_date' => '',
'post_date_gmt' => '',
'post_content' => '',
'post_title' => '',
];

$post_data = array_merge( $defaults, $assoc_args );

$call_time = current_time( 'mysql' );

if ( false === $post_data['post_date_gmt'] ) {
$post_data['post_date_gmt'] = $post_data['post_date'] ?: $call_time;
}

if ( false === $post_data['post_date'] ) {
$post_data['post_date'] = $post_data['post_date_gmt'] ?: $call_time;
}
$post_data['post_date'] = $this->maybe_convert_hyphenated_date_format( $post_data['post_date'] );
$post_data['post_date_gmt'] = $this->maybe_convert_hyphenated_date_format( $post_data['post_date_gmt'] );

if ( ! post_type_exists( $post_data['post_type'] ) ) {
WP_CLI::error( "'{$post_data['post_type']}' is not a registered post type." );
Expand Down Expand Up @@ -1006,4 +999,22 @@ public function exists( $args ) {
WP_CLI::halt( 1 );
}
}

/**
* Convert a date-time string with a hyphen separator to a space separator.
*
* @param string $date_string The date-time string to convert.
* @return string The converted date-time string.
*
* Example:
* maybe_convert_hyphenated_date_format( "2018-07-05-17:17:17" );
* Returns: "2018-07-05 17:17:17"
*/
private function maybe_convert_hyphenated_date_format( $date_string ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a new behavior? Can you share more detail about it? It seems like a new behavior to me, and a different change than the original issue describes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it's actually done to ensure compatibility with previous versions. The docs advises using a datetime string with a hyphen between the date and time. If we don't remove this hyphen before calling wp_insert_post, the timestamp will be saved as 0.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rafaelzaleski Ok, great. Can you include some additional tests to capture this behavior then?

Also, it looks like your test introduces one break in a different test...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rafaelzaleski Ok, great. Can you include some additional tests to capture this behavior then?

It's already here and here.

Also, it looks like your test introduces one break in a different test...

I'm not sure how to reproduce it locally. Do you have any ideas? 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how to reproduce it locally. Do you have any ideas?

I tried to get tests running against PHP 5.6 locally and failed 😕

First, I tried this act tool. It was able to run the unit tests but not the functional tests:

CleanShot 2023-09-08 at 07 29 06@2x

I think this is the specific failure:

CleanShot 2023-09-08 at 07 29 36@2x

Next, I tried to install PHP 5.6 directly on my machine:

brew install shivammathur/php/php@5.6
brew link --overwrite --force shivammathur/php/php@5.6

Something is funky with the tests, though:

CleanShot 2023-09-08 at 07 31 33@2x

I then manually tried to install WordPress 3.7, but received this MySQL error:

CleanShot 2023-09-08 at 07 34 56@2x

My guess is that something changed in the behavior of wp_insert_post() between WP 3.7 and today.

Based on the failed test, it looks like your code changes the result of --post_date="2018-07-01"`` from 2018-07-01 00:00:00to1970-01-01 00:00:00`

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @danielbachhuber. I think I was able to fix the edge cases on WP 3.7 and I have requested a new review.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I was able to fix the edge cases on WP 3.7

@rafaelzaleski Out of curiosity, how did you end up debugging/fixing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rafaelzaleski Out of curiosity, how did you end up debugging/fixing this?

I encountered some challenges installing 3.7 locally too, mainly because of database issues.

So I went through the failed test data and reviewed the wp_insert_post source code from tag 3.7. This helped me understand the nuances of the test data processing and the root cause of the failures.
To ensure the solutions were working, I validated them using CI checks on my cloned repository.

// Check if the date string matches the format with the hyphen between date and time.
if ( preg_match( '/^(\d{4}-\d{2}-\d{2})-(\d{2}:\d{2}:\d{2})$/', $date_string, $matches ) ) {
return $matches[1] . ' ' . $matches[2];
}
return $date_string;
}
}