Skip to content

Commit dd50c2a

Browse files
geroplgeropl
authored
[api, server, dashboard] Cleanup UpdateOrganizationSettings API (#20603)
* [api, server, dashboard] Cleanup UpdateOrganizationSettings API Tool: gitpod/catfood.gitpod.cloud * Org settings partial updates improvements (#20626) Tool: gitpod/catfood.gitpod.cloud * review comment Tool: gitpod/catfood.gitpod.cloud
1 parent 16468dc commit dd50c2a

File tree

19 files changed

+1321
-1048
lines changed

19 files changed

+1321
-1048
lines changed

‎components/dashboard/src/data/organizations/org-settings-query.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,6 @@ export function useOrgSettingsQuery() {
3636
);
3737
}
3838

39-
function getQueryKey(organizationId?: string) {
39+
export function getQueryKey(organizationId?: string) {
4040
return ["getOrganizationSettings", organizationId || "undefined"];
4141
}

‎components/dashboard/src/data/organizations/suggested-repositories-query.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,12 @@
66

77
import { useQuery, useQueryClient } from "@tanstack/react-query";
88
import { useCallback } from "react";
9-
import { configurationClient, organizationClient } from "../../service/public-api";
9+
import { configurationClient } from "../../service/public-api";
1010
import { useCurrentOrg } from "./orgs-query";
1111
import { SuggestedRepository } from "@gitpod/public-api/lib/gitpod/v1/scm_pb";
1212
import { PlainMessage } from "@bufbuild/protobuf";
1313
import { Configuration } from "@gitpod/public-api/lib/gitpod/v1/configuration_pb";
14+
import { useOrgSettingsQuery } from "./org-settings-query";
1415

1516
export function useOrgRepoSuggestionsInvalidator() {
1617
const organizationId = useCurrentOrg().data?.id;
@@ -27,13 +28,12 @@ export type SuggestedOrgRepository = PlainMessage<SuggestedRepository> & {
2728

2829
export function useOrgSuggestedRepos() {
2930
const organizationId = useCurrentOrg().data?.id;
31+
const orgSettings = useOrgSettingsQuery();
32+
3033
const query = useQuery<SuggestedOrgRepository[], Error>(
3134
getQueryKey(organizationId),
3235
async () => {
33-
const response = await organizationClient.getOrganizationSettings({
34-
organizationId,
35-
});
36-
const repos = response.settings?.onboardingSettings?.recommendedRepositories ?? [];
36+
const repos = orgSettings.data?.onboardingSettings?.recommendedRepositories ?? [];
3737

3838
const suggestions: SuggestedOrgRepository[] = [];
3939
for (const configurationId of repos) {

‎components/dashboard/src/data/organizations/update-org-settings-mutation.ts

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

7-
import { useMutation } from "@tanstack/react-query";
8-
import { useOrgSettingsQueryInvalidator } from "./org-settings-query";
7+
import { useMutation, useQueryClient } from "@tanstack/react-query";
8+
import { getQueryKey, useOrgSettingsQueryInvalidator } from "./org-settings-query";
99
import { useCurrentOrg } from "./orgs-query";
1010
import { organizationClient } from "../../service/public-api";
1111
import {
@@ -24,11 +24,13 @@ export const useUpdateOrgSettingsMutation = () => {
2424
const invalidateOrgSettings = useOrgSettingsQueryInvalidator();
2525
const invalidateWorkspaceClasses = useOrgWorkspaceClassesQueryInvalidator();
2626
const invalidateOrgRepoSuggestions = useOrgRepoSuggestionsInvalidator();
27+
28+
const queryClient = useQueryClient();
2729
const organizationId = org?.id ?? "";
2830

2931
return useMutation<OrganizationSettings, Error, UpdateOrganizationSettingsArgs>({
3032
mutationFn: async (partialUpdate) => {
31-
const update: PartialMessage<UpdateOrganizationSettingsRequest> = {
33+
const update: UpdateOrganizationSettingsArgs = {
3234
...partialUpdate,
3335
};
3436
update.organizationId = organizationId;
@@ -44,13 +46,18 @@ export const useUpdateOrgSettingsMutation = () => {
4446
}
4547
}
4648

47-
const settings = await organizationClient.updateOrganizationSettings(update);
48-
return settings.settings!;
49+
const { settings } = await organizationClient.updateOrganizationSettings(update);
50+
return settings!;
4951
},
50-
onSuccess: () => {
51-
invalidateOrgSettings();
52+
onSuccess: (settings) => {
5253
invalidateWorkspaceClasses();
5354
invalidateOrgRepoSuggestions();
55+
56+
if (settings) {
57+
queryClient.setQueryData(getQueryKey(organizationId), settings);
58+
} else {
59+
invalidateOrgSettings();
60+
}
5461
},
5562
onError: (err) => {
5663
if (!ErrorCode.isUserError((err as any)?.["code"])) {

‎components/dashboard/src/service/json-rpc-organization-client.ts

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

7-
import { PartialMessage } from "@bufbuild/protobuf";
7+
import { PartialMessage, PlainMessage } from "@bufbuild/protobuf";
88
import { CallOptions, PromiseClient } from "@connectrpc/connect";
99
import { OrganizationService } from "@gitpod/public-api/lib/gitpod/v1/organization_connect";
1010
import {
@@ -41,7 +41,6 @@ import {
4141
import { getGitpodService } from "./service";
4242
import { converter } from "./public-api";
4343
import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
44-
import { OrgMemberRole, RoleRestrictions } from "@gitpod/gitpod-protocol";
4544

4645
export class JsonRpcOrganizationClient implements PromiseClient<typeof OrganizationService> {
4746
async createOrganization(
@@ -228,56 +227,62 @@ export class JsonRpcOrganizationClient implements PromiseClient<typeof Organizat
228227
if (!request.organizationId) {
229228
throw new ApplicationError(ErrorCodes.BAD_REQUEST, "organizationId is required");
230229
}
231-
const update: Partial<OrganizationSettings> = {
232-
workspaceSharingDisabled: request?.workspaceSharingDisabled,
233-
defaultWorkspaceImage: request?.defaultWorkspaceImage,
234-
allowedWorkspaceClasses: request?.allowedWorkspaceClasses,
235-
restrictedEditorNames: request?.restrictedEditorNames,
236-
defaultRole: request?.defaultRole,
237-
};
238-
if (request.updatePinnedEditorVersions) {
239-
update.pinnedEditorVersions = request.pinnedEditorVersions;
240-
} else if (request.pinnedEditorVersions && Object.keys(request.pinnedEditorVersions).length > 0) {
230+
231+
if (
232+
request.restrictedEditorNames &&
233+
request.restrictedEditorNames.length > 0 &&
234+
!request.updateRestrictedEditorNames
235+
) {
241236
throw new ApplicationError(
242237
ErrorCodes.BAD_REQUEST,
243-
"updatePinnedEditorVersions is required to be true to update pinnedEditorVersions",
238+
"updateRestrictedEditorNames is required to be true to update restrictedEditorNames",
244239
);
245240
}
246-
if (request.updateRestrictedEditorNames) {
247-
update.restrictedEditorNames = request.restrictedEditorNames;
248-
} else if (request.restrictedEditorNames && request.restrictedEditorNames.length > 0) {
241+
242+
if (
243+
request.allowedWorkspaceClasses &&
244+
request.allowedWorkspaceClasses.length > 0 &&
245+
!request.updateAllowedWorkspaceClasses
246+
) {
249247
throw new ApplicationError(
250248
ErrorCodes.BAD_REQUEST,
251-
"updateRestrictedEditorNames is required to be true to update restrictedEditorNames",
249+
"updateAllowedWorkspaceClasses is required to be true to update allowedWorkspaceClasses",
252250
);
253251
}
254-
const roleRestrictions: RoleRestrictions = {};
255-
if (request.updateRoleRestrictions) {
256-
for (const roleRestriction of request?.roleRestrictions ?? []) {
257-
if (!roleRestriction.role) {
258-
throw new ApplicationError(ErrorCodes.BAD_REQUEST, "role is required");
259-
}
260-
const role = converter.fromOrgMemberRole(roleRestriction.role);
261-
const permissions = roleRestriction?.permissions?.map((p) => converter.fromOrganizationPermission(p));
262252

263-
roleRestrictions[role] = permissions;
264-
}
265-
} else if (request.roleRestrictions && Object.keys(request.roleRestrictions).length > 0) {
253+
if (
254+
request.pinnedEditorVersions &&
255+
Object.keys(request.pinnedEditorVersions).length > 0 &&
256+
!request.updatePinnedEditorVersions
257+
) {
266258
throw new ApplicationError(
267259
ErrorCodes.BAD_REQUEST,
268-
"updateRoleRestrictions is required to be true to update roleRestrictions",
260+
"updatePinnedEditorVersions is required to be true to update pinnedEditorVersions",
269261
);
270262
}
271263

272-
await getGitpodService().server.updateOrgSettings(request.organizationId, {
273-
...update,
274-
defaultRole: request.defaultRole as OrgMemberRole,
275-
timeoutSettings: {
276-
inactivity: converter.toDurationStringOpt(request.timeoutSettings?.inactivity),
277-
denyUserTimeouts: request.timeoutSettings?.denyUserTimeouts,
278-
},
279-
roleRestrictions,
280-
});
264+
if (request.roleRestrictions && request.roleRestrictions.length > 0 && !request.updateRoleRestrictions) {
265+
throw new ApplicationError(
266+
ErrorCodes.BAD_REQUEST,
267+
"updateRoleRestrictions is required to be true when updating roleRestrictions",
268+
);
269+
}
270+
if (
271+
request.onboardingSettings?.recommendedRepositories &&
272+
request.onboardingSettings.recommendedRepositories.length > 0 &&
273+
!request.onboardingSettings.updateRecommendedRepositories
274+
) {
275+
throw new ApplicationError(
276+
ErrorCodes.BAD_REQUEST,
277+
"recommendedRepositories can only be set when updateRecommendedRepositories is true",
278+
);
279+
}
280+
281+
// gpl: We accept the little bit of uncertainty here because a) the partial/not-partial mismatch is only about
282+
// technical/private fields and b) this path should not be exercised anymore anyway.
283+
const update = converter.fromOrganizationSettings(request as PlainMessage<OrganizationSettings>);
284+
285+
await getGitpodService().server.updateOrgSettings(request.organizationId, update);
281286
return new UpdateOrganizationSettingsResponse();
282287
}
283288
}

‎components/dashboard/src/teams/TeamOnboarding.tsx

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,7 @@ export default function TeamOnboardingPage() {
5353
throw new Error("no organization settings change permission");
5454
}
5555
try {
56-
await updateTeamSettings.mutateAsync({
57-
...settings,
58-
...newSettings,
59-
});
56+
await updateTeamSettings.mutateAsync(newSettings);
6057
toast("Organization settings updated");
6158
} catch (error) {
6259
if (options?.throwMutateError) {
@@ -66,7 +63,7 @@ export default function TeamOnboardingPage() {
6663
console.error(error);
6764
}
6865
},
69-
[updateTeamSettings, org?.id, isOwner, settings, toast],
66+
[updateTeamSettings, org?.id, isOwner, toast],
7067
);
7168

7269
const handleUpdateInternalLink = useCallback(

‎components/dashboard/src/teams/onboarding/OrgMemberAvatarInput.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export const OrgMemberAvatarInput = ({ settings, setFeaturedMemberId }: Props) =
2626
const handleSelectionChange = useCallback(
2727
(selectedId: string) => {
2828
const member = members?.find((m) => m.userId === selectedId);
29-
setFeaturedMemberId(selectedId || undefined);
29+
setFeaturedMemberId(selectedId);
3030
setAvatarUrl(member?.avatarUrl);
3131
},
3232
[members, setFeaturedMemberId],

‎components/dashboard/src/teams/onboarding/WelcomeMessageConfigurationField.tsx

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

7-
import { PlainMessage } from "@bufbuild/protobuf";
8-
import type { OnboardingSettings_WelcomeMessage } from "@gitpod/public-api/lib/gitpod/v1/organization_pb";
97
import { SwitchInputField } from "@podkit/switch/Switch";
108
import { Heading3, Subheading } from "@podkit/typography/Headings";
119
import { useCallback, useState } from "react";
@@ -40,20 +38,20 @@ export const WelcomeMessageConfigurationField = ({ handleUpdateTeamSettings }: P
4038
const [welcomeMessageEditorOpen, setWelcomeMessageEditorOpen] = useState(false);
4139

4240
const handleUpdateWelcomeMessage = useCallback(
43-
async (newSettings: PlainMessage<OnboardingSettings_WelcomeMessage>, options?: UpdateTeamSettingsOptions) => {
41+
async (
42+
newSettings: NonNullable<UpdateOrganizationSettingsArgs["onboardingSettings"]>["welcomeMessage"],
43+
options?: UpdateTeamSettingsOptions,
44+
) => {
4445
await handleUpdateTeamSettings(
4546
{
4647
onboardingSettings: {
47-
welcomeMessage: {
48-
...settings?.onboardingSettings?.welcomeMessage,
49-
...newSettings,
50-
},
48+
welcomeMessage: newSettings,
5149
},
5250
},
5351
options,
5452
);
5553
},
56-
[handleUpdateTeamSettings, settings?.onboardingSettings?.welcomeMessage],
54+
[handleUpdateTeamSettings],
5755
);
5856

5957
return (

‎components/dashboard/src/teams/onboarding/WelcomeMessageEditor.tsx

Lines changed: 3 additions & 4 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 { PlainMessage } from "@bufbuild/protobuf";
87
import type { OnboardingSettings_WelcomeMessage } from "@gitpod/public-api/lib/gitpod/v1/organization_pb";
98
import { Button } from "@podkit/buttons/Button";
109
import { LoadingButton } from "@podkit/buttons/LoadingButton";
@@ -17,14 +16,15 @@ import { TextInput } from "../../components/forms/TextInputField";
1716
import { UpdateTeamSettingsOptions } from "../TeamOnboarding";
1817
import { OrgMemberAvatarInput } from "./OrgMemberAvatarInput";
1918
import { gitpodWelcomeSubheading } from "./WelcomeMessageConfigurationField";
19+
import { UpdateOrganizationSettingsArgs } from "../../data/organizations/update-org-settings-mutation";
2020

2121
type Props = {
2222
settings: OnboardingSettings_WelcomeMessage | undefined;
2323
isLoading: boolean;
2424
isOwner: boolean;
2525
isOpen: boolean;
2626
handleUpdateWelcomeMessage: (
27-
newSettings: PlainMessage<OnboardingSettings_WelcomeMessage>,
27+
newSettings: NonNullable<UpdateOrganizationSettingsArgs["onboardingSettings"]>["welcomeMessage"],
2828
options?: UpdateTeamSettingsOptions,
2929
) => Promise<void>;
3030
setIsOpen: (isOpen: boolean) => void;
@@ -49,7 +49,6 @@ export const WelcomeMessageEditorModal = ({
4949
{
5050
message,
5151
featuredMemberId,
52-
enabled: settings?.enabled ?? false,
5352
},
5453
{
5554
throwMutateError: true,
@@ -61,7 +60,7 @@ export const WelcomeMessageEditorModal = ({
6160
setError(error.message);
6261
}
6362
},
64-
[handleUpdateWelcomeMessage, message, featuredMemberId, settings?.enabled, setIsOpen],
63+
[handleUpdateWelcomeMessage, message, featuredMemberId, setIsOpen],
6564
);
6665

6766
return (

‎components/gitpod-db/src/typeorm/entity/db-team-settings.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,34 +25,34 @@ export class DBOrgSettings implements OrganizationSettings {
2525
workspaceSharingDisabled?: boolean;
2626

2727
@Column("varchar", { nullable: true })
28-
defaultWorkspaceImage?: string | null;
28+
defaultWorkspaceImage?: string;
2929

3030
@Column("json", { nullable: true })
31-
allowedWorkspaceClasses?: string[] | null;
31+
allowedWorkspaceClasses?: string[];
3232

3333
@Column("json", { nullable: true })
34-
pinnedEditorVersions?: { [key: string]: string } | null;
34+
pinnedEditorVersions?: { [key: string]: string };
3535

3636
@Column("json", { nullable: true })
37-
restrictedEditorNames?: string[] | null;
37+
restrictedEditorNames?: string[];
3838

3939
@Column("varchar", { nullable: true })
40-
defaultRole?: OrgMemberRole | undefined;
40+
defaultRole?: OrgMemberRole;
4141

4242
@Column("json", { nullable: true })
43-
timeoutSettings?: TimeoutSettings | undefined;
43+
timeoutSettings?: TimeoutSettings;
4444

4545
@Column("json", { nullable: true })
46-
roleRestrictions?: RoleRestrictions | undefined;
46+
roleRestrictions?: RoleRestrictions;
4747

4848
@Column({ type: "int", default: 0 })
4949
maxParallelRunningWorkspaces: number;
5050

5151
@Column("json", { nullable: true })
52-
onboardingSettings?: OnboardingSettings | undefined;
52+
onboardingSettings?: OnboardingSettings;
5353

5454
@Column({ type: "boolean", default: false })
55-
annotateGitCommits?: boolean | undefined;
55+
annotateGitCommits?: boolean;
5656

5757
@Column()
5858
deleted: boolean;

‎components/gitpod-protocol/src/teams-projects-protocol.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -221,15 +221,15 @@ export interface Organization {
221221

222222
export interface OrganizationSettings {
223223
workspaceSharingDisabled?: boolean;
224-
// null or empty string to reset to default
225-
defaultWorkspaceImage?: string | null;
224+
// undefined or empty string to reset to default
225+
defaultWorkspaceImage?: string;
226226

227227
// empty array to allow all kind of workspace classes
228-
allowedWorkspaceClasses?: string[] | null;
228+
allowedWorkspaceClasses?: string[];
229229

230-
pinnedEditorVersions?: { [key: string]: string } | null;
230+
pinnedEditorVersions?: { [key: string]: string };
231231

232-
restrictedEditorNames?: string[] | null;
232+
restrictedEditorNames?: string[];
233233

234234
// what role new members will get, default is "member"
235235
defaultRole?: OrgMemberRole;

0 commit comments

Comments
 (0)