-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[usage] Handle customer.subscription.deleted
event from Stripe
#13016
Conversation
started the job as gitpod-build-se-stripe-cancellation.1 because the annotations in the pull request description changed |
ef44216
to
d4853d2
Compare
d4853d2
to
600d13d
Compare
600d13d
to
a3d2eab
Compare
customer.subscription.deleted
event form Stripe
customer.subscription.deleted
event form Stripecustomer.subscription.deleted
event from Stripe
a3d2eab
to
2365191
Compare
2365191
to
1bb1ab3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM in general. Left a couple of comments around readability
/hold
@@ -244,6 +245,19 @@ func (c *Client) GetInvoice(ctx context.Context, invoiceID string) (*stripe.Invo | |||
return invoice, nil | |||
} | |||
|
|||
func (c *Client) GetAttributionIdForSubscriptionId(ctx context.Context, subscriptionID string) (db.AttributionID, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could use this method in the FinalizeInvoice RPC now
https://github.com/gitpod-io/gitpod/pull/13016/files#diff-0d6333b0d3d47166ad0d69ee3f28afaa9bb48eeb72de0e648582fa3852def7fcR76-R86
// we don't manage stripe billing cycle | ||
costCenter.NextBillingTime = VarcharTime{} | ||
} else if costCenter.BillingStrategy == CostCenter_Other { | ||
// cancelled from stripe reset the spending limit | ||
if costCenter.ID.IsEntity(AttributionEntity_Team) { | ||
costCenter.SpendingLimit = c.cfg.ForTeams | ||
} else { | ||
costCenter.SpendingLimit = c.cfg.ForUsers | ||
} | ||
// see you next month | ||
costCenter.NextBillingTime = NewVarcharTime(now.AddDate(0, 1, 0)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the individual field assignment into a CostCenter object really hurts readability here. The alternative approach is to construct the CostCenter in each branch seperately (with some possible duplication) but making it very explicit what each branch will result in.
invoiceUpdatedEventType = "invoice.updated" | ||
invoiceFinalizedEventType = "invoice.finalized" | ||
customerCreatedEventType = "customer.created" | ||
customerSubscriptionDeleted = "customer.subscription.deleted" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might make sense to move these as consts in the stripe.go
file and re-use in tests
1bb1ab3
to
be4ee50
Compare
This API is called by Stripe when a customer cancels their subscription
This event is fired by Stripe when a customer cancels their subscription
be4ee50
to
ca5ac1f
Compare
/unhold |
Description
When a user cancels their subscription, we get an event from Stripe, which we need to handle in order to update our system.
This PR does
CancelSubscription
on usage-apicustomer.subscription.deleted
event in public-api-server which callsCancelSubscription
CancelSubscription
in usage.Related Issue(s)
See #13010
How to test
Release Notes
Documentation
Werft options: