Skip to content

InventoryPage: a write's refresh re-activates its own item, yanking a user who has navigated elsewhere #630

Closed

Description

@mforce

Found while reviewing PR #628 (#511). Pre-existing, not introduced by #511 — verified byte-identical at fae8d821, the base commit of that PR:

git show fae8d821:web/src/routes/InventoryPage.tsx   # refreshAll, lines 133-142

What happens

refreshAll(openItemId) ends with:

const target = openItemId ?? active?.id;
if (target) {
  const stillThere = fresh.find((i) => i.id === target) ?? null;
  setActive(stillThere);          // <-- re-activates the WRITE's item
  if (stillThere) await loadLots(stillThere.id);
}

Every write on this screen calls it, and recordInventoryPurchase / the adjustment write pass active.id captured when the handler ran. So: open item A, start a purchase, open item B while the POST is in flight, and when the write settles the panel switches itself back to A. The user is navigated away from the item they chose, by a write they already finished with.

onCreate and the edit write pass no openItemId, so they fall back to active?.id read from the handler's closure — the same staleness by a different route.

Why it is worth recording rather than fixing in passing

It is the reason a guard added in #511's review round turned out to be unreachable: because refreshAll always re-activates its target immediately before loading that target's lots, an "is this item still open?" check downstream can never disagree. Deleting that guard left the entire InventoryPage suite green. Anyone who later tries to make lots and movements agree about "which item is open" will hit this same wall, so the re-activation is the thing to change, not the downstream reads.

Suggested shape

Either drop setActive(stillThere) and let the refresh update items/lots without moving the user, or make it conditional on the target still being the active item. The second is smaller; the first is probably what users expect. Needs a test that starts a write on A, switches to B mid-flight, and asserts the panel still shows B when the write settles — no such interleaving exists in the suite today.

Related: #511, PR #628, #629.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:frontendReact/Vite web clientbugSomething isn't workingepic-1.5severity:p2Defect: user-visible wrong behaviour, no data loss

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions