From 74a957069494caeab93ab12d752d2063e40fe197 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Wed, 14 Oct 2026 11:46:05 +0530 Subject: [PATCH 01/10] feat(store): billing account delete hides the account's token transactions and reports the forfeit --- billing/customer/customer.go | 5 ++ billing/customer/errors.go | 1 + billing/customer/mocks/repository.go | 26 ++++-- billing/customer/service.go | 5 +- .../postgres/billing_customer_repository.go | 38 ++++++-- .../billing_customer_repository_pg_test.go | 89 +++++++++++++++++-- 6 files changed, 138 insertions(+), 26 deletions(-) diff --git a/billing/customer/customer.go b/billing/customer/customer.go index 43bf88d092..77a3c1fc78 100644 --- a/billing/customer/customer.go +++ b/billing/customer/customer.go @@ -135,3 +135,8 @@ func GetStripeWebhookSignatureFromContext(ctx context.Context) (string, bool) { u, ok := ctx.Value(consts.BillingStripeWebhookSignatureContextKey).(string) return u, ok } + +type ForfeitedTokens struct { + Amount int64 + Purchased int64 +} diff --git a/billing/customer/errors.go b/billing/customer/errors.go index 3c417e2b03..efdabf37a4 100644 --- a/billing/customer/errors.go +++ b/billing/customer/errors.go @@ -14,4 +14,5 @@ var ( ErrDisabled = errors.New("billing customer is disabled") ErrExistingAccountWithPendingDues = errors.New("existing account with pending dues found") ErrHasLiveRecords = errors.New("billing account still has live subscriptions, invoices or checkouts") + ErrHasTokenDebt = errors.New("billing account owes tokens, settle the balance before deleting it") ) diff --git a/billing/customer/mocks/repository.go b/billing/customer/mocks/repository.go index a6a22ad903..be020feda9 100644 --- a/billing/customer/mocks/repository.go +++ b/billing/customer/mocks/repository.go @@ -80,21 +80,31 @@ func (_c *Repository_Create_Call) RunAndReturn(run func(context.Context, custome } // Delete provides a mock function with given fields: ctx, id -func (_m *Repository) Delete(ctx context.Context, id string) error { +func (_m *Repository) Delete(ctx context.Context, id string) (customer.ForfeitedTokens, error) { ret := _m.Called(ctx, id) if len(ret) == 0 { panic("no return value specified for Delete") } - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string) error); ok { + var r0 customer.ForfeitedTokens + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, string) (customer.ForfeitedTokens, error)); ok { + return rf(ctx, id) + } + if rf, ok := ret.Get(0).(func(context.Context, string) customer.ForfeitedTokens); ok { r0 = rf(ctx, id) } else { - r0 = ret.Error(0) + r0 = ret.Get(0).(customer.ForfeitedTokens) } - return r0 + if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = rf(ctx, id) + } else { + r1 = ret.Error(1) + } + + return r0, r1 } // Repository_Delete_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Delete' @@ -116,12 +126,12 @@ func (_c *Repository_Delete_Call) Run(run func(ctx context.Context, id string)) return _c } -func (_c *Repository_Delete_Call) Return(_a0 error) *Repository_Delete_Call { - _c.Call.Return(_a0) +func (_c *Repository_Delete_Call) Return(_a0 customer.ForfeitedTokens, _a1 error) *Repository_Delete_Call { + _c.Call.Return(_a0, _a1) return _c } -func (_c *Repository_Delete_Call) RunAndReturn(run func(context.Context, string) error) *Repository_Delete_Call { +func (_c *Repository_Delete_Call) RunAndReturn(run func(context.Context, string) (customer.ForfeitedTokens, error)) *Repository_Delete_Call { _c.Call.Return(run) return _c } diff --git a/billing/customer/service.go b/billing/customer/service.go index 133af7db81..ecd3303b7b 100644 --- a/billing/customer/service.go +++ b/billing/customer/service.go @@ -30,7 +30,7 @@ type Repository interface { List(ctx context.Context, filter Filter) ([]Customer, error) Create(ctx context.Context, customer Customer) (Customer, error) UpdateByID(ctx context.Context, customer Customer) (Customer, error) - Delete(ctx context.Context, id string) error + Delete(ctx context.Context, id string) (ForfeitedTokens, error) UpdateCreditMinByID(ctx context.Context, customerID string, limit int64) (Details, error) GetDetailsByID(ctx context.Context, customerID string) (Details, error) UpdateDetailsByID(ctx context.Context, customerID string, details Details) (Details, error) @@ -290,7 +290,8 @@ func (s *Service) Delete(ctx context.Context, id string) error { } } - return s.repository.Delete(ctx, id) + _, err = s.repository.Delete(ctx, id) + return err } func (s *Service) ListPaymentMethods(ctx context.Context, id string) ([]PaymentMethod, error) { diff --git a/internal/store/postgres/billing_customer_repository.go b/internal/store/postgres/billing_customer_repository.go index 4d75ac9524..ed5d46bbac 100644 --- a/internal/store/postgres/billing_customer_repository.go +++ b/internal/store/postgres/billing_customer_repository.go @@ -16,6 +16,7 @@ import ( "github.com/doug-martin/goqu/v9" "github.com/doug-martin/goqu/v9/exp" "github.com/jmoiron/sqlx/types" + "github.com/raystack/frontier/billing/credit" "github.com/raystack/frontier/billing/customer" "github.com/raystack/frontier/pkg/auditrecord" "github.com/raystack/frontier/pkg/db" @@ -497,27 +498,38 @@ func (r BillingCustomerRepository) UpdateDetailsByID(ctx context.Context, custom }, nil } -func (r BillingCustomerRepository) Delete(ctx context.Context, id string) error { +func (r BillingCustomerRepository) Delete(ctx context.Context, id string) (customer.ForfeitedTokens, error) { lockQuery, lockParams, err := fromLive(TABLE_BILLING_CUSTOMERS). Select("id"). Where(goqu.Ex{"id": id}). ForUpdate(exp.Wait). ToSQL() if err != nil { - return fmt.Errorf("%w: %w", errParse, err) + return customer.ForfeitedTokens{}, fmt.Errorf("%w: %w", errParse, err) } query, params, err := softDelete(TABLE_BILLING_CUSTOMERS). Where(goqu.Ex{"id": id}). Returning(&Customer{}).ToSQL() if err != nil { - return fmt.Errorf("%w: %w", errParse, err) + return customer.ForfeitedTokens{}, fmt.Errorf("%w: %w", errParse, err) } liveRecordsQuery, liveRecordsParams, err := liveBillingRecordsQuery(id) if err != nil { - return fmt.Errorf("%w: %w", errParse, err) + return customer.ForfeitedTokens{}, fmt.Errorf("%w: %w", errParse, err) } + 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 forfeit customer.ForfeitedTokens if err = r.dbc.WithTxn(ctx, sql.TxOptions{}, func(tx *sqlx.Tx) error { return r.dbc.WithTimeout(ctx, TABLE_BILLING_CUSTOMERS, "Delete", func(ctx context.Context) error { var lockedID string @@ -532,6 +544,14 @@ func (r BillingCustomerRepository) Delete(ctx context.Context, id string) error if liveRecords { return customer.ErrHasLiveRecords } + var bought int64 + if err := tx.QueryRowxContext(ctx, hideTransactionsQuery, hideTransactionsParams...).Scan(&forfeit.Amount, &bought); err != nil { + return err + } + if forfeit.Amount < 0 { + return customer.ErrHasTokenDebt + } + forfeit.Purchased = min(max(bought, 0), forfeit.Amount) if err := tx.QueryRowxContext(ctx, query, params...).StructScan(&customerModel); err != nil { if errors.Is(err, sql.ErrNoRows) { return customer.ErrNotFound @@ -568,15 +588,17 @@ func (r BillingCustomerRepository) Delete(ctx context.Context, id string) error err = checkPostgresError(err) switch { case errors.Is(err, customer.ErrNotFound), errors.Is(err, sql.ErrNoRows): - return customer.ErrNotFound + return customer.ForfeitedTokens{}, customer.ErrNotFound case errors.Is(err, customer.ErrHasLiveRecords): - return customer.ErrHasLiveRecords + return customer.ForfeitedTokens{}, customer.ErrHasLiveRecords + case errors.Is(err, customer.ErrHasTokenDebt): + return customer.ForfeitedTokens{}, customer.ErrHasTokenDebt default: - return fmt.Errorf("%w: %w", errTxn, err) + return customer.ForfeitedTokens{}, fmt.Errorf("%w: %w", errTxn, err) } } - return nil + return forfeit, nil } func liveBillingRecordsQuery(customerID string) (string, []any, error) { diff --git a/internal/store/postgres/billing_customer_repository_pg_test.go b/internal/store/postgres/billing_customer_repository_pg_test.go index 5546f4c04b..8fe727408e 100644 --- a/internal/store/postgres/billing_customer_repository_pg_test.go +++ b/internal/store/postgres/billing_customer_repository_pg_test.go @@ -138,7 +138,7 @@ func TestBillingCustomerRepositoryPG(t *testing.T) { func (s *BillingCustomerRepositoryPGTestSuite) TestDeleteKeepsTheRowAndHidesIt() { id := s.customerID("bc-cust-live") - s.Require().NoError(s.repository.Delete(s.ctx, id)) + s.deleteAccount(id) deletedAt := scalarSQL(s.T(), s.ctx, s.client, `SELECT coalesce(deleted_at::text, '') FROM billing_customers WHERE id = $1`, id) @@ -149,7 +149,7 @@ func (s *BillingCustomerRepositoryPGTestSuite) TestDeleteKeepsTheRowAndHidesIt() s.Equal(deletedAt, occurredAt, "the audit record carries the delete time") _, err := s.repository.GetByID(s.ctx, id) s.ErrorIs(err, customer.ErrNotFound) - s.ErrorIs(s.repository.Delete(s.ctx, id), customer.ErrNotFound, "a second delete answers not found") + s.ErrorIs(s.deleteError(id), customer.ErrNotFound, "a second delete answers not found") } func (s *BillingCustomerRepositoryPGTestSuite) TestDeleteRefusesWhileLiveRecordsPointAtTheAccount() { @@ -179,13 +179,13 @@ func (s *BillingCustomerRepositoryPGTestSuite) TestDeleteRefusesWhileLiveRecords id := s.customerID("bc-cust-live") s.exec(tc.seed, id) - s.ErrorIs(s.repository.Delete(s.ctx, id), customer.ErrHasLiveRecords) + s.ErrorIs(s.deleteError(id), customer.ErrHasLiveRecords) s.Equal("", scalarSQL(s.T(), s.ctx, s.client, `SELECT coalesce(deleted_at::text, '') FROM billing_customers WHERE id = $1`, id), "the refused delete leaves the row live") s.exec(fmt.Sprintf(`UPDATE %s SET deleted_at = now() WHERE customer_id = $1`, tc.table), id) - s.NoError(s.repository.Delete(s.ctx, id), "a deleted child no longer blocks") + s.NoError(s.deleteError(id), "a deleted child no longer blocks") s.exec(`TRUNCATE TABLE billing_plans RESTART IDENTITY CASCADE`) s.exec(`UPDATE billing_customers SET deleted_at = NULL WHERE id = $1`, id) @@ -224,7 +224,9 @@ func (s *BillingCustomerRepositoryPGTestSuite) TestTeardownInTheDeleterOrderKeep s.Require().NoError(s.invoices.Delete(s.ctx, invID)) s.Require().NoError(s.checkouts.DeleteByCustomerID(s.ctx, cust)) s.Require().NoError(s.transactions.DeleteByAccountID(s.ctx, cust)) - s.Require().NoError(s.repository.Delete(s.ctx, cust)) + forfeit, err := s.repository.Delete(s.ctx, cust) + s.Require().NoError(err) + s.Equal(customer.ForfeitedTokens{}, forfeit, "the deleter hid the transactions already, so the account delete forfeits nothing a second time") s.Equal("1", s.deletedRows(postgres.TABLE_BILLING_SUBSCRIPTIONS, "customer_id", cust)) s.Equal("1", s.deletedRows(postgres.TABLE_BILLING_INVOICES, "customer_id", cust)) @@ -264,8 +266,8 @@ func (s *BillingCustomerRepositoryPGTestSuite) TestTeardownCanBeRunAgain() { s.Require().NoError(s.checkouts.DeleteByCustomerID(s.ctx, cust)) s.Require().NoError(s.transactions.DeleteByAccountID(s.ctx, cust)) s.Require().NoError(s.transactions.DeleteByAccountID(s.ctx, cust)) - s.Require().NoError(s.repository.Delete(s.ctx, cust)) - s.ErrorIs(s.repository.Delete(s.ctx, cust), customer.ErrNotFound) + s.Require().NoError(s.deleteError(cust)) + s.ErrorIs(s.deleteError(cust), customer.ErrNotFound) } func (s *BillingCustomerRepositoryPGTestSuite) TestDeleteWaitsForAnInFlightChildInsertAndRefuses() { @@ -280,7 +282,7 @@ func (s *BillingCustomerRepositoryPGTestSuite) TestDeleteWaitsForAnInFlightChild s.Require().NoError(err) result := make(chan error, 1) - go func() { result <- s.repository.Delete(s.ctx, id) }() + go func() { result <- s.deleteError(id) }() s.Require().Eventually(func() bool { waiting, err := blockedBy(s.ctx, s.client, holderPID) @@ -292,3 +294,74 @@ func (s *BillingCustomerRepositoryPGTestSuite) TestDeleteWaitsForAnInFlightChild s.Equal("", scalarSQL(s.T(), s.ctx, s.client, `SELECT coalesce(deleted_at::text, '') FROM billing_customers WHERE id = $1`, id)) } + +func (s *BillingCustomerRepositoryPGTestSuite) seedTransaction(account, txType, source string, amount int64) string { + s.T().Helper() + return scalarSQL(s.T(), s.ctx, s.client, + `INSERT INTO billing_transactions (account_id, type, source, amount, description) + VALUES ($1, $2, $3, $4, $2) RETURNING id`, account, txType, source, amount) +} + +func (s *BillingCustomerRepositoryPGTestSuite) transactionDeletedAt(id string) string { + s.T().Helper() + return scalarSQL(s.T(), s.ctx, s.client, + `SELECT coalesce(deleted_at::text, '') FROM billing_transactions WHERE id = $1`, id) +} + +func (s *BillingCustomerRepositoryPGTestSuite) TestDeleteHidesTheAccountsTransactions() { + id := s.customerID("bc-cust-live") + bought := s.seedTransaction(id, "credit", "system.buy", 200) + refunded := s.seedTransaction(id, "debit", "system.buy", 20) + awarded := s.seedTransaction(id, "credit", "system.awarded", 100) + used := s.seedTransaction(id, "debit", "system", 30) + alreadyHidden := s.seedTransaction(id, "credit", "system.buy", 50) + s.exec(`UPDATE billing_transactions SET deleted_at = now() - interval '1 day' WHERE id = $1`, alreadyHidden) + hiddenAt := s.transactionDeletedAt(alreadyHidden) + + forfeit, err := s.repository.Delete(s.ctx, id) + s.Require().NoError(err) + + s.Equal(customer.ForfeitedTokens{Amount: 250, Purchased: 180}, forfeit, + "the live credits minus the live debits, and the share of it bought net of refunds") + for _, txn := range []string{bought, refunded, awarded, used} { + s.NotEmpty(s.transactionDeletedAt(txn)) + } + s.Equal(hiddenAt, s.transactionDeletedAt(alreadyHidden), "an older delete time is kept") + balance, err := s.transactions.GetBalance(s.ctx, id) + s.Require().NoError(err) + s.Equal(int64(0), balance) +} + +func (s *BillingCustomerRepositoryPGTestSuite) TestDeleteRefusedByLiveRecordsLeavesTheTransactionsLive() { + id := s.customerID("bc-cust-live") + s.exec(`INSERT INTO billing_checkouts (customer_id, provider_id, checkout_url, state) + VALUES ($1, 'bc-session', 'bc-session', 'pending')`, id) + txn := s.seedTransaction(id, "credit", "system.buy", 200) + + s.ErrorIs(s.deleteError(id), customer.ErrHasLiveRecords) + + s.Empty(s.transactionDeletedAt(txn)) +} + +func (s *BillingCustomerRepositoryPGTestSuite) TestDeleteRollsBackWhenTheTransactionsNetToDebt() { + id := s.customerID("bc-cust-live") + credited := s.seedTransaction(id, "credit", "system.buy", 20) + used := s.seedTransaction(id, "debit", "system", 70) + + s.ErrorIs(s.deleteError(id), customer.ErrHasTokenDebt) + + s.Empty(s.transactionDeletedAt(credited)) + s.Empty(s.transactionDeletedAt(used)) + s.Equal("", scalarSQL(s.T(), s.ctx, s.client, + `SELECT coalesce(deleted_at::text, '') FROM billing_customers WHERE id = $1`, id)) +} + +func (s *BillingCustomerRepositoryPGTestSuite) deleteError(id string) error { + _, err := s.repository.Delete(s.ctx, id) + return err +} + +func (s *BillingCustomerRepositoryPGTestSuite) deleteAccount(id string) { + s.T().Helper() + s.Require().NoError(s.deleteError(id)) +} From 274437e039593eac7673fbfc546f46aa451fa9a3 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Wed, 14 Oct 2026 11:46:34 +0530 Subject: [PATCH 02/10] feat(billing): billing account delete refuses while the account owes tokens --- billing/customer/service.go | 8 ++++++ billing/customer/service_test.go | 26 +++++++++++++++++++ internal/api/v1beta1connect/billing_errors.go | 2 ++ .../api/v1beta1connect/billing_errors_test.go | 6 +++++ 4 files changed, 42 insertions(+) diff --git a/billing/customer/service.go b/billing/customer/service.go index ecd3303b7b..95558304d9 100644 --- a/billing/customer/service.go +++ b/billing/customer/service.go @@ -273,6 +273,14 @@ func (s *Service) Delete(ctx context.Context, id string) error { return err } + balance, err := s.creditService.GetBalance(ctx, id) + if err != nil { + return fmt.Errorf("failed to get token balance of billing account[%s]: %w", id, err) + } + if balance < 0 { + return ErrHasTokenDebt + } + // TODO: cancel and delete all subscriptions before deleting the customer if customer.ProviderID != "" { diff --git a/billing/customer/service_test.go b/billing/customer/service_test.go index 348c282157..3105c2153d 100644 --- a/billing/customer/service_test.go +++ b/billing/customer/service_test.go @@ -509,3 +509,29 @@ func TestService_List(t *testing.T) { }) } } + +func TestService_Delete(t *testing.T) { + ctx := context.Background() + account := customer.Customer{ID: "acc-1", OrgID: "org-1"} + + t.Run("refuses while the account owes tokens", func(t *testing.T) { + _, _, repo, creditSvc := mockService(t) + repo.EXPECT().GetByID(ctx, "acc-1").Return(account, nil) + creditSvc.EXPECT().GetBalance(ctx, "acc-1").Return(int64(-50), nil) + svc := customer.NewService(slog.Default(), nil, repo, billing.Config{}, creditSvc) + + err := svc.Delete(ctx, "acc-1") + + assert.ErrorIs(t, err, customer.ErrHasTokenDebt) + }) + + t.Run("deletes the account when it owes nothing", func(t *testing.T) { + _, _, repo, creditSvc := mockService(t) + repo.EXPECT().GetByID(ctx, "acc-1").Return(account, nil) + creditSvc.EXPECT().GetBalance(ctx, "acc-1").Return(int64(0), nil) + repo.EXPECT().Delete(ctx, "acc-1").Return(customer.ForfeitedTokens{}, nil) + svc := customer.NewService(slog.Default(), nil, repo, billing.Config{}, creditSvc) + + assert.NoError(t, svc.Delete(ctx, "acc-1")) + }) +} diff --git a/internal/api/v1beta1connect/billing_errors.go b/internal/api/v1beta1connect/billing_errors.go index af840537fd..f83ca1996a 100644 --- a/internal/api/v1beta1connect/billing_errors.go +++ b/internal/api/v1beta1connect/billing_errors.go @@ -71,6 +71,8 @@ func mapBillingErrorCode(err error) *connect.Error { return connect.NewError(connect.CodeFailedPrecondition, customer.ErrExistingAccountWithPendingDues) case errors.Is(err, customer.ErrHasLiveRecords): return connect.NewError(connect.CodeFailedPrecondition, customer.ErrHasLiveRecords) + case errors.Is(err, customer.ErrHasTokenDebt): + return connect.NewError(connect.CodeFailedPrecondition, customer.ErrHasTokenDebt) case errors.Is(err, customer.ErrNotFound): return connect.NewError(connect.CodeNotFound, ErrCustomerNotFound) case errors.Is(err, checkout.ErrAlreadySubscribed): diff --git a/internal/api/v1beta1connect/billing_errors_test.go b/internal/api/v1beta1connect/billing_errors_test.go index e3e3706286..dbb2a8cfdc 100644 --- a/internal/api/v1beta1connect/billing_errors_test.go +++ b/internal/api/v1beta1connect/billing_errors_test.go @@ -113,6 +113,12 @@ func TestMapBillingError(t *testing.T) { wantCode: connect.CodeFailedPrecondition, wantMsg: customer.ErrHasLiveRecords.Error(), }, + { + name: "token debt blocks the account delete", + err: fmt.Errorf("DeleteBillingAccount.Delete: customer_id=x: %w", customer.ErrHasTokenDebt), + wantCode: connect.CodeFailedPrecondition, + wantMsg: customer.ErrHasTokenDebt.Error(), + }, { name: "customer not found", err: fmt.Errorf("CreateCheckout.GetBillingAccountFromOrgID: org_id=abc: %w", customer.ErrNotFound), From c7cdfe2076694d5aa92242da1e36b0a9c96f9be8 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Wed, 14 Oct 2026 11:47:21 +0530 Subject: [PATCH 03/10] feat(billing): billing account delete writes the forfeited tokens audit record --- billing/customer/service.go | 24 +++++++++++- billing/customer/service_test.go | 65 +++++++++++++++++++++++++++++++- 2 files changed, 85 insertions(+), 4 deletions(-) diff --git a/billing/customer/service.go b/billing/customer/service.go index 95558304d9..ba93b26286 100644 --- a/billing/customer/service.go +++ b/billing/customer/service.go @@ -6,9 +6,11 @@ import ( "fmt" "log/slog" "math/rand" + "strconv" "sync" "time" + "github.com/raystack/frontier/core/audit" "github.com/raystack/frontier/pkg/utils" "github.com/robfig/cron/v3" @@ -298,8 +300,26 @@ func (s *Service) Delete(ctx context.Context, id string) error { } } - _, err = s.repository.Delete(ctx, id) - return err + forfeit, err := s.repository.Delete(ctx, id) + if err != nil { + return err + } + if forfeit.Amount > 0 { + s.logTokensForfeited(context.WithoutCancel(ctx), customer, forfeit) + } + return nil +} + +func (s *Service) logTokensForfeited(ctx context.Context, customer Customer, forfeit ForfeitedTokens) { + if err := audit.NewLogger(ctx, customer.OrgID).LogWithAttrs(audit.BillingTokensForfeitedEvent, audit.Target{ + ID: customer.ID, + Type: "billing_account", + }, map[string]string{ + "amount": strconv.FormatInt(forfeit.Amount, 10), + "purchased": strconv.FormatInt(forfeit.Purchased, 10), + }); err != nil { + s.log.WarnContext(ctx, "failed to write audit log", "error", err, "event", audit.BillingTokensForfeitedEvent, "customer_id", customer.ID) + } } func (s *Service) ListPaymentMethods(ctx context.Context, id string) ([]PaymentMethod, error) { diff --git a/billing/customer/service_test.go b/billing/customer/service_test.go index 3105c2153d..c73ff18bf3 100644 --- a/billing/customer/service_test.go +++ b/billing/customer/service_test.go @@ -7,18 +7,37 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/raystack/frontier/billing" "github.com/raystack/frontier/billing/customer" "github.com/raystack/frontier/billing/customer/mocks" stripemock "github.com/raystack/frontier/billing/stripetest/mocks" + "github.com/raystack/frontier/core/audit" "github.com/stripe/stripe-go/v79" "github.com/stripe/stripe-go/v79/client" ) var errSample = errors.New("sample error") +type recordingAuditRepository struct { + audit.NoopRepository + logs []audit.Log +} + +func (r *recordingAuditRepository) Create(ctx context.Context, log *audit.Log) error { + if err := ctx.Err(); err != nil { + return err + } + r.logs = append(r.logs, *log) + return nil +} + +func contextWithAudit(parent context.Context, repository audit.Repository) context.Context { + return audit.SetContextWithService(parent, audit.NewService("test", repository, audit.NewNoopWebhookService())) +} + func mockService(t *testing.T) (*client.API, *stripemock.Backend, *mocks.Repository, *mocks.CreditService) { t.Helper() mockRepository := mocks.NewRepository(t) @@ -525,13 +544,55 @@ func TestService_Delete(t *testing.T) { assert.ErrorIs(t, err, customer.ErrHasTokenDebt) }) - t.Run("deletes the account when it owes nothing", func(t *testing.T) { + t.Run("a zero forfeit writes no audit record", func(t *testing.T) { _, _, repo, creditSvc := mockService(t) + auditRepo := &recordingAuditRepository{} + ctx := contextWithAudit(ctx, auditRepo) repo.EXPECT().GetByID(ctx, "acc-1").Return(account, nil) creditSvc.EXPECT().GetBalance(ctx, "acc-1").Return(int64(0), nil) repo.EXPECT().Delete(ctx, "acc-1").Return(customer.ForfeitedTokens{}, nil) svc := customer.NewService(slog.Default(), nil, repo, billing.Config{}, creditSvc) - assert.NoError(t, svc.Delete(ctx, "acc-1")) + require.NoError(t, svc.Delete(ctx, "acc-1")) + + assert.Empty(t, auditRepo.logs) + }) + + t.Run("writes the forfeited tokens audit record", func(t *testing.T) { + _, _, repo, creditSvc := mockService(t) + auditRepo := &recordingAuditRepository{} + ctx := contextWithAudit(ctx, auditRepo) + repo.EXPECT().GetByID(ctx, "acc-1").Return(account, nil) + creditSvc.EXPECT().GetBalance(ctx, "acc-1").Return(int64(100), nil) + repo.EXPECT().Delete(ctx, "acc-1").Return(customer.ForfeitedTokens{Amount: 100, Purchased: 40}, nil) + svc := customer.NewService(slog.Default(), nil, repo, billing.Config{}, creditSvc) + + require.NoError(t, svc.Delete(ctx, "acc-1")) + + require.Len(t, auditRepo.logs, 1) + log := auditRepo.logs[0] + assert.Equal(t, string(audit.BillingTokensForfeitedEvent), log.Action) + assert.Equal(t, "org-1", log.OrgID) + assert.Equal(t, audit.Target{ID: "acc-1", Type: "billing_account"}, log.Target) + assert.Equal(t, "100", log.Metadata["amount"]) + assert.Equal(t, "40", log.Metadata["purchased"]) + }) + + t.Run("writes the record even when the request context is cancelled", func(t *testing.T) { + _, _, repo, creditSvc := mockService(t) + auditRepo := &recordingAuditRepository{} + requestCtx, cancel := context.WithCancel(contextWithAudit(ctx, auditRepo)) + repo.EXPECT().GetByID(mock.Anything, "acc-1").Return(account, nil) + creditSvc.EXPECT().GetBalance(mock.Anything, "acc-1").Return(int64(100), nil) + repo.EXPECT().Delete(mock.Anything, "acc-1").RunAndReturn( + func(context.Context, string) (customer.ForfeitedTokens, error) { + cancel() + return customer.ForfeitedTokens{Amount: 100, Purchased: 40}, nil + }) + svc := customer.NewService(slog.Default(), nil, repo, billing.Config{}, creditSvc) + + require.NoError(t, svc.Delete(requestCtx, "acc-1")) + + assert.Len(t, auditRepo.logs, 1) }) } From 0e3fec12c49e8384563f7b1c1bc3ea9339333f7c Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Wed, 14 Oct 2026 14:28:17 +0530 Subject: [PATCH 04/10] fix(billing): billing account delete checks for live records before it calls the billing provider --- billing/customer/mocks/repository.go | 57 +++++++++++++++++++ billing/customer/service.go | 9 +++ billing/customer/service_test.go | 29 ++++++++++ .../postgres/billing_customer_repository.go | 15 +++++ .../billing_customer_repository_pg_test.go | 19 +++++++ 5 files changed, 129 insertions(+) diff --git a/billing/customer/mocks/repository.go b/billing/customer/mocks/repository.go index be020feda9..6ffbbe86ae 100644 --- a/billing/customer/mocks/repository.go +++ b/billing/customer/mocks/repository.go @@ -250,6 +250,63 @@ func (_c *Repository_GetDetailsByID_Call) RunAndReturn(run func(context.Context, return _c } +// HasLiveRecords provides a mock function with given fields: ctx, id +func (_m *Repository) HasLiveRecords(ctx context.Context, id string) (bool, error) { + ret := _m.Called(ctx, id) + + if len(ret) == 0 { + panic("no return value specified for HasLiveRecords") + } + + var r0 bool + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, string) (bool, error)); ok { + return rf(ctx, id) + } + if rf, ok := ret.Get(0).(func(context.Context, string) bool); ok { + r0 = rf(ctx, id) + } else { + r0 = ret.Get(0).(bool) + } + + if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = rf(ctx, id) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Repository_HasLiveRecords_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'HasLiveRecords' +type Repository_HasLiveRecords_Call struct { + *mock.Call +} + +// HasLiveRecords is a helper method to define mock.On call +// - ctx context.Context +// - id string +func (_e *Repository_Expecter) HasLiveRecords(ctx interface{}, id interface{}) *Repository_HasLiveRecords_Call { + return &Repository_HasLiveRecords_Call{Call: _e.mock.On("HasLiveRecords", ctx, id)} +} + +func (_c *Repository_HasLiveRecords_Call) Run(run func(ctx context.Context, id string)) *Repository_HasLiveRecords_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string)) + }) + return _c +} + +func (_c *Repository_HasLiveRecords_Call) Return(_a0 bool, _a1 error) *Repository_HasLiveRecords_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *Repository_HasLiveRecords_Call) RunAndReturn(run func(context.Context, string) (bool, error)) *Repository_HasLiveRecords_Call { + _c.Call.Return(run) + return _c +} + // List provides a mock function with given fields: ctx, filter func (_m *Repository) List(ctx context.Context, filter customer.Filter) ([]customer.Customer, error) { ret := _m.Called(ctx, filter) diff --git a/billing/customer/service.go b/billing/customer/service.go index ba93b26286..ddf7fbbfb5 100644 --- a/billing/customer/service.go +++ b/billing/customer/service.go @@ -33,6 +33,7 @@ type Repository interface { Create(ctx context.Context, customer Customer) (Customer, error) UpdateByID(ctx context.Context, customer Customer) (Customer, error) Delete(ctx context.Context, id string) (ForfeitedTokens, error) + HasLiveRecords(ctx context.Context, id string) (bool, error) UpdateCreditMinByID(ctx context.Context, customerID string, limit int64) (Details, error) GetDetailsByID(ctx context.Context, customerID string) (Details, error) UpdateDetailsByID(ctx context.Context, customerID string, details Details) (Details, error) @@ -283,6 +284,14 @@ func (s *Service) Delete(ctx context.Context, id string) error { return ErrHasTokenDebt } + hasLiveRecords, err := s.repository.HasLiveRecords(ctx, id) + if err != nil { + return fmt.Errorf("failed to check live records of billing account[%s]: %w", id, err) + } + if hasLiveRecords { + return ErrHasLiveRecords + } + // TODO: cancel and delete all subscriptions before deleting the customer if customer.ProviderID != "" { diff --git a/billing/customer/service_test.go b/billing/customer/service_test.go index c73ff18bf3..ecad3753e2 100644 --- a/billing/customer/service_test.go +++ b/billing/customer/service_test.go @@ -550,6 +550,7 @@ func TestService_Delete(t *testing.T) { ctx := contextWithAudit(ctx, auditRepo) repo.EXPECT().GetByID(ctx, "acc-1").Return(account, nil) creditSvc.EXPECT().GetBalance(ctx, "acc-1").Return(int64(0), nil) + repo.EXPECT().HasLiveRecords(ctx, "acc-1").Return(false, nil) repo.EXPECT().Delete(ctx, "acc-1").Return(customer.ForfeitedTokens{}, nil) svc := customer.NewService(slog.Default(), nil, repo, billing.Config{}, creditSvc) @@ -564,6 +565,7 @@ func TestService_Delete(t *testing.T) { ctx := contextWithAudit(ctx, auditRepo) repo.EXPECT().GetByID(ctx, "acc-1").Return(account, nil) creditSvc.EXPECT().GetBalance(ctx, "acc-1").Return(int64(100), nil) + repo.EXPECT().HasLiveRecords(ctx, "acc-1").Return(false, nil) repo.EXPECT().Delete(ctx, "acc-1").Return(customer.ForfeitedTokens{Amount: 100, Purchased: 40}, nil) svc := customer.NewService(slog.Default(), nil, repo, billing.Config{}, creditSvc) @@ -584,6 +586,7 @@ func TestService_Delete(t *testing.T) { requestCtx, cancel := context.WithCancel(contextWithAudit(ctx, auditRepo)) repo.EXPECT().GetByID(mock.Anything, "acc-1").Return(account, nil) creditSvc.EXPECT().GetBalance(mock.Anything, "acc-1").Return(int64(100), nil) + repo.EXPECT().HasLiveRecords(mock.Anything, "acc-1").Return(false, nil) repo.EXPECT().Delete(mock.Anything, "acc-1").RunAndReturn( func(context.Context, string) (customer.ForfeitedTokens, error) { cancel() @@ -595,4 +598,30 @@ func TestService_Delete(t *testing.T) { assert.Len(t, auditRepo.logs, 1) }) + + t.Run("refuses before the billing provider is called while live records exist", func(t *testing.T) { + stripeClient, _, repo, creditSvc := mockService(t) + online := customer.Customer{ID: "acc-1", OrgID: "org-1", ProviderID: "cus_1"} + repo.EXPECT().GetByID(ctx, "acc-1").Return(online, nil) + creditSvc.EXPECT().GetBalance(ctx, "acc-1").Return(int64(0), nil) + repo.EXPECT().HasLiveRecords(ctx, "acc-1").Return(true, nil) + svc := customer.NewService(slog.Default(), stripeClient, repo, billing.Config{}, creditSvc) + + err := svc.Delete(ctx, "acc-1") + + assert.ErrorIs(t, err, customer.ErrHasLiveRecords) + }) + + t.Run("deletes the billing provider customer when no live records exist", func(t *testing.T) { + stripeClient, stripeBackend, repo, creditSvc := mockService(t) + online := customer.Customer{ID: "acc-1", OrgID: "org-1", ProviderID: "cus_1"} + repo.EXPECT().GetByID(ctx, "acc-1").Return(online, nil) + creditSvc.EXPECT().GetBalance(ctx, "acc-1").Return(int64(0), nil) + repo.EXPECT().HasLiveRecords(ctx, "acc-1").Return(false, nil) + stripeBackend.EXPECT().Call("DELETE", "/v1/customers/cus_1", "key_123", mock.Anything, mock.Anything).Return(nil) + repo.EXPECT().Delete(ctx, "acc-1").Return(customer.ForfeitedTokens{}, nil) + svc := customer.NewService(slog.Default(), stripeClient, repo, billing.Config{}, creditSvc) + + require.NoError(t, svc.Delete(ctx, "acc-1")) + }) } diff --git a/internal/store/postgres/billing_customer_repository.go b/internal/store/postgres/billing_customer_repository.go index ed5d46bbac..d556639b5f 100644 --- a/internal/store/postgres/billing_customer_repository.go +++ b/internal/store/postgres/billing_customer_repository.go @@ -601,6 +601,21 @@ func (r BillingCustomerRepository) Delete(ctx context.Context, id string) (custo return forfeit, nil } +func (r BillingCustomerRepository) HasLiveRecords(ctx context.Context, id string) (bool, error) { + query, params, err := liveBillingRecordsQuery(id) + if err != nil { + return false, fmt.Errorf("%w: %w", errParse, err) + } + + var liveRecords bool + if err = r.dbc.WithTimeout(ctx, TABLE_BILLING_CUSTOMERS, "HasLiveRecords", func(ctx context.Context) error { + return r.dbc.QueryRowxContext(ctx, query, params...).Scan(&liveRecords) + }); err != nil { + return false, fmt.Errorf("%w: %w", errDB, checkPostgresError(err)) + } + return liveRecords, nil +} + func liveBillingRecordsQuery(customerID string) (string, []any, error) { return dialect.Select(goqu.Or( goqu.L("EXISTS (SELECT 1 FROM billing_subscriptions WHERE customer_id = ? AND deleted_at IS NULL)", customerID), diff --git a/internal/store/postgres/billing_customer_repository_pg_test.go b/internal/store/postgres/billing_customer_repository_pg_test.go index 8fe727408e..1d603dd39e 100644 --- a/internal/store/postgres/billing_customer_repository_pg_test.go +++ b/internal/store/postgres/billing_customer_repository_pg_test.go @@ -356,6 +356,25 @@ func (s *BillingCustomerRepositoryPGTestSuite) TestDeleteRollsBackWhenTheTransac `SELECT coalesce(deleted_at::text, '') FROM billing_customers WHERE id = $1`, id)) } +func (s *BillingCustomerRepositoryPGTestSuite) TestHasLiveRecordsSeesOnlyLiveChildren() { + id := s.customerID("bc-cust-live") + + has, err := s.repository.HasLiveRecords(s.ctx, id) + s.Require().NoError(err) + s.False(has) + + s.exec(`INSERT INTO billing_checkouts (customer_id, provider_id, checkout_url, state) + VALUES ($1, 'bc-session', 'bc-session', 'pending')`, id) + has, err = s.repository.HasLiveRecords(s.ctx, id) + s.Require().NoError(err) + s.True(has) + + s.exec(`UPDATE billing_checkouts SET deleted_at = now() WHERE customer_id = $1`, id) + has, err = s.repository.HasLiveRecords(s.ctx, id) + s.Require().NoError(err) + s.False(has, "a deleted child does not count") +} + func (s *BillingCustomerRepositoryPGTestSuite) deleteError(id string) error { _, err := s.repository.Delete(s.ctx, id) return err From 6dc91739d5cbc823da398424d56950fd346c1db2 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Mon, 19 Oct 2026 13:33:33 +0530 Subject: [PATCH 05/10] feat(store): billing account delete records the forfeited tokens on its audit record --- .../store/postgres/billing_customer_repository.go | 5 ++++- .../postgres/billing_customer_repository_pg_test.go | 11 +++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/internal/store/postgres/billing_customer_repository.go b/internal/store/postgres/billing_customer_repository.go index d556639b5f..23aed52eab 100644 --- a/internal/store/postgres/billing_customer_repository.go +++ b/internal/store/postgres/billing_customer_repository.go @@ -579,7 +579,10 @@ func (r BillingCustomerRepository) Delete(ctx context.Context, id string) (custo }, nil, customerModel.OrgID, - nil, + map[string]any{ + "forfeited_tokens": forfeit.Amount, + "purchased_tokens": forfeit.Purchased, + }, deletedAt, ) return InsertAuditRecordInTx(ctx, tx, auditRecord) diff --git a/internal/store/postgres/billing_customer_repository_pg_test.go b/internal/store/postgres/billing_customer_repository_pg_test.go index 1d603dd39e..8252cd8795 100644 --- a/internal/store/postgres/billing_customer_repository_pg_test.go +++ b/internal/store/postgres/billing_customer_repository_pg_test.go @@ -147,6 +147,8 @@ func (s *BillingCustomerRepositoryPGTestSuite) TestDeleteKeepsTheRowAndHidesIt() `SELECT occurred_at::text FROM audit_records WHERE event = $1 AND resource_id = $2`, string(auditrecord.BillingCustomerDeletedEvent), id) s.Equal(deletedAt, occurredAt, "the audit record carries the delete time") + s.Equal("0", s.recordedMetadata(id, "forfeited_tokens"), "an account without transactions forfeits zero") + s.Equal("0", s.recordedMetadata(id, "purchased_tokens")) _, err := s.repository.GetByID(s.ctx, id) s.ErrorIs(err, customer.ErrNotFound) s.ErrorIs(s.deleteError(id), customer.ErrNotFound, "a second delete answers not found") @@ -302,6 +304,13 @@ func (s *BillingCustomerRepositoryPGTestSuite) seedTransaction(account, txType, VALUES ($1, $2, $3, $4, $2) RETURNING id`, account, txType, source, amount) } +func (s *BillingCustomerRepositoryPGTestSuite) recordedMetadata(accountID, key string) string { + s.T().Helper() + return scalarSQL(s.T(), s.ctx, s.client, + `SELECT coalesce(metadata->>$3, '') FROM audit_records WHERE event = $1 AND resource_id = $2`, + string(auditrecord.BillingCustomerDeletedEvent), accountID, key) +} + func (s *BillingCustomerRepositoryPGTestSuite) transactionDeletedAt(id string) string { s.T().Helper() return scalarSQL(s.T(), s.ctx, s.client, @@ -327,6 +336,8 @@ func (s *BillingCustomerRepositoryPGTestSuite) TestDeleteHidesTheAccountsTransac s.NotEmpty(s.transactionDeletedAt(txn)) } s.Equal(hiddenAt, s.transactionDeletedAt(alreadyHidden), "an older delete time is kept") + s.Equal("250", s.recordedMetadata(id, "forfeited_tokens")) + s.Equal("180", s.recordedMetadata(id, "purchased_tokens")) balance, err := s.transactions.GetBalance(s.ctx, id) s.Require().NoError(err) s.Equal(int64(0), balance) From 04889a9ae791b16dcbbb861acc08c617d8209a4d Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Mon, 19 Oct 2026 13:35:13 +0530 Subject: [PATCH 06/10] refactor(billing): drop the forfeit audit log event from the service --- billing/customer/customer.go | 5 -- billing/customer/mocks/repository.go | 26 ++----- billing/customer/service.go | 25 +------ billing/customer/service_test.go | 75 +------------------ .../postgres/billing_customer_repository.go | 30 ++++---- .../billing_customer_repository_pg_test.go | 23 ++---- 6 files changed, 34 insertions(+), 150 deletions(-) diff --git a/billing/customer/customer.go b/billing/customer/customer.go index 77a3c1fc78..43bf88d092 100644 --- a/billing/customer/customer.go +++ b/billing/customer/customer.go @@ -135,8 +135,3 @@ func GetStripeWebhookSignatureFromContext(ctx context.Context) (string, bool) { u, ok := ctx.Value(consts.BillingStripeWebhookSignatureContextKey).(string) return u, ok } - -type ForfeitedTokens struct { - Amount int64 - Purchased int64 -} diff --git a/billing/customer/mocks/repository.go b/billing/customer/mocks/repository.go index 6ffbbe86ae..5348994df2 100644 --- a/billing/customer/mocks/repository.go +++ b/billing/customer/mocks/repository.go @@ -80,31 +80,21 @@ func (_c *Repository_Create_Call) RunAndReturn(run func(context.Context, custome } // Delete provides a mock function with given fields: ctx, id -func (_m *Repository) Delete(ctx context.Context, id string) (customer.ForfeitedTokens, error) { +func (_m *Repository) Delete(ctx context.Context, id string) error { ret := _m.Called(ctx, id) if len(ret) == 0 { panic("no return value specified for Delete") } - var r0 customer.ForfeitedTokens - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string) (customer.ForfeitedTokens, error)); ok { - return rf(ctx, id) - } - if rf, ok := ret.Get(0).(func(context.Context, string) customer.ForfeitedTokens); ok { + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string) error); ok { r0 = rf(ctx, id) } else { - r0 = ret.Get(0).(customer.ForfeitedTokens) + r0 = ret.Error(0) } - if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { - r1 = rf(ctx, id) - } else { - r1 = ret.Error(1) - } - - return r0, r1 + return r0 } // Repository_Delete_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Delete' @@ -126,12 +116,12 @@ func (_c *Repository_Delete_Call) Run(run func(ctx context.Context, id string)) return _c } -func (_c *Repository_Delete_Call) Return(_a0 customer.ForfeitedTokens, _a1 error) *Repository_Delete_Call { - _c.Call.Return(_a0, _a1) +func (_c *Repository_Delete_Call) Return(_a0 error) *Repository_Delete_Call { + _c.Call.Return(_a0) return _c } -func (_c *Repository_Delete_Call) RunAndReturn(run func(context.Context, string) (customer.ForfeitedTokens, error)) *Repository_Delete_Call { +func (_c *Repository_Delete_Call) RunAndReturn(run func(context.Context, string) error) *Repository_Delete_Call { _c.Call.Return(run) return _c } diff --git a/billing/customer/service.go b/billing/customer/service.go index ddf7fbbfb5..4c6b570a6a 100644 --- a/billing/customer/service.go +++ b/billing/customer/service.go @@ -6,11 +6,9 @@ import ( "fmt" "log/slog" "math/rand" - "strconv" "sync" "time" - "github.com/raystack/frontier/core/audit" "github.com/raystack/frontier/pkg/utils" "github.com/robfig/cron/v3" @@ -32,7 +30,7 @@ type Repository interface { List(ctx context.Context, filter Filter) ([]Customer, error) Create(ctx context.Context, customer Customer) (Customer, error) UpdateByID(ctx context.Context, customer Customer) (Customer, error) - Delete(ctx context.Context, id string) (ForfeitedTokens, error) + Delete(ctx context.Context, id string) error HasLiveRecords(ctx context.Context, id string) (bool, error) UpdateCreditMinByID(ctx context.Context, customerID string, limit int64) (Details, error) GetDetailsByID(ctx context.Context, customerID string) (Details, error) @@ -309,26 +307,7 @@ func (s *Service) Delete(ctx context.Context, id string) error { } } - forfeit, err := s.repository.Delete(ctx, id) - if err != nil { - return err - } - if forfeit.Amount > 0 { - s.logTokensForfeited(context.WithoutCancel(ctx), customer, forfeit) - } - return nil -} - -func (s *Service) logTokensForfeited(ctx context.Context, customer Customer, forfeit ForfeitedTokens) { - if err := audit.NewLogger(ctx, customer.OrgID).LogWithAttrs(audit.BillingTokensForfeitedEvent, audit.Target{ - ID: customer.ID, - Type: "billing_account", - }, map[string]string{ - "amount": strconv.FormatInt(forfeit.Amount, 10), - "purchased": strconv.FormatInt(forfeit.Purchased, 10), - }); err != nil { - s.log.WarnContext(ctx, "failed to write audit log", "error", err, "event", audit.BillingTokensForfeitedEvent, "customer_id", customer.ID) - } + return s.repository.Delete(ctx, id) } func (s *Service) ListPaymentMethods(ctx context.Context, id string) ([]PaymentMethod, error) { diff --git a/billing/customer/service_test.go b/billing/customer/service_test.go index ecad3753e2..9315f5e48b 100644 --- a/billing/customer/service_test.go +++ b/billing/customer/service_test.go @@ -14,30 +14,12 @@ import ( "github.com/raystack/frontier/billing/customer" "github.com/raystack/frontier/billing/customer/mocks" stripemock "github.com/raystack/frontier/billing/stripetest/mocks" - "github.com/raystack/frontier/core/audit" "github.com/stripe/stripe-go/v79" "github.com/stripe/stripe-go/v79/client" ) var errSample = errors.New("sample error") -type recordingAuditRepository struct { - audit.NoopRepository - logs []audit.Log -} - -func (r *recordingAuditRepository) Create(ctx context.Context, log *audit.Log) error { - if err := ctx.Err(); err != nil { - return err - } - r.logs = append(r.logs, *log) - return nil -} - -func contextWithAudit(parent context.Context, repository audit.Repository) context.Context { - return audit.SetContextWithService(parent, audit.NewService("test", repository, audit.NewNoopWebhookService())) -} - func mockService(t *testing.T) (*client.API, *stripemock.Backend, *mocks.Repository, *mocks.CreditService) { t.Helper() mockRepository := mocks.NewRepository(t) @@ -544,61 +526,6 @@ func TestService_Delete(t *testing.T) { assert.ErrorIs(t, err, customer.ErrHasTokenDebt) }) - t.Run("a zero forfeit writes no audit record", func(t *testing.T) { - _, _, repo, creditSvc := mockService(t) - auditRepo := &recordingAuditRepository{} - ctx := contextWithAudit(ctx, auditRepo) - repo.EXPECT().GetByID(ctx, "acc-1").Return(account, nil) - creditSvc.EXPECT().GetBalance(ctx, "acc-1").Return(int64(0), nil) - repo.EXPECT().HasLiveRecords(ctx, "acc-1").Return(false, nil) - repo.EXPECT().Delete(ctx, "acc-1").Return(customer.ForfeitedTokens{}, nil) - svc := customer.NewService(slog.Default(), nil, repo, billing.Config{}, creditSvc) - - require.NoError(t, svc.Delete(ctx, "acc-1")) - - assert.Empty(t, auditRepo.logs) - }) - - t.Run("writes the forfeited tokens audit record", func(t *testing.T) { - _, _, repo, creditSvc := mockService(t) - auditRepo := &recordingAuditRepository{} - ctx := contextWithAudit(ctx, auditRepo) - repo.EXPECT().GetByID(ctx, "acc-1").Return(account, nil) - creditSvc.EXPECT().GetBalance(ctx, "acc-1").Return(int64(100), nil) - repo.EXPECT().HasLiveRecords(ctx, "acc-1").Return(false, nil) - repo.EXPECT().Delete(ctx, "acc-1").Return(customer.ForfeitedTokens{Amount: 100, Purchased: 40}, nil) - svc := customer.NewService(slog.Default(), nil, repo, billing.Config{}, creditSvc) - - require.NoError(t, svc.Delete(ctx, "acc-1")) - - require.Len(t, auditRepo.logs, 1) - log := auditRepo.logs[0] - assert.Equal(t, string(audit.BillingTokensForfeitedEvent), log.Action) - assert.Equal(t, "org-1", log.OrgID) - assert.Equal(t, audit.Target{ID: "acc-1", Type: "billing_account"}, log.Target) - assert.Equal(t, "100", log.Metadata["amount"]) - assert.Equal(t, "40", log.Metadata["purchased"]) - }) - - t.Run("writes the record even when the request context is cancelled", func(t *testing.T) { - _, _, repo, creditSvc := mockService(t) - auditRepo := &recordingAuditRepository{} - requestCtx, cancel := context.WithCancel(contextWithAudit(ctx, auditRepo)) - repo.EXPECT().GetByID(mock.Anything, "acc-1").Return(account, nil) - creditSvc.EXPECT().GetBalance(mock.Anything, "acc-1").Return(int64(100), nil) - repo.EXPECT().HasLiveRecords(mock.Anything, "acc-1").Return(false, nil) - repo.EXPECT().Delete(mock.Anything, "acc-1").RunAndReturn( - func(context.Context, string) (customer.ForfeitedTokens, error) { - cancel() - return customer.ForfeitedTokens{Amount: 100, Purchased: 40}, nil - }) - svc := customer.NewService(slog.Default(), nil, repo, billing.Config{}, creditSvc) - - require.NoError(t, svc.Delete(requestCtx, "acc-1")) - - assert.Len(t, auditRepo.logs, 1) - }) - t.Run("refuses before the billing provider is called while live records exist", func(t *testing.T) { stripeClient, _, repo, creditSvc := mockService(t) online := customer.Customer{ID: "acc-1", OrgID: "org-1", ProviderID: "cus_1"} @@ -619,7 +546,7 @@ func TestService_Delete(t *testing.T) { creditSvc.EXPECT().GetBalance(ctx, "acc-1").Return(int64(0), nil) repo.EXPECT().HasLiveRecords(ctx, "acc-1").Return(false, nil) stripeBackend.EXPECT().Call("DELETE", "/v1/customers/cus_1", "key_123", mock.Anything, mock.Anything).Return(nil) - repo.EXPECT().Delete(ctx, "acc-1").Return(customer.ForfeitedTokens{}, nil) + repo.EXPECT().Delete(ctx, "acc-1").Return(nil) svc := customer.NewService(slog.Default(), stripeClient, repo, billing.Config{}, creditSvc) require.NoError(t, svc.Delete(ctx, "acc-1")) diff --git a/internal/store/postgres/billing_customer_repository.go b/internal/store/postgres/billing_customer_repository.go index 23aed52eab..50bc4d96bd 100644 --- a/internal/store/postgres/billing_customer_repository.go +++ b/internal/store/postgres/billing_customer_repository.go @@ -498,24 +498,24 @@ func (r BillingCustomerRepository) UpdateDetailsByID(ctx context.Context, custom }, nil } -func (r BillingCustomerRepository) Delete(ctx context.Context, id string) (customer.ForfeitedTokens, error) { +func (r BillingCustomerRepository) Delete(ctx context.Context, id string) error { lockQuery, lockParams, err := fromLive(TABLE_BILLING_CUSTOMERS). Select("id"). Where(goqu.Ex{"id": id}). ForUpdate(exp.Wait). ToSQL() if err != nil { - return customer.ForfeitedTokens{}, fmt.Errorf("%w: %w", errParse, err) + return fmt.Errorf("%w: %w", errParse, err) } query, params, err := softDelete(TABLE_BILLING_CUSTOMERS). Where(goqu.Ex{"id": id}). Returning(&Customer{}).ToSQL() if err != nil { - return customer.ForfeitedTokens{}, fmt.Errorf("%w: %w", errParse, err) + return fmt.Errorf("%w: %w", errParse, err) } liveRecordsQuery, liveRecordsParams, err := liveBillingRecordsQuery(id) if err != nil { - return customer.ForfeitedTokens{}, fmt.Errorf("%w: %w", errParse, err) + return fmt.Errorf("%w: %w", errParse, err) } hideTransactionsQuery := `WITH hidden AS ( @@ -529,7 +529,7 @@ func (r BillingCustomerRepository) Delete(ctx context.Context, id string) (custo hideTransactionsParams := []any{id, credit.CreditType, credit.SourceSystemBuyEvent} var customerModel Customer - var forfeit customer.ForfeitedTokens + var forfeitedTokens, purchasedTokens int64 if err = r.dbc.WithTxn(ctx, sql.TxOptions{}, func(tx *sqlx.Tx) error { return r.dbc.WithTimeout(ctx, TABLE_BILLING_CUSTOMERS, "Delete", func(ctx context.Context) error { var lockedID string @@ -545,13 +545,13 @@ func (r BillingCustomerRepository) Delete(ctx context.Context, id string) (custo return customer.ErrHasLiveRecords } var bought int64 - if err := tx.QueryRowxContext(ctx, hideTransactionsQuery, hideTransactionsParams...).Scan(&forfeit.Amount, &bought); err != nil { + if err := tx.QueryRowxContext(ctx, hideTransactionsQuery, hideTransactionsParams...).Scan(&forfeitedTokens, &bought); err != nil { return err } - if forfeit.Amount < 0 { + if forfeitedTokens < 0 { return customer.ErrHasTokenDebt } - forfeit.Purchased = min(max(bought, 0), forfeit.Amount) + purchasedTokens = min(max(bought, 0), forfeitedTokens) if err := tx.QueryRowxContext(ctx, query, params...).StructScan(&customerModel); err != nil { if errors.Is(err, sql.ErrNoRows) { return customer.ErrNotFound @@ -580,8 +580,8 @@ func (r BillingCustomerRepository) Delete(ctx context.Context, id string) (custo nil, customerModel.OrgID, map[string]any{ - "forfeited_tokens": forfeit.Amount, - "purchased_tokens": forfeit.Purchased, + "forfeited_tokens": forfeitedTokens, + "purchased_tokens": purchasedTokens, }, deletedAt, ) @@ -591,17 +591,17 @@ func (r BillingCustomerRepository) Delete(ctx context.Context, id string) (custo err = checkPostgresError(err) switch { case errors.Is(err, customer.ErrNotFound), errors.Is(err, sql.ErrNoRows): - return customer.ForfeitedTokens{}, customer.ErrNotFound + return customer.ErrNotFound case errors.Is(err, customer.ErrHasLiveRecords): - return customer.ForfeitedTokens{}, customer.ErrHasLiveRecords + return customer.ErrHasLiveRecords case errors.Is(err, customer.ErrHasTokenDebt): - return customer.ForfeitedTokens{}, customer.ErrHasTokenDebt + return customer.ErrHasTokenDebt default: - return customer.ForfeitedTokens{}, fmt.Errorf("%w: %w", errTxn, err) + return fmt.Errorf("%w: %w", errTxn, err) } } - return forfeit, nil + return nil } func (r BillingCustomerRepository) HasLiveRecords(ctx context.Context, id string) (bool, error) { diff --git a/internal/store/postgres/billing_customer_repository_pg_test.go b/internal/store/postgres/billing_customer_repository_pg_test.go index 8252cd8795..73ad43da52 100644 --- a/internal/store/postgres/billing_customer_repository_pg_test.go +++ b/internal/store/postgres/billing_customer_repository_pg_test.go @@ -218,17 +218,16 @@ func (s *BillingCustomerRepositoryPGTestSuite) deletedRows(table, column, id str fmt.Sprintf(`SELECT count(*)::text FROM %s WHERE %s = $1 AND deleted_at IS NOT NULL`, table, column), id) } -func (s *BillingCustomerRepositoryPGTestSuite) TestTeardownInTheDeleterOrderKeepsEveryRow() { +func (s *BillingCustomerRepositoryPGTestSuite) TestTeardownInTheDeleterOrderForfeitsTheTokens() { cust := s.customerID("bc-cust-live") subID, invID := s.seedChildRows(cust) s.Require().NoError(s.subscriptions.Delete(s.ctx, subID)) s.Require().NoError(s.invoices.Delete(s.ctx, invID)) s.Require().NoError(s.checkouts.DeleteByCustomerID(s.ctx, cust)) - s.Require().NoError(s.transactions.DeleteByAccountID(s.ctx, cust)) - forfeit, err := s.repository.Delete(s.ctx, cust) - s.Require().NoError(err) - s.Equal(customer.ForfeitedTokens{}, forfeit, "the deleter hid the transactions already, so the account delete forfeits nothing a second time") + s.Require().NoError(s.repository.Delete(s.ctx, cust)) + s.Equal("70", s.recordedMetadata(cust, "forfeited_tokens"), "the customer delete hides the transactions and records what they held") + s.Equal("70", s.recordedMetadata(cust, "purchased_tokens")) s.Equal("1", s.deletedRows(postgres.TABLE_BILLING_SUBSCRIPTIONS, "customer_id", cust)) s.Equal("1", s.deletedRows(postgres.TABLE_BILLING_INVOICES, "customer_id", cust)) @@ -266,8 +265,6 @@ func (s *BillingCustomerRepositoryPGTestSuite) TestTeardownCanBeRunAgain() { s.Require().NoError(s.subscriptions.Delete(s.ctx, subID)) s.Require().NoError(s.invoices.Delete(s.ctx, invID)) s.Require().NoError(s.checkouts.DeleteByCustomerID(s.ctx, cust)) - s.Require().NoError(s.transactions.DeleteByAccountID(s.ctx, cust)) - s.Require().NoError(s.transactions.DeleteByAccountID(s.ctx, cust)) s.Require().NoError(s.deleteError(cust)) s.ErrorIs(s.deleteError(cust), customer.ErrNotFound) } @@ -327,17 +324,14 @@ func (s *BillingCustomerRepositoryPGTestSuite) TestDeleteHidesTheAccountsTransac s.exec(`UPDATE billing_transactions SET deleted_at = now() - interval '1 day' WHERE id = $1`, alreadyHidden) hiddenAt := s.transactionDeletedAt(alreadyHidden) - forfeit, err := s.repository.Delete(s.ctx, id) - s.Require().NoError(err) + s.Require().NoError(s.repository.Delete(s.ctx, id)) - s.Equal(customer.ForfeitedTokens{Amount: 250, Purchased: 180}, forfeit, - "the live credits minus the live debits, and the share of it bought net of refunds") for _, txn := range []string{bought, refunded, awarded, used} { s.NotEmpty(s.transactionDeletedAt(txn)) } s.Equal(hiddenAt, s.transactionDeletedAt(alreadyHidden), "an older delete time is kept") - s.Equal("250", s.recordedMetadata(id, "forfeited_tokens")) - s.Equal("180", s.recordedMetadata(id, "purchased_tokens")) + s.Equal("250", s.recordedMetadata(id, "forfeited_tokens"), "the live credits minus the live debits") + s.Equal("180", s.recordedMetadata(id, "purchased_tokens"), "the share of it bought, net of refunds") balance, err := s.transactions.GetBalance(s.ctx, id) s.Require().NoError(err) s.Equal(int64(0), balance) @@ -387,8 +381,7 @@ func (s *BillingCustomerRepositoryPGTestSuite) TestHasLiveRecordsSeesOnlyLiveChi } func (s *BillingCustomerRepositoryPGTestSuite) deleteError(id string) error { - _, err := s.repository.Delete(s.ctx, id) - return err + return s.repository.Delete(s.ctx, id) } func (s *BillingCustomerRepositoryPGTestSuite) deleteAccount(id string) { From 279ef03458e5cdcf3917b106177de5fd6ebe2d19 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Mon, 19 Oct 2026 13:38:30 +0530 Subject: [PATCH 07/10] refactor(deleter): org delete leaves the forfeit record to the billing account delete --- cmd/serve.go | 2 +- core/deleter/delete_notice.go | 63 ++++--------- core/deleter/delete_notice_test.go | 63 +++++++++++++ core/deleter/mocks/audit_record_service.go | 95 +++++++++++++++++++ core/deleter/mocks/credit_service.go | 47 ---------- core/deleter/service.go | 52 +++-------- core/deleter/service_test.go | 101 +++++++++------------ 7 files changed, 235 insertions(+), 188 deletions(-) create mode 100644 core/deleter/delete_notice_test.go create mode 100644 core/deleter/mocks/audit_record_service.go diff --git a/cmd/serve.go b/cmd/serve.go index 59463a558e..6cb4e5abe7 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -601,7 +601,7 @@ func buildAPIDependencies( cascadeDeleter := deleter.NewCascadeDeleter(organizationService, projectService, resourceService, groupService, membershipService, policyService, roleService, invitationService, userService, userPATService, domainService, serviceUserService, customerService, subscriptionService, invoiceService, checkoutService, - creditService, orgKycService, planService, + creditService, orgKycService, planService, auditRecordService, mailDialer, cfg.Billing.OrgDeleteNotice, cfg.App.Deletion, ) diff --git a/core/deleter/delete_notice.go b/core/deleter/delete_notice.go index 6b08223244..e34472d53b 100644 --- a/core/deleter/delete_notice.go +++ b/core/deleter/delete_notice.go @@ -6,21 +6,24 @@ import ( "fmt" htmltemplate "html/template" "log/slog" - "strconv" texttemplate "text/template" "github.com/raystack/frontier/billing/credit" "github.com/raystack/frontier/billing/customer" - "github.com/raystack/frontier/core/audit" "github.com/raystack/frontier/core/authenticate" "github.com/raystack/frontier/core/membership" "github.com/raystack/frontier/core/organization" "github.com/raystack/frontier/core/user" "github.com/raystack/frontier/internal/bootstrap/schema" + "github.com/raystack/frontier/pkg/auditrecord" + "github.com/raystack/frontier/pkg/utils" + "github.com/raystack/salt/rql" "gopkg.in/mail.v2" ) +const forfeitRecordsLimit = 1000 + // plain fallbacks used when the config leaves the templates empty const ( defaultDeleteNoticeSubject = "Your organization was deleted" @@ -50,8 +53,7 @@ type accountTokens struct { // deleteNotice is everything sendDeleteNotices needs once the org is gone. // It has to be collected before teardown removes the owners and the token -// balances. Accounts keeps the per-account numbers so the teardown can audit -// them without reading the balances a second time. +// balances. Accounts keeps the per-account numbers of the live accounts. type deleteNotice struct { Amount int64 Purchased int64 @@ -128,26 +130,22 @@ func (d Service) resolveOwners(ctx context.Context, orgID string) []user.User { return owners } -// recoverForfeitFromAudit adds the forfeits a failed earlier teardown wrote -// audit records for, so the retry that completes the delete still reports -// them in the owner notice. It reconciles per billing account: only the -// newest record per account counts (a retried teardown can write the same -// forfeit twice), and an account that still holds a live balance is already -// counted by the collection pass, so its records are skipped. Best-effort: -// without a readable audit store the notice keeps only the live amounts. +// recoverForfeitFromAudit adds the forfeits that billing_customer.deleted +// 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) { - logs, err := audit.GetService(ctx).List(ctx, audit.Filter{ - OrgID: orgID, - Action: string(audit.BillingTokensForfeitedEvent), - }) + 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)) if err != nil { slog.WarnContext(ctx, "failed to check audit records for forfeited tokens", "org_id", orgID, "error", err) return } - // the list is newest first, so the first record per account wins seen := map[string]struct{}{} - for _, l := range logs { - accountID := l.Target.ID + for _, record := range records.AuditRecords { + accountID := record.Resource.ID if accountID == "" { continue } @@ -158,32 +156,11 @@ func (d Service) recoverForfeitFromAudit(ctx context.Context, orgID string, noti if _, live := notice.Accounts[accountID]; live { continue } - amount, _ := strconv.ParseInt(l.Metadata["amount"], 10, 64) - purchased, _ := strconv.ParseInt(l.Metadata["purchased"], 10, 64) - notice.Amount += amount - notice.Purchased += purchased - } -} - -// resolveAccountTokens returns one account's balance and purchased share, -// from the caller's already-collected amounts when given, otherwise read -// fresh. -func (d Service) resolveAccountTokens(ctx context.Context, accountID string, amounts map[string]accountTokens) (accountTokens, error) { - if amounts != nil { - return amounts[accountID], nil - } - balance, err := d.creditService.GetBalance(ctx, accountID) - if err != nil { - return accountTokens{}, err - } - if balance <= 0 { - return accountTokens{Balance: balance}, nil - } - bought, err := d.purchasedTokens(ctx, accountID, balance) - if err != nil { - return accountTokens{}, err + forfeited, _ := record.Metadata["forfeited_tokens"].(float64) + purchased, _ := record.Metadata["purchased_tokens"].(float64) + notice.Amount += int64(forfeited) + notice.Purchased += int64(purchased) } - return accountTokens{Balance: balance, Purchased: bought}, nil } // purchasedTokens returns how many of the account's remaining tokens came diff --git a/core/deleter/delete_notice_test.go b/core/deleter/delete_notice_test.go new file mode 100644 index 0000000000..ec58b42edc --- /dev/null +++ b/core/deleter/delete_notice_test.go @@ -0,0 +1,63 @@ +package deleter + +import ( + "context" + "errors" + "testing" + + "github.com/raystack/frontier/core/auditrecord" + "github.com/raystack/frontier/core/deleter/mocks" + pkgauditrecord "github.com/raystack/frontier/pkg/auditrecord" + "github.com/raystack/frontier/pkg/metadata" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" +) + +func deletedRecord(accountID string, meta metadata.Metadata) auditrecord.AuditRecord { + return auditrecord.AuditRecord{ + Event: pkgauditrecord.BillingCustomerDeletedEvent, + Resource: auditrecord.Resource{ID: accountID, Type: pkgauditrecord.BillingCustomerType}, + OrgID: "org-1", + Metadata: meta, + } +} + +func recoverWith(t *testing.T, records []auditrecord.AuditRecord, listErr error, notice deleteNotice) deleteNotice { + t.Helper() + auditSvc := mocks.NewAuditRecordService(t) + auditSvc.EXPECT().List(mock.Anything, mock.Anything). + Return(auditrecord.AuditRecordsList{AuditRecords: records}, listErr) + Service{auditRecordService: auditSvc}.recoverForfeitFromAudit(context.Background(), "org-1", ¬ice) + return notice +} + +func TestRecoverForfeitFromAudit(t *testing.T) { + forfeit := metadata.Metadata{"forfeited_tokens": float64(70), "purchased_tokens": float64(30)} + + t.Run("adds the forfeit of an account that is gone", func(t *testing.T) { + got := recoverWith(t, []auditrecord.AuditRecord{deletedRecord("cust-1", forfeit)}, nil, deleteNotice{}) + assert.Equal(t, int64(70), got.Amount) + assert.Equal(t, int64(30), got.Purchased) + }) + + t.Run("skips an account that is still live", func(t *testing.T) { + live := deleteNotice{ + Amount: 100, + Purchased: 100, + Accounts: map[string]accountTokens{"cust-1": {Balance: 100, Purchased: 100}}, + } + got := recoverWith(t, []auditrecord.AuditRecord{deletedRecord("cust-1", forfeit)}, nil, live) + assert.Equal(t, live, got) + }) + + t.Run("leaves the notice unchanged when the audit records cannot be listed", func(t *testing.T) { + start := deleteNotice{Amount: 5, Purchased: 2} + got := recoverWith(t, []auditrecord.AuditRecord{deletedRecord("cust-1", forfeit)}, errors.New("db down"), start) + assert.Equal(t, start, got) + }) + + t.Run("ignores a record with no resource id", func(t *testing.T) { + got := recoverWith(t, []auditrecord.AuditRecord{deletedRecord("", forfeit)}, nil, deleteNotice{}) + assert.Equal(t, deleteNotice{}, got) + }) +} diff --git a/core/deleter/mocks/audit_record_service.go b/core/deleter/mocks/audit_record_service.go new file mode 100644 index 0000000000..69ced04724 --- /dev/null +++ b/core/deleter/mocks/audit_record_service.go @@ -0,0 +1,95 @@ +// Code generated by mockery v2.53.5. DO NOT EDIT. + +package mocks + +import ( + context "context" + + models "github.com/raystack/frontier/core/auditrecord/models" + rql "github.com/raystack/salt/rql" + mock "github.com/stretchr/testify/mock" +) + +// AuditRecordService is an autogenerated mock type for the AuditRecordService type +type AuditRecordService struct { + mock.Mock +} + +type AuditRecordService_Expecter struct { + mock *mock.Mock +} + +func (_m *AuditRecordService) EXPECT() *AuditRecordService_Expecter { + return &AuditRecordService_Expecter{mock: &_m.Mock} +} + +// List provides a mock function with given fields: ctx, query +func (_m *AuditRecordService) List(ctx context.Context, query *rql.Query) (models.AuditRecordsList, error) { + ret := _m.Called(ctx, query) + + if len(ret) == 0 { + panic("no return value specified for List") + } + + var r0 models.AuditRecordsList + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, *rql.Query) (models.AuditRecordsList, error)); ok { + return rf(ctx, query) + } + if rf, ok := ret.Get(0).(func(context.Context, *rql.Query) models.AuditRecordsList); ok { + r0 = rf(ctx, query) + } else { + r0 = ret.Get(0).(models.AuditRecordsList) + } + + if rf, ok := ret.Get(1).(func(context.Context, *rql.Query) error); ok { + r1 = rf(ctx, query) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// AuditRecordService_List_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'List' +type AuditRecordService_List_Call struct { + *mock.Call +} + +// List is a helper method to define mock.On call +// - ctx context.Context +// - query *rql.Query +func (_e *AuditRecordService_Expecter) List(ctx interface{}, query interface{}) *AuditRecordService_List_Call { + return &AuditRecordService_List_Call{Call: _e.mock.On("List", ctx, query)} +} + +func (_c *AuditRecordService_List_Call) Run(run func(ctx context.Context, query *rql.Query)) *AuditRecordService_List_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(*rql.Query)) + }) + return _c +} + +func (_c *AuditRecordService_List_Call) Return(_a0 models.AuditRecordsList, _a1 error) *AuditRecordService_List_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *AuditRecordService_List_Call) RunAndReturn(run func(context.Context, *rql.Query) (models.AuditRecordsList, error)) *AuditRecordService_List_Call { + _c.Call.Return(run) + return _c +} + +// NewAuditRecordService creates a new instance of AuditRecordService. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewAuditRecordService(t interface { + mock.TestingT + Cleanup(func()) +}) *AuditRecordService { + mock := &AuditRecordService{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/core/deleter/mocks/credit_service.go b/core/deleter/mocks/credit_service.go index 2923863096..c20a0ad2ab 100644 --- a/core/deleter/mocks/credit_service.go +++ b/core/deleter/mocks/credit_service.go @@ -23,53 +23,6 @@ func (_m *CreditService) EXPECT() *CreditService_Expecter { return &CreditService_Expecter{mock: &_m.Mock} } -// DeleteByAccountID provides a mock function with given fields: ctx, accountID -func (_m *CreditService) DeleteByAccountID(ctx context.Context, accountID string) error { - ret := _m.Called(ctx, accountID) - - if len(ret) == 0 { - panic("no return value specified for DeleteByAccountID") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string) error); ok { - r0 = rf(ctx, accountID) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// CreditService_DeleteByAccountID_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeleteByAccountID' -type CreditService_DeleteByAccountID_Call struct { - *mock.Call -} - -// DeleteByAccountID is a helper method to define mock.On call -// - ctx context.Context -// - accountID string -func (_e *CreditService_Expecter) DeleteByAccountID(ctx interface{}, accountID interface{}) *CreditService_DeleteByAccountID_Call { - return &CreditService_DeleteByAccountID_Call{Call: _e.mock.On("DeleteByAccountID", ctx, accountID)} -} - -func (_c *CreditService_DeleteByAccountID_Call) Run(run func(ctx context.Context, accountID string)) *CreditService_DeleteByAccountID_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string)) - }) - return _c -} - -func (_c *CreditService_DeleteByAccountID_Call) Return(_a0 error) *CreditService_DeleteByAccountID_Call { - _c.Call.Return(_a0) - return _c -} - -func (_c *CreditService_DeleteByAccountID_Call) RunAndReturn(run func(context.Context, string) error) *CreditService_DeleteByAccountID_Call { - _c.Call.Return(run) - return _c -} - // GetBalance provides a mock function with given fields: ctx, accountID func (_m *CreditService) GetBalance(ctx context.Context, accountID string) (int64, error) { ret := _m.Called(ctx, accountID) diff --git a/core/deleter/service.go b/core/deleter/service.go index 668252f0f3..8924aff130 100644 --- a/core/deleter/service.go +++ b/core/deleter/service.go @@ -5,9 +5,9 @@ import ( "errors" "fmt" "log/slog" - "strconv" "github.com/raystack/frontier/core/audit" + "github.com/raystack/frontier/core/auditrecord" "github.com/raystack/frontier/core/authenticate" "github.com/raystack/frontier/core/domain" @@ -44,6 +44,7 @@ import ( "github.com/raystack/frontier/core/serviceuser" "github.com/raystack/frontier/core/user" "github.com/raystack/frontier/pkg/mailer" + "github.com/raystack/salt/rql" ) type ProjectService interface { @@ -135,7 +136,10 @@ type CheckoutService interface { 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 +} + +type AuditRecordService interface { + List(ctx context.Context, query *rql.Query) (auditrecord.AuditRecordsList, error) } type KycService interface { @@ -166,6 +170,7 @@ type Service struct { creditService CreditService kycService KycService planService PlanService + auditRecordService AuditRecordService // mailDialer and deleteNoticeConfig drive the email that tells the org // owners their organization was deleted mailDialer mailer.Dialer @@ -183,7 +188,7 @@ func NewCascadeDeleter(orgService OrganizationService, projService ProjectServic customerService CustomerService, subService SubscriptionService, invoiceService InvoiceService, checkoutService CheckoutService, creditService CreditService, kycService KycService, - planService PlanService, + planService PlanService, auditRecordService AuditRecordService, mailDialer mailer.Dialer, deleteNoticeConfig billing.OrgDeleteNoticeConfig, deletionConfig Config) *Service { return &Service{ @@ -206,6 +211,7 @@ func NewCascadeDeleter(orgService OrganizationService, projService ProjectServic creditService: creditService, kycService: kycService, planService: planService, + auditRecordService: auditRecordService, mailDialer: mailDialer, deleteNoticeConfig: deleteNoticeConfig, deletionConfig: deletionConfig, @@ -281,7 +287,7 @@ func (d Service) DeleteOrganization(ctx context.Context, id string) error { // the delete notice reads the token balances, so it has to be collected // while they still exist; its balance reads are reused by the blocker - // check and the teardown audit below + // check below notice, err := d.collectDeleteNotice(ctx, customers) if err != nil { return err @@ -302,7 +308,7 @@ func (d Service) DeleteOrganization(ctx context.Context, id string) error { notice.Owners = d.resolveOwners(ctx, id) // delete all billing accounts - if err := d.deleteCustomers(ctx, id, customers, notice.Accounts); err != nil { + if err := d.deleteCustomers(ctx, id, customers); err != nil { return err } @@ -427,9 +433,6 @@ func (d Service) DeleteOrganization(ctx context.Context, id string) error { return nil } -// DeleteCustomers lists the accounts and reads the balances itself; -// DeleteOrganization goes through deleteCustomers with what it already -// collected. func (d Service) DeleteCustomers(ctx context.Context, id string) error { customers, err := d.customerService.List(ctx, customer.Filter{ OrgID: id, @@ -437,13 +440,12 @@ func (d Service) DeleteCustomers(ctx context.Context, id string) error { if err != nil { return err } - return d.deleteCustomers(ctx, id, customers, nil) + return d.deleteCustomers(ctx, id, customers) } -// deleteCustomers tears down the org's billing accounts. amounts carries the -// per-account token numbers the caller already read; nil means read them -// here. -func (d Service) deleteCustomers(ctx context.Context, id string, customers []customer.Customer, amounts map[string]accountTokens) error { +// deleteCustomers tears down the org's billing accounts. The account delete +// hides the token transactions and records the forfeited tokens itself. +func (d Service) deleteCustomers(ctx context.Context, id string, customers []customer.Customer) error { for _, c := range customers { // cancels active subscriptions on the billing provider and hides the local rows if err := d.subService.DeleteByCustomer(ctx, c); err != nil { @@ -485,30 +487,6 @@ func (d Service) deleteCustomers(ctx context.Context, id string, customers []cus slog.WarnContext(ctx, "failed to write audit log", "error", err, "event", audit.BillingCheckoutDeletedEvent, "checkout_id", ch.ID) } } - // tokens still on the account are forfeited by this delete, so - // record the amount before the transactions are removed. The - // purchased share goes on the record too: the transaction rows are - // hidden right after, and support settles a transfer from this - // number later - account, err := d.resolveAccountTokens(ctx, c.ID, amounts) - if err != nil { - return fmt.Errorf("failed to delete org while checking balance of billing account[%s]: %w", c.ID, err) - } - if account.Balance > 0 { - if err := auditLogger.LogWithAttrs(audit.BillingTokensForfeitedEvent, audit.Target{ - ID: c.ID, - Type: "billing_account", - }, map[string]string{ - "amount": strconv.FormatInt(account.Balance, 10), - "purchased": strconv.FormatInt(account.Purchased, 10), - }); err != nil { - slog.WarnContext(ctx, "failed to write audit log", "error", err, "event", audit.BillingTokensForfeitedEvent, "customer_id", c.ID) - } - } - if err := d.creditService.DeleteByAccountID(ctx, c.ID); err != nil { - return fmt.Errorf("failed to delete org while deleting a billing account transactions[%s]: %w", c.ID, err) - } - // delete customer if err = d.customerService.Delete(ctx, c.ID); err != nil { return fmt.Errorf("failed to delete org while deleting a billing account[%s]: %w", c.ID, err) diff --git a/core/deleter/service_test.go b/core/deleter/service_test.go index b6514c2d2f..f222f6d40e 100644 --- a/core/deleter/service_test.go +++ b/core/deleter/service_test.go @@ -18,6 +18,7 @@ import ( "github.com/raystack/frontier/billing/product" "github.com/raystack/frontier/billing/subscription" "github.com/raystack/frontier/core/audit" + "github.com/raystack/frontier/core/auditrecord" "github.com/raystack/frontier/core/deleter" "github.com/raystack/frontier/core/deleter/mocks" "github.com/raystack/frontier/core/domain" @@ -33,7 +34,10 @@ import ( "github.com/raystack/frontier/core/user" "github.com/raystack/frontier/core/webhook" "github.com/raystack/frontier/internal/bootstrap/schema" + pkgauditrecord "github.com/raystack/frontier/pkg/auditrecord" mailermocks "github.com/raystack/frontier/pkg/mailer/mocks" + "github.com/raystack/frontier/pkg/metadata" + "github.com/raystack/salt/rql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "gopkg.in/mail.v2" @@ -57,6 +61,7 @@ type deleterMocks struct { invocSvc *mocks.InvoiceService checkoutSvc *mocks.CheckoutService creditSvc *mocks.CreditService + auditRecSvc *mocks.AuditRecordService kycSvc *mocks.KycService planSvc *mocks.PlanService dialer *mailermocks.Dialer @@ -89,6 +94,7 @@ func newBareMocks(t *testing.T) deleterMocks { invocSvc: mocks.NewInvoiceService(t), checkoutSvc: mocks.NewCheckoutService(t), creditSvc: mocks.NewCreditService(t), + auditRecSvc: mocks.NewAuditRecordService(t), kycSvc: mocks.NewKycService(t), planSvc: mocks.NewPlanService(t), dialer: mailermocks.NewDialer(t), @@ -101,14 +107,20 @@ func newBareMocks(t *testing.T) deleterMocks { }}, nil).Maybe() m.planSvc.EXPECT().GetByID(mock.Anything, "plan-free"). Return(plan.Plan{ID: "plan-free"}, nil).Maybe() + m.auditRecSvc.EXPECT().List(mock.Anything, mock.Anything). + Return(auditrecord.AuditRecordsList{}, nil).Maybe() return m } func (m deleterMocks) build() *deleter.Service { + return m.buildWithNotice(billing.OrgDeleteNoticeConfig{}) +} + +func (m deleterMocks) buildWithNotice(notice billing.OrgDeleteNoticeConfig) *deleter.Service { return deleter.NewCascadeDeleter(m.orgSvc, m.projSvc, m.resSvc, m.grpSvc, m.mbrSvc, m.polSvc, m.roleSvc, m.invSvc, m.usrSvc, m.patSvc, m.domainSvc, m.suSvc, m.custSvc, m.subSvc, m.invocSvc, m.checkoutSvc, m.creditSvc, m.kycSvc, - m.planSvc, m.dialer, billing.OrgDeleteNoticeConfig{}, m.deletion) + m.planSvc, m.auditRecSvc, m.dialer, notice, m.deletion) } func TestDeleteProject(t *testing.T) { @@ -222,7 +234,6 @@ func TestDeleteOrganization(t *testing.T) { m.checkoutSvc.EXPECT().List(mock.Anything, checkout.Filter{CustomerID: "cust-1"}). Return([]checkout.Checkout{{ID: "chk-1", ProviderID: "cs_1", CustomerID: "cust-1"}}, nil) m.checkoutSvc.EXPECT().DeleteByCustomer(mock.Anything, "cust-1").Return(nil) - m.creditSvc.EXPECT().DeleteByAccountID(mock.Anything, "cust-1").Return(nil) m.custSvc.EXPECT().Delete(mock.Anything, "cust-1").Return(nil) // projects (each triggers DeleteProject) @@ -585,7 +596,6 @@ func TestDeleteOrganization(t *testing.T) { m.checkoutSvc.EXPECT().List(mock.Anything, checkout.Filter{CustomerID: "cust-1"}). Return([]checkout.Checkout{}, nil) m.checkoutSvc.EXPECT().DeleteByCustomer(mock.Anything, "cust-1").Return(nil) - m.creditSvc.EXPECT().DeleteByAccountID(mock.Anything, "cust-1").Return(nil) m.custSvc.EXPECT().Delete(mock.Anything, "cust-1").Return(nil) m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). @@ -637,7 +647,6 @@ func TestDeleteOrganization(t *testing.T) { m.checkoutSvc.EXPECT().List(mock.Anything, checkout.Filter{CustomerID: "cust-1"}). Return([]checkout.Checkout{}, nil) m.checkoutSvc.EXPECT().DeleteByCustomer(mock.Anything, "cust-1").Return(nil) - m.creditSvc.EXPECT().DeleteByAccountID(mock.Anything, "cust-1").Return(nil) m.custSvc.EXPECT().Delete(mock.Anything, "cust-1").Return(nil) m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). @@ -743,7 +752,6 @@ func TestDeleteOrganization(t *testing.T) { m.checkoutSvc.EXPECT().List(mock.Anything, checkout.Filter{CustomerID: "cust-1"}). Return([]checkout.Checkout{}, nil) m.checkoutSvc.EXPECT().DeleteByCustomer(mock.Anything, "cust-1").Return(nil) - m.creditSvc.EXPECT().DeleteByAccountID(mock.Anything, "cust-1").Return(nil) m.custSvc.EXPECT().Delete(mock.Anything, "cust-1").Return(nil) m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). @@ -1229,7 +1237,7 @@ func auditContext(repo *fakeAuditRepository) context.Context { } func TestForfeitAuditRecord(t *testing.T) { - t.Run("the forfeited amounts land in the audit record", func(t *testing.T) { + t.Run("org delete leaves the forfeit record to the billing account delete", func(t *testing.T) { m := newMocks(t) repo := &fakeAuditRepository{} @@ -1241,7 +1249,6 @@ func TestForfeitAuditRecord(t *testing.T) { m.invocSvc.EXPECT().ListPayableOnProvider(mock.Anything, c). Return([]invoice.Invoice{}, nil) m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(100, nil) - // 60 bought, 20 of those refunded: the record must say purchased 40 m.creditSvc.EXPECT().List(mock.Anything, credit.Filter{CustomerID: "cust-1"}). Return([]credit.Transaction{ {Type: credit.CreditType, Source: credit.SourceSystemBuyEvent, Amount: 60}, @@ -1255,7 +1262,6 @@ func TestForfeitAuditRecord(t *testing.T) { m.checkoutSvc.EXPECT().List(mock.Anything, checkout.Filter{CustomerID: "cust-1"}). Return([]checkout.Checkout{}, nil) m.checkoutSvc.EXPECT().DeleteByCustomer(mock.Anything, "cust-1").Return(nil) - m.creditSvc.EXPECT().DeleteByAccountID(mock.Anything, "cust-1").Return(nil) m.custSvc.EXPECT().Delete(mock.Anything, "cust-1").Return(nil) m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). Return([]project.Project{}, nil) @@ -1280,31 +1286,34 @@ func TestForfeitAuditRecord(t *testing.T) { err := m.build().DeleteOrganization(auditContext(repo), "org-1") assert.NoError(t, err) - - forfeits, listErr := repo.List(context.Background(), audit.Filter{ - OrgID: "org-1", - Action: string(audit.BillingTokensForfeitedEvent), - }) - assert.NoError(t, listErr) - assert.Len(t, forfeits, 1) - assert.Equal(t, "cust-1", forfeits[0].Target.ID) - assert.Equal(t, "100", forfeits[0].Metadata["amount"]) - assert.Equal(t, "40", forfeits[0].Metadata["purchased"]) + assert.Len(t, repo.logs, 1) + assert.Equal(t, string(audit.OrgDeletedEvent), repo.logs[0].Action) }) - t.Run("a retry recovers the amounts from the audit records", func(t *testing.T) { + t.Run("a retry recovers the amounts from the billing customer deleted records", func(t *testing.T) { m := newMocks(t) - // the earlier attempt tore down billing and wrote the record before - // failing; this retry finds no billing accounts at all - repo := &fakeAuditRepository{logs: []audit.Log{{ - OrgID: "org-1", - Action: string(audit.BillingTokensForfeitedEvent), - Target: audit.Target{ID: "cust-1", Type: "billing_account"}, - Metadata: map[string]string{ - "amount": "750", - "purchased": "200", - }, - }}} + m.auditRecSvc = mocks.NewAuditRecordService(t) + repo := &fakeAuditRepository{} + forfeitRecord := func(accountID string, forfeited, purchased float64) auditrecord.AuditRecord { + return auditrecord.AuditRecord{ + Event: pkgauditrecord.BillingCustomerDeletedEvent, + Resource: auditrecord.Resource{ID: accountID, Type: pkgauditrecord.BillingCustomerType}, + OrgID: "org-1", + Metadata: metadata.Metadata{"forfeited_tokens": forfeited, "purchased_tokens": purchased}, + } + } + m.auditRecSvc.EXPECT().List(mock.Anything, mock.MatchedBy(func(q *rql.Query) bool { + return q.Limit > 0 && + assert.ObjectsAreEqual([]rql.Filter{ + {Name: "event", Operator: "eq", Value: pkgauditrecord.BillingCustomerDeletedEvent.String()}, + {Name: "org_id", Operator: "eq", Value: "org-1"}, + }, q.Filters) && + assert.ObjectsAreEqual([]rql.Sort{{Name: "occurred_at", Order: "desc"}}, q.Sort) + })).Return(auditrecord.AuditRecordsList{AuditRecords: []auditrecord.AuditRecord{ + forfeitRecord("cust-1", 750, 200), + forfeitRecord("cust-1", 10, 10), + forfeitRecord("cust-2", 50, 0), + }}, nil) m.orgSvc.EXPECT().GetRaw(mock.Anything, "org-1"). Return(organization.Organization{ID: "org-1", Title: "Org One"}, nil) @@ -1344,11 +1353,10 @@ func TestForfeitAuditRecord(t *testing.T) { _, err := msg.WriteTo(&raw) assert.NoError(t, err) body := strings.ReplaceAll(raw.String(), "=\r\n", "") - // the recovered purchased share puts the settlement line back - assert.Contains(t, body, "Unused purchased tokens will be settled by the support team") + assert.Contains(t, body, "amount 800 purchased 200") }).Return(nil) - err := m.build().DeleteOrganization(auditContext(repo), "org-1") + err := m.buildWithNotice(billing.OrgDeleteNoticeConfig{Body: "amount {{.Amount}} purchased {{.Purchased}}"}).DeleteOrganization(auditContext(repo), "org-1") assert.NoError(t, err) }) } @@ -1368,37 +1376,12 @@ func TestDeleteCustomers(t *testing.T) { {ID: "chk-2", ProviderID: "cs_2", CustomerID: "cust-1", State: "expired"}, }, nil) m.checkoutSvc.EXPECT().DeleteByCustomer(mock.Anything, "cust-1").Return(nil) - m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(100, nil) - m.creditSvc.EXPECT().List(mock.Anything, credit.Filter{CustomerID: "cust-1"}). - Return([]credit.Transaction{ - {Type: credit.CreditType, Source: credit.SourceSystemBuyEvent, Amount: 40}, - }, nil) - m.creditSvc.EXPECT().DeleteByAccountID(mock.Anything, "cust-1").Return(nil) m.custSvc.EXPECT().Delete(mock.Anything, "cust-1").Return(nil) err := m.build().DeleteCustomers(context.Background(), "org-1") assert.NoError(t, err) }) - t.Run("balance check failure stops the customer delete", func(t *testing.T) { - m := newMocks(t) - - c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"} - m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). - Return([]customer.Customer{c}, nil) - m.subSvc.EXPECT().DeleteByCustomer(mock.Anything, c).Return(nil) - m.invocSvc.EXPECT().DeleteByCustomer(mock.Anything, c).Return(nil) - m.checkoutSvc.EXPECT().List(mock.Anything, checkout.Filter{CustomerID: "cust-1"}). - Return([]checkout.Checkout{}, nil) - m.checkoutSvc.EXPECT().DeleteByCustomer(mock.Anything, "cust-1").Return(nil) - m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1"). - Return(0, errors.New("balance check failed")) - // strict mocks: creditSvc.DeleteByAccountID and custSvc.Delete must not be called - - err := m.build().DeleteCustomers(context.Background(), "org-1") - assert.ErrorContains(t, err, "balance check failed") - }) - t.Run("offline account still removes local billing records", func(t *testing.T) { m := newMocks(t) @@ -1410,8 +1393,6 @@ func TestDeleteCustomers(t *testing.T) { m.checkoutSvc.EXPECT().List(mock.Anything, checkout.Filter{CustomerID: "cust-no-provider"}). Return([]checkout.Checkout{}, nil) m.checkoutSvc.EXPECT().DeleteByCustomer(mock.Anything, "cust-no-provider").Return(nil) - m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-no-provider").Return(0, nil) - m.creditSvc.EXPECT().DeleteByAccountID(mock.Anything, "cust-no-provider").Return(nil) m.custSvc.EXPECT().Delete(mock.Anything, "cust-no-provider").Return(nil) err := m.build().DeleteCustomers(context.Background(), "org-1") From 234d734b1ea542bffbf4e6f8940256d9ad120963 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Mon, 19 Oct 2026 13:39:05 +0530 Subject: [PATCH 08/10] chore: remove the unused tokens forfeited audit event --- core/audit/audit.go | 2 -- docs/content/docs/reference/webhook.mdx | 1 - web/sdk/admin/utils/webhook-events.ts | 1 - 3 files changed, 4 deletions(-) diff --git a/core/audit/audit.go b/core/audit/audit.go index c68b896b17..d16d501863 100644 --- a/core/audit/audit.go +++ b/core/audit/audit.go @@ -97,7 +97,6 @@ const ( BillingAccountDetailsUpdatedEvent EventName = "app.billing.account.details.updated" BillingCheckoutDeletedEvent EventName = "app.billing.checkout.deleted" - BillingTokensForfeitedEvent EventName = "app.billing.tokens.forfeited" ) var systemEvents = []EventName{ @@ -114,7 +113,6 @@ var systemEvents = []EventName{ OrgDeletedEvent, OrgDisabledEvent, BillingCheckoutDeletedEvent, - BillingTokensForfeitedEvent, } func IsSystemEvent(event EventName) bool { diff --git a/docs/content/docs/reference/webhook.mdx b/docs/content/docs/reference/webhook.mdx index 082bd50658..7b11b37e91 100644 --- a/docs/content/docs/reference/webhook.mdx +++ b/docs/content/docs/reference/webhook.mdx @@ -52,7 +52,6 @@ app.permission.checked app.billing.entitlement.checked app.billing.checkout.deleted -app.billing.tokens.forfeited app.policy.created app.policy.deleted diff --git a/web/sdk/admin/utils/webhook-events.ts b/web/sdk/admin/utils/webhook-events.ts index 5ab8c95583..28c0bc85b6 100644 --- a/web/sdk/admin/utils/webhook-events.ts +++ b/web/sdk/admin/utils/webhook-events.ts @@ -24,7 +24,6 @@ const events = [ "app.billing.entitlement.checked", "app.billing.checkout.deleted", - "app.billing.tokens.forfeited", "app.policy.created", "app.policy.deleted", From e77803944a3dbf26efaaa2933521d4291ea37767 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Tue, 20 Oct 2026 16:15:58 +0530 Subject: [PATCH 09/10] refactor(billing): remove the unused credit DeleteByAccountID --- .../credit/mocks/transaction_repository.go | 47 ------------------- billing/credit/service.go | 7 --- .../billing_transactions_repository.go | 17 ------- ...billing_transactions_repository_pg_test.go | 21 --------- 4 files changed, 92 deletions(-) diff --git a/billing/credit/mocks/transaction_repository.go b/billing/credit/mocks/transaction_repository.go index 76453bfda4..f1ad2880fd 100644 --- a/billing/credit/mocks/transaction_repository.go +++ b/billing/credit/mocks/transaction_repository.go @@ -83,53 +83,6 @@ func (_c *TransactionRepository_CreateEntry_Call) RunAndReturn(run func(context. return _c } -// DeleteByAccountID provides a mock function with given fields: ctx, accountID -func (_m *TransactionRepository) DeleteByAccountID(ctx context.Context, accountID string) error { - ret := _m.Called(ctx, accountID) - - if len(ret) == 0 { - panic("no return value specified for DeleteByAccountID") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string) error); ok { - r0 = rf(ctx, accountID) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// TransactionRepository_DeleteByAccountID_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeleteByAccountID' -type TransactionRepository_DeleteByAccountID_Call struct { - *mock.Call -} - -// DeleteByAccountID is a helper method to define mock.On call -// - ctx context.Context -// - accountID string -func (_e *TransactionRepository_Expecter) DeleteByAccountID(ctx interface{}, accountID interface{}) *TransactionRepository_DeleteByAccountID_Call { - return &TransactionRepository_DeleteByAccountID_Call{Call: _e.mock.On("DeleteByAccountID", ctx, accountID)} -} - -func (_c *TransactionRepository_DeleteByAccountID_Call) Run(run func(ctx context.Context, accountID string)) *TransactionRepository_DeleteByAccountID_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string)) - }) - return _c -} - -func (_c *TransactionRepository_DeleteByAccountID_Call) Return(_a0 error) *TransactionRepository_DeleteByAccountID_Call { - _c.Call.Return(_a0) - return _c -} - -func (_c *TransactionRepository_DeleteByAccountID_Call) RunAndReturn(run func(context.Context, string) error) *TransactionRepository_DeleteByAccountID_Call { - _c.Call.Return(run) - return _c -} - // GetBalance provides a mock function with given fields: ctx, id func (_m *TransactionRepository) GetBalance(ctx context.Context, id string) (int64, error) { ret := _m.Called(ctx, id) diff --git a/billing/credit/service.go b/billing/credit/service.go index 81396c53d1..5ff4ee1231 100644 --- a/billing/credit/service.go +++ b/billing/credit/service.go @@ -20,7 +20,6 @@ type TransactionRepository interface { GetByID(ctx context.Context, id string) (Transaction, error) GetBalanceForRange(ctx context.Context, accountID string, start time.Time, end time.Time) (int64, error) GetBalanceForRangeWithoutOverdraft(ctx context.Context, accountID string, start time.Time, end time.Time) (int64, error) - DeleteByAccountID(ctx context.Context, accountID string) error } type CustomerRepository interface { @@ -176,12 +175,6 @@ func (s Service) GetByID(ctx context.Context, id string) (Transaction, error) { return s.transactionRepository.GetByID(ctx, id) } -// DeleteByAccountID marks every credit transaction of a billing account as deleted. It is -// meant for account teardown. -func (s Service) DeleteByAccountID(ctx context.Context, accountID string) error { - return s.transactionRepository.DeleteByAccountID(ctx, accountID) -} - // createAuditRecord creates an audit record for billing transaction events. func (s Service) createAuditRecord(ctx context.Context, customerID string, eventType pkgAuditRecord.Event, txID string, txEntry Transaction) error { customerAcc, err := s.customerRepository.GetByID(ctx, customerID) diff --git a/internal/store/postgres/billing_transactions_repository.go b/internal/store/postgres/billing_transactions_repository.go index 10f98932ed..31a39c4a0b 100644 --- a/internal/store/postgres/billing_transactions_repository.go +++ b/internal/store/postgres/billing_transactions_repository.go @@ -312,23 +312,6 @@ func (r BillingTransactionRepository) UpdateByID(ctx context.Context, toUpdate c return customerModel.transform() } -func (r BillingTransactionRepository) DeleteByAccountID(ctx context.Context, accountID string) error { - query, params, err := softDelete(TABLE_BILLING_TRANSACTIONS).Where(goqu.Ex{ - "account_id": accountID, - }).ToSQL() - if err != nil { - return fmt.Errorf("%w: %w", errParse, err) - } - - if err = r.dbc.WithTimeout(ctx, TABLE_BILLING_TRANSACTIONS, "DeleteByAccountID", func(ctx context.Context) error { - _, err := r.dbc.ExecContext(ctx, query, params...) - return err - }); err != nil { - return fmt.Errorf("%w: %w", errTxn, err) - } - return nil -} - func (r BillingTransactionRepository) List(ctx context.Context, filter credit.Filter) ([]credit.Transaction, error) { stmt := fromLive(TABLE_BILLING_TRANSACTIONS).Order(goqu.I("created_at").Desc()) if filter.CustomerID != "" { diff --git a/internal/store/postgres/billing_transactions_repository_pg_test.go b/internal/store/postgres/billing_transactions_repository_pg_test.go index ab78d9546f..9f56101518 100644 --- a/internal/store/postgres/billing_transactions_repository_pg_test.go +++ b/internal/store/postgres/billing_transactions_repository_pg_test.go @@ -206,24 +206,3 @@ func (s *BillingTransactionRepositoryTestSuite) TestCreateEntryRefusesACreditToA func TestBillingTransactionRepository(t *testing.T) { suite.Run(t, new(BillingTransactionRepositoryTestSuite)) } - -func (s *BillingTransactionRepositoryTestSuite) TestDeleteByAccountIDKeepsTheRowsAndZeroesTheBalance() { - account := uuid.New().String() - s.txn(account, string(credit.CreditType), credit.SourceSystemBuyEvent, 100) - s.txn(account, string(credit.DebitType), "usage", 30) - gone := s.txn(account, string(credit.CreditType), credit.SourceSystemAwardedEvent, 5) - s.softDelete(gone) - before := scalarSQL(s.T(), s.ctx, s.client, `SELECT deleted_at::text FROM billing_transactions WHERE id = $1`, gone) - - s.Require().NoError(s.repository.DeleteByAccountID(s.ctx, account)) - - s.Equal("3", scalarSQL(s.T(), s.ctx, s.client, - `SELECT count(*)::text FROM billing_transactions WHERE account_id = $1 AND deleted_at IS NOT NULL`, account)) - s.Equal(before, scalarSQL(s.T(), s.ctx, s.client, `SELECT deleted_at::text FROM billing_transactions WHERE id = $1`, gone)) - balance, err := s.repository.GetBalance(s.ctx, account) - s.Require().NoError(err) - s.Equal(int64(0), balance) - got, err := s.repository.List(s.ctx, credit.Filter{CustomerID: account}) - s.Require().NoError(err) - s.Empty(got) -} From fc474d71e12f3bf1a8d811f34c807055f5369871 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Tue, 20 Oct 2026 17:19:58 +0530 Subject: [PATCH 10/10] feat(billing): only org teardown deletes are recovered for the owner notice --- billing/customer/customer.go | 10 +++++++++ billing/customer/mocks/repository.go | 21 ++++++++++--------- billing/customer/service.go | 6 +++--- billing/customer/service_test.go | 8 +++---- core/deleter/delete_notice.go | 21 ++++++++----------- core/deleter/delete_notice_test.go | 17 +++++++++++++-- core/deleter/mocks/customer_service.go | 21 ++++++++++--------- core/deleter/service.go | 4 ++-- core/deleter/service_test.go | 16 +++++++------- .../api/v1beta1connect/billing_customer.go | 2 +- internal/api/v1beta1connect/interfaces.go | 2 +- .../v1beta1connect/mocks/customer_service.go | 21 ++++++++++--------- .../postgres/billing_customer_repository.go | 7 ++++--- .../billing_customer_repository_pg_test.go | 8 ++++--- 14 files changed, 95 insertions(+), 69 deletions(-) diff --git a/billing/customer/customer.go b/billing/customer/customer.go index 43bf88d092..5db44daee5 100644 --- a/billing/customer/customer.go +++ b/billing/customer/customer.go @@ -135,3 +135,13 @@ func GetStripeWebhookSignatureFromContext(ctx context.Context) (string, bool) { u, ok := ctx.Value(consts.BillingStripeWebhookSignatureContextKey).(string) return u, ok } + +type DeleteOptions struct { + OrgTeardown bool +} + +const ( + DeleteRecordForfeitedTokensKey = "forfeited_tokens" + DeleteRecordPurchasedTokensKey = "purchased_tokens" + DeleteRecordOrgTeardownKey = "org_teardown" +) diff --git a/billing/customer/mocks/repository.go b/billing/customer/mocks/repository.go index 5348994df2..fd6f374b25 100644 --- a/billing/customer/mocks/repository.go +++ b/billing/customer/mocks/repository.go @@ -79,17 +79,17 @@ func (_c *Repository_Create_Call) RunAndReturn(run func(context.Context, custome return _c } -// Delete provides a mock function with given fields: ctx, id -func (_m *Repository) Delete(ctx context.Context, id string) error { - ret := _m.Called(ctx, id) +// Delete provides a mock function with given fields: ctx, id, opts +func (_m *Repository) Delete(ctx context.Context, id string, opts customer.DeleteOptions) error { + ret := _m.Called(ctx, id, opts) if len(ret) == 0 { panic("no return value specified for Delete") } var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string) error); ok { - r0 = rf(ctx, id) + if rf, ok := ret.Get(0).(func(context.Context, string, customer.DeleteOptions) error); ok { + r0 = rf(ctx, id, opts) } else { r0 = ret.Error(0) } @@ -105,13 +105,14 @@ type Repository_Delete_Call struct { // Delete is a helper method to define mock.On call // - ctx context.Context // - id string -func (_e *Repository_Expecter) Delete(ctx interface{}, id interface{}) *Repository_Delete_Call { - return &Repository_Delete_Call{Call: _e.mock.On("Delete", ctx, id)} +// - opts customer.DeleteOptions +func (_e *Repository_Expecter) Delete(ctx interface{}, id interface{}, opts interface{}) *Repository_Delete_Call { + return &Repository_Delete_Call{Call: _e.mock.On("Delete", ctx, id, opts)} } -func (_c *Repository_Delete_Call) Run(run func(ctx context.Context, id string)) *Repository_Delete_Call { +func (_c *Repository_Delete_Call) Run(run func(ctx context.Context, id string, opts customer.DeleteOptions)) *Repository_Delete_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string)) + run(args[0].(context.Context), args[1].(string), args[2].(customer.DeleteOptions)) }) return _c } @@ -121,7 +122,7 @@ func (_c *Repository_Delete_Call) Return(_a0 error) *Repository_Delete_Call { return _c } -func (_c *Repository_Delete_Call) RunAndReturn(run func(context.Context, string) error) *Repository_Delete_Call { +func (_c *Repository_Delete_Call) RunAndReturn(run func(context.Context, string, customer.DeleteOptions) error) *Repository_Delete_Call { _c.Call.Return(run) return _c } diff --git a/billing/customer/service.go b/billing/customer/service.go index 4c6b570a6a..be9c383f1e 100644 --- a/billing/customer/service.go +++ b/billing/customer/service.go @@ -30,7 +30,7 @@ type Repository interface { List(ctx context.Context, filter Filter) ([]Customer, error) Create(ctx context.Context, customer Customer) (Customer, error) UpdateByID(ctx context.Context, customer Customer) (Customer, error) - Delete(ctx context.Context, id string) error + Delete(ctx context.Context, id string, opts DeleteOptions) error HasLiveRecords(ctx context.Context, id string) (bool, error) UpdateCreditMinByID(ctx context.Context, customerID string, limit int64) (Details, error) GetDetailsByID(ctx context.Context, customerID string) (Details, error) @@ -268,7 +268,7 @@ func (s *Service) Disable(ctx context.Context, id string) error { return err } -func (s *Service) Delete(ctx context.Context, id string) error { +func (s *Service) Delete(ctx context.Context, id string, opts DeleteOptions) error { customer, err := s.repository.GetByID(ctx, id) if err != nil { return err @@ -307,7 +307,7 @@ func (s *Service) Delete(ctx context.Context, id string) error { } } - return s.repository.Delete(ctx, id) + return s.repository.Delete(ctx, id, opts) } func (s *Service) ListPaymentMethods(ctx context.Context, id string) ([]PaymentMethod, error) { diff --git a/billing/customer/service_test.go b/billing/customer/service_test.go index 9315f5e48b..68c41f13ba 100644 --- a/billing/customer/service_test.go +++ b/billing/customer/service_test.go @@ -521,7 +521,7 @@ func TestService_Delete(t *testing.T) { creditSvc.EXPECT().GetBalance(ctx, "acc-1").Return(int64(-50), nil) svc := customer.NewService(slog.Default(), nil, repo, billing.Config{}, creditSvc) - err := svc.Delete(ctx, "acc-1") + err := svc.Delete(ctx, "acc-1", customer.DeleteOptions{OrgTeardown: true}) assert.ErrorIs(t, err, customer.ErrHasTokenDebt) }) @@ -534,7 +534,7 @@ func TestService_Delete(t *testing.T) { repo.EXPECT().HasLiveRecords(ctx, "acc-1").Return(true, nil) svc := customer.NewService(slog.Default(), stripeClient, repo, billing.Config{}, creditSvc) - err := svc.Delete(ctx, "acc-1") + err := svc.Delete(ctx, "acc-1", customer.DeleteOptions{OrgTeardown: true}) assert.ErrorIs(t, err, customer.ErrHasLiveRecords) }) @@ -546,9 +546,9 @@ func TestService_Delete(t *testing.T) { creditSvc.EXPECT().GetBalance(ctx, "acc-1").Return(int64(0), nil) repo.EXPECT().HasLiveRecords(ctx, "acc-1").Return(false, nil) stripeBackend.EXPECT().Call("DELETE", "/v1/customers/cus_1", "key_123", mock.Anything, mock.Anything).Return(nil) - repo.EXPECT().Delete(ctx, "acc-1").Return(nil) + repo.EXPECT().Delete(ctx, "acc-1", customer.DeleteOptions{}).Return(nil) svc := customer.NewService(slog.Default(), stripeClient, repo, billing.Config{}, creditSvc) - require.NoError(t, svc.Delete(ctx, "acc-1")) + require.NoError(t, svc.Delete(ctx, "acc-1", customer.DeleteOptions{})) }) } diff --git a/core/deleter/delete_notice.go b/core/deleter/delete_notice.go index e34472d53b..3706a8c7f0 100644 --- a/core/deleter/delete_notice.go +++ b/core/deleter/delete_notice.go @@ -45,19 +45,13 @@ type deleteNoticeData struct { DeletedBy string } -// accountTokens is what one billing account holds at delete time. -type accountTokens struct { - Balance int64 - Purchased int64 -} - // deleteNotice is everything sendDeleteNotices needs once the org is gone. // It has to be collected before teardown removes the owners and the token -// balances. Accounts keeps the per-account numbers of the live accounts. +// balances. Accounts holds the ids of the live accounts that hold tokens. type deleteNotice struct { Amount int64 Purchased int64 - Accounts map[string]accountTokens + Accounts map[string]struct{} // Balances holds every account's balance, positive or not, so the // blocker check can reuse the reads Balances map[string]int64 @@ -74,7 +68,7 @@ type deleteNotice struct { // purchased share is settled with the customer. func (d Service) collectDeleteNotice(ctx context.Context, customers []customer.Customer) (deleteNotice, error) { var total, purchased int64 - accounts := make(map[string]accountTokens, len(customers)) + accounts := make(map[string]struct{}, len(customers)) balances := make(map[string]int64, len(customers)) for _, c := range customers { balance, err := d.creditService.GetBalance(ctx, c.ID) @@ -89,7 +83,7 @@ func (d Service) collectDeleteNotice(ctx context.Context, customers []customer.C } total += balance purchased += bought - accounts[c.ID] = accountTokens{Balance: balance, Purchased: bought} + accounts[c.ID] = struct{}{} } } return deleteNotice{ @@ -149,6 +143,9 @@ func (d Service) recoverForfeitFromAudit(ctx context.Context, orgID string, noti if accountID == "" { continue } + if teardown, _ := record.Metadata[customer.DeleteRecordOrgTeardownKey].(bool); !teardown { + continue + } if _, ok := seen[accountID]; ok { continue } @@ -156,8 +153,8 @@ func (d Service) recoverForfeitFromAudit(ctx context.Context, orgID string, noti if _, live := notice.Accounts[accountID]; live { continue } - forfeited, _ := record.Metadata["forfeited_tokens"].(float64) - purchased, _ := record.Metadata["purchased_tokens"].(float64) + forfeited, _ := record.Metadata[customer.DeleteRecordForfeitedTokensKey].(float64) + purchased, _ := record.Metadata[customer.DeleteRecordPurchasedTokensKey].(float64) notice.Amount += int64(forfeited) notice.Purchased += int64(purchased) } diff --git a/core/deleter/delete_notice_test.go b/core/deleter/delete_notice_test.go index ec58b42edc..9d0a51686c 100644 --- a/core/deleter/delete_notice_test.go +++ b/core/deleter/delete_notice_test.go @@ -5,6 +5,7 @@ import ( "errors" "testing" + "github.com/raystack/frontier/billing/customer" "github.com/raystack/frontier/core/auditrecord" "github.com/raystack/frontier/core/deleter/mocks" pkgauditrecord "github.com/raystack/frontier/pkg/auditrecord" @@ -32,7 +33,7 @@ func recoverWith(t *testing.T, records []auditrecord.AuditRecord, listErr error, } func TestRecoverForfeitFromAudit(t *testing.T) { - forfeit := metadata.Metadata{"forfeited_tokens": float64(70), "purchased_tokens": float64(30)} + forfeit := metadata.Metadata{customer.DeleteRecordForfeitedTokensKey: float64(70), customer.DeleteRecordPurchasedTokensKey: float64(30), customer.DeleteRecordOrgTeardownKey: true} t.Run("adds the forfeit of an account that is gone", func(t *testing.T) { got := recoverWith(t, []auditrecord.AuditRecord{deletedRecord("cust-1", forfeit)}, nil, deleteNotice{}) @@ -40,11 +41,23 @@ func TestRecoverForfeitFromAudit(t *testing.T) { assert.Equal(t, int64(30), got.Purchased) }) + t.Run("skips an account deleted on its own", func(t *testing.T) { + standalone := metadata.Metadata{customer.DeleteRecordForfeitedTokensKey: float64(70), customer.DeleteRecordPurchasedTokensKey: float64(30)} + got := recoverWith(t, []auditrecord.AuditRecord{deletedRecord("cust-1", standalone)}, nil, deleteNotice{}) + assert.Equal(t, deleteNotice{}, got) + }) + + t.Run("skips a record whose teardown flag is not a boolean", func(t *testing.T) { + malformed := metadata.Metadata{customer.DeleteRecordForfeitedTokensKey: float64(70), customer.DeleteRecordOrgTeardownKey: "true"} + got := recoverWith(t, []auditrecord.AuditRecord{deletedRecord("cust-1", malformed)}, nil, deleteNotice{}) + assert.Equal(t, deleteNotice{}, got) + }) + t.Run("skips an account that is still live", func(t *testing.T) { live := deleteNotice{ Amount: 100, Purchased: 100, - Accounts: map[string]accountTokens{"cust-1": {Balance: 100, Purchased: 100}}, + Accounts: map[string]struct{}{"cust-1": {}}, } got := recoverWith(t, []auditrecord.AuditRecord{deletedRecord("cust-1", forfeit)}, nil, live) assert.Equal(t, live, got) diff --git a/core/deleter/mocks/customer_service.go b/core/deleter/mocks/customer_service.go index dbd0afbe1d..e328fdba47 100644 --- a/core/deleter/mocks/customer_service.go +++ b/core/deleter/mocks/customer_service.go @@ -23,17 +23,17 @@ func (_m *CustomerService) EXPECT() *CustomerService_Expecter { return &CustomerService_Expecter{mock: &_m.Mock} } -// Delete provides a mock function with given fields: ctx, id -func (_m *CustomerService) Delete(ctx context.Context, id string) error { - ret := _m.Called(ctx, id) +// Delete provides a mock function with given fields: ctx, id, opts +func (_m *CustomerService) Delete(ctx context.Context, id string, opts customer.DeleteOptions) error { + ret := _m.Called(ctx, id, opts) if len(ret) == 0 { panic("no return value specified for Delete") } var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string) error); ok { - r0 = rf(ctx, id) + if rf, ok := ret.Get(0).(func(context.Context, string, customer.DeleteOptions) error); ok { + r0 = rf(ctx, id, opts) } else { r0 = ret.Error(0) } @@ -49,13 +49,14 @@ type CustomerService_Delete_Call struct { // Delete is a helper method to define mock.On call // - ctx context.Context // - id string -func (_e *CustomerService_Expecter) Delete(ctx interface{}, id interface{}) *CustomerService_Delete_Call { - return &CustomerService_Delete_Call{Call: _e.mock.On("Delete", ctx, id)} +// - opts customer.DeleteOptions +func (_e *CustomerService_Expecter) Delete(ctx interface{}, id interface{}, opts interface{}) *CustomerService_Delete_Call { + return &CustomerService_Delete_Call{Call: _e.mock.On("Delete", ctx, id, opts)} } -func (_c *CustomerService_Delete_Call) Run(run func(ctx context.Context, id string)) *CustomerService_Delete_Call { +func (_c *CustomerService_Delete_Call) Run(run func(ctx context.Context, id string, opts customer.DeleteOptions)) *CustomerService_Delete_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string)) + run(args[0].(context.Context), args[1].(string), args[2].(customer.DeleteOptions)) }) return _c } @@ -65,7 +66,7 @@ func (_c *CustomerService_Delete_Call) Return(_a0 error) *CustomerService_Delete return _c } -func (_c *CustomerService_Delete_Call) RunAndReturn(run func(context.Context, string) error) *CustomerService_Delete_Call { +func (_c *CustomerService_Delete_Call) RunAndReturn(run func(context.Context, string, customer.DeleteOptions) error) *CustomerService_Delete_Call { _c.Call.Return(run) return _c } diff --git a/core/deleter/service.go b/core/deleter/service.go index 8924aff130..744349ec7e 100644 --- a/core/deleter/service.go +++ b/core/deleter/service.go @@ -112,7 +112,7 @@ type ServiceUserService interface { } type CustomerService interface { - Delete(ctx context.Context, id string) error + Delete(ctx context.Context, id string, opts customer.DeleteOptions) error List(ctx context.Context, filter customer.Filter) ([]customer.Customer, error) } @@ -488,7 +488,7 @@ func (d Service) deleteCustomers(ctx context.Context, id string, customers []cus } } // delete customer - if err = d.customerService.Delete(ctx, c.ID); err != nil { + if err = d.customerService.Delete(ctx, c.ID, customer.DeleteOptions{OrgTeardown: true}); err != nil { return fmt.Errorf("failed to delete org while deleting a billing account[%s]: %w", c.ID, err) } } diff --git a/core/deleter/service_test.go b/core/deleter/service_test.go index f222f6d40e..8b80ab9a37 100644 --- a/core/deleter/service_test.go +++ b/core/deleter/service_test.go @@ -234,7 +234,7 @@ func TestDeleteOrganization(t *testing.T) { m.checkoutSvc.EXPECT().List(mock.Anything, checkout.Filter{CustomerID: "cust-1"}). Return([]checkout.Checkout{{ID: "chk-1", ProviderID: "cs_1", CustomerID: "cust-1"}}, nil) m.checkoutSvc.EXPECT().DeleteByCustomer(mock.Anything, "cust-1").Return(nil) - m.custSvc.EXPECT().Delete(mock.Anything, "cust-1").Return(nil) + m.custSvc.EXPECT().Delete(mock.Anything, "cust-1", customer.DeleteOptions{OrgTeardown: true}).Return(nil) // projects (each triggers DeleteProject) m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). @@ -596,7 +596,7 @@ func TestDeleteOrganization(t *testing.T) { m.checkoutSvc.EXPECT().List(mock.Anything, checkout.Filter{CustomerID: "cust-1"}). Return([]checkout.Checkout{}, nil) m.checkoutSvc.EXPECT().DeleteByCustomer(mock.Anything, "cust-1").Return(nil) - m.custSvc.EXPECT().Delete(mock.Anything, "cust-1").Return(nil) + m.custSvc.EXPECT().Delete(mock.Anything, "cust-1", customer.DeleteOptions{OrgTeardown: true}).Return(nil) m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). Return([]project.Project{}, nil) @@ -647,7 +647,7 @@ func TestDeleteOrganization(t *testing.T) { m.checkoutSvc.EXPECT().List(mock.Anything, checkout.Filter{CustomerID: "cust-1"}). Return([]checkout.Checkout{}, nil) m.checkoutSvc.EXPECT().DeleteByCustomer(mock.Anything, "cust-1").Return(nil) - m.custSvc.EXPECT().Delete(mock.Anything, "cust-1").Return(nil) + m.custSvc.EXPECT().Delete(mock.Anything, "cust-1", customer.DeleteOptions{OrgTeardown: true}).Return(nil) m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). Return([]project.Project{}, nil) @@ -752,7 +752,7 @@ func TestDeleteOrganization(t *testing.T) { m.checkoutSvc.EXPECT().List(mock.Anything, checkout.Filter{CustomerID: "cust-1"}). Return([]checkout.Checkout{}, nil) m.checkoutSvc.EXPECT().DeleteByCustomer(mock.Anything, "cust-1").Return(nil) - m.custSvc.EXPECT().Delete(mock.Anything, "cust-1").Return(nil) + m.custSvc.EXPECT().Delete(mock.Anything, "cust-1", customer.DeleteOptions{OrgTeardown: true}).Return(nil) m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). Return([]project.Project{}, nil) @@ -1262,7 +1262,7 @@ func TestForfeitAuditRecord(t *testing.T) { m.checkoutSvc.EXPECT().List(mock.Anything, checkout.Filter{CustomerID: "cust-1"}). Return([]checkout.Checkout{}, nil) m.checkoutSvc.EXPECT().DeleteByCustomer(mock.Anything, "cust-1").Return(nil) - m.custSvc.EXPECT().Delete(mock.Anything, "cust-1").Return(nil) + m.custSvc.EXPECT().Delete(mock.Anything, "cust-1", customer.DeleteOptions{OrgTeardown: true}).Return(nil) m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). Return([]project.Project{}, nil) m.resSvc.EXPECT().List(mock.Anything, resource.Filter{OrgID: "org-1"}). @@ -1299,7 +1299,7 @@ func TestForfeitAuditRecord(t *testing.T) { Event: pkgauditrecord.BillingCustomerDeletedEvent, Resource: auditrecord.Resource{ID: accountID, Type: pkgauditrecord.BillingCustomerType}, OrgID: "org-1", - Metadata: metadata.Metadata{"forfeited_tokens": forfeited, "purchased_tokens": purchased}, + Metadata: metadata.Metadata{"forfeited_tokens": forfeited, "purchased_tokens": purchased, "org_teardown": true}, } } m.auditRecSvc.EXPECT().List(mock.Anything, mock.MatchedBy(func(q *rql.Query) bool { @@ -1376,7 +1376,7 @@ func TestDeleteCustomers(t *testing.T) { {ID: "chk-2", ProviderID: "cs_2", CustomerID: "cust-1", State: "expired"}, }, nil) m.checkoutSvc.EXPECT().DeleteByCustomer(mock.Anything, "cust-1").Return(nil) - m.custSvc.EXPECT().Delete(mock.Anything, "cust-1").Return(nil) + m.custSvc.EXPECT().Delete(mock.Anything, "cust-1", customer.DeleteOptions{OrgTeardown: true}).Return(nil) err := m.build().DeleteCustomers(context.Background(), "org-1") assert.NoError(t, err) @@ -1393,7 +1393,7 @@ func TestDeleteCustomers(t *testing.T) { m.checkoutSvc.EXPECT().List(mock.Anything, checkout.Filter{CustomerID: "cust-no-provider"}). Return([]checkout.Checkout{}, nil) m.checkoutSvc.EXPECT().DeleteByCustomer(mock.Anything, "cust-no-provider").Return(nil) - m.custSvc.EXPECT().Delete(mock.Anything, "cust-no-provider").Return(nil) + m.custSvc.EXPECT().Delete(mock.Anything, "cust-no-provider", customer.DeleteOptions{OrgTeardown: true}).Return(nil) err := m.build().DeleteCustomers(context.Background(), "org-1") assert.NoError(t, err) diff --git a/internal/api/v1beta1connect/billing_customer.go b/internal/api/v1beta1connect/billing_customer.go index 40d4beab31..d5445621d5 100644 --- a/internal/api/v1beta1connect/billing_customer.go +++ b/internal/api/v1beta1connect/billing_customer.go @@ -159,7 +159,7 @@ func (h *ConnectHandler) ListBillingAccounts(ctx context.Context, request *conne } func (h *ConnectHandler) DeleteBillingAccount(ctx context.Context, request *connect.Request[frontierv1beta1.DeleteBillingAccountRequest]) (*connect.Response[frontierv1beta1.DeleteBillingAccountResponse], error) { - err := h.customerService.Delete(ctx, request.Msg.GetId()) + err := h.customerService.Delete(ctx, request.Msg.GetId(), customer.DeleteOptions{}) if err != nil { return nil, mapBillingError(ctx, fmt.Errorf("DeleteBillingAccount.Delete: customer_id=%s: %w", request.Msg.GetId(), err)) } diff --git a/internal/api/v1beta1connect/interfaces.go b/internal/api/v1beta1connect/interfaces.go index 96595b7aac..98eff4169d 100644 --- a/internal/api/v1beta1connect/interfaces.go +++ b/internal/api/v1beta1connect/interfaces.go @@ -236,7 +236,7 @@ type CustomerService interface { GetByOrgID(ctx context.Context, orgID string) (customer.Customer, error) Create(ctx context.Context, customer customer.Customer, offline bool) (customer.Customer, error) List(ctx context.Context, filter customer.Filter) ([]customer.Customer, error) - Delete(ctx context.Context, id string) error + Delete(ctx context.Context, id string, opts customer.DeleteOptions) error ListPaymentMethods(ctx context.Context, id string) ([]customer.PaymentMethod, error) Update(ctx context.Context, customer customer.Customer) (customer.Customer, error) RegisterToProviderIfRequired(ctx context.Context, customerID string) (customer.Customer, error) diff --git a/internal/api/v1beta1connect/mocks/customer_service.go b/internal/api/v1beta1connect/mocks/customer_service.go index 8adc0b89f6..ea858e9be4 100644 --- a/internal/api/v1beta1connect/mocks/customer_service.go +++ b/internal/api/v1beta1connect/mocks/customer_service.go @@ -80,17 +80,17 @@ func (_c *CustomerService_Create_Call) RunAndReturn(run func(context.Context, cu return _c } -// Delete provides a mock function with given fields: ctx, id -func (_m *CustomerService) Delete(ctx context.Context, id string) error { - ret := _m.Called(ctx, id) +// Delete provides a mock function with given fields: ctx, id, opts +func (_m *CustomerService) Delete(ctx context.Context, id string, opts customer.DeleteOptions) error { + ret := _m.Called(ctx, id, opts) if len(ret) == 0 { panic("no return value specified for Delete") } var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string) error); ok { - r0 = rf(ctx, id) + if rf, ok := ret.Get(0).(func(context.Context, string, customer.DeleteOptions) error); ok { + r0 = rf(ctx, id, opts) } else { r0 = ret.Error(0) } @@ -106,13 +106,14 @@ type CustomerService_Delete_Call struct { // Delete is a helper method to define mock.On call // - ctx context.Context // - id string -func (_e *CustomerService_Expecter) Delete(ctx interface{}, id interface{}) *CustomerService_Delete_Call { - return &CustomerService_Delete_Call{Call: _e.mock.On("Delete", ctx, id)} +// - opts customer.DeleteOptions +func (_e *CustomerService_Expecter) Delete(ctx interface{}, id interface{}, opts interface{}) *CustomerService_Delete_Call { + return &CustomerService_Delete_Call{Call: _e.mock.On("Delete", ctx, id, opts)} } -func (_c *CustomerService_Delete_Call) Run(run func(ctx context.Context, id string)) *CustomerService_Delete_Call { +func (_c *CustomerService_Delete_Call) Run(run func(ctx context.Context, id string, opts customer.DeleteOptions)) *CustomerService_Delete_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string)) + run(args[0].(context.Context), args[1].(string), args[2].(customer.DeleteOptions)) }) return _c } @@ -122,7 +123,7 @@ func (_c *CustomerService_Delete_Call) Return(_a0 error) *CustomerService_Delete return _c } -func (_c *CustomerService_Delete_Call) RunAndReturn(run func(context.Context, string) error) *CustomerService_Delete_Call { +func (_c *CustomerService_Delete_Call) RunAndReturn(run func(context.Context, string, customer.DeleteOptions) error) *CustomerService_Delete_Call { _c.Call.Return(run) return _c } diff --git a/internal/store/postgres/billing_customer_repository.go b/internal/store/postgres/billing_customer_repository.go index 50bc4d96bd..b63a87e277 100644 --- a/internal/store/postgres/billing_customer_repository.go +++ b/internal/store/postgres/billing_customer_repository.go @@ -498,7 +498,7 @@ func (r BillingCustomerRepository) UpdateDetailsByID(ctx context.Context, custom }, nil } -func (r BillingCustomerRepository) Delete(ctx context.Context, id string) error { +func (r BillingCustomerRepository) Delete(ctx context.Context, id string, opts customer.DeleteOptions) error { lockQuery, lockParams, err := fromLive(TABLE_BILLING_CUSTOMERS). Select("id"). Where(goqu.Ex{"id": id}). @@ -580,8 +580,9 @@ func (r BillingCustomerRepository) Delete(ctx context.Context, id string) error nil, customerModel.OrgID, map[string]any{ - "forfeited_tokens": forfeitedTokens, - "purchased_tokens": purchasedTokens, + customer.DeleteRecordForfeitedTokensKey: forfeitedTokens, + customer.DeleteRecordPurchasedTokensKey: purchasedTokens, + customer.DeleteRecordOrgTeardownKey: opts.OrgTeardown, }, deletedAt, ) diff --git a/internal/store/postgres/billing_customer_repository_pg_test.go b/internal/store/postgres/billing_customer_repository_pg_test.go index 73ad43da52..3097c33b19 100644 --- a/internal/store/postgres/billing_customer_repository_pg_test.go +++ b/internal/store/postgres/billing_customer_repository_pg_test.go @@ -225,7 +225,8 @@ func (s *BillingCustomerRepositoryPGTestSuite) TestTeardownInTheDeleterOrderForf s.Require().NoError(s.subscriptions.Delete(s.ctx, subID)) s.Require().NoError(s.invoices.Delete(s.ctx, invID)) s.Require().NoError(s.checkouts.DeleteByCustomerID(s.ctx, cust)) - s.Require().NoError(s.repository.Delete(s.ctx, cust)) + s.Require().NoError(s.repository.Delete(s.ctx, cust, customer.DeleteOptions{OrgTeardown: true})) + s.Equal("true", s.recordedMetadata(cust, "org_teardown")) s.Equal("70", s.recordedMetadata(cust, "forfeited_tokens"), "the customer delete hides the transactions and records what they held") s.Equal("70", s.recordedMetadata(cust, "purchased_tokens")) @@ -324,12 +325,13 @@ func (s *BillingCustomerRepositoryPGTestSuite) TestDeleteHidesTheAccountsTransac s.exec(`UPDATE billing_transactions SET deleted_at = now() - interval '1 day' WHERE id = $1`, alreadyHidden) hiddenAt := s.transactionDeletedAt(alreadyHidden) - s.Require().NoError(s.repository.Delete(s.ctx, id)) + s.Require().NoError(s.repository.Delete(s.ctx, id, customer.DeleteOptions{})) for _, txn := range []string{bought, refunded, awarded, used} { s.NotEmpty(s.transactionDeletedAt(txn)) } s.Equal(hiddenAt, s.transactionDeletedAt(alreadyHidden), "an older delete time is kept") + s.Equal("false", s.recordedMetadata(id, "org_teardown"), "a delete outside the org teardown is not marked") s.Equal("250", s.recordedMetadata(id, "forfeited_tokens"), "the live credits minus the live debits") s.Equal("180", s.recordedMetadata(id, "purchased_tokens"), "the share of it bought, net of refunds") balance, err := s.transactions.GetBalance(s.ctx, id) @@ -381,7 +383,7 @@ func (s *BillingCustomerRepositoryPGTestSuite) TestHasLiveRecordsSeesOnlyLiveChi } func (s *BillingCustomerRepositoryPGTestSuite) deleteError(id string) error { - return s.repository.Delete(s.ctx, id) + return s.repository.Delete(s.ctx, id, customer.DeleteOptions{}) } func (s *BillingCustomerRepositoryPGTestSuite) deleteAccount(id string) {