-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Add revisioning of post meta, including ‘footnotes’ by default #4859
Conversation
… last stored revision
@adamsilverstein, should we try using |
Sure, good idea - I'll give it a try |
src/wp-includes/rest-api/endpoints/class-wp-rest-autosaves-controller.php
Outdated
Show resolved
Hide resolved
Pinging @swissspidy as this change may effect the web stories plugin that has a custom revision / autosave endpoint. |
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 tested this with the web stories plugin that has many custom post types and meta. Seems to work well. Just some little bits of feedback and we should be nearly ready to commit.
…troller.php Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
…troller.php Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
Co-authored-by: Pascal Birchler <pascal.birchler@gmail.com>
Co-authored-by: Pascal Birchler <pascal.birchler@gmail.com>
$revision_id = _wp_put_post_revision( $post_data, true ); | ||
} | ||
|
||
if( is_wp_error( $revision_id ) ) { |
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.
@spacedmonkey thinking we should check for 0 here as well since _wp_put_post_revision
can return 0.
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.
How about this?
if( 0 === $revision_id ) {
return new WP_Error(...
}
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.
$revision_id could be 0 OR a wp_error here so I think we need to check for both which is what I did.
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
Do we need to update |
src/wp-includes/rest-api/endpoints/class-wp-rest-autosaves-controller.php
Outdated
Show resolved
Hide resolved
…troller.php Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
Not sure, I'm going to get this committed then we can revisit during beta. |
src/wp-includes/rest-api/endpoints/class-wp-rest-autosaves-controller.php
Show resolved
Hide resolved
// Restore any revisioned meta fields. | ||
wp_restore_post_revision_meta( $post_id, $revision['ID'] ); |
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.
// Restore any revisioned meta fields. | |
wp_restore_post_revision_meta( $post_id, $revision['ID'] ); |
@adamsilverstein This is not need anymore, as it hooked in the next line.
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.
👍🏼 will commit in a follow up
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.
Good to commit. We will follow up with any issues.
Trac ticket: https://core.trac.wordpress.org/ticket/20564
Related Gutenberg Ticket: WordPress/gutenberg#52988
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.