Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

EntitlementService Usage-Based Pricing #11936

Merged
merged 4 commits into from
Aug 10, 2022
Merged

EntitlementService Usage-Based Pricing #11936

merged 4 commits into from
Aug 10, 2022

Conversation

geropl
Copy link
Member

@geropl geropl commented Aug 5, 2022

Description

Based on: #11814

  1. Introduce EntitlementServiceUBP
  2. Implement BillingService.checkSpendingLimitReached (based on usage until Implement BillingService.GetLatestInvoice聽#11692 got merged)#
  3. Wiring, re-use of BillingService (commit)

Related Issue(s)

Fixes #11402

How to test

Actually, after #11814 has been tested thoroughly, this can be tested in prod by enableing isUsageBasedPricingEnabled, and testing the rollout flow. 馃檭

Release Notes

NONE

Documentation

Werft options:

  • /werft with-preview
  • /werft with-payment

@geropl geropl changed the title Gpl/ets ubb EntitlementService Usage-Based Pricing Aug 5, 2022
@geropl
Copy link
Member Author

geropl commented Aug 8, 2022

@jankeromnes @AlexTugarev This should build now. Can be moved to "ready for review" after #11814 has been merged, and this PR has been rebased on that! 馃憤

@roboquat roboquat added size/XL and removed size/XXL labels Aug 9, 2022
@AlexTugarev AlexTugarev marked this pull request as ready for review August 9, 2022 10:26
@AlexTugarev AlexTugarev requested a review from a team August 9, 2022 10:26
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Aug 9, 2022
@AlexTugarev
Copy link
Member

Got a inversify dependency cycle on

Server 
--> GithubApp 
--> PrebuildManager 
--> WorkspaceStarter 
--> WorkspaceManagerClientProvider 
--> WorkspaceManagerClientProviderCompositeSource 
--> Symbol(WorkspaceManagerClientProviderSource) 
--> Symbol(WorkspaceManagerClientProviderSource)

which seems to be unrelated to this PR changes on first look. Quickly checked with current main's state to rule out issues from there, and updated once more.
Not sure, if that comes from the multiInject thing (which happens to cause headaches sometimes) or a subtle change of imports which also causes module to be loaded differently.

@AlexTugarev
Copy link
Member

AlexTugarev commented Aug 9, 2022

/werft run

馃憤 started the job as gitpod-build-gpl-ets-ubb.5
(with .werft/ from main)

@AlexTugarev
Copy link
Member

AlexTugarev commented Aug 9, 2022

'Wasted 1h of debugging in the wrong direction. Turns out the problematic import is import { BillingService } from "./billing-service", which is completely unrelated to the above mentioned cycle reported by inversify.

Ah, EntitlementServiceUBP -> BillingService -> UserService -> EntitlementService is the bogus path. Actually, UserService does not use EntitlementService at all.

@AlexTugarev AlexTugarev force-pushed the gpl/ets-ubb branch 3 times, most recently from bd874fd to 932f055 Compare August 10, 2022 11:47

protected async hasPaidSubscription(user: User, date: Date): Promise<boolean> {
// TODO(gpl) UBP personal: implement!
return true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: As currently implemented, it reads like everyone on Usage-Based gets XL workspaces.

@jankeromnes
Copy link
Contributor

Problematic situation:

  • I have a personal Chargebee plan + a legacy Chargebee team plan
  • I also have a team that has Usage-Based enabled

In the team, I'm able to view the Usage-Based UI (e.g. Usage, Billing), but if I click on upgrade, the credit card form does not show up.

That's because the API methods getStripePublishableKey and getStripeSetupIntentClientSecret get rejected for my user, because my user is still in Chargebee mode (technically, these calls are made on behalf of a team that has Usage-Based enabled, but we're not sending through the teamId because it's not needed).

Possible solutions:

  • Display a proper error message in the UI (e.g. "You need to cancel all your personal Chargebee plans before you can upgrade a team to Usage-Based")
  • Authorize anyone to make the calls getStripePublishableKey (get the publicly-readable Gitpod front-end key) and getStripeSetupIntentClientSecret (create a session that allows you to submit credit card details)

Copy link
Contributor

@jankeromnes jankeromnes left a comment

Choose a reason for hiding this comment

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

Ok, thanks @geropl and @AlexTugarev for completing this big work item.

The code looks good to me, and after testing it, it seems that all the Chargebee flows work as well as before (with one slight worry about the out-of-hour situation).

Approving so that this lands, and so that we can test all the recent changes together on staging before they go live tomorrow morning. 馃

Allegory of this PR: 馃挘馃浌 (a skateboard that makes me nervous 馃槄)

@roboquat roboquat merged commit b4a9939 into main Aug 10, 2022
@roboquat roboquat deleted the gpl/ets-ubb branch August 10, 2022 14:53
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Aug 11, 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/XL team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Define & Implement Entitlement Service APIs
4 participants