Skip to content

Commit

Permalink
fix: do not sync deleted test billing customer/subscriptions (#684)
Browse files Browse the repository at this point in the history
If a customer is deleted from billing provider but not in stripe,
it was not getting marked deleted earlier, not it will be marked
disabled correctly.
Similarly if a subscription is deleted from billing provider
which was marked as a test resource will also be marked canceled
correctly in frontier and sync will no longer happen for these
resources.
Additionally, 0 amount billing usage reporting is disabled.

Signed-off-by: Kush Sharma <[email protected]>
  • Loading branch information
kushsharma authored Jul 24, 2024
1 parent 60de7e4 commit 0381894
Show file tree
Hide file tree
Showing 10 changed files with 92 additions and 57 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ TAG := $(shell git rev-list --tags --max-count=1)
VERSION := $(shell git describe --tags ${TAG})
.PHONY: build check fmt lint test test-race vet test-cover-html help install proto ui compose-up-dev
.DEFAULT_GOAL := build
PROTON_COMMIT := "1c6e24b3b0fd693d80546370c66a1e5f05e3ab61"
PROTON_COMMIT := "fecbdc10ba9cfb77b27cdca1722d23460eabbf04"

ui:
@echo " > generating ui build"
Expand Down
10 changes: 6 additions & 4 deletions billing/checkout/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ func (s *Service) backgroundSync(ctx context.Context) {
}

for _, customer := range customers {
if customer.DeletedAt != nil || customer.IsOffline() {
if !customer.IsActive() || customer.IsOffline() {
continue
}
if err := s.SyncWithProvider(ctx, customer.ID); err != nil {
Expand Down Expand Up @@ -639,6 +639,7 @@ func (s *Service) ensureSubscription(ctx context.Context, ch Checkout) (string,
// create subscription
md := metadata.Build(ch.Metadata)
md[CheckoutIDMetadataKey] = ch.ID
md[subscription.ProviderTestResource] = !stripeSubscription.Livemode
sub, err := s.subscriptionService.Create(ctx, subscription.Subscription{
ID: uuid.New().String(),
ProviderID: subProviderID,
Expand Down Expand Up @@ -875,9 +876,10 @@ func (s *Service) Apply(ctx context.Context, ch Checkout) (*subscription.Subscri
CustomerID: billingCustomer.ID,
PlanID: plan.ID,
Metadata: map[string]any{
"org_id": billingCustomer.OrgID,
"delegated": "true",
"checkout_id": ch.ID,
"org_id": billingCustomer.OrgID,
"delegated": "true",
"checkout_id": ch.ID,
subscription.ProviderTestResource: !stripeSubscription.Livemode,
},
State: string(stripeSubscription.Status),
TrialEndsAt: utils.AsTimeFromEpoch(stripeSubscription.TrialEnd),
Expand Down
4 changes: 4 additions & 0 deletions billing/customer/customer.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ func (c Customer) IsOffline() bool {
return c.ProviderID == ""
}

func (c Customer) IsActive() bool {
return c.State == ActiveState && c.DeletedAt == nil
}

type Address struct {
City string `json:"city"`
Country string `json:"country"`
Expand Down
89 changes: 46 additions & 43 deletions billing/customer/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -387,54 +387,57 @@ func (s *Service) SyncWithProvider(ctx context.Context, customr Customer) error
shouldUpdate = true
}
}
if stripeCustomer.TaxIDs != nil {
var taxData []Tax
for _, taxID := range stripeCustomer.TaxIDs.Data {
taxData = append(taxData, Tax{
ID: taxID.Value,
Type: string(taxID.Type),
})
if customr.IsActive() {
// don't update for disabled state
if stripeCustomer.TaxIDs != nil {
var taxData []Tax
for _, taxID := range stripeCustomer.TaxIDs.Data {
taxData = append(taxData, Tax{
ID: taxID.Value,
Type: string(taxID.Type),
})
}
if !slices.EqualFunc(customr.TaxData, taxData, func(a Tax, b Tax) bool {
return a.ID == b.ID && a.Type == b.Type
}) {
customr.TaxData = taxData
shouldUpdate = true
}
}
if !slices.EqualFunc(customr.TaxData, taxData, func(a Tax, b Tax) bool {
return a.ID == b.ID && a.Type == b.Type
}) {
customr.TaxData = taxData
if stripeCustomer.Phone != customr.Phone {
customr.Phone = stripeCustomer.Phone
shouldUpdate = true
}
}
if stripeCustomer.Phone != customr.Phone {
customr.Phone = stripeCustomer.Phone
shouldUpdate = true
}
if stripeCustomer.Email != customr.Email {
customr.Email = stripeCustomer.Email
shouldUpdate = true
}
if stripeCustomer.Name != customr.Name {
customr.Name = stripeCustomer.Name
shouldUpdate = true
}
if stripeCustomer.Currency != "" && string(stripeCustomer.Currency) != customr.Currency {
customr.Currency = string(stripeCustomer.Currency)
shouldUpdate = true
}
if stripeCustomer.Address != nil {
if stripeCustomer.Address.City != customr.Address.City ||
stripeCustomer.Address.Country != customr.Address.Country ||
stripeCustomer.Address.Line1 != customr.Address.Line1 ||
stripeCustomer.Address.Line2 != customr.Address.Line2 ||
stripeCustomer.Address.PostalCode != customr.Address.PostalCode ||
stripeCustomer.Address.State != customr.Address.State {
customr.Address = Address{
City: stripeCustomer.Address.City,
Country: stripeCustomer.Address.Country,
Line1: stripeCustomer.Address.Line1,
Line2: stripeCustomer.Address.Line2,
PostalCode: stripeCustomer.Address.PostalCode,
State: stripeCustomer.Address.State,
}
if stripeCustomer.Email != customr.Email {
customr.Email = stripeCustomer.Email
shouldUpdate = true
}
if stripeCustomer.Name != customr.Name {
customr.Name = stripeCustomer.Name
shouldUpdate = true
}
if stripeCustomer.Currency != "" && string(stripeCustomer.Currency) != customr.Currency {
customr.Currency = string(stripeCustomer.Currency)
shouldUpdate = true
}
if stripeCustomer.Address != nil {
if stripeCustomer.Address.City != customr.Address.City ||
stripeCustomer.Address.Country != customr.Address.Country ||
stripeCustomer.Address.Line1 != customr.Address.Line1 ||
stripeCustomer.Address.Line2 != customr.Address.Line2 ||
stripeCustomer.Address.PostalCode != customr.Address.PostalCode ||
stripeCustomer.Address.State != customr.Address.State {
customr.Address = Address{
City: stripeCustomer.Address.City,
Country: stripeCustomer.Address.Country,
Line1: stripeCustomer.Address.Line1,
Line2: stripeCustomer.Address.Line2,
PostalCode: stripeCustomer.Address.PostalCode,
State: stripeCustomer.Address.State,
}
shouldUpdate = true
}
}
}
if shouldUpdate {
if _, err := s.repository.UpdateByID(ctx, customr); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion billing/invoice/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func (s *Service) backgroundSync(ctx context.Context) {
return
}
for _, customer := range customers {
if customer.DeletedAt != nil || customer.ProviderID == "" {
if !customer.IsActive() || customer.ProviderID == "" {
continue
}
if err := s.SyncWithProvider(ctx, customer); err != nil {
Expand Down
29 changes: 25 additions & 4 deletions billing/subscription/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ import (
)

const (
SyncDelay = time.Second * 60

SyncDelay = time.Second * 60
ProviderTestResource = "test_resource"
InitiatorIDMetadataKey = "initiated_by"
)

Expand Down Expand Up @@ -147,7 +147,7 @@ func (s *Service) backgroundSync(ctx context.Context) {
}

for _, customer := range customers {
if customer.DeletedAt != nil || customer.IsOffline() {
if !customer.IsActive() || customer.IsOffline() {
continue
}
if err := s.SyncWithProvider(ctx, customer); err != nil {
Expand Down Expand Up @@ -188,9 +188,26 @@ func (s *Service) SyncWithProvider(ctx context.Context, customr customer.Custome

var subErrs []error
for _, sub := range subs {
if sub.IsCanceled() {
continue
}

stripeSubscription, stripeSchedule, err := s.createOrGetSchedule(ctx, sub)
if err != nil {
subErrs = append(subErrs, fmt.Errorf("%s: %w", err, ErrSubscriptionOnProviderNotFound))
if errors.Is(err, ErrSubscriptionOnProviderNotFound) {
// if it's a test resource, mark it as canceled
if val, ok := sub.Metadata[ProviderTestResource].(bool); ok && val {
sub.State = StateCanceled.String()
sub.CanceledAt = time.Now().UTC()
if _, err := s.repository.UpdateByID(ctx, sub); err != nil {
subErrs = append(subErrs, err)
}
} else {
subErrs = append(subErrs, err)
}
} else {
subErrs = append(subErrs, err)
}
continue
}

Expand Down Expand Up @@ -392,6 +409,10 @@ func (s *Service) createOrGetSchedule(ctx context.Context, sub Subscription) (*s
},
})
if err != nil {
// check if it's a subscription not found err
if stripeErr, ok := err.(*stripe.Error); ok && stripeErr.Code == stripe.ErrorCodeResourceMissing {
return nil, nil, ErrSubscriptionOnProviderNotFound
}
return nil, nil, fmt.Errorf("failed to get subscription from billing provider: %w", err)
}

Expand Down
5 changes: 5 additions & 0 deletions billing/subscription/subscription.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const (
StateActive State = "active"
StateTrialing State = "trialing"
StatePastDue State = "past_due"
StateCanceled State = "canceled"
)

type PhaseReason string
Expand Down Expand Up @@ -83,6 +84,10 @@ func (s Subscription) IsActive() bool {
return State(s.State) == StateActive || State(s.State) == StateTrialing
}

func (s Subscription) IsCanceled() bool {
return State(s.State) == StateCanceled || !s.DeletedAt.IsZero() || !s.CanceledAt.IsZero()
}

type Filter struct {
CustomerID string
ProviderID string
Expand Down
2 changes: 1 addition & 1 deletion proto/apidocs.swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9891,7 +9891,7 @@ definitions:
`NullValue` is a singleton enumeration to represent the null value for the
`Value` type union.
The JSON representation for `NullValue` is JSON `null`.
The JSON representation for `NullValue` is JSON `null`.
- NULL_VALUE: Null value.
rpcStatus:
Expand Down
2 changes: 1 addition & 1 deletion proto/v1beta1/models.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions proto/v1beta1/models.pb.validate.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 0381894

Please sign in to comment.