🏗️ PUT-1712: emit workspace billing events for the payment integration - #3733
Conversation
Coverage Report
File Coverage |
86d5fb1 to
d149568
Compare
d149568 to
d3c1aa9
Compare
d3c1aa9 to
a0a76f3
Compare
a0a76f3 to
c0dd00b
Compare
|
Warning Superseded — the payloads below are out of date. They show Kept for history. See the re-validation comment below for the current shape. |
c0dd00b to
b039058
Compare
b039058 to
2ae83fd
Compare
2ae83fd to
3bb15f4
Compare
Local validation (re-run after the payload change)Replaces the comment above, which showed a Engine: mysql 8 · What a consumer now gets
Why the field wentNot tidiness — it was a hazard. The value was a snapshot taken at emit time, and the payer's row is alive throughout (unlike Concretely: provision two seats in quick succession for an owner who has never paid. Both events capture
Everything else is unchangedThe five events still fire at the same transitions, Full suite 7733 passed | 26 skipped, typecheck clean. |
3bb15f4 to
7359db3
Compare
|
Rebased onto One collision worth naming: those commits took migration Verified per commit rather than at the tip — each of the 9 typechecks on its own, so no intermediate commit is broken. Full suite 7883 passed | 26 skipped. |
Salazareo
left a comment
Salazareo
left a comment
There was a problem hiding this comment.
looks good but I think we should cache some of these store queries
7359db3 to
bb5e32b
Compare
Terminology: "workspace" is now "team", everywhereThe feature said Applied to every commit rather than added on top, so each PR shows its own code already named correctly and no intermediate commit references a name a later one changes. All 13 commits still typecheck individually. The contract-bearing renamesThese were free to change now and expensive after release, which is why they went in this pass:
Plus Scoped, and checked"workspace" means other things in this repo. These were deliberately left alone, and verified untouched:
Audited three ways: every changed line mentions team or workspace (no collateral edits), no stutters or mangled words ( One thing not renamedThe git branch names still say |
Done, in #3735 — and it now covers more than my first reply said, including the two I had originally argued for leaving alone. Cached, all on a 60s TTL with explicit invalidation:
I initially said I would leave Every path that can change either value busts it:
That last one is the reason this needed care: a TTL alone leaves a deleted account passing Falsified individually — each bust removed on its own, and only its own case fails:
Still not cached, deliberately: Backend 7897 passed | 26 skipped, typecheck clean, all 13 commits typecheck individually. |
`addMember` refuses to turn an account that already has a password into a team seat. No service path did this — `provisionAccount` always creates — but the store permitted it, and the design rules out existing accounts joining a team. Provisioning passes the guard because it admits the account before setting its temporary password. There is no bypass parameter. Both HTTP suites now provision a real seat and authenticate as it, using the same token-minting the harness uses for `POST /login`. That surfaced something worth knowing: an unactivated seat cannot call the API at all. Provisioning leaves `requires_email_confirmation` set and `requireVerified` rejects it, so the suites activate the seat first — which is the state a member is actually in when making requests. `listMembers` and `getMembership` also return `u.uuid`, which the billing events need in order to name the account without a second lookup.
The charge lives outside this repo, the way the marketplace extension cancels Stripe subscriptions off `user.delete`. This is the trigger, and it is the whole of what OSS owes billing. team.account.created a seat exists and can be used team.account.disabled it stopped, and still holds its bytes team.account.enabled it resumed team.account.deleted it is gone team.deleted the team is gone; its accounts are not Each carries the team uid, the affected account, and the owner's `stripe_customer_id`. That column ships in the mysql and postgres schemas but not sqlite, so the read is guarded and degrades to null, as `cascadeDelete` already does. Deleting a team emits one `team.account.disabled` per seat plus the team event, rather than one bulk event: the accounts persist, suspended, holding their files and their usernames. Deleting a team is not a way to stop paying for the accounts in it. `team.account.deleted` is captured before the row is deleted. `jct_user_group.user_id` is ON DELETE CASCADE, so by the time a listener on `user.delete` runs, nothing can say which team paid for the account. `getOrgSeat` deliberately admits soft-deleted teams: their accounts still exist, so the charge is still running. Closes PUT-1712.
bb5e32b to
fb97d72
Compare
2 participants
PUT-1712. The trigger for billing, and after the OSS/prod split this is the whole of what OSS owes it.
The charge lives outside this repo — the same place that cancels Stripe subscriptions off
user.delete. That event is the precedent: emit a typed event, let an extension do the external side effect.The events
team.account.createdteam.account.disabledteam.account.enabledteam.account.deletedteam.deletedEach carries the workspace uid, the affected account, and the owner's
stripe_customer_id.⚠
team.account.deletedhas to be captured before the deletejct_user_group.user_idisON DELETE CASCADE(0015_group.sql). By the time a listener onuser.deleteruns, the membership row is gone and nothing can say which workspace paid for the account — the payload is the only surviving evidence.So
cascadeDeletecaptures the seat identity before teardown and emits after, exactly as it already does foruuid/stripe_customer_idand for the same reason. A test asserts the membership is unreadable afterwards, so the constraint that forces this design is pinned rather than assumed.⚠
stripe_customer_iddoes not exist on sqliteIt ships in
mysql_mig_1andpostgres_mig_1but no sqlite migration —cascadeDeletealready wraps its read in atry/catchfor this.getStripeCustomerIddoes the same and degrades tonull, so a dev or self-hosted install emits usable events rather than throwing inside a lifecycle operation. The postgres suite covers the branch that returns a real value.Deleting a workspace emits per seat
One
team.account.disabledper seat plus the workspace marker, not one bulk event: the accounts persist, suspended, holding their files and their usernames.Deleting a workspace is not a way to stop paying for the accounts in it. Deleting the accounts is. PUT-1740's confirmation dialog has to say so, because the opposite is the natural assumption.
getOrgSeatdeliberately admits soft-deleted workspaces for the same reason — their accounts still exist, so the charge is still running.Verification
Per-site falsification
Each emit disabled individually:
Every site fails exactly one test.
What changed since the original #3722
The held-bytes report and
TeamBillingServiceare gone, not moved. Under the rescoped model a disabled seat is an ordinary suspended account holding files — not a team-specific case — so there is nothing team-shaped to measure. Whether the owner is charged for held data is now an open pricing question; prod can computeSUM(size)itself if we decide we want it.held_bytesstill rides on the disable and enable payloads, so prod has the figure at the transition without querying back.See
TEAMS-BILLING-SPLIT.mdand PUT-1765.Closes PUT-1712. PUT-1747 is superseded.