-
Notifications
You must be signed in to change notification settings - Fork 1.3k
When starting a workspace but usage attribution is unclear, prompt for explicit user choice #11777
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
/** | ||
* Copyright (c) 2022 Gitpod GmbH. All rights reserved. | ||
* Licensed under the GNU Affero General Public License (AGPL). | ||
* See License-AGPL.txt in the project root for license information. | ||
*/ | ||
|
||
import { useContext, useEffect, useState } from "react"; | ||
import { Team } from "@gitpod/gitpod-protocol"; | ||
import { AttributionId } from "@gitpod/gitpod-protocol/lib/attribution"; | ||
import { getGitpodService } from "../service/service"; | ||
import { TeamsContext } from "../teams/teams-context"; | ||
import { UserContext } from "../user-context"; | ||
import SelectableCardSolid from "../components/SelectableCardSolid"; | ||
import { ReactComponent as Spinner } from "../icons/Spinner.svg"; | ||
|
||
export function BillingAccountSelector(props: { onSelected?: () => void }) { | ||
const { user, setUser } = useContext(UserContext); | ||
const { teams } = useContext(TeamsContext); | ||
const [teamsWithBillingEnabled, setTeamsWithBillingEnabled] = useState<Team[] | undefined>(); | ||
|
||
useEffect(() => { | ||
if (!teams) { | ||
setTeamsWithBillingEnabled(undefined); | ||
return; | ||
} | ||
const teamsWithBilling: Team[] = []; | ||
Promise.all( | ||
teams.map(async (t) => { | ||
const subscriptionId = await getGitpodService().server.findStripeSubscriptionIdForTeam(t.id); | ||
if (subscriptionId) { | ||
teamsWithBilling.push(t); | ||
} | ||
}), | ||
).then(() => setTeamsWithBillingEnabled(teamsWithBilling)); | ||
}, [teams]); | ||
|
||
const setUsageAttributionTeam = async (team?: Team) => { | ||
if (!user) { | ||
return; | ||
} | ||
const usageAttributionId = AttributionId.render( | ||
team ? { kind: "team", teamId: team.id } : { kind: "user", userId: user.id }, | ||
); | ||
await getGitpodService().server.setUsageAttribution(usageAttributionId); | ||
setUser(await getGitpodService().server.getLoggedInUser()); | ||
if (props.onSelected) { | ||
props.onSelected(); | ||
} | ||
}; | ||
return ( | ||
<> | ||
{teamsWithBillingEnabled === undefined && <Spinner className="m-2 h-5 w-5 animate-spin" />} | ||
{teamsWithBillingEnabled && ( | ||
<div> | ||
<p>Bill all my usage to:</p> | ||
<div className="mt-4 flex space-x-3"> | ||
<SelectableCardSolid | ||
className="w-36 h-32" | ||
title="(myself)" | ||
selected={ | ||
!teamsWithBillingEnabled.find( | ||
(t) => | ||
AttributionId.render({ kind: "team", teamId: t.id }) === | ||
user?.usageAttributionId, | ||
)?.name | ||
} | ||
onClick={() => setUsageAttributionTeam(undefined)} | ||
> | ||
<div className="flex-grow flex items-end p-1"></div> | ||
</SelectableCardSolid> | ||
{teamsWithBillingEnabled.map((t) => ( | ||
<SelectableCardSolid | ||
className="w-36 h-32" | ||
title={t.name} | ||
selected={ | ||
!!teamsWithBillingEnabled.find( | ||
(t) => | ||
AttributionId.render({ kind: "team", teamId: t.id }) === | ||
user?.usageAttributionId, | ||
)?.name | ||
} | ||
onClick={() => setUsageAttributionTeam(t)} | ||
> | ||
<div className="flex-grow flex items-end p-1"></div> | ||
</SelectableCardSolid> | ||
))} | ||
</div> | ||
</div> | ||
)} | ||
</> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,15 +32,18 @@ export namespace ErrorCodes { | |
// 430 Repository not whitelisted (custom status code) | ||
export const REPOSITORY_NOT_WHITELISTED = 430; | ||
|
||
// 450 Payment error | ||
export const PAYMENT_ERROR = 450; | ||
|
||
// 455 Invalid cost center (custom status code) | ||
export const INVALID_COST_CENTER = 455; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we agree on a common prefix for everything payment related, please? and feel free to comment on that BTW! alternative way to approach this, we could use just There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see much value in aligning every payment-related error around a single prefix or error code range (apart from readability of the error codes list -- which we arguably don't read that often). Personally, I'd prefer to leave this as is (unless you have a strong opinion towards a different error name or code), and you can use whichever convention seems best in your PR. We can always align later if you think it's valuable, but I suspect that aligning these is not important in the grand scheme of things. 😇 |
||
|
||
// 460 Context Parse Error (custom status code) | ||
export const CONTEXT_PARSE_ERROR = 460; | ||
|
||
// 461 Invalid gitpod yml | ||
// 461 Invalid gitpod yml (custom status code) | ||
export const INVALID_GITPOD_YML = 461; | ||
|
||
// 450 Payment error | ||
export const PAYMENT_ERROR = 450; | ||
|
||
// 470 User Blocked (custom status code) | ||
export const USER_BLOCKED = 470; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,7 @@ import { | |
WORKSPACE_TIMEOUT_DEFAULT_LONG, | ||
WORKSPACE_TIMEOUT_EXTENDED, | ||
WORKSPACE_TIMEOUT_EXTENDED_ALT, | ||
Team, | ||
} from "@gitpod/gitpod-protocol"; | ||
import { CostCenterDB, ProjectDB, TeamDB, TermsAcceptanceDB, UserDB } from "@gitpod/gitpod-db/lib"; | ||
import { HostContextProvider } from "../auth/host-context-provider"; | ||
|
@@ -29,6 +30,8 @@ import { EmailAddressAlreadyTakenException, SelectAccountException } from "../au | |
import { SelectAccountPayload } from "@gitpod/gitpod-protocol/lib/auth"; | ||
import { AttributionId } from "@gitpod/gitpod-protocol/lib/attribution"; | ||
import { StripeService } from "../../ee/src/user/stripe-service"; | ||
import { ResponseError } from "vscode-ws-jsonrpc"; | ||
import { ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error"; | ||
|
||
export interface FindUserByIdentityStrResult { | ||
user: User; | ||
|
@@ -191,6 +194,66 @@ export class UserService { | |
} | ||
} | ||
|
||
protected async findTeamUsageBasedSubscriptionId(team: Team): Promise<string | undefined> { | ||
const customer = await this.stripeService.findCustomerByTeamId(team.id); | ||
if (!customer) { | ||
return; | ||
} | ||
const subscription = await this.stripeService.findUncancelledSubscriptionByCustomer(customer.id); | ||
return subscription?.id; | ||
} | ||
|
||
protected async validateUsageAttributionId(user: User, usageAttributionId: string): Promise<void> { | ||
const attribution = AttributionId.parse(usageAttributionId); | ||
if (attribution?.kind === "team") { | ||
const team = await this.teamDB.findTeamById(attribution.teamId); | ||
if (!team) { | ||
throw new ResponseError( | ||
ErrorCodes.INVALID_COST_CENTER, | ||
"The billing team you've selected no longer exists.", | ||
); | ||
} | ||
const members = await this.teamDB.findMembersByTeam(team.id); | ||
if (!members.find((m) => m.userId === user.id)) { | ||
throw new ResponseError( | ||
ErrorCodes.INVALID_COST_CENTER, | ||
"You're no longer a member of the selected billing team.", | ||
); | ||
} | ||
const subscriptionId = await this.findTeamUsageBasedSubscriptionId(team); | ||
if (!subscriptionId) { | ||
throw new ResponseError( | ||
ErrorCodes.INVALID_COST_CENTER, | ||
"The billing team you've selected has no active subscription.", | ||
); | ||
} | ||
} | ||
} | ||
|
||
protected async findSingleTeamWithUsageBasedBilling(user: User): Promise<Team | undefined> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI: This can also come out of But that's improvement after both got merged. |
||
// Find all the user's teams with usage-based billing enabled. | ||
const teams = await this.teamDB.findTeamsByUser(user.id); | ||
const teamsWithBilling: Team[] = []; | ||
await Promise.all( | ||
teams.map(async (team) => { | ||
const subscriptionId = await this.findTeamUsageBasedSubscriptionId(team); | ||
if (subscriptionId) { | ||
teamsWithBilling.push(team); | ||
} | ||
}), | ||
); | ||
if (teamsWithBilling.length > 1) { | ||
// Multiple teams with usage-based billing enabled -- ask the user to make an explicit choice. | ||
throw new ResponseError(ErrorCodes.INVALID_COST_CENTER, "Multiple teams have billing enabled."); | ||
} | ||
if (teamsWithBilling.length === 1) { | ||
// Single team with usage-based billing enabled -- attribute all usage to it. | ||
return teamsWithBilling[0]; | ||
} | ||
// No team with usage-based billing enabled. | ||
return undefined; | ||
} | ||
|
||
/** | ||
* Identifies the team or user to which a workspace instance's running time should be attributed to | ||
* (e.g. for usage analytics or billing purposes). | ||
|
@@ -209,12 +272,18 @@ export class UserService { | |
async getWorkspaceUsageAttributionId(user: User, projectId?: string): Promise<string | undefined> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit, and out-of-scope here: It feels like this method (and it's descendants) do not necessarily belong into UserService, but more into |
||
// A. Billing-based attribution | ||
jankeromnes marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (this.config.enablePayment) { | ||
if (!user.usageAttributionId) { | ||
// No explicit user attribution ID yet -- attribute all usage to the user by default (regardless of project/team). | ||
return AttributionId.render({ kind: "user", userId: user.id }); | ||
if (user.usageAttributionId) { | ||
await this.validateUsageAttributionId(user, user.usageAttributionId); | ||
// Return the user's explicit attribution ID. | ||
return user.usageAttributionId; | ||
} | ||
// Return the user's explicit attribution ID. | ||
return user.usageAttributionId; | ||
const billingTeam = await this.findSingleTeamWithUsageBasedBilling(user); | ||
if (billingTeam) { | ||
// Single team with usage-based billing enabled -- attribute all usage to it. | ||
return AttributionId.render({ kind: "team", teamId: billingTeam.id }); | ||
} | ||
// Attribute all usage to the user by default (regardless of project/team). | ||
return AttributionId.render({ kind: "user", userId: user.id }); | ||
} | ||
|
||
// B. Project-based attribution | ||
|
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.
@jankeromnes, I bet this is not the single place, where we'd need this kind of info. have you considered promoting this flag to the result of getTeams?
team.isBillingEnabled
would be nice and it shave off the additional requests.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.
@AlexTugarev I think this would be premature, because:
getTeams
load all the team subscriptions on every call, even when most callers don't need that currently