-
Notifications
You must be signed in to change notification settings - Fork 3.1k
wp_update_post overwrites post_date when updating post_status #9699
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.
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
base: trunk
Are you sure you want to change the base?
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN:
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a few notes inline.
@peterwilsoncc as a rule of thumb, in a bug, I don't like to add tests that cannot be proven with and without the patch (like in TDD). But I've taken advantage of the |
1735afb
to
47c1ae6
Compare
For this reviewed patch I've considered these elements:
I've taken as the fix: https://core.trac.wordpress.org/attachment/ticket/62468/fix-wp-update-post.patch
It was unnecesary to preserving date of
publish
andprivate
ondraft/pending
posts. Onlyfuture
is relevant in this scenarioFor the unit tests I've taken this one by @Sukhendu2002, removing all the extra AI fluff. The data provider feels a bit overkill but I think it will don't do any harm.
Finally, given that here is a good opportunity to open a specific file to cover
wp_update_posts
I've brought all the tests covering this function and merged them all into this file for Unit Tests organizational purposes.Trac ticket: https://core.trac.wordpress.org/ticket/62468
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.