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

Add module that creates a permanent revision from a significantly modified autosave #13148

Closed
wants to merge 3 commits into from

Conversation

jsnajdr
Copy link
Member

@jsnajdr jsnajdr commented Jul 30, 2019

When the editor does an autosave repeatedly, create a permanent revision from the old
autosave if the new one is significantly different. Makes losing edited content less
likely.

Adapted from a WP.com patch (D30389-code), converted the code to a Jetpack module and
added (optional) recording of StatsD events using the new jetpack_stats_statsd action
(that will be hooked into on WP.com).

Fixes an issue reported in Calypso repo: Automattic/wp-calypso#20265

Testing instructions:

(adapted from D30389-code)
This module creates a permanent revision if new autosave's content length is more than 250 characters smaller or bigger than the old one's.

  • edit a draft or published post in various editors (Classic or Block)
  • add one character and wait for autosave. Check that permanent revision was not created. (best done with revisions page opened in a second browser tab) You see a revision for the post content before you started editing, and an autosave revision with the one new char.
  • add 250+ characters and wait for autosave. Check that permanent revision was created from the previous autosave (+1 char edit)
  • start editing again from a fully saved state. Add 250+ characters and wait for autosave. Check that permanent revision was not created: the content before the autosave already has a revision and the one created during autosave would be an identical one.

Proposed changelog entry for your changes:

  • Writing: more reliable autosaves that save a permanent revision before large changes

…ified autosave

When the editor does an autosave repeatedly, create a permanent revision from the old
autosave if the new one is significantly different. Makes losing edited content less
likely.

Adapted from a WP.com patch (D30389-code), converted the code to a Jetpack module and
added (optional) recording of StatsD events using the new `jetpack_stats_statsd` action
(that will be hooked into on WP.com).
@jsnajdr jsnajdr added the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Jul 30, 2019
@jsnajdr jsnajdr requested review from donpark, jeherve and a team July 30, 2019 09:49
@jsnajdr jsnajdr self-assigned this Jul 30, 2019
@jetpackbot
Copy link

jetpackbot commented Jul 30, 2019

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: August 6, 2019.
Scheduled code freeze: July 30, 2019

Generated by 🚫 dangerJS against ed96d57

Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

Since you are adding a brand new file, could you add it to the PHPCS whitelist?
https://github.com/Automattic/jetpack/blob/5a2bb2b2423a42a59ced3674f30339605fbc7fd5/bin/phpcs-whitelist.js

That will allow you to get some recommendations on commit, and to fix any PHPCS issues on the file.

@jeherve jeherve added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! [Type] Feature Request [Feature] Autosave Revisions and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Jul 30, 2019
@jeherve jeherve added this to the 7.7 milestone Jul 30, 2019
@jsnajdr
Copy link
Member Author

jsnajdr commented Jul 30, 2019

@jeherve The PHPCS whitelist add is already in 65edf25

Copy link
Contributor

@donpark donpark left a comment

Choose a reason for hiding this comment

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

LGTM

Works like WPCOM version of the patch. BTW, I'm not a Jetpack code owner so you'll need ✅ from @jeherve .

@jeherve
Copy link
Member

jeherve commented Jul 31, 2019

@jeherve The PHPCS whitelist add is already in 65edf25

Excellent! Do you think you could now fix the issues that pop up? You should see them in your editor, or when running vendor/bin/phpcs -w modules/autosave-revisions.php.

@jsnajdr
Copy link
Member Author

jsnajdr commented Jul 31, 2019

Do you think you could now fix the issues that pop up?

Yes! 🙂 This PR will be most likely closed and we'll ship the code to Atomic via a different route. Let's keep in open while other options are evaluated.

@jsnajdr
Copy link
Member Author

jsnajdr commented Aug 12, 2019

We shipped this to Atomic as part of wpcomsh instead, can be closed. Thanks everyone for cooperation.

@jsnajdr jsnajdr closed this Aug 12, 2019
@jsnajdr jsnajdr deleted the add/autosave-revisions-module branch August 12, 2019 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Autosave Revisions [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! [Type] Feature Request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants