Added logic for saving revisions on explicit saves (#16688)

refs @TryGhost/Team#3076

- added `save_revision` option to edit post endpoint
- this change covers the following cases:
1. we will not save a `post_revision` on every background autosave that
occurs after 3 seconds of inactivity in the editor
2. we will save a `post_revision` when the user hits `cmd+s` in the
editor to explicitly save
3. we will save a `post_revision` when the user navigates away from the
editor (e.g. by clicking the 'Posts' breadcrumb in the editor)
4. we will save a `post_revision` when the user publishes a post
5. we will save a `post_revision` when a user updates an already
published post

ghost/admin/app/adapters/post.js

@@ -20,6 +20,10 @@ export default class Post extends ApplicationAdapter {
             }
         }
 
+        if (snapshot?.adapterOptions?.saveRevision) {
+            const saveRevision = snapshot.adapterOptions.saveRevision;
+            parsedUrl.searchParams.append('save_revision', saveRevision);
+        }
         return parsedUrl.toString();
     }
 

ghost/admin/app/controllers/lexical-editor.js

@@ -406,11 +406,12 @@ export default class LexicalEditorController extends Controller {
 
     // separate task for autosave so that it doesn't override a manual save
     @dropTask
-    *autosaveTask() {
+    *autosaveTask(options) {
         if (!this.get('saveTask.isRunning')) {
             return yield this.saveTask.perform({
                 silent: true,
-                backgroundSave: true
+                backgroundSave: true,
+                ...options
             });
         }
     }
@@ -422,10 +423,11 @@ export default class LexicalEditorController extends Controller {
         let prevStatus = this.get('post.status');
         let isNew = this.get('post.isNew');
         let status;
+        const adapterOptions = {};
 
         this.cancelAutosave();
 
-        if (options.backgroundSave && !this.hasDirtyAttributes) {
+        if (options.backgroundSave && !this.hasDirtyAttributes && !options.leavingEditor) {
             return;
         }
 
@@ -452,10 +454,15 @@ export default class LexicalEditorController extends Controller {
         // new publishing flow sets the post status manually on publish
         this.set('post.status', status);
 
+        const explicitSave = !options.backgroundSave;
+        const leavingEditor = options.leavingEditor;
+        if (explicitSave || leavingEditor) {
+            adapterOptions.saveRevision = 1;
+        }
         yield this.beforeSaveTask.perform(options);
 
         try {
-            let post = yield this._savePostTask.perform(options);
+            let post = yield this._savePostTask.perform({...options, adapterOptions});
 
             post.set('statusScratch', null);
 
@@ -826,6 +833,11 @@ export default class LexicalEditorController extends Controller {
         let hasDirtyAttributes = this.hasDirtyAttributes;
         let state = post.getProperties('isDeleted', 'isSaving', 'hasDirtyAttributes', 'isNew');
 
+        // Check if anything has changed since the last revision
+        let postRevisions = post.get('postRevisions').toArray();
+        let latestRevision = postRevisions[postRevisions.length - 1];
+        let hasChangedSinceLastRevision = post.get('lexical') !== latestRevision.get('lexical');
+
         let fromNewToEdit = this.router.currentRouteName === 'lexical-editor.new'
             && transition.targetName === 'lexical-editor.edit'
             && transition.intent.contexts
@@ -835,6 +847,17 @@ export default class LexicalEditorController extends Controller {
         let deletedWithoutChanges = state.isDeleted
             && (state.isSaving || !state.hasDirtyAttributes);
 
+        // If leaving the editor and the post has changed since we last saved a revision, always save a new revision
+        if (hasChangedSinceLastRevision) {
+            transition.abort();
+            if (this._autosaveRunning) {
+                this.cancelAutosave();
+                this.autosaveTask.cancelAll();
+            }
+            await this.autosaveTask.perform({leavingEditor: true});
+            return transition.retry();
+        }
+
         // controller is dirty and we aren't in a new->edit or delete->index
         // transition so show our "are you sure you want to leave?" modal
         if (!this._leaveConfirmed && !fromNewToEdit && !deletedWithoutChanges && hasDirtyAttributes) {
@@ -851,7 +874,8 @@ export default class LexicalEditorController extends Controller {
                 this.cancelAutosave();
                 this.autosaveTask.cancelAll();
 
-                await this.autosaveTask.perform();
+                // If leaving the editor, always save a revision
+                await this.autosaveTask.perform({leavingEditor: true});
                 return transition.retry();
             }
 

ghost/core/core/server/api/endpoints/posts.js

@@ -176,6 +176,7 @@ module.exports = {
             'email_segment',
             'newsletter',
             'force_rerender',
+            'save_revision',
             // NOTE: only for internal context
             'forUpdate',
             'transacting'

ghost/core/core/server/models/post.js

@@ -913,7 +913,7 @@ Post = ghostBookshelf.Model.extend({
                     const revisionModels = await ghostBookshelf.model('PostRevision')
                         .findAll(Object.assign({
                             filter: `post_id:${model.id}`,
-                            columns: ['id']
+                            columns: ['id', 'lexical', 'created_at', 'author_id', 'title']
                         }, _.pick(options, 'transacting')));
 
                     const revisions = revisionModels.toJSON();
@@ -932,7 +932,12 @@ Post = ghostBookshelf.Model.extend({
                         title: model.get('title')
                     };
 
-                    const newRevisions = await postRevisions.getRevisions(previous, current, revisions);
+                    // This can be refactored once we have the status stored in each revision
+                    const revisionOptions = {
+                        forceRevision: options.save_revision,
+                        isPublished: newStatus === 'published'
+                    };
+                    const newRevisions = await postRevisions.getRevisions(previous, current, revisions, revisionOptions);
                     model.set('post_revisions', newRevisions);
                 });
             }
@@ -1170,7 +1175,7 @@ Post = ghostBookshelf.Model.extend({
             findPage: ['status'],
             findAll: ['columns', 'filter'],
             destroy: ['destroyAll', 'destroyBy'],
-            edit: ['filter', 'email_segment', 'force_rerender', 'newsletter']
+            edit: ['filter', 'email_segment', 'force_rerender', 'newsletter', 'save_revision']
         };
 
         // The post model additionally supports having a formats option

ghost/core/test/e2e-api/admin/posts.test.js

@@ -389,7 +389,7 @@ describe('Posts API', function () {
             const [postResponse] = postBody.posts;
 
             await agent
-                .put(`/posts/${postResponse.id}/?formats=mobiledoc,lexical,html`)
+                .put(`/posts/${postResponse.id}/?formats=mobiledoc,lexical,html&save_revision=true`)
                 .body({posts: [Object.assign({}, postResponse, {lexical: updatedLexical})]})
                 .expectStatus(200)
                 .matchBodySnapshot({

ghost/post-revisions/lib/post-revisions.js

@@ -32,26 +32,41 @@ class PostRevisions {
      * @param {PostLike} previous
      * @param {PostLike} current
      * @param {Revision[]} revisions
+     * @param {object} options
      * @returns {boolean}
      */
-    shouldGenerateRevision(previous, current, revisions) {
+    shouldGenerateRevision(previous, current, revisions, options) {
+        const latestRevision = revisions[revisions.length - 1];
         if (!previous) {
             return false;
         }
+        // If there's no revisions for this post, we should always save a revision
         if (revisions.length === 0) {
             return true;
         }
-        return previous.html !== current.html || previous.title !== current.title;
+        const isPublished = options && options.isPublished;
+        if (isPublished) {
+            return true;
+        }
+
+        const forceRevision = options && options.forceRevision;
+        const lexicalHasChangedSinceLatestRevision = latestRevision.lexical !== current.lexical;
+        const titleHasChanged = previous.title !== current.title;
+        if ((lexicalHasChangedSinceLatestRevision || titleHasChanged) && forceRevision) {
+            return true;
+        }
+        return false;
     }
 
     /**
      * @param {PostLike} previous
      * @param {PostLike} current
      * @param {Revision[]} revisions
+     * @param {object} options
      * @returns {Promise<Revision[]>}
      */
-    async getRevisions(previous, current, revisions) {
-        if (!this.shouldGenerateRevision(previous, current, revisions)) {
+    async getRevisions(previous, current, revisions, options) {
+        if (!this.shouldGenerateRevision(previous, current, revisions, options)) {
             return revisions;
         }
 

ghost/post-revisions/test/hello.test.js

@@ -1,6 +1,6 @@
 const assert = require('assert');
 const sinon = require('sinon');
-const PostRevisions = require('../');
+const PostRevisions = require('..');
 
 const config = {
     max_revisions: 10
@@ -43,7 +43,7 @@ describe('PostRevisions', function () {
             assert.equal(actual, expected);
         });
 
-        it('should return true if the current and previous html values are different', function () {
+        it('should return true if forceRevision is true and the lexical has changed since the latest revision', function () {
             const postRevisions = new PostRevisions({config});
 
             const expected = true;
@@ -55,12 +55,16 @@ describe('PostRevisions', function () {
                 html: 'blah2'
             }, [{
                 lexical: 'blah'
-            }]);
+            }, {
+                lexical: 'blah2'
+            }], {
+                forceRevision: true
+            });
 
             assert.equal(actual, expected);
         });
 
-        it('should return true if the current and previous title values are different', function () {
+        it('should return true if the current and previous title values are different and forceRevision is true', function () {
             const postRevisions = new PostRevisions({config});
 
             const expected = true;
@@ -74,7 +78,30 @@ describe('PostRevisions', function () {
                 title: 'blah2'
             }, [{
                 lexical: 'blah'
-            }]);
+            }], {
+                forceRevision: true
+            });
+
+            assert.equal(actual, expected);
+        });
+
+        it('should always return true if isPublished is true', function () {
+            const postRevisions = new PostRevisions({config});
+
+            const expected = true;
+            const actual = postRevisions.shouldGenerateRevision({
+                lexical: 'blah',
+                html: 'blah',
+                title: 'blah'
+            }, {
+                lexical: 'blah',
+                html: 'blah',
+                title: 'blah2'
+            }, [{
+                lexical: 'blah'
+            }], {
+                isPublished: true
+            });
 
             assert.equal(actual, expected);
         });
@@ -161,7 +188,9 @@ describe('PostRevisions', function () {
                 id: '1',
                 lexical: 'new',
                 html: 'new'
-            }, revisions);
+            }, revisions, {
+                forceRevision: true
+            });
 
             assert.equal(actual.length, 2);
         });