Skip to content

fix(site): allow editing parameters when the template forces an update #29795

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

Open
blinkagent wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(site): allow editing parameters when the template forces an update #29795

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions ...c/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ export const StartWorkspace: Story = {
},
};

export const RequireActiveVersionBlocked: Story = {
export const RequireActiveVersionForcedUpdate: Story = {
parameters: {
reactRouter: workspaceRouterParameters(
MockOutdatedStoppedWorkspaceRequireActiveVersion,
Expand All @@ -183,11 +183,11 @@ export const RequireActiveVersionBlocked: Story = {
const submitButton = canvas.getByRole("button", {
name: "Update and start",
});
expect(submitButton).toBeDisabled();
expect(submitButton).not.toBeDisabled();
},
};

export const RequireActiveVersionBlockedRunning: Story = {
export const RequireActiveVersionForcedUpdateRunning: Story = {
parameters: {
reactRouter: workspaceRouterParameters(
MockOutdatedRunningWorkspaceRequireActiveVersion,
Expand All @@ -208,7 +208,7 @@ export const RequireActiveVersionBlockedRunning: Story = {
const submitButton = canvas.getByRole("button", {
name: "Update and restart",
});
expect(submitButton).toBeDisabled();
expect(submitButton).not.toBeDisabled();
},
};

Expand Down
61 changes: 61 additions & 0 deletions .../src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,4 +358,65 @@ describe("WorkspaceParametersPage", () => {
});
await waitFor(() => expect(submitButton).toBeDisabled());
});

it("stays editable when the template forces an update", async () => {
const workspace: TypesGen.Workspace = {
...MockWorkspace,
outdated: true,
template_require_active_version: true,
template_active_version_id: "active-template-version-id",
};
const workspaceSpy = vi.spyOn(API, "getWorkspaceByOwnerAndName");
workspaceSpy.mockReset();
workspaceSpy.mockResolvedValue(workspace);

// A user with no template update permission, such as a member.
vi.spyOn(API, "checkAuthorization").mockResolvedValue({
readWorkspace: true,
shareWorkspace: true,
updateWorkspace: true,
updateWorkspaceVersion: false,
deleteFailedWorkspace: false,
});

vi.spyOn(API, "getWorkspaceBuildParameters").mockResolvedValueOnce([]);

const [, mockPublisher] = mockDynamicParameterWebSocket();

renderWorkspaceParametersPage();

await connectWithInitialParameters(mockPublisher, [
MockPreviewParameter1,
MockPreviewParameter4,
]);

// Parameters are evaluated against the active version, since that is the
// only version this user is able to build.
expect(API.templateVersionDynamicParameters).toHaveBeenCalledWith(
workspace.template_active_version_id,
workspace.owner_id,
expect.anything(),
);

await waitForLoaderToBeRemoved();

const form = screen.getByTestId("form");

// Mutable parameters are editable even though the workspace is outdated.
const mutableField = within(form).getByTestId(
`parameter-field-${MockPreviewParameter1.name}`,
);
expect(within(mutableField).getByRole("textbox")).toBeEnabled();

// Immutable parameters are still locked.
const immutableField = within(form).getByTestId(
`parameter-field-${MockPreviewParameter4.name}`,
);
expect(within(immutableField).getByRole("textbox")).toBeDisabled();

const submitButton = within(form).getByRole("button", {
name: /update and restart/i,
});
await waitFor(() => expect(submitButton).toBeEnabled());
});
});
21 changes: 17 additions & 4 deletions site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,22 @@ const WorkspaceParametersPage: FC = () => {
const { permissions, workspace } = useWorkspaceSettings();
const navigate = useNavigate();
const [searchParams] = useSearchParams();
const templateVersionId = searchParams.get("templateVersionId") ?? undefined;
const requestedTemplateVersionId =
searchParams.get("templateVersionId") ?? undefined;

const canChangeVersions = Boolean(permissions?.updateWorkspaceVersion);

// When the template requires the active version and the user is not allowed
// to pin versions, the only build they can ever submit is one against the
// active version, so edit the parameters against it. Otherwise a new version
// that adds a validation the current values do not satisfy would leave the
// user unable to fix those values, and therefore unable to start their
// workspace at all.
const forcedActiveVersion =
workspace.template_require_active_version && !canChangeVersions;
const templateVersionId = forcedActiveVersion
? workspace.template_active_version_id
: requestedTemplateVersionId;

const [confirmingRestart, setConfirmingRestart] = useState<{
open: boolean;
Expand Down Expand Up @@ -209,8 +224,6 @@ const WorkspaceParametersPage: FC = () => {
},
});

const canChangeVersions = Boolean(permissions?.updateWorkspaceVersion);

const handleSubmit = (values: {
rich_parameter_values: WorkspaceBuildParameter[];
}) => {
Expand Down Expand Up @@ -310,7 +323,7 @@ const WorkspaceParametersPage: FC = () => {
templateVersionId={templateVersionId}
workspace={workspace}
autofillParameters={autofillParameters}
canChangeVersions={canChangeVersions}
updatesToActiveVersion={forcedActiveVersion && workspace.outdated}
parameters={sortedParams}
diagnostics={latestResponse?.diagnostics ?? []}
isSubmitting={
Expand Down
20 changes: 7 additions & 13 deletions site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ type WorkspaceParametersPageViewProps = {
autofillParameters: AutofillBuildParameter[];
parameters: PreviewParameter[];
diagnostics: PreviewParameter["diagnostics"];
canChangeVersions: boolean;
updatesToActiveVersion: boolean;
isSubmitting: boolean;
submitLabel: string;
onCancel: () => void;
Expand All @@ -44,7 +44,7 @@ export const WorkspaceParametersPageView: FC<
autofillParameters,
parameters,
diagnostics,
canChangeVersions,
updatesToActiveVersion,
isSubmitting,
submitLabel,
onSubmit,
Expand All @@ -69,11 +69,6 @@ export const WorkspaceParametersPageView: FC<
validateOnBlur: true,
});

const disabled =
workspace.outdated &&
workspace.template_require_active_version &&
!canChangeVersions;

// Debounce websocket sends to avoid stale responses overwriting
// the form while the user is still typing.
const { debounced: sendDynamicParamsRequest } = useDebouncedFunction(
Expand Down Expand Up @@ -142,10 +137,11 @@ export const WorkspaceParametersPageView: FC<

return (
<>
{disabled && (
<Alert severity="warning" className="mb-8" prominent>
The template for this workspace requires automatic updates. Update the
workspace to edit parameters.
{updatesToActiveVersion && (
<Alert severity="info" className="mb-8" prominent>
The template for this workspace requires automatic updates. These
parameters are for the latest template version, and submitting them
will update the workspace to it.
</Alert>
)}

Expand Down Expand Up @@ -253,7 +249,6 @@ export const WorkspaceParametersPageView: FC<

const parameterField = `rich_parameter_values.${parameterFieldIndex}`;
const isDisabled =
disabled ||
parameter.styling?.disabled ||
!parameter.mutable ||
isSubmitting;
Expand Down Expand Up @@ -282,7 +277,6 @@ export const WorkspaceParametersPageView: FC<
type="submit"
disabled={
isSubmitting ||
disabled ||
hasUnsyncedParameters ||
diagnostics.some(
(diagnostic) => diagnostic.severity === "error",
Expand Down
Loading