Repository navigation
feat(store): billing account delete hides the account's token transactions with the row - #1999
Open
rohilsurana wants to merge 10 commits into
Open
rohilsurana wants to merge 10 commits into
rohilsurana wants to merge 10 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 Summary
Merge Risk: 🟡 Moderate · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Linked Issues check |  |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving 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 @coderabbitai help to get the list of available commands.
Coverage Report for CI Build 39420937993Coverage increased (+0.05%) to 55.627%Details
Uncovered Changes
Coverage Regressions1 previously-covered line in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
rohilsurana
force-pushed
the
feat/billing-account-delete-hides-transactions
branch
from
October 14, 2026 06:27
4f3a2c0 to
88f48ee
Compare
rohilsurana
force-pushed
the
feat/billing-rows-soft-delete
branch
from
October 14, 2026 09:03
be61aad to
49f5fe5
Compare
rohilsurana
force-pushed
the
feat/billing-account-delete-hides-transactions
branch
from
October 14, 2026 09:04
88f48ee to
55510df
Compare
rohilsurana
marked this pull request as ready for review
October 14, 2026 10:45
rohilsurana
force-pushed
the
feat/billing-rows-soft-delete
branch
from
October 16, 2026 08:44
49f5fe5 to
170adbe
Compare
rohilsurana
force-pushed
the
feat/billing-account-delete-hides-transactions
branch
from
October 16, 2026 10:04
55510df to
5a6a5bc
Compare
rohilsurana
force-pushed
the
feat/billing-rows-soft-delete
branch
from
October 16, 2026 10:04
170adbe to
c2b82b8
Compare
rohilsurana
force-pushed
the
feat/billing-rows-soft-delete
branch
from
October 16, 2026 10:20
c2b82b8 to
e625872
Compare
rohilsurana
force-pushed
the
feat/billing-account-delete-hides-transactions
branch
from
October 16, 2026 10:21
5a6a5bc to
0286c5f
Compare
AmanGIT07
reviewed
Oct 16, 2026
rohilsurana
force-pushed
the
feat/billing-account-delete-hides-transactions
branch
from
October 19, 2026 09:13
5d7f7fc to
7f9267e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: raystack/frontier/.coderabbit.yaml
- Review profile: CHILL
- Plan: Advanced
- Run ID:
e8cc6bc4-0fe1-4ef3-ac62-1754a045a56d
📒 Files selected for processing: 18
billing/customer/errors.gobilling/customer/mocks/repository.gobilling/customer/service.gobilling/customer/service_test.gocmd/serve.gocore/audit/audit.gocore/deleter/delete_notice.gocore/deleter/delete_notice_test.gocore/deleter/mocks/audit_record_service.gocore/deleter/mocks/credit_service.gocore/deleter/service.gocore/deleter/service_test.godocs/content/docs/reference/webhook.mdxinternal/api/v1beta1connect/billing_errors.gointernal/api/v1beta1connect/billing_errors_test.gointernal/store/postgres/billing_customer_repository.gointernal/store/postgres/billing_customer_repository_pg_test.goweb/sdk/admin/utils/webhook-events.ts
💤 Files with no reviewable changes: 4
- docs/content/docs/reference/webhook.mdx
- core/deleter/mocks/credit_service.go
- web/sdk/admin/utils/webhook-events.ts
- core/audit/audit.go
Comment on lines
+521
to
+532
| hideTransactionsQuery := `WITH hidden AS ( | ||
| UPDATE billing_transactions SET deleted_at = now() | ||
| WHERE account_id = $1 AND deleted_at IS NULL | ||
| RETURNING type, source, amount | ||
| ) | ||
| SELECT coalesce(sum(CASE WHEN type = $2 THEN amount ELSE -amount END), 0), | ||
| coalesce(sum(CASE WHEN source = $3 THEN (CASE WHEN type = $2 THEN amount ELSE -amount END) ELSE 0 END), 0) | ||
| FROM hidden` | ||
| hideTransactionsParams := []any{id, credit.CreditType, credit.SourceSystemBuyEvent} | ||
|
|
||
| var customerModel Customer | ||
| var forfeitedTokens, purchasedTokens int64 |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
rg -n 'func .*CreateEntry|func .*CreateTransaction|Insert.*Transaction|insert into billing_transactions|Insert\("billing_transactions"|billing_transactions.*INSERT' billing internal | head -120
sed -n '105,130p' internal/store/postgres/migrations/20231012175641_create_billing_tables.up.sql
sed -n '500,595p' internal/store/postgres/billing_customer_repository.goRepository: raystack/frontier
Length of output: 6128
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- transaction repository ---'
sed -n '1,280p' internal/store/postgres/billing_transactions_repository.go
printf '%s\n' '--- concrete CreateEntry callers ---'
rg -n -F --glob '*.go' -e '.CreateEntry(' -e 'CreateEntry(' billing internal | head -240
printf '%s\n' '--- credit service and repository declarations ---'
rg -n -F --glob '*.go' -e 'type TransactionRepository' -e 'func (.*Credit' -e 'Credit(' billing internal | head -240
printf '%s\n' '--- focused tests ---'
sed -n '1,240p' internal/store/postgres/billing_transactions_repository_pg_test.go
printf '%s\n' '--- billing transaction migrations ---'
rg -n -F --glob '*.sql' -e 'billing_transactions' -e 'ALTER TABLE billing_transactions' internal/store/postgres/migrations | head -240Repository: raystack/frontier
Length of output: 23712
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- CreateEntry dependencies ---'
rg -n -F --glob 'internal/store/postgres/billing_transactions_repository.go' -e 'func (r BillingTransactionRepository) getBalanceInTx' -e 'func (r BillingTransactionRepository) GetBalance' -e 'customerRepo.GetByID' -e 'fromLive(TABLE_BILLING_TRANSACTIONS)' internal/store/postgres/billing_transactions_repository.go
rg -n -F --glob 'internal/store/postgres/billing_customer_repository.go' -e 'func (r BillingCustomerRepository) GetByID' -e 'func (r BillingCustomerRepository) Delete' internal/store/postgres/billing_customer_repository.go
printf '%s\n' '--- relevant transaction and customer blocks ---'
sed -n '100,245p' internal/store/postgres/billing_transactions_repository.go
sed -n '70,135p' internal/store/postgres/billing_customer_repository.go
sed -n '485,585p' internal/store/postgres/billing_customer_repository.go
printf '%s\n' '--- credit service interface and callers ---'
sed -n '1,175p' billing/credit/service.go
rg -n -F --glob '*.go' -e 'NewCreditService' -e 'credit.Service' -e 'Credit(ctx' -e 'Debit(ctx' billing internal cmd | head -240
printf '%s\n' '--- all billing transaction constraints in migrations ---'
rg -n -i --glob '*.sql' -e 'foreign key' -e 'references billing_customers' -e 'billing_transactions' internal/store/postgres/migrationsRepository: raystack/frontier
Length of output: 21189
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- production Add callers and wiring ---'
rg -n -F --glob '*.go' -g '!**/*_test.go' -e '.CreditService.Add(' -e '.creditService.Add(' -e 'CreditService.Add(' -e 'creditService.Add(' -e 'credit.Service' -e 'NewService(' billing internal cmd | head -260
printf '%s\n' '--- exact customer lookup and SQL helper blocks ---'
sed -n '175,235p' internal/store/postgres/billing_customer_repository.go
rg -n -F --glob 'internal/store/postgres/*.go' -e 'func fromLive' -e 'func softDelete' -e 'func (.*WithTxn' -e 'WithTxn(ctx' internal/store/postgres pkg/db
sed -n '1,120p' internal/store/postgres/query.go 2>/dev/null || true
sed -n '1,220p' pkg/db/client.go 2>/dev/null || trueRepository: raystack/frontier
Length of output: 14266
Lock the live customer in the credit transaction.
CreateEntry checks the target customer before it starts its transaction. credit.Service.Add then inserts the customer credit without reading or locking that customer inside the transaction.
If the pre-check succeeds and BillingCustomerRepository.Delete commits before the insert transaction starts, createTransactionEntry can insert a new live transaction for the soft-deleted account. The deletion hides only transactions that exist when its update runs. This leaves a live transaction outside the deletion totals.
Add a transaction-scoped live-customer check with FOR UPDATE before the balance check and transaction inserts. Return customer.ErrNotFound when the row is already soft-deleted.
Contributor
There was a problem hiding this comment.
@rohilsurana This is the gap the description lists. Can we close it in a follow-up with a FOR KEY SHARE read of the live customer inside CreateEntry's transaction? It only has to clash with the delete's FOR UPDATE.
Member
Author
There was a problem hiding this comment.
Agreed, opened #2010 for it with the FOR KEY SHARE approach. Leaving it out of this PR. The description points to it.
Member
Author
There was a problem hiding this comment.
Real, but not a fix for this PR. Tracked in #2010: the credit insert will take a FOR KEY SHARE read of the live customer inside its transaction, so it conflicts with the delete lock.
Contributor
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
AmanGIT07
reviewed
Oct 20, 2026
AmanGIT07
reviewed
Oct 20, 2026
| type CreditService interface { | ||
| GetBalance(ctx context.Context, accountID string) (int64, error) | ||
| List(ctx context.Context, flt credit.Filter) ([]credit.Transaction, error) | ||
| DeleteByAccountID(ctx context.Context, accountID string) error |
Contributor
There was a problem hiding this comment.
credit.Service.DeleteByAccountID has no callers after this change. Can we remove it, along with the repository method?
Member
Author
There was a problem hiding this comment.
Removed in e778039, along with the repository method, its test and the mock.
Contributor
|
The description still says the forfeit goes to |
…t calls the billing provider
rohilsurana
force-pushed
the
feat/billing-account-delete-hides-transactions
branch
from
October 20, 2026 12:15
7f9267e to
fc474d7
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: raystack/frontier/.coderabbit.yaml
- Review profile: CHILL
- Plan: Advanced
- Run ID:
b586b7de-26b5-4bc9-9f41-9ab440e0527f
📒 Files selected for processing: 19
billing/credit/mocks/transaction_repository.gobilling/credit/service.gobilling/customer/customer.gobilling/customer/mocks/repository.gobilling/customer/service.gobilling/customer/service_test.gocmd/serve.gocore/deleter/delete_notice.gocore/deleter/delete_notice_test.gocore/deleter/mocks/customer_service.gocore/deleter/service.gocore/deleter/service_test.gointernal/api/v1beta1connect/billing_customer.gointernal/api/v1beta1connect/interfaces.gointernal/api/v1beta1connect/mocks/customer_service.gointernal/store/postgres/billing_customer_repository.gointernal/store/postgres/billing_customer_repository_pg_test.gointernal/store/postgres/billing_transactions_repository.gointernal/store/postgres/billing_transactions_repository_pg_test.go
💤 Files with no reviewable changes: 4
- internal/store/postgres/billing_transactions_repository.go
- billing/credit/service.go
- internal/store/postgres/billing_transactions_repository_pg_test.go
- billing/credit/mocks/transaction_repository.go
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.
Comment on lines
+132
to
+135
| records, err := d.auditRecordService.List(ctx, utils.NewRQLQuery("", 0, forfeitRecordsLimit, []rql.Filter{ | ||
| {Name: "event", Operator: "eq", Value: auditrecord.BillingCustomerDeletedEvent.String()}, | ||
| {Name: "org_id", Operator: "eq", Value: orgID}, | ||
| }, []rql.Sort{{Name: "occurred_at", Order: "desc"}}, nil)) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '110,170p' core/deleter/delete_notice.go
rg -n 'forfeitRecordsLimit|recoverForfeitFromAudit|\\.List\\(ctx,.*query|Limit\\(' core/deleter internal/store/postgres/audit_record_repository.goRepository: raystack/frontier
Length of output: 2917
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- identifiers and call sites ---'
rg -n -F -- 'forfeitRecordsLimit' core internal || true
rg -n -F -- 'recoverForfeitFromAudit' . || true
rg -n -F -- 'DeleteRecordOrgTeardownKey' . || true
rg -n -F -- 'org_teardown' . || true
printf '%s\n' '--- delete notice source ---'
sed -n '1,260p' core/deleter/delete_notice.go
printf '%s\n' '--- delete notice callers ---'
rg -n -F -- 'deleteNotice' core/deleter || true
rg -n -F -- 'BuildDeleteNotice' core internal || true
rg -n -F -- 'DeleteNotice' core/deleter || true
printf '%s\n' '--- audit service and repository references ---'
rg -n -F -- 'List(ctx context.Context' internal core | grep -Ei 'audit|Audit' || true
rg -n -F -- 'NewRQLQuery' internal/store/postgres core | grep -Ei 'audit|Audit' || true
rg -n -F -- 'AuditRecord' internal/store/postgres | head -160 || true
printf '%s\n' '--- relevant diff ---'
git diff --stat 43d85db2d7cee24e0ba43851eb5cdeb3ae6f848f fc474d71e12f3bf1a8d811f34c807055f5369871 -- core/deleter internal/store/postgres billing
git diff --unified=35 43d85db2d7cee24e0ba43851eb5cdeb3ae6f848f fc474d71e12f3bf1a8d811f34c807055f5369871 -- core/deleter/delete_notice.goRepository: raystack/frontier
Length of output: 41723
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- identifiers and call sites ---'
rg -n -F -- 'forfeitRecordsLimit' core internal || true
rg -n -F -- 'recoverForfeitFromAudit' . || true
rg -n -F -- 'DeleteRecordOrgTeardownKey' . || true
rg -n -F -- 'org_teardown' . || true
printf '%s\n' '--- delete notice source ---'
sed -n '1,280p' core/deleter/delete_notice.go
printf '%s\n' '--- delete notice callers ---'
rg -n -F -- 'deleteNotice' core/deleter || true
rg -n -F -- 'BuildDeleteNotice' core internal || true
rg -n -F -- 'DeleteNotice' core/deleter || true
printf '%s\n' '--- audit service and repository references ---'
rg -n -F -- 'List(ctx context.Context' internal core | grep -Ei 'audit|Audit' || true
rg -n -F -- 'NewRQLQuery' internal/store/postgres core | grep -Ei 'audit|Audit' || true
rg -n -F -- 'AuditRecord' internal/store/postgres | head -160 || true
printf '%s\n' '--- relevant diff ---'
git diff --stat 43d85db2d7cee24e0ba43851eb5cdeb3ae6f848f fc474d71e12f3bf1a8d811f34c807055f5369871 -- core/deleter internal/store/postgres billing
git diff --unified=35 43d85db2d7cee24e0ba43851eb5cdeb3ae6f848f fc474d71e12f3bf1a8d811f34c807055f5369871 -- core/deleter/delete_notice.goRepository: raystack/frontier
Length of output: 42060
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- organization delete lifecycle ---'
sed -n '250,455p' core/deleter/service.go
printf '%s\n' '--- customer deletion and teardown metadata ---'
sed -n '470,625p' internal/store/postgres/billing_customer_repository.go
printf '%s\n' '--- audit record service and repository ---'
sed -n '175,235p' core/auditrecord/service.go
sed -n '145,225p' internal/store/postgres/audit_record_repository.go
printf '%s\n' '--- RQL query helper and SQL construction ---'
rg -n -F -- 'func NewRQLQuery' .
rg -n -F -- 'ApplyRQL' internal/store/postgres core pkg || true
rg -n -F -- 'rqlQuery.Limit' internal/store/postgres core pkg || true
rg -n -F -- 'rqlQuery.Offset' internal/store/postgres core pkg || trueRepository: raystack/frontier
Length of output: 17793
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- deleteCustomers continuation and options ---'
sed -n '430,505p' core/deleter/service.go
rg -n -F -- 'OrgTeardown:' . || true
rg -n -F -- 'DeleteOptions{' . || true
printf '%s\n' '--- standalone customer deletion callers ---'
rg -n -F -- 'customerService.Delete' core internal billing | head -100 || true
rg -n -F -- 'Delete(ctx context.Context, id string' billing/customer core internal | head -80 || true
printf '%s\n' '--- customer listing behavior ---'
rg -n -F -- 'func (.* List' internal/store/postgres/billing_customer_repository.go billing/customer core | head -80 || true
sed -n '1,180p' billing/customer/service.go
printf '%s\n' '--- audit filtered query and pagination ---'
rg -n -F -- 'func (r AuditRecordRepository) buildFilteredQuery' internal/store/postgres/audit_record_repository.go
sed -n '1,160p' internal/store/postgres/audit_record_repository.go
sed -n '1,120p' pkg/utils/rql.go
rg -n -F -- 'func AddRQLPaginationInQuery' pkg internal
rg -n -F -- 'func AddRQLFiltersInQuery' pkg internalRepository: raystack/frontier
Length of output: 29945
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- audit filtering and pagination implementation ---'
sed -n '130,225p' pkg/utils/rql.go
sed -n '335,435p' internal/store/postgres/audit_record_repository.go
printf '%s\n' '--- live customer listing implementation ---'
rg -n -F -- 'func (r BillingCustomerRepository) List' internal/store/postgres/billing_customer_repository.go
sed -n '120,205p' internal/store/postgres/billing_customer_repository.go
printf '%s\n' '--- customer service delete binding ---'
sed -n '255,300p' billing/customer/service.go
sed -n '145,175p' internal/api/v1beta1connect/billing_customer.goRepository: raystack/frontier
Length of output: 12334
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- exact customer List implementation and live scope ---'
sed -n '210,275p' internal/store/postgres/billing_customer_repository.go
rg -n -F -- 'func fromLive' internal/store/postgres
sed -n '1,45p' internal/store/postgres/billing_customer_repository.go
printf '%s\n' '--- audit list response shape ---'
rg -n -F -- 'type AuditRecordsList' core/auditrecord pkg
sed -n '1,55p' core/auditrecord/service.go
rg -n -F -- 'AuditRecordsList struct' .Repository: raystack/frontier
Length of output: 5086
🏁 Script executed:
set -o pipefail
sed -n '34,48p' internal/store/postgres/postgres.go
sed -n '40,62p' core/auditrecord/models/models.goRepository: raystack/frontier
Length of output: 1720
Paginate audit recovery before applying org_teardown.
A retry can omit forfeited and purchased amounts. The helper reads only the newest 1,000 billing_customer.deleted records for the organization. org_teardown is not an RQL-filterable field, so standalone deletions can fill that page before the helper checks the metadata. If more than 1,000 standalone accounts were deleted after a failed organization teardown, the older teardown record is not recovered. The retry then sends an understated notice because deleted accounts are excluded from the live customer list.
Suggested fix
--- "a/core/deleter/delete_notice.go"
+++ "b/core/deleter/delete_notice.go"
@@ -128,17 +128,25 @@
// audit records hold for accounts a failed earlier attempt already deleted,
// so the retry still reports them in the owner notice. Best-effort: without
// a readable audit store the notice keeps only the live amounts.
func (d Service) recoverForfeitFromAudit(ctx context.Context, orgID string, notice *deleteNotice) {
- records, err := d.auditRecordService.List(ctx, utils.NewRQLQuery("", 0, forfeitRecordsLimit, []rql.Filter{
+ filters := []rql.Filter{
{Name: "event", Operator: "eq", Value: auditrecord.BillingCustomerDeletedEvent.String()},
{Name: "org_id", Operator: "eq", Value: orgID},
- }, []rql.Sort{{Name: "occurred_at", Order: "desc"}}, nil))
- if err != nil {
- slog.WarnContext(ctx, "failed to check audit records for forfeited tokens", "org_id", orgID, "error", err)
- return
+ }
+ var recovered []auditrecord.AuditRecord
+ for offset := 0; ; offset += forfeitRecordsLimit {
+ page, err := d.auditRecordService.List(ctx, utils.NewRQLQuery("", offset, forfeitRecordsLimit, filters, []rql.Sort{{Name: "occurred_at", Order: "desc"}}, nil))
+ if err != nil {
+ slog.WarnContext(ctx, "failed to check audit records for forfeited tokens", "org_id", orgID, "error", err)
+ return
+ }
+ recovered = append(recovered, page.AuditRecords...)
+ if len(recovered) >= int(page.Page.TotalCount) || len(page.AuditRecords) < forfeitRecordsLimit {
+ break
+ }
}
seen := map[string]struct{}{}
- for _, record := range records.AuditRecords {
+ for _, record := range recovered {
accountID := record.Resource.ID
if accountID == "" {
continue
AmanGIT07
approved these changes
Oct 20, 2026
This branch was successfully deployed
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
3 participants
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.
Summary
Deleting a billing account now hides the account's token transactions and records what was forfeited on the account's
billing_customer.deletedaudit record.An account that owes tokens cannot be deleted. The API answers with failed precondition.
Changes
HasLiveRecordsrepository method.ErrHasTokenDebterror to failed precondition.billing_customer.deletedrecords. Only records written by an org delete count, so an account deleted on its own is not added to the notice. The record carriesorg_teardown: truefor this.app.billing.tokens.forfeitedaudit log event is removed, together with its entries in the webhook docs and the admin webhook event list. The event has shipped since v0.116.0. It was only an audit log entry and the forfeit now lives on the audit record, so it will no longer be sent as a webhook event.DeleteByAccountIDon the credit service and repository is removed.Technical Details
account_idis the customer id, never the system account.system.buy, kept between zero and the forfeited amount.org_teardownflag, so a retry that started before the deploy recovers nothing.Test Plan
go build ./...go veton the touched packagesgo test ./internal/store/postgres/ ./billing/... ./core/deleter/... ./internal/api/v1beta1connect/golangci-lint runon the touched packages, 0 issuesgofmton the changed filesSQL Safety