Prevent unexpected missed schedules when generating new posts#418
Conversation
| * 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 ) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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...
There was a problem hiding this comment.
@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...
I'm not sure how to reproduce it locally. Do you have any ideas? 🤔
There was a problem hiding this comment.
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:
I think this is the specific failure:
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:
I then manually tried to install WordPress 3.7, but received this MySQL error:
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`
There was a problem hiding this comment.
Thank you @danielbachhuber. I think I was able to fix the edge cases on WP 3.7 and I have requested a new review.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
@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.
wp_insert_post behavior for post_date
danielbachhuber
left a comment
There was a problem hiding this comment.
Nice work on this, @rafaelzaleski 👏
This PR uses an empty string as the default value for the
--post_dateand--post_date_gmtparameters ofwp generate posts.This will align with the behavior of
wp_insert_postand use the current time for the generated posts when a date is not provided. If a date is provided, it's passed down towp_insert_post.Closes #268
Related wp-cli/wp-cli#5832