feat(community): add subscription billing - #331
Open
henry-casper wants to merge 5 commits into
Open
henry-casper wants to merge 5 commits into
henry-casper wants to merge 5 commits into
Conversation
Implement OCOM community subscription billing end to end, per task.md. Verification (Serenity/Cucumber): - Shared subscription-billing.feature with API, UI and e2e screenplay layers. Backend: - Community finance aggregate (subscription tier, payment instrument, transaction history) and a CommunityConfig aggregate supplying the effective per-member price. - @ocom/service-payment with an in-package mock provider, swap-ready for a real gateway. - GraphQL: communityCreate billing input, communityUpdateSubscriptionTier, communityUpdatePaymentInstrument, communityProcessSubscriptionCharge, communitySubscription, Community.finance and Community.paymentInstrument. - Billing commands resolve the acting member from the JWT subject plus the community, and require canManageCommunitySettings. Community portal: - Create form collects a plan and payment instrument; a payment token is required only when a subscription is being started. - Admin billing screen at settings/billing showing plan, price per member, billed member count, amount, card on file and charge history, with plan changes, instrument updates and manual charges. - Shared payment components in @ocom/ui-community-shared. Also fixes pre-existing issues that blocked a green build: - Implement the staff role scenario the feature file already specified. - Enforce enterprise app role permissions on staffRoleUpdate and staffUserAssignRole, which the feature file specified but the resolvers never applied; align the two mismatched test steps. - Add the missing tsconfig.vitest.json to six ui-staff packages and fix the type and selector errors this uncovered. - Refresh stale dependency pins and expired Snyk ignores so audit and snyk pass again. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Sorry @henry-casper, your pull request is larger than the review limit of 150,000 diff characters
Resolves the conflict with main and the PR #331 review. Merge: - Restores the tsgo language service work from PR #327 that this branch, based on an older main, appeared to revert (.grok/lsp.json, AGENTS.md, ADR-0034, .vscode and mise/package configuration now match main exactly). - Takes main's exact dependency pins instead of the ranges this branch introduced, keeping only the overrides main lacks (compression, moment, proxy-addr) and raising joi to 17.13.8 and fast-uri to 4.1.5 for current advisories. pnpm-lock.yaml was regenerated, not hand-edited. Blockers: - Billing reads now require canManageCommunitySettings. communitySubscription rejects unauthorized actors, and Community.finance and Community.paymentInstrument resolve to null for them so a member can still read the rest of the community. Covered by a new negative acceptance-api scenario. - The API refuses to start in production while PAYMENT_PROVIDER is the in-memory mock, so the mock cannot back real billing. - The subscription charge now happens outside the database transaction, which only records the outcome, and each attempt carries a unique referenceId so a retry can be deduplicated. - staffRoleUpdate validates the role being edited, not just the requested enterpriseAppRole, so omitting it no longer bypasses the check; staffUserAssignRole resolves the target role by id and rejects unknown ids. Significant: - Community provisioning has a single owner: the create service. The duplicate CommunityCreated handler and the existence checks that hid its races are gone. - GraphQL maps CommunityFinance to CommunityFinanceEntityReference, removing the dual-shape probing and the transactions cast. - The UI no longer keeps its own price table; prices come from CommunityConfig. - An unrecognised subscription tier is preserved rather than coerced to Pro. - Reading a community no longer writes a default finance value onto the document. - A declined charge is surfaced to the admin instead of reporting success. Also: bounds the money-parsing regexes flagged by CodeQL as polynomial, uses PermissionError for billing authorization failures, formats currency via Intl, and documents that money is stored in minor units. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Addresses the second review pass on PR #331. - PAYMENT_PROVIDER now selects the implementation instead of only being string-checked. Previously PAYMENT_PROVIDER=gateway booted and still registered the in-memory mock, so every charge reported success and instruments vanished on restart. A new ServicePaymentUnavailable lets the API start without a gateway while failing every billing operation loudly, "mock" stays rejected in production, and any other value refuses to start. The bootstrap test that encoded the old hole now asserts the rejection. - Subscription charge references are derived rather than random: communityId:YYYY-MM:<attempt>, where the attempt only advances once a charge has been recorded. Retrying a charge whose result was never persisted therefore reuses the key so the gateway can deduplicate it, while a deliberate second charge in the same period still gets a distinct key. - Community create and payment instrument updates vault the card before opening the Mongo transaction, so an abort can no longer orphan a vaulted instrument or lose the new instrument id. This matches what the charge path already did. - The community create container checks communityCreate.status.success, so a billing failure no longer reports "Community Created" and navigates away. - Clearing paymentInstrumentId goes through doc.set, because delete on a mongoose nested path is a silent no-op that left the stale id persisted. - The subscription query error reaches ComponentQueryLoader instead of rendering "$0.00 due", and querySubscription now throws for a missing CommunityConfig, matching processSubscriptionCharge for the same state. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes the reported runtime failure and the third review pass.
Runtime failure ("No community config found for subscription tier pro"):
nothing outside the verification seed ever created CommunityConfig rows, so every
real environment failed on community create and rendered an empty billing screen.
Adds an idempotent ensureDefaultConfigs, called from create, the subscription
query, tier updates and charges, mirroring how default staff roles are created.
- Community create validates the plan before vaulting a card or writing anything,
so a missing configuration can no longer leave an orphan community behind that a
retry would duplicate. A failure while raising the initial charge no longer fails
the create either, since the community is already committed.
- Saving a payment instrument no longer charges. It billed at whatever tier was
applied earlier in the same submit, contradicting "changing the plan does not
charge", and had no coverage.
- Subscription charges accept an idempotency key and skip billing when a charge is
already recorded for it; the billing screen also refuses to submit a second
charge while one is in flight.
- The billing screen renders a real error instead of a permanent skeleton: the
shared loader reports errors through a global toast on every render and falls
back to an empty skeleton, which looked like a screen stuck loading.
- The community list cache update tolerates a list that was never fetched and only
runs for a successful create, so a new community appears without a refresh.
E2E robustness, after verify failures caused by state left by interrupted runs:
- Pruning portless routes retries and then warns instead of aborting the suite
when the shared route lock is briefly held.
- A mongod left on the fixed port now reports what happened and how to clear it,
rather than a wall of stack traces.
- Adds `pnpm run e2e:doctor`, which reports any process still holding the e2e
ports and the command to stop it.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Addresses the fourth review pass on PR #331. - The payment provider now defaults to `unavailable` rather than the in-memory mock. dev-pri.json sets NODE_ENV=production without PAYMENT_PROVIDER, so the startup guard threw and the whole Function App failed to boot; a missing setting now disables billing instead. The setting is also declared explicitly in the deployed app settings, the tracked e2e settings, and the local dev settings builder, so no deployment can silently record fake successful charges. - Billing counts only billable seats. Every member record was charged, including deactivated members (a REJECTED account) and invitations that were never accepted (CREATED), which systematically over-billed. A member with no accounts is a seat an administrator created directly and still counts. - Charge idempotency is now enforced by the database and the provider, not just by an application check: a unique partial index on the transaction reference, and the mock provider returns the recorded result for a repeated reference instead of charging again, so the behaviour the application relies on is exercised. - communitySubscription no longer writes. Seeding default plan configuration was running from a read-only query under a system passport; it now happens only on write paths, and a unique index on tier plus effective date prevents duplicate rows. The index is declared separately from the existing one, because changing an existing index's options would conflict on deployed databases. - Community.finance and Community.paymentInstrument share one memoised permission lookup per request instead of each re-running the full actor-passport chain for every community in a result set. - Removes moment and proxy-addr from minimumReleaseAgeExclude. The supply-chain cooldown that forced those entries has since elapsed, so the overrides remain without the bypass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
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.
Implement OCOM community subscription billing end to end, per task.md.
Verification (Serenity/Cucumber):
Backend:
Community portal:
Also fixes pre-existing issues that blocked a green build: