Added email provider adapter wiring - #29553
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds an Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ghost/core/core/server/adapters/email/email-provider-base.js`:
- Around line 7-8: Update the email-provider contract in the constructor’s
requiredFns definition to include getMaximumRecipients and
getTargetDeliveryWindow alongside send, ensuring AdapterManager rejects
providers that omit methods used by SendingService.
🪄 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 Plus
Run ID: f00efc6d-1f90-46c3-86ac-4f91c991672f
📒 Files selected for processing (4)
ghost/core/core/server/adapters/email/email-provider-base.jsghost/core/core/server/services/adapter-manager/index.tsghost/core/core/server/services/email-service/email-service-wrapper.jsghost/core/test/unit/server/services/email-service/email-service-wrapper.test.js
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@compose.dev.ses.yaml`:
- Line 15: Update the adapters__email__ses__fromEmail configuration in
compose.dev.ses.yaml to remove the newsletter@example.test fallback; require
SES_FROM_EMAIL explicitly or replace the fallback with a verified development
sender address.
🪄 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 Plus
Run ID: 7f9c20ef-c64d-4393-8756-e95a3c84e148
📒 Files selected for processing (1)
compose.dev.ses.yaml
Allows community email providers while preserving Mailgun as the default.
Mounts the local adapter checkout at Ghost's container discovery path.
Rejects adapters that omit methods required by newsletter sending.
2353fad to
11f63f2
Compare
|
@acburdine, since you've been driving most of the recent adapter-manager work, tagging you for a look. This PR adds a minimal email adapter type to Scope is intentionally small:5 files, ~90 lines, one new required-fns contract in Why this is worth doing beyond just this one PR:right now Opening this one seam doesn't commit the core team to maintaining any of those providers, it lets the community carry that weight the same way it already does for storage and SSO adapters, while removing a real adoption blocker for self-hosters who'd otherwise have to fork core or skip newsletters entirely. First consumer is a standalone package ghost-ses-email-adapter; happy to help make sure the contract is generic enough for Postmark/Brevo/Postal to build on it too. Context:this follows the maintainer discussion in #25367, where the direction landed on "adapter mechanism in core, providers maintained externally" rather than bundling SES/Postmark/etc. into Ghost. One thing I can't do myself: CI (ci.yml, tinybird.yml) is sitting at action_required since I'm an external contributor on a fork, would appreciate an "Approve and run workflows" when you get a chance, so the required checks can actually run. Happy to adjust the approach if there's a preferred shape for this. |
|
hey @wakqasahmed, thanks for the effort here 😄 it's worth reading through this comment on a similar PR: #28247 (comment). At the moment, the main blocker to a standardized email adapter pattern isn't the email sending bits itself, it's the analytics/suppression bits that have proven to be more complex. At the moment, Ghost polls Mailgun's Events API for that analytics/suppression data, and we'd need to refactor some stuff in Ghost internally to allow for webhooks to be utilized instead in order to unblock full adapter support. We are looking into it, but we'd likely want to get that solved first before merging any adapter work 😕 |
|
Throwing the blocked label on this. This is an area of active discussion and we want to make sure we get this right, so the core team will likely take this over or adapt contributions. |
|
@acburdine @9larsons @belatedly I specifically hit the same blocker as #28247 (comment) and had to go to alternate like SES and found out there was a half baked adapter available in PRs, hence I completed it at https://www.github.com/wakqasahmed/ghost-ses-email-adapter and while doing so, I found out why so many are blocked at this gate. I totally understand the limitation and would love to help getting this blocker out. Let me (and others) help get this through fast, please. |
|
@acburdine @9larsons @belatedly would appreciate a suggested way forward and how can I be of help in this. Thanks |
|
Filed #29828 to track the webhook-based analytics/suppression ingestion path @acburdine and @9larsons flagged above as the actual blocker here (and on #28247). Scoped it as additive to the existing Mailgun poll loop, per @9larsons' "send-only seam, edges documented, scheduler untouched" framing on #28247, so it's a standalone reviewable unit of work rather than something that has to land inside this PR. Happy to help prototype it against ghost-ses-email-adapter if useful. |
ref TryGhost#29828 Additive to the existing Mailgun poll loop, per the send-only-seam / scheduler-untouched framing from TryGhost#28247 and TryGhost#29553: - adapters/email/email-provider-base.js gains optional verifyWebhookRequest()/parseWebhookEvents(), defaulting to unsupported rather than throwing, so Mailgun and any adapter that only implements send() are unaffected. - email-analytics-webhook-controller.js normalizes an adapter's webhook events and forwards them to the same EmailEventProcessor the poll loop already writes through, so analytics and the suppression list converge on one write path regardless of which ingestion mechanism produced the event. - Wired as POST /webhooks/email-analytics on the members app, mirroring the existing Stripe webhook route. - adapter-manager now registers an 'email' base class (also proposed standalone in TryGhost#29553), so this can compose with that PR instead of duplicating it. This is a prototype attached to the design discussion in TryGhost#29828, not a claim on the final shape - posted for concrete review, not as a finished implementation.
What does this PR do?
Adds the minimal email-provider adapter mechanism discussed in #25367. Ghost
continues to use Mailgun unless
adapters.emailis configured. When it isconfigured,
EmailServiceWrapperresolves the provider throughAdapterManager.This unlocks community-maintained email providers without bundling a provider,
analytics integration, or Admin UI changes in Ghost core. The first intended
consumer is ghost-ses-email-adapter.
Why is this needed?
The existing
AdapterManagercan discover third-party adapters, but the emailservice currently instantiates
MailgunEmailProviderdirectly. In themaintainer discussion on #25367, Ghost indicated that a generic adapter
mechanism is the preferred direction for self-hosted providers. This PR is
limited to that mechanism and leaves the default Mailgun behavior unchanged.
How to test
Ghost 6 /
main(this PR)Check out this branch from a fork of
TryGhost/Ghost, then runpnpm setupfrom the repository root.
Install the adapter dependencies in the sibling checkout. The Compose
overlay bind-mounts that checkout into the Ghost container, so a host-only
symlink must not be used:
cd ../ghost-ses-email-adapter npm ciFrom the Ghost repository root, start the Docker development stack with the
adapter overlay.
SES_ADAPTER_PATHis configurable and defaults to thesibling
../ghost-ses-email-adaptercheckout:SES_ADAPTER_PATH=../ghost-ses-email-adapter \ SES_FROM_EMAIL=news@example.com \ DEV_COMPOSE_FILES='-f compose.dev.ses.yaml' \ pnpm devSES_FROM_EMAILmust be a verified SES sender. SetSES_REGIONas neededand supply AWS credentials through the SDK default credential provider
chain; do not commit credentials or local configuration.
In a second terminal, verify the service is healthy rather than merely
started:
The container must remain running and pass its health check. The logs must
not contain an
Unable to find email adapter sesconfiguration error.Open
http://localhost:2368/ghost, create a test newsletter, and send a testemail. Confirm that the send is accepted by SES.
Stop the overlay, then run Ghost without
DEV_COMPOSE_FILESand repeat thenewsletter test to confirm the default Mailgun provider is still used.
Run the focused automated test:
cd ghost/core pnpm test:single test/unit/server/services/email-service/email-service-wrapper.test.jsThe test covers the default Mailgun path and configured-adapter resolution.
Ghost 5 (not covered by this PR)
This PR targets
mainand does not add Ghost 5 support.For a browser smoke test of the existing Ghost 5 interim integration, use that
repository's
ghost-5.x-email-adapter-wiring.patchagainst the exact Ghost 5runtime, install the adapter as
ses, configureadapters.email, restartGhost, and test a newsletter at
/ghost. Before doing so, run its disposablecheck:
cd ../ghost-ses-email-adapter test/integration/ghost-5.shThat check validates the interim patch and adapter wiring only; it is not
evidence that this Ghost 6/main PR applies to Ghost 5.
Checklist