Skip to content

Conversation

@troyciesco
Copy link
Contributor

@troyciesco troyciesco commented Dec 9, 2025

closes https://linear.app/ghost/issue/NY-804

  • The settings UI allows publishers to enable and configure a paid welcome email (with a flag enabled), but Ghost doesn't actually send the welcome emails for new paid members.

  • This adds the functionality to send the paid welcome emails for newly created paid members, and members upgrading from a free account to a paid plan.


Note

Implements paid welcome email sending for new or upgraded paid members, leveraging Outbox and existing automated email templates.

  • In MemberRepository.linkSubscription, when status changes to paid, enqueue MemberCreatedEvent to Outbox and dispatch processing if memberWelcomeEmailTestInbox is set, source is member, and the paid template is active
  • Determines source from request context; disallows import, admin, and api sources for sending
  • Validates paid template existence and active status before enqueuing
  • Keeps free welcome email behavior intact and ensures no free email is sent for paid signups
  • Integration tests seed paid template and verify Outbox processing no-ops when paid template is inactive/missing
  • Unit tests cover Outbox enqueue on paid transition, and negative cases (config unset, disallowed sources, inactive template)

Written by Cursor Bugbot for commit 0aae0f3. This will update automatically on new commits. Configure here.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 9, 2025

Walkthrough

MemberRepository.update now optionally enqueues a paid welcome email via the Outbox when a member’s status transitions to "paid". The change resolves request/source context, checks a paid-welcome-email config flag and a source whitelist, and—if a paid welcome template is active—creates an Outbox entry containing a MemberCreatedEvent with status 'paid' and emits StartOutboxProcessingEvent. Integration and unit tests were added to cover active/inactive/missing paid templates and disallowed sources.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding functionality to send paid member welcome emails.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The pull request description clearly explains the feature being implemented: adding paid welcome email functionality for new or upgraded paid members, with specific implementation details.
✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cmraible cmraible force-pushed the chris-ny-857-free-member-welcome-email-is-sent-to-new-paid-members branch from 5b45a25 to 67dc363 Compare December 9, 2025 23:53
Base automatically changed from chris-ny-857-free-member-welcome-email-is-sent-to-new-paid-members to main December 10, 2025 01:07
@cmraible cmraible force-pushed the NY-804_paid-member-welcome-emails branch from e930dff to 02b8ad6 Compare December 10, 2025 01:12
@troyciesco troyciesco force-pushed the NY-804_paid-member-welcome-emails branch from 02b8ad6 to 0683c5f Compare December 10, 2025 14:08
@cmraible cmraible marked this pull request as ready for review December 11, 2025 04:17
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
ghost/core/test/unit/server/services/members/members-api/repositories/MemberRepository.test.js (1)

557-557: Optional: Remove unnecessary blank line.

This blank line appears to serve no purpose in the assertion block. Consider removing it for consistency.

             assert.equal(outboxCall.event_type, 'MemberCreatedEvent');
-
             const payload = JSON.parse(outboxCall.payload);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 259fee3 and 69a0905.

📒 Files selected for processing (3)
  • ghost/core/core/server/services/members/members-api/repositories/MemberRepository.js (1 hunks)
  • ghost/core/test/integration/services/member-welcome-emails.test.js (2 hunks)
  • ghost/core/test/unit/server/services/members/members-api/repositories/MemberRepository.test.js (2 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 25288
File: ghost/core/core/server/services/member-welcome-emails/jobs/lib/process-entries.js:46-64
Timestamp: 2025-11-10T23:10:17.470Z
Learning: In ghost/core/core/server/services/member-welcome-emails/jobs/lib/process-entries.js and the outbox processing flow, entries are marked as PROCESSING before being processed. If a failure occurs after email send but before deletion, the entry remains stuck in PROCESSING state (not reprocessed). This intentional design prevents duplicate emails. Handling stuck PROCESSING entries is planned for a separate PR.
📚 Learning: 2025-11-10T23:10:17.470Z
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 25288
File: ghost/core/core/server/services/member-welcome-emails/jobs/lib/process-entries.js:46-64
Timestamp: 2025-11-10T23:10:17.470Z
Learning: In ghost/core/core/server/services/member-welcome-emails/jobs/lib/process-entries.js and the outbox processing flow, entries are marked as PROCESSING before being processed. If a failure occurs after email send but before deletion, the entry remains stuck in PROCESSING state (not reprocessed). This intentional design prevents duplicate emails. Handling stuck PROCESSING entries is planned for a separate PR.

Applied to files:

  • ghost/core/test/integration/services/member-welcome-emails.test.js
  • ghost/core/test/unit/server/services/members/members-api/repositories/MemberRepository.test.js
  • ghost/core/core/server/services/members/members-api/repositories/MemberRepository.js
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.

Applied to files:

  • ghost/core/test/integration/services/member-welcome-emails.test.js
  • ghost/core/test/unit/server/services/members/members-api/repositories/MemberRepository.test.js
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in Ghost's ActivityPub module are thoroughly tested in `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`, which covers `generatePendingActivityId`, `isPendingActivity`, and `generatePendingActivity` functions.

Applied to files:

  • ghost/core/test/integration/services/member-welcome-emails.test.js
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Use factory pattern for all test data creation instead of hard-coded data or direct database manipulation

Applied to files:

  • ghost/core/test/integration/services/member-welcome-emails.test.js
📚 Learning: 2025-08-11T19:39:00.428Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 24651
File: ghost/core/test/utils/urlUtils.js:53-57
Timestamp: 2025-08-11T19:39:00.428Z
Learning: In Ghost's test utilities, when fixing specific issues like async behavior, it's preferred to maintain existing error handling patterns (even if suboptimal) to keep PRs focused on their primary objective. Error handling improvements can be addressed in separate, dedicated PRs.

Applied to files:

  • ghost/core/core/server/services/members/members-api/repositories/MemberRepository.js
📚 Learning: 2025-11-24T11:12:15.712Z
Learnt from: sagzy
Repo: TryGhost/Ghost PR: 24673
File: ghost/i18n/lib/i18n.js:34-35
Timestamp: 2025-11-24T11:12:15.712Z
Learning: In the Ghost i18n package (ghost/i18n/lib/i18n.js), changing existing locale codes requires backwards compatibility handling for users who have already configured those locales. Such changes should be done in a separate PR with migration logic rather than included in feature PRs.

Applied to files:

  • ghost/core/core/server/services/members/members-api/repositories/MemberRepository.js
🧬 Code graph analysis (1)
ghost/core/test/integration/services/member-welcome-emails.test.js (5)
ghost/core/core/server/services/outbox/jobs/lib/process-outbox.js (1)
  • db (2-2)
ghost/core/core/server/services/members/members-api/repositories/MemberRepository.js (1)
  • ObjectId (7-7)
ghost/core/core/server/services/member-welcome-emails/constants.js (1)
  • MEMBER_WELCOME_EMAIL_SLUGS (3-6)
ghost/core/core/boot.js (1)
  • models (93-93)
ghost/core/core/server/models/outbox.js (1)
  • OUTBOX_STATUSES (3-8)
🔇 Additional comments (5)
ghost/core/core/server/services/members/members-api/repositories/MemberRepository.js (1)

1423-1447: LGTM: Paid welcome email logic properly integrated.

The implementation correctly:

  • Checks config flag and source whitelist before proceeding
  • Lazily fetches the paid welcome email only when needed
  • Validates the email is active before creating the outbox entry
  • Uses new Date() for timestamp, which is appropriate since this represents a real-time status transition (unlike the create flow where historical timestamps may be preserved)
  • Dispatches the processing event with proper transactional options
ghost/core/test/unit/server/services/members/members-api/repositories/MemberRepository.test.js (1)

717-1036: LGTM: Comprehensive test coverage for paid welcome email outbox integration.

The new test suite properly validates:

  1. ✓ Outbox entry creation when member status transitions to paid with all required payload fields
  2. ✓ Skipping outbox creation when config is not set (preventing unintended sends)
  3. ✓ Skipping for disallowed sources (import, admin, api) to respect source whitelist
  4. ✓ Skipping when paid welcome email template is inactive

The test setup with comprehensive mocks follows existing patterns and allows each test to override specific behaviors as needed. All assertions are appropriate and thorough.

ghost/core/test/integration/services/member-welcome-emails.test.js (3)

50-58: LGTM: Paid welcome email fixture properly configured.

The fixture setup correctly mirrors the free email pattern and uses the appropriate constant for the slug. The subject line clearly distinguishes this as the paid member welcome email.


65-65: LGTM: Cleanup properly maintains test isolation.

The paid email cleanup follows the existing pattern and ensures proper test isolation.


250-296: LGTM: Integration tests properly validate paid welcome email edge cases.

Both test cases correctly verify that:

  1. ✓ No email is sent when the paid template is inactive (lines 250-273)
  2. ✓ No email is sent when the paid template doesn't exist (lines 275-296)
  3. ✓ Outbox entries remain with appropriate status/messages for audit trail
  4. ✓ GhostMailer.send is never invoked in these scenarios

The tests follow the established pattern from the free email tests and align with the learned behavior where entries remain in the outbox when processing cannot complete, preventing duplicate sends.

@troyciesco troyciesco force-pushed the NY-804_paid-member-welcome-emails branch from 69a0905 to 406a8de Compare December 16, 2025 20:40
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
ghost/core/test/integration/services/member-welcome-emails.test.js (1)

275-296: Consider adding a happy path test for paid welcome emails.

While the negative test cases (inactive and missing templates) are well-covered, there's no integration test verifying that a paid welcome email is successfully sent when the template is active and a member upgrades to paid status. Consider adding a test similar to the existing ones but with status: 'paid' and an active paid template to ensure end-to-end functionality.

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 69a0905 and 406a8de.

📒 Files selected for processing (3)
  • ghost/core/core/server/services/members/members-api/repositories/MemberRepository.js (1 hunks)
  • ghost/core/test/integration/services/member-welcome-emails.test.js (2 hunks)
  • ghost/core/test/unit/server/services/members/members-api/repositories/MemberRepository.test.js (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • ghost/core/core/server/services/members/members-api/repositories/MemberRepository.js
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 25288
File: ghost/core/core/server/services/member-welcome-emails/jobs/lib/process-entries.js:46-64
Timestamp: 2025-11-10T23:10:17.470Z
Learning: In ghost/core/core/server/services/member-welcome-emails/jobs/lib/process-entries.js and the outbox processing flow, entries are marked as PROCESSING before being processed. If a failure occurs after email send but before deletion, the entry remains stuck in PROCESSING state (not reprocessed). This intentional design prevents duplicate emails. Handling stuck PROCESSING entries is planned for a separate PR.
📚 Learning: 2025-11-10T23:10:17.470Z
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 25288
File: ghost/core/core/server/services/member-welcome-emails/jobs/lib/process-entries.js:46-64
Timestamp: 2025-11-10T23:10:17.470Z
Learning: In ghost/core/core/server/services/member-welcome-emails/jobs/lib/process-entries.js and the outbox processing flow, entries are marked as PROCESSING before being processed. If a failure occurs after email send but before deletion, the entry remains stuck in PROCESSING state (not reprocessed). This intentional design prevents duplicate emails. Handling stuck PROCESSING entries is planned for a separate PR.

Applied to files:

  • ghost/core/test/integration/services/member-welcome-emails.test.js
  • ghost/core/test/unit/server/services/members/members-api/repositories/MemberRepository.test.js
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.

Applied to files:

  • ghost/core/test/integration/services/member-welcome-emails.test.js
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in Ghost's ActivityPub module are thoroughly tested in `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`, which covers `generatePendingActivityId`, `isPendingActivity`, and `generatePendingActivity` functions.

Applied to files:

  • ghost/core/test/integration/services/member-welcome-emails.test.js
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Use factory pattern for all test data creation instead of hard-coded data or direct database manipulation

Applied to files:

  • ghost/core/test/integration/services/member-welcome-emails.test.js
🧬 Code graph analysis (1)
ghost/core/test/unit/server/services/members/members-api/repositories/MemberRepository.test.js (4)
ghost/core/core/server/models/outbox.js (1)
  • Outbox (10-19)
ghost/core/core/server/services/members/members-api/repositories/MemberRepository.js (2)
  • subscriptionData (1057-1087)
  • config (11-11)
ghost/core/core/boot.js (1)
  • config (445-445)
ghost/core/core/server/services/outbox/jobs/lib/process-entries.js (1)
  • payload (51-51)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: Legacy tests (Node 22.18.0, sqlite3)
  • GitHub Check: Legacy tests (Node 22.18.0, mysql8)
  • GitHub Check: Acceptance tests (Node 22.18.0, sqlite3)
  • GitHub Check: Ghost-CLI tests
  • GitHub Check: Acceptance tests (Node 22.18.0, mysql8)
  • GitHub Check: Unit tests (Node 22.18.0)
  • GitHub Check: Lint
  • GitHub Check: Build & Push Docker Image
🔇 Additional comments (8)
ghost/core/test/integration/services/member-welcome-emails.test.js (3)

50-58: LGTM!

The paid template setup mirrors the free template pattern and includes all necessary fields.


65-65: LGTM!

Proper cleanup of the paid template ensures tests remain isolated.


250-273: LGTM!

The test correctly verifies that inactive paid templates prevent email sending, with appropriate assertions on both the mailer call count and the outbox message.

ghost/core/test/unit/server/services/members/members-api/repositories/MemberRepository.test.js (5)

717-849: LGTM!

The test suite setup is comprehensive with appropriate mocks for all dependencies. The mock structures correctly simulate Bookshelf models and Stripe objects.


855-900: LGTM!

The test correctly simulates a member upgrade scenario (free → paid) and thoroughly validates the outbox payload structure, including all required fields and timestamp.


902-940: LGTM!

The test correctly verifies that the paid welcome email feature respects the configuration flag and doesn't create outbox entries when the config is not set.


942-988: LGTM!

The test comprehensively validates source filtering, ensuring paid welcome emails are only sent for member-initiated subscriptions and not for imports, admin actions, or API calls.


990-1035: LGTM!

The test correctly verifies that inactive paid welcome email templates prevent outbox entry creation, ensuring the feature respects template activation status.

@troyciesco troyciesco force-pushed the NY-804_paid-member-welcome-emails branch from 406a8de to c7d2f49 Compare December 18, 2025 15:24
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
ghost/core/test/unit/server/services/members/members-api/repositories/MemberRepository.test.js (2)

924-969: Add event_type assertion for completeness.

The test thoroughly verifies the payload structure, but it doesn't assert the event_type field like the corresponding create test does at line 625. For consistency and completeness, consider adding this assertion.

🔎 Apply this diff to add the event_type assertion:
 sinon.assert.calledOnce(Outbox.add);
+const outboxCall = Outbox.add.firstCall.args[0];
+assert.equal(outboxCall.event_type, 'MemberCreatedEvent');
+
-const payload = JSON.parse(Outbox.add.firstCall.args[0].payload);
+const payload = JSON.parse(outboxCall.payload);
 assert.equal(payload.status, 'paid');

1059-1104: LGTM! Consider adding test for missing template.

This test correctly verifies that inactive paid welcome emails don't trigger outbox entries. The AutomatedEmail mock override is properly implemented.

While integration tests cover the case where no paid template exists (as noted in the AI summary), consider adding a unit test here where AutomatedEmail.findOne returns null for completeness and to catch issues earlier in the test pyramid.

Optional: Add test case for missing template
it('does NOT create outbox entry when paid welcome email does not exist', async function () {
    sinon.stub(config, 'get').withArgs('memberWelcomeEmailTestInbox').returns('test-inbox@example.com');

    Member.edit.resolves({
        attributes: {status: 'paid'},
        _previousAttributes: {status: 'free'},
        get: sinon.stub().callsFake((key) => {
            const data = {status: 'paid'};
            return data[key];
        })
    });

    AutomatedEmail.findOne.resolves(null);

    const repo = new MemberRepository({
        Member,
        Outbox,
        MemberPaidSubscriptionEvent,
        StripeCustomerSubscription,
        MemberProductEvent,
        MemberStatusEvent,
        stripeAPIService,
        productRepository,
        AutomatedEmail,
        OfferRedemption: mockOfferRedemption
    });

    sinon.stub(repo, 'getSubscriptionByStripeID').resolves(null);

    await repo.linkSubscription({
        id: 'member_id_123',
        subscription: subscriptionData
    }, {
        transacting: {
            executionPromise: Promise.resolve()
        },
        context: {}
    });

    sinon.assert.notCalled(Outbox.add);
});
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 406a8de and c7d2f49.

📒 Files selected for processing (3)
  • ghost/core/core/server/services/members/members-api/repositories/MemberRepository.js (1 hunks)
  • ghost/core/test/integration/services/member-welcome-emails.test.js (2 hunks)
  • ghost/core/test/unit/server/services/members/members-api/repositories/MemberRepository.test.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • ghost/core/core/server/services/members/members-api/repositories/MemberRepository.js
  • ghost/core/test/integration/services/member-welcome-emails.test.js
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 25288
File: ghost/core/core/server/services/member-welcome-emails/jobs/lib/process-entries.js:46-64
Timestamp: 2025-11-10T23:10:17.470Z
Learning: In ghost/core/core/server/services/member-welcome-emails/jobs/lib/process-entries.js and the outbox processing flow, entries are marked as PROCESSING before being processed. If a failure occurs after email send but before deletion, the entry remains stuck in PROCESSING state (not reprocessed). This intentional design prevents duplicate emails. Handling stuck PROCESSING entries is planned for a separate PR.
📚 Learning: 2025-11-10T23:10:17.470Z
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 25288
File: ghost/core/core/server/services/member-welcome-emails/jobs/lib/process-entries.js:46-64
Timestamp: 2025-11-10T23:10:17.470Z
Learning: In ghost/core/core/server/services/member-welcome-emails/jobs/lib/process-entries.js and the outbox processing flow, entries are marked as PROCESSING before being processed. If a failure occurs after email send but before deletion, the entry remains stuck in PROCESSING state (not reprocessed). This intentional design prevents duplicate emails. Handling stuck PROCESSING entries is planned for a separate PR.

Applied to files:

  • ghost/core/test/unit/server/services/members/members-api/repositories/MemberRepository.test.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: Acceptance tests (Node 22.18.0, mysql8)
  • GitHub Check: Acceptance tests (Node 22.18.0, sqlite3)
  • GitHub Check: Ghost-CLI tests
  • GitHub Check: Legacy tests (Node 22.18.0, sqlite3)
  • GitHub Check: Legacy tests (Node 22.18.0, mysql8)
  • GitHub Check: Unit tests (Node 22.18.0)
  • GitHub Check: Lint
  • GitHub Check: Build & Push Docker Image
🔇 Additional comments (4)
ghost/core/test/unit/server/services/members/members-api/repositories/MemberRepository.test.js (4)

786-797: LGTM!

The test suite setup is well-structured and declares all necessary dependencies for testing the paid welcome email outbox integration.


798-918: LGTM!

Comprehensive beforeEach setup with appropriate mocks for all dependencies. The mock structure correctly mirrors the production models and allows for flexible test scenarios.


971-1009: LGTM!

This test correctly verifies that the outbox entry is not created when the configuration flag is missing, ensuring the feature respects the configuration settings.


1011-1057: LGTM!

The test properly validates that paid welcome emails are not sent for disallowed sources (import, admin, api). The iteration pattern with history reset is the correct approach for testing multiple scenarios.

@github-actions
Copy link
Contributor

React E2E Tests Failed

To view the Playwright test report locally, run:

REPORT_DIR=$(mktemp -d) && gh run download 20341960495 -n playwright-report-react -D "$REPORT_DIR" && npx playwright show-report "$REPORT_DIR"

@troyciesco troyciesco force-pushed the NY-804_paid-member-welcome-emails branch from c7d2f49 to 0a24207 Compare January 5, 2026 14:37
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Fix all issues with AI Agents 🤖
In
@ghost/core/core/server/services/members/members-api/repositories/MemberRepository.js:
- Line 1441: The timestamp is set to new Date() which can mismatch the actual
status change time; update the code that builds the event/audit record (the
object with the timestamp property in MemberRepository.js) to prefer a
status-change timestamp: use eventData.subscription?.start_date when present,
else use member.updated_at (or member.updated_at date object), and only fall
back to new Date() if neither exists so the audit timestamp aligns with
subscription start or member update times.
🧹 Nitpick comments (2)
ghost/core/core/server/services/members/members-api/repositories/MemberRepository.js (2)

1423-1446: Consider extracting duplicate welcome email logic.

The welcome email outbox creation logic is duplicated between the create method (lines 346-389) and here. Extracting this to a helper method would improve maintainability and reduce duplication.

Example refactoring approach

Consider extracting a helper method like:

async _maybeEnqueueWelcomeEmail({member, source, timestamp, status, transacting}) {
    const shouldSend = config.get('memberWelcomeEmailTestInbox') && 
                       WELCOME_EMAIL_SOURCES.includes(source);
    
    if (!shouldSend || !this._AutomatedEmail) {
        return false;
    }

    const slug = status === 'paid' ? 
        MEMBER_WELCOME_EMAIL_SLUGS.paid : 
        MEMBER_WELCOME_EMAIL_SLUGS.free;
    
    const welcomeEmail = await this._AutomatedEmail.findOne({slug}, {transacting});
    const isActive = welcomeEmail && 
                     welcomeEmail.get('lexical') && 
                     welcomeEmail.get('status') === 'active';
    
    if (!isActive) {
        return false;
    }

    await this._Outbox.add({
        id: ObjectId().toHexString(),
        event_type: MemberCreatedEvent.name,
        payload: JSON.stringify({
            memberId: member.id,
            email: member.get('email'),
            name: member.get('name'),
            source,
            timestamp,
            status
        })
    }, {transacting});
    
    return true;
}

This would simplify both call sites and centralize the welcome email logic.


1424-1425: Reuse context source resolution from line 699.

The context source is already resolved at line 699 for newsletter events. Consider storing it in a variable and reusing it here to avoid redundant resolution.

Suggested optimization

At line 699:

-        const context = options && options.context || {};
-        const source = this._resolveContextSource(context);
+        const context = options && options.context || {};
+        const source = this._resolveContextSource(context);

Then at line 1424:

-            const context = options?.context || {};
-            const source = this._resolveContextSource(context);
+            // Reuse source from line 699

Since both resolutions happen in the same method execution, the source value will be identical.

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c7d2f49 and 0a24207.

📒 Files selected for processing (3)
  • ghost/core/core/server/services/members/members-api/repositories/MemberRepository.js
  • ghost/core/test/integration/services/member-welcome-emails.test.js
  • ghost/core/test/unit/server/services/members/members-api/repositories/MemberRepository.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • ghost/core/test/unit/server/services/members/members-api/repositories/MemberRepository.test.js
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 25288
File: ghost/core/core/server/services/member-welcome-emails/jobs/lib/process-entries.js:46-64
Timestamp: 2025-11-10T23:10:17.470Z
Learning: In ghost/core/core/server/services/member-welcome-emails/jobs/lib/process-entries.js and the outbox processing flow, entries are marked as PROCESSING before being processed. If a failure occurs after email send but before deletion, the entry remains stuck in PROCESSING state (not reprocessed). This intentional design prevents duplicate emails. Handling stuck PROCESSING entries is planned for a separate PR.
📚 Learning: 2025-11-10T23:10:17.470Z
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 25288
File: ghost/core/core/server/services/member-welcome-emails/jobs/lib/process-entries.js:46-64
Timestamp: 2025-11-10T23:10:17.470Z
Learning: In ghost/core/core/server/services/member-welcome-emails/jobs/lib/process-entries.js and the outbox processing flow, entries are marked as PROCESSING before being processed. If a failure occurs after email send but before deletion, the entry remains stuck in PROCESSING state (not reprocessed). This intentional design prevents duplicate emails. Handling stuck PROCESSING entries is planned for a separate PR.

Applied to files:

  • ghost/core/core/server/services/members/members-api/repositories/MemberRepository.js
  • ghost/core/test/integration/services/member-welcome-emails.test.js
📚 Learning: 2025-08-11T19:39:00.428Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 24651
File: ghost/core/test/utils/urlUtils.js:53-57
Timestamp: 2025-08-11T19:39:00.428Z
Learning: In Ghost's test utilities, when fixing specific issues like async behavior, it's preferred to maintain existing error handling patterns (even if suboptimal) to keep PRs focused on their primary objective. Error handling improvements can be addressed in separate, dedicated PRs.

Applied to files:

  • ghost/core/core/server/services/members/members-api/repositories/MemberRepository.js
📚 Learning: 2025-11-24T11:12:15.712Z
Learnt from: sagzy
Repo: TryGhost/Ghost PR: 24673
File: ghost/i18n/lib/i18n.js:34-35
Timestamp: 2025-11-24T11:12:15.712Z
Learning: In the Ghost i18n package (ghost/i18n/lib/i18n.js), changing existing locale codes requires backwards compatibility handling for users who have already configured those locales. Such changes should be done in a separate PR with migration logic rather than included in feature PRs.

Applied to files:

  • ghost/core/core/server/services/members/members-api/repositories/MemberRepository.js
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.

Applied to files:

  • ghost/core/test/integration/services/member-welcome-emails.test.js
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in Ghost's ActivityPub module are thoroughly tested in `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`, which covers `generatePendingActivityId`, `isPendingActivity`, and `generatePendingActivity` functions.

Applied to files:

  • ghost/core/test/integration/services/member-welcome-emails.test.js
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Use factory pattern for all test data creation instead of hard-coded data or direct database manipulation

Applied to files:

  • ghost/core/test/integration/services/member-welcome-emails.test.js
🧬 Code graph analysis (2)
ghost/core/core/server/services/members/members-api/repositories/MemberRepository.js (3)
ghost/core/core/server/services/outbox/jobs/lib/process-entries.js (1)
  • MemberCreatedEvent (4-4)
ghost/core/core/server/services/outbox/jobs/lib/process-outbox.js (1)
  • MemberCreatedEvent (3-3)
ghost/core/core/server/services/outbox/index.js (1)
  • StartOutboxProcessingEvent (3-3)
ghost/core/test/integration/services/member-welcome-emails.test.js (2)
ghost/core/core/server/services/member-welcome-emails/constants.js (1)
  • MEMBER_WELCOME_EMAIL_SLUGS (3-6)
ghost/core/core/server/models/outbox.js (1)
  • OUTBOX_STATUSES (3-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: Unit tests (Node 22.18.0)
  • GitHub Check: Ghost-CLI tests
  • GitHub Check: Legacy tests (Node 22.18.0, sqlite3)
  • GitHub Check: Lint
  • GitHub Check: Acceptance tests (Node 22.18.0, sqlite3)
  • GitHub Check: Acceptance tests (Node 22.18.0, mysql8)
  • GitHub Check: Legacy tests (Node 22.18.0, mysql8)
  • GitHub Check: Cursor Bugbot
  • GitHub Check: Build & Push Docker Image
🔇 Additional comments (4)
ghost/core/test/integration/services/member-welcome-emails.test.js (4)

50-58: LGTM! Paid welcome email test setup follows the established pattern.

The paid member welcome email entry is properly configured with active status, correct slug, and lexical content, mirroring the free welcome email setup above.


65-65: LGTM! Cleanup properly removes the paid welcome email entry.


250-273: LGTM! Test properly verifies no email is sent when paid template is inactive.

The test correctly validates that:

  • GhostMailer.send is not called
  • Outbox entry message includes 'inactive'

275-296: LGTM! Test properly verifies no email is sent when paid template doesn't exist.

The test correctly validates that:

  • GhostMailer.send is not called
  • Outbox entry has a message indicating the issue

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is being reviewed by Cursor Bugbot

Details

Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

@troyciesco troyciesco merged commit 8f25d2f into main Jan 6, 2026
38 checks passed
@troyciesco troyciesco deleted the NY-804_paid-member-welcome-emails branch January 6, 2026 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants