fix(docker): pre-fill the copied case instead of blanking two fields

The previous version cleared the case name and the in-container directory on the
grounds that they must differ. That left a form with three fields mysteriously
filled and two empty, and turned the most common operation — changing
/srv/app/api to /srv/app/web — into retyping a long path.

Both are now pre-filled, with focus on the in-container directory and the caret
at the end, since the tail is what changes. What stops an unmodified submit is no
longer an empty field but a guard: the values applied are recorded, compared at
submit time, and if nothing changed the reason is stated next to the field and
focus moves to it, without sending a request that is certain to be refused.

The server refuses these anyway (a duplicate case name, a twin case on the same
container and directory) and its errors are clear; but making a round trip to be
told "you forgot to edit the field you are looking at" is worse than saying so on
the spot. The guard only applies when a source case was actually selected, so
filling the adopt form from scratch is unaffected.

⚠️ The status text is written into dockerLinkStatus. My first version referenced
an id that does not exist (dockerAdoptStatus), which made the explanation vanish
silently and left only a toast. The test now extracts that id from the code and
looks it up in index.html, pinning that it must really exist.

(cherry picked from commit ba21ae11f4)
This commit is contained in:
d fei
2026-09-14 23:56:19 +02:00
committed by Codeman maintainer
parent 7a5543da09
commit 025f061383
2 changed files with 103 additions and 12 deletions
+55 -3
View File
@@ -3210,6 +3210,7 @@ Object.assign(CodemanApp.prototype, {
option.dataset.container = c.docker.container;
option.dataset.hostId = c.docker.hostId;
option.dataset.path = c.docker.path;
option.dataset.workdir = c.docker.containerWorkdir || c.docker.path;
select.appendChild(option);
}
// Nothing to duplicate yet: an empty picker is noise on the first adoption.
@@ -3236,9 +3237,49 @@ Object.assign(CodemanApp.prototype, {
set('dockerContainerName', option.dataset.container);
set('dockerHostId', option.dataset.hostId);
set('dockerWorkspacePath', option.dataset.path);
set('dockerCaseName', '');
set('dockerAdoptWorkdir', '');
document.getElementById('dockerAdoptWorkdir')?.focus();
// Pre-filled, NOT cleared: these two must differ from the source, but editing
// `/srv/app/api` into `/srv/app/web` beats retyping a long path, and the same
// goes for the name. What keeps a duplicate from being submitted unchanged is
// the guard below (dockerCloneGuard), which is a better trade than an empty
// field: the form stays a starting point instead of a blank form with three
// fields mysteriously filled in.
set('dockerCaseName', option.value);
set('dockerAdoptWorkdir', option.dataset.workdir);
// Remembered so the guard can tell "unchanged" from "happens to look similar".
select.dataset.appliedName = option.value;
select.dataset.appliedWorkdir = option.dataset.workdir || '';
const workdir = document.getElementById('dockerAdoptWorkdir');
workdir?.focus();
// Caret at the end: the tail is the part that changes.
if (workdir) workdir.setSelectionRange(workdir.value.length, workdir.value.length);
},
/**
* Refuse a duplicate that still carries the source case's name or directory.
*
* Both are pre-filled so they can be EDITED, which means both can also be left
* alone by accident. The server refuses either (an existing case name, or an
* exact same-container-same-directory twin) with a clear message, but a
* round-trip to be told "you forgot to change the field you were looking at" is
* worse than saying so here, next to the field, before anything is sent.
*
* Returns the offending element, or null when the form is fine.
*/
dockerCloneGuard() {
const select = document.getElementById('dockerAdoptCloneFrom');
if (!select || !select.value) return null;
const name = document.getElementById('dockerCaseName');
const workdir = document.getElementById('dockerAdoptWorkdir');
if (name && name.value.trim() === (select.dataset.appliedName || '')) {
return { el: name, message: `"${name.value.trim()}" is the case you copied from — give this one a new name.` };
}
if (workdir && workdir.value.trim() === (select.dataset.appliedWorkdir || '')) {
return {
el: workdir,
message: 'Same container and same directory as the case you copied from — point this one at another directory.',
};
}
return null;
},
async _loadDockerContainerOptions() {
@@ -3341,6 +3382,17 @@ Object.assign(CodemanApp.prototype, {
this.showToast('Enter the name of the running container to attach to', 'error');
return;
}
// A duplicate that still carries the source's name or directory: say so here,
// beside the field, rather than sending a request certain to come back refused.
const cloneIssue = adopting ? this.dockerCloneGuard() : null;
if (cloneIssue) {
this.showToast(cloneIssue.message, 'error');
const statusEl = document.getElementById('dockerLinkStatus');
if (statusEl) statusEl.textContent = cloneIssue.message;
cloneIssue.el.focus();
cloneIssue.el.select?.();
return;
}
try {
if (statusEl) {
+48 -9
View File
@@ -55,8 +55,8 @@ describe('the picker only offers what the server would accept', () => {
});
});
describe('applying a source fills what stays and clears what must differ', () => {
const fn = ui.slice(ui.indexOf('applyDockerCloneSource()'), ui.indexOf('applyDockerCloneSource()') + 1400);
describe('applying a source fills every field, including the two that must differ', () => {
const fn = ui.slice(ui.indexOf('applyDockerCloneSource()'), ui.indexOf('dockerCloneGuard()'));
it('carries over container, host and workspace', () => {
for (const id of ['dockerContainerName', 'dockerHostId', 'dockerWorkspacePath']) {
@@ -64,15 +64,21 @@ describe('applying a source fills what stays and clears what must differ', () =>
}
});
it('clears the case name and the container workdir', () => {
// Keeping either would pre-fill a value the server is certain to refuse —
// the name as an existing case, the workdir as an exact twin.
expect(fn).toContain("set('dockerCaseName', '')");
expect(fn).toContain("set('dockerAdoptWorkdir', '')");
it('PRE-FILLS the case name and container workdir rather than clearing them', () => {
// Editing `/srv/app/api` into `/srv/app/web` beats retyping a long path, and a
// form with three fields mysteriously filled and two blank reads as broken.
// What stops an unchanged submit is the guard, not an empty field.
expect(fn).toContain("set('dockerCaseName', option.value)");
expect(fn).toContain("set('dockerAdoptWorkdir', option.dataset.workdir)");
});
it('focuses the workdir, the field the user came here to change', () => {
expect(fn).toMatch(/getElementById\('dockerAdoptWorkdir'\)\?\.focus\(\)/);
it('remembers what it applied, so the guard can tell unchanged from similar', () => {
expect(fn).toContain('select.dataset.appliedName = option.value');
expect(fn).toContain('select.dataset.appliedWorkdir =');
});
it('focuses the workdir with the caret at the END, where the edit happens', () => {
expect(fn).toMatch(/setSelectionRange\(workdir\.value\.length, workdir\.value\.length\)/);
});
it('does nothing for the blank "start from scratch" option', () => {
@@ -80,6 +86,39 @@ describe('applying a source fills what stays and clears what must differ', () =>
});
});
describe('the guard refuses a duplicate that was never edited', () => {
const fn = ui.slice(ui.indexOf('dockerCloneGuard()'), ui.indexOf('dockerCloneGuard()') + 1400);
it('flags an unchanged case name', () => {
expect(fn).toMatch(/appliedName/);
expect(fn).toContain('give this one a new name');
});
it('flags an unchanged container workdir', () => {
expect(fn).toMatch(/appliedWorkdir/);
expect(fn).toContain('another directory');
});
it('stays silent when no source was picked', () => {
// Typing a fresh adoption by hand must not be second-guessed.
expect(fn).toMatch(/if \(!select \|\| !select\.value\) return null;/);
});
it('runs BEFORE the request, and focuses the offending field', () => {
const submit = ui.slice(ui.indexOf('const cloneIssue'), ui.indexOf('const cloneIssue') + 500);
expect(submit).toContain('cloneIssue.el.focus()');
expect(submit).toContain('return;');
});
it('reports into a status element that actually exists', () => {
// A dead id would silently drop the explanation next to the field.
const submit = ui.slice(ui.indexOf('const cloneIssue'), ui.indexOf('const cloneIssue') + 500);
const id = /getElementById\('([^']+)'\)/.exec(submit)?.[1];
expect(id).toBeTruthy();
expect(html).toContain(`id="${id}"`);
});
});
describe('the row is wired into the adoption panel', () => {
it('lives in the adopt-only block and starts hidden', () => {
expect(html).toMatch(/id="dockerAdoptCloneRow"[^>]*hidden/);