"Move to Trash" with metaboxes prompts about unsaved changes, 500 server error

[aduth]

Describe the bug

The "Move to Trash" button does not work very well when meta boxes are present on the page:

1. The user is prompted about there being unsaved changes
2. In the browser Developer Tools Network log, a 500 error is shown for a failed request

It appears this is largely caused by a combination of:

• The meta box values try to save at the same time the browser redirect of a trashed post tries to occur (the meta box save occurs as a separate step after the "default" post save)
• The meta box save attempt fails because the post is already trashed

gutenberg/packages/edit-post/src/components/browser-url/index.js

Lines 58 to 66 in 3d2b41a

	/**
	* Navigates the browser to the post trashed URL to show a notice about the trashed post.
	*
	* @param {number} postId Post ID.
	* @param {string} postType Post Type.
	*/
	setTrashURL( postId, postType ) {
	window.location.href = getPostTrashedURL( postId, postType );
	}

{"code":"wp_die","message":"You can’t edit this item because it is in the Trash. Please restore it and try again.","data":{"status":500},"additional_errors":[]}

To reproduce

Steps to reproduce the behavior:

1. Use an addon which includes a save step for meta boxes in the post editor (e.g. AdvancedCustomFields with fields present on the post editor)
2. Navigate to an existing post
3. Press "Move to Trash"
4. See prompt, failed network request

Expected behavior

The user is redirected without a prompt or error'd request.

My expectation is that the save request for meta boxes would be skipped when the user tries to save the post, or it must occur prior (or at least prior to the saving of the status to "trash", since as noted in the error message, post values cannot be saved when the post is trashed).

Desktop (please complete the following information):

• OS: macOS Catalina 10.15 (19A603)
• Browser: Chrome Version 78.0.3904.70 (Official Build) (64-bit)

Additional Context

Most of the relevant code here is contained in the BrowserURL component (responsible for redirecting after trashing a post), the trashPost action (which sets the status and saves the post), and the edit-post "effect" for saving meta box values after the default post save procedure.

gutenberg/packages/edit-post/src/components/browser-url/index.js

Lines 48 to 51 in 3d2b41a

	if ( postStatus === 'trash' ) {
	this.setTrashURL( postId, postType );
	return;
	}

gutenberg/packages/editor/src/store/actions.js

Lines 324 to 365 in 3d2b41a

	/**
	* Action generator for trashing the current post in the editor.
	*/
	export function* trashPost() {
	const postTypeSlug = yield select(
	STORE_KEY,
	'getCurrentPostType'
	);
	const postType = yield select(
	'core',
	'getPostType',
	postTypeSlug
	);
	yield dispatch(
	'core/notices',
	'removeNotice',
	TRASH_POST_NOTICE_ID
	);
	try {
	const post = yield select(
	STORE_KEY,
	'getCurrentPost'
	);
	yield apiFetch(
	{
	path: `/wp/v2/${ postType.rest_base }/${ post.id }`,
	method: 'DELETE',
	}
	);
	yield dispatch(
	STORE_KEY,
	'savePost'
	);
	} catch ( error ) {
	yield dispatch(
	'core/notices',
	'createErrorNotice',
	...getNotificationArgumentsForTrashFail( { error } ),
	);
	}
	}

gutenberg/packages/edit-post/src/store/effects.js

Lines 51 to 70 in 3d2b41a

	// Save metaboxes when performing a full save on the post.
	saveMetaboxUnsubscribe = subscribe( () => {
	const isSavingPost = select( 'core/editor' ).isSavingPost();
	const isAutosavingPost = select( 'core/editor' ).isAutosavingPost();
	// Save metaboxes on save completion, except for autosaves that are not a post preview.
	const shouldTriggerMetaboxesSave = (
	hasActiveMetaBoxes && (
	( wasSavingPost && ! isSavingPost && ! wasAutosavingPost )
	)
	);
	// Save current state for next inspection.
	wasSavingPost = isSavingPost;
	wasAutosavingPost = isAutosavingPost;
	if ( shouldTriggerMetaboxesSave ) {
	store.dispatch( requestMetaBoxUpdates() );
	}
	} );

[paaljoachim]

I see a merged PR in relation to this issue.

Should it be closed?
@WordPress/gutenberg-core

[adamsilverstein]

@paaljoachim yes, as far as I can tell this issue was resolved by #17427 along with #17426. Closing as resolved.