From 5172886787af76ece634dcff3db9d2fefaf9d997 Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Mon, 10 Aug 2026 10:23:57 +0530 Subject: [PATCH] feat(deleter): clear or report org delete blockers up front MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The delete first checks everything that blocks it and returns all the reasons together as one failed_precondition response: a running subscription on a paid plan (the caller downgrades it to the standard plan), invoices that still ask for money, and a negative token balance which support has to settle. Plans resolve lazily, only when a running subscription references one. When nothing blocks, subscriptions still running on a free plan are canceled immediately with unbilled usage invoiced on the spot — tolerating copies already gone on the provider — and the invoice check runs again so a final invoice still blocks. The plan is judged again in that pass, so a paid subscription created mid-delete blocks instead of being canceled. Unused tokens do not block: the delete forfeits them and writes the amount to an audit record. An already-deleted org returns not found before any checks run. Invoices are judged straight from the billing provider through a new cheap ListPayableOnProvider (three status-filtered pages, no local writes) instead of a full sync, and drafts with a non-zero amount now block too: the provider finalizes them shortly, and deleting inside that window would silently lose the charge. --- billing/invoice/invoice.go | 15 + billing/invoice/service.go | 53 +++ cmd/serve.go | 2 +- core/audit/audit.go | 2 + core/deleter/deleter.go | 37 +- core/deleter/service.go | 300 ++++++++++++-- core/deleter/service_test.go | 416 +++++++++++++++++++- go.mod | 2 +- internal/api/v1beta1connect/deleter.go | 34 ++ internal/api/v1beta1connect/deleter_test.go | 49 +++ 10 files changed, 873 insertions(+), 37 deletions(-) diff --git a/billing/invoice/invoice.go b/billing/invoice/invoice.go index d40f2f3534..3e63be0da3 100644 --- a/billing/invoice/invoice.go +++ b/billing/invoice/invoice.go @@ -2,6 +2,7 @@ package invoice import ( "fmt" + "slices" "time" "github.com/raystack/frontier/pkg/pagination" @@ -43,8 +44,22 @@ const ( DraftState State = "draft" OpenState State = "open" PaidState State = "paid" + // UncollectibleState marks an invoice the provider has written off; it + // can still be paid. + UncollectibleState State = "uncollectible" ) +// PayableStates are the invoice states that can still ask the customer for +// money: open and uncollectible invoices are payable now, a draft becomes +// payable once the provider finalizes it. The delete blockers and the +// provider-side payable listing must agree on this set. +var PayableStates = []State{DraftState, OpenState, UncollectibleState} + +// IsPayable reports whether the invoice's state is one of PayableStates. +func (i Invoice) IsPayable() bool { + return slices.Contains(PayableStates, i.State) +} + type Invoice struct { ID string CustomerID string diff --git a/billing/invoice/service.go b/billing/invoice/service.go index 14c72b08a9..15b4fb8969 100644 --- a/billing/invoice/service.go +++ b/billing/invoice/service.go @@ -244,6 +244,59 @@ func (s *Service) isCreditOverdraftDayOfInvoice() bool { return time.Now().UTC().Day() == s.creditOverdraftInvoiceDay } +// ListPayableOnProvider returns the customer's invoices that still ask for +// money — open, uncollectible, or being prepared as drafts — read straight +// from the billing provider so the answer is current. Zero-amount invoices +// are skipped. Where a local row exists for the invoice it keeps its local +// id; an invoice the sync has not seen yet is returned with an empty id and +// only its provider reference. Unlike SyncWithProvider this touches no local +// rows, expands nothing, and reads one status-filtered listing per payable +// state — a customer with very many payable invoices pages through more +// requests, but the cost scales with what is still owed, not with the whole +// invoice history. +func (s *Service) ListPayableOnProvider(ctx context.Context, customr customer.Customer) ([]Invoice, error) { + localInvoices, err := s.repository.List(ctx, Filter{ + CustomerID: customr.ID, + }) + if err != nil { + return nil, err + } + localByProviderID := make(map[string]Invoice, len(localInvoices)) + for _, inv := range localInvoices { + localByProviderID[inv.ProviderID] = inv + } + + var payable []Invoice + for _, status := range PayableStates { + stripeInvoices := s.stripeClient.Invoices.List(&stripe.InvoiceListParams{ + Customer: stripe.String(customr.ProviderID), + Status: stripe.String(string(status)), + ListParams: stripe.ListParams{ + Context: ctx, + }, + }) + for stripeInvoices.Next() { + stripeInvoice := stripeInvoices.Invoice() + // AmountRemaining is what the customer still owes; Total is not: + // a negative total is a credit owed to the customer, and an open + // invoice fully covered by their credit balance has a positive + // total with nothing due. Neither asks for money. + if stripeInvoice.AmountRemaining <= 0 { + continue + } + inv := stripeInvoiceToInvoice(customr.ID, stripeInvoice) + if local, ok := localByProviderID[stripeInvoice.ID]; ok { + inv.ID = local.ID + } + payable = append(payable, inv) + } + if err := stripeInvoices.Err(); err != nil { + return nil, fmt.Errorf("failed to list %s invoices: %w", status, billingerrors.TranslateStripeError(err)) + } + } + return payable, nil +} + func (s *Service) SyncWithProvider(ctx context.Context, customr customer.Customer) error { s.mu.Lock() defer s.mu.Unlock() diff --git a/cmd/serve.go b/cmd/serve.go index 346d9ecad2..2f7547f128 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -587,7 +587,7 @@ func buildAPIDependencies( cascadeDeleter := deleter.NewCascadeDeleter(organizationService, projectService, resourceService, groupService, membershipService, policyService, roleService, invitationService, userService, userPATService, serviceUserService, customerService, subscriptionService, invoiceService, checkoutService, - creditService, orgKycService, + creditService, orgKycService, planService, ) // we should default it with a stdout logger repository as postgres can start to bloat really fast diff --git a/core/audit/audit.go b/core/audit/audit.go index d16d501863..c68b896b17 100644 --- a/core/audit/audit.go +++ b/core/audit/audit.go @@ -97,6 +97,7 @@ const ( BillingAccountDetailsUpdatedEvent EventName = "app.billing.account.details.updated" BillingCheckoutDeletedEvent EventName = "app.billing.checkout.deleted" + BillingTokensForfeitedEvent EventName = "app.billing.tokens.forfeited" ) var systemEvents = []EventName{ @@ -113,6 +114,7 @@ var systemEvents = []EventName{ OrgDeletedEvent, OrgDisabledEvent, BillingCheckoutDeletedEvent, + BillingTokensForfeitedEvent, } func IsSystemEvent(event EventName) bool { diff --git a/core/deleter/deleter.go b/core/deleter/deleter.go index cad34469c3..63eea45d4d 100644 --- a/core/deleter/deleter.go +++ b/core/deleter/deleter.go @@ -1,7 +1,38 @@ package deleter -import "fmt" +import "strings" -var ( - ErrDeleteNotAllowed = fmt.Errorf("deletion not allowed for billed accounts") +// Reasons an organization delete can be blocked. The API error carries +// them as violation types, so a client can tell the reasons apart +// without reading the message text. +const ( + BlockerActiveSubscription = "ACTIVE_SUBSCRIPTION" + BlockerUnpaidInvoice = "UNPAID_INVOICE" + BlockerNegativeTokenBalance = "NEGATIVE_TOKEN_BALANCE" ) + +// Blocker is one reason an organization cannot be deleted right now. +type Blocker struct { + // Type is one of the Blocker* constants. + Type string + // Subject is the id of the entity behind the reason. + Subject string + // Message says what blocks the delete and how to clear it. + Message string +} + +// BlockedError carries every blocker the up-front check found, so the +// caller gets one checklist instead of discovering blockers one retry at +// a time. +type BlockedError struct { + OrgID string + Blockers []Blocker +} + +func (e *BlockedError) Error() string { + msgs := make([]string, 0, len(e.Blockers)) + for _, b := range e.Blockers { + msgs = append(msgs, b.Message) + } + return "organization cannot be deleted yet: " + strings.Join(msgs, "; ") +} diff --git a/core/deleter/service.go b/core/deleter/service.go index fb4be96ebc..50e6e43127 100644 --- a/core/deleter/service.go +++ b/core/deleter/service.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "log/slog" + "strconv" "github.com/raystack/frontier/core/audit" @@ -16,6 +17,10 @@ import ( "github.com/raystack/frontier/billing/customer" + "github.com/raystack/frontier/billing/plan" + + "github.com/raystack/frontier/billing/subscription" + "github.com/raystack/frontier/core/organization" "github.com/raystack/frontier/internal/bootstrap/schema" @@ -34,10 +39,6 @@ import ( "github.com/raystack/frontier/core/serviceuser" ) -const ( - DisableDeleteIfBilled = true -) - type ProjectService interface { List(ctx context.Context, flt project.Filter) ([]project.Project, error) DeleteModel(ctx context.Context, id string) error @@ -98,11 +99,14 @@ type CustomerService interface { } type SubscriptionService interface { + List(ctx context.Context, filter subscription.Filter) ([]subscription.Subscription, error) + Cancel(ctx context.Context, id string, immediate bool) (subscription.Subscription, error) DeleteByCustomer(ctx context.Context, customr customer.Customer) error } type InvoiceService interface { List(ctx context.Context, flt invoice.Filter) ([]invoice.Invoice, error) + ListPayableOnProvider(ctx context.Context, customr customer.Customer) ([]invoice.Invoice, error) DeleteByCustomer(ctx context.Context, customr customer.Customer) error } @@ -112,6 +116,7 @@ type CheckoutService interface { } type CreditService interface { + GetBalance(ctx context.Context, accountID string) (int64, error) DeleteByAccountID(ctx context.Context, accountID string) error } @@ -119,6 +124,10 @@ type KycService interface { DeleteKyc(ctx context.Context, orgID string) error } +type PlanService interface { + GetByID(ctx context.Context, id string) (plan.Plan, error) +} + type Service struct { projService ProjectService orgService OrganizationService @@ -137,6 +146,7 @@ type Service struct { checkoutService CheckoutService creditService CreditService kycService KycService + planService PlanService } func NewCascadeDeleter(orgService OrganizationService, projService ProjectService, @@ -148,7 +158,8 @@ func NewCascadeDeleter(orgService OrganizationService, projService ProjectServic serviceUserService ServiceUserService, customerService CustomerService, subService SubscriptionService, invoiceService InvoiceService, checkoutService CheckoutService, - creditService CreditService, kycService KycService) *Service { + creditService CreditService, kycService KycService, + planService PlanService) *Service { return &Service{ projService: projService, orgService: orgService, @@ -167,6 +178,7 @@ func NewCascadeDeleter(orgService OrganizationService, projService ProjectServic checkoutService: checkoutService, creditService: creditService, kycService: kycService, + planService: planService, } } @@ -215,15 +227,32 @@ func (d Service) DeleteGroup(ctx context.Context, id string) error { // likely to fail. Identity (projects, policies, and so on) goes after, with // org policies (the org owners) near the end. This way a failure at any step // leaves the org owned and the delete can simply be run again. Every step -// treats already-deleted data as success for the same reason. +// treats already-deleted data as success for the same reason. That applies +// to data inside a half-deleted org; an org whose row is already fully gone +// answers not found instead, so a caller can tell a finished delete from a +// repeatable one (and a mistyped org id from a success). func (d Service) DeleteOrganization(ctx context.Context, id string) error { - // check if delete is allowed - if err := d.canDelete(ctx, id); err != nil { - return fmt.Errorf("%s: %w", err.Error(), ErrDeleteNotAllowed) + // an org that is already gone has nothing left to check or tear down; + // disabled orgs stay deletable + if _, err := d.orgService.Get(ctx, id); err != nil && !errors.Is(err, organization.ErrDisabled) { + return err + } + + customers, err := d.customerService.List(ctx, customer.Filter{ + OrgID: id, + }) + if err != nil { + return err + } + + // clear what we can and collect what still blocks the delete, before + // touching any data + if err := d.ensureDeletable(ctx, id, customers); err != nil { + return err } // delete all billing accounts - if err := d.DeleteCustomers(ctx, id); err != nil { + if err := d.deleteCustomers(ctx, id, customers); err != nil { return err } @@ -316,6 +345,8 @@ func (d Service) DeleteOrganization(ctx context.Context, id string) error { return nil } +// DeleteCustomers lists the org's billing accounts itself; DeleteOrganization +// goes through deleteCustomers with the accounts it already listed. func (d Service) DeleteCustomers(ctx context.Context, id string) error { customers, err := d.customerService.List(ctx, customer.Filter{ OrgID: id, @@ -323,6 +354,10 @@ func (d Service) DeleteCustomers(ctx context.Context, id string) error { if err != nil { return err } + return d.deleteCustomers(ctx, id, customers) +} + +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 removes local records if err := d.subService.DeleteByCustomer(ctx, c); err != nil { @@ -364,6 +399,22 @@ func (d Service) DeleteCustomers(ctx context.Context, id string) error { 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 + balance, err := d.creditService.GetBalance(ctx, c.ID) + if err != nil { + return fmt.Errorf("failed to delete org while checking balance of billing account[%s]: %w", c.ID, err) + } + if balance > 0 { + if err := auditLogger.LogWithAttrs(audit.BillingTokensForfeitedEvent, audit.Target{ + ID: c.ID, + Type: "billing_account", + }, map[string]string{ + "amount": strconv.FormatInt(balance, 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) } @@ -412,23 +463,228 @@ func (d Service) DeleteUser(ctx context.Context, userID string) error { return d.userService.Delete(ctx, userID) } -func (d Service) canDelete(ctx context.Context, id string) error { - // check if any invoice is present for customer - customers, err := d.customerService.List(ctx, customer.Filter{ - OrgID: id, - }) - if err != nil { - return err +// ensureDeletable collects everything that blocks deleting the organization and +// returns it all as one BlockedError, so the caller gets a full checklist +// instead of discovering blockers one retry at a time. +// +// A running subscription on a paid plan blocks the delete: the caller has +// to downgrade it to the standard plan first. A running subscription on a +// free plan is not a blocker — once no blockers are left, ensureDeletable +// cancels it itself, before any deletion starts. The cancel is immediate +// and bills unbilled usage on the spot, so the invoice check runs once more +// after it: a final invoice created by the cancel still blocks the delete +// until it is paid or its automatic charge settles. +// +// Unused tokens do not block either: the delete forfeits them. The client +// gets the caller's confirmation before sending the delete, and the +// forfeited amount is written to an audit record during teardown. +// +// Accounts without a billing provider are only checked for token balances: +// their subscription and invoice rows have nothing behind them the caller +// could cancel or pay. +func (d Service) ensureDeletable(ctx context.Context, id string, customers []customer.Customer) error { + // each plan resolves at most once per call, and only when a running + // subscription actually references it + paidPlans := map[string]bool{} + + var blockers []Blocker + for _, c := range customers { + if !c.IsOffline() { + bs, err := d.subscriptionBlockers(ctx, c, paidPlans) + if err != nil { + return err + } + blockers = append(blockers, bs...) + + bs, err = d.invoiceBlockers(ctx, c) + if err != nil { + return err + } + blockers = append(blockers, bs...) + } + + balance, err := d.creditService.GetBalance(ctx, c.ID) + if err != nil { + return fmt.Errorf("failed to check token balance of billing account[%s]: %w", c.ID, err) + } + // the balance goes below zero when the account has an overdraft + // floor (credit_min under zero, the postpaid setup) and tokens were + // spent on credit. That debt is money owed, so it must be settled + // before the org can go + if balance < 0 { + blockers = append(blockers, Blocker{ + Type: BlockerNegativeTokenBalance, + Subject: c.ID, + Message: fmt.Sprintf("billing account[%s] owes %d tokens: contact support to settle the balance, then retry the delete", c.ID, -balance), + }) + } + } + if len(blockers) > 0 { + return &BlockedError{OrgID: id, Blockers: blockers} } + // no blockers were found, so the delete will happen. Only free-plan + // subscriptions can still be running: cancel them, then check the + // invoices again because the cancel may have created a final one. Every + // account is judged again before anything is canceled — a paid + // subscription created since the first pass, on any account, must block + // the delete without costing another account its subscription first + type cancelTarget struct { + customer customer.Customer + subID string + } + var toCancel []cancelTarget for _, c := range customers { - if invoices, err := d.invoiceService.List(ctx, invoice.Filter{CustomerID: c.ID}); err != nil { - return fmt.Errorf("failed to check invoices for billing account[%s]: %w", c.ID, err) - } else if len(invoices) > 0 { - if DisableDeleteIfBilled { - return fmt.Errorf("cannot delete organization with billing account[%s]", c.ID) + if c.IsOffline() { + continue + } + subs, err := d.subService.List(ctx, subscription.Filter{CustomerID: c.ID}) + if err != nil { + return fmt.Errorf("failed to check subscriptions for billing account[%s]: %w", c.ID, err) + } + for _, sub := range subs { + if !sub.IsActive() { + continue } + if sub.PlanID == "" { + blockers = append(blockers, unresolvablePlanBlocker(sub)) + continue + } + paid, err := d.isPaidPlan(ctx, sub.PlanID, paidPlans) + if errors.Is(err, plan.ErrNotFound) { + blockers = append(blockers, unresolvablePlanBlocker(sub)) + continue + } + if err != nil { + return err + } + if paid { + blockers = append(blockers, paidSubscriptionBlocker(sub)) + continue + } + toCancel = append(toCancel, cancelTarget{customer: c, subID: sub.ID}) + } + } + if len(blockers) > 0 { + return &BlockedError{OrgID: id, Blockers: blockers} + } + + canceledFor := map[string]customer.Customer{} + for _, t := range toCancel { + // a subscription whose provider copy is already gone has nothing + // to cancel; the teardown removes its local rows + if _, err := d.subService.Cancel(ctx, t.subID, true); err != nil && !errors.Is(err, subscription.ErrSubscriptionOnProviderNotFound) { + return fmt.Errorf("failed to cancel subscription[%s] of billing account[%s]: %w", t.subID, t.customer.ID, err) + } + canceledFor[t.customer.ID] = t.customer + } + for _, c := range canceledFor { + bs, err := d.invoiceBlockers(ctx, c) + if err != nil { + return err } + blockers = append(blockers, bs...) + } + if len(blockers) > 0 { + return &BlockedError{OrgID: id, Blockers: blockers} } return nil } + +// subscriptionBlockers returns a blocker for every running subscription on a +// paid plan; the caller downgrades those to the standard plan. Running +// free-plan subscriptions are not blockers, the delete cancels them itself. +func (d Service) subscriptionBlockers(ctx context.Context, c customer.Customer, paidPlans map[string]bool) ([]Blocker, error) { + subs, err := d.subService.List(ctx, subscription.Filter{CustomerID: c.ID}) + if err != nil { + return nil, fmt.Errorf("failed to check subscriptions for billing account[%s]: %w", c.ID, err) + } + var blockers []Blocker + for _, sub := range subs { + if !sub.IsActive() { + continue + } + // a missing or dangling plan reference must not make the org + // undeletable, and "downgrade" is no advice for it — the caller + // can still cancel the subscription themselves + if sub.PlanID == "" { + blockers = append(blockers, unresolvablePlanBlocker(sub)) + continue + } + paid, err := d.isPaidPlan(ctx, sub.PlanID, paidPlans) + if errors.Is(err, plan.ErrNotFound) { + blockers = append(blockers, unresolvablePlanBlocker(sub)) + continue + } + if err != nil { + return nil, err + } + if paid { + blockers = append(blockers, paidSubscriptionBlocker(sub)) + } + } + return blockers, nil +} + +func unresolvablePlanBlocker(sub subscription.Subscription) Blocker { + return Blocker{ + Type: BlockerActiveSubscription, + Subject: sub.ID, + Message: fmt.Sprintf("subscription[%s] is %s on a plan that cannot be resolved: cancel the subscription, then retry the delete", sub.ID, sub.State), + } +} + +func paidSubscriptionBlocker(sub subscription.Subscription) Blocker { + return Blocker{ + Type: BlockerActiveSubscription, + Subject: sub.ID, + Message: fmt.Sprintf("subscription[%s] is %s on a paid plan: downgrade to the standard plan, then retry the delete", sub.ID, sub.State), + } +} + +// isPaidPlan reports whether the plan charges money, caching each plan for +// the length of one delete. Callers handle a subscription without a plan +// before coming here. +func (d Service) isPaidPlan(ctx context.Context, planID string, cache map[string]bool) (bool, error) { + if paid, ok := cache[planID]; ok { + return paid, nil + } + pln, err := d.planService.GetByID(ctx, planID) + if err != nil { + return false, fmt.Errorf("failed to resolve plan[%s]: %w", planID, err) + } + cache[planID] = !pln.IsFree() + return cache[planID], nil +} + +// invoiceBlockers returns a blocker for every invoice of the account that +// still asks for money. Open and uncollectible invoices the caller can pay. +// A draft is money the provider is still preparing to charge — the provider +// finalizes it shortly — and deleting before that would silently lose the +// charge, so it blocks too. The answer comes straight from the billing +// provider, so a just-paid invoice does not block and a just-created one +// does. +func (d Service) invoiceBlockers(ctx context.Context, c customer.Customer) ([]Blocker, error) { + invoices, err := d.invoiceService.ListPayableOnProvider(ctx, c) + if err != nil { + return nil, fmt.Errorf("failed to check invoices for billing account[%s]: %w", c.ID, err) + } + var blockers []Blocker + for _, inv := range invoices { + // an invoice the sync has not stored yet carries no local id + subject := inv.ID + if subject == "" { + subject = inv.ProviderID + } + message := fmt.Sprintf("invoice[%s] is unpaid: pay it via its hosted payment page, then retry the delete", subject) + if inv.State == invoice.DraftState { + message = fmt.Sprintf("invoice[%s] is still being prepared by the billing provider: retry the delete once it finalizes, then pay it", subject) + } + blockers = append(blockers, Blocker{ + Type: BlockerUnpaidInvoice, + Subject: subject, + Message: message, + }) + } + return blockers, nil +} diff --git a/core/deleter/service_test.go b/core/deleter/service_test.go index 2e96132838..1a22ba0f15 100644 --- a/core/deleter/service_test.go +++ b/core/deleter/service_test.go @@ -9,10 +9,14 @@ import ( "github.com/raystack/frontier/billing/checkout" "github.com/raystack/frontier/billing/customer" "github.com/raystack/frontier/billing/invoice" + "github.com/raystack/frontier/billing/plan" + "github.com/raystack/frontier/billing/product" + "github.com/raystack/frontier/billing/subscription" "github.com/raystack/frontier/core/deleter" "github.com/raystack/frontier/core/deleter/mocks" "github.com/raystack/frontier/core/group" "github.com/raystack/frontier/core/invitation" + "github.com/raystack/frontier/core/organization" "github.com/raystack/frontier/core/policy" "github.com/raystack/frontier/core/project" "github.com/raystack/frontier/core/resource" @@ -41,11 +45,12 @@ type deleterMocks struct { checkoutSvc *mocks.CheckoutService creditSvc *mocks.CreditService kycSvc *mocks.KycService + planSvc *mocks.PlanService } func newMocks(t *testing.T) deleterMocks { t.Helper() - return deleterMocks{ + m := deleterMocks{ orgSvc: mocks.NewOrganizationService(t), projSvc: mocks.NewProjectService(t), resSvc: mocks.NewResourceService(t), @@ -63,13 +68,24 @@ func newMocks(t *testing.T) deleterMocks { checkoutSvc: mocks.NewCheckoutService(t), creditSvc: mocks.NewCreditService(t), kycSvc: mocks.NewKycService(t), + planSvc: mocks.NewPlanService(t), } + // plans resolve lazily by the subscription's plan id; stub a paid and a + // free one for every test + m.planSvc.EXPECT().GetByID(mock.Anything, "plan-paid"). + Return(plan.Plan{ID: "plan-paid", Products: []product.Product{ + {Prices: []product.Price{{Amount: 500}}}, + }}, nil).Maybe() + m.planSvc.EXPECT().GetByID(mock.Anything, "plan-free"). + Return(plan.Plan{ID: "plan-free"}, nil).Maybe() + return m } func (m deleterMocks) build() *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.suSvc, - m.custSvc, m.subSvc, m.invocSvc, m.checkoutSvc, m.creditSvc, m.kycSvc) + m.custSvc, m.subSvc, m.invocSvc, m.checkoutSvc, m.creditSvc, m.kycSvc, + m.planSvc) } func TestDeleteProject(t *testing.T) { @@ -130,12 +146,18 @@ func TestDeleteOrganization(t *testing.T) { t.Run("full cascade delete", func(t *testing.T) { m := newMocks(t) - // canDelete and DeleteCustomers both list customers + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + + // the up-front check and DeleteCustomers both list customers 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.invocSvc.EXPECT().List(mock.Anything, invoice.Filter{CustomerID: "cust-1"}). + m.invocSvc.EXPECT().ListPayableOnProvider(mock.Anything, c). Return([]invoice.Invoice{}, nil) + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(0, nil) + m.subSvc.EXPECT().List(mock.Anything, subscription.Filter{CustomerID: "cust-1"}). + Return([]subscription.Subscription{{ID: "sub-1", State: "canceled"}}, nil) // billing teardown m.subSvc.EXPECT().DeleteByCustomer(mock.Anything, c).Return(nil) @@ -192,21 +214,365 @@ func TestDeleteOrganization(t *testing.T) { assert.NoError(t, err) }) - t.Run("blocked when billed customer has invoices", func(t *testing.T) { + t.Run("already deleted org returns not found without touching anything", func(t *testing.T) { + m := newMocks(t) + + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{}, organization.ErrNotExist) + // strict mocks: no other service may be called + + err := m.build().DeleteOrganization(context.Background(), "org-1") + assert.ErrorIs(t, err, organization.ErrNotExist) + }) + + t.Run("collects all blockers in one error", func(t *testing.T) { + m := newMocks(t) + + c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"} + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{c}, nil) + m.subSvc.EXPECT().List(mock.Anything, subscription.Filter{CustomerID: "cust-1"}). + Return([]subscription.Subscription{ + {ID: "sub-1", State: "active", PlanID: "plan-paid"}, + {ID: "sub-2", State: "canceled", PlanID: "plan-paid"}, + }, nil) + m.invocSvc.EXPECT().ListPayableOnProvider(mock.Anything, c). + Return([]invoice.Invoice{{ID: "inv-1", State: invoice.OpenState}}, nil) + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(-50, nil) + // strict mocks: nothing may be deleted and no subscription may be + // canceled on a blocked delete + + err := m.build().DeleteOrganization(context.Background(), "org-1") + + var blocked *deleter.BlockedError + assert.ErrorAs(t, err, &blocked) + assert.Equal(t, "org-1", blocked.OrgID) + types := make([]string, 0, len(blocked.Blockers)) + for _, b := range blocked.Blockers { + types = append(types, b.Type) + } + assert.Equal(t, []string{ + deleter.BlockerActiveSubscription, + deleter.BlockerUnpaidInvoice, + deleter.BlockerNegativeTokenBalance, + }, types) + }) + + t.Run("paid plan subscription blocks until the caller downgrades", func(t *testing.T) { + m := newMocks(t) + + c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"} + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{c}, nil) + m.subSvc.EXPECT().List(mock.Anything, subscription.Filter{CustomerID: "cust-1"}). + Return([]subscription.Subscription{{ID: "sub-1", State: "active", PlanID: "plan-paid"}}, nil) + m.invocSvc.EXPECT().ListPayableOnProvider(mock.Anything, c). + Return([]invoice.Invoice{}, nil) + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(0, nil) + // strict mocks: the paid subscription must not be canceled + + err := m.build().DeleteOrganization(context.Background(), "org-1") + + var blocked *deleter.BlockedError + assert.ErrorAs(t, err, &blocked) + assert.Len(t, blocked.Blockers, 1) + assert.Equal(t, deleter.BlockerActiveSubscription, blocked.Blockers[0].Type) + assert.Contains(t, blocked.Blockers[0].Message, "downgrade to the standard plan") + }) + + t.Run("negative token balance blocks the delete", func(t *testing.T) { + m := newMocks(t) + + c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"} + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{c}, nil) + m.subSvc.EXPECT().List(mock.Anything, subscription.Filter{CustomerID: "cust-1"}). + Return([]subscription.Subscription{}, nil) + m.invocSvc.EXPECT().ListPayableOnProvider(mock.Anything, c). + Return([]invoice.Invoice{}, nil) + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(-50, nil) + + err := m.build().DeleteOrganization(context.Background(), "org-1") + + var blocked *deleter.BlockedError + assert.ErrorAs(t, err, &blocked) + assert.Len(t, blocked.Blockers, 1) + assert.Equal(t, deleter.BlockerNegativeTokenBalance, blocked.Blockers[0].Type) + assert.Contains(t, blocked.Blockers[0].Message, "contact support") + }) + + t.Run("unused tokens do not block the delete", func(t *testing.T) { + m := newMocks(t) + + c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"} + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{c}, nil) + m.invocSvc.EXPECT().ListPayableOnProvider(mock.Anything, c). + Return([]invoice.Invoice{}, nil) + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(100, nil) + m.subSvc.EXPECT().List(mock.Anything, subscription.Filter{CustomerID: "cust-1"}). + Return([]subscription.Subscription{}, 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().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"}). + Return([]project.Project{}, nil) + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + Return([]group.Group{}, nil) + m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). + Return([]serviceuser.ServiceUser{}, nil) + m.invSvc.EXPECT().List(mock.Anything, invitation.Filter{OrgID: "org-1"}). + Return([]invitation.Invitation{}, nil) + m.kycSvc.EXPECT().DeleteKyc(mock.Anything, "org-1").Return(nil) + m.polSvc.EXPECT().List(mock.Anything, policy.Filter{OrgID: "org-1"}). + Return([]policy.Policy{}, nil) + m.roleSvc.EXPECT().List(mock.Anything, role.Filter{OrgID: "org-1"}). + Return([]role.Role{}, nil) + m.orgSvc.EXPECT().DeleteModel(mock.Anything, "org-1").Return(nil) + + err := m.build().DeleteOrganization(context.Background(), "org-1") + assert.NoError(t, err) + }) + + t.Run("running subscription is canceled by the delete", func(t *testing.T) { + m := newMocks(t) + + c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"} + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{c}, nil) + m.invocSvc.EXPECT().ListPayableOnProvider(mock.Anything, c). + Return([]invoice.Invoice{}, nil) + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(0, nil) + m.subSvc.EXPECT().List(mock.Anything, subscription.Filter{CustomerID: "cust-1"}). + Return([]subscription.Subscription{ + {ID: "sub-1", State: "active", PlanID: "plan-free"}, + {ID: "sub-2", State: "canceled", PlanID: "plan-paid"}, + }, nil) + // only the running standard-plan subscription is canceled, immediately + m.subSvc.EXPECT().Cancel(mock.Anything, "sub-1", true). + Return(subscription.Subscription{ID: "sub-1", State: "canceled"}, 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().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"}). + Return([]project.Project{}, nil) + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + Return([]group.Group{}, nil) + m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). + Return([]serviceuser.ServiceUser{}, nil) + m.invSvc.EXPECT().List(mock.Anything, invitation.Filter{OrgID: "org-1"}). + Return([]invitation.Invitation{}, nil) + m.kycSvc.EXPECT().DeleteKyc(mock.Anything, "org-1").Return(nil) + m.polSvc.EXPECT().List(mock.Anything, policy.Filter{OrgID: "org-1"}). + Return([]policy.Policy{}, nil) + m.roleSvc.EXPECT().List(mock.Anything, role.Filter{OrgID: "org-1"}). + Return([]role.Role{}, nil) + m.orgSvc.EXPECT().DeleteModel(mock.Anything, "org-1").Return(nil) + + err := m.build().DeleteOrganization(context.Background(), "org-1") + assert.NoError(t, err) + }) + + t.Run("final invoice from the cancellation blocks the delete", func(t *testing.T) { m := newMocks(t) + c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"} + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). - Return([]customer.Customer{{ID: "cust-1", ProviderID: "stripe-1"}}, nil) - m.invocSvc.EXPECT().List(mock.Anything, invoice.Filter{CustomerID: "cust-1"}). - Return([]invoice.Invoice{{ID: "inv-1"}}, nil) + Return([]customer.Customer{c}, nil) + // no unpaid invoice before the cancel, one after it + m.invocSvc.EXPECT().ListPayableOnProvider(mock.Anything, c). + Return([]invoice.Invoice{}, nil).Once() + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(0, nil) + m.subSvc.EXPECT().List(mock.Anything, subscription.Filter{CustomerID: "cust-1"}). + Return([]subscription.Subscription{{ID: "sub-1", State: "active", PlanID: "plan-free"}}, nil) + m.subSvc.EXPECT().Cancel(mock.Anything, "sub-1", true). + Return(subscription.Subscription{ID: "sub-1", State: "canceled"}, nil) + m.invocSvc.EXPECT().ListPayableOnProvider(mock.Anything, c). + Return([]invoice.Invoice{{ID: "inv-final", State: invoice.OpenState}}, nil).Once() + // strict mocks: nothing may be deleted err := m.build().DeleteOrganization(context.Background(), "org-1") - assert.ErrorIs(t, err, deleter.ErrDeleteNotAllowed) + + var blocked *deleter.BlockedError + assert.ErrorAs(t, err, &blocked) + assert.Len(t, blocked.Blockers, 1) + assert.Equal(t, deleter.BlockerUnpaidInvoice, blocked.Blockers[0].Type) + assert.Equal(t, "inv-final", blocked.Blockers[0].Subject) + }) + + t.Run("draft invoice blocks until the provider finalizes it", func(t *testing.T) { + m := newMocks(t) + + c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"} + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{c}, nil) + m.subSvc.EXPECT().List(mock.Anything, subscription.Filter{CustomerID: "cust-1"}). + Return([]subscription.Subscription{}, nil) + // a renewal invoice the provider has not finalized yet, no local row + m.invocSvc.EXPECT().ListPayableOnProvider(mock.Anything, c). + Return([]invoice.Invoice{{ProviderID: "in_draft1", State: invoice.DraftState, Amount: 500}}, nil) + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(0, nil) + // strict mocks: nothing may be deleted + + err := m.build().DeleteOrganization(context.Background(), "org-1") + + var blocked *deleter.BlockedError + assert.ErrorAs(t, err, &blocked) + assert.Len(t, blocked.Blockers, 1) + assert.Equal(t, deleter.BlockerUnpaidInvoice, blocked.Blockers[0].Type) + assert.Equal(t, "in_draft1", blocked.Blockers[0].Subject) + assert.Contains(t, blocked.Blockers[0].Message, "being prepared") + }) + + t.Run("subscription gone on the provider does not brick the delete", func(t *testing.T) { + m := newMocks(t) + + c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"} + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{c}, nil) + m.invocSvc.EXPECT().ListPayableOnProvider(mock.Anything, c). + Return([]invoice.Invoice{}, nil) + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(0, nil) + // locally active free-plan sub whose Stripe copy no longer exists + m.subSvc.EXPECT().List(mock.Anything, subscription.Filter{CustomerID: "cust-1"}). + Return([]subscription.Subscription{{ID: "sub-1", State: "active", PlanID: "plan-free"}}, nil) + m.subSvc.EXPECT().Cancel(mock.Anything, "sub-1", true). + Return(subscription.Subscription{}, subscription.ErrSubscriptionOnProviderNotFound) + + 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().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"}). + Return([]project.Project{}, nil) + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + Return([]group.Group{}, nil) + m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). + Return([]serviceuser.ServiceUser{}, nil) + m.invSvc.EXPECT().List(mock.Anything, invitation.Filter{OrgID: "org-1"}). + Return([]invitation.Invitation{}, nil) + m.kycSvc.EXPECT().DeleteKyc(mock.Anything, "org-1").Return(nil) + m.polSvc.EXPECT().List(mock.Anything, policy.Filter{OrgID: "org-1"}). + Return([]policy.Policy{}, nil) + m.roleSvc.EXPECT().List(mock.Anything, role.Filter{OrgID: "org-1"}). + Return([]role.Role{}, nil) + m.orgSvc.EXPECT().DeleteModel(mock.Anything, "org-1").Return(nil) + + err := m.build().DeleteOrganization(context.Background(), "org-1") + assert.NoError(t, err) + }) + + t.Run("paid subscription appearing between the passes blocks instead of being canceled", func(t *testing.T) { + m := newMocks(t) + + c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"} + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{c}, nil) + m.invocSvc.EXPECT().ListPayableOnProvider(mock.Anything, c). + Return([]invoice.Invoice{}, nil) + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(0, nil) + // first pass sees nothing, a paid checkout completes in between + m.subSvc.EXPECT().List(mock.Anything, subscription.Filter{CustomerID: "cust-1"}). + Return([]subscription.Subscription{}, nil).Once() + m.subSvc.EXPECT().List(mock.Anything, subscription.Filter{CustomerID: "cust-1"}). + Return([]subscription.Subscription{{ID: "sub-new", State: "active", PlanID: "plan-paid"}}, nil).Once() + // strict mocks: the paid subscription must not be canceled and + // nothing may be deleted + + err := m.build().DeleteOrganization(context.Background(), "org-1") + + var blocked *deleter.BlockedError + assert.ErrorAs(t, err, &blocked) + assert.Len(t, blocked.Blockers, 1) + assert.Equal(t, deleter.BlockerActiveSubscription, blocked.Blockers[0].Type) + assert.Equal(t, "sub-new", blocked.Blockers[0].Subject) + }) + + t.Run("dangling plan reference blocks instead of bricking the delete", func(t *testing.T) { + m := newMocks(t) + + c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"} + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{c}, nil) + m.subSvc.EXPECT().List(mock.Anything, subscription.Filter{CustomerID: "cust-1"}). + Return([]subscription.Subscription{{ID: "sub-1", State: "active", PlanID: "plan-gone"}}, nil) + m.planSvc.EXPECT().GetByID(mock.Anything, "plan-gone"). + Return(plan.Plan{}, plan.ErrNotFound) + m.invocSvc.EXPECT().ListPayableOnProvider(mock.Anything, c). + Return([]invoice.Invoice{}, nil) + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(0, nil) + // strict mocks: nothing may be canceled or deleted + + err := m.build().DeleteOrganization(context.Background(), "org-1") + + var blocked *deleter.BlockedError + assert.ErrorAs(t, err, &blocked) + assert.Len(t, blocked.Blockers, 1) + assert.Equal(t, deleter.BlockerActiveSubscription, blocked.Blockers[0].Type) + assert.Contains(t, blocked.Blockers[0].Message, "cannot be resolved") + }) + + t.Run("offline account only gets token checks", func(t *testing.T) { + m := newMocks(t) + + c := customer.Customer{ID: "cust-offline", ProviderID: ""} + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{c}, nil) + // strict mocks: no subscription or invoice call may happen + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-offline").Return(-10, nil) + + err := m.build().DeleteOrganization(context.Background(), "org-1") + + var blocked *deleter.BlockedError + assert.ErrorAs(t, err, &blocked) + assert.Len(t, blocked.Blockers, 1) + assert.Equal(t, deleter.BlockerNegativeTokenBalance, blocked.Blockers[0].Type) }) t.Run("kyc delete failure keeps owner policies", func(t *testing.T) { m := newMocks(t) + // a disabled org is still deletable + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{}, organization.ErrDisabled) m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). Return([]customer.Customer{}, nil) m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). @@ -229,10 +595,15 @@ func TestDeleteOrganization(t *testing.T) { m := newMocks(t) c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"} + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). Return([]customer.Customer{c}, nil) - m.invocSvc.EXPECT().List(mock.Anything, invoice.Filter{CustomerID: "cust-1"}). + m.invocSvc.EXPECT().ListPayableOnProvider(mock.Anything, c). Return([]invoice.Invoice{}, nil) + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(0, nil) + m.subSvc.EXPECT().List(mock.Anything, subscription.Filter{CustomerID: "cust-1"}). + Return([]subscription.Subscription{}, nil) m.subSvc.EXPECT().DeleteByCustomer(mock.Anything, c). Return(errors.New("provider is down")) // strict mocks: no policy, project, group, or org deletion may happen @@ -244,6 +615,8 @@ func TestDeleteOrganization(t *testing.T) { t.Run("propagates error when service user list fails", func(t *testing.T) { m := newMocks(t) + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). Return([]customer.Customer{}, nil) m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). @@ -260,6 +633,8 @@ func TestDeleteOrganization(t *testing.T) { t.Run("propagates error when service user delete fails", func(t *testing.T) { m := newMocks(t) + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). Return([]customer.Customer{}, nil) m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). @@ -291,6 +666,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.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(100, nil) m.creditSvc.EXPECT().DeleteByAccountID(mock.Anything, "cust-1").Return(nil) m.custSvc.EXPECT().Delete(mock.Anything, "cust-1").Return(nil) @@ -298,6 +674,25 @@ func TestDeleteCustomers(t *testing.T) { 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) @@ -309,6 +704,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) diff --git a/go.mod b/go.mod index 6c822de1a9..413d9e8673 100644 --- a/go.mod +++ b/go.mod @@ -51,6 +51,7 @@ require ( golang.org/x/oauth2 v0.36.0 golang.org/x/sync v0.22.0 google.golang.org/genproto/googleapis/api v0.0.0-20260414002931-afd174a4e478 + google.golang.org/genproto/googleapis/rpc v0.0.0-20260414002931-afd174a4e478 google.golang.org/grpc v1.82.1 google.golang.org/protobuf v1.36.11 gopkg.in/dnaeon/go-vcr.v3 v3.1.2 @@ -133,7 +134,6 @@ require ( golang.org/x/time v0.5.0 // indirect golang.org/x/tools v0.47.0 // indirect google.golang.org/genproto v0.0.0-20240401170217-c3f982113cda // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20260414002931-afd174a4e478 // indirect gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc // indirect gopkg.in/square/go-jose.v2 v2.6.0 // indirect ) diff --git a/internal/api/v1beta1connect/deleter.go b/internal/api/v1beta1connect/deleter.go index 6d14cfbb3f..4cf66c6b35 100644 --- a/internal/api/v1beta1connect/deleter.go +++ b/internal/api/v1beta1connect/deleter.go @@ -2,10 +2,15 @@ package v1beta1connect import ( "context" + "errors" "fmt" + "log/slog" "connectrpc.com/connect" + "github.com/raystack/frontier/core/deleter" + "github.com/raystack/frontier/core/organization" frontierv1beta1 "github.com/raystack/frontier/proto/v1beta1" + "google.golang.org/genproto/googleapis/rpc/errdetails" ) func (h *ConnectHandler) DeleteProject(ctx context.Context, request *connect.Request[frontierv1beta1.DeleteProjectRequest]) (*connect.Response[frontierv1beta1.DeleteProjectResponse], error) { @@ -17,7 +22,36 @@ func (h *ConnectHandler) DeleteProject(ctx context.Context, request *connect.Req func (h *ConnectHandler) DeleteOrganization(ctx context.Context, request *connect.Request[frontierv1beta1.DeleteOrganizationRequest]) (*connect.Response[frontierv1beta1.DeleteOrganizationResponse], error) { if err := h.deleterService.DeleteOrganization(ctx, request.Msg.GetId()); err != nil { + var blocked *deleter.BlockedError + if errors.As(err, &blocked) { + return nil, deleteBlockedError(ctx, blocked) + } + if errors.Is(err, organization.ErrNotExist) || errors.Is(err, organization.ErrInvalidUUID) || errors.Is(err, organization.ErrInvalidID) { + return nil, connect.NewError(connect.CodeNotFound, organization.ErrNotExist) + } return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("DeleteOrganization.DeleteOrganization: organization_id=%s: %w", request.Msg.GetId(), err)) } return connect.NewResponse(&frontierv1beta1.DeleteOrganizationResponse{}), nil } + +// deleteBlockedError turns the delete's blockers into a failed_precondition +// error carrying one PreconditionFailure violation per blocker, so a caller +// sees everything to fix in a single response. +func deleteBlockedError(ctx context.Context, blocked *deleter.BlockedError) *connect.Error { + connectErr := connect.NewError(connect.CodeFailedPrecondition, blocked) + failure := &errdetails.PreconditionFailure{} + for _, b := range blocked.Blockers { + failure.Violations = append(failure.Violations, &errdetails.PreconditionFailure_Violation{ + Type: b.Type, + Subject: b.Subject, + Description: b.Message, + }) + } + if detail, err := connect.NewErrorDetail(failure); err != nil { + slog.WarnContext(ctx, "failed to attach precondition failure details", "error", err, "org_id", blocked.OrgID) + } else { + connectErr.AddDetail(detail) + } + slog.WarnContext(ctx, "organization delete blocked", "org_id", blocked.OrgID, "blockers", len(blocked.Blockers)) + return connectErr +} diff --git a/internal/api/v1beta1connect/deleter_test.go b/internal/api/v1beta1connect/deleter_test.go index 351769c9e7..3898fbf5c7 100644 --- a/internal/api/v1beta1connect/deleter_test.go +++ b/internal/api/v1beta1connect/deleter_test.go @@ -6,11 +6,14 @@ import ( "testing" "connectrpc.com/connect" + "github.com/raystack/frontier/core/deleter" + "github.com/raystack/frontier/core/organization" "github.com/raystack/frontier/internal/api/v1beta1connect/mocks" "github.com/raystack/frontier/pkg/errors" frontierv1beta1 "github.com/raystack/frontier/proto/v1beta1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "google.golang.org/genproto/googleapis/rpc/errdetails" ) func TestHandler_DeleteProject(t *testing.T) { @@ -77,6 +80,17 @@ func TestHandler_DeleteOrganization(t *testing.T) { want: connect.NewResponse(&frontierv1beta1.DeleteOrganizationResponse{}), wantErr: nil, }, + { + name: "should return not found when the org is already gone", + setup: func(as *mocks.CascadeDeleter) { + as.EXPECT().DeleteOrganization(mock.Anything, "some-id").Return(organization.ErrNotExist) + }, + request: connect.NewRequest(&frontierv1beta1.DeleteOrganizationRequest{ + Id: "some-id", + }), + want: nil, + wantErr: connect.NewError(connect.CodeNotFound, organization.ErrNotExist), + }, { name: "should return error if deleter service encounters an error", setup: func(as *mocks.CascadeDeleter) { @@ -101,4 +115,39 @@ func TestHandler_DeleteOrganization(t *testing.T) { assert.Equal(t, tt.wantErr, err) }) } + + t.Run("should return failed precondition with one violation per blocker when the delete is blocked", func(t *testing.T) { + blocked := &deleter.BlockedError{ + OrgID: "some-id", + Blockers: []deleter.Blocker{ + {Type: deleter.BlockerUnpaidInvoice, Subject: "inv-1", Message: "invoice[inv-1] is unpaid: pay it via its hosted payment page, then retry the delete"}, + {Type: deleter.BlockerNegativeTokenBalance, Subject: "cust-1", Message: "billing account[cust-1] owes 50 tokens: contact support to settle the balance, then retry the delete"}, + }, + } + mockDelOrg := new(mocks.CascadeDeleter) + mockDelOrg.EXPECT().DeleteOrganization(mock.Anything, "some-id").Return(blocked) + mockDep := &ConnectHandler{deleterService: mockDelOrg} + + resp, err := mockDep.DeleteOrganization(context.Background(), connect.NewRequest(&frontierv1beta1.DeleteOrganizationRequest{ + Id: "some-id", + })) + assert.Nil(t, resp) + + var connectErr *connect.Error + assert.ErrorAs(t, err, &connectErr) + assert.Equal(t, connect.CodeFailedPrecondition, connectErr.Code()) + assert.Contains(t, connectErr.Message(), "pay it") + assert.Contains(t, connectErr.Message(), "contact support") + + assert.Len(t, connectErr.Details(), 1) + detail, detailErr := connectErr.Details()[0].Value() + assert.NoError(t, detailErr) + failure, ok := detail.(*errdetails.PreconditionFailure) + assert.True(t, ok) + assert.Len(t, failure.GetViolations(), 2) + assert.Equal(t, deleter.BlockerUnpaidInvoice, failure.GetViolations()[0].GetType()) + assert.Equal(t, "inv-1", failure.GetViolations()[0].GetSubject()) + assert.Equal(t, deleter.BlockerNegativeTokenBalance, failure.GetViolations()[1].GetType()) + assert.Equal(t, "cust-1", failure.GetViolations()[1].GetSubject()) + }) }