🐛 Fixed non-deterministic member attribution resolution

[9larsons]

Summary

Makes member- and subscription-created attribution lookups deterministic.

getMemberCreatedAttribution and getSubscriptionCreatedAttribution in member-attribution-service.js looked up the created event with findOne({member_id}) / findOne({subscription_id}). Those columns are not unique and the queries had no ORDER BY, so when more than one created-event row existed for a member or subscription the database could return an arbitrary row — making attribution resolution non-deterministic (and a source of flakiness).

The lookups now order by created_at, id (descending) and fetch the most recent row, matching the existing deterministic pattern already used in member-created-event.js.

Changes

• member-attribution-service.js: findOne(...) → .where(col, id).query(qb => qb.orderBy('created_at','desc').orderBy('id','desc')).fetch(...) in both created-attribution lookups
• service.test.js: updated the model mocks to the new .where().query().fetch() chain

Testing

• pnpm --dir ghost/core test:single test/unit/server/services/member-attribution/service.test.js — 20 passing
• pnpm --dir ghost/core test:single test/e2e-server/services/member-attribution.test.js — 16 passing
• lint clean

Context

Found while reviewing #23146 — this core attribution fix was bundled with an unrelated serializer field addition, so it's been split out into its own PR.

[coderabbitai]

Walkthrough

This PR refactors member and subscription attribution event lookups to use deterministic ordered queries instead of implicit single-record lookup. The service methods getMemberCreatedAttribution and getSubscriptionCreatedAttribution now query their respective event models with explicit ordering by created_at (descending) and id (descending) using .where(...).query(...), replacing .findOne(). The return behavior and null-handling remain unchanged. Tests are updated with a new createEventModelMock helper to support the chainable mock pattern, and all test scenarios are adjusted accordingly.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: fixing non-deterministic member attribution resolution by making lookups deterministic with proper ordering.
Description check	✅ Passed	The description is directly related to the changeset, providing clear context about the non-deterministic lookup issue, the specific solution implemented, and testing validation.
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.

_{✏️ 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 member-attribution-deterministic-ordering

---

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/member-attribution/service.test.js (1)

5-14: ⚡ Quick win

Assert ordering in the chainable mock to protect the determinism contract.

Current tests validate output shape but don’t verify that the query callback applies orderBy('created_at','desc') then orderBy('id','desc'). A small enhancement to the helper would prevent silent regressions to non-deterministic lookup behavior.

Proposed test-helper hardening

 function createEventModelMock(event) {
+    const orderByCalls = [];
     const chainable = {
         where: () => chainable,
-        query: () => chainable,
+        query: (cb) => {
+            cb({
+                orderBy: (col, dir) => {
+                    orderByCalls.push([col, dir]);
+                    return this;
+                }
+            });
+            return chainable;
+        },
         fetch: async () => event
     };
+    chainable._getOrderByCalls = () => orderByCalls;
     return chainable;
 }

Then assert in each deterministic lookup test that:

• ['created_at', 'desc']
• ['id', 'desc']

were both applied in order.

Also applies to: 282-283, 293-305, 336-337, 347-359

🤖 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/member-attribution/service.test.js`
around lines 5 - 14, The chainable test helper createEventModelMock currently
ignores the query callback so tests don't validate ordering; update
createEventModelMock so its query(fn) invokes the provided fn with a mock
builder that records calls to orderBy (pushing each [column, direction] into an
array) and then returns chainable, and expose that recorded array (e.g., as
chainable._appliedOrders) so tests can assert the sequence equals
[['created_at','desc'], ['id','desc']] in the deterministic lookup tests; ensure
where and fetch behavior is unchanged (where returns chainable, fetch resolves
to event).

🤖 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/member-attribution/service.test.js`:
- Around line 5-14: The chainable test helper createEventModelMock currently
ignores the query callback so tests don't validate ordering; update
createEventModelMock so its query(fn) invokes the provided fn with a mock
builder that records calls to orderBy (pushing each [column, direction] into an
array) and then returns chainable, and expose that recorded array (e.g., as
chainable._appliedOrders) so tests can assert the sequence equals
[['created_at','desc'], ['id','desc']] in the deterministic lookup tests; ensure
where and fetch behavior is unchanged (where returns chainable, fetch resolves
to event).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: dbbd84e4-e4e9-4dd7-8a7f-bcf7fca98b12

📥 Commits

Reviewing files that changed from the base of the PR and between 3596b42 and 819117e.

📒 Files selected for processing (2)

• ghost/core/core/server/services/member-attribution/member-attribution-service.js
• ghost/core/test/unit/server/services/member-attribution/service.test.js