Process and schedule gift email deliveries - #29851
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
github-actions
Bot
added
the
migration
|
It looks like this PR contains a migration 👀 General requirements
Schema changes
Data changes
|
fcd82ee to
eb53f62
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fcd82eee8a
ℹ️ 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".
|
|
||
| try { | ||
| const key = await this.#internalKeys.get('ghost-scheduler'); | ||
| this.#adapter.schedule(this.#buildJob(time.getTime(), key)); |
There was a problem hiding this comment.
When obtaining the scheduler key or queueing the job fails, scheduleAt only logs the error and resolves; moreover, the scheduler adapter contract permits schedule() to return a rejecting promise, which this un-awaited call cannot catch. Because retryDelivery has already moved the gift to pending with a future delivery_attempt_at and there is no periodic delivery poll, a failed one-shot leaves that email undelivered until a restart, an unrelated flush, or another recovery pass. Surface the failure or arrange a fallback wake/poll so the persisted retry cannot be stranded.
Useful? React with 👍 / 👎.
| if (!labsService.isSet('giftSubCustomization')) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
If giftSubCustomization is disabled when a scheduled delivery callback fires, this subscriber returns while the endpoint still responds successfully, so the one-shot job is consumed without changing or re-scheduling the pending gift. Labs settings are updated at runtime, but re-enabling the flag does not call recoverAll; therefore a delivery that was already queued before a temporary disable remains pending until Ghost restarts or an unrelated flush occurs. Re-arm skipped work when the flag becomes enabled, or retain a periodic recovery path.
Useful? React with 👍 / 👎.
| const collection = await this.model.findAll({ | ||
| filter: 'status:purchased+delivery_method:email+delivery_status:pending' | ||
| }); |
There was a problem hiding this comment.
For sites with many future scheduled gifts or delayed retries, every one-shot flush loads every pending email gift because this query has no deliver_at or delivery_attempt_at cutoff. processDeliveries then opens a claim transaction for each result, even though claimPendingDelivery rejects all but the due rows; since each pending gift also has its own scheduled flush, processing N future gifts performs roughly N² reads and claim attempts over their lifetime. Use a due-only query for batch processing while retaining a separate all-pending query for startup schedule recovery.
Useful? React with 👍 / 👎.
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run ghost:test:ci:integration |
✅ Succeeded | 2m 40s | View ↗ |
nx run ghost:test:integration |
✅ Succeeded | 3m 6s | View ↗ |
nx run ghost:test:e2e |
✅ Succeeded | 2m 40s | View ↗ |
nx run ghost:test:legacy |
✅ Succeeded | 2m 57s | View ↗ |
nx run-many -t test:unit -p ghost |
✅ Succeeded | 33s | View ↗ |
nx run ghost-monorepo:lint:boundaries |
✅ Succeeded | 21s | View ↗ |
nx run @tryghost/admin:build |
✅ Succeeded | 5s | View ↗ |
nx run-many --target=build --projects=tag:publi... |
✅ Succeeded | 1s | View ↗ |
nx run-many -t lint -p ghost,ghost-monorepo |
✅ Succeeded | 21s | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-08-11 16:56:16 UTC
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## agent/ber-3851-delivery-email #29851 +/- ##
=================================================================
+ Coverage 75.32% 75.37% +0.05%
=================================================================
Files 1611 1614 +3
Lines 141142 141889 +747
Branches 17464 17583 +119
=================================================================
+ Hits 106311 106949 +638
- Misses 33753 33888 +135
+ Partials 1078 1052 -26
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
eb53f62 to
e947423
Compare
e947423 to
451853d
Compare
99a0a28 to
0353712
Compare
0353712 to
f704014
Compare
ref https://linear.app/ghost/issue/BER-3851/establish-immediate-email-delivery-for-gift-subscriptions Atomic claims and exact one-shot wakeups make delivery durable and retryable without representing gifts as automations or introducing polling.
ref https://linear.app/ghost/issue/BER-3851/establish-immediate-email-delivery-for-gift-subscriptions The lifecycle tests need to use the provider outcome error field introduced by the persistence layer rename.
ref https://linear.app/ghost/issue/BER-3851/establish-immediate-email-delivery-for-gift-subscriptions Delivery attempts now operate through their own repository so scheduling, concurrency, retries, and cancellation no longer overload the gift redemption model.
f704014 to
ec484c7
Compare
1 participant
Summary
giftSubCustomizationwith no checkout producer yetStack
4 of 6. Stacked on #29850; provider outcome collection is next.
Issue: https://linear.app/ghost/issue/BER-3851/establish-immediate-email-delivery-for-gift-subscriptions