mirror of
https://github.com/coder/coder.git
synced 2026-10-16 14:48:56 +08:00
fix(cli): offer a None option in the coder create preset prompt (#30068)
When a template has presets but no default preset, `coder create`
prompts for a preset, but the prompt listed only the template's presets.
The only way to skip a preset was `--preset none`, while the dashboard
lists **None** first.
This adds a **None** entry at the top of the prompt.
Selecting it behaves like `--preset none`: the CLI sends no preset ID
and prints `No preset applied.`
If a preset's label is also `None`, the no-preset entry is labeled `None
(no preset)` so both stay selectable.
Default preset auto-selection is unchanged.
> [!NOTE]
> "No preset ID sent" doesn't always mean the build records no preset.
> When a create request has no preset ID, coderd matches the parameter
values against the template's presets (`prebuilds.FindMatchingPresetID`)
and records any exact match, as `--preset none` and the dashboard's None
option already do.
Fixes ENG-3407
<details>
<summary>Validation and decision log</summary>
### Validation
Tested at e5f52ffe4b against Postgres.
- `TestCreate`, `TestCreateWithPreset`, and the new
`TestPresetSelectOptions` pass against Postgres 13.
- Ran a dev server from this branch (`./scripts/develop.sh
--port-offset`) and drove `coder create` through a real PTY against a
template with two presets and no default:
- The menu lists `None`, `eu-preset: Deploys to EU`, `us-preset`.
- `None` prints `No preset applied.`, then prompts for parameters.
- Selecting either preset applies its parameters.
- `--preset none` and the prompt's `None` produce identical
`workspace_builds.template_version_preset_id` values.
With `region=us`, both record the matching `us-preset` because of
server-side preset matching.
- Template with a preset literally named `None`:
- The dashboard shows `None` twice. Each entry has its own internal
value, so both work, but they look identical.
- The first commit on this branch showed `None` twice in the CLI too,
and both entries meant "no preset". That made the `None` preset
unreachable from the prompt, which is a regression from `main`.
- The second commit fixes that: the menu shows `None (no preset)`,
`None`, `us-preset`, and selecting `None` applies the preset
(`region=eu`).
### Decisions
- **Label `None` to match the dashboard.** Switch to `None (no preset)`
only when a preset's label collides, so the common case stays identical
to the dashboard.
- **Out of scope: `--preset None` flag.** The flag matches `none`
case-insensitively before any preset name, so a preset named `None` or
`none` can't be selected by flag. That's unchanged from `main`.
- **Out of scope: default presets.** The dashboard preselects the
default preset but still lets you pick another preset or None. The CLI
applies the default without prompting. That behavior needs a separate
decision.
</details>
---
_Generated by Coder Agents on behalf of @nickvigilante._
This commit is contained in:
+34
-6
@@ -302,12 +302,15 @@ func (r *RootCmd) Create(opts CreateOptions) *serpent.Command {
|
||||
if !errors.Is(err, ErrNoPresetFound) {
|
||||
return xerrors.Errorf("unable to resolve preset: %w", err)
|
||||
}
|
||||
// If no preset found, prompt the user to choose a preset
|
||||
// If no preset found, prompt the user to choose a preset.
|
||||
// A nil preset means the user chose "None".
|
||||
if preset, err = promptPresetSelection(inv, tvPresets); err != nil {
|
||||
return xerrors.Errorf("unable to prompt user for preset: %w", err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if preset != nil {
|
||||
// Convert preset parameters into workspace build parameters
|
||||
presetParameters = presetParameterAsWorkspaceBuildParameters(preset.Parameters)
|
||||
// Inform the user which preset was applied and its parameters
|
||||
@@ -516,12 +519,21 @@ func resolvePreset(presets []codersdk.Preset, presetName string) (*codersdk.Pres
|
||||
return nil, ErrNoPresetFound
|
||||
}
|
||||
|
||||
// promptPresetSelection shows a CLI selection menu of the presets defined in the template version.
|
||||
// Returns the selected preset
|
||||
func promptPresetSelection(inv *serpent.Invocation, presets []codersdk.Preset) (*codersdk.Preset, error) {
|
||||
presetMap := make(map[string]*codersdk.Preset)
|
||||
var presetOptions []string
|
||||
const (
|
||||
// presetNoneOption is the selection menu label for creating a workspace
|
||||
// without a preset, matching the dashboard's "None" option.
|
||||
presetNoneOption = "None"
|
||||
// presetNoneFallbackOption replaces presetNoneOption when a preset's
|
||||
// label is also "None", so the menu never shows two identical entries.
|
||||
presetNoneFallbackOption = "None (no preset)"
|
||||
)
|
||||
|
||||
// presetSelectOptions builds the preset selection menu entries. The first
|
||||
// entry is the label for applying no preset, which is also returned as
|
||||
// noneOption. presetMap maps every other entry to its preset.
|
||||
func presetSelectOptions(presets []codersdk.Preset) (options []string, noneOption string, presetMap map[string]*codersdk.Preset) {
|
||||
presetMap = make(map[string]*codersdk.Preset, len(presets))
|
||||
presetOptions := make([]string, 0, len(presets))
|
||||
for _, preset := range presets {
|
||||
var option string
|
||||
if preset.Description == "" {
|
||||
@@ -533,6 +545,19 @@ func promptPresetSelection(inv *serpent.Invocation, presets []codersdk.Preset) (
|
||||
presetMap[option] = &preset
|
||||
}
|
||||
|
||||
noneOption = presetNoneOption
|
||||
if _, ok := presetMap[noneOption]; ok {
|
||||
noneOption = presetNoneFallbackOption
|
||||
}
|
||||
return append([]string{noneOption}, presetOptions...), noneOption, presetMap
|
||||
}
|
||||
|
||||
// promptPresetSelection shows a CLI selection menu of the presets defined in the template version,
|
||||
// preceded by an option to apply no preset.
|
||||
// Returns the selected preset, or nil if the user chose to apply no preset.
|
||||
func promptPresetSelection(inv *serpent.Invocation, presets []codersdk.Preset) (*codersdk.Preset, error) {
|
||||
presetOptions, noneOption, presetMap := presetSelectOptions(presets)
|
||||
|
||||
// Show selection UI
|
||||
_, _ = fmt.Fprintln(inv.Stdout, pretty.Sprint(cliui.DefaultStyles.Wrap, "Select a preset below:"))
|
||||
selected, err := cliui.Select(inv, cliui.SelectOptions{
|
||||
@@ -543,6 +568,9 @@ func promptPresetSelection(inv *serpent.Invocation, presets []codersdk.Preset) (
|
||||
return nil, xerrors.Errorf("failed to select preset: %w", err)
|
||||
}
|
||||
|
||||
if selected == noneOption {
|
||||
return nil, nil //nolint:nilnil // A nil preset means no preset is applied.
|
||||
}
|
||||
return presetMap[selected], nil
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,67 @@
|
||||
package cli
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
)
|
||||
|
||||
func TestPresetSelectOptions(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
eu := codersdk.Preset{ID: uuid.New(), Name: "eu", Description: "Deploys to EU"}
|
||||
us := codersdk.Preset{ID: uuid.New(), Name: "us"}
|
||||
namedNone := codersdk.Preset{ID: uuid.New(), Name: "None"}
|
||||
namedNoneWithDescription := codersdk.Preset{ID: uuid.New(), Name: "None", Description: "Minimal"}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
presets []codersdk.Preset
|
||||
wantOptions []string
|
||||
wantNone string
|
||||
wantPresets map[string]uuid.UUID
|
||||
}{
|
||||
{
|
||||
name: "NoneFirst",
|
||||
presets: []codersdk.Preset{eu, us},
|
||||
wantOptions: []string{"None", "eu: Deploys to EU", "us"},
|
||||
wantNone: "None",
|
||||
wantPresets: map[string]uuid.UUID{"eu: Deploys to EU": eu.ID, "us": us.ID},
|
||||
},
|
||||
{
|
||||
// A preset whose label is "None" must stay selectable, so the
|
||||
// option for applying no preset gets a distinct label.
|
||||
name: "PresetNamedNone",
|
||||
presets: []codersdk.Preset{namedNone, us},
|
||||
wantOptions: []string{"None (no preset)", "None", "us"},
|
||||
wantNone: "None (no preset)",
|
||||
wantPresets: map[string]uuid.UUID{"None": namedNone.ID, "us": us.ID},
|
||||
},
|
||||
{
|
||||
name: "PresetNamedNoneWithDescription",
|
||||
presets: []codersdk.Preset{namedNoneWithDescription},
|
||||
wantOptions: []string{"None", "None: Minimal"},
|
||||
wantNone: "None",
|
||||
wantPresets: map[string]uuid.UUID{"None: Minimal": namedNoneWithDescription.ID},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
options, noneOption, presetMap := presetSelectOptions(tt.presets)
|
||||
require.Equal(t, tt.wantOptions, options)
|
||||
require.Equal(t, tt.wantNone, noneOption)
|
||||
require.NotContains(t, presetMap, noneOption)
|
||||
require.Len(t, presetMap, len(tt.wantPresets))
|
||||
for option, id := range tt.wantPresets {
|
||||
require.Contains(t, presetMap, option)
|
||||
require.Equal(t, id, presetMap[option].ID)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
+8
-11
@@ -1395,7 +1395,8 @@ func TestCreateWithPreset(t *testing.T) {
|
||||
|
||||
// This test verifies that when a template has presets but no default preset,
|
||||
// and the user does not provide the `--preset` flag,
|
||||
// the CLI prompts the user to select a preset.
|
||||
// the CLI prompts the user to select a preset, offering "None" first.
|
||||
// Selecting "None" behaves like `--preset none`.
|
||||
t.Run("NoDefaultPresetPromptUser", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
logger := testutil.Logger(t)
|
||||
@@ -1436,32 +1437,28 @@ func TestCreateWithPreset(t *testing.T) {
|
||||
// Should: prompt the user for the preset
|
||||
stdout.ExpectMatch(ctx, "Select a preset below:")
|
||||
// We don't actually have to respond to the selector, since we hardcode the cliui.Select to return the
|
||||
// first option in test scenarios (c.f. cliui/select.go)
|
||||
stdout.ExpectMatch(ctx, "Preset 'preset-test' applied")
|
||||
// first option in test scenarios (c.f. cliui/select.go). The first option is "None".
|
||||
stdout.ExpectMatch(ctx, "No preset applied.")
|
||||
stdout.ExpectMatch(ctx, "Confirm create?")
|
||||
stdin.WriteLine("yes")
|
||||
|
||||
<-doneChan
|
||||
|
||||
// Verify if the new workspace uses expected parameters.
|
||||
tvPresets, err := client.TemplateVersionPresets(ctx, version.ID)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, tvPresets, 1)
|
||||
|
||||
workspaces, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{
|
||||
Name: workspaceName,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.Len(t, workspaces.Workspaces, 1)
|
||||
|
||||
// Should: create a workspace using the expected template version and the preset-defined parameters
|
||||
// Should: create a workspace using the expected template version, no preset,
|
||||
// and the parameters from the command line flags
|
||||
workspaceLatestBuild := workspaces.Workspaces[0].LatestBuild
|
||||
require.Equal(t, version.ID, workspaceLatestBuild.TemplateVersionID)
|
||||
require.Equal(t, tvPresets[0].ID, *workspaceLatestBuild.TemplateVersionPresetID)
|
||||
require.Nil(t, workspaceLatestBuild.TemplateVersionPresetID)
|
||||
buildParameters, err := client.WorkspaceBuildParameters(ctx, workspaceLatestBuild.ID)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, buildParameters, 2)
|
||||
require.Contains(t, buildParameters, codersdk.WorkspaceBuildParameter{Name: firstParameterName, Value: secondOptionalParameterValue})
|
||||
require.Contains(t, buildParameters, codersdk.WorkspaceBuildParameter{Name: firstParameterName, Value: firstOptionalParameterValue})
|
||||
require.Contains(t, buildParameters, codersdk.WorkspaceBuildParameter{Name: thirdParameterName, Value: thirdParameterValue})
|
||||
})
|
||||
|
||||
|
||||
Reference in New Issue
Block a user