Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@
`coder update` non-interactively) now passes newly-required template
parameters into the REST-API fallback build, instead of silently omitting
them and letting the server reject the build.
- Updating a workspace now re-prompts for an option or multi-select
parameter whose stored value is no longer one of the new template
version's options, instead of carrying a stale value forward and
failing the build. Immutable parameters without a stored value are
now prompted as well, closing a gap with the web dashboard.
- Updating a workspace from VS Code no longer hangs when the new template
version requires parameters. The extension now prompts for any missing
required values through VS Code input boxes and passes them to
Expand Down
69 changes: 57 additions & 12 deletions src/api/updateParameters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ export class WorkspaceUpdateCancelledError extends Error {
}

/**
* Prompts the user for any newly-required template parameters and returns the
* collected `{ name, value }` pairs. Throws `WorkspaceUpdateCancelledError` if
* the user dismisses a prompt.
* Prompts the user for any template parameters that the new version needs
* answered, and returns the collected `{ name, value }` pairs. Throws
* `WorkspaceUpdateCancelledError` if the user dismisses a prompt.
*/
export async function collectUpdateParameters(
restClient: Api,
Expand All @@ -30,16 +30,18 @@ export async function collectUpdateParameters(
),
restClient.getWorkspaceBuildParameters(workspace.latest_build.id),
]);
const candidates = newParams.filter((p) => p.required && !p.default_value);
if (candidates.length === 0) return [];

const existing = new Set(currentValues.map((p) => p.name));
const toPrompt = candidates.filter((p) => !existing.has(p.name));
const stored = new Map(currentValues.map((p) => [p.name, p.value]));
const toPrompt = newParams.filter((p) => needsPrompt(p, stored.get(p.name)));

const collected: WorkspaceBuildParameter[] = [];
for (let i = 0; i < toPrompt.length; i++) {
const param = toPrompt[i];
const value = await promptForParameter(param, i + 1, toPrompt.length);
const value = await promptForParameter(
param,
i + 1,
toPrompt.length,
stored.get(param.name),
);
if (value === undefined) {
throw new WorkspaceUpdateCancelledError();
}
Expand All @@ -48,10 +50,48 @@ export async function collectUpdateParameters(
return collected;
}

/** Based on the dashboard's `getMissingParameters` (coder/site/src/api/api.ts). */
function needsPrompt(
param: TemplateVersionParameter,
storedValue: string | undefined,
): boolean {
if (storedValue === undefined) {
// Accepting the default locks it in.
if (!param.mutable) return true;
// Deviation: dashboard prompts mutable+required even when a default
// exists; we skip so the server can apply the default during
// auto-update without surfacing a modal.
return param.required && !param.default_value;
}
if (param.options.length === 0) return false;

const validValues = new Set(param.options.map((o) => o.value));
if (param.form_type === "multi-select") {
Comment thread
EhabY marked this conversation as resolved.
// Beyond dashboard: detect multi-select drift too.
const picks = parseMultiSelectValue(storedValue);
return picks === null || picks.some((v) => !validValues.has(v));
Comment thread
EhabY marked this conversation as resolved.
}
return !validValues.has(storedValue);
}

/** Multi-select values are stored as a JSON-encoded string array. */
function parseMultiSelectValue(raw: string): string[] | null {
let parsed: unknown;
try {
parsed = JSON.parse(raw);
} catch {
return null;
}
return Array.isArray(parsed) && parsed.every((v) => typeof v === "string")
? parsed
: null;
}

function promptForParameter(
param: TemplateVersionParameter,
step: number,
totalSteps: number,
storedValue: string | undefined,
): Promise<string | undefined> {
const title = param.display_name || param.name;
const items = quickPickItems(param);
Expand All @@ -66,11 +106,16 @@ function promptForParameter(
qp.items = items;
qp.canSelectMany = multi;
qp.ignoreFocusOut = true;
if (multi && storedValue !== undefined) {
const previous = new Set(parseMultiSelectValue(storedValue) ?? []);
qp.selectedItems = items.filter((item) => previous.has(item.value));
}
return collectInput(qp, () => {
if (multi) {
return qp.selectedItems.length > 0
? JSON.stringify(qp.selectedItems.map((i) => i.value))
: undefined;
if (qp.selectedItems.length === 0) {
return param.required ? undefined : "[]";
}
return JSON.stringify(qp.selectedItems.map((i) => i.value));
}
return qp.selectedItems[0]?.value;
});
Expand Down
260 changes: 260 additions & 0 deletions test/unit/api/updateParameters.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,266 @@ describe("collectUpdateParameters", () => {
expect(vscode.window.createInputBox).not.toHaveBeenCalled();
});

it("prompts an immutable param that has no stored value (even if not required)", async () => {
const { restClient, workspace } = createCollectCtx([
{ name: "zone", required: false, mutable: false },
]);
const qi = mockCreateInputBox();

const result = collectUpdateParameters(restClient, workspace);
await waitShown(qi);
qi.accept({ value: "us-east" });

await expect(result).resolves.toEqual([{ name: "zone", value: "us-east" }]);
});

it("prompts an immutable param even when a template default exists", async () => {
const { restClient, workspace } = createCollectCtx([
{ name: "zone", mutable: false, default_value: "us-east" },
]);
const qi = mockCreateInputBox();

const result = collectUpdateParameters(restClient, workspace);
await waitShown(qi);
qi.accept({ value: "us-west" });

await expect(result).resolves.toEqual([{ name: "zone", value: "us-west" }]);
});

it("skips an option param whose stored value is still in the new options", async () => {
const optionParam: Partial<TemplateVersionParameter> = {
name: "size",
options: [
{ name: "Small", description: "", value: "s", icon: "" },
{ name: "Large", description: "", value: "l", icon: "" },
],
};
const { restClient, workspace } = createCollectCtx(
[optionParam],
[{ name: "size", value: "l" }],
);

await expect(
collectUpdateParameters(restClient, workspace),
).resolves.toEqual([]);
expect(vscode.window.createQuickPick).not.toHaveBeenCalled();
});

it("re-prompts when a stored option value drifted out of the new options", async () => {
const optionParam: Partial<TemplateVersionParameter> = {
name: "size",
options: [
{ name: "Small", description: "", value: "s", icon: "" },
{ name: "Large", description: "", value: "l", icon: "" },
],
};
const { restClient, workspace } = createCollectCtx(
[optionParam],
[{ name: "size", value: "xl-retired" }],
);
const qi = mockCreateQuickPick();

const result = collectUpdateParameters(restClient, workspace);
await waitShown(qi);
qi.accept({ selectedItems: [{ value: "l" }] });

await expect(result).resolves.toEqual([{ name: "size", value: "l" }]);
});

it("re-prompts on drift even when the template parameter has a default value", async () => {
const optionParam: Partial<TemplateVersionParameter> = {
name: "size",
default_value: "s",
options: [
{ name: "Small", description: "", value: "s", icon: "" },
{ name: "Large", description: "", value: "l", icon: "" },
],
};
const { restClient, workspace } = createCollectCtx(
[optionParam],
[{ name: "size", value: "xl-retired" }],
);
const qi = mockCreateQuickPick();

const result = collectUpdateParameters(restClient, workspace);
await waitShown(qi);
qi.accept({ selectedItems: [{ value: "l" }] });

await expect(result).resolves.toEqual([{ name: "size", value: "l" }]);
});

it("re-prompts on drift for an immutable param too", async () => {
const optionParam: Partial<TemplateVersionParameter> = {
name: "zone",
mutable: false,
options: [{ name: "US", description: "", value: "us", icon: "" }],
};
const { restClient, workspace } = createCollectCtx(
[optionParam],
[{ name: "zone", value: "eu-retired" }],
);
const qi = mockCreateQuickPick();

const result = collectUpdateParameters(restClient, workspace);
await waitShown(qi);
qi.accept({ selectedItems: [{ value: "us" }] });

await expect(result).resolves.toEqual([{ name: "zone", value: "us" }]);
});

it("re-prompts a multi-select when any stored pick drifted out", async () => {
Comment thread
EhabY marked this conversation as resolved.
Comment thread
EhabY marked this conversation as resolved.
const multiParam: Partial<TemplateVersionParameter> = {
name: "regions",
form_type: "multi-select",
options: [
{ name: "US", description: "", value: "us", icon: "" },
{ name: "EU", description: "", value: "eu", icon: "" },
],
};
const { restClient, workspace } = createCollectCtx(
[multiParam],
[{ name: "regions", value: '["us","apac-retired"]' }],
);
const qi = mockCreateQuickPick();

const result = collectUpdateParameters(restClient, workspace);
await waitShown(qi);
qi.accept({ selectedItems: [{ value: "us" }] });

await expect(result).resolves.toEqual([
{ name: "regions", value: '["us"]' },
]);
});

it("re-prompts a multi-select when the stored value isn't a JSON string array", async () => {
const multiParam: Partial<TemplateVersionParameter> = {
name: "regions",
form_type: "multi-select",
options: [{ name: "US", description: "", value: "us", icon: "" }],
};
const { restClient, workspace } = createCollectCtx(
[multiParam],
[{ name: "regions", value: "not-json" }],
);
const qi = mockCreateQuickPick();

const result = collectUpdateParameters(restClient, workspace);
await waitShown(qi);
qi.accept({ selectedItems: [{ value: "us" }] });

await expect(result).resolves.toEqual([
{ name: "regions", value: '["us"]' },
]);
});

it("skips a multi-select when every stored pick is still in the new options", async () => {
const multiParam: Partial<TemplateVersionParameter> = {
name: "regions",
form_type: "multi-select",
options: [
{ name: "US", description: "", value: "us", icon: "" },
{ name: "EU", description: "", value: "eu", icon: "" },
],
};
const { restClient, workspace } = createCollectCtx(
[multiParam],
[{ name: "regions", value: '["us","eu"]' }],
);

await expect(
collectUpdateParameters(restClient, workspace),
).resolves.toEqual([]);
expect(vscode.window.createQuickPick).not.toHaveBeenCalled();
});

it("skips a multi-select when the stored value is an empty JSON array", async () => {
const multiParam: Partial<TemplateVersionParameter> = {
name: "regions",
form_type: "multi-select",
options: [{ name: "US", description: "", value: "us", icon: "" }],
};
const { restClient, workspace } = createCollectCtx(
[multiParam],
[{ name: "regions", value: "[]" }],
);

await expect(
collectUpdateParameters(restClient, workspace),
).resolves.toEqual([]);
expect(vscode.window.createQuickPick).not.toHaveBeenCalled();
});

it("pre-selects still-valid picks on a multi-select drift re-prompt", async () => {
const multiParam: Partial<TemplateVersionParameter> = {
name: "regions",
form_type: "multi-select",
options: [
{ name: "US", description: "", value: "us", icon: "" },
{ name: "EU", description: "", value: "eu", icon: "" },
],
};
const { restClient, workspace } = createCollectCtx(
[multiParam],
[{ name: "regions", value: '["us","apac-retired"]' }],
);
const qi = mockCreateQuickPick();

const result = collectUpdateParameters(restClient, workspace);
await waitShown(qi);
expect(qi.mock.selectedItems).toEqual([
{ label: "US", description: "", value: "us" },
]);
qi.accept({ selectedItems: [{ value: "us" }, { value: "eu" }] });

await expect(result).resolves.toEqual([
{ name: "regions", value: '["us","eu"]' },
]);
});

it("keeps the prompt open on an empty submission for a required multi-select", async () => {
const multiParam: Partial<TemplateVersionParameter> = {
name: "regions",
required: true,
form_type: "multi-select",
options: [{ name: "US", description: "", value: "us", icon: "" }],
};
const { restClient, workspace } = createCollectCtx(
[multiParam],
[{ name: "regions", value: '["apac-retired"]' }],
);
const qi = mockCreateQuickPick();

const result = collectUpdateParameters(restClient, workspace);
await waitShown(qi);
qi.accept({ selectedItems: [] });
expect(qi.mock.dispose).not.toHaveBeenCalled();
qi.accept({ selectedItems: [{ value: "us" }] });

await expect(result).resolves.toEqual([
{ name: "regions", value: '["us"]' },
]);
});

it("accepts an empty selection on a non-required multi-select re-prompt", async () => {
Comment thread
EhabY marked this conversation as resolved.
const multiParam: Partial<TemplateVersionParameter> = {
name: "regions",
required: false,
form_type: "multi-select",
options: [{ name: "US", description: "", value: "us", icon: "" }],
};
const { restClient, workspace } = createCollectCtx(
[multiParam],
[{ name: "regions", value: '["apac-retired"]' }],
);
const qi = mockCreateQuickPick();

const result = collectUpdateParameters(restClient, workspace);
await waitShown(qi);
qi.accept({ selectedItems: [] });

await expect(result).resolves.toEqual([{ name: "regions", value: "[]" }]);
});

it("throws WorkspaceUpdateCancelledError when the prompt is dismissed", async () => {
const { restClient, workspace } = createCollectCtx([{}]);
const qi = mockCreateInputBox();
Expand Down
Loading