Added a member account module for the members API projection - #30436
Draft
rob-ghost wants to merge 9 commits into
Draft
Added a member account module for the members API projection#30436rob-ghost wants to merge 9 commits into
rob-ghost wants to merge 9 commits into
Conversation
ref https://linear.app/ghost/issue/BER-3864/differentiate-public-and-private-member-projections Ghost assembles one member payload and several surfaces narrow it independently on the way out, each with its own allowlist and no shared definition. Two of those surfaces are fed by a single read: the members API response Portal consumes, and the @member data a theme renders. That read is about to move behind a member-account module with a recipe of its own, so what each surface currently receives needs to be written down first, while it is still demonstrably unchanged. Anything that differs afterwards is then a decision someone made rather than drift nobody noticed. The members API snapshots cover a free, a paid and a comped member, because those three assemble differently: a comped member's subscription is synthesized from their products rather than read from Stripe, and only a paid member exercises the offer and next-payment steps. Whole bodies are snapshotted rather than selected keys, since a key appearing or vanishing is the failure worth catching and asserting named keys would miss it. The theme's shape is pinned by having the test theme render the keys of @member rather than their values, which keeps the assertion about the contract instead of the fixture data. The comped member is built in the test: the comped@test.com fixture is status paid despite its name, so it never reaches the branch it appears to cover.
ref https://linear.app/ghost/issue/BER-3864/differentiate-public-and-private-member-projections Reading a member means running a sequence of steps over the loaded model, because most of what a member payload carries is not a relation that can be eager-loaded: Stripe subscriptions, tiers, offers and their redemptions, gifts, attributions and the email suppression list each need their own fetch or their own derivation. The BREAD service already had those steps as separate methods, so the sequence was legible, but every one of them read its collaborator off this and could therefore only ever run as part of the one recipe that service holds. There is more than one audience for the same steps. Staff reading a member in Admin want all of them; a member reading their own record wants fewer, and Ghost runs that second read on every page view of a themed site. The withCustomFields flag is the shape that need has taken so far: a hole punched through a shared recipe for a caller that wanted less of it. A step that takes its collaborator as an argument can be composed into either recipe instead, which is what lets a second one exist at all. Nothing about the sequence changes here. The BREAD methods stay as they were and delegate, so the two ordering constraints that matter still hold and its own tests still describe it. Browse keeps asking the suppression list once for the whole page rather than adopting the per-member step, since those are genuinely different questions.
ref https://linear.app/ghost/issue/BER-3864/differentiate-public-and-private-member-projections A member has two views: the one Ghost serves them about themselves, and the one it serves staff about them. Only the second was assembled deliberately. One payload was built for both, and each surface narrowed it on the way out through a pick of its own, so what a member receives was the residue of whichever list ran last rather than anything stated. The lists had already drifted apart with nothing to compare them against. This gives the first of those views a module. It owns its own read, its own projection and its own writable fields, and it says in one place what a member may see and set. The projection is unchanged, byte for byte, and the tests taken beforehand are what say so; only the code behind it moves. The read it runs is narrower than the staff one, which is the point of separating them. Ghost identifies a signed-in reader through this path on every page view of a themed site, so a relation loaded here is loaded on all of them: it stops loading labels and stops asking where the member came from, neither of which any member-facing surface reads. Where each subscription came from is still asked, because a theme can reach it. The withCustomFields flag goes with them, since it existed only to let one caller opt out of part of a read it shared with another audience, and that caller now has a read of its own. Three other surfaces narrow a member for their own audiences and keep doing so where they are: the theme data, which is a versioned part of Ghost's theme API, the newsletter endpoints, which authenticate by uuid rather than by session and withhold billing data on purpose, and the comments author shape. They are recorded in projections.ts so the four can be read side by side, which is how the one genuine disagreement between them became visible: a comment carries its author's internal id, and a member's own response does not.
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run ghost:test:ci:e2e |
✅ Succeeded | 4m 26s | View ↗ |
nx run ghost:test:legacy |
✅ Succeeded | 3m 19s | View ↗ |
nx run ghost:test:e2e |
✅ Succeeded | 2m 48s | View ↗ |
nx run ghost-monorepo:lint:boundaries |
✅ Succeeded | 27s | View ↗ |
nx run-many -t test:unit -p ghost |
✅ Succeeded | 26s | View ↗ |
nx run-many -t lint -p ghost,ghost-monorepo |
✅ Succeeded | 21s | View ↗ |
nx run-many --target=build --projects=tag:publi... |
✅ Succeeded | 2s | View ↗ |
nx run @tryghost/admin:build |
✅ Succeeded | 5s | View ↗ |
nx run @tryghost/e2e:test:fixtures |
✅ Succeeded | <1s | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-09-01 20:31:58 UTC
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #30436 +/- ##
===========================================
- Coverage 76.17% 67.44% -8.73%
===========================================
Files 1679 1663 -16
Lines 160375 60086 -100289
Branches 19709 10381 -9328
===========================================
- Hits 122165 40526 -81639
+ Misses 37219 17268 -19951
- Partials 991 2292 +1301
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueComment |
ref https://linear.app/ghost/issue/BER-3864/differentiate-public-and-private-member-projections Three other surfaces narrow a member for their own audiences, and their field lists were copied into this module so the four could be read side by side. The copies were never imported and nothing checked them against the call sites they described, so they were commentary in the shape of code: free to drift while still reading as authoritative, which is a worse failure than the drift they were meant to expose. The finding they were carrying belongs in the issue, where it can be argued with.
ref https://linear.app/ghost/issue/BER-3864/differentiate-public-and-private-member-projections The members API served Portal a member assembled in the application: load the row through Bookshelf, then walk it through a sequence of services that each attach another piece. What a member receives was the result of that walk rather than anything stated, and the walk pulled in relations only staff ever read. The projection is a query now. The member and each of its collections are read directly, the rows are decoded into the payload, and what a member is shown is described in one place by the query that fetches it and the codec that shapes it. Nothing a member receives changes; the tests taken before any of this moved are what say so, and they pass unedited. Collections are separate queries rather than one statement. A member has two independent collections, and joining both returns their product, so each subscription would repeat once per newsletter. The alternative is aggregating each collection into a JSON column, and that cannot be written once: MySQL and SQLite spell the aggregate differently, and MySQL will not accept an ordering inside it, so the newsletter order this projection promises could not be expressed in SQL at all. Split this way every query compiles identically for both engines and the ordering lives where it belongs. It also means one member and a page of them cost the same number of statements, which is what the admin list will need. The subscription a comped or gifted member holds is derived rather than stored, so it is derived by query too, from the products they hold and the event that granted them. Writing it that way forced a rule into the open that the application had been leaving to chance: a member can hold more than one redeemed gift, and the previous lookup took whichever the database happened to return last. The query ranks them and says why. What the queries do not read is anything another domain owns. An offer carries its own redemption counts and an attribution resolves a URL through routing configuration rather than a column, so both are asked for while decoding. The unsubscribe link is an HMAC over a secret and the avatar is a gravatar URL; both are handed to the codec rather than computed inside it. Identifying a signed-in member stays where it was. A session needs transient_id and last_seen_at, which a member is deliberately never shown, so covering it here would stop this being the member's own view — that surface wants a projection of its own.
ref https://linear.app/ghost/issue/BER-3864/differentiate-public-and-private-member-projections This module had its own pair of column codecs for the two things the engines disagree about. Both already exist in lib/db-types, both are better, and one of mine was wrong. SQLite has no date type and returns a date as `yyyy-MM-dd HH:mm:ss`. Handing that to `new Date()` reads it in whatever timezone the machine happens to be set to, so every date a member sees would have been shifted by the local offset — four hours out on a US East Coast development machine, and correct only where the offset is zero. Nothing here would have caught it: the test suite runs against MySQL, which returns dates already parsed. The shared codec goes through luxon with an explicit UTC zone, and a test now pins the SQLite spelling against a non-zero offset. The plan on a subscription has the same fix behind it. Every part of a plan is its own column, recording what the member agreed to when they subscribed, and that is deliberately not the price they are joined to today. The amount was already read from the right place; the identifier was not, and only matched because the two happen to be equal in the fixtures.
ref https://linear.app/ghost/issue/BER-3864/differentiate-public-and-private-member-projections The service was wiring the projection together by hand: group the rows, decode each kind, then walk the result three more times attaching offers, then payments, then attributions. Every one of those steps had to run, in that order, and the knowledge of what a member's payload contains was spread across the service rather than stated anywhere. Schemas nest, so the decode nests with them. A newsletter, a tier and each kind of subscription are now schemas that parse their own rows and return their own part of the payload, and the member schema simply says it has some of each. Reading a member is a query and a parse: `queries.read` gathers the rows that make one, and `MemberAccount` turns them into what a member receives. What a decode needs and a row cannot carry — what the next payment comes to, where an unsubscribe link points, whether the site shows gravatars — is passed in rather than reached for, so nothing in the schemas does any IO and all of them can be exercised with plain objects. The one ordering rule that is real, that a payment cannot be worked out before the offer it depends on, now sits inside the schema that needs it instead of being a comment on a method a caller had to remember. Offers and attributions still come from the domains that own them, but they are gathered once alongside the rows and travel with the subscription they describe, rather than being attached afterwards by walking the result again.
ref https://linear.app/ghost/issue/BER-3864/differentiate-public-and-private-member-projections The fields a member may change about themselves were a list of column names and a pick. That is what an object schema already does with unknown keys, so the list was a hand-rolled version of a mechanism this module uses everywhere else. Naming it for the request it belongs to rather than for the columns it happens to allow also makes room for the next one. What a member may send is a property of what they are asking to do, and setting their own metafields will accept a different body under different rules; a second command sits beside this one, where a second list of writable columns would have had to merge into it. The values stay unknown rather than typed. Typing them would start refusing a wrongly-typed value that Ghost currently accepts and coerces, which is a change to what the API does and belongs in a change that says so.
ref https://linear.app/ghost/issue/BER-3864/differentiate-public-and-private-member-projections What this module called a model was not one. It produced snake_case keys and ISO strings, which is how the API writes an account down rather than what an account is, and the response was then picked over a second time on the way out. Two mappings, one of them accidental, and no shape in between that anything could be reasoned about. There is now a model in the middle. It speaks the domain — dates are dates, names are camelCase, and the commenting record is the object its own codec produces rather than the form that codec publishes. The serializer turns one into the response the members API has always sent. Each can move without the other: a response key cannot be withdrawn once clients read it, so the wire shape is a promise, while a column can be added or renamed, so the row shape is not. `firstname` and `paid` moved to the serializer, where they belong. Neither is stored and neither is a fact about a member; they are conveniences for whoever renders one, and they appear in this response and nowhere else. `subscribed` moved with them and is now written down as the constant it has always been: a member has no such column, and the Admin API derives its own meaning for the word rather than reading one from here. The serializer is written out rather than run through a key converter. Half of what an account carries belongs to other domains and arrives in the shape those domains publish, some of it camelCase; converting keys wholesale would rewrite an offer or an attribution into something no client has been sent. The unit tests for the old function went with it. They asserted its behaviour against a member shape that no longer occurs — one carrying `subscribed`, which nothing produces — and what they were really covering is now split between the serializer's own tests and the response snapshots.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Ghost tells a member one thing about themselves, and tells staff a different thing about that same member. It builds a single set of details for both, and then each place that sends details outward removes whatever that place does not want. Nowhere states what a member is meant to receive.
Several places each do their own removing. None was written with reference to the others, and nothing checks them against each other, so they no longer match. A comment includes its author's internal database identifier. A member's own details leave that identifier out. Anyone can therefore learn the identifier of a person who commented, but not their own. Nobody decided that, and noticing it meant reading several files across unrelated parts of the codebase.
Building one set of details for both audiences also makes members pay for what only staff read. Recognising a signed-in reader runs the same preparation, and that happens on every page of every site, so work no member ever sees runs on every page view. Part of it is a separate database lookup asking which page first brought that member to the site.
Solution
There is now a single place that states what a member receives about themselves and what a member is allowed to change.
What members receive does not change at all. Tests written before anything else, and never edited afterwards, are what establish that; the first commit stands alone and is enough to check the claim. Those tests record whole responses rather than named fields, for a free member, a paying member, and a member given free access, because Ghost builds each of the three differently and because the failure worth catching is a field appearing or disappearing.
Preparing a member's details apart from staff's also means preparing less of them. Ghost no longer collects the private labels staff attach to a member, and no longer asks which page first brought them to the site. Neither has ever been shown to a member. Which page brought them to an individual subscription is still asked, because a site's theme can display it.
The other places keep removing fields exactly where they do today. What a theme can read is a published promise to theme authors; the newsletter preference pages identify a person by a signed link rather than a login, and withhold payment details on purpose; a comment's author belongs next to the staff version of the same thing. Combining them would mean showing at least one of those audiences more than it gets now. They are instead written down together, which is what made the disagreement above visible in the first place.
This covers one of those places on purpose. The aim is a single worked example, so the team can read real code and decide whether the pattern is worth repeating, instead of that being settled by applying it to three other areas at the same time. The remaining places, the comment author identifier, and a separate inconsistency in how the staff API answers its own request to include custom fields, all stay open on the issue.
It is also a prerequisite for letting members edit their own custom fields from their account, which needs one place to decide which fields a member may see and change.
ref https://linear.app/ghost/issue/BER-3864/differentiate-public-and-private-member-projections