Updated gift subscription reminder email copy and layout#27509
Merged
minimaluminium merged 3 commits intomainfrom Apr 23, 2026
Merged
Updated gift subscription reminder email copy and layout#27509minimaluminium merged 3 commits intomainfrom
minimaluminium merged 3 commits intomainfrom
Conversation
- rewrote body copy to "Continue with a paid subscription to keep reading", making the next step explicit and matching the new CTA
- renamed CTA from "Manage subscription" to "Continue subscription" to match the button being added on the Portal account page and to reflect that a gift member has no subscription to manage yet
- removed the conditional "Hi {name}," greeting so the h1 leads directly; most gift redeemers don't have a name on file so the conditional was branching for an edge case
- the cadence row (e.g. "Bronze • 3 months") described the gift that was already received, not what the redeemer would pay to continue; the end date already communicates the gift length, so it was replaced with a "Price after gift ends" row - added a "Price after gift ends" row so the redeemer can see the ongoing subscription price without clicking through to Portal — reduces surprise at checkout and helps the decision - wired tier currency and monthly/yearly price through the reminder flow and picked the price matching the gift cadence - if pricing has been nulled between purchase and reminder, fail fast before marking the gift as reminded so the next run recovers once an admin restores pricing
Contributor
WalkthroughRemoved member name personalization and cadence label from gift reminder templates and data. Introduced post-gift pricing fields ( 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
ghost/core/core/server/services/gifts/gift-service.ts (1)
495-503: Tighten the error message to reflect both failure modes.The guard fires when either the cadence-specific price or the tier's currency is missing, but the message only mentions "pricing". A currency-only failure will emit a misleading
Tier missing yearly pricing for gift: ...log line that points an on-call admin at the wrong field. Including which field is actually null makes the recovery path (restore pricing vs. restore currency) obvious from the log.♻️ Proposed message tweak
- // Throw before the transaction so the gift isn't marked as reminded; - // the next run recovers after an admin restores pricing. - const tierPrice = gift.cadence === 'month' ? tier.monthlyPrice : tier.yearlyPrice; - - if (tierPrice === null || tier.currency === null) { - throw new errors.NotFoundError({ - message: `Tier missing ${gift.cadence}ly pricing for gift: ${gift.tierId}` - }); - } + // Throw before the transaction so the gift isn't marked as reminded; + // the next run recovers after an admin restores pricing. + const tierPrice = gift.cadence === 'month' ? tier.monthlyPrice : tier.yearlyPrice; + + if (tierPrice === null || tier.currency === null) { + const missing = [ + tierPrice === null ? `${gift.cadence}ly price` : null, + tier.currency === null ? 'currency' : null + ].filter(Boolean).join(' and '); + throw new errors.NotFoundError({ + message: `Tier missing ${missing} for gift: ${gift.tierId}` + }); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ghost/core/core/server/services/gifts/gift-service.ts` around lines 495 - 503, The thrown NotFoundError in gift-service.ts currently only mentions missing pricing even though the guard checks both cadence-specific price (tierPrice) and tier.currency; update the error construction (the throw new errors.NotFoundError call) to include which field(s) are null (e.g., "missing monthly/yearly pricing" and/or "missing currency") by inspecting tierPrice and tier.currency and composing a clear message that references gift.tierId so operators know whether to restore pricing or currency.ghost/core/test/unit/server/services/gifts/gift-service.test.ts (1)
914-936: Consider also covering thecurrency === nullbranch.
sendReminderForGiftthrows when eithertierPriceortier.currencyis null, but this suite only exercises the missing-price branch. A sibling case withcurrency: null(and valid prices) would lock in both halves of the guard and protect against future regressions where one side is relaxed independently.♻️ Proposed additional test
+ it('does not mark the gift as reminded when tier currency is missing', async function () { + const gift = buildRedeemedGift({cadence: 'year'}); + + giftRepository.findPendingReminder.resolves([gift]); + giftRepository.getByToken.resolves(gift); + tiersService.api.read.resolves({ + id: 'tier_1', + name: 'Bronze', + currency: null, + monthlyPrice: 1000, + yearlyPrice: 10000 + }); + + const service = createService(); + const result = await service.processReminders(); + + assert.equal(result.remindedCount, 0); + assert.equal(result.skippedCount, 0); + assert.equal(result.failedCount, 1); + + sinon.assert.notCalled(giftRepository.update); + sinon.assert.notCalled(giftEmailService.sendReminder); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ghost/core/test/unit/server/services/gifts/gift-service.test.ts` around lines 914 - 936, Add a sibling unit test to cover the currency === null branch: in gift-service.test.ts create a new it(...) similar to the existing "does not mark the gift as reminded when tier pricing is missing for the gift cadence" but have tiersService.api.read resolve with a valid price (e.g., monthlyPrice or yearlyPrice non-null matching the gift cadence) and currency: null; keep giftRepository.findPendingReminder and getByToken resolving the gift and call service.processReminders(), then assert remindedCount is 0, skippedCount is 0, failedCount is 1 and that giftRepository.update and giftEmailService.sendReminder were not called to ensure sendReminderForGift’s guard on tier.currency is exercised.
🤖 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/gifts/gift-service.ts`:
- Around line 495-503: The thrown NotFoundError in gift-service.ts currently
only mentions missing pricing even though the guard checks both cadence-specific
price (tierPrice) and tier.currency; update the error construction (the throw
new errors.NotFoundError call) to include which field(s) are null (e.g.,
"missing monthly/yearly pricing" and/or "missing currency") by inspecting
tierPrice and tier.currency and composing a clear message that references
gift.tierId so operators know whether to restore pricing or currency.
In `@ghost/core/test/unit/server/services/gifts/gift-service.test.ts`:
- Around line 914-936: Add a sibling unit test to cover the currency === null
branch: in gift-service.test.ts create a new it(...) similar to the existing
"does not mark the gift as reminded when tier pricing is missing for the gift
cadence" but have tiersService.api.read resolve with a valid price (e.g.,
monthlyPrice or yearlyPrice non-null matching the gift cadence) and currency:
null; keep giftRepository.findPendingReminder and getByToken resolving the gift
and call service.processReminders(), then assert remindedCount is 0,
skippedCount is 0, failedCount is 1 and that giftRepository.update and
giftEmailService.sendReminder were not called to ensure sendReminderForGift’s
guard on tier.currency is exercised.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 029cfd48-a8ee-4443-a44a-5b4a3306c5f2
📒 Files selected for processing (6)
ghost/core/core/server/services/gifts/email-templates/gift-reminder.hbsghost/core/core/server/services/gifts/email-templates/gift-reminder.tsghost/core/core/server/services/gifts/gift-email-service.tsghost/core/core/server/services/gifts/gift-service.tsghost/core/test/unit/server/services/gifts/gift-email-service.test.jsghost/core/test/unit/server/services/gifts/gift-service.test.ts
sagzy
reviewed
Apr 23, 2026
| throw new errors.NotFoundError({ | ||
| message: `Tier missing ${gift.cadence}ly pricing for gift: ${gift.tierId}` | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
nit: currency/price would be missing only happen if the tier was "Free". We should probably do that check on the gift purchase rather than here, but it also doesn't harm to have an extra check here 👍
Member
Author
There was a problem hiding this comment.
👍 fair point, leaving in as a safety net for now
- removed the "does not include a greeting" test: it asserted the absence of something deleted in the previous commit, which is low-signal - renamed the CTA test to "renders a 'Continue subscription' CTA" for clarity, dropping "new" since it ages out after merge
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ghost/core/test/unit/server/services/gifts/gift-email-service.test.js (1)
177-187: Mirror these formatting assertions against the text body too.The default price test covers both bodies, but the monthly and EUR edge cases only check HTML. Adding text assertions keeps the plain-text reminder from drifting.
🧪 Proposed test coverage tweak
it('formats month cadence in the post-gift price', async function () { await service.sendReminder({...reminderData, cadence: 'month', tierPrice: 1000}); sinon.assert.calledWith(mailer.send, sinon.match.has('html', sinon.match('$10.00/month'))); + sinon.assert.calledWith(mailer.send, sinon.match.has('text', sinon.match('$10.00/month'))); }); it('formats non-USD currency correctly in the post-gift price', async function () { await service.sendReminder({...reminderData, tierCurrency: 'eur', tierPrice: 1500}); sinon.assert.calledWith(mailer.send, sinon.match.has('html', sinon.match('€15.00/year'))); + sinon.assert.calledWith(mailer.send, sinon.match.has('text', sinon.match('€15.00/year'))); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ghost/core/test/unit/server/services/gifts/gift-email-service.test.js` around lines 177 - 187, The tests for monthly cadence and non-USD currency only assert the formatted price appears in the HTML body; update the two it blocks that call service.sendReminder (the tests referencing cadence: 'month' and tierCurrency: 'eur' / tierPrice) to also assert the same formatted string appears in the plain-text body by adding sinon.assert.calledWith(mailer.send, sinon.match.has('text', sinon.match(...))) mirroring the existing sinon.match.has('html', ...) assertions for '$10.00/month' and '€15.00/year' respectively.
🤖 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/test/unit/server/services/gifts/gift-email-service.test.js`:
- Around line 177-187: The tests for monthly cadence and non-USD currency only
assert the formatted price appears in the HTML body; update the two it blocks
that call service.sendReminder (the tests referencing cadence: 'month' and
tierCurrency: 'eur' / tierPrice) to also assert the same formatted string
appears in the plain-text body by adding sinon.assert.calledWith(mailer.send,
sinon.match.has('text', sinon.match(...))) mirroring the existing
sinon.match.has('html', ...) assertions for '$10.00/month' and '€15.00/year'
respectively.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c931597b-8192-4914-b528-1da25eddf813
📒 Files selected for processing (1)
ghost/core/test/unit/server/services/gifts/gift-email-service.test.js
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
no issues
Follow-up to #27436 with a few design tweaks to the new gift reminder email. Also aligns the CTA with the in-progress "Continue subscription" button on the Portal account page.