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

Editor: Move auto-draft status change to the server. #16814

Merged
merged 1 commit into from
Aug 1, 2019
Merged

Editor: Move auto-draft status change to the server. #16814

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions lib/compat.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,32 @@ function gutenberg_safe_style_css_column_flex_basis( $attr ) {
return $attr;
}
add_filter( 'safe_style_css', 'gutenberg_safe_style_css_column_flex_basis' );

/**
* Filters inserted post data to unset any auto-draft assigned post title. The status
* of an auto-draft should be read from its `post_status` and not inferred via its
* title. A post with an explicit title should be created with draft status, not
* with auto-draft status. It will also update an existing post's status to draft if
* currently an auto-draft. This is intended to ensure that a post which is
* explicitly updated should no longer be subject to auto-draft purge.
*
* @see https://core.trac.wordpress.org/ticket/43316#comment:88
* @see https://core.trac.wordpress.org/ticket/43316#comment:89
*
* @param array $data An array of slashed post data.
* @param array $postarr An array of sanitized, but otherwise unmodified post
* data.
*
* @return array Filtered post data.
*/
function gutenberg_filter_wp_insert_post_data( $data, $postarr ) {
if ( 'auto-draft' === $postarr['post_status'] ) {
if ( ! empty( $postarr['ID'] ) ) {
$data['post_status'] = 'draft';
} else {
$data['post_title'] = '';
}
}
return $data;
}
add_filter( 'wp_insert_post_data', 'gutenberg_filter_wp_insert_post_data', 10, 2 );
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems right for me but I'd like to check with the REST API people if this is legit as if I understand properly. I don't see why people would want to save "auto-drafts" using the REST API (it's an internal status). Any API request saving auto-drafts will create drafts with this change. Also, can we limit this change to the REST API instead of a global wp_insert_post_data filter?

cc @kadamwhite @TimothyBJacobs

Copy link
Member

Choose a reason for hiding this comment

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

Also, can we limit this change to the REST API instead of a global wp_insert_post_data filter?

The rest_pre_insert_{post_type} filter would likely work for this - it is applied to the prepared post data before calling update_post.

Can we limit the behavior to Gutenberg? or is this the expected behavior for all API updates to auto-draft posts? If so, does this change belong in the controller itself?

Copy link
Contributor

Choose a reason for hiding this comment

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

My gut feeling tells me that this should be the expected behavior for all API updates to auto-draft posts and that it belongs to the controller. That filter could be a way to test it in Gutenberg early before the Core update.

Copy link
Member

Choose a reason for hiding this comment

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

Also, can we limit this change to the REST API instead of a global wp_insert_post_data filter?

Can we limit the behavior to Gutenberg? or is this the expected behavior for all API updates to auto-draft posts? If so, does this change belong in the controller itself?

The change as I considered it in #16761 is based on the assumption that there should be no circumstance that a post be considered as auto-draft after an explicit save. For this reason, I don't think it's something which should be exclusive to either REST API updates or the Gutenberg plugin, but rather apply to any post update.

Copy link
Member

Choose a reason for hiding this comment

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

there should be no circumstance that a post be considered as auto-draft after an explicit save

Makes sense 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Separately, I'm a bit confused as to why a title is being disallowed for an auto-draft. I understand that the code should check against the post's status instead of the title to determine if it is an auto-draft or not, but I don't see why that means unsetting it here.

To avoid saving it, the default "Auto Draft" title, when saving for the first time without having edited the title.

Copy link
Contributor

Choose a reason for hiding this comment

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

What if the title was changed? We should reset only if it's equal to "Auto Draft" right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, I think the fix for that one (the title) shouldn't be on the "save" hook but more on the "auto draft" creation. I don't understand why we assign "Auto Draft" as a title in the first place?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. This should work:

if ( 'auto-draft' === $postarr['post_status'] ) {
	if ( ! empty( $postarr['ID'] ) ) {
		$data['post_status'] = 'draft';
	} else {
		$data['post_title'] = '';
	}
}

Post ID will be empty when creating right?

Copy link
Member

Choose a reason for hiding this comment

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

Aside: A benefit of having these two as separate commits a74c59c f5748c0 in the original #16761 was in optimizing not for concise code as it exists in Gutenberg, but for clarity in an eventual merge into core that each of these changes be considered independently.

Respectively, I expect likely here:

https://github.com/WordPress/wordpress-develop/blob/a8a4c09f33356475f4a3f3a9f5bdacd0c25bcb42/src/wp-includes/post.php#L4077-L4091

...and here:

https://github.com/WordPress/wordpress-develop/blob/a8a4c09f33356475f4a3f3a9f5bdacd0c25bcb42/src/wp-admin/includes/post.php#L669