mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(site): correct autostop restart prompt on the workspace schedule page (#27632)
> 🤖 This PR was written by Coder Agents on behalf of Jake Howell. ## Problem Enabling autostop on a running workspace never showed the "restart now to apply?" dialog. The new TTL was saved, but a running build's deadline is only calculated when the build starts, so autostop silently did not take effect until a manual restart. Changing an already-enabled value did prompt, so the behavior was inconsistent. Separately, dismissing that dialog with **Apply later** navigated the user away to the workspace, exactly like confirming did. ## Root cause The dialog was gated on `getAutostop(workspace).autostopEnabled`, which derives from `workspace.ttl_ms` — the **pre-submit** state (the workspace is not refetched until after the mutation). That expression means "was autostop enabled *before* this change", which maps to: | Transition | Pre-submit enabled | Dialog | Correct? | |---|---|---|---| | Disabled → enabled (add) | `false` | not shown | ❌ the reported bug | | Enabled → new value (modify) | `true` | shown | ✅ | | Enabled → disabled (remove) | `true` | shown | ❌ (nothing to apply) | This guard came from #16085, which intended to *skip* the prompt when **removing** autostop (safe, because that PR also made the backend clear a running build's deadline server-side when TTL is set to null). By keying off the old state it actually skipped the prompt on **add** and still showed it on **remove** — the opposite of its intent. ## Fix - Key the prompt off the submitted form value (`values.autostopEnabled`), with an inline comment documenting the trigger conditions. This fixes the reported bug and restores #16085's intent: - add → prompt ✅ - modify → prompt ✅ - remove → no prompt ✅ (deadline cleared server-side) - stopped / unchanged → no prompt ✅ - **Apply later** now just closes the dialog and keeps the user on the schedule page; the saved value still applies on the next start. **Restart** is unchanged (restarts and navigates to the workspace). ## Testing Storybook `play`-function coverage on `WorkspaceSchedulePage` for all cases: enable, change value, disable, enable-while-stopped, autostart-only, and Apply-later-stays-on-page (the last also asserts `restartWorkspace` is not called). A prior story that asserted the pre-fix behavior (prompting on disable) was reworked. `tsc` and `biome` clean. <details> <summary>Investigation notes</summary> History of the gating condition: - Originally `if (data.autostopChanged) { ... }` — prompted on every autostop change (enable included). - #16085 ("allow removing deadline for running workspace", fixes #9775) added `&& getAutostop(workspace).autostopEnabled`. Its description states the goal was to "not show a confirmation dialog when the change is to remove autostop", and the same PR added backend logic in `putWorkspaceTTL` to clear a running build's deadline when TTL is null. - A later refactor added `&& workspace.latest_build.status === "running"`. Because the backend already clears the deadline live on removal, no restart is needed there; the frontend only needs to prompt when autostop ends up enabled on a running workspace. The fix keys off the submitted state so all three transitions behave correctly. </details>
This commit is contained in:
+91
-14
@@ -1,5 +1,5 @@
|
||||
import type { Meta, StoryObj } from "@storybook/react-vite";
|
||||
import { expect, spyOn, userEvent, within } from "storybook/test";
|
||||
import { expect, spyOn, userEvent, waitFor, within } from "storybook/test";
|
||||
import {
|
||||
reactRouterOutlet,
|
||||
reactRouterParameters,
|
||||
@@ -73,13 +73,15 @@ export const EnablingAutostopUsesTemplateDefault: Story = {
|
||||
},
|
||||
};
|
||||
|
||||
export const ChangingAutostopShowsRestartDialog: Story = {
|
||||
export const EnablingAutostopShowsRestartDialog: Story = {
|
||||
parameters: {
|
||||
reactRouter: workspaceRouterParameters(MockWorkspace),
|
||||
queries: workspaceQueries(MockWorkspace),
|
||||
reactRouter: workspaceRouterParameters(autostopDisabledWorkspace),
|
||||
queries: workspaceQueries(autostopDisabledWorkspace),
|
||||
},
|
||||
beforeEach: () => {
|
||||
spyOn(API, "getWorkspaceByOwnerAndName").mockResolvedValue(MockWorkspace);
|
||||
spyOn(API, "getWorkspaceByOwnerAndName").mockResolvedValue(
|
||||
autostopDisabledWorkspace,
|
||||
);
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
@@ -94,19 +96,94 @@ export const ChangingAutostopShowsRestartDialog: Story = {
|
||||
},
|
||||
};
|
||||
|
||||
const stoppedWorkspace: Workspace = {
|
||||
...MockWorkspace,
|
||||
latest_build: { ...MockWorkspaceBuild, status: "stopped" },
|
||||
};
|
||||
|
||||
export const ChangingAutostopWhileStoppedSkipsDialog: Story = {
|
||||
export const ApplyLaterKeepsUserOnSchedulePage: Story = {
|
||||
parameters: {
|
||||
reactRouter: workspaceRouterParameters(stoppedWorkspace),
|
||||
queries: workspaceQueries(stoppedWorkspace),
|
||||
reactRouter: workspaceRouterParameters(autostopDisabledWorkspace),
|
||||
queries: workspaceQueries(autostopDisabledWorkspace),
|
||||
},
|
||||
beforeEach: () => {
|
||||
spyOn(API, "getWorkspaceByOwnerAndName").mockResolvedValue(
|
||||
stoppedWorkspace,
|
||||
autostopDisabledWorkspace,
|
||||
);
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
const body = within(document.body);
|
||||
const user = userEvent.setup();
|
||||
const restartSpy = spyOn(API, "restartWorkspace");
|
||||
await user.click(await canvas.findByLabelText("Enable Autostop"));
|
||||
await user.click(await canvas.findByRole("button", { name: /save/i }));
|
||||
await body.findByText("Restart workspace?");
|
||||
await user.click(await body.findByRole("button", { name: /apply later/i }));
|
||||
// The dialog closes without restarting or leaving the schedule page.
|
||||
await waitFor(() =>
|
||||
expect(body.queryByText("Restart workspace?")).not.toBeInTheDocument(),
|
||||
);
|
||||
expect(restartSpy).not.toHaveBeenCalled();
|
||||
await canvas.findByLabelText("Enable Autostop");
|
||||
},
|
||||
};
|
||||
|
||||
export const ChangingAutostopValueShowsRestartDialog: Story = {
|
||||
parameters: {
|
||||
reactRouter: workspaceRouterParameters(MockWorkspace),
|
||||
queries: workspaceQueries(MockWorkspace),
|
||||
},
|
||||
beforeEach: () => {
|
||||
spyOn(API, "getWorkspaceByOwnerAndName").mockResolvedValue(MockWorkspace);
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
const body = within(document.body);
|
||||
const user = userEvent.setup();
|
||||
const ttlInput = await canvas.findByLabelText(
|
||||
"Time until shutdown (hours)",
|
||||
);
|
||||
await user.clear(ttlInput);
|
||||
await user.type(ttlInput, "4");
|
||||
await user.click(await canvas.findByRole("button", { name: /save/i }));
|
||||
await body.findByText(
|
||||
`Schedule for workspace "${MockWorkspace.name}" updated successfully.`,
|
||||
);
|
||||
await body.findByText("Restart workspace?");
|
||||
},
|
||||
};
|
||||
|
||||
export const DisablingAutostopSkipsRestartDialog: Story = {
|
||||
parameters: {
|
||||
reactRouter: workspaceRouterParameters(MockWorkspace),
|
||||
queries: workspaceQueries(MockWorkspace),
|
||||
},
|
||||
beforeEach: () => {
|
||||
spyOn(API, "getWorkspaceByOwnerAndName").mockResolvedValue(MockWorkspace);
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
const body = within(document.body);
|
||||
const user = userEvent.setup();
|
||||
// MockWorkspace has autostop enabled, so clicking the toggle disables it.
|
||||
await user.click(await canvas.findByLabelText("Enable Autostop"));
|
||||
await user.click(await canvas.findByRole("button", { name: /save/i }));
|
||||
await body.findByText(
|
||||
`Schedule for workspace "${MockWorkspace.name}" updated successfully.`,
|
||||
);
|
||||
expect(body.queryByText("Restart workspace?")).not.toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
const stoppedAutostopDisabledWorkspace: Workspace = {
|
||||
...autostopDisabledWorkspace,
|
||||
latest_build: { ...MockWorkspaceBuild, status: "stopped" },
|
||||
};
|
||||
|
||||
export const EnablingAutostopWhileStoppedSkipsDialog: Story = {
|
||||
parameters: {
|
||||
reactRouter: workspaceRouterParameters(stoppedAutostopDisabledWorkspace),
|
||||
queries: workspaceQueries(stoppedAutostopDisabledWorkspace),
|
||||
},
|
||||
beforeEach: () => {
|
||||
spyOn(API, "getWorkspaceByOwnerAndName").mockResolvedValue(
|
||||
stoppedAutostopDisabledWorkspace,
|
||||
);
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
|
||||
+14
-2
@@ -140,9 +140,19 @@ const WorkspaceSchedulePage: FC = () => {
|
||||
|
||||
await submitScheduleMutation.mutateAsync(data);
|
||||
|
||||
// A running build's autostop deadline is calculated when the
|
||||
// build starts, so updating the TTL does not retroactively
|
||||
// change it. Prompt the user to restart so the new value takes
|
||||
// effect immediately, but only when all of the following hold:
|
||||
// - autostop actually changed (toggled or new TTL value),
|
||||
// - autostop is enabled after the change; disabling clears the
|
||||
// running build's deadline server-side, so no restart is
|
||||
// needed, and
|
||||
// - the workspace is running; a stopped workspace picks up the
|
||||
// new value on its next start.
|
||||
if (
|
||||
data.autostopChanged &&
|
||||
getAutostop(workspace).autostopEnabled &&
|
||||
values.autostopEnabled &&
|
||||
workspace.latest_build.status === "running"
|
||||
) {
|
||||
setIsConfirmingApply(true);
|
||||
@@ -163,7 +173,9 @@ const WorkspaceSchedulePage: FC = () => {
|
||||
navigate(`/@${username}/${workspaceName}`);
|
||||
}}
|
||||
onClose={() => {
|
||||
navigate(`/@${username}/${workspaceName}`);
|
||||
// Keep the user on the schedule page; the saved value still
|
||||
// applies on the next workspace start.
|
||||
setIsConfirmingApply(false);
|
||||
}}
|
||||
/>
|
||||
</div>
|
||||
|
||||
Reference in New Issue
Block a user