Skip to content

🐛 Fix Admin API Post update method to not save a revision when save_revision=false#26678

Open
markstos wants to merge 1 commit intoTryGhost:mainfrom
markstos:issue-26677-save-revision-false
Open

🐛 Fix Admin API Post update method to not save a revision when save_revision=false#26678
markstos wants to merge 1 commit intoTryGhost:mainfrom
markstos:issue-26677-save-revision-false

Conversation

@markstos
Copy link
Contributor

@markstos markstos commented Mar 4, 2026

fixes #26677

There's a sort order bug in Ghost's revision logic. Here's the chain:

post-revision.js:32-33 — orderDefaultRaw() returns 'created_at_ts DESC' — newest first
post.js:920-926 — revisions are fetched via findAll → gets them in DESC order (newest first)
post-revisions.ts:56 — const latestRevision = revisions[revisions.length - 1] — grabs the last element

So latestRevision is actually the oldest revision, not the newest. The time check on line 84 compares Date.now() against the oldest revision's timestamp — which is almost always >10 minutes old. This means the background_save condition passes every time content has changed, regardless of how recently the last revision was created.

This bug is masked in the Ghost Admin UI because it always sends save_revision=true and there was no test coverage in the test suite for the condition.

The flag is not documented in the Admin API docs, helping the issue to go undetected.
Steps to Reproduce

Make an Admin API call to create a post.
Check revisions.
Make an Admin API call to update a post with save_revision=false in the query string.
Check revision count.
Make an Admin API call to update a post with save_revision=false in the query string.

Revision count should not be increasing, but it is.

  • I've read and followed the Contributor Guide
  • I've explained my change
  • I've written an automated test to prove my change works

Note

Cursor Bugbot is generating a summary for commit fc10425. Configure here.

fixes TryGhost#26677

Reversing the sort here does not adversely impact anything else.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 4, 2026

Walkthrough

The changes fix a revision ordering bug in the post model. The code now reverses the revision records array before processing revisions, correcting the order in which they are handled. Additionally, a new end-to-end test is added to verify that when editing a post without the save_revision flag within the revision interval, no additional PostRevision records are created. Together, these changes ensure the revision timestamp comparison works correctly and unnecessary revisions are not generated.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies a bug fix related to the save_revision=false flag in the Admin API Post update method, which directly matches the main change in the changeset.
Description check ✅ Passed The description provides detailed context about the revision ordering bug, root cause analysis, and explains why the issue went undetected, all of which relate to the changeset modifications.
Linked Issues check ✅ Passed The PR addresses all coding requirements from issue #26677: fixes the revision ordering logic by reversing the array [post.js], adds test coverage for save_revision=false condition [posts.test.js], and validates the fix prevents unnecessary revisions.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the revision ordering bug: post.js reverses revisions array, posts.test.js adds test coverage for the specific save_revision=false scenario. No unrelated modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Free Tier Details

Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

.where('post_id', postResponse.id)
.fetchAll();
assert.equal(revisionsAfter.length, 2, 'No new revision should be created within the interval without save_revision');
});
Copy link

Choose a reason for hiding this comment

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

Test doesn't actually validate the sort-order fix

Medium Severity

The new test claims to verify that no revision is created within the interval without save_revision, but it passes regardless of whether the .reverse() fix is applied. All revisions are created within milliseconds of each other in the test, so even when the sort-order bug causes latestRevision to point to the oldest revision, that revision is still well within the 10-minute POST_REVISIONS_INTERVAL_MS. To actually catch a regression of the sort-order bug, the oldest revision's created_at_ts would need to be more than 10 minutes in the past.

Fix in Cursor Fix in Web

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
ghost/core/core/server/models/post.js (1)

926-926: Document the revision-ordering contract at Line 926.

This reverse is correct, but it encodes a non-obvious dependency (PostRevision default DESC order + PostRevisions expecting oldest→newest). A brief inline comment here would make the intent explicit and reduce future regressions.

💡 Minimal clarity diff
-                const revisions = revisionModels.toJSON().reverse();
+                // PostRevision is fetched newest-first by default; PostRevisions expects oldest->newest.
+                const revisions = revisionModels.toJSON().reverse();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ghost/core/core/server/models/post.js` at line 926, Add a short inline
comment next to the const revisions = revisionModels.toJSON().reverse(); line
documenting the ordering contract: state that PostRevision model is returned in
default DESC (newest→oldest) order and PostRevisions consumer expects
oldest→newest, so we reverse the array here; reference the revisionModels
variable and the PostRevision model name in the comment so future readers
understand the dependency and won't remove the reverse unintentionally.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@ghost/core/core/server/models/post.js`:
- Line 926: Add a short inline comment next to the const revisions =
revisionModels.toJSON().reverse(); line documenting the ordering contract: state
that PostRevision model is returned in default DESC (newest→oldest) order and
PostRevisions consumer expects oldest→newest, so we reverse the array here;
reference the revisionModels variable and the PostRevision model name in the
comment so future readers understand the dependency and won't remove the reverse
unintentionally.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 70a1d003-fe0a-4fca-9a8c-8a35f9869eb2

📥 Commits

Reviewing files that changed from the base of the PR and between 2bbc0bf and fc10425.

📒 Files selected for processing (2)
  • ghost/core/core/server/models/post.js
  • ghost/core/test/e2e-api/admin/posts.test.js

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐛 Admin API always saves revisions even when save_revision=false

1 participant