TryGhost
🐛 Fixed scheduled email-only posts to require newsletter reference.
fixes #24490
This prevents accidentally scheduling an email to zero recipients.
[!NOTE] Enforces a newsletter reference when scheduling email-only posts and updates tests to cover the validation and preview behavior.
- Backend
- Adds validation in
core/server/models/post.jsto reject scheduling email-only posts (posts_meta.email_only) without a newsletter reference (options.newsletter/newsletter_id).- Introduces
messages.emailOnlyWithoutNewslettererror text.- Tests
- Adds e2e test in
test/e2e-api/admin/posts-legacy.test.jsasserting 422 when scheduling email-only without a newsletter.- Updates
test/e2e-frontend/preview_routes.test.jsto setnewsletter_idfor scheduled email-only preview.Written by Cursor Bugbot for commit d604e53d2583f9c03d0488dcc928c7d96fe841f0. This will update automatically on new commits. Configure here.
Walkthrough
A validation was added to the Post model's onSaving hook that rejects saves when a post is marked email_only, the new status is scheduled, the status changed, and neither newsletter_id nor the newsletter option is provided; the save now throws a ValidationError with message key emailOnlyWithoutNewsletter. Tests were added/updated: an API e2e test asserts scheduling an email-only post without a newsletter is rejected (422 with validation context), and a frontend preview test now attaches a newsletter_id to a scheduled email-only post fixture.
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
- Areas to review closely:
- ghost/core/core/server/models/post.js — new conditional branch and error construction in
onSaving. - ghost/core/test/e2e-api/admin/posts-legacy.test.js — new/duplicated test cases asserting 422 and validation message.
- ghost/core/test/e2e-frontend/preview_routes.test.js — fixture change adding
newsletter_idto scheduled posts.
- ghost/core/core/server/models/post.js — new conditional branch and error construction in
Assessment against linked issues
| Objective | Addressed | Explanation |
|---|---|---|
Prevent invalid state transitions for scheduled email-only posts lacking newsletter reference (#24490) |
✅ | Validation enforces presence of a newsletter reference before allowing scheduling. |
Return a clear validation error when attempting to schedule an email-only post without a newsletter (#24490) |
✅ | Save now throws a ValidationError with key emailOnlyWithoutNewsletter. |
Add automated test to verify scheduling email-only post without newsletter is rejected (#24490) |
✅ | New e2e API test asserts rejection and expected validation response. |
Pre-merge checks and finishing touches
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly and concisely describes the main bug fix: enforcing newsletter reference requirement for scheduled email-only posts. |
| Description check | ✅ Passed | The description is directly related to the changeset, referencing the linked issue and explaining the validation changes and test updates introduced. |
| Linked Issues check | ✅ Passed | The PR successfully addresses the root cause identified in #24490 by adding validation to prevent scheduling email-only posts without a newsletter reference, preventing the silent failure scenario. |
| Out of Scope Changes check | ✅ Passed | All changes are scoped to addressing the linked issue: validation logic in post.js, API test coverage, and frontend test updates to reflect the new validation requirement. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✨ Finishing touches
- [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
- [ ] Create PR with unit tests
- [ ] Post copyable unit tests in a comment
📜 Recent review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 3132f67b5abb278835a5c853bac66b255c0e3a5b and d604e53d2583f9c03d0488dcc928c7d96fe841f0.
📒 Files selected for processing (1)
ghost/core/test/e2e-api/admin/posts-legacy.test.js(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 25288
File: ghost/core/core/server/services/member-welcome-emails/jobs/lib/process-entries.js:46-64
Timestamp: 2025-11-10T23:10:17.470Z
Learning: In ghost/core/core/server/services/member-welcome-emails/jobs/lib/process-entries.js and the outbox processing flow, entries are marked as PROCESSING before being processed. If a failure occurs after email send but before deletion, the entry remains stuck in PROCESSING state (not reprocessed). This intentional design prevents duplicate emails. Handling stuck PROCESSING entries is planned for a separate PR.
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
Applied to files:
ghost/core/test/e2e-api/admin/posts-legacy.test.js
📚 Learning: 2025-08-11T19:39:00.428Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 24651
File: ghost/core/test/utils/urlUtils.js:53-57
Timestamp: 2025-08-11T19:39:00.428Z
Learning: In Ghost's test utilities, when fixing specific issues like async behavior, it's preferred to maintain existing error handling patterns (even if suboptimal) to keep PRs focused on their primary objective. Error handling improvements can be addressed in separate, dedicated PRs.
Applied to files:
ghost/core/test/e2e-api/admin/posts-legacy.test.js
📚 Learning: 2025-05-29T10:37:26.369Z
Learnt from: ErisDS
Repo: TryGhost/Ghost PR: 23588
File: ghost/core/test/e2e-api/admin/backup.test.js:136-148
Timestamp: 2025-05-29T10:37:26.369Z
Learning: In the Ghost test framework, when testing CSV exports via the admin API, the response `body` field is empty while the actual CSV data is provided through the `text` field. Tests should use `expectEmptyBody()` and then validate the CSV content via `.expect(({text}) => ...)` - this is not contradictory behavior.
Applied to files:
ghost/core/test/e2e-api/admin/posts-legacy.test.js
📚 Learning: 2025-10-30T17:13:26.190Z
Learnt from: sam-lord
Repo: TryGhost/Ghost PR: 25303
File: ghost/core/core/server/services/email-service/BatchSendingService.js:19-19
Timestamp: 2025-10-30T17:13:26.190Z
Learning: In ghost/core/core/server/services/email-service/BatchSendingService.js and similar files in the Ghost codebase, prefer using `{...options}` spread syntax without explicit guards like `...(options || {})` when spreading potentially undefined objects, as the maintainer prefers cleaner syntax over defensive patterns when the behavior is safe.
Applied to files:
ghost/core/test/e2e-api/admin/posts-legacy.test.js
🧬 Code graph analysis (1)
ghost/core/test/e2e-api/admin/posts-legacy.test.js (1)
ghost/core/test/legacy/api/admin/posts.test.js (12)
post(797-800)testUtils(6-6)res(415-426)res(463-474)res(509-520)res(581-592)res(625-635)res(668-678)request(16-16)localUtils(9-9)config(7-7)moment(5-5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Cursor Bugbot
- GitHub Check: Setup
- GitHub Check: Setup
- GitHub Check: Build & Push
🔇 Additional comments (1)
ghost/core/test/e2e-api/admin/posts-legacy.test.js (1)
1475-1507: New validation test for scheduling email-only posts looks correctThe test setup and expectations match the intended behavior: changing a draft to
status: 'scheduled'withemail_only = trueand a futurepublished_atbut no newsletter reference, then asserting a 422 with the specific validation message/context. The use ofresultin the final.expectcallback also avoids the previousresshadowing issue. No changes needed.
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.
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot] avatar](https://avatars.githubusercontent.com/in/347564?v=4 "Published by a pub.dev verified publisher"){kind=small}
Looks like a test started failing since this was submitted 5 months ago because elsewhere another test was added that that attempted to schedule an email without any target newsletter-- Something which makes no sense in the real world, and would represent an error if done via the API.
That illustrate why this PR is valuable-- to catch error cases like that throw a clear error to communicate the mistake.
@markstos Thanks! Could you please update the PR to fix tests and then we can get this merged ☺️
I've fixed the test failures from newer changes which conflicted with this older PR.
There is one more test failure related to a test file that doesn't seem to exist in Git, so I'm not sure where to go next with that:
e2e/tests/admin/posts/post-preview.test.ts.
In the tree, e2e/tests/admin is empty?
If it's related to this PR, it's probably the same issue that some test code is trying to schedule an email without supplying any target newsletter_id, and the test needs to be updated in additional place to supply a newsletter_id in this case.
@markstos this is the relevant file: https://github.com/TryGhost/Ghost/blob/main/e2e/tests/admin/posts/post-preview.test.ts. It's part of a new e2e suite that will replace the browser tests suite, are you not getting that in your local clone?
