Collect gift email delivery outcomes - #29852
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 |
04758b8 to
ae1f5fb
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: 04758b8fac
ℹ️ 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".
| if (labs.isSet('giftSubCustomization')) { | ||
| gifts.init({ |
There was a problem hiding this comment.
When giftSubCustomization is disabled during boot and later enabled through the Labs UI, this conditional permanently skips gifts.init() for the process lifetime. Accepted deliveries then call scheduleRecurringGiftDeliveriesJob(true) and register the worker, but no subscriber exists for StartGiftEmailAnalyticsJobEvent, so every run is a no-op and delivery outcomes remain unknown until Ghost restarts. Initialize the wrapper unconditionally and gate only job scheduling/processing, or initialize it when the flag changes.
Useful? React with 👍 / 👎.
| const updated = await transacting('gifts') | ||
| .where({email_provider_message_id: providerMessageId}) | ||
| .where((builder) => { | ||
| builder.whereNull('delivery_outcome_at').orWhere('delivery_outcome_at', '<', timestamp); |
There was a problem hiding this comment.
On MySQL-backed sites, when two Mailgun outcomes for the same message occur within one second and an older event is later replayed, delivery_outcome_at loses the provider timestamp's milliseconds because Ghost's dateTime column has whole-second precision. For example, after storing an outcome from 12:00:00.900 as 12:00:00, a replay of an older 12:00:00.100 event satisfies this < timestamp predicate and overwrites the newer result, contrary to the newest-outcome contract. Store comparable provider precision or use an ordering value that survives persistence.
Useful? React with 👍 / 👎.
| gifts.init({ | ||
| event: StartGiftEmailAnalyticsJobEvent, | ||
| mailgunTags: ['gift-delivery'], |
There was a problem hiding this comment.
When gift mail is sent through Ghost's transactional mail.transport=Mailgun configuration but newsletter Mailgun is absent or uses a different account/domain, this wrapper cannot find the resulting events. GiftEmailService sends with GhostMailer, which reads config.mail.options, while EmailAnalyticsServiceWrapper constructs MailgunClient, whose only credential sources are bulkEmail.mailgun and the mailgun_* newsletter settings. The recurring job therefore either reports Mailgun as unconfigured or searches the wrong domain, leaving every accepted gift's outcome unknown; the gift collector needs to use the same Mailgun credentials and domain as the transport that produced its message ID.
Useful? React with 👍 / 👎.
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run ghost:test:ci:integration |
✅ Succeeded | 2m 33s | View ↗ |
nx run ghost:test:integration |
✅ Succeeded | 2m 47s | View ↗ |
nx run ghost:test:e2e |
✅ Succeeded | 2m 26s | View ↗ |
nx run ghost:test:legacy |
✅ Succeeded | 2m 29s | View ↗ |
nx run ghost-monorepo:lint:boundaries |
✅ Succeeded | 22s | View ↗ |
nx run-many -t lint -p ghost,ghost-monorepo |
✅ Succeeded | 20s | View ↗ |
nx run-many -t test:unit -p ghost |
✅ Succeeded | 34s | View ↗ |
nx run @tryghost/admin:build |
✅ Succeeded | 4s | View ↗ |
nx run-many --target=build --projects=tag:publi... |
✅ Succeeded | <1s | 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:57:22 UTC
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## agent/ber-3851-delivery-lifecycle #29852 +/- ##
=====================================================================
+ Coverage 75.37% 75.39% +0.01%
=====================================================================
Files 1614 1617 +3
Lines 141889 142057 +168
Branches 17583 17596 +13
=====================================================================
+ Hits 106949 107104 +155
- Misses 33888 33901 +13
Partials 1052 1052
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:
|
ae1f5fb to
636cd8f
Compare
636cd8f to
cb5d7cd
Compare
23de72d to
a162118
Compare
a162118 to
37823f5
Compare
ref https://linear.app/ghost/issue/BER-3851/establish-immediate-email-delivery-for-gift-subscriptions Mailgun telemetry is useful for diagnosis after acceptance, but it must remain separate from the durable sent fact and never trigger another delivery.
37823f5 to
ace1b32
Compare
1 participant
Summary
giftSubCustomizationis enabledStack
5 of 6. Stacked on #29851; feature-gated checkout activation is the final PR.
Issue: https://linear.app/ghost/issue/BER-3851/establish-immediate-email-delivery-for-gift-subscriptions