feat(deleter): clear or report org delete blockers up front - #1857
feat(deleter): clear or report org delete blockers up front#1857whoAbhishekSah wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughSummary by CodeRabbit
WalkthroughOrganization deletion now performs billing preflight checks, aggregates structured blockers, and accepts token-forfeit acknowledgement. Billing-account deletion audits forfeited tokens. The Connect API returns structured failed-precondition details for blocked deletions. ChangesOrganization deletion workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
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. Comment |
29a0e18 to
783da24
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 51d78c26-5a73-43e9-8050-b9aec5ac46cb
⛔ Files ignored due to path filters (1)
proto/v1beta1/frontier.pb.gois excluded by!**/*.pb.go,!proto/**
📒 Files selected for processing (13)
Makefilebilling/invoice/invoice.gocore/audit/audit.gocore/deleter/deleter.gocore/deleter/mocks/credit_service.gocore/deleter/mocks/subscription_service.gocore/deleter/service.gocore/deleter/service_test.gogo.modinternal/api/v1beta1connect/deleter.gointernal/api/v1beta1connect/deleter_test.gointernal/api/v1beta1connect/interfaces.gointernal/api/v1beta1connect/mocks/cascade_deleter.go
| // the ack flag cannot bypass a debt | ||
| err = m.build().DeleteOrganization(context.Background(), "org-1", true) | ||
| assert.ErrorAs(t, err, &blocked) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the blocker type on the acknowledged retry.
The second call only checks that the error is a BlockedError. A regression that returns a different blocker type for ackTokenForfeit=true would still pass. Assert the type to close the gap.
💚 Proposed test assertion
// the ack flag cannot bypass a debt
err = m.build().DeleteOrganization(context.Background(), "org-1", true)
assert.ErrorAs(t, err, &blocked)
+ assert.Len(t, blocked.Blockers, 1)
+ assert.Equal(t, deleter.BlockerNegativeTokenBalance, blocked.Blockers[0].Type)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // the ack flag cannot bypass a debt | |
| err = m.build().DeleteOrganization(context.Background(), "org-1", true) | |
| assert.ErrorAs(t, err, &blocked) | |
| // the ack flag cannot bypass a debt | |
| err = m.build().DeleteOrganization(context.Background(), "org-1", true) | |
| assert.ErrorAs(t, err, &blocked) | |
| assert.Len(t, blocked.Blockers, 1) | |
| assert.Equal(t, deleter.BlockerNegativeTokenBalance, blocked.Blockers[0].Type) |
783da24 to
3c17128
Compare
3c17128 to
969ef16
Compare
whoAbhishekSah
changed the title
Coverage Report for CI Build 31694738096Coverage increased (+0.1%) to 48.363%Details
Uncovered Changes
Coverage Regressions1 previously-covered line in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
The delete first checks everything that blocks it and returns all the reasons together as one failed_precondition response: a running subscription on a paid plan (the caller downgrades it to the standard plan), unpaid (open or uncollectible) invoices, and a negative token balance which support has to settle. When nothing blocks, subscriptions still running on the standard plan are canceled immediately with unbilled usage invoiced on the spot, and the invoice check runs again so a final invoice still blocks the delete. Unused tokens do not block: the delete forfeits them and writes the amount to an audit record. An already-deleted org returns not found before any checks run. Invoice checks always sync from the billing provider first so the decision is made on fresh data.
969ef16 to
7117b6a
Compare
2 participants
Closes #1837. Top of the stack, based on #1865.
DeleteOrganizationnow checks everything that blocks the delete before touching any data, and returns every reason together as onefailed_preconditionresponse with aPreconditionFailuredetail (one violation per blocker). The design changed after review discussions, so this differs from the issue text in two ways: paid subscriptions ask for a downgrade instead of a cancel, and unused tokens no longer need an acknowledgement field (the proto change was dropped, raystack/proton#497 is closed).What blocks the delete:
ACTIVE_SUBSCRIPTION): the caller downgrades it to the standard plan (the configuredbilling.customer.default_plan) through the normal plan-change flow, then retries.UNPAID_INVOICE): an open or uncollectible invoice with a non-zero total. The caller pays it via its hosted payment page and retries.NEGATIVE_TOKEN_BALANCE): the account owes tokens (overdraft). Support has to settle it; the message says so.What does not block:
app.billing.tokens.forfeitedaudit record during teardown. A follow-up PR notifies the account owners by email.Other changes:
not_foundbefore any checks run. Disabled orgs stay deletable.UncollectibleState.errdetailsin the repo;google.golang.org/genproto/googleapis/rpcbecomes a direct dependency.🤖 Generated with Claude Code