Skip to content

[usage] Use createStripeSubscription behind feature flag #14332

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

Merged
merged 1 commit into from
Nov 2, 2022

Conversation

laushinka
Copy link
Contributor

@laushinka laushinka commented Nov 1, 2022

Description

Uses the CreateStripeSubscription RPC behind a feature flag.

Related Issue(s)

Relates #13198

How to test

I tested by:

  1. kubectl port-forward server 9500:9500
  2. curl localhost:9500/metrics
  3. Created subscription from a user behind the feature flag and another user that isn't.
  4. The one that isn't feature flagged incremented the gitpod_server_stripe_client_requests_completed_duration_seconds metric.

Release Notes

NONE

Documentation

Werft options:

  • /werft with-local-preview
    If enabled this will build install/preview
  • /werft with-preview
  • /werft with-large-vm
  • /werft with-integration-tests=all
    Valid options are all, workspace, webapp, ide

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-lau-ff-stripe-13198.2 because the annotations in the pull request description changed
(with .werft/ from main)

@laushinka laushinka force-pushed the lau/ff-stripe-13198 branch 2 times, most recently from 70a42ae to 2775cfc Compare November 2, 2022 09:57
@laushinka laushinka force-pushed the lau/ff-stripe-13198 branch from da6df28 to ca2970c Compare November 2, 2022 11:34
@laushinka laushinka marked this pull request as ready for review November 2, 2022 11:35
@laushinka laushinka requested a review from a team November 2, 2022 11:35
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Nov 2, 2022
"createStripeSubscriptionOnUsage",
false,
{
user: user,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: which user/team should we be feature flagging for now?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if you run through the subscription workflow in staging for both a user and a team, and it works through the usage component, then I'd largely go and enable it for all in prod, but monitor.

Adding specific target rules here is mostly for completeness, and for if it does go wrong that we can disable for all but reproduce for our team or something like that.

"createStripeSubscriptionOnUsage",
false,
{
user: user,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should supply the team_id as well for the case when it's enabled by a team

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely - will add it.

@laushinka laushinka force-pushed the lau/ff-stripe-13198 branch from ca2970c to fdbbf7c Compare November 2, 2022 11:39
@laushinka laushinka requested a review from easyCZ November 2, 2022 11:40
@@ -2177,6 +2178,7 @@ export class GitpodServerEEImpl extends GitpodServerImpl {

const user = this.checkAndBlockUser("subscribeToStripe");

let team: Team | undefined;
try {
if (attrId.kind === "team") {
const team = await this.guardTeamOperation(attrId.teamId, "update");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I this that when this is declared as a const it will shadow the let team above and won't actually get assigned

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops good catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed - thanks 🙏🏽

@laushinka laushinka force-pushed the lau/ff-stripe-13198 branch from fdbbf7c to bd15072 Compare November 2, 2022 11:43
} else {
await this.stripeService.setDefaultPaymentMethodForCustomer(customerId, setupIntentId);
await this.stripeService.createSubscriptionForCustomer(customerId, attributionId);
}

// Creating a cost center for this customer
const { costCenter } = await this.usageService.setCostCenter({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a follow-up, we should then move the cost-center setting into the CreateStripeSubscription call

@roboquat roboquat merged commit 84e1f36 into main Nov 2, 2022
@roboquat roboquat deleted the lau/ff-stripe-13198 branch November 2, 2022 11:52
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note-none size/S team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants