fix(#571): the admin save bar never appeared — f.internalShare #586

Merged

Cordy merged 1 commit from fix-savebar into main

2026-09-18 00:14:35 +00:00

Cordy commented

2026-09-18 00:12:26 +00:00

Owner

Found on the v0.6.212 dogfood: the sharing policy toggles moved but nothing saved, no save bar appeared, and the console carried Load failed: f is not defined.

One character. loadSettingsValues() read the new switch off f — the parameter name of sharingPolicyFromForm, eleven lines below — instead of shp:

$("set-publiclinks").checked = shp.publicLinks;
$("set-internalshare").checked = f.internalShare;   // ← here
$("set-uploadlinks").checked  = shp.uploadLinks;

The blast radius is the whole admin surface, not the sharing switch. That line sits near the top of the function, so the ReferenceError aborted everything below it: upload links, audit logging, OCM, download marking, retention, trash auto-purge, purge-after, both quotas, warn percent, lock max, notification and transfer retention, space folder locks. None of those got loaded — and none got dataset.base. admDirtySync() diffs against dataset.base to count dirty fields, so with no baseline the count was always zero and no admin setting could be saved at all in v0.6.212. What the page displayed below that line was markup defaults, not stored state.

Why the existing tests were green. sharing-policy.test.js lifts the pure pure:sharing-policy block and exercises it hard, including four #571 cases for exactly this field — and the block is correct in both directions. The bug was in the DOM glue one line outside the fence. The comment above that block says a flipped negation "needs a test rather than a careful reader"; the same argument applies to the glue, and nothing covered it.

New: web/test/settings-load.test.js. Lifts loadSettingsValues out of index.html, stubs $/api/retLoad/syncTrashAuto/GIB, and runs it. Four claims, the first two deliberately broader than this bug:

it runs to completion — catches any undefined reference or thrown call anywhere in the function;
it assigns every set-* field it references, with the field list read from the source so the test does not go stale as fields are added — this is the one that would have caught the save-bar symptom rather than just the ReferenceError;
the internal-sharing switch reflects stored state in both directions;
an absent key leaves internal sharing on (upgrade safety, at the glue rather than the mapping).

Also straightened the indentation on the two pure: block lines the same splice left at six spaces.

Red witnessed: all four failed with f is not defined before the fix; the workflow aborts if the test passes pre-fix. Green: all four pass, plus the other eleven web tests and the #389 extract gate.

One process note: the first attempt died on my own post-condition, which asserted on the bare substring " internalShare:". index.html legitimately carries that key at six spaces elsewhere, so the check failed on an unrelated line and the splice correctly wrote nothing. Tightened to full-line matches with their values. That is the third instance of the hazard already in the house notes — assert on the line, never on the word.

Found on the v0.6.212 dogfood: the sharing policy toggles moved but nothing saved, no save bar appeared, and the console carried `Load failed: f is not defined`. **One character.** `loadSettingsValues()` read the new switch off `f` — the parameter name of `sharingPolicyFromForm`, eleven lines below — instead of `shp`: ```js $("set-publiclinks").checked = shp.publicLinks; $("set-internalshare").checked = f.internalShare; // ← here $("set-uploadlinks").checked = shp.uploadLinks; ``` **The blast radius is the whole admin surface, not the sharing switch.** That line sits near the top of the function, so the ReferenceError aborted everything below it: upload links, audit logging, OCM, download marking, retention, trash auto-purge, purge-after, both quotas, warn percent, lock max, notification and transfer retention, space folder locks. None of those got loaded — and none got `dataset.base`. `admDirtySync()` diffs against `dataset.base` to count dirty fields, so with no baseline the count was always zero and **no admin setting could be saved at all** in v0.6.212. What the page displayed below that line was markup defaults, not stored state. **Why the existing tests were green.** `sharing-policy.test.js` lifts the pure `pure:sharing-policy` block and exercises it hard, including four #571 cases for exactly this field — and the block is correct in both directions. The bug was in the DOM glue one line outside the fence. The comment above that block says a flipped negation "needs a test rather than a careful reader"; the same argument applies to the glue, and nothing covered it. **New: `web/test/settings-load.test.js`.** Lifts `loadSettingsValues` out of index.html, stubs `$`/`api`/`retLoad`/`syncTrashAuto`/`GIB`, and runs it. Four claims, the first two deliberately broader than this bug: 1. it runs to completion — catches any undefined reference or thrown call anywhere in the function; 2. it assigns every `set-*` field it references, with the field list read from the source so the test does not go stale as fields are added — this is the one that would have caught the save-bar symptom rather than just the ReferenceError; 3. the internal-sharing switch reflects stored state in both directions; 4. an absent key leaves internal sharing on (upgrade safety, at the glue rather than the mapping). Also straightened the indentation on the two `pure:` block lines the same splice left at six spaces. **Red witnessed:** all four failed with `f is not defined` before the fix; the workflow aborts if the test passes pre-fix. **Green:** all four pass, plus the other eleven web tests and the #389 extract gate. One process note: the first attempt died on my own post-condition, which asserted on the bare substring `" internalShare:"`. index.html legitimately carries that key at six spaces elsewhere, so the check failed on an unrelated line and the splice correctly wrote nothing. Tightened to full-line matches with their values. That is the third instance of the hazard already in the house notes — assert on the line, never on the word.

Cordy added 1 commit

2026-09-18 00:12:28 +00:00