Skip to content

fix(ui): address row actions by the cached record, not the filtered row number - #227

Merged
argszero merged 1 commit into
mainfrom
fix/ui-row-action-filtered-index
Sep 13, 2026
Merged

argszero merged 1 commit into
mainfrom
fix/ui-row-action-filtered-index

Conversation

@argszero

Copy link
Copy Markdown
Owner

Summary

Searching a table rebuilt its rows with Array.prototype.filter, and the row buttons were then tagged with the index inside the searched list — while the click handlers resolve that index against the full cached array:

table buttons handler reads
#api-keys (Settings) copy / rename / delete Live.apiKeys[i]
#model-body (Admin ▸ Models) edit / delete Live.adminModels[i]
#dept-body (Admin ▸ Departments) edit / delete Live.departments[i]

So once the search box hides one earlier row, a button acts on a different record than the row it sits in. Reproduced in a real-DOM harness (jsdom running ui/index.html + the four real scripts, only fetch stubbed): search gamma in Settings, click that row's delete → DELETE /api/api-keys/11 (alpha) instead of /33; the inline rename PATCHes /api/api-keys/11; copy puts alpha's full secret on the clipboard. Same for the model and department tables (edit form prefilled with another record's values; delete hits the wrong id).

Two correct idioms already exist in the very same file — #emp-body tags rows with users.indexOf(u) and #ops-body tags them with u.id — so this is drift introduced when the search boxes were wired up (#79, #80, #97), not a design decision.

Related Issue

Changes

  • ui/js/app.js
    • #api-keys: carry the cached index through the mapping (idx), add data-key-row, and resolve the row in renameKey by that locator instead of tr:nth-child(i+1).
    • #model-body / #dept-body: derive the locator with indexOf on the cached array, mirroring #emp-body.
    • Comment on each site stating that the locator is the index in the cached array, not the row number of the searched list.
  • ui/index.html: bump the app.js cache-bust (?v=20260914-4).
  • No config/config.example.toml change (no new data/config keys). No new i18n keys.

Tests

  • cargo test — 235 passed, 0 failed (unchanged baseline)

  • cargo fmt --check — clean

  • cargo clippy --all-targets -- -D warnings — clean

  • New/updated unit tests — not applicable: this repo has no JS test infrastructure and app.js behaviour is not reachable from cargo test. Verified instead with a real-DOM (jsdom) harness driving ui/index.html + the four real scripts, plus an A/B run:

    leg red checks (behavioural, per table)
    app.js at origin/main (pre-change) A4 A5 A6 · B4 B5 · C4 C5 C6 (9/18 pass)
    fixed tree 0 of those; 17/18 pass
    fixed tree with only the api-keys locator reverted A4 A5 A6
    fixed tree with only the model-body locator reverted B4 B5
    fixed tree with only the dept-body locator reverted C4 C5 C6

    The three partial reverts are pairwise disjoint and together account for exactly the pre-change failure set; the unfiltered-control checks (A2/B2/C2) stay green in every leg. The one remaining red check in the fixed tree is an unrelated defect (the model form's submit button never fires its request — nested withLoading busy-guard in confirmModel); it fails identically before and after this change and is deliberately left for its own PR.

Checklist

  • Branch命名符合约定 (fix/)
  • Commit message 使用 Conventional Commits 格式
  • 单一职责,改动最小化 (2 files, +23/−13, UI only)

…ow number

Three tables rebuild their list with .filter() and then tag every row button with
the index *inside the searched list*, while the handlers resolve that index
against the full cached array:

  #api-keys   copyKey / renameKey / deleteKey   -> Live.apiKeys[i]
  #model-body editModelRow / deleteModel        -> Live.adminModels[i]
  #dept-body  openDeptForm / deleteDept         -> Live.departments[i]

So as soon as the search box hides one earlier row, a button acts on a different
record than the row it sits in: the delete removed the wrong API key / model /
department, copy handed out another key's secret, and the edit form opened with
another record's values.

Both correct idioms already exist in the same file — #emp-body tags rows with
users.indexOf(u) and #ops-body with u.id — these three tables were simply never
updated when their search boxes were added (#79, #80, #97).

- api-keys: carry the cached index through the mapping, add data-key-row and
  resolve the row by that locator instead of nth-child
- model-body / dept-body: derive the locator with indexOf, mirroring #emp-body
- bump the app.js cache-bust
@argszero
argszero merged commit 26d448b into main Sep 13, 2026
1 check passed
@argszero
argszero deleted the fix/ui-row-action-filtered-index branch September 13, 2026 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant