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/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/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..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 } @@ -240,6 +241,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 133af7db81..be9c383f1e 100644 --- a/billing/customer/service.go +++ b/billing/customer/service.go @@ -30,7 +30,8 @@ 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) UpdateDetailsByID(ctx context.Context, customerID string, details Details) (Details, error) @@ -267,12 +268,28 @@ 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 } + 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 + } + + 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 != "" { @@ -290,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 348c282157..68c41f13ba 100644 --- a/billing/customer/service_test.go +++ b/billing/customer/service_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/raystack/frontier/billing" @@ -509,3 +510,45 @@ 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", customer.DeleteOptions{OrgTeardown: true}) + + assert.ErrorIs(t, err, customer.ErrHasTokenDebt) + }) + + 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", customer.DeleteOptions{OrgTeardown: true}) + + 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", customer.DeleteOptions{}).Return(nil) + svc := customer.NewService(slog.Default(), stripeClient, repo, billing.Config{}, creditSvc) + + require.NoError(t, svc.Delete(ctx, "acc-1", customer.DeleteOptions{})) + }) +} 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/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/core/deleter/delete_notice.go b/core/deleter/delete_notice.go index 6b08223244..3706a8c7f0 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" @@ -42,20 +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 so the teardown can audit -// them without reading the balances a second time. +// 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 @@ -72,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) @@ -87,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{ @@ -128,29 +124,28 @@ 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 } + if teardown, _ := record.Metadata[customer.DeleteRecordOrgTeardownKey].(bool); !teardown { + continue + } if _, ok := seen[accountID]; ok { continue } @@ -158,32 +153,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[customer.DeleteRecordForfeitedTokensKey].(float64) + purchased, _ := record.Metadata[customer.DeleteRecordPurchasedTokensKey].(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..9d0a51686c --- /dev/null +++ b/core/deleter/delete_notice_test.go @@ -0,0 +1,76 @@ +package deleter + +import ( + "context" + "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" + "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{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{}) + assert.Equal(t, int64(70), got.Amount) + 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]struct{}{"cust-1": {}}, + } + 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/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 668252f0f3..744349ec7e 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 { @@ -111,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) } @@ -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,32 +487,8 @@ 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 { + 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 b6514c2d2f..8b80ab9a37 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,8 +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.creditSvc.EXPECT().DeleteByAccountID(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}). @@ -585,8 +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.creditSvc.EXPECT().DeleteByAccountID(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) @@ -637,8 +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.creditSvc.EXPECT().DeleteByAccountID(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) @@ -743,8 +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.creditSvc.EXPECT().DeleteByAccountID(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) @@ -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,8 +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.creditSvc.EXPECT().DeleteByAccountID(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"}). @@ -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, "org_teardown": true}, + } + } + 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) + 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) }) - 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,9 +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.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) + 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/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/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/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), 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 4d75ac9524..b63a87e277 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,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}). @@ -517,7 +518,18 @@ func (r BillingCustomerRepository) Delete(ctx context.Context, id string) error return 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 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 @@ -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(&forfeitedTokens, &bought); err != nil { + return err + } + if forfeitedTokens < 0 { + return customer.ErrHasTokenDebt + } + 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 @@ -559,7 +579,11 @@ func (r BillingCustomerRepository) Delete(ctx context.Context, id string) error }, nil, customerModel.OrgID, - nil, + map[string]any{ + customer.DeleteRecordForfeitedTokensKey: forfeitedTokens, + customer.DeleteRecordPurchasedTokensKey: purchasedTokens, + customer.DeleteRecordOrgTeardownKey: opts.OrgTeardown, + }, deletedAt, ) return InsertAuditRecordInTx(ctx, tx, auditRecord) @@ -571,6 +595,8 @@ func (r BillingCustomerRepository) Delete(ctx context.Context, id string) error return customer.ErrNotFound case errors.Is(err, customer.ErrHasLiveRecords): return customer.ErrHasLiveRecords + case errors.Is(err, customer.ErrHasTokenDebt): + return customer.ErrHasTokenDebt default: return fmt.Errorf("%w: %w", errTxn, err) } @@ -579,6 +605,21 @@ func (r BillingCustomerRepository) Delete(ctx context.Context, id string) error return 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 5546f4c04b..3097c33b19 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) @@ -147,9 +147,11 @@ 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.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 +181,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) @@ -216,15 +218,17 @@ 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)) - 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")) s.Equal("1", s.deletedRows(postgres.TABLE_BILLING_SUBSCRIPTIONS, "customer_id", cust)) s.Equal("1", s.deletedRows(postgres.TABLE_BILLING_INVOICES, "customer_id", cust)) @@ -262,10 +266,8 @@ 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.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,99 @@ 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) 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, + `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) + + 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) + 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) 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 { + return s.repository.Delete(s.ctx, id, customer.DeleteOptions{}) +} + +func (s *BillingCustomerRepositoryPGTestSuite) deleteAccount(id string) { + s.T().Helper() + s.Require().NoError(s.deleteError(id)) +} 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) -} 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",