Ghost
Ghost copied to clipboard
š® Postmark integration #19398
I have been working on an integration of Postmark with Ghost. Please provide your feedback before I continue.
To-Do
- [x] Implement analytics
- [ ] Handle Postmark errors
- [ ] Add tests
This a copy of a previous PR, submitting it again because our main branch got corrupted due to a merge conflict.
It looks like this PR contains a migration š Here's the checklist for reviewing migrations:
General requirements
- [ ] Satisfies idempotency requirement (both
up()anddown()) - [ ] Does not reference models
- [ ] Filename is in the correct format (and correctly ordered)
- [ ] Targets the next minor version
- [ ] All code paths have appropriate log messages
- [ ] Uses the correct utils
- [ ] Contains a minimal changeset
- [ ] Does not mix DDL/DML operations
Schema changes
- [ ] Both schema change and related migration have been implemented
- [ ] For index changes: has been performance tested for large tables
- [ ] For new tables/columns: fields use the appropriate predefined field lengths
- [ ] For new tables/columns: field names follow the appropriate conventions
- [ ] Does not drop a non-alpha table outside of a major version
Data changes
- [ ] Mass updates/inserts are batched appropriately
- [ ] Does not loop over large tables/datasets
- [ ] Defends against missing or invalid data
- [ ] For settings updates: follows the appropriate guidelines
Could you comment on the feedback I left in the last PR?
https://github.com/TryGhost/Ghost/pull/19398#issuecomment-2132520491
@andreascreten you requested feedback and I left some here: https://github.com/TryGhost/Ghost/pull/19398#issuecomment-2132520491
I've updated this PR to go in the direction that @markstos has suggested.
As far as I noticed, controlling the used adapter is only done using the config file, so I still don't have a solid idea of controlling the provider using the settings UI.
I'd love to get some directions for my next step.
Now that this has an updated to use the adapter pattern, the next step is for a Ghost core team member to take a look and discuss what further refinements might be required to get it merged.
One thing I see is that this solution is involves some mentions of Postmark in the React code, like here:
https://github.com/TryGhost/Ghost/pull/20530/files#diff-b74336cc721823b891ffb6e8f7b9d367c7dea2e7d7bdfb6b80db78972d7763f7R7
This means that once merged, it will be the responsibility of the Ghost team to maintain it. Is this approach acceptable to the team? Or can the web admin interface also use the config system to look-up an alternate admin UI to load instead of the Mailgun UI elements?
Walkthrough
This pull request transitions the email configuration from a Mailgunāonly approach to a more flexible bulk email provider integration. New settings and schema entries for ābulk_email_providerā and āpostmark_api_tokenā are added, and UI components are updated to replace Mailgunāspecific wording with a generic āEmail provider.ā The backend services and utility methods have been refactored to use new getters and methods that check the bulk email setup, including the introduction of a Postmark provider option. In addition, new packages and tests have been introduced to support Postmark analytics and client functionality.
Changes
| File(s) | Summary |
|---|---|
apps/admin-x-framework/src/test/responses/settings.json, ghost/admin/mirage/fixtures/settings.js, ghost/core/core/server/data/schema/default-settings/default-settings.json, ghost/admin/app/models/setting.js |
Added new email settings and schema properties: ābulk_email_providerā, āpostmark_api_tokenā (and model attribute postmarkApiToken) |
apps/admin-x-settings/src/components/Sidebar.tsx, apps/admin-x-settings/src/components/settings/email/BulkEmail.tsx, apps/admin-x-settings/src/components/settings/email/EmailSettings.tsx, apps/admin-x-settings/src/components/settings/email/Mailgun.tsx |
Updated UI components: renamed āMailgun settingsā to āEmail providerā, introduced new BulkEmail component, removed the Mailgun component, and updated search keywords |
apps/admin-x-settings/test/acceptance/email/mailgun.test.ts, ghost/admin/tests/acceptance/editor/publish-flow-test.js |
Revised test identifiers and error messages to reflect bulk email terminology |
ghost/admin/app/components/editor/publish-options/publish-type.hbs, ghost/admin/app/services/settings.js, ghost/admin/app/utils/publish-options.js |
Modified conditional checks and getters: replaced mailgunIsConfigured with bulkEmailIsConfigured |
ghost/core/core/server/services/email-analytics/EmailAnalyticsServiceWrapper.js, ghost/core/core/server/services/email-service/EmailServiceWrapper.js, files in ghost/core/core/server/services/email-suppression-list/, ghost/core/core/server/services/public-config/config.js |
Refactored backend services: introduced a PostmarkProvider, new getMailClient method, BulkEmailProvider, and renamed services and suppression list classes |
ghost/core/package.json |
Added dependencies: @tryghost/email-analytics-provider-postmark and @tryghost/postmark-client |
ghost/email-analytics-provider-postmark/**/*, ghost/email-service/index.js, ghost/email-service/lib/BulkEmailProvider.js, ghost/email-service/test/bulk-email-provider.test.js |
New/updated packages for email analytics and bulk email: added Postmark analytics provider package, refactored BulkEmailProvider and updated exports/tests |
ghost/postmark-client/**/* |
Introduced a new Postmark client package: includes ESLint config, README, implementation of PostmarkClient, package metadata, and corresponding tests |
ghost/core/content/themes/* |
Updated subproject commit references for Casper and Source themes |
| Other files (e.g. ESLint configs, minor test files) | Added new configurations and test suites for Postmark provider and client |
Sequence Diagram(s)
sequenceDiagram
participant UI as Admin UI (BulkEmail)
participant ES as EmailServiceWrapper
participant BP as BulkEmailProvider
participant MC as Mail Client (Mailgun/Postmark)
UI->>ES: Request mail client instance via getMailClient(settingsCache)
ES->>ES: Evaluate settings.bulk_email_provider
alt Provider is "postmark"
ES->>MC: Instantiate PostmarkClient
else
ES->>MC: Instantiate MailgunClient (default)
end
ES->>BP: Return mail client instance to BulkEmailProvider
BP->>MC: Proceed with sending email
sequenceDiagram
participant EA as EmailAnalyticsServiceWrapper
participant MP as MailgunProvider
participant PP as PostmarkProvider
EA->>EA: Initialize analytics providers
EA->>MP: Instantiate MailgunProvider
EA->>PP: Instantiate PostmarkProvider
EA-->>EA: Consolidate and process analytics data from both providers
[!TIP]
š Web search-backed reviews and chat
- We have enabled web search-based reviews and chat for all users. This feature allows CodeRabbit to access the latest documentation and information on the web.
- You can disable this feature by setting
web_search: falsein theknowledge_basesettings.- Please share any feedback in the Discord discussion.
⨠Finishing Touches
- [ ] š Generate Docstrings (Beta)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
šŖ§ Tips
Chat
There are 3 ways to chat with CodeRabbit:
- Review comments: Directly reply to a review comment made by CodeRabbit. Example:
I pushed a fix in commit <commit_id>, please review it.Generate unit testing code for this file.Open a follow-up GitHub issue for this discussion.
- Files and specific lines of code (under the "Files changed" tab): Tag
@coderabbitaiin a new review comment at the desired location with your query. Examples:@coderabbitai generate unit testing code for this file.@coderabbitai modularize this function.
- PR comments: Tag
@coderabbitaiin a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.@coderabbitai read src/utils.ts and generate unit testing code.@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.@coderabbitai help me debug CodeRabbit configuration file.
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.
CodeRabbit Commands (Invoked using PR comments)
@coderabbitai pauseto pause the reviews on a PR.@coderabbitai resumeto resume the paused reviews.@coderabbitai reviewto trigger an incremental review. This is useful when automatic reviews are disabled for the repository.@coderabbitai full reviewto do a full review from scratch and review all the files again.@coderabbitai summaryto regenerate the summary of the PR.@coderabbitai generate docstringsto generate docstrings for this PR. (Beta)@coderabbitai resolveresolve all the CodeRabbit review comments.@coderabbitai configurationto show the current CodeRabbit configuration for the repository.@coderabbitai helpto get help.
Other keywords and placeholders
- Add
@coderabbitai ignoreanywhere in the PR description to prevent this PR from being reviewed. - Add
@coderabbitai summaryto generate the high-level summary at a specific location in the PR description. - Add
@coderabbitaianywhere in the PR title to generate the title automatically.
CodeRabbit Configuration File (.coderabbit.yaml)
- You can programmatically configure CodeRabbit by adding a
.coderabbit.yamlfile to the root of your repository. - Please see the configuration documentation for more information.
- If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation:
# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
Documentation and Community
- Visit our Documentation for detailed information on how to use CodeRabbit.
- Join our Discord Community to get help, request features, and share feedback.
- Follow us on X/Twitter for updates and announcements.
Maybe there was a Github glitch, but the referenced merged commit that closed this PR ( https://github.com/TryGhost/Ghost/commit/5a33b3b3c8a7cbdf21439aeb8964e9061e1c53fc ) does not appear related to Postmark.
I'm not sure what happened here, but it's probably related to what I did on our fork.
I wanted to continue on this PR and the first step was to get the fork in sync. In an attempt to ease the process I
- reset the branch on our fork to an earlier commit
- synced the fork using the github button
- applied back the commits that were originally in the branch.
I guess that at some point our fork and the main branch were the same, which may have automatically merged this PR?