Ghost icon indicating copy to clipboard operation
Ghost copied to clipboard

šŸ“® Postmark integration #19398

Open andreascreten opened this issue 1 year ago • 3 comments

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.

andreascreten avatar Jul 03 '24 15:07 andreascreten

It looks like this PR contains a migration šŸ‘€ Here's the checklist for reviewing migrations:

General requirements

  • [ ] Satisfies idempotency requirement (both up() and down())
  • [ ] 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

github-actions[bot] avatar Jul 03 '24 15:07 github-actions[bot]

Could you comment on the feedback I left in the last PR?

https://github.com/TryGhost/Ghost/pull/19398#issuecomment-2132520491

markstos avatar Jul 15 '24 14:07 markstos

@andreascreten you requested feedback and I left some here: https://github.com/TryGhost/Ghost/pull/19398#issuecomment-2132520491

markstos avatar Aug 21 '24 15:08 markstos

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.

AboMoutaz avatar Nov 05 '24 08:11 AboMoutaz

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?

markstos avatar Nov 06 '24 14:11 markstos

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: false in the knowledge_base settings.
  • 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?

ā¤ļø Share
🪧 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 @coderabbitai in 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 @coderabbitai in 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 pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file 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.

coderabbitai[bot] avatar Feb 04 '25 09:02 coderabbitai[bot]

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.

markstos avatar Apr 02 '25 14:04 markstos

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

  1. reset the branch on our fork to an earlier commit
  2. synced the fork using the github button
  3. 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?

peterpacket avatar Apr 02 '25 15:04 peterpacket