Added paid welcome email for gift member signups#27648
Conversation
WalkthroughWelcome-email enqueueing when members are created or change status was refactored to select a 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ghost/core/test/e2e-api/members/gift-subscriptions.test.js (1)
846-856: ⚡ Quick winAdd the missing existing-member no-welcome assertion.
This proves the new-member gift path gets the paid welcome, but the suite still does not assert the PR’s “existing members redeeming gifts should not get a welcome” rule. A regression there would still pass.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ghost/core/test/e2e-api/members/gift-subscriptions.test.js` around lines 846 - 856, Add an assertion that existing members who redeem gifts do NOT get a welcome: after the existing-member gift redemption step, query models.WelcomeEmailAutomationRun for runs with member_id equal to the existing member's id (same pattern used for new-member check) and assert that the returned collection has length 0 (use assert.equal(welcomeRuns.length, 0) or equivalent) to prove no welcome automation was enqueued for the existing member.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ghost/core/test/e2e-api/members/gift-subscriptions.test.js`:
- Around line 779-808: The setup creates persistent WelcomeEmailAutomation and
WelcomeEmailAutomatedEmail rows (via models.WelcomeEmailAutomation.add and
models.WelcomeEmailAutomatedEmail.add, using slugs like
'member-welcome-email-free' and 'member-welcome-email-paid') before registering
any cleanup, so failures can leak those fixtures; fix by registering cleanup
handlers (or pushing their identifiers into the existing cleanup array/registry)
immediately before calling each add (or allocate the cleanup slot first) so that
if models.EmailDesignSetting.findOne or any add() throws, the cleanup routine
will still remove any partially created rows for those slugs; apply the same
pattern to the other occurrences noted (lines around uses of
WelcomeEmailAutomation.add / WelcomeEmailAutomatedEmail.add at the other
ranges).
---
Nitpick comments:
In `@ghost/core/test/e2e-api/members/gift-subscriptions.test.js`:
- Around line 846-856: Add an assertion that existing members who redeem gifts
do NOT get a welcome: after the existing-member gift redemption step, query
models.WelcomeEmailAutomationRun for runs with member_id equal to the existing
member's id (same pattern used for new-member check) and assert that the
returned collection has length 0 (use assert.equal(welcomeRuns.length, 0) or
equivalent) to prove no welcome automation was enqueued for the existing member.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fc733fff-dd83-4772-9707-e10cdaf1447b
📒 Files selected for processing (3)
ghost/core/core/server/services/members/members-api/repositories/member-repository.jsghost/core/test/e2e-api/members/gift-subscriptions.test.jsghost/core/test/unit/server/services/members/members-api/repositories/member-repository.test.js
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 281d37cf87
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
110f4ca to
2bac73a
Compare
4bf6db7 to
55c4bd2
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ghost/core/core/server/services/welcome-email-automations/poll.js (1)
209-210: ⚡ Quick winGuard missing eligibility mapping before calling
.includes()On Line 209,
eligibleStatusescan becomeundefinedif constants drift, and Line 210 would then throw at runtime and enter retry flow as a send failure. Add a defensive fallback/guard to avoid misclassifying config errors.Suggested patch
- const eligibleStatuses = MEMBER_WELCOME_EMAIL_ELIGIBLE_STATUSES[memberStatus]; - if (!eligibleStatuses.includes(member.get('status'))) { + const eligibleStatuses = MEMBER_WELCOME_EMAIL_ELIGIBLE_STATUSES[memberStatus]; + if (!Array.isArray(eligibleStatuses)) { + await markExited(run.id, 'email send failed'); + logging.error( + { + system: { + event: 'welcome_email_automations.missing_eligible_statuses', + member_status: memberStatus + } + }, + `${LOG_KEY} Missing eligible statuses config for member status: ${memberStatus}` + ); + return; + } + if (!eligibleStatuses.includes(member.get('status'))) { await markExited(run.id, 'member changed status'); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ghost/core/core/server/services/welcome-email-automations/poll.js` around lines 209 - 210, The code assumes MEMBER_WELCOME_EMAIL_ELIGIBLE_STATUSES[memberStatus] is always defined; guard against it by checking eligibleStatuses before calling .includes() (e.g. treat undefined as an empty array or early-return/skip and log a config error), so modify the block that computes eligibleStatuses and the conditional that uses eligibleStatuses.includes(member.get('status')) to first verify eligibleStatuses is an array (or use a safe fallback) and handle the misconfiguration path without throwing.
🤖 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/services/welcome-email-automations/poll.js`:
- Around line 209-210: The code assumes
MEMBER_WELCOME_EMAIL_ELIGIBLE_STATUSES[memberStatus] is always defined; guard
against it by checking eligibleStatuses before calling .includes() (e.g. treat
undefined as an empty array or early-return/skip and log a config error), so
modify the block that computes eligibleStatuses and the conditional that uses
eligibleStatuses.includes(member.get('status')) to first verify eligibleStatuses
is an array (or use a safe fallback) and handle the misconfiguration path
without throwing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eed57a68-f464-4905-bdea-08eacdf5f9d2
📒 Files selected for processing (6)
ghost/core/core/server/services/member-welcome-emails/constants.jsghost/core/core/server/services/members/members-api/repositories/member-repository.jsghost/core/core/server/services/welcome-email-automations/poll.jsghost/core/test/e2e-api/members/gift-subscriptions.test.jsghost/core/test/integration/services/welcome-email-automations/poll.test.jsghost/core/test/unit/server/services/members/members-api/repositories/member-repository.test.js
✅ Files skipped from review due to trivial changes (1)
- ghost/core/core/server/services/members/members-api/repositories/member-repository.js
🚧 Files skipped from review as they are similar to previous changes (3)
- ghost/core/core/server/services/member-welcome-emails/constants.js
- ghost/core/test/integration/services/welcome-email-automations/poll.test.js
- ghost/core/test/unit/server/services/members/members-api/repositories/member-repository.test.js
|
@sagzy with the changes that are currently in this branch: if someone was a pre-existing free member, and they redeem a gift, would they get a paid welcome email? I think no, and I assume we'd want them to - I'm still context loading and about to try it myself but figured I'd drop the question here. |
|
@troyciesco correct: this only covers new members redeeming a gift, not existing free members redeeming a gift. Intention was to keep the changeset ~small and reviewable and to cover the second path in a follow-up PR |
ref https://linear.app/ghost/issue/BER-3541
Summary
Send the paid welcome email when a new member redeems a gift subscription, and suppress
the duplicate paid welcome when that gift member subsequently starts a real paid
subscription.
Previously, gift members received no welcome email on signup.
Changes
All changes are localized to
member-repository.jsto keep welcome-email policyin one place —
gift-service.tsand the rest of the gift flow are untouched.create()— When a new member is created withstatus: 'gift'(gift-redemptionpath, no Stripe customer), enqueue the paid welcome email automation
(
member-welcome-email-paid) in the same transaction as member creation. Mirrorsthe existing free-signup branch; refactored both branches to share a single
transaction wrapper via a
welcomeEmailToEnqueueconfig.update()— Skip the paid welcome email on thegift → paidtransition byguarding on
updatedMember._previousAttributes.status !== 'gift'. Without thisguard, the same person would receive the paid welcome twice.
Test plan
pnpm test:single test/unit/server/services/members/members-api/repositories/member-repository.test.js— 52 passing.'create - automation run integration'block: gift signupnow asserts the paid automation slug is looked up and the run is enqueued
with the paid automation/email IDs.
'gift signup when paid welcome inactive'unit test (no run enqueued).'previous status was gift'unit test underlinkSubscription(no runenqueued on
gift → paid).free and paid welcome automations and assert exactly one paid run is enqueued
for the gift member.
'does not enqueue a second paid welcome email when a gift member upgrades to paid'covering the gift → paid suppression.body content.
the paid welcome email arrives in Mailpit (
http://localhost:8025).checkout. Confirm Mailpit shows no second paid welcome email.
gift, confirm member is created with
status: 'gift'and no welcome is sent.