Skip to content

Commit af88bed

Browse files
geroplgeropl
authored
[server, dashboard] Remove api.maySetTimeout (+ code cleanup in User- + EntitlementService) (#18769)
* [dashbaord, server] Remove api.maySetTimeout * [server] Remove dead code in Entitlement- and UserService * review comments
1 parent 6c7f47d commit af88bed

File tree

10 files changed

+60
-163
lines changed

10 files changed

+60
-163
lines changed

‎components/dashboard/src/data/current-user/may-set-timeout-query.ts

Lines changed: 0 additions & 27 deletions
This file was deleted.

‎components/dashboard/src/user-settings/Preferences.tsx

Lines changed: 45 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -9,24 +9,23 @@ import { getGitpodService } from "../service/service";
99
import { UserContext } from "../user-context";
1010
import { PageWithSettingsSubMenu } from "./PageWithSettingsSubMenu";
1111
import { ThemeSelector } from "../components/ThemeSelector";
12-
import Alert from "../components/Alert";
1312
import { Link } from "react-router-dom";
1413
import { Heading2, Heading3, Subheading } from "../components/typography/headings";
15-
import { useUserMaySetTimeout } from "../data/current-user/may-set-timeout-query";
1614
import { Button } from "../components/Button";
1715
import SelectIDE from "./SelectIDE";
1816
import { InputField } from "../components/forms/InputField";
1917
import { TextInput } from "../components/forms/TextInputField";
2018
import { useToast } from "../components/toasts/Toasts";
2119
import { useUpdateCurrentUserDotfileRepoMutation } from "../data/current-user/update-mutation";
2220
import { AdditionalUserData } from "@gitpod/gitpod-protocol";
21+
import { useOrgBillingMode } from "../data/billing-mode/org-billing-mode-query";
2322

2423
export type IDEChangedTrackLocation = "workspace_list" | "workspace_start" | "preferences";
2524

2625
export default function Preferences() {
2726
const { toast } = useToast();
2827
const { user, setUser } = useContext(UserContext);
29-
const maySetTimeout = useUserMaySetTimeout();
28+
const billingMode = useOrgBillingMode();
3029
const updateDotfileRepo = useUpdateCurrentUserDotfileRepoMutation();
3130

3231
const [dotfileRepo, setDotfileRepo] = useState<string>(user?.additionalData?.dotfileRepo || "");
@@ -57,15 +56,29 @@ export default function Preferences() {
5756
const updatedUser = await getGitpodService().server.getLoggedInUser();
5857
setUser(updatedUser);
5958

60-
toast("Your default workspace timeout was updated.");
59+
let toastMessage = <>Default workspace timeout was updated.</>;
60+
if (billingMode.data?.mode === "usage-based") {
61+
if (!billingMode.data.paid) {
62+
toastMessage = (
63+
<>
64+
{toastMessage} Changes will only affect workspaces in paid organizations. Go to{" "}
65+
<Link to="/billing" className="gp-link">
66+
billing
67+
</Link>{" "}
68+
to upgrade your organization.
69+
</>
70+
);
71+
}
72+
}
73+
toast(toastMessage);
6174
} catch (e) {
6275
// TODO: Convert this to an error style toast
6376
alert("Cannot set custom workspace timeout: " + e.message);
6477
} finally {
6578
setTimeoutUpdating(false);
6679
}
6780
},
68-
[toast, setUser, workspaceTimeout],
81+
[toast, setUser, workspaceTimeout, billingMode],
6982
);
7083

7184
const clearAutostartWorkspaceOptions = useCallback(async () => {
@@ -136,46 +149,34 @@ export default function Preferences() {
136149
<Subheading>Workspaces will stop after a period of inactivity without any user input.</Subheading>
137150

138151
<div className="mt-4 max-w-xl">
139-
{!maySetTimeout.isLoading && maySetTimeout.data === false && (
140-
<Alert type="message">
141-
Upgrade organization{" "}
142-
<Link to="/billing" className="gp-link">
143-
billing
144-
</Link>{" "}
145-
plan to use a custom inactivity timeout.
146-
</Alert>
147-
)}
148-
149-
{maySetTimeout.data === true && (
150-
<form onSubmit={saveWorkspaceTimeout}>
151-
<InputField
152-
label="Default Workspace Timeout"
153-
hint={
154-
<span>
155-
Use minutes or hours, like <span className="font-semibold">30m</span> or{" "}
156-
<span className="font-semibold">2h</span>
157-
</span>
158-
}
159-
>
160-
<div className="flex space-x-2">
161-
<div className="flex-grow">
162-
<TextInput
163-
value={workspaceTimeout}
164-
placeholder="e.g. 30m"
165-
onChange={setWorkspaceTimeout}
166-
/>
167-
</div>
168-
<Button
169-
htmlType="submit"
170-
loading={timeoutUpdating}
171-
disabled={workspaceTimeout === user?.additionalData?.workspaceTimeout ?? ""}
172-
>
173-
Save
174-
</Button>
152+
<form onSubmit={saveWorkspaceTimeout}>
153+
<InputField
154+
label="Default Workspace Timeout"
155+
hint={
156+
<span>
157+
Use minutes or hours, like <span className="font-semibold">30m</span> or{" "}
158+
<span className="font-semibold">2h</span>
159+
</span>
160+
}
161+
>
162+
<div className="flex space-x-2">
163+
<div className="flex-grow">
164+
<TextInput
165+
value={workspaceTimeout}
166+
placeholder="e.g. 30m"
167+
onChange={setWorkspaceTimeout}
168+
/>
175169
</div>
176-
</InputField>
177-
</form>
178-
)}
170+
<Button
171+
htmlType="submit"
172+
loading={timeoutUpdating}
173+
disabled={workspaceTimeout === user?.additionalData?.workspaceTimeout ?? ""}
174+
>
175+
Save
176+
</Button>
177+
</div>
178+
</InputField>
179+
</form>
179180
</div>
180181
</PageWithSettingsSubMenu>
181182
</div>

‎components/gitpod-protocol/src/billing-mode.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ export namespace BillingMode {
2222
}
2323

2424
export function canSetCostCenter(billingMode: BillingMode): boolean {
25-
// if has any Stripe Subscription, either directly or per team
2625
return billingMode.mode === "usage-based";
2726
}
2827
}
@@ -36,6 +35,6 @@ interface None {
3635
interface UsageBased {
3736
mode: "usage-based";
3837

39-
/** True iff this is a team, and is based on a paid plan. Currently only set for teams! */
40-
paid?: boolean;
38+
/** True if the org has a paid plan. */
39+
paid: boolean;
4140
}

‎components/gitpod-protocol/src/gitpod-service.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,6 @@ export interface GitpodServer extends JsonRpcServer<GitpodClient>, AdminServer,
261261
reportErrorBoundary(url: string, message: string): Promise<void>;
262262

263263
getSupportedWorkspaceClasses(): Promise<SupportedWorkspaceClass[]>;
264-
maySetTimeout(): Promise<boolean>;
265264
updateWorkspaceTimeoutSetting(setting: Partial<WorkspaceTimeoutSetting>): Promise<void>;
266265

267266
/**

‎components/server/src/auth/rate-limiter.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,6 @@ const defaultFunctions: FunctionsConfig = {
190190
getCostCenter: { group: "default", points: 1 },
191191
setUsageLimit: { group: "default", points: 1 },
192192
getSupportedWorkspaceClasses: { group: "default", points: 1 },
193-
maySetTimeout: { group: "default", points: 1 },
194193
updateWorkspaceTimeoutSetting: { group: "default", points: 1 },
195194
getIDToken: { group: "default", points: 1 },
196195
reportErrorBoundary: { group: "default", points: 1 },

‎components/server/src/billing/billing-mode.ts

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,4 @@ export class BillingModes {
3131
const paid = billingStrategy === CostCenter_BillingStrategy.BILLING_STRATEGY_STRIPE;
3232
return { mode: "usage-based", paid };
3333
}
34-
35-
/**
36-
* @deprecated use getBillingMode(userId, organizationId) instead
37-
* @returns
38-
*/
39-
async getBillingModeForUser(): Promise<BillingMode> {
40-
if (!this.config.enablePayment) {
41-
// Payment is not enabled. E.g. Self-Hosted.
42-
return { mode: "none" };
43-
}
44-
45-
// "paid" is not set here, just as before. Also, it's we should remove this whole method once the org-migration is done, and center all capabilities around Organizations
46-
return {
47-
mode: "usage-based",
48-
};
49-
}
5034
}

‎components/server/src/billing/entitlement-service-ubp.ts

Lines changed: 10 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
* See License.AGPL.txt in the project root for license information.
55
*/
66

7-
import { TeamDB } from "@gitpod/gitpod-db/lib";
87
import {
98
WorkspaceInstance,
109
WorkspaceTimeoutDuration,
@@ -14,7 +13,6 @@ import {
1413
WORKSPACE_LIFETIME_SHORT,
1514
User,
1615
BillingTier,
17-
Team,
1816
} from "@gitpod/gitpod-protocol";
1917
import { AttributionId } from "@gitpod/gitpod-protocol/lib/attribution";
2018
import { inject, injectable } from "inversify";
@@ -31,10 +29,7 @@ const MAX_PARALLEL_WORKSPACES_PAID = 16;
3129
*/
3230
@injectable()
3331
export class EntitlementServiceUBP implements EntitlementService {
34-
constructor(
35-
@inject(UsageService) private readonly usageService: UsageService,
36-
@inject(TeamDB) private readonly teamDB: TeamDB,
37-
) {}
32+
constructor(@inject(UsageService) private readonly usageService: UsageService) {}
3833

3934
async mayStartWorkspace(
4035
user: User,
@@ -79,7 +74,7 @@ export class EntitlementServiceUBP implements EntitlementService {
7974
}
8075
}
8176

82-
async maySetTimeout(userId: string, organizationId?: string): Promise<boolean> {
77+
async maySetTimeout(userId: string, organizationId: string): Promise<boolean> {
8378
return this.hasPaidSubscription(userId, organizationId);
8479
}
8580

@@ -109,48 +104,15 @@ export class EntitlementServiceUBP implements EntitlementService {
109104
return true;
110105
}
111106

112-
private async hasPaidSubscription(userId: string, organizationId?: string): Promise<boolean> {
113-
if (organizationId) {
114-
try {
115-
// This is the "stricter", more correct version: We only allow privileges on the Organization that is paying for it
116-
const { billingStrategy } = await this.usageService.getCostCenter(userId, organizationId);
117-
return billingStrategy === CostCenter_BillingStrategy.BILLING_STRATEGY_STRIPE;
118-
} catch (err) {
119-
log.warn({ userId, organizationId }, "Error checking if user is subscribed to organization", err);
120-
return false;
121-
}
122-
}
123-
124-
// TODO(gpl) Remove everything below once organizations are fully rolled out
125-
// This is the old behavior, stemming from our transition to PAYF, where our API did-/doesn't pass organizationId, yet
126-
// Member of paid team?
127-
const teams = await this.teamDB.findTeamsByUser(userId);
128-
const isTeamSubscribedPromises = teams.map(async (team: Team) => {
129-
const { billingStrategy } = await this.usageService.getCostCenter(userId, team.id);
107+
private async hasPaidSubscription(userId: string, organizationId: string): Promise<boolean> {
108+
try {
109+
// This is the "stricter", more correct version: We only allow privileges on the Organization that is paying for it
110+
const { billingStrategy } = await this.usageService.getCostCenter(userId, organizationId);
130111
return billingStrategy === CostCenter_BillingStrategy.BILLING_STRATEGY_STRIPE;
131-
});
132-
// Return the first truthy promise, or false if all the promises were falsy.
133-
// Source: https://gist.github.com/jbreckmckye/66364021ebaa0785e426deec0410a235
134-
return new Promise((resolve, reject) => {
135-
// If any promise returns true, immediately resolve with true
136-
isTeamSubscribedPromises.forEach(async (isTeamSubscribedPromise: Promise<boolean>) => {
137-
try {
138-
const isTeamSubscribed = await isTeamSubscribedPromise;
139-
if (isTeamSubscribed) resolve(true);
140-
} catch (err) {
141-
log.warn({ userId, organizationId }, "Error checking if user is subscribed to organization", err);
142-
resolve(false);
143-
}
144-
});
145-
146-
// If neither of the above fires, resolve with false
147-
// Check truthiness just in case callbacks fire out-of-band
148-
Promise.all(isTeamSubscribedPromises)
149-
.then((areTeamsSubscribed) => {
150-
resolve(!!areTeamsSubscribed.find((isTeamSubscribed: boolean) => !!isTeamSubscribed));
151-
})
152-
.catch(reject);
153-
});
112+
} catch (err) {
113+
log.warn({ userId, organizationId }, "Error checking if user is subscribed to organization", err);
114+
return false;
115+
}
154116
}
155117

156118
async getBillingTier(userId: string, organizationId: string): Promise<BillingTier> {

‎components/server/src/billing/entitlement-service.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ export interface EntitlementService {
5454
* @param userId
5555
* @param organizationId
5656
*/
57-
maySetTimeout(userId: string, organizationId?: string): Promise<boolean>;
57+
maySetTimeout(userId: string, organizationId: string): Promise<boolean>;
5858

5959
/**
6060
* Returns the default workspace timeout for the given user at a given point in time
@@ -127,10 +127,9 @@ export class EntitlementServiceImpl implements EntitlementService {
127127
}
128128
}
129129

130-
async maySetTimeout(userId: string, organizationId?: string): Promise<boolean> {
130+
async maySetTimeout(userId: string, organizationId: string): Promise<boolean> {
131131
try {
132-
// TODO(gpl): We need to replace this with ".getBillingMode(user.id, organizationId);" once all callers forward organizationId
133-
const billingMode = await this.billingModes.getBillingModeForUser();
132+
const billingMode = await this.billingModes.getBillingMode(userId, organizationId);
134133
switch (billingMode.mode) {
135134
case "none":
136135
// when payment is disabled users can do everything

‎components/server/src/user/user-service.ts

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import { CreateUserParams } from "./user-authentication";
2323
import { IAnalyticsWriter } from "@gitpod/gitpod-protocol/lib/analytics";
2424
import { TransactionalContext } from "@gitpod/gitpod-db/lib/typeorm/transactional-db-impl";
2525
import { RelationshipUpdater } from "../authorization/relationship-updater";
26-
import { EntitlementService } from "../billing/entitlement-service";
2726

2827
@injectable()
2928
export class UserService {
@@ -33,7 +32,6 @@ export class UserService {
3332
@inject(Authorizer) private readonly authorizer: Authorizer,
3433
@inject(IAnalyticsWriter) private readonly analytics: IAnalyticsWriter,
3534
@inject(RelationshipUpdater) private readonly relationshipUpdater: RelationshipUpdater,
36-
@inject(EntitlementService) private readonly entitlementService: EntitlementService,
3735
) {}
3836

3937
public async createUser(
@@ -144,13 +142,6 @@ export class UserService {
144142
}
145143
}
146144

147-
if (!(await this.entitlementService.maySetTimeout(targetUserId))) {
148-
throw new ApplicationError(
149-
ErrorCodes.PERMISSION_DENIED,
150-
"Configure workspace timeout only available for paid user.",
151-
);
152-
}
153-
154145
const user = await this.findUserById(userId, targetUserId);
155146
AdditionalUserData.set(user, setting);
156147
await this.userDb.updateUserPartial(user);

‎components/server/src/workspace/gitpod-server-impl.ts

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,6 @@ import {
137137
import { ListUsageRequest, ListUsageResponse } from "@gitpod/gitpod-protocol/lib/usage";
138138
import { VerificationService } from "../auth/verification-service";
139139
import { BillingMode } from "@gitpod/gitpod-protocol/lib/billing-mode";
140-
import { EntitlementService } from "../billing/entitlement-service";
141140
import { formatPhoneNumber } from "../user/phone-numbers";
142141
import { IDEService } from "../ide-service";
143142
import { AttributionId } from "@gitpod/gitpod-protocol/lib/attribution";
@@ -240,7 +239,6 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
240239
@inject(IDEService) private readonly ideService: IDEService,
241240

242241
@inject(VerificationService) private readonly verificationService: VerificationService,
243-
@inject(EntitlementService) private readonly entitlementService: EntitlementService,
244242

245243
@inject(Authorizer) private readonly auth: Authorizer,
246244

@@ -669,14 +667,6 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
669667
return updatedUser;
670668
}
671669

672-
public async maySetTimeout(ctx: TraceContext): Promise<boolean> {
673-
const user = await this.checkUser("maySetTimeout");
674-
await this.guardAccess({ kind: "user", subject: user }, "get");
675-
await this.auth.checkPermissionOnUser(user.id, "read_info", user.id);
676-
677-
return await this.entitlementService.maySetTimeout(user.id);
678-
}
679-
680670
public async updateWorkspaceTimeoutSetting(
681671
ctx: TraceContext,
682672
setting: Partial<WorkspaceTimeoutSetting>,

0 commit comments

Comments
 (0)