diff --git a/CHANGELOG.md b/CHANGELOG.md index 0bcecbc5d..e1c5cdc09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## Fixed +- The `dr workload config` setup wizard, which `dr workload up` also opens for a project with no manifest, no longer shows a fresh directory's suggested name as one you typed. Choosing another directory on the way back, or setting up a new workload after looking at an existing one, rebuilds the draft from this run's flags, but the mark that says the name was given by hand outlived the draft it belonged to, so the new directory's suggestion came back filled in as a value. One reflexive Enter could then put a name like `src` on a deployed workload, which is what the name screen exists to prevent. The draft and that mark now start over together, so the suggestion is offered as a placeholder again; a `--name` still counts as given, because that answer belongs to the run rather than to the draft being replaced. - `dr workload up` now deploys onto an errored workload instead of refusing it, whenever the deploy gives it something new to run: a code change, a change to the manifest or the sizing, or `--force-build`. What the workload is running has failed, so there is nothing serving for the swap to endanger, and the plan says so with the platform's own reason for the failure beside the state. A deploy with nothing new in it is still refused, and the refusal names what would work: `--force-build` for a project whose image the platform builds, a change to the image or artifact otherwise, and the delete as the last resort. An errored workload never has its image copied forward, since on a locked version the copy would be locked too, permanently, pointing at an image the registry may no longer have; the version is built instead. A stopped workload whose start comes up errored ahead of a roll is rolled anyway, and the name conflict on an errored holder advises binding to it again. Together these close a deadlock in which a locked production workload whose image the registry had pruned could be neither deployed onto, nor started, nor rebuilt. - `dr workload up --force-build` now rebuilds and rolls even when the working tree and the manifest have not changed, which is the case the flag exists for: an image gone from the registry leaves both exactly as they were. The flag used to be read only by the build step, so a plan with nothing else in it came out empty and the run reported the workload as up to date. A forced build on a locked version locks its successor to match; on a manifest that names its image rather than building it the flag stays idle and says so. - `dr artifact code sync` and `dr workload up` now replace files in the catalog when the upload takes the zip route, which is any change set of more than 20 files or 50 MB. The overwrite mode was sent only as a query parameter of the Files API's `fromFile` upload, which the server accepts and ignores, so its rename default applied: every path already in the catalog came back as a `name (2).ext` duplicate holding the new bytes while the original kept the old ones, the version had twice the files, and an image built from it ran stale code next to junk. Smaller change sets, which go through a stage, were never affected. The mode now travels in the multipart form, ahead of the file, where the server reads it; it is still sent in the query as well until the API says which of the two is authoritative. diff --git a/internal/workload/wizard/model.go b/internal/workload/wizard/model.go index a26b9abe2..0bc89870f 100644 --- a/internal/workload/wizard/model.go +++ b/internal/workload/wizard/model.go @@ -180,10 +180,7 @@ type secretsStoredMsg struct { func newFlow(detected Detected, workloads []workload.Workload, answers Answers) flow { f := flow{detected: detected, workloads: workloads, answers: answers, pendingBind: answers.WorkloadID} - f.draft = answers.draftOrPartial(detected) - // A name passed as a flag is an answer the user gave, so the screen shows - // it as a value rather than re-offering the directory's suggestion. - f.nameGiven = answers.Name != "" + f.startFrom(answers.draftOrPartial(detected)) f.at = f.first(answers) if f.at == screenDirectory { @@ -210,6 +207,18 @@ func newFlow(detected Detected, workloads []workload.Workload, answers Answers) return f } +// startFrom installs a draft built from this run's flags, at the start and +// again whenever the flow starts over. The name question starts over with it: +// a name typed for the draft being replaced went with that draft, so only a +// --name still counts as given. That flag is an answer the user gave and the +// screen shows it as a value; the suggestion a fresh draft carries is a +// placeholder, and left marked as given it would come back looking typed, +// one reflexive Enter away from naming a deployed workload "src". +func (f *flow) startFrom(draft manifest.Draft) { + f.draft = draft + f.nameGiven = f.answers.Name != "" +} + // defaultDraft is where a run starts when nothing else has been said: the // project's own suggestions and the documented defaults. func defaultDraft(detected Detected) manifest.Draft { @@ -707,17 +716,19 @@ func (f *flow) acceptKind() (tea.Cmd, error) { } // acceptDirectory re-reads the project from the directory chosen. This is -// always the first screen, so no answer beyond this one exists yet and the -// draft is rebuilt the way newFlow built it — everything Detect suggests -// (name, port, Dockerfile, .env) belonged to the old directory. The env -// table is dropped for the same reason: built once from the old tree, it -// would otherwise write the old directory's variables into the new one's -// manifest. f.offer stays untouched throughout, so Escape re-poses the +// always the first screen, and choosing another directory starts the run over +// from it: the draft is rebuilt the way newFlow built it, because everything +// Detect suggests (name, port, Dockerfile, .env) belonged to the old +// directory. On the way back that also discards the answers given after it, +// the typed name included: they were given for the old tree's draft. +// The env table is dropped for the same reason: built once from the old +// tree, it would otherwise write the old directory's variables into the new +// one's manifest. f.offer stays untouched throughout, so Escape re-poses the // original question. func (f *flow) acceptDirectory() (tea.Cmd, error) { if chosen := f.choice.value(); chosen != f.detected.Dir { f.detected = Detect(chosen) - f.draft = f.answers.draftOrPartial(f.detected) + f.startFrom(f.answers.draftOrPartial(f.detected)) f.envTable = envTable{} } @@ -787,7 +798,7 @@ func (f *flow) acceptBinding() (tea.Cmd, error) { // a reason to withdraw it. if f.live != nil { f.live = nil - f.draft = f.answers.partialDraft(f.detected) + f.startFrom(f.answers.partialDraft(f.detected)) } f.draft.WorkloadID = "" @@ -1131,6 +1142,14 @@ func (f flow) liveLoaded(msg liveLoadedMsg) (tea.Model, tea.Cmd) { // --replicas and --memory were given before the fetch and still stand. // Replacing the draft outright would drop every flag the wizard has no // screen to re-ask, which is the same layering the headless bind does. + // + // This is the one draft rebuild that does not go through startFrom, so + // nameGiven is left as it stands. A bound workload is already named and + // never gets asked: branch sends the binding screen to screenKind while + // live is set, and firstQuestion sends a flag-named workload there too, + // so screenName is unreachable and the flag's only reader never runs. + // Give a bound workload a rename screen and that stops being true; this + // site then has to start over from the draft like the other two. f.draft = f.answers.partialApplyTo(live.Defaults(), f.detected) f.pendingBind = "" diff --git a/internal/workload/wizard/regression_test.go b/internal/workload/wizard/regression_test.go index 10f88b4b7..284592ec6 100644 --- a/internal/workload/wizard/regression_test.go +++ b/internal/workload/wizard/regression_test.go @@ -1794,6 +1794,74 @@ func TestFlow_ChangingDirectoryDropsTheOldEnvTable(t *testing.T) { assert.Empty(t, model.envTable.rows, "another tree starts from its own .env") } +// Choosing another directory starts the name over along with the rest of the +// draft. A name typed for the old tree belonged to that draft, and the new +// tree's suggestion is a placeholder, not something the user typed: shown as a +// value, one reflexive Enter would put a name like "src" on a deployed +// workload, which is what the name screen exists to prevent. Staying put keeps +// the typed name, because nothing it was given for has changed. +func TestFlow_ChangingDirectoryStartsTheNameOver(t *testing.T) { + model := newFlow(parentOfProject(t), nil, Answers{}) + require.Equal(t, screenDirectory, model.at) + + model = press(t, model, "down", "enter") // stay put + require.Equal(t, screenName, model.at) + + model = press(t, typeInto(t, model, "chosen-name"), "enter") + require.Equal(t, screenKind, model.at) + + model = press(t, model, "esc", "esc") + require.Equal(t, screenDirectory, model.at) + + model = press(t, model, "down", "enter") // stay put again + require.Equal(t, screenName, model.at) + assert.Equal(t, "chosen-name", model.inputs[0].Value(), "the same tree keeps the name given for it") + + model = press(t, model, "esc") + require.Equal(t, screenDirectory, model.at) + + model = press(t, model, "enter") // take the candidate + require.Equal(t, screenName, model.at) + require.Equal(t, "my-app", model.detected.Name) + assert.Empty(t, model.inputs[0].Value(), "another tree's suggestion is not shown as a typed value") + assert.Equal(t, namePlaceholder, model.inputs[0].Placeholder) +} + +// "Create a new workload" after looking at an existing one starts over, and +// the name is part of that: the one typed before the bind went with the draft +// the bind replaced, and the directory's suggestion the fresh draft carries +// has to come back as a placeholder, not as a value the user appears to have +// typed. +func TestFlow_CreateNewAfterABindStartsTheNameOver(t *testing.T) { + stubLiveDocs(t) + + workloads := []workload.Workload{{ID: "68b0", Name: "live-app", Status: "running", UpdatedAt: time.Now()}} + model := newFlow(dockerfileProject(t), workloads, Answers{}) + require.Equal(t, screenBinding, model.at) + + model = press(t, model, "enter") // create a new workload + require.Equal(t, screenName, model.at) + + model = press(t, typeInto(t, model, "chosen-name"), "enter") + require.Equal(t, screenKind, model.at) + + model = press(t, model, "esc", "esc") + require.Equal(t, screenBinding, model.at) + + model = press(t, model, "down", "enter") // bind live-app + require.NotNil(t, model.live) + require.Equal(t, screenKind, model.at) + + model = press(t, model, "esc") + require.Equal(t, screenBinding, model.at) + + model = press(t, model, "up", "enter") // create a new workload after all + require.Nil(t, model.live) + require.Equal(t, screenName, model.at) + assert.Empty(t, model.inputs[0].Value(), "the suggestion is not shown as a typed value") + assert.Equal(t, namePlaceholder, model.inputs[0].Placeholder) +} + // A --workload-id bind must not hide the directory question behind the // loading view: the fetch waits for the answer, then starts. func TestFlow_BindWaitsForTheDirectoryAnswer(t *testing.T) {