From 3aec11328f3e7beb3ab4659c6d4de95b4b59a4b4 Mon Sep 17 00:00:00 2001 From: Chris Raible Date: Fri, 21 Apr 2023 14:36:35 +0100 Subject: [PATCH] Populated reason field in post-revisions when revision is created (#16700) no issue --- ghost/admin/app/models/post-revision.js | 2 +- ghost/core/core/server/models/post.js | 2 +- .../admin/__snapshots__/posts.test.js.snap | 6 +- ghost/post-revisions/lib/post-revisions.js | 24 +++++--- ghost/post-revisions/test/hello.test.js | 58 ++++++++++++++----- 5 files changed, 66 insertions(+), 26 deletions(-) diff --git a/ghost/admin/app/models/post-revision.js b/ghost/admin/app/models/post-revision.js index 24c15b77c8..d417c5fedf 100644 --- a/ghost/admin/app/models/post-revision.js +++ b/ghost/admin/app/models/post-revision.js @@ -5,8 +5,8 @@ export default class PostRevisionModel extends Model { @attr('string') lexical; @attr('string') title; @attr('string') featureImage; + @attr('string') reason; @attr('moment-utc') createdAt; @belongsTo('user') author; @attr('string') postStatus; - @attr('string') reason; } diff --git a/ghost/core/core/server/models/post.js b/ghost/core/core/server/models/post.js index 783f9f144b..1c7f560664 100644 --- a/ghost/core/core/server/models/post.js +++ b/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', 'lexical', 'created_at', 'author_id', 'title'] + columns: ['id', 'lexical', 'created_at', 'author_id', 'title', 'reason'] }, _.pick(options, 'transacting'))); const revisions = revisionModels.toJSON(); diff --git a/ghost/core/test/e2e-api/admin/__snapshots__/posts.test.js.snap b/ghost/core/test/e2e-api/admin/__snapshots__/posts.test.js.snap index e274ae1ea9..713a0c2f2b 100644 --- a/ghost/core/test/e2e-api/admin/__snapshots__/posts.test.js.snap +++ b/ghost/core/test/e2e-api/admin/__snapshots__/posts.test.js.snap @@ -831,7 +831,7 @@ exports[`Posts API Create Can create a post with lexical 2: [headers] 1`] = ` Object { "access-control-allow-origin": "http://127.0.0.1:2369", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", - "content-length": "5295", + "content-length": "5309", "content-type": "application/json; charset=utf-8", "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, @@ -1261,7 +1261,7 @@ exports[`Posts API Update Can update a post with lexical 2: [headers] 1`] = ` Object { "access-control-allow-origin": "http://127.0.0.1:2369", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", - "content-length": "5232", + "content-length": "5246", "content-type": "application/json; charset=utf-8", "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, @@ -1371,7 +1371,7 @@ exports[`Posts API Update Can update a post with lexical 4: [headers] 1`] = ` Object { "access-control-allow-origin": "http://127.0.0.1:2369", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", - "content-length": "6486", + "content-length": "6511", "content-type": "application/json; charset=utf-8", "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, diff --git a/ghost/post-revisions/lib/post-revisions.js b/ghost/post-revisions/lib/post-revisions.js index 1fdc61de59..19ba29c15b 100644 --- a/ghost/post-revisions/lib/post-revisions.js +++ b/ghost/post-revisions/lib/post-revisions.js @@ -6,6 +6,7 @@ * @property {string} author_id * @property {string} feature_image * @property {string} title + * @property {string} reason */ /** @@ -16,6 +17,7 @@ * @property {string} author_id * @property {string} feature_image * @property {string} title + * @property {string} reason */ class PostRevisions { @@ -35,29 +37,29 @@ class PostRevisions { * @param {PostLike} current * @param {Revision[]} revisions * @param {object} options - * @returns {boolean} + * @returns {object} */ shouldGenerateRevision(previous, current, revisions, options) { const latestRevision = revisions[revisions.length - 1]; if (!previous) { - return false; + return {value: false}; } // If there's no revisions for this post, we should always save a revision if (revisions.length === 0) { - return true; + return {value: true, reason: 'initial_revision'}; } const isPublished = options && options.isPublished; if (isPublished) { - return true; + return {value: true, reason: 'published'}; } const forceRevision = options && options.forceRevision; const lexicalHasChangedSinceLatestRevision = latestRevision.lexical !== current.lexical; const titleHasChanged = previous.title !== current.title; if ((lexicalHasChangedSinceLatestRevision || titleHasChanged) && forceRevision) { - return true; + return {value: true, reason: 'explicit_save'}; } - return false; + return {value: false}; } /** @@ -68,11 +70,13 @@ class PostRevisions { * @returns {Promise} */ async getRevisions(previous, current, revisions, options) { - if (!this.shouldGenerateRevision(previous, current, revisions, options)) { + const shouldGenerateRevision = this.shouldGenerateRevision(previous, current, revisions, options); + if (!shouldGenerateRevision.value) { return revisions; } const currentRevision = this.convertPostLikeToRevision(current); + currentRevision.reason = shouldGenerateRevision.reason; if (revisions.length === 0) { return [ @@ -82,7 +86,11 @@ class PostRevisions { // Grab the most recent revisions, limited by max_revisions const updatedRevisions = [...revisions, currentRevision]; - return updatedRevisions.slice(updatedRevisions.length - this.config.max_revisions, updatedRevisions.length); + if (updatedRevisions.length > this.config.max_revisions) { + return updatedRevisions.slice(updatedRevisions.length - this.config.max_revisions, updatedRevisions.length); + } else { + return updatedRevisions; + } } /** diff --git a/ghost/post-revisions/test/hello.test.js b/ghost/post-revisions/test/hello.test.js index ae1564a1c6..40fb5f0a93 100644 --- a/ghost/post-revisions/test/hello.test.js +++ b/ghost/post-revisions/test/hello.test.js @@ -11,25 +11,25 @@ describe('PostRevisions', function () { it('should return false if there is no previous', function () { const postRevisions = new PostRevisions({config}); - const expected = false; + const expected = {value: false}; const actual = postRevisions.shouldGenerateRevision(null, {}, []); - assert.equal(actual, expected); + assert.deepEqual(actual, expected); }); it('should return true if there are no revisions', function () { const postRevisions = new PostRevisions({config}); - const expected = true; + const expected = {value: true, reason: 'initial_revision'}; const actual = postRevisions.shouldGenerateRevision({}, {}, []); - assert.equal(actual, expected); + assert.deepEqual(actual, expected); }); it('should return false if the current and previous html values are the same', function () { const postRevisions = new PostRevisions({config}); - const expected = false; + const expected = {value: false}; const actual = postRevisions.shouldGenerateRevision({ lexical: 'previous', html: 'blah' @@ -40,13 +40,13 @@ describe('PostRevisions', function () { lexical: 'blah' }]); - assert.equal(actual, expected); + assert.deepEqual(actual, expected); }); 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; + const expected = {value: true, reason: 'explicit_save'}; const actual = postRevisions.shouldGenerateRevision({ lexical: 'blah', html: 'blah' @@ -61,13 +61,13 @@ describe('PostRevisions', function () { forceRevision: true }); - assert.equal(actual, expected); + assert.deepEqual(actual, expected); }); 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; + const expected = {value: true, reason: 'explicit_save'}; const actual = postRevisions.shouldGenerateRevision({ lexical: 'blah', html: 'blah', @@ -82,13 +82,13 @@ describe('PostRevisions', function () { forceRevision: true }); - assert.equal(actual, expected); + assert.deepEqual(actual, expected); }); it('should always return true if isPublished is true', function () { const postRevisions = new PostRevisions({config}); - const expected = true; + const expected = {value: true, reason: 'published'}; const actual = postRevisions.shouldGenerateRevision({ lexical: 'blah', html: 'blah', @@ -103,7 +103,7 @@ describe('PostRevisions', function () { isPublished: true }); - assert.equal(actual, expected); + assert.deepEqual(actual, expected); }); }); @@ -163,7 +163,7 @@ describe('PostRevisions', function () { assert.equal(actual[0].title, 'foo bar baz'); }); - it('limits the number of revisions to the max_revisions count', async function () { + it('does not limit the number of revisions if under the max_revisions count', async function () { const postRevisions = new PostRevisions({ config: { max_revisions: 2 @@ -194,6 +194,38 @@ describe('PostRevisions', function () { assert.equal(actual.length, 2); }); + + it('limits the number of revisions to the max_revisions count', async function () { + const postRevisions = new PostRevisions({ + config: { + max_revisions: 1 + } + }); + + const revisions = await postRevisions.getRevisions({ + id: '1', + lexical: 'previous', + html: 'previous' + }, { + id: '1', + lexical: 'current', + html: 'current' + }, []); + + const actual = await postRevisions.getRevisions({ + id: '1', + lexical: 'old', + html: 'old' + }, { + id: '1', + lexical: 'new', + html: 'new' + }, revisions, { + forceRevision: true + }); + + assert.equal(actual.length, 1); + }); }); describe('removeAuthor', function () {