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

Saving: Autosaveable does not account for autosave existing at start of editing #7416

Open
aduth opened this Issue Jun 20, 2018 · 5 comments

Comments

Projects
None yet
10 participants
@aduth
Copy link
Member

aduth commented Jun 20, 2018

Describe the bug

If an autosave revision exists for a published post (or draft authored by another user), and non-content field edits exist, isEditedPostAutosaveable wrongly returns true, resulting in a 400 bad request response from the autosaves endpoint.

To Reproduce

Steps to reproduce the behavior:

  1. Navigate to Posts > Add New
  2. Enter a title
  3. Publish the post
  4. Change the title
  5. Press Preview
  6. Close Preview tab
    • (Ignore fact it may or may not redirect to the preview)
  7. Reload the page, dismissing the prompt about unsaved changes
  8. Note there is an "Autosave exists" notice
  9. Toggle the post as "Stick to the Front Page" in Document settings
  10. Press preview

Expected behavior

There are no changes to the saved copy of the post to be previewed (title, content, excerpt), so the preview window should direct to the saved post link and not initiate a request to the autosaves endpoint.

Actual behavior

The "Preview Loading" interstitial is wrongly displayed. A request is sent to the autosaves endpoint and responds with a 400 Bad Request code.

Possible solution

The isEditedPostAutosaveable selector returns early with true if there is no known autosave:

// If we don't already have an autosave, the post is autosaveable.
if ( ! hasAutosave( state ) ) {
return true;
}

In the above flow, an autosave exists but it is not known to state. A solution may be to simply ensure that the autosave is populated at the start of the editor session, so it can continue to the condition of the selector which would cause false to be returned correctly.

Alternatively, we should consider removing the 400 response from the autosaves controller when there are no changes to the autosave, instead treating it as though it were a successful save.

return new WP_Error( 'rest_autosave_no_changes', __( 'There is nothing to save. The autosave and the post content are the same.', 'gutenberg' ), array( 'status' => 400 ) );

This would arguably be a needless request (a noop), but a preview link could be pulled from its successful response and the user redirected as expected.

Additional context

Part of ongoing struggles with autosave partly addressed in #7130 and relevant Trac ticket.

@ginigangadharan

This comment has been minimized.

Copy link

ginigangadharan commented Jun 21, 2018

Thats a good news indeed.
Can we expect an updated plugin release soon ?

Thanks

@aduth

This comment has been minimized.

Copy link
Member

aduth commented Jul 4, 2018

Another problematic case with lack of initial autosave described in #7703

@talldan talldan referenced a pull request that will close this issue Jul 13, 2018

Open

If a more recent revision/autosave exists, store its state on editor setup #7945

4 of 4 tasks complete
@talldan

This comment has been minimized.

Copy link
Contributor

talldan commented Jul 13, 2018

@aduth I noticed this issue while working on #7883

I've attempted a fix, but because I'm new to the codebase it's probably a fairly naive attempt. Would be good to get some feedback on it - #7945

@designsimply

This comment has been minimized.

Copy link
Contributor

designsimply commented Aug 15, 2018

Closed #7852 as a duplicate and noting that when a 400 Bad Request code happens for an autosave it results in previews getting stuck. (41s)

@mtias

This comment has been minimized.

Copy link
Contributor

mtias commented Nov 20, 2018

Adding to 5.0.1 since it has a proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment