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.

Sign up for GitHub

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

[api, server, dashboard] Cleanup UpdateOrganizationSettings API #20603

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
+1,335 −1,055
Draft

[api, server, dashboard] Cleanup UpdateOrganizationSettings API #20603

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fa14fac
[api, server, dashboard] Cleanup UpdateOrganizationSettings API
geropl Feb 18, 2025
f36f3a1
Org settings partial updates improvements (#20626)
filiptronicek Feb 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion components/dashboard/src/data/organizations/org-settings-query.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,6 @@ export function useOrgSettingsQuery() {
);
}

function getQueryKey(organizationId?: string) {
export function getQueryKey(organizationId?: string) {
return ["getOrganizationSettings", organizationId || "undefined"];
}
10 changes: 5 additions & 5 deletions components/dashboard/src/data/organizations/suggested-repositories-query.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@

import { useQuery, useQueryClient } from "@tanstack/react-query";
import { useCallback } from "react";
import { configurationClient, organizationClient } from "../../service/public-api";
import { configurationClient } from "../../service/public-api";
import { useCurrentOrg } from "./orgs-query";
import { SuggestedRepository } from "@gitpod/public-api/lib/gitpod/v1/scm_pb";
import { PlainMessage } from "@bufbuild/protobuf";
import { Configuration } from "@gitpod/public-api/lib/gitpod/v1/configuration_pb";
import { useOrgSettingsQuery } from "./org-settings-query";

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

export function useOrgSuggestedRepos() {
const organizationId = useCurrentOrg().data?.id;
const orgSettings = useOrgSettingsQuery();

const query = useQuery<SuggestedOrgRepository[], Error>(
getQueryKey(organizationId),
async () => {
const response = await organizationClient.getOrganizationSettings({
organizationId,
});
const repos = response.settings?.onboardingSettings?.recommendedRepositories ?? [];
const repos = orgSettings.data?.onboardingSettings?.recommendedRepositories ?? [];

const suggestions: SuggestedOrgRepository[] = [];
for (const configurationId of repos) {
Expand Down
21 changes: 14 additions & 7 deletions components/dashboard/src/data/organizations/update-org-settings-mutation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
* See License.AGPL.txt in the project root for license information.
*/

import { useMutation } from "@tanstack/react-query";
import { useOrgSettingsQueryInvalidator } from "./org-settings-query";
import { useMutation, useQueryClient } from "@tanstack/react-query";
import { getQueryKey, useOrgSettingsQueryInvalidator } from "./org-settings-query";
import { useCurrentOrg } from "./orgs-query";
import { organizationClient } from "../../service/public-api";
import {
Expand All @@ -24,11 +24,13 @@ export const useUpdateOrgSettingsMutation = () => {
const invalidateOrgSettings = useOrgSettingsQueryInvalidator();
const invalidateWorkspaceClasses = useOrgWorkspaceClassesQueryInvalidator();
const invalidateOrgRepoSuggestions = useOrgRepoSuggestionsInvalidator();

const queryClient = useQueryClient();
const organizationId = org?.id ?? "";

return useMutation<OrganizationSettings, Error, UpdateOrganizationSettingsArgs>({
mutationFn: async (partialUpdate) => {
const update: PartialMessage<UpdateOrganizationSettingsRequest> = {
const update: UpdateOrganizationSettingsArgs = {
...partialUpdate,
};
update.organizationId = organizationId;
Expand All @@ -44,13 +46,18 @@ export const useUpdateOrgSettingsMutation = () => {
}
}

const settings = await organizationClient.updateOrganizationSettings(update);
return settings.settings!;
const { settings } = await organizationClient.updateOrganizationSettings(update);
return settings!;
},
onSuccess: () => {
invalidateOrgSettings();
onSuccess: (settings) => {
invalidateWorkspaceClasses();
invalidateOrgRepoSuggestions();

if (settings) {
queryClient.setQueryData(getQueryKey(organizationId), settings);
} else {
invalidateOrgSettings();
}
},
onError: (err) => {
if (!ErrorCode.isUserError((err as any)?.["code"])) {
Expand Down
81 changes: 43 additions & 38 deletions components/dashboard/src/service/json-rpc-organization-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* See License.AGPL.txt in the project root for license information.
*/

import { PartialMessage } from "@bufbuild/protobuf";
import { PartialMessage, PlainMessage } from "@bufbuild/protobuf";
import { CallOptions, PromiseClient } from "@connectrpc/connect";
import { OrganizationService } from "@gitpod/public-api/lib/gitpod/v1/organization_connect";
import {
Expand Down Expand Up @@ -41,7 +41,6 @@ import {
import { getGitpodService } from "./service";
import { converter } from "./public-api";
import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
import { OrgMemberRole, RoleRestrictions } from "@gitpod/gitpod-protocol";

export class JsonRpcOrganizationClient implements PromiseClient<typeof OrganizationService> {
async createOrganization(
Expand Down Expand Up @@ -228,56 +227,62 @@ export class JsonRpcOrganizationClient implements PromiseClient<typeof Organizat
if (!request.organizationId) {
throw new ApplicationError(ErrorCodes.BAD_REQUEST, "organizationId is required");
}
const update: Partial<OrganizationSettings> = {
workspaceSharingDisabled: request?.workspaceSharingDisabled,
defaultWorkspaceImage: request?.defaultWorkspaceImage,
allowedWorkspaceClasses: request?.allowedWorkspaceClasses,
restrictedEditorNames: request?.restrictedEditorNames,
defaultRole: request?.defaultRole,
};
if (request.updatePinnedEditorVersions) {
update.pinnedEditorVersions = request.pinnedEditorVersions;
} else if (request.pinnedEditorVersions && Object.keys(request.pinnedEditorVersions).length > 0) {

if (
request.restrictedEditorNames &&
request.restrictedEditorNames.length > 0 &&
!request.updateRestrictedEditorNames
) {
throw new ApplicationError(
ErrorCodes.BAD_REQUEST,
"updatePinnedEditorVersions is required to be true to update pinnedEditorVersions",
"updateRestrictedEditorNames is required to be true to update restrictedEditorNames",
);
}
if (request.updateRestrictedEditorNames) {
update.restrictedEditorNames = request.restrictedEditorNames;
} else if (request.restrictedEditorNames && request.restrictedEditorNames.length > 0) {

if (
request.allowedWorkspaceClasses &&
request.allowedWorkspaceClasses.length > 0 &&
!request.updateAllowedWorkspaceClasses
) {
throw new ApplicationError(
ErrorCodes.BAD_REQUEST,
"updateRestrictedEditorNames is required to be true to update restrictedEditorNames",
"updateAllowedWorkspaceClasses is required to be true to update allowedWorkspaceClasses",
);
}
const roleRestrictions: RoleRestrictions = {};
if (request.updateRoleRestrictions) {
for (const roleRestriction of request?.roleRestrictions ?? []) {
if (!roleRestriction.role) {
throw new ApplicationError(ErrorCodes.BAD_REQUEST, "role is required");
}
const role = converter.fromOrgMemberRole(roleRestriction.role);
const permissions = roleRestriction?.permissions?.map((p) => converter.fromOrganizationPermission(p));

roleRestrictions[role] = permissions;
}
} else if (request.roleRestrictions && Object.keys(request.roleRestrictions).length > 0) {
if (
request.pinnedEditorVersions &&
Object.keys(request.pinnedEditorVersions).length > 0 &&
!request.updatePinnedEditorVersions
) {
throw new ApplicationError(
ErrorCodes.BAD_REQUEST,
"updateRoleRestrictions is required to be true to update roleRestrictions",
"updatePinnedEditorVersions is required to be true to update pinnedEditorVersions",
);
}

await getGitpodService().server.updateOrgSettings(request.organizationId, {
...update,
defaultRole: request.defaultRole as OrgMemberRole,
timeoutSettings: {
inactivity: converter.toDurationStringOpt(request.timeoutSettings?.inactivity),
denyUserTimeouts: request.timeoutSettings?.denyUserTimeouts,
},
roleRestrictions,
});
if (request.roleRestrictions && request.roleRestrictions.length > 0 && !request.updateRoleRestrictions) {
throw new ApplicationError(
ErrorCodes.BAD_REQUEST,
"updateRoleRestrictions is required to be true when updating roleRestrictions",
);
}
if (
request.onboardingSettings?.recommendedRepositories &&
request.onboardingSettings.recommendedRepositories.length > 0 &&
!request.onboardingSettings.updateRecommendedRepositories
) {
throw new ApplicationError(
ErrorCodes.BAD_REQUEST,
"recommendedRepositories can only be set when updateRecommendedRepositories is true",
);
}

// gpl: We accept the little bit of uncertainty here because a) the partial/not-partial mismatch is only about
// technical/private fields and b) this path should not be exercised anymore anyway.
const update = converter.fromOrganizationSettings(request as PlainMessage<OrganizationSettings>);

await getGitpodService().server.updateOrgSettings(request.organizationId, update);
return new UpdateOrganizationSettingsResponse();
}
}
7 changes: 2 additions & 5 deletions components/dashboard/src/teams/TeamOnboarding.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,7 @@ export default function TeamOnboardingPage() {
throw new Error("no organization settings change permission");
}
try {
await updateTeamSettings.mutateAsync({
...settings,
...newSettings,
});
await updateTeamSettings.mutateAsync(newSettings);
toast("Organization settings updated");
} catch (error) {
if (options?.throwMutateError) {
Expand All @@ -66,7 +63,7 @@ export default function TeamOnboardingPage() {
console.error(error);
}
},
[updateTeamSettings, org?.id, isOwner, settings, toast],
[updateTeamSettings, org?.id, isOwner, toast],
);

const handleUpdateInternalLink = useCallback(
Expand Down
2 changes: 1 addition & 1 deletion components/dashboard/src/teams/onboarding/OrgMemberAvatarInput.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export const OrgMemberAvatarInput = ({ settings, setFeaturedMemberId }: Props) =
const handleSelectionChange = useCallback(
(selectedId: string) => {
const member = members?.find((m) => m.userId === selectedId);
setFeaturedMemberId(selectedId || undefined);
setFeaturedMemberId(selectedId);
setAvatarUrl(member?.avatarUrl);
},
[members, setFeaturedMemberId],
Expand Down
14 changes: 6 additions & 8 deletions components/dashboard/src/teams/onboarding/WelcomeMessageConfigurationField.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
* See License.AGPL.txt in the project root for license information.
*/

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

const handleUpdateWelcomeMessage = useCallback(
async (newSettings: PlainMessage<OnboardingSettings_WelcomeMessage>, options?: UpdateTeamSettingsOptions) => {
async (
newSettings: NonNullable<UpdateOrganizationSettingsArgs["onboardingSettings"]>["welcomeMessage"],
options?: UpdateTeamSettingsOptions,
) => {
await handleUpdateTeamSettings(
{
onboardingSettings: {
welcomeMessage: {
...settings?.onboardingSettings?.welcomeMessage,
...newSettings,
},
welcomeMessage: newSettings,
},
},
options,
);
},
[handleUpdateTeamSettings, settings?.onboardingSettings?.welcomeMessage],
[handleUpdateTeamSettings],
);

return (
Expand Down
7 changes: 3 additions & 4 deletions components/dashboard/src/teams/onboarding/WelcomeMessageEditor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
* See License.AGPL.txt in the project root for license information.
*/

import { PlainMessage } from "@bufbuild/protobuf";
import type { OnboardingSettings_WelcomeMessage } from "@gitpod/public-api/lib/gitpod/v1/organization_pb";
import { Button } from "@podkit/buttons/Button";
import { LoadingButton } from "@podkit/buttons/LoadingButton";
Expand All @@ -17,14 +16,15 @@ import { TextInput } from "../../components/forms/TextInputField";
import { UpdateTeamSettingsOptions } from "../TeamOnboarding";
import { OrgMemberAvatarInput } from "./OrgMemberAvatarInput";
import { gitpodWelcomeSubheading } from "./WelcomeMessageConfigurationField";
import { UpdateOrganizationSettingsArgs } from "../../data/organizations/update-org-settings-mutation";

type Props = {
settings: OnboardingSettings_WelcomeMessage | undefined;
isLoading: boolean;
isOwner: boolean;
isOpen: boolean;
handleUpdateWelcomeMessage: (
newSettings: PlainMessage<OnboardingSettings_WelcomeMessage>,
newSettings: NonNullable<UpdateOrganizationSettingsArgs["onboardingSettings"]>["welcomeMessage"],
options?: UpdateTeamSettingsOptions,
) => Promise<void>;
setIsOpen: (isOpen: boolean) => void;
Expand All @@ -49,7 +49,6 @@ export const WelcomeMessageEditorModal = ({
{
message,
featuredMemberId,
enabled: settings?.enabled ?? false,
},
{
throwMutateError: true,
Expand All @@ -61,7 +60,7 @@ export const WelcomeMessageEditorModal = ({
setError(error.message);
}
},
[handleUpdateWelcomeMessage, message, featuredMemberId, settings?.enabled, setIsOpen],
[handleUpdateWelcomeMessage, message, featuredMemberId, setIsOpen],
);

return (
Expand Down
18 changes: 9 additions & 9 deletions components/gitpod-db/src/typeorm/entity/db-team-settings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,34 +25,34 @@ export class DBOrgSettings implements OrganizationSettings {
workspaceSharingDisabled?: boolean;

@Column("varchar", { nullable: true })
defaultWorkspaceImage?: string | null;
defaultWorkspaceImage?: string;
Copy link
Member Author

Choose a reason for hiding this comment

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

The null is treated as special value by typeorm to clear the column.
This is the only place we did this, and it's also unnecessary: We treat the default value "" the same anyway. 🤷 So we can get rid of that complexity.


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

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

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

@Column("varchar", { nullable: true })
defaultRole?: OrgMemberRole | undefined;
defaultRole?: OrgMemberRole;

@Column("json", { nullable: true })
timeoutSettings?: TimeoutSettings | undefined;
timeoutSettings?: TimeoutSettings;

@Column("json", { nullable: true })
roleRestrictions?: RoleRestrictions | undefined;
roleRestrictions?: RoleRestrictions;

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

@Column("json", { nullable: true })
onboardingSettings?: OnboardingSettings | undefined;
onboardingSettings?: OnboardingSettings;

@Column({ type: "boolean", default: false })
annotateGitCommits?: boolean | undefined;
annotateGitCommits?: boolean;

@Column()
deleted: boolean;
Expand Down
10 changes: 5 additions & 5 deletions components/gitpod-protocol/src/teams-projects-protocol.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,15 +221,15 @@ export interface Organization {

export interface OrganizationSettings {
workspaceSharingDisabled?: boolean;
// null or empty string to reset to default
defaultWorkspaceImage?: string | null;
// undefined or empty string to reset to default
defaultWorkspaceImage?: string;

// empty array to allow all kind of workspace classes
allowedWorkspaceClasses?: string[] | null;
allowedWorkspaceClasses?: string[];

pinnedEditorVersions?: { [key: string]: string } | null;
pinnedEditorVersions?: { [key: string]: string };

restrictedEditorNames?: string[] | null;
restrictedEditorNames?: string[];

// what role new members will get, default is "member"
defaultRole?: OrgMemberRole;
Expand Down
Loading
Loading