Skip to content

Commit 84b65a8

Browse files
BrunoQuaresmaBrunoQuaresma
committed
Apply PR reviews
1 parent 139548c commit 84b65a8

File tree

5 files changed

+40
-39
lines changed

5 files changed

+40
-39
lines changed

‎site/src/modules/workspaces/WorkspaceMoreActions/ChangeWorkspaceVersionDialog.tsx

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { Pill } from "components/Pill/Pill";
1616
import { Stack } from "components/Stack/Stack";
1717
import { InfoIcon } from "lucide-react";
1818
import { TemplateUpdateMessage } from "modules/templates/TemplateUpdateMessage";
19-
import { type FC, useEffect, useState } from "react";
19+
import { type FC, useState } from "react";
2020
import { useQuery } from "react-query";
2121
import { createDayString } from "utils/createDayString";
2222

@@ -31,28 +31,23 @@ export const ChangeWorkspaceVersionDialog: FC<
3131
> = ({ workspace, onClose, onConfirm, ...dialogProps }) => {
3232
const { data: versions } = useQuery({
3333
...templateVersions(workspace.template_id),
34-
select: (data) => data.reverse(),
34+
select: (data) => [...data].reverse(),
3535
});
3636
const [isAutocompleteOpen, setIsAutocompleteOpen] = useState(false);
3737
const activeVersion = versions?.find(
3838
(v) => workspace.template_active_version_id === v.id,
3939
);
40-
const [selectedVersion, setSelectedVersion] = useState<TemplateVersion>();
40+
const [newVersion, setNewVersion] = useState<TemplateVersion>();
4141
const validVersions = versions?.filter((v) => v.job.status === "succeeded");
42-
43-
useEffect(() => {
44-
if (activeVersion) {
45-
setSelectedVersion(activeVersion);
46-
}
47-
}, [activeVersion]);
42+
const selectedVersion = newVersion || activeVersion;
4843

4944
return (
5045
<ConfirmDialog
5146
{...dialogProps}
5247
onClose={onClose}
5348
onConfirm={() => {
54-
if (selectedVersion) {
55-
onConfirm(selectedVersion);
49+
if (newVersion) {
50+
onConfirm(newVersion);
5651
}
5752
}}
5853
hideCancel={false}
@@ -73,7 +68,7 @@ export const ChangeWorkspaceVersionDialog: FC<
7368
id="template-version-autocomplete"
7469
open={isAutocompleteOpen}
7570
onChange={(_, newTemplateVersion) => {
76-
setSelectedVersion(newTemplateVersion);
71+
setNewVersion(newTemplateVersion);
7772
}}
7873
onOpen={() => {
7974
setIsAutocompleteOpen(true);

‎site/src/modules/workspaces/WorkspaceMoreActions/WorkspaceMoreActions.tsx

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {
2121
SettingsIcon,
2222
TrashIcon,
2323
} from "lucide-react";
24-
import { type FC, useState } from "react";
24+
import { type FC, useEffect, useState } from "react";
2525
import { useMutation, useQuery, useQueryClient } from "react-query";
2626
import { Link as RouterLink } from "react-router-dom";
2727
import { ChangeWorkspaceVersionDialog } from "./ChangeWorkspaceVersionDialog";
@@ -32,7 +32,7 @@ import { useWorkspaceDuplication } from "./useWorkspaceDuplication";
3232

3333
type WorkspaceMoreActionsProps = {
3434
workspace: Workspace;
35-
disabled?: boolean;
35+
disabled: boolean;
3636
};
3737

3838
export const WorkspaceMoreActions: FC<WorkspaceMoreActionsProps> = ({
@@ -63,9 +63,17 @@ export const WorkspaceMoreActions: FC<WorkspaceMoreActionsProps> = ({
6363
const { duplicateWorkspace, isDuplicationReady } =
6464
useWorkspaceDuplication(workspace);
6565

66+
// Since the workspace state is not updated immediately after the mutation, we
67+
// need to be sure the menu is closed when the action gets disabled.
68+
// Reference: https://github.com/coder/coder/pull/17775#discussion_r2087273706
69+
const [open, setOpen] = useState(false);
70+
useEffect(() => {
71+
setOpen((open) => (disabled ? false : open));
72+
});
73+
6674
return (
6775
<>
68-
<DropdownMenu>
76+
<DropdownMenu open={open} onOpenChange={setOpen}>
6977
<DropdownMenuTrigger asChild>
7078
<Button
7179
size="icon-lg"
@@ -136,16 +144,16 @@ export const WorkspaceMoreActions: FC<WorkspaceMoreActionsProps> = ({
136144

137145
<UpdateBuildParametersDialog
138146
missedParameters={
139-
isMissingBuildParameters(changeVersionMutation.error)
147+
changeVersionMutation.error instanceof MissingBuildParameters
140148
? changeVersionMutation.error.parameters
141149
: []
142150
}
143-
open={isMissingBuildParameters(changeVersionMutation.error)}
151+
open={changeVersionMutation.error instanceof MissingBuildParameters}
144152
onClose={() => {
145153
changeVersionMutation.reset();
146154
}}
147155
onUpdate={(buildParameters) => {
148-
if (isMissingBuildParameters(changeVersionMutation.error)) {
156+
if (changeVersionMutation.error instanceof MissingBuildParameters) {
149157
changeVersionMutation.mutate({
150158
versionId: changeVersionMutation.error.versionId,
151159
buildParameters,
@@ -181,7 +189,3 @@ export const WorkspaceMoreActions: FC<WorkspaceMoreActionsProps> = ({
181189
</>
182190
);
183191
};
184-
185-
const isMissingBuildParameters = (e: unknown): e is MissingBuildParameters => {
186-
return Boolean(e && e instanceof MissingBuildParameters);
187-
};

‎site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.stories.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { Meta, StoryObj } from "@storybook/react";
22
import { expect, userEvent, within } from "@storybook/test";
3+
import { deploymentConfigQueryKey } from "api/queries/deployment";
34
import { agentLogsKey, buildLogsKey } from "api/queries/workspaces";
45
import * as Mocks from "testHelpers/entities";
56
import {
@@ -8,7 +9,6 @@ import {
89
withDesktopViewport,
910
} from "testHelpers/storybook";
1011
import { WorkspaceActions } from "./WorkspaceActions";
11-
import { deploymentConfigQueryKey } from "api/queries/deployment";
1212

1313
const meta: Meta<typeof WorkspaceActions> = {
1414
title: "pages/WorkspacePage/WorkspaceActions",

‎site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ export const WorkspaceActions: FC<WorkspaceActionsProps> = ({
6565
workspace,
6666
{
6767
canDebug: !!deployment?.config.enable_terraform_debug_mode,
68-
isOwner: !!user.roles.find((role) => role.name === "owner"),
68+
isOwner: user.roles.some((role) => role.name === "owner"),
6969
},
7070
);
7171

‎site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.tsx

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import Button from "@mui/material/Button";
33
import { API } from "api/api";
44
import { isApiValidationError } from "api/errors";
55
import { checkAuthorization } from "api/queries/authCheck";
6-
import { templateByName } from "api/queries/templates";
76
import type { Workspace, WorkspaceBuildParameter } from "api/typesGenerated";
87
import { ErrorAlert } from "components/Alert/ErrorAlert";
98
import { EmptyState } from "components/EmptyState/EmptyState";
@@ -43,17 +42,11 @@ const WorkspaceParametersPage: FC = () => {
4342
},
4443
});
4544

46-
const templateQuery = useQuery({
47-
...templateByName(workspace.organization_id, workspace.template_name ?? ""),
48-
enabled: workspace !== undefined,
49-
});
50-
const template = templateQuery.data;
51-
5245
// Permissions
53-
const checks = workspace && template ? workspaceChecks(workspace) : {};
46+
const checks = workspace ? workspaceChecks(workspace) : {};
5447
const permissionsQuery = useQuery({
5548
...checkAuthorization({ checks }),
56-
enabled: workspace !== undefined && template !== undefined,
49+
enabled: workspace !== undefined,
5750
});
5851
const permissions = permissionsQuery.data as WorkspacePermissions | undefined;
5952
const canChangeVersions = Boolean(permissions?.updateWorkspaceVersion);
@@ -71,14 +64,23 @@ const WorkspaceParametersPage: FC = () => {
7164
submitError={updateParameters.error}
7265
isSubmitting={updateParameters.isLoading}
7366
onSubmit={(values) => {
67+
if (!parameters.data) {
68+
return;
69+
}
7470
// When updating the parameters, the API does not accept immutable
7571
// values so we need to filter them
76-
const onlyMultableValues = parameters
77-
.data!.templateVersionRichParameters.filter((p) => p.mutable)
78-
.map(
79-
(p) =>
80-
values.rich_parameter_values.find((v) => v.name === p.name)!,
81-
);
72+
const onlyMultableValues =
73+
parameters.data.templateVersionRichParameters
74+
.filter((p) => p.mutable)
75+
.map((p) => {
76+
const value = values.rich_parameter_values.find(
77+
(v) => v.name === p.name,
78+
);
79+
if (!value) {
80+
throw new Error(`Missing value for parameter ${p.name}`);
81+
}
82+
return value;
83+
});
8284
updateParameters.mutate(onlyMultableValues);
8385
}}
8486
onCancel={() => {

0 commit comments

Comments
 (0)