GetUpcomingInvoice for BillingService #12302
Conversation
|
started the job as gitpod-build-at-upcoming.8 because the annotations in the pull request description changed |
AlexTugarev
force-pushed
the
at/upcoming
branch
from
053e2ab to
2b8e998
Compare
|
/werft run 👍 started the job as gitpod-build-at-upcoming.10 |
|
started the job as gitpod-build-at-upcoming.11 because the annotations in the pull request description changed |
AlexTugarev
force-pushed
the
at/upcoming
branch
from
2b8e998 to
e30f823
Compare
AlexTugarev
force-pushed
the
at/upcoming
branch
from
e30f823 to
31b8647
Compare
AlexTugarev
force-pushed
the
at/upcoming
branch
from
31b8647 to
b0b24d5
Compare
roboquat
added
release-note-none
and removed
do-not-merge/release-note-label-needed
labels
| const sessions = response.getSessionsList().map((s) => UsageService.mapBilledSession(s)); | ||
| return sessions; | ||
| async getUpcomingInvoice(teamId: string): Promise<GetUpcomingInvoiceResponse> { | ||
| const response = await this.billingServiceClientProvider.getDefault().getUpcomingInvoice(teamId); |
There was a problem hiding this comment.
WDYT of using attributionId here, and using ParseAttributionID on the side of BillingService? This way, we do not need to make any further changes here once UB is enabled for users.
There was a problem hiding this comment.
☝️ Thought about the same. 🧠 ✨ 👍
| const sessions = response.getSessionsList().map((s) => UsageService.mapBilledSession(s)); | ||
| return sessions; | ||
| async getUpcomingInvoice(teamId: string): Promise<GetUpcomingInvoiceResponse> { | ||
| const response = await this.billingServiceClientProvider.getDefault().getUpcomingInvoice(teamId); |
There was a problem hiding this comment.
When this rolls out, you can't guarantee that when this is executed, the usage component has been rolled out to a newer version and that the getUpcomingInvoice RPC actually exists.
In the happy case, this inconsistency won't happen, but in the bad case, where for example usage fails to start and remains on the old version, it will start failing here.
There was a problem hiding this comment.
The way to fix this is to roll-out the RPC first, and only then change the behaviour on server. Alternatively, you can use a feature flag to control this also.
There was a problem hiding this comment.
Good point! Let me thinks about where to add the feature flag. First idea is to guard the spending limit check.
There was a problem hiding this comment.
Personally, I think you can achieve the same thing (with fewer extra pieces) by splitting the PR up and first landing (and deploying) the changes which introduce the RPC on the proto level, but also implement it.
AlexTugarev
force-pushed
the
at/upcoming
branch
from
b0b24d5 to
1cd0fbf
Compare
|
GitHub, why? :-(
|
|
Reopened as #12377 |
😢 |
|
How did this PR get merged? I'm really confused. |
Description
This PR implements
GetLatestInvoiceGetUpcomingInvoice. The rename is done intentionally, to a) align with terminology used in stripe, and b) to distinguish between the next invoice and an already paid invoice. The upcoming invoice contains the the amount of credits currently used (🤞🏻 after discounts), which can be compared with the spending limit configured.Related Issue(s)
Fixes #11692
How to test
Setup UBB in for a team and lower the spending limit to 1 or 2 credits for a simple test, then start a workspace or two to hit that limit. After a while, cannot tell it precisely, you'll need to reload the dashboard to refresh the spending limit checks. In the
servercomponent you'll find a log message on spending limit events, and the known dashboard notification should be visible:Release Notes
Werft options: