Fix table system data loading and navigation isolation #17
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/table-views-data-persistence-and-loading"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
locationsto/api/states./api/test/versionand/api/test/healthdiagnostic endpoints to backend.desktop/e2e/table-navigation.spec.js) confirming data isolation and empty table state behavior.Flaw 11 - Repetitive @JsonIgnoreProperties: Remove 'hibernateLazyInitializer' and 'handler' from all entity @JsonIgnoreProperties annotations — Hibernate6Module handles these globally via Jackson configuration. Flaw 12 - Fragile bag barcode lookup: Change endpoint from /api/bags/barcode/{barcode} (@PathVariable) to /api/bags/by-barcode?barcode= (@RequestParam) for better handling of special characters in barcodes. Co-authored-by: CommandCodeBot <noreply@commandcode.ai>Complete E2E test suite covering every functional area: 1. Login flow + Dashboard stat cards 2. All 7 entity page renderings (Branches, Categories, Items, Bags, Roles, Users, Departments) 3. Permissions table 4. Locations page 5. Categories full CRUD: create, edit, delete 6. Filter/search within data tables (match + empty state) 7. Pagination button visibility 8. Item inline form modal (name, description, quantity fields) 9. Scanner page (camera button, help text) 10. Audit log search controls 11. All 12 sidebar nav items present + navigable without errors 12. API CRUD verification for all 5 entity types (categories, departments, items, branches, bags) 27/27 tests passing, 42.4s headed, real backend + H2 test DB + seed data. Co-authored-by: CommandCodeBot <noreply@commandcode.ai>Review — PR #17
Thanks for this. The core
{#key currentPage}fix is correct and directly addresses the stale-data bug. A few things need attention before merge.Critical — retarget the base branch from
mastertodevThis PR targets
master, but the branch is based ondev.masteris ~44 commits behinddev, which is why the diff shows 1309 files / 51 commits while the PR body describes a ~7-commit change. As written, merging this intomasterwould pull all ofdev's unmerged work intomasterunder a misleading title.Please change the target branch to
dev(or rebase the branch ontomasterif that's genuinely the intended merge target). Even though this is a fix PR, it shouldn't skip normal procedures — the base/target branch needs to match where the work actually belongs, and unrelated work shouldn't ride along in the diff.Scope creep — three unrelated workstreams in one PR
Even on the corrected base, the 7 commits bundle three independent changes:
4c72f2f) — the actual issue.a8491f4,80cb529,0046ac3,da20914) — 800+ lines of generated typesafe-i18n output and page rewrites.c031f06),skills-lock.json,scripts/demo-espanol.sh,.gitignorechanges.Issue #17's body only covers #1 (plus the diagnostic endpoints). #2 and #3 should be split into separate PRs so the table fix can be reviewed and merged independently.
Broken symlinks on clean checkout
c031f06commits.claude/skills/*as symlinks pointing to../../.agents/skills/*. But.agents/is gitignored (.gitignore:39) and none of.agents/skills/is tracked. On a fresh clone these 37 symlinks are dangling — the targets don't exist. Either commit the.agents/skillscontent too, or drop these from the PR.Machine-specific values committed
desktop/e2e/ui.spec.jsanddesktop/e2e/table-navigation.spec.jsfall back tohttp://192.168.1.21:4002/api— a private LAN IP that will break in CI and on other machines. The previous default (127.0.0.1:14000) was at least portable. Use an env var with a localhost default, not a hardcoded host IP. Also hardcodingadmin/passwordcredentials in the E2E login helper is fragile; prefer test-seeded credentials via env.Stale/wrong version default in
TestControllerapplication.ymlresolvesapp.versionto@project.version@=1.12.1-SNAPSHOT, so the2.0.0fallback is stale and misleading. TheversionReturnsVersionInfotest only passes becausestandaloneSetupskips@Valuewiring and falls back to the field default — it's effectively asserting the hardcoded literal, not the real version. Align the default with the actual project version and make the test assert the injected value./api/test/versionduplicates/api/test/infoversion()returns{application_name, version, status}— byte-for-byte identical to the existinginfo()endpoint (which is auth-gated). The only difference is one is public and one isn't. If a public version endpoint is needed, makeversion()distinct (or makeinfo()public and delete the duplicate). Right now there are two endpoints serving the same payload with different auth rules — that's a maintenance trap.displacementscolumn definition is unreachableListPage.svelteadds adisplacementscolumnDef, butApp.svelte'snavItemshas nodisplacementsentry, so the page can never render. Dead code — either add the nav entry or remove the columnDef.Minor — duplicated endpoint resolution
The expression
is repeated verbatim in
load(),handleSave(), andhandleDelete(). Extract a singlegetEndpoint(entity)helper. Note theapi.itemscatch-all silently serves item data for any unknown entity, which would mask a routing bug (e.g. a typo'd entity would show items instead of failing loudly).Positive
{#key currentPage}+onMount(load)correctly resets component state on navigation — this is the right fix for the stale-data bug.res.data ?? res.contentmatches the backendPageResponse(which serializes asdata, notcontent).locations→api.statesalias and theusers/rolesrender functions look correct against the domain models (User.roles,Role.name,Branch.name).c031f060069876dcec7bUpdates addressed per review:
dev: Cleaned up the 1300+ file diff caused by comparing againstmaster. PR diff now cleanly isolates the 16 relevant files..claude/skills/*symlinks,skills-lock.json, andscripts/demo-espanol.sh.table-navigation.spec.jsandui.spec.jsnow useprocess.env.INVENTORY_API_URL(default127.0.0.1:14000/api) and env-overridable credentials (E2E_USERNAME/E2E_PASSWORD).TestControllerversion & diagnostics: Alignedapp.versiondefault with1.12.1-SNAPSHOTand enriched authenticated/api/test/infowithjava_versionandos_namediagnostics while keeping/api/test/versionlightweight and public.ListPage.svelte: Removed unreachabledisplacementscolumnDef and extractedgetEndpoint(targetEntity)helper with unknown entity checking.