Fixed gift-to-paid member activity entry reverting after first payment

[sagzy]

closes https://linear.app/ghost/issue/BER-3542

Fixes the member activity feed entry for gift members who continue into a paid subscription.

When a gift member upgraded to a paid subscription, the subscription event initially had enough context to show continued paid subscription after gift. However, getSubscriptionEvents() deleted the eager-loaded paidStatusEvent relation from the shared SubscriptionCreatedEvent Bookshelf model while serializing the first event row.

Because multiple MemberPaidSubscriptionEvent rows can share the same SubscriptionCreatedEvent instance, later rows could lose access to paidStatusEvent. That caused previous_status to become null, and the activity feed could revert to the generic started paid subscription wording after the next payment/update event.

This change:

• keeps paidStatusEvent available on the shared relation while deriving previous_status
• removes subscriptionCreatedEvent.paidStatusEvent only from the serialized event payload
• adds unit coverage for shared subscription-created-event relations so the regression is caught

[coderabbitai]

Walkthrough

Reworked getSubscriptionEvents to stop removing the helper relation from the in-memory subscriptionCreatedEvent prior to building the serialized payload. The code now deletes model.relations.stripeSubscription before serialization and then removes subscriptionCreatedEvent.paidStatusEvent from the final data object (delete d.subscriptionCreatedEvent?.paidStatusEvent). Added unit tests that mock Bookshelf-like models where multiple paid-subscription events share one SubscriptionCreatedEvent (and a shared paidStatusEvent) to assert previous_status is preserved and the internal paidStatusEvent is not leaked in the serialized output.

Possibly related PRs

• TryGhost/Ghost#27582: Modifies the same getSubscriptionEvents handling of SubscriptionCreatedEvent.paidStatusEvent, ensuring it’s used for deriving previous_status but not leaked in the serialized subscription_event payload.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main bug fix: preventing a member activity entry from reverting after the first payment for gift-to-paid conversions.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The pull request description clearly explains the bug being fixed, root cause analysis, the solution approach, and testing coverage.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch gift-subscriptions/member-activity-on-gift-to-paid

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

ghost/core/test/unit/server/services/members/members-api/repositories/event-repository.test.js (1)

760-871: 💤 Low value

Solid regression coverage; consider also asserting the updated row.

The mock faithfully reproduces Bookshelf's shared-instance behaviour for subscriptionCreatedEvent (same reference returned to both rows), and the final assertion on Line 870 nicely guarantees the shared origin is left unmutated — exactly the invariant the production fix establishes. Ordering the rows as updated then created correctly mirrors the buggy iteration order.

A small enhancement to consider: the test only asserts previous_status for the created row. Adding assert.equal(updated.data.previous_status, 'gift'); would tighten the regression so a future "skip previous_status for non-created rows" change couldn't accidentally bypass the check. Optional.

One minor cleanup: makeRelated/sharedRelated (Lines 766–773) are unreachable in this test because relations (Lines 776–779) already contains both subscriptionCreatedEvent and stripeSubscription, so the ?? sharedRelated(name) fallback never fires. You can drop makeRelated/sharedRelated without changing behaviour.

🤖 Prompt for 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.

In
`@ghost/core/test/unit/server/services/members/members-api/repositories/event-repository.test.js`
around lines 760 - 871, The test getSubscriptionEvents's regression should both
assert the updated row's previous_status and remove dead helper code: add an
assertion similar to assert.equal(updated.data.previous_status, 'gift') in the
"preserves previous_status..." it block so the updated row is explicitly
checked, and delete the unused helper functions makeRelated and sharedRelated
(and their references) from buildModels since relations already contains
subscriptionCreatedEvent and stripeSubscription and the fallback never runs;
update buildModels to rely only on the relations object and toJSON as it
currently does.

🤖 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.

Nitpick comments:
In
`@ghost/core/test/unit/server/services/members/members-api/repositories/event-repository.test.js`:
- Around line 760-871: The test getSubscriptionEvents's regression should both
assert the updated row's previous_status and remove dead helper code: add an
assertion similar to assert.equal(updated.data.previous_status, 'gift') in the
"preserves previous_status..." it block so the updated row is explicitly
checked, and delete the unused helper functions makeRelated and sharedRelated
(and their references) from buildModels since relations already contains
subscriptionCreatedEvent and stripeSubscription and the fallback never runs;
update buildModels to rely only on the relations object and toJSON as it
currently does.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 267f7008-f99e-4150-8da4-4f1904321b47

📥 Commits

Reviewing files that changed from the base of the PR and between e7f993c and 9b6482d.

📒 Files selected for processing (2)

• ghost/core/core/server/services/members/members-api/repositories/event-repository.js
• ghost/core/test/unit/server/services/members/members-api/repositories/event-repository.test.js