Ghost
Ghost copied to clipboard
core: refactor bulk email service to allow non-mailgun senders.
Use Case
Ghost documentation says "More bulk mailers besides Mailgun are planned". This PR helps along that goal with some foundational work.
Also, I signed up with Mailgun today, and there is no indication there is a free tier. The lowest publish tier is the $35/month "Foundation" plan. I contact support today to clarify.
Design Goal
This refactor is designed so that the Ghost does not need to support an alternate bulk email sender in the core. Alternate providers could be implemented as unsupported, third-party NPM modules.
Retrieving analytics back from the bulk email provider could be handled in the same way.
Implementation
- Removes references to mailgun in bulk-email-processor.js
- Defines a bulk emailer module in configuration, defaulting to Mailgun
- Loads the bulk mailer module based on the config
Now a new bulk email service can be defined by:
- Creating a bulk email module that exports
BATCH_SIZE,getInstanceandsendcompatible with Ghost'smailgun.js. - Upload that module to NPM.
- Reference that module in config at
buklEmail:module
The alternate module will likely require its own API key, which can also be defined in the config system or setting system.
Codecov Report
Merging #14984 (16bfb67) into main (630149e) will increase coverage by
0.00%. The diff coverage is55.55%.
@@ Coverage Diff @@
## main #14984 +/- ##
=======================================
Coverage 61.53% 61.53%
=======================================
Files 573 573
Lines 46430 46431 +1
Branches 4209 4209
=======================================
+ Hits 28571 28572 +1
Misses 17813 17813
Partials 46 46
| Impacted Files | Coverage Δ | |
|---|---|---|
| core/server/services/bulk-email/mailgun.js | 19.51% <0.00%> (-0.16%) |
:arrow_down: |
| ...server/services/bulk-email/bulk-email-processor.js | 25.57% <62.50%> (+0.38%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 630149e...16bfb67. Read the comment docs.
After I created this PR, I discovered that Ghost uses a formal "adapter" pattern in other places for extensibility on the backend. I presume the preference would be to use the same pattern to support alternatives to the Mailgun backend. For now I've decided to use Mailgun and not pursue this, but here are some references if someone else is interested to explore this more:
adapter-manager
Here's the adapter-manager project:
https://github.com/TryGhost/Utils/blob/f2b428a139e26d8f1a0947cbc3b6f24d1834c2ea/packages/adapter-manager/README.md
registering adapters
Here you can see where adapters get registered:
https://github.com/TryGhost/Ghost/blob/main/core/server/services/adapter-manager/index.js
What appears to be happening is that a path to where a custom solution is stored gets declared under the "adapters" section in the config file. If this is not present, the internal implementation in used.
My patch here provided the same kind of logic in spirit, but this since this formal pattern is in use in other places, it seems best to use it for consistency with supporting alternative mailing backends as well.
Hey @markstos, apologies for taking a while to get back to you on this PR - I've been slowly, slowly trying to catch up with all the issues and PRs across our repos after getting quite behind!
As you correctly identified, we have an adapter pattern that we would use to abstract out the bulk mail provider.
I think this is quite a desirable move for a lot of self-hosters and I'd be happy to support you or anyone else that would like to pursue laying some foundation pieces for abstracting out bulk mail and/or email analytics to work with other providers.
For now, I'll close this PR - but again I'm happy to discuss it further or offer support to make progress with this.
Hey! @ErisDS would it be possible to identify what would be an acceptable end goal here? Would it include an additional adapter alongside the abstraction? Any chance there is a shortlist of desired adapters perhaps ?
Also is there any comments over the stuff that was proposed in this PR? I would love to pick it up, but want to understand a bit on more as to what was missing from the proposed changes.
@jugglingjsons I like to think I had the right idea in this PR, but didn't formally follow the established adapter pattern, I doubt they want to support any other mail providers internally, so it seems fair if a new PR just implements the adapter pattern, creating a place to plug external adapters into without actually shipping other adapters. People could publish other implementations to NPM on Github. I ended up using Mailgun, so I'm not pursuing this myself.