mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
chore: remove unecessary API.getWorkspaceParameters (#20462)
I initially created `API.getWorkspaceParameters` to group two related requests, but after revisiting the implementation, I realized this abstraction doesn’t add much value. It also prevents us from taking full advantage of React Query’s built-in caching and invalidation. So instead of grouping them, I removed the helper and replaced it with separate queries — this simplifies the flow and lets React Query handle caching more efficiently. Related to https://github.com/coder/coder/pull/20431#discussion_r2457010137
This commit is contained in:
@@ -2385,19 +2385,6 @@ class ApiMethods {
|
||||
return response.data;
|
||||
};
|
||||
|
||||
getWorkspaceParameters = async (workspace: TypesGen.Workspace) => {
|
||||
const latestBuild = workspace.latest_build;
|
||||
const [templateVersionRichParameters, buildParameters] = await Promise.all([
|
||||
this.getTemplateVersionRichParameters(latestBuild.template_version_id),
|
||||
this.getWorkspaceBuildParameters(latestBuild.id),
|
||||
]);
|
||||
|
||||
return {
|
||||
templateVersionRichParameters,
|
||||
buildParameters,
|
||||
};
|
||||
};
|
||||
|
||||
getInsightsUserLatency = async (
|
||||
filters: InsightsParams,
|
||||
): Promise<TypesGen.UserLatencyInsightsResponse> => {
|
||||
|
||||
@@ -249,9 +249,15 @@ export const templateVersionLogs = (versionId: string) => {
|
||||
};
|
||||
};
|
||||
|
||||
export const richParametersKey = (versionId: string) => [
|
||||
templateVersionRoot,
|
||||
versionId,
|
||||
"richParameters",
|
||||
];
|
||||
|
||||
export const richParameters = (versionId: string) => {
|
||||
return {
|
||||
queryKey: [templateVersionRoot, versionId, "richParameters"],
|
||||
queryKey: richParametersKey(versionId),
|
||||
queryFn: () => API.getTemplateVersionRichParameters(versionId),
|
||||
};
|
||||
};
|
||||
|
||||
@@ -6,7 +6,7 @@ import type {
|
||||
} from "api/typesGenerated";
|
||||
import type { QueryOptions, UseInfiniteQueryOptions } from "react-query";
|
||||
|
||||
function workspaceBuildParametersKey(workspaceBuildId: string) {
|
||||
export function workspaceBuildParametersKey(workspaceBuildId: string) {
|
||||
return ["workspaceBuilds", workspaceBuildId, "parameters"] as const;
|
||||
}
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { API } from "api/api";
|
||||
import { getErrorDetail, getErrorMessage, isApiError } from "api/errors";
|
||||
import { template as templateQueryOptions } from "api/queries/templates";
|
||||
import { workspaceBuildParameters } from "api/queries/workspaceBuilds";
|
||||
import {
|
||||
startWorkspace,
|
||||
workspaceByOwnerAndName,
|
||||
@@ -198,10 +199,9 @@ type WorkspaceNotRunningProps = {
|
||||
const WorkspaceNotRunning: FC<WorkspaceNotRunningProps> = ({ workspace }) => {
|
||||
const queryClient = useQueryClient();
|
||||
|
||||
const { data: parameters } = useQuery({
|
||||
queryKey: ["workspace", workspace.id, "parameters"],
|
||||
queryFn: () => API.getWorkspaceParameters(workspace),
|
||||
});
|
||||
const { data: buildParameters } = useQuery(
|
||||
workspaceBuildParameters(workspace.latest_build.id),
|
||||
);
|
||||
|
||||
const mutateStartWorkspace = useMutation({
|
||||
...startWorkspace(workspace, queryClient),
|
||||
@@ -237,7 +237,7 @@ const WorkspaceNotRunning: FC<WorkspaceNotRunningProps> = ({ workspace }) => {
|
||||
disabled={isWaitingForStart}
|
||||
onClick={() => {
|
||||
mutateStartWorkspace.mutate({
|
||||
buildParameters: parameters?.buildParameters,
|
||||
buildParameters,
|
||||
});
|
||||
}}
|
||||
>
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { useTheme } from "@emotion/react";
|
||||
import visuallyHidden from "@mui/utils/visuallyHidden";
|
||||
import { API } from "api/api";
|
||||
import { richParameters } from "api/queries/templates";
|
||||
import { workspaceBuildParameters } from "api/queries/workspaceBuilds";
|
||||
import type {
|
||||
TemplateVersionParameter,
|
||||
Workspace,
|
||||
@@ -48,12 +49,15 @@ export const BuildParametersPopover: FC<BuildParametersPopoverProps> = ({
|
||||
onSubmit,
|
||||
}) => {
|
||||
const [isOpen, setIsOpen] = useState(false);
|
||||
const { data: parameters } = useQuery({
|
||||
queryKey: ["workspace", workspace.id, "parameters"],
|
||||
queryFn: () => API.getWorkspaceParameters(workspace),
|
||||
});
|
||||
const ephemeralParameters = parameters
|
||||
? parameters.templateVersionRichParameters.filter((p) => p.ephemeral)
|
||||
const build = workspace.latest_build;
|
||||
const { data: templateVersionParameters } = useQuery(
|
||||
richParameters(build.template_version_id),
|
||||
);
|
||||
const { data: buildParameters } = useQuery(
|
||||
workspaceBuildParameters(build.id),
|
||||
);
|
||||
const ephemeralParameters = templateVersionParameters
|
||||
? templateVersionParameters.filter((p) => p.ephemeral)
|
||||
: undefined;
|
||||
|
||||
return (
|
||||
@@ -75,7 +79,7 @@ export const BuildParametersPopover: FC<BuildParametersPopoverProps> = ({
|
||||
<BuildParametersPopoverContent
|
||||
workspace={workspace}
|
||||
ephemeralParameters={ephemeralParameters}
|
||||
buildParameters={parameters?.buildParameters}
|
||||
buildParameters={buildParameters}
|
||||
onSubmit={onSubmit}
|
||||
setIsOpen={setIsOpen}
|
||||
/>
|
||||
|
||||
@@ -1,5 +1,10 @@
|
||||
import { MockWorkspace } from "testHelpers/entities";
|
||||
import {
|
||||
MockTemplateVersionParameter1,
|
||||
MockWorkspace,
|
||||
} from "testHelpers/entities";
|
||||
import type { Meta, StoryObj } from "@storybook/react-vite";
|
||||
import { richParametersKey } from "api/queries/templates";
|
||||
import { workspaceBuildParametersKey } from "api/queries/workspaceBuilds";
|
||||
import { expect, screen, userEvent, waitFor } from "storybook/test";
|
||||
import { DebugButton } from "./DebugButton";
|
||||
|
||||
@@ -21,8 +26,12 @@ export const WithBuildParameters: Story = {
|
||||
parameters: {
|
||||
queries: [
|
||||
{
|
||||
key: ["workspace", MockWorkspace.id, "parameters"],
|
||||
data: { templateVersionRichParameters: [], buildParameters: [] },
|
||||
key: richParametersKey(MockWorkspace.latest_build.template_version_id),
|
||||
data: [],
|
||||
},
|
||||
{
|
||||
key: workspaceBuildParametersKey(MockWorkspace.latest_build.id),
|
||||
data: [],
|
||||
},
|
||||
],
|
||||
},
|
||||
@@ -36,8 +45,12 @@ export const WithOpenBuildParameters: Story = {
|
||||
parameters: {
|
||||
queries: [
|
||||
{
|
||||
key: ["workspace", MockWorkspace.id, "parameters"],
|
||||
data: { templateVersionRichParameters: [], buildParameters: [] },
|
||||
key: richParametersKey(MockWorkspace.latest_build.template_version_id),
|
||||
data: [MockTemplateVersionParameter1],
|
||||
},
|
||||
{
|
||||
key: workspaceBuildParametersKey(MockWorkspace.latest_build.id),
|
||||
data: [],
|
||||
},
|
||||
],
|
||||
},
|
||||
|
||||
@@ -1,9 +1,12 @@
|
||||
import {
|
||||
MockNonClassicParameterFlowWorkspace,
|
||||
MockTemplateVersionParameter1,
|
||||
MockTemplateVersionParameter6,
|
||||
MockWorkspace,
|
||||
} from "testHelpers/entities";
|
||||
import type { Meta, StoryObj } from "@storybook/react-vite";
|
||||
import { richParametersKey } from "api/queries/templates";
|
||||
import { workspaceBuildParametersKey } from "api/queries/workspaceBuilds";
|
||||
import { expect, screen, userEvent, waitFor } from "storybook/test";
|
||||
import { RetryButton } from "./RetryButton";
|
||||
|
||||
@@ -25,8 +28,12 @@ export const WithBuildParameters: Story = {
|
||||
parameters: {
|
||||
queries: [
|
||||
{
|
||||
key: ["workspace", MockWorkspace.id, "parameters"],
|
||||
data: { templateVersionRichParameters: [], buildParameters: [] },
|
||||
key: richParametersKey(MockWorkspace.latest_build.template_version_id),
|
||||
data: [],
|
||||
},
|
||||
{
|
||||
key: workspaceBuildParametersKey(MockWorkspace.latest_build.id),
|
||||
data: [],
|
||||
},
|
||||
],
|
||||
},
|
||||
@@ -40,8 +47,12 @@ export const WithOpenBuildParameters: Story = {
|
||||
parameters: {
|
||||
queries: [
|
||||
{
|
||||
key: ["workspace", MockWorkspace.id, "parameters"],
|
||||
data: { templateVersionRichParameters: [], buildParameters: [] },
|
||||
key: richParametersKey(MockWorkspace.latest_build.template_version_id),
|
||||
data: [MockTemplateVersionParameter1],
|
||||
},
|
||||
{
|
||||
key: workspaceBuildParametersKey(MockWorkspace.latest_build.id),
|
||||
data: [],
|
||||
},
|
||||
],
|
||||
},
|
||||
@@ -63,11 +74,12 @@ export const WithOpenEphemeralBuildParameters: Story = {
|
||||
parameters: {
|
||||
queries: [
|
||||
{
|
||||
key: ["workspace", MockWorkspace.id, "parameters"],
|
||||
data: {
|
||||
templateVersionRichParameters: [MockTemplateVersionParameter6],
|
||||
buildParameters: [],
|
||||
},
|
||||
key: richParametersKey(MockWorkspace.latest_build.template_version_id),
|
||||
data: [MockTemplateVersionParameter6],
|
||||
},
|
||||
{
|
||||
key: workspaceBuildParametersKey(MockWorkspace.latest_build.id),
|
||||
data: [],
|
||||
},
|
||||
],
|
||||
},
|
||||
@@ -91,15 +103,12 @@ export const WithOpenEphemeralBuildParametersNotClassic: Story = {
|
||||
parameters: {
|
||||
queries: [
|
||||
{
|
||||
key: [
|
||||
"workspace",
|
||||
MockNonClassicParameterFlowWorkspace.id,
|
||||
"parameters",
|
||||
],
|
||||
data: {
|
||||
templateVersionRichParameters: [MockTemplateVersionParameter6],
|
||||
buildParameters: [],
|
||||
},
|
||||
key: richParametersKey(MockWorkspace.latest_build.template_version_id),
|
||||
data: [MockTemplateVersionParameter6],
|
||||
},
|
||||
{
|
||||
key: workspaceBuildParametersKey(MockWorkspace.latest_build.id),
|
||||
data: [],
|
||||
},
|
||||
],
|
||||
},
|
||||
|
||||
@@ -47,7 +47,6 @@ const renderWorkspacePage = async (
|
||||
) => {
|
||||
jest.spyOn(API, "getWorkspaceByOwnerAndName").mockResolvedValue(workspace);
|
||||
jest.spyOn(API, "getTemplate").mockResolvedValueOnce(MockTemplate);
|
||||
jest.spyOn(API, "getTemplateVersionRichParameters").mockResolvedValueOnce([]);
|
||||
jest
|
||||
.spyOn(API, "getDeploymentConfig")
|
||||
.mockResolvedValueOnce(MockDeploymentConfig);
|
||||
@@ -380,18 +379,18 @@ describe("WorkspacePage", () => {
|
||||
|
||||
it("restart the workspace with one time parameters when having the confirmation dialog", async () => {
|
||||
localStorage.removeItem(`${MockUserOwner.id}_ignoredWarnings`);
|
||||
jest.spyOn(API, "getWorkspaceParameters").mockResolvedValue({
|
||||
templateVersionRichParameters: [
|
||||
{
|
||||
...MockTemplateVersionParameter1,
|
||||
ephemeral: true,
|
||||
name: "rebuild",
|
||||
description: "Rebuild",
|
||||
required: false,
|
||||
},
|
||||
],
|
||||
buildParameters: [{ name: "rebuild", value: "false" }],
|
||||
});
|
||||
jest.spyOn(API, "getTemplateVersionRichParameters").mockResolvedValue([
|
||||
{
|
||||
...MockTemplateVersionParameter1,
|
||||
ephemeral: true,
|
||||
name: "rebuild",
|
||||
description: "Rebuild",
|
||||
required: false,
|
||||
},
|
||||
]);
|
||||
jest
|
||||
.spyOn(API, "getWorkspaceBuildParameters")
|
||||
.mockResolvedValue([{ name: "rebuild", value: "false" }]);
|
||||
const restartWorkspaceSpy = jest.spyOn(API, "restartWorkspace");
|
||||
const user = userEvent.setup();
|
||||
await renderWorkspacePage(MockWorkspace);
|
||||
|
||||
+41
-50
@@ -21,22 +21,19 @@ const meta: Meta<typeof WorkspaceParametersPageView> = {
|
||||
workspace: MockWorkspace,
|
||||
canChangeVersions: true,
|
||||
onCancel: action("onCancel"),
|
||||
|
||||
data: {
|
||||
buildParameters: [
|
||||
MockWorkspaceBuildParameter1,
|
||||
MockWorkspaceBuildParameter2,
|
||||
MockWorkspaceBuildParameter3,
|
||||
],
|
||||
templateVersionRichParameters: [
|
||||
MockTemplateVersionParameter1,
|
||||
MockTemplateVersionParameter2,
|
||||
{
|
||||
...MockTemplateVersionParameter3,
|
||||
mutable: false,
|
||||
},
|
||||
],
|
||||
},
|
||||
buildParameters: [
|
||||
MockWorkspaceBuildParameter1,
|
||||
MockWorkspaceBuildParameter2,
|
||||
MockWorkspaceBuildParameter3,
|
||||
],
|
||||
templateVersionParameters: [
|
||||
MockTemplateVersionParameter1,
|
||||
MockTemplateVersionParameter2,
|
||||
{
|
||||
...MockTemplateVersionParameter3,
|
||||
mutable: false,
|
||||
},
|
||||
],
|
||||
},
|
||||
};
|
||||
|
||||
@@ -47,10 +44,8 @@ const Example: Story = {};
|
||||
|
||||
export const Empty: Story = {
|
||||
args: {
|
||||
data: {
|
||||
buildParameters: [],
|
||||
templateVersionRichParameters: [],
|
||||
},
|
||||
buildParameters: [],
|
||||
templateVersionParameters: [],
|
||||
},
|
||||
};
|
||||
|
||||
@@ -58,21 +53,19 @@ export const RequireActiveVersionNoChangeVersion: Story = {
|
||||
args: {
|
||||
workspace: MockOutdatedStoppedWorkspaceRequireActiveVersion,
|
||||
canChangeVersions: false,
|
||||
data: {
|
||||
buildParameters: [
|
||||
MockWorkspaceBuildParameter1,
|
||||
MockWorkspaceBuildParameter2,
|
||||
MockWorkspaceBuildParameter3,
|
||||
],
|
||||
templateVersionRichParameters: [
|
||||
MockTemplateVersionParameter1,
|
||||
MockTemplateVersionParameter2,
|
||||
{
|
||||
...MockTemplateVersionParameter3,
|
||||
mutable: false,
|
||||
},
|
||||
],
|
||||
},
|
||||
buildParameters: [
|
||||
MockWorkspaceBuildParameter1,
|
||||
MockWorkspaceBuildParameter2,
|
||||
MockWorkspaceBuildParameter3,
|
||||
],
|
||||
templateVersionParameters: [
|
||||
MockTemplateVersionParameter1,
|
||||
MockTemplateVersionParameter2,
|
||||
{
|
||||
...MockTemplateVersionParameter3,
|
||||
mutable: false,
|
||||
},
|
||||
],
|
||||
},
|
||||
};
|
||||
|
||||
@@ -80,21 +73,19 @@ export const RequireActiveVersionCanChangeVersion: Story = {
|
||||
args: {
|
||||
workspace: MockOutdatedStoppedWorkspaceRequireActiveVersion,
|
||||
canChangeVersions: true,
|
||||
data: {
|
||||
buildParameters: [
|
||||
MockWorkspaceBuildParameter1,
|
||||
MockWorkspaceBuildParameter2,
|
||||
MockWorkspaceBuildParameter3,
|
||||
],
|
||||
templateVersionRichParameters: [
|
||||
MockTemplateVersionParameter1,
|
||||
MockTemplateVersionParameter2,
|
||||
{
|
||||
...MockTemplateVersionParameter3,
|
||||
mutable: false,
|
||||
},
|
||||
],
|
||||
},
|
||||
buildParameters: [
|
||||
MockWorkspaceBuildParameter1,
|
||||
MockWorkspaceBuildParameter2,
|
||||
MockWorkspaceBuildParameter3,
|
||||
],
|
||||
templateVersionParameters: [
|
||||
MockTemplateVersionParameter1,
|
||||
MockTemplateVersionParameter2,
|
||||
{
|
||||
...MockTemplateVersionParameter3,
|
||||
mutable: false,
|
||||
},
|
||||
],
|
||||
},
|
||||
};
|
||||
|
||||
|
||||
+37
-26
@@ -1,7 +1,13 @@
|
||||
import { API } from "api/api";
|
||||
import { isApiValidationError } from "api/errors";
|
||||
import { checkAuthorization } from "api/queries/authCheck";
|
||||
import type { Workspace, WorkspaceBuildParameter } from "api/typesGenerated";
|
||||
import { richParameters } from "api/queries/templates";
|
||||
import { workspaceBuildParameters } from "api/queries/workspaceBuilds";
|
||||
import type {
|
||||
TemplateVersionParameter,
|
||||
Workspace,
|
||||
WorkspaceBuildParameter,
|
||||
} from "api/typesGenerated";
|
||||
import { ErrorAlert } from "components/Alert/ErrorAlert";
|
||||
import { Button } from "components/Button/Button";
|
||||
import { EmptyState } from "components/EmptyState/EmptyState";
|
||||
@@ -24,10 +30,13 @@ import {
|
||||
|
||||
const WorkspaceParametersPage: FC = () => {
|
||||
const workspace = useWorkspaceSettings();
|
||||
const parameters = useQuery({
|
||||
queryKey: ["workspace", workspace.id, "parameters"],
|
||||
queryFn: () => API.getWorkspaceParameters(workspace),
|
||||
});
|
||||
const build = workspace.latest_build;
|
||||
const { data: templateVersionParameters } = useQuery(
|
||||
richParameters(build.template_version_id),
|
||||
);
|
||||
const { data: buildParameters } = useQuery(
|
||||
workspaceBuildParameters(build.id),
|
||||
);
|
||||
const navigate = useNavigate();
|
||||
const updateParameters = useMutation({
|
||||
mutationFn: (buildParameters: WorkspaceBuildParameter[]) =>
|
||||
@@ -75,30 +84,30 @@ const WorkspaceParametersPage: FC = () => {
|
||||
|
||||
<WorkspaceParametersPageView
|
||||
workspace={workspace}
|
||||
templateVersionParameters={templateVersionParameters}
|
||||
buildParameters={buildParameters}
|
||||
canChangeVersions={canChangeVersions}
|
||||
templatePermissions={templatePermissions}
|
||||
data={parameters.data}
|
||||
submitError={updateParameters.error}
|
||||
isSubmitting={updateParameters.isPending}
|
||||
onSubmit={(values) => {
|
||||
if (!parameters.data) {
|
||||
if (!templateVersionParameters) {
|
||||
return;
|
||||
}
|
||||
// When updating the parameters, the API does not accept immutable
|
||||
// values so we need to filter them
|
||||
const onlyMultableValues =
|
||||
parameters.data.templateVersionRichParameters
|
||||
.filter((p) => p.mutable)
|
||||
.map((p) => {
|
||||
const value = values.rich_parameter_values.find(
|
||||
(v) => v.name === p.name,
|
||||
);
|
||||
if (!value) {
|
||||
throw new Error(`Missing value for parameter ${p.name}`);
|
||||
}
|
||||
return value;
|
||||
});
|
||||
updateParameters.mutate(onlyMultableValues);
|
||||
const onlyMutableValues = templateVersionParameters
|
||||
.filter((p) => p.mutable)
|
||||
.map((p) => {
|
||||
const value = values.rich_parameter_values.find(
|
||||
(v) => v.name === p.name,
|
||||
);
|
||||
if (!value) {
|
||||
throw new Error(`Missing value for parameter ${p.name}`);
|
||||
}
|
||||
return value;
|
||||
});
|
||||
updateParameters.mutate(onlyMutableValues);
|
||||
}}
|
||||
onCancel={() => {
|
||||
navigate("../..");
|
||||
@@ -112,7 +121,8 @@ type WorkspaceParametersPageViewProps = {
|
||||
workspace: Workspace;
|
||||
canChangeVersions: boolean;
|
||||
templatePermissions: { canUpdateTemplate: boolean } | undefined;
|
||||
data: Awaited<ReturnType<typeof API.getWorkspaceParameters>> | undefined;
|
||||
templateVersionParameters?: TemplateVersionParameter[];
|
||||
buildParameters?: WorkspaceBuildParameter[];
|
||||
submitError: unknown;
|
||||
isSubmitting: boolean;
|
||||
onSubmit: (formValues: WorkspaceParametersFormValues) => void;
|
||||
@@ -125,7 +135,8 @@ export const WorkspaceParametersPageView: FC<
|
||||
workspace,
|
||||
canChangeVersions,
|
||||
templatePermissions,
|
||||
data,
|
||||
templateVersionParameters,
|
||||
buildParameters,
|
||||
submitError,
|
||||
onSubmit,
|
||||
isSubmitting,
|
||||
@@ -143,17 +154,17 @@ export const WorkspaceParametersPageView: FC<
|
||||
<ErrorAlert error={submitError} css={{ marginBottom: 48 }} />
|
||||
) : null}
|
||||
|
||||
{data ? (
|
||||
data.templateVersionRichParameters.length > 0 ? (
|
||||
{templateVersionParameters && buildParameters ? (
|
||||
templateVersionParameters.length > 0 ? (
|
||||
<WorkspaceParametersForm
|
||||
workspace={workspace}
|
||||
canChangeVersions={canChangeVersions}
|
||||
templatePermissions={templatePermissions}
|
||||
autofillParams={data.buildParameters.map((p) => ({
|
||||
autofillParams={buildParameters.map((p) => ({
|
||||
...p,
|
||||
source: "active_build",
|
||||
}))}
|
||||
templateVersionRichParameters={data.templateVersionRichParameters}
|
||||
templateVersionRichParameters={templateVersionParameters}
|
||||
error={submitError}
|
||||
isSubmitting={isSubmitting}
|
||||
onSubmit={onSubmit}
|
||||
|
||||
Reference in New Issue
Block a user