From 9f0a1558620e53ad31bd31df12094cbd18529563 Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Tue, 18 Aug 2026 10:09:31 +0530 Subject: [PATCH] feat(deleter): serve CheckOrganizationDelete for delete eligibility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clients need to know whether an org delete would go through before the user tries it — for example to grey out the delete button and say why. CheckOrganizationDelete returns the same blocker list the delete's failed_precondition carries (paid-plan subscription, unpaid invoice, negative token balance) without changing anything: no subscription gets canceled and the invoice rows are read as they are. The answer is advisory; DeleteOrganization still re-checks against fresh provider data. The RPC requires the same delete permission on the org. --- core/deleter/deleter.go | 11 ++ core/deleter/service.go | 144 ++++++++++++------ core/deleter/service_test.go | 70 +++++++++ internal/api/v1beta1connect/deleter.go | 25 +++ internal/api/v1beta1connect/deleter_test.go | 48 ++++++ internal/api/v1beta1connect/interfaces.go | 2 + .../v1beta1connect/mocks/cascade_deleter.go | 60 ++++++++ .../connect_interceptors/authorization.go | 4 + 8 files changed, 317 insertions(+), 47 deletions(-) diff --git a/core/deleter/deleter.go b/core/deleter/deleter.go index 63eea45d4d..a272ffe67d 100644 --- a/core/deleter/deleter.go +++ b/core/deleter/deleter.go @@ -11,12 +11,23 @@ const ( BlockerNegativeTokenBalance = "NEGATIVE_TOKEN_BALANCE" ) +// Kinds of entity a Blocker's Subject id refers to. They match the naming +// the audit records use for billing entities. +const ( + SubjectSubscription = "billing_subscription" + SubjectInvoice = "billing_invoice" + SubjectBillingAccount = "billing_account" +) + // 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 + // SubjectType is one of the Subject* constants and says what kind of + // entity the Subject id refers to. + SubjectType string // Message says what blocks the delete and how to clear it. Message string } diff --git a/core/deleter/service.go b/core/deleter/service.go index c2f11b0fdf..3d441856f7 100644 --- a/core/deleter/service.go +++ b/core/deleter/service.go @@ -517,41 +517,10 @@ func (d Service) ensureDeletable(ctx context.Context, id string) error { return err } - // 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), - }) - } + // the delete is about to happen, so judge invoices on fresh provider data + blockers, err := d.collectBlockers(ctx, customers, true) + if err != nil { + return err } if len(blockers) > 0 { return &BlockedError{OrgID: id, Blockers: blockers} @@ -563,6 +532,7 @@ func (d Service) ensureDeletable(ctx context.Context, id string) error { // runs only after every blocker is clear and never touches a paid // subscription, so a blocked delete never costs the caller a plan they // pay for + paidPlans := map[string]bool{} for _, c := range customers { if c.IsOffline() { continue @@ -594,7 +564,7 @@ func (d Service) ensureDeletable(ctx context.Context, id string) error { canceled = true } if canceled { - bs, err := d.invoiceBlockers(ctx, c) + bs, err := d.invoiceBlockers(ctx, c, true) if err != nil { return err } @@ -607,6 +577,69 @@ func (d Service) ensureDeletable(ctx context.Context, id string) error { return nil } +// CheckOrganizationDelete reports everything that currently blocks deleting +// the organization, without changing anything: no subscription gets canceled +// and the invoice rows are read as they are (they sync from the provider on +// a timer). The answer is advisory — DeleteOrganization re-checks against +// fresh provider data before actually deleting. +func (d Service) CheckOrganizationDelete(ctx context.Context, id string) ([]Blocker, error) { + if _, err := d.orgService.GetRaw(ctx, id); err != nil { + return nil, err + } + customers, err := d.customerService.List(ctx, customer.Filter{ + OrgID: id, + }) + if err != nil { + return nil, err + } + return d.collectBlockers(ctx, customers, false) +} + +// collectBlockers gathers the blockers across the org's billing accounts: +// running subscriptions on a paid plan, invoices that still ask for money, +// and token debt. fromProvider judges the invoices on data read straight +// from the billing provider instead of the local rows. +func (d Service) collectBlockers(ctx context.Context, customers []customer.Customer, fromProvider bool) ([]Blocker, 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 nil, err + } + blockers = append(blockers, bs...) + + bs, err = d.invoiceBlockers(ctx, c, fromProvider) + if err != nil { + return nil, err + } + blockers = append(blockers, bs...) + } + + balance, err := d.creditService.GetBalance(ctx, c.ID) + if err != nil { + return nil, 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, + SubjectType: SubjectBillingAccount, + Message: fmt.Sprintf("billing account[%s] owes %d tokens: contact support to settle the balance, then retry the delete", c.ID, -balance), + }) + } + } + return blockers, 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. @@ -633,9 +666,10 @@ func (d Service) subscriptionBlockers(ctx context.Context, c customer.Customer, 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), + Type: BlockerActiveSubscription, + Subject: sub.ID, + SubjectType: SubjectSubscription, + Message: fmt.Sprintf("subscription[%s] is %s on a paid plan: downgrade to the standard plan, then retry the delete", sub.ID, sub.State), } } @@ -661,14 +695,29 @@ func (d Service) isPaidPlan(ctx context.Context, planID string, cache map[string // 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) +// charge, so it blocks too. +// +// fromProvider reads the invoices straight from the billing provider, so a +// just-paid invoice does not block a delete and a just-created one does. +// Without it the local rows answer, cheap enough for every check call. +func (d Service) invoiceBlockers(ctx context.Context, c customer.Customer, fromProvider bool) ([]Blocker, error) { + var invoices []invoice.Invoice + var err error + if fromProvider { + invoices, err = d.invoiceService.ListPayableOnProvider(ctx, c) + } else { + var all []invoice.Invoice + all, err = d.invoiceService.List(ctx, invoice.Filter{CustomerID: c.ID, NonZeroOnly: true}) + for _, inv := range all { + if inv.State == invoice.DraftState || inv.State == invoice.OpenState || inv.State == invoice.UncollectibleState { + invoices = append(invoices, inv) + } + } + } 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 @@ -681,9 +730,10 @@ func (d Service) invoiceBlockers(ctx context.Context, c customer.Customer) ([]Bl 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, + Type: BlockerUnpaidInvoice, + Subject: subject, + SubjectType: SubjectInvoice, + Message: message, }) } return blockers, nil diff --git a/core/deleter/service_test.go b/core/deleter/service_test.go index 4211d6a5ac..78464b64f2 100644 --- a/core/deleter/service_test.go +++ b/core/deleter/service_test.go @@ -671,6 +671,76 @@ func TestDeleteOrganization(t *testing.T) { }) } +func TestCheckOrganizationDelete(t *testing.T) { + t.Run("reports every blocker without changing anything", func(t *testing.T) { + m := newMocks(t) + + c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"} + m.orgSvc.EXPECT().GetRaw(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().List(mock.Anything, invoice.Filter{CustomerID: "cust-1", NonZeroOnly: true}). + Return([]invoice.Invoice{{ID: "inv-1", State: invoice.OpenState}}, nil) + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(-50, nil) + // strict mocks: the check must not sync invoices, cancel + // subscriptions, or delete anything + + blockers, err := m.build().CheckOrganizationDelete(context.Background(), "org-1") + assert.NoError(t, err) + + types := make([]string, 0, len(blockers)) + subjectTypes := make([]string, 0, len(blockers)) + for _, b := range blockers { + types = append(types, b.Type) + subjectTypes = append(subjectTypes, b.SubjectType) + } + assert.Equal(t, []string{ + deleter.BlockerActiveSubscription, + deleter.BlockerUnpaidInvoice, + deleter.BlockerNegativeTokenBalance, + }, types) + assert.Equal(t, []string{ + deleter.SubjectSubscription, + deleter.SubjectInvoice, + deleter.SubjectBillingAccount, + }, subjectTypes) + }) + + t.Run("returns empty when there are no blockers", func(t *testing.T) { + m := newMocks(t) + + c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"} + m.orgSvc.EXPECT().GetRaw(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-free"}}, nil) + m.invocSvc.EXPECT().List(mock.Anything, invoice.Filter{CustomerID: "cust-1", NonZeroOnly: true}). + Return([]invoice.Invoice{}, nil) + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(100, nil) + // a standard-plan subscription and unused tokens don't block, and the + // check must not cancel the subscription + + blockers, err := m.build().CheckOrganizationDelete(context.Background(), "org-1") + assert.NoError(t, err) + assert.Empty(t, blockers) + }) + + t.Run("missing org surfaces not found", func(t *testing.T) { + m := newMocks(t) + + m.orgSvc.EXPECT().GetRaw(mock.Anything, "org-1"). + Return(organization.Organization{}, organization.ErrNotExist) + + _, err := m.build().CheckOrganizationDelete(context.Background(), "org-1") + assert.ErrorIs(t, err, organization.ErrNotExist) + }) +} + func TestDeleteCustomers(t *testing.T) { t.Run("deletes subscriptions invoices checkouts transactions and customer", func(t *testing.T) { m := newMocks(t) diff --git a/internal/api/v1beta1connect/deleter.go b/internal/api/v1beta1connect/deleter.go index 4cf66c6b35..94cc1dcaab 100644 --- a/internal/api/v1beta1connect/deleter.go +++ b/internal/api/v1beta1connect/deleter.go @@ -34,6 +34,31 @@ func (h *ConnectHandler) DeleteOrganization(ctx context.Context, request *connec return connect.NewResponse(&frontierv1beta1.DeleteOrganizationResponse{}), nil } +// CheckOrganizationDelete reports what currently blocks deleting the org +// without changing anything, so a client can disable its delete control and +// say why before the user ever tries. +func (h *ConnectHandler) CheckOrganizationDelete(ctx context.Context, request *connect.Request[frontierv1beta1.CheckOrganizationDeleteRequest]) (*connect.Response[frontierv1beta1.CheckOrganizationDeleteResponse], error) { + blockers, err := h.deleterService.CheckOrganizationDelete(ctx, request.Msg.GetId()) + if err != nil { + 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("CheckOrganizationDelete: organization_id=%s: %w", request.Msg.GetId(), err)) + } + resp := &frontierv1beta1.CheckOrganizationDeleteResponse{ + CanDelete: len(blockers) == 0, + } + for _, b := range blockers { + resp.Blockers = append(resp.Blockers, &frontierv1beta1.CheckOrganizationDeleteResponse_Blocker{ + Type: b.Type, + Subject: b.Subject, + SubjectType: b.SubjectType, + Message: b.Message, + }) + } + return connect.NewResponse(resp), 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. diff --git a/internal/api/v1beta1connect/deleter_test.go b/internal/api/v1beta1connect/deleter_test.go index 3898fbf5c7..ef3e60c613 100644 --- a/internal/api/v1beta1connect/deleter_test.go +++ b/internal/api/v1beta1connect/deleter_test.go @@ -151,3 +151,51 @@ func TestHandler_DeleteOrganization(t *testing.T) { assert.Equal(t, "cust-1", failure.GetViolations()[1].GetSubject()) }) } + +func TestHandler_CheckOrganizationDelete(t *testing.T) { + t.Run("should report the blockers with can_delete false", func(t *testing.T) { + mockDel := new(mocks.CascadeDeleter) + mockDel.EXPECT().CheckOrganizationDelete(mock.Anything, "some-id").Return([]deleter.Blocker{ + {Type: deleter.BlockerActiveSubscription, Subject: "sub-1", SubjectType: deleter.SubjectSubscription, Message: "subscription[sub-1] is active on a paid plan: downgrade to the standard plan, then retry the delete"}, + {Type: deleter.BlockerUnpaidInvoice, Subject: "inv-1", SubjectType: deleter.SubjectInvoice, Message: "invoice[inv-1] is unpaid: pay it via its hosted payment page, then retry the delete"}, + }, nil) + mockDep := &ConnectHandler{deleterService: mockDel} + + resp, err := mockDep.CheckOrganizationDelete(context.Background(), connect.NewRequest(&frontierv1beta1.CheckOrganizationDeleteRequest{ + Id: "some-id", + })) + assert.NoError(t, err) + assert.False(t, resp.Msg.GetCanDelete()) + assert.Len(t, resp.Msg.GetBlockers(), 2) + assert.Equal(t, deleter.BlockerActiveSubscription, resp.Msg.GetBlockers()[0].GetType()) + assert.Equal(t, "sub-1", resp.Msg.GetBlockers()[0].GetSubject()) + assert.Equal(t, deleter.SubjectSubscription, resp.Msg.GetBlockers()[0].GetSubjectType()) + assert.Equal(t, deleter.BlockerUnpaidInvoice, resp.Msg.GetBlockers()[1].GetType()) + assert.Equal(t, deleter.SubjectInvoice, resp.Msg.GetBlockers()[1].GetSubjectType()) + }) + + t.Run("should report can_delete true when nothing blocks", func(t *testing.T) { + mockDel := new(mocks.CascadeDeleter) + mockDel.EXPECT().CheckOrganizationDelete(mock.Anything, "some-id").Return(nil, nil) + mockDep := &ConnectHandler{deleterService: mockDel} + + resp, err := mockDep.CheckOrganizationDelete(context.Background(), connect.NewRequest(&frontierv1beta1.CheckOrganizationDeleteRequest{ + Id: "some-id", + })) + assert.NoError(t, err) + assert.True(t, resp.Msg.GetCanDelete()) + assert.Empty(t, resp.Msg.GetBlockers()) + }) + + t.Run("should return not found for a missing org", func(t *testing.T) { + mockDel := new(mocks.CascadeDeleter) + mockDel.EXPECT().CheckOrganizationDelete(mock.Anything, "some-id").Return(nil, organization.ErrNotExist) + mockDep := &ConnectHandler{deleterService: mockDel} + + resp, err := mockDep.CheckOrganizationDelete(context.Background(), connect.NewRequest(&frontierv1beta1.CheckOrganizationDeleteRequest{ + Id: "some-id", + })) + assert.Nil(t, resp) + assert.Equal(t, connect.NewError(connect.CodeNotFound, organization.ErrNotExist), err) + }) +} diff --git a/internal/api/v1beta1connect/interfaces.go b/internal/api/v1beta1connect/interfaces.go index d2d81ef67c..542bf5ec94 100644 --- a/internal/api/v1beta1connect/interfaces.go +++ b/internal/api/v1beta1connect/interfaces.go @@ -29,6 +29,7 @@ import ( "github.com/raystack/frontier/core/auditrecord" "github.com/raystack/frontier/core/authenticate" frontiersession "github.com/raystack/frontier/core/authenticate/session" + "github.com/raystack/frontier/core/deleter" "github.com/raystack/frontier/core/domain" "github.com/raystack/frontier/core/event" "github.com/raystack/frontier/core/group" @@ -388,6 +389,7 @@ type NamespaceService interface { type CascadeDeleter interface { DeleteProject(ctx context.Context, id string) error DeleteOrganization(ctx context.Context, id string) error + CheckOrganizationDelete(ctx context.Context, id string) ([]deleter.Blocker, error) DeleteGroup(ctx context.Context, id string) error DeleteUser(ctx context.Context, userID string) error } diff --git a/internal/api/v1beta1connect/mocks/cascade_deleter.go b/internal/api/v1beta1connect/mocks/cascade_deleter.go index 6805e1323a..5098ef4974 100644 --- a/internal/api/v1beta1connect/mocks/cascade_deleter.go +++ b/internal/api/v1beta1connect/mocks/cascade_deleter.go @@ -5,6 +5,7 @@ package mocks import ( context "context" + deleter "github.com/raystack/frontier/core/deleter" mock "github.com/stretchr/testify/mock" ) @@ -21,6 +22,65 @@ func (_m *CascadeDeleter) EXPECT() *CascadeDeleter_Expecter { return &CascadeDeleter_Expecter{mock: &_m.Mock} } +// CheckOrganizationDelete provides a mock function with given fields: ctx, id +func (_m *CascadeDeleter) CheckOrganizationDelete(ctx context.Context, id string) ([]deleter.Blocker, error) { + ret := _m.Called(ctx, id) + + if len(ret) == 0 { + panic("no return value specified for CheckOrganizationDelete") + } + + var r0 []deleter.Blocker + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, string) ([]deleter.Blocker, error)); ok { + return rf(ctx, id) + } + if rf, ok := ret.Get(0).(func(context.Context, string) []deleter.Blocker); ok { + r0 = rf(ctx, id) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]deleter.Blocker) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = rf(ctx, id) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// CascadeDeleter_CheckOrganizationDelete_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CheckOrganizationDelete' +type CascadeDeleter_CheckOrganizationDelete_Call struct { + *mock.Call +} + +// CheckOrganizationDelete is a helper method to define mock.On call +// - ctx context.Context +// - id string +func (_e *CascadeDeleter_Expecter) CheckOrganizationDelete(ctx interface{}, id interface{}) *CascadeDeleter_CheckOrganizationDelete_Call { + return &CascadeDeleter_CheckOrganizationDelete_Call{Call: _e.mock.On("CheckOrganizationDelete", ctx, id)} +} + +func (_c *CascadeDeleter_CheckOrganizationDelete_Call) Run(run func(ctx context.Context, id string)) *CascadeDeleter_CheckOrganizationDelete_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string)) + }) + return _c +} + +func (_c *CascadeDeleter_CheckOrganizationDelete_Call) Return(_a0 []deleter.Blocker, _a1 error) *CascadeDeleter_CheckOrganizationDelete_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *CascadeDeleter_CheckOrganizationDelete_Call) RunAndReturn(run func(context.Context, string) ([]deleter.Blocker, error)) *CascadeDeleter_CheckOrganizationDelete_Call { + _c.Call.Return(run) + return _c +} + // DeleteGroup provides a mock function with given fields: ctx, id func (_m *CascadeDeleter) DeleteGroup(ctx context.Context, id string) error { ret := _m.Called(ctx, id) diff --git a/pkg/server/connect_interceptors/authorization.go b/pkg/server/connect_interceptors/authorization.go index 5aa93b08ab..95671dcd19 100644 --- a/pkg/server/connect_interceptors/authorization.go +++ b/pkg/server/connect_interceptors/authorization.go @@ -456,6 +456,10 @@ var authorizationValidationMap = map[string]func(ctx context.Context, handler *v pbreq := req.(*connect.Request[frontierv1beta1.DeleteOrganizationRequest]) return handler.IsAuthorized(ctx, relation.Object{Namespace: schema.OrganizationNamespace, ID: pbreq.Msg.GetId()}, schema.DeletePermission, req) }, + "/raystack.frontier.v1beta1.FrontierService/CheckOrganizationDelete": func(ctx context.Context, handler *v1beta1connect.ConnectHandler, req connect.AnyRequest) error { + pbreq := req.(*connect.Request[frontierv1beta1.CheckOrganizationDeleteRequest]) + return handler.IsAuthorized(ctx, relation.Object{Namespace: schema.OrganizationNamespace, ID: pbreq.Msg.GetId()}, schema.DeletePermission, req) + }, // group "/raystack.frontier.v1beta1.FrontierService/ListOrganizationGroups": func(ctx context.Context, handler *v1beta1connect.ConnectHandler, req connect.AnyRequest) error {