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.

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

Autosaves: Create a separate revision when autosaving a draft #26345

Closed
wants to merge 5 commits into from

Conversation

jblz
Copy link
Member

@jblz jblz commented Jul 26, 2018

As of #19261, we are passing the autosave param to the API when the Editor saves its buffer. That param results in the DOING_AUTOSAVE constant getting set &, consequently, revisions are not saved for drafts unless the Save button is explicitly clicked.

This change conditionally initiates a separate request to save a revision for drafts when they're autosaved.

Fixes #20265

Additionally, this stops showing the autosave <RestorePostDialog /> when relevant fields are identical.

To Test

  • run this branch
  • open / create a draft in the editor
  • change some content & wait for the Save button to change to Saved
  • a revision should be saved (The History button should appear when there are revisions & you should see a revision for your edits)

@jblz jblz added [Feature] Post/Page Editor The editor for editing posts and pages. [Status] In Progress labels Jul 26, 2018
@jblz jblz self-assigned this Jul 26, 2018
@matticbot
Copy link
Contributor

@jblz jblz requested a review from spen July 27, 2018 05:35
@jblz jblz force-pushed the add/revisions-for-drafts branch from 3474adb to 20d02ee Compare July 27, 2018 05:59
@jblz jblz added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Jul 27, 2018
@jblz jblz requested a review from jsnajdr July 27, 2018 06:09
Copy link
Contributor

@spen spen left a comment

Choose a reason for hiding this comment

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

screen shot 2018-07-27 at 11 38 43

My only concern is that this creates a lot of revisions so there ends up being more noise than signal and the 25 limit will fill up fast. Still I think this is better than losing data 👍

No issues with the code :)

@alisterscott alisterscott added [Status] Needs e2e Testing [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it labels Aug 7, 2018
@alisterscott
Copy link
Contributor

e2e tests are fine on this one - just expecting the "Import" option which is in master but not this PR

@alisterscott alisterscott added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Aug 7, 2018
parent: post.ID,
status: 'inherit',
type: 'revision',
} );
Copy link
Member

Choose a reason for hiding this comment

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

The savePost action dispatches POST_SAVE, POST_SAVE_SUCCESS and POSTS_RECEIVE while saving the post or revision. That updates the structures in state.posts. And the revision doesn't really belong there.

It doesn't immediately break anything, just adds more entropy to already complex and fragile code.

It would be better to directly call wpcom.site().post().add() in the saveRevision action thunk, without the extra dispatches.

@jblz jblz force-pushed the add/revisions-for-drafts branch from fe8f2ea to a7a3020 Compare August 9, 2018 11:35
@jblz
Copy link
Member Author

jblz commented Aug 16, 2018

I'm going to go ahead and close this.

To prevent superfluous network requests, we're pursuing a server-side solution in:

@jblz jblz closed this Aug 16, 2018
@jblz jblz deleted the add/revisions-for-drafts branch August 16, 2018 03:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Post/Page Editor The editor for editing posts and pages. [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants