Added ability to test member welcome emails #25692
Conversation
github-actions
bot
added
the
migration
WalkthroughAdds a test-send feature for automated member welcome emails: a new frontend mutation hook Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
troyciesco
force-pushed
the
NY-788_send-test-welcome-emails
branch
from
4446c0b to
a45fed3
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
ghost/core/core/server/services/member-welcome-emails/service.js (1)
119-123: Consider extracting siteSettings construction to reduce duplication.The siteSettings object construction is duplicated between the
send()method (lines 62-66) and thissendTestEmail()method. Consider extracting this into a private helper method.Apply this refactor to reduce duplication:
+ #buildSiteSettings() { + return { + title: settingsCache.get('title') || 'Ghost', + url: urlUtils.urlFor('home', true), + accentColor: settingsCache.get('accent_color') || '#15212A' + }; + } + async send({member, memberStatus}) { // ... - const siteSettings = { - title: settingsCache.get('title') || 'Ghost', - url: urlUtils.urlFor('home', true), - accentColor: settingsCache.get('accent_color') || '#15212A' - }; + const siteSettings = this.#buildSiteSettings(); // ... } async sendTestEmail({email, automatedEmailId}) { // ... - const siteSettings = { - title: settingsCache.get('title') || 'Ghost', - url: urlUtils.urlFor('home', true), - accentColor: settingsCache.get('accent_color') || '#15212A' - }; + const siteSettings = this.#buildSiteSettings(); // ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/admin-x-framework/src/api/automated-emails.ts(1 hunks)apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx(5 hunks)ghost/core/core/server/api/endpoints/automated-emails.js(2 hunks)ghost/core/core/server/data/schema/fixtures/fixtures.json(1 hunks)ghost/core/core/server/services/member-welcome-emails/service.js(1 hunks)ghost/core/core/server/web/api/endpoints/admin/routes.js(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 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/core/server/services/member-welcome-emails/service.js
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: API routes should be defined in `ghost/core/core/server/api/`
Applied to files:
ghost/core/core/server/web/api/endpoints/admin/routes.js
🧬 Code graph analysis (3)
apps/admin-x-framework/src/api/automated-emails.ts (1)
apps/admin-x-framework/src/utils/api/hooks.ts (1)
createMutation(184-215)
ghost/core/core/server/api/endpoints/automated-emails.js (4)
ghost/core/core/server/services/member-welcome-emails/service.js (2)
require(9-9)require(11-11)ghost/core/core/server/web/api/endpoints/admin/routes.js (1)
require(3-3)ghost/core/core/server/api/endpoints/utils/validators/input/automated_emails.js (1)
require(4-4)ghost/core/test/e2e-api/admin/automated-emails.test.js (1)
require(1-1)
apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx (1)
apps/admin-x-framework/src/api/automated-emails.ts (1)
useSendTestWelcomeEmail(52-56)
⏰ 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). (10)
- GitHub Check: ActivityPub tests
- GitHub Check: Acceptance tests (Node 22.18.0, sqlite3)
- GitHub Check: Acceptance tests (Node 22.18.0, mysql8)
- GitHub Check: Admin-X Settings tests
- GitHub Check: Ghost-CLI tests
- GitHub Check: Legacy tests (Node 22.18.0, sqlite3)
- GitHub Check: Legacy tests (Node 22.18.0, mysql8)
- GitHub Check: Lint
- GitHub Check: Unit tests (Node 22.18.0)
- GitHub Check: Build & Push Docker Image
🔇 Additional comments (7)
ghost/core/core/server/data/schema/fixtures/fixtures.json (1)
539-543: LGTM!The new permission entry for sending test welcome emails is properly structured and follows the established pattern used for other permissions in the system.
ghost/core/core/server/web/api/endpoints/admin/routes.js (1)
186-186: LGTM!The new test email route is properly secured with brute-force protection and admin authentication, following the same pattern as the email preview test endpoint (line 343).
apps/admin-x-framework/src/api/automated-emails.ts (1)
52-56: LGTM!The mutation hook is properly typed and follows the established pattern for API mutations in this file.
ghost/core/core/server/api/endpoints/automated-emails.js (3)
4-4: LGTM!The import is necessary for the new sendTestEmail functionality.
142-148: Service initialization is idempotent and safe.The
memberWelcomeEmailService.init()method uses a guard clause (if (this.api) { return; }) to ensure the service is only initialized once. Subsequent calls on every request are harmless and incur negligible overhead.
139-141: Remove this review comment—the factual basis is incorrect.The fixtures.json does not contain a
sendTestEmailpermission forautomated_email. The permissions defined forautomated_emailare:browse,read,edit,add, anddestroy(lines 516–537). ThesendTestEmailaction type only exists foremail_previewresource (line 471), not forautomated_email.The endpoint correctly uses
method: 'edit'which is a valid permission for theautomated_emailresource. There is no inconsistency with email preview endpoints—they use different resources (email_previewvsautomated_email) with different permission models.Likely an incorrect or invalid review comment.
apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx (1)
2-2: LGTM!The imports and state management for the test email functionality are well-structured.
Also applies to: 11-11, 50-50, 53-54
apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx
Show resolved
Hide resolved
apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx
Show resolved
Hide resolved
| const lexical = automatedEmail.get('lexical'); | ||
| const subject = automatedEmail.get('subject'); | ||
|
|
||
| const siteSettings = { | ||
| title: settingsCache.get('title') || 'Ghost', | ||
| url: urlUtils.urlFor('home', true), | ||
| accentColor: settingsCache.get('accent_color') || '#15212A' | ||
| }; | ||
|
|
||
| const testMember = { | ||
| name: 'Jamie Larson', | ||
| email: email | ||
| }; | ||
|
|
||
| const {html, text, subject: renderedSubject} = await this.#renderer.render({ | ||
| lexical, | ||
| subject, | ||
| member: testMember, | ||
| siteSettings | ||
| }); |
There was a problem hiding this comment.
Validate lexical content before rendering.
The method doesn't verify that lexical contains valid content before passing it to the renderer. If the AutomatedEmail has a null or empty lexical field, the renderer might fail or produce unexpected output.
Consider adding validation after line 117:
const lexical = automatedEmail.get('lexical');
const subject = automatedEmail.get('subject');
+
+if (!lexical) {
+ throw new errors.ValidationError({
+ message: 'Email content is required to send a test email'
+ });
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const lexical = automatedEmail.get('lexical'); | |
| const subject = automatedEmail.get('subject'); | |
| const siteSettings = { | |
| title: settingsCache.get('title') || 'Ghost', | |
| url: urlUtils.urlFor('home', true), | |
| accentColor: settingsCache.get('accent_color') || '#15212A' | |
| }; | |
| const testMember = { | |
| name: 'Jamie Larson', | |
| email: email | |
| }; | |
| const {html, text, subject: renderedSubject} = await this.#renderer.render({ | |
| lexical, | |
| subject, | |
| member: testMember, | |
| siteSettings | |
| }); | |
| const lexical = automatedEmail.get('lexical'); | |
| const subject = automatedEmail.get('subject'); | |
| if (!lexical) { | |
| throw new errors.ValidationError({ | |
| message: 'Email content is required to send a test email' | |
| }); | |
| } | |
| const siteSettings = { | |
| title: settingsCache.get('title') || 'Ghost', | |
| url: urlUtils.urlFor('home', true), | |
| accentColor: settingsCache.get('accent_color') || '#15212A' | |
| }; | |
| const testMember = { | |
| name: 'Jamie Larson', | |
| email: email | |
| }; | |
| const {html, text, subject: renderedSubject} = await this.#renderer.render({ | |
| lexical, | |
| subject, | |
| member: testMember, | |
| siteSettings | |
| }); |
🤖 Prompt for AI Agents
In ghost/core/core/server/services/member-welcome-emails/service.js around lines
116 to 135, the code passes automatedEmail.get('lexical') to the renderer
without validating it; add a check after retrieving lexical (line ~117) to
ensure it is a non-empty string or contains expected content, and if not, either
assign a safe default lexical/template or return/throw a clear error so
rendering is not attempted with null/empty input; include a brief log or error
message indicating the automated email id/subject is missing lexical content to
aid debugging.
0677b52 to
56087c9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
ghost/core/core/server/services/member-welcome-emails/service.js (1)
116-135: Validatelexicalcontent before rendering.The method retrieves
lexicalfrom the automated email but doesn't validate it before passing to the renderer. Iflexicalisnullor empty, the renderer may fail or produce unexpected output. This mirrors the validation done inloadMemberWelcomeEmails(line 28).const lexical = automatedEmail.get('lexical'); const subject = automatedEmail.get('subject'); + +if (!lexical) { + throw new errors.ValidationError({ + message: MESSAGES.INVALID_LEXICAL_STRUCTURE + }); +}
🧹 Nitpick comments (1)
ghost/core/core/server/services/member-welcome-emails/service.js (1)
119-123: Consider extractingsiteSettingsconstruction to a helper.This block duplicates the same logic from lines 62-66 in the
sendmethod. A small private helper like#getSiteSettings()would reduce duplication.+ #getSiteSettings() { + return { + title: settingsCache.get('title') || 'Ghost', + url: urlUtils.urlFor('home', true), + accentColor: settingsCache.get('accent_color') || '#15212A' + }; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/admin-x-framework/src/api/automated-emails.ts(1 hunks)apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx(5 hunks)ghost/core/core/server/api/endpoints/automated-emails.js(2 hunks)ghost/core/core/server/services/member-welcome-emails/service.js(1 hunks)ghost/core/core/server/web/api/endpoints/admin/routes.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- ghost/core/core/server/web/api/endpoints/admin/routes.js
- apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 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/core/server/services/member-welcome-emails/service.js
🧬 Code graph analysis (2)
apps/admin-x-framework/src/api/automated-emails.ts (1)
apps/admin-x-framework/src/utils/api/hooks.ts (1)
createMutation(184-215)
ghost/core/core/server/services/member-welcome-emails/service.js (1)
ghost/core/core/server/services/member-welcome-emails/constants.js (2)
MEMBER_WELCOME_EMAIL_LOG_KEY(1-1)MESSAGES(8-13)
⏰ 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). (7)
- GitHub Check: Admin-X Settings tests
- GitHub Check: Ghost-CLI tests
- GitHub Check: Acceptance tests (Node 22.18.0, mysql8)
- GitHub Check: Legacy tests (Node 22.18.0, mysql8)
- GitHub Check: Acceptance tests (Node 22.18.0, sqlite3)
- GitHub Check: Unit tests (Node 22.18.0)
- GitHub Check: Build & Push Docker Image
🔇 Additional comments (3)
ghost/core/core/server/services/member-welcome-emails/service.js (1)
105-144: Well-structured test email implementation.The method follows the existing patterns in the service, with appropriate logging, error handling, and the
[Test]subject prefix to clearly identify test emails. The hardcoded test member name "Jamie Larson" is a reasonable approach for previews.apps/admin-x-framework/src/api/automated-emails.ts (1)
52-56: LGTM!The mutation hook correctly follows the established patterns in this file. Using
unknownfor the response type is appropriate since the endpoint returns 204 No Content, and omitting cache invalidation is correct as test emails don't modify any data.ghost/core/core/server/api/endpoints/automated-emails.js (1)
142-148: Service initialization pattern is correct.The
init()call before accessingapi.sendTestEmail()follows the wrapper pattern used throughout Ghost services, ensuring the service instance is created before use.
troyciesco
force-pushed
the
NY-788_send-test-welcome-emails
branch
from
56087c9 to
18e8dc5
Compare
troyciesco
force-pushed
the
NY-788_send-test-welcome-emails
branch
2 times, most recently
from
1a4edeb to
ac0258d
Compare
troyciesco
force-pushed
the
NY-788_send-test-welcome-emails
branch
from
ac0258d to
5e3dc97
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
ghost/core/core/server/services/member-welcome-emails/service.js (1)
105-144: Add a defensive check for missinglexicalbefore rendering test emailsThe new
sendTestEmailflow looks good overall: it reuses the existing renderer/mailer pipeline, logs clearly, and prefixes subjects with[Test]to distinguish these messages.There are two edge cases worth tightening up:
Missing lexical content (duplicate of earlier review concern)
You readconst lexical = automatedEmail.get('lexical');and pass it straight intothis.#renderer.render(...). Iflexicalis null/empty, the renderer may fail or produce confusing output. Adding a simple guard here (and returning a clearValidationError/IncorrectUsageErrormessage) would make failures deterministic and easier to debug.Non–welcome automated emails
AutomatedEmail.findOne({id: automatedEmailId})does not verify that the record’s slug/type matches one ofMEMBER_WELCOME_EMAIL_SLUGS. That means any automated email id could be used with the member‑welcome renderer. If that’s not intended, consider asserting the slug is one of the known welcome‑email slugs and throwing aNotFoundError(or a more specific error) otherwise.Both of these are relatively small guards that would make the endpoint more robust without changing the main behavior.
🧹 Nitpick comments (1)
ghost/core/core/server/api/endpoints/utils/validators/input/automated_emails.js (1)
4-5: Email validation forsendTestEmailis correctly moved into the custom validator; consider error type consistencyThe
sendTestEmailvalidator doingtypeof email === 'string' && validator.isEmail(email)and throwing on failure is aligned with how other endpoints handle email format checks and matches the earlier review direction to keep this logic in the validator module.One minor point to double‑check: other validations in this file use
ValidationError, while this path usesBadRequestError. If the API layer or clients rely on consistent error classes for input issues, you may want to standardize on one of them here (or confirm thatBadRequestErroris the intended distinction for this endpoint).Also applies to: 16-18, 69-77
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ghost/core/test/e2e-api/admin/__snapshots__/automated-emails.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (7)
apps/admin-x-framework/src/api/automated-emails.ts(1 hunks)apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx(4 hunks)ghost/core/core/server/api/endpoints/automated-emails.js(2 hunks)ghost/core/core/server/api/endpoints/utils/validators/input/automated_emails.js(3 hunks)ghost/core/core/server/services/member-welcome-emails/service.js(1 hunks)ghost/core/core/server/web/api/endpoints/admin/routes.js(1 hunks)ghost/core/test/e2e-api/admin/automated-emails.test.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- ghost/core/test/e2e-api/admin/automated-emails.test.js
- apps/admin-x-framework/src/api/automated-emails.ts
- ghost/core/core/server/web/api/endpoints/admin/routes.js
- apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx
🧰 Additional context used
🧠 Learnings (5)
📚 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/core/server/services/member-welcome-emails/service.jsghost/core/core/server/api/endpoints/automated-emails.js
📚 Learning: 2025-11-10T23:10:17.470Z
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.
Applied to files:
ghost/core/core/server/api/endpoints/automated-emails.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/core/server/api/endpoints/automated-emails.js
📚 Learning: 2025-06-13T11:57:58.226Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 23824
File: ghost/core/core/server/services/email-service/email-templates/partials/styles.hbs:919-926
Timestamp: 2025-06-13T11:57:58.226Z
Learning: In `ghost/core/core/server/services/email-service/email-templates/partials/styles.hbs`, some style blocks (e.g., `.latest-post p` and `.latest-post p a`) still use the legacy colour `#73818c` on purpose; they are later overridden by `emailCustomization` feature rules, as noted by inline TODO comments. These occurrences should not be flagged as inconsistencies.
Applied to files:
ghost/core/core/server/api/endpoints/automated-emails.js
📚 Learning: 2025-07-09T18:06:09.856Z
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 24291
File: apps/admin-x-settings/package.json:45-45
Timestamp: 2025-07-09T18:06:09.856Z
Learning: In Ghost admin-x-settings, webhook URL validation uses {require_tld: false} to allow localhost URLs for local integration testing, while social URL validation uses the default {require_tld: true} since social URLs should be public-facing.
Applied to files:
ghost/core/core/server/api/endpoints/automated-emails.js
🧬 Code graph analysis (2)
ghost/core/core/server/api/endpoints/utils/validators/input/automated_emails.js (1)
ghost/core/core/server/api/endpoints/automated-emails.js (2)
tpl(1-1)messages(6-8)
ghost/core/core/server/api/endpoints/automated-emails.js (3)
ghost/core/core/server/api/endpoints/utils/validators/input/automated_emails.js (1)
require(5-5)ghost/core/core/server/services/member-welcome-emails/service.js (2)
require(9-9)require(11-11)ghost/core/core/server/web/api/endpoints/admin/routes.js (1)
require(3-3)
⏰ 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). (8)
- GitHub Check: Legacy tests (Node 22.18.0, mysql8)
- GitHub Check: Legacy tests (Node 22.18.0, sqlite3)
- GitHub Check: Lint
- GitHub Check: Ghost-CLI tests
- GitHub Check: Admin-X Settings tests
- GitHub Check: ActivityPub tests
- GitHub Check: Unit tests (Node 22.18.0)
- GitHub Check: Build & Push Docker Image
🔇 Additional comments (1)
ghost/core/core/server/api/endpoints/automated-emails.js (1)
4-4: Endpoint wiring forsendTestEmaillooks consistent with existing patternsThe new
sendTestEmailcontroller action is wired cleanly:
- Uses
statusCode: 204with no cache, matching other “fire‑and‑forget” endpoints.- Accepts
options: ['id']anddata: ['email'], withidrequired and email validation delegated to theautomated_emailsinput validator.- Initializes the
memberWelcomeEmailServicewrapper before callingapi.sendTestEmail({ email, automatedEmailId: frame.options.id }), which matches how other services are used in this layer.- Permissions piggy‑back on the
editmethod, in line with the stated intent that editing and sending test welcome emails are coupled for admins.No issues from a controller/API‑layer perspective.
Also applies to: 116-143
no ref Original PR first saves the automated email before sending the test email, but that creates a scenario where you'll _also_ have the changes live, which you might not want. This PR, pointed at that one for comparison, switches the logic around so whatever is currently in the form for subject and lexical is what gets sent. This also happens to make validation and error/loading/success states a lot less jumpy.
troyciesco
force-pushed
the
NY-788_send-test-welcome-emails
branch
from
04cc23f to
a45ccd5
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
ghost/core/core/server/api/endpoints/utils/validators/input/automated_emails.js (1)
4-20: Test-email validator is solid; consider also enforcing lexical JSON validityThe new
sendTestEmailvalidator cleanly enforces:
- Well-formed email (via
isEmail).- Non-empty
subject.- Non-empty
lexical.That’s a good baseline.
One improvement to keep behavior aligned with
add/editand to avoid renderer-time failures: also ensurelexicalis valid JSON here, so malformed payloads result in a clear 400 error instead of bubbling up as a 500 when the renderer parses it.For example:
if (typeof lexical !== 'string' || !lexical.trim()) { throw new BadRequestError({ message: tpl(messages.lexicalRequired) }); } + + try { + JSON.parse(lexical); + } catch (e) { + throw new BadRequestError({ + message: tpl(messages.invalidLexical) + }); + }This keeps all input-shape validation in one place and makes the test endpoint more robust against bad clients.
Also applies to: 70-93
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ghost/core/test/e2e-api/admin/__snapshots__/automated-emails.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (7)
apps/admin-x-framework/src/api/automated-emails.ts(1 hunks)apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx(4 hunks)ghost/core/core/server/api/endpoints/automated-emails.js(2 hunks)ghost/core/core/server/api/endpoints/utils/validators/input/automated_emails.js(3 hunks)ghost/core/core/server/services/member-welcome-emails/service.js(1 hunks)ghost/core/core/server/web/api/endpoints/admin/routes.js(1 hunks)ghost/core/test/e2e-api/admin/automated-emails.test.js(3 hunks)
🧰 Additional context used
🧠 Learnings (19)
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: API routes should be defined in `ghost/core/core/server/api/`
Applied to files:
ghost/core/core/server/web/api/endpoints/admin/routes.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/core/server/services/member-welcome-emails/service.jsghost/core/core/server/api/endpoints/automated-emails.jsapps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Test suite names should follow the format 'Ghost Admin - Feature' or 'Ghost Public - Feature'
Applied to files:
ghost/core/test/e2e-api/admin/automated-emails.test.js
📚 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/automated-emails.test.js
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn test:integration` for integration tests in ghost/core
Applied to files:
ghost/core/test/e2e-api/admin/automated-emails.test.js
📚 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 Ghost's ActivityPub module are thoroughly tested in `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`, which covers `generatePendingActivityId`, `isPendingActivity`, and `generatePendingActivity` functions.
Applied to files:
ghost/core/test/e2e-api/admin/automated-emails.test.js
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Ensure tests have fresh Ghost instance isolation (automatic) and do not create test dependencies where Test B needs Test A
Applied to files:
ghost/core/test/e2e-api/admin/automated-emails.test.js
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Each test receives fresh Ghost instance for automatic isolation
Applied to files:
ghost/core/test/e2e-api/admin/automated-emails.test.js
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Use factory pattern for all test data creation instead of hard-coded data or direct database manipulation
Applied to files:
ghost/core/test/e2e-api/admin/automated-emails.test.js
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Manual login should not be used - authentication is automatic via fixture
Applied to files:
ghost/core/test/e2e-api/admin/automated-emails.test.js
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Use Playwright's auto-waiting capabilities and run tests multiple times to ensure stability
Applied to files:
ghost/core/test/e2e-api/admin/automated-emails.test.js
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Applies to e2e/**/*.{ts,js} : E2E tests should use Playwright with Docker container isolation
Applied to files:
ghost/core/test/e2e-api/admin/automated-emails.test.js
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Always refer to `.claude/E2E_TEST_WRITING_GUIDE.md` for comprehensive testing guidelines and patterns when creating or modifying E2E tests
Applied to files:
ghost/core/test/e2e-api/admin/automated-emails.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/core/server/api/endpoints/automated-emails.jsapps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx
📚 Learning: 2025-11-10T23:10:17.470Z
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.
Applied to files:
ghost/core/core/server/api/endpoints/automated-emails.jsapps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx
📚 Learning: 2025-06-13T11:57:58.226Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 23824
File: ghost/core/core/server/services/email-service/email-templates/partials/styles.hbs:919-926
Timestamp: 2025-06-13T11:57:58.226Z
Learning: In `ghost/core/core/server/services/email-service/email-templates/partials/styles.hbs`, some style blocks (e.g., `.latest-post p` and `.latest-post p a`) still use the legacy colour `#73818c` on purpose; they are later overridden by `emailCustomization` feature rules, as noted by inline TODO comments. These occurrences should not be flagged as inconsistencies.
Applied to files:
ghost/core/core/server/api/endpoints/automated-emails.js
📚 Learning: 2025-07-09T18:06:09.856Z
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 24291
File: apps/admin-x-settings/package.json:45-45
Timestamp: 2025-07-09T18:06:09.856Z
Learning: In Ghost admin-x-settings, webhook URL validation uses {require_tld: false} to allow localhost URLs for local integration testing, while social URL validation uses the default {require_tld: true} since social URLs should be public-facing.
Applied to files:
ghost/core/core/server/api/endpoints/automated-emails.js
📚 Learning: 2025-11-10T11:30:41.316Z
Learnt from: rob-ghost
Repo: TryGhost/Ghost PR: 25357
File: apps/admin/test-utils/test-helpers.ts:6-12
Timestamp: 2025-11-10T11:30:41.316Z
Learning: In apps/admin/test-utils/test-helpers.ts, the waitForQuerySettled helper is intentionally designed to timeout for idle/disabled queries. It should only treat queries as settled when they reach a terminal state (isSuccess or isError) and are not fetching. This ensures tests properly wait for active queries to complete.
Applied to files:
apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Never use hard-coded waits like `waitForTimeout()`
Applied to files:
apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx
🧬 Code graph analysis (4)
ghost/core/core/server/web/api/endpoints/admin/routes.js (2)
ghost/core/core/server/web/api/endpoints/content/routes.js (3)
router(12-12)mw(5-5)api(3-3)ghost/core/core/server/web/api/endpoints/admin/middleware.js (1)
shared(4-4)
ghost/core/test/e2e-api/admin/automated-emails.test.js (3)
ghost/core/core/server/api/endpoints/utils/validators/input/automated_emails.js (1)
require(5-5)ghost/core/core/server/services/member-welcome-emails/service.js (2)
require(9-9)require(11-11)ghost/core/core/server/web/api/endpoints/admin/routes.js (1)
require(3-3)
apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx (2)
apps/admin-x-framework/src/api/automated-emails.ts (1)
useSendTestWelcomeEmail(52-56)ghost/core/core/server/api/endpoints/utils/validators/input/automated_emails.js (1)
validator(4-4)
apps/admin-x-framework/src/api/automated-emails.ts (1)
apps/admin-x-framework/src/utils/api/hooks.ts (1)
createMutation(184-215)
⏰ 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). (10)
- GitHub Check: ActivityPub tests
- GitHub Check: Legacy tests (Node 22.18.0, mysql8)
- GitHub Check: Ghost-CLI tests
- GitHub Check: Admin-X Settings tests
- GitHub Check: Legacy tests (Node 22.18.0, sqlite3)
- GitHub Check: Unit tests (Node 22.18.0)
- GitHub Check: Lint
- GitHub Check: Acceptance tests (Node 22.18.0, sqlite3)
- GitHub Check: Acceptance tests (Node 22.18.0, mysql8)
- GitHub Check: Build & Push Docker Image
🔇 Additional comments (6)
ghost/core/core/server/web/api/endpoints/admin/routes.js (1)
182-188: New automated email test route wiring looks correctThe
POST /automated_emails/:id/testroute is wired consistently with other email test/preview routes (rate-limited viapreviewEmailLimiterand guarded byauthAdminApi), and correctly delegates toapi.automatedEmails.sendTestEmail. No issues from the routing/security perspective.ghost/core/core/server/services/member-welcome-emails/service.js (1)
105-142: sendTestEmail implementation is consistent with existing welcome-email flowThe
sendTestEmailmethod correctly:
- Verifies the
AutomatedEmailexists (404 otherwise).- Reuses the same
siteSettingspattern as the main send path.- Renders using the caller-supplied
lexicalandsubjectfor a synthetic test member.- Sends to the provided address with a
[Test]subject prefix.This matches the intended behavior for testing unsaved welcome email content without impacting real members.
ghost/core/core/server/api/endpoints/automated-emails.js (1)
4-5: Controller-level wiring for sendTestEmail is soundThe new
sendTestEmailendpoint:
- Uses a 204 response with
cacheInvalidate: false, consistent with other “test/send-only” actions.- Requires
options.idand passes it through asautomatedEmailId.- Delegates all business logic to
memberWelcomeEmailService.api.sendTestEmailafter initialization.- Uses
{method: 'edit'}permissions, aligning with the PR’s intent that edit + sendTestEmail are coupled for member welcome emails.No issues spotted here.
Also applies to: 116-147
ghost/core/test/e2e-api/admin/automated-emails.test.js (1)
3-5: E2E coverage for SendTestEmail is comprehensive and isolation-safe
- Truncating
bruteandautomated_emailsinbeforeEachkeeps the rate-limiter/table state clean between tests.- Stubbing
GhostMailer.prototype.sendavoids real email sends while still exercising the end-to-end flow.- The
SendTestEmailsuite covers:
- 204 success for a valid request.
- 404 for non-existent automated email id.
- 400s for missing email, invalid email, missing subject, and missing lexical.
This gives solid confidence in the new endpoint’s behavior.
Also applies to: 36-39, 320-472
apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx (1)
2-3: Test-welcome-email UI flow is well-integrated and handles state/cleanup correctly
- Client-side checks (email via
validator.isEmailanduseForm’s subject/lexical validation) prevent most bad requests before hitting the API.sendState+sendStateTimeoutRefgive clear “sending/sent” feedback, and the unmount cleanup (useEffectwithclearTimeout) avoids lingering timers or setState-on-unmounted issues.- Error handling maps JSON API errors (including validation messages from the new endpoint) into concise user-visible copy, falling back to a generic message when needed.
- The dropdown, test email field, and Send button are wired cleanly into the existing modal without interfering with the main save flow.
No changes needed here.
Also applies to: 9-13, 47-58, 62-85, 87-93, 95-109, 116-127, 129-167, 187-216
apps/admin-x-framework/src/api/automated-emails.ts (1)
52-56: API hook matches backend contract for test welcome emails
useSendTestWelcomeEmailcorrectly:
- Targets
/automated_emails/{id}/test/with method POST.- Sends
{email, subject, lexical}in the body, matching the server’s expected payload.- Uses
unknownresponse type, appropriate for a 204/empty response.Hook signature lines up with the modal’s usage and the new backend endpoint.
closes https://linear.app/ghost/issue/NY-788
sendTestEmail- where edit and sendTestEmail can be pretty distinct on posts depending the permissions of the staff user, in the case of member welcome emails the concepts are pretty tightly coupled (and only admins have permission anyway)