vehicles: /vehicle list + sidebar details (iter 1, #26)#67
Conversation
Single source of truth for the chip filter set used on vehicle-domain list views. Adds the NeTEx TransportModeEnumeration union plus the matching FilterDefinition[] keyed under transportMode.* i18n keys. Consumed by the new /vehicle list (#26) from day one; #24 retrofits VehicleType and DeckPlan ViewConfigs separately. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Conventional Inanna feature directory replacing the packages/vehicle-details PoC approach. Mirrors src/data/vehicle-types/. - vehicleTypes.ts: flat Vehicle row + VehicleColumnKey - fetchVehicles.ts: thin flatten over ../vehicle-types/fetchVehicleTypes (no duplicated GraphQL query; enriches each Vehicle with parent VehicleType context for display + client-side TransportMode chip filter) - useVehicles.ts: data hook mirroring useVehicleTypes (single fetch, client-side sort/pagination) - vehicleSortValue.ts: comparator via compareWithEmptyLast - vehicleViewConfig.tsx: ViewConfig consuming shared transportModeFilters - VehicleView.tsx: thin GenericDataViewPage wrapper - VehicleDetails.tsx: sidebar editor mounted via EditingContext; read-only by default, Edit chip toggles to form view, no Save button (gated on layout approval per #26 iter 2) - cells/RowClickCell.tsx: row trigger calling setEditingItem(...) packages/vehicle-details/ is intentionally left untouched as the prior PoC. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ProtectedRoute mounting VehicleView, placed beside /vehicle-type and /deck-plan inside the existing EditingProvider so the sidebar pattern works out of the box. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both en and nb gain: - transportMode.* (13 NeTEx-aligned modes; rail/metro/tram/bus/coach/ trolleyBus/water/air/taxi/cableway/funicular/lift/snowAndIce) - vehicles.* (title, detailsTitle, edit/view chip labels, notFound, field.* labels) transportMode.* is shared infrastructure for #24's chip-filter retrofit across VehicleType/DeckPlan as well. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- isTransportMode: runtime guard for narrowing unknown strings (e.g. GraphQL responses, URL params) to the TransportMode union. Keeps unknown backend values out of the typed surface and surfaces drift via undefined rather than crashing or silently mis-rendering. - transportModeLabelKey: maps a TransportMode to its transportMode.* i18n key for use in column cells and detail labels. Pure additions; existing callers unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Multi-agent review surfaced 5 must-fix items + a missing test file.
- Rename Vehicle → VehicleRow to avoid collision with vehicle-types'
existing Vehicle type (the wire shape nested under VehicleType). The
new VehicleRow is a flat view model with parent* enrichment fields.
- Drop the unchecked `as TransportMode` cast in fetchVehicles; use the
new isTransportMode guard so unknown backend values become undefined
rather than silently polluting the typed surface.
- Pass the full row via closure from RowClickCell to VehicleDetails.
VehicleDetails no longer calls useVehicles() — removes a full-dataset
re-fetch on every row click (high-severity efficiency finding).
- Add TransportModeChip cell that renders the localized label
(t('transportMode.*')) rather than the raw NeTEx enum string.
Same chip is used in the column cell and in the details fieldValue.
- Drop the redundant Cancel button in VehicleDetails — the Chip
already toggles edit→view; Cancel was a second affordance for the
same direction. Close remains for dismissing the panel entirely.
- Drop a future-rot "iter 1" comment; the JSDoc on the component
already states the no-save contract.
- Add vehicleSortValue.test.ts (8 cases) mirroring the convention
established by vehicleTypeSortValue.test.ts and deckPlanSortValue.test.ts.
Deferred (consistent with existing template; bigger blast radius):
shared GraphQL error helper across the 3 list hooks, useMemo on
sort/pagination, AbortController in doFetch, i18n for column headers
and page title.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`undefined` was a leaky state — every caller had to handle the missing case. Adding `'unknown'` as a first-class member of the TransportMode union keeps the typed surface total and encapsulates the mapping inside `netex/transportMode.ts`: - TransportMode union gains `'unknown'`. - New `toTransportMode(s)`: total mapping. Unknown / missing / null / unrecognised backend strings all collapse to `'unknown'`. - `isTransportMode` now also accepts `'unknown'` (it is a valid mode). - New `transportModeSortValue(m)`: returns `''` for `'unknown'` so rows still sink via `compareWithEmptyLast`; known modes sort by id. - `transportModeFilters` intentionally omits `'unknown'` — chip filters are for specific known modes; rows with `'unknown'` simply don't match any chip. Downstream: - VehicleRow.parentTransportMode is now non-optional `TransportMode`. - fetchVehicles uses toTransportMode at the boundary; the conditional is gone. - TransportModeChip drops the em-dash fallback; renders the localized label (en: "Unknown", nb: "Ukjent"). - VehicleDetails fieldValue handles parentTransportMode explicitly; em-dash branch only fires for other genuinely-missing fields. - vehicleSortValue uses transportModeSortValue. - i18n: added `transportMode.unknown` (en + nb). - Tests: new transportMode.test.ts (8 cases); vehicleSortValue.test.ts updated to use `'unknown'` instead of `undefined`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires a query-param URL contract for the Vehicle list — chips on /vehicle-type rows and the per-row icon on /vehicle both navigate to /vehicle?selected=<id>, which the new useVehicleUrlSelection hook reconciles with EditingContext, sidebar visibility, and pagination (auto-jumps to the page containing the selected row). GenericDataViewPage gains an optional useUrlEffect ViewConfig field (stable no-op fallback preserves hook order on pages that don't opt in) and a symmetric sidebar collapse so the editor's Close button also drops ?selected= from the URL and collapses the drawer. Archives the design rationale in a new FORK_DECISIONS.md — ADR-style log for resolved fork-stage decisions; companion to OPEN_QUESTIONS.md. Backfill of earlier decisions tracked in #66. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds an initial /vehicle list + sidebar-details experience, introduces a shared NeTEx TransportMode model + chip filters, and wires deep-link selection (/vehicle?selected=<id>) into the existing GenericDataViewPage via a new optional view-config hook.
Changes:
- Introduces shared
src/data/netex/transportMode.tsutilities (type/guard/mapping/sort + filter definitions) with unit tests and i18n keys. - Adds a new
/vehiclefeature area (src/data/vehicles/) with list view config, client-side sorting, details sidebar, and URL-driven selection/pagination reconciliation. - Extends the generic table page to support an optional per-page URL→state hook (
useUrlEffect) and adds/vehicleroute + menu entry; updates/vehicle-typevehicle chips to deep-link into/vehicle.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pages/viewConfigTypes.ts | Adds optional useUrlEffect hook to ViewConfig. |
| src/pages/GenericDataViewPage.tsx | Invokes useUrlEffect and updates sidebar open/close behavior based on editor state. |
| src/locales/en/translation.json | Adds transportMode.* + vehicles.* translations. |
| src/locales/nb/translation.json | Adds transportMode.* + vehicles.* translations. |
| src/data/vehicles/vehicleViewConfig.tsx | Defines /vehicle table columns, sorting/filtering, and URL selection hook wiring. |
| src/data/vehicles/VehicleView.tsx | New /vehicle route entry point using GenericDataViewPage. |
| src/data/vehicles/vehicleUrlParams.ts | Centralizes ?selected= param key and link builder. |
| src/data/vehicles/vehicleTypes.ts | Adds VehicleRow view model and sortable column key union. |
| src/data/vehicles/vehicleSortValue.ts | Implements vehicle sort value mapping + comparator. |
| src/data/vehicles/vehicleSortValue.test.ts | Unit tests for vehicle sorting behavior. |
| src/data/vehicles/VehicleDetails.tsx | Sidebar details view with view/edit toggle (read-only in iter 1) and URL-close behavior. |
| src/data/vehicles/useVehicleUrlSelection.tsx | Hook reconciling ?selected= with editor state and pagination. |
| src/data/vehicles/useVehicles.ts | Data hook for /vehicle (fetch + client-side sort/pagination). |
| src/data/vehicles/fetchVehicles.ts | Flattens vehicleTypes[].vehicles[] into VehicleRow[] with parent enrichment. |
| src/data/vehicles/cells/TransportModeChip.tsx | Chip renderer for localized transport mode display. |
| src/data/vehicles/cells/RowClickCell.tsx | Per-row action that navigates to /vehicle?selected=<id>. |
| src/data/vehicle-types/cells/VehicleListCell.tsx | Makes vehicle chips deep-link into the new /vehicle route. |
| src/data/netex/transportMode.ts | Shared transport-mode union + mapping/guard/sort + filter definition list. |
| src/data/netex/transportMode.test.ts | Unit tests for the transport-mode utilities. |
| src/components/Menu.tsx | Adds “Vehicles” menu entry. |
| src/App.tsx | Registers the /vehicle route. |
| FORK_DECISIONS.md | Documents the deep-link design decision (ADR-style). |
| CLAUDE.md | Adds references to FORK_DECISIONS.md and OPEN_QUESTIONS.md. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - **Resolver:** `VehicleView` runs an effect on `searchParams.selected` change. After `useVehicles().loading` flips false, it does `allData.find(v => v.id === selected)`: | ||
| - **Found:** opens slider via `setEditingItem(...)` and pages the table to the row's index in the currently-sorted+filtered dataset (`setPage(Math.floor(idx / rowsPerPage))`). If the row is filtered out, slider opens but pagination stays put. | ||
| - **Not found:** opens slider with a not-found body using existing translation key `vehicles.notFound`. `VehicleDetails` accepts `vehicle: VehicleRow | null`. | ||
| - **Writers:** `RowClickCell` and the `/vehicle-type` chips both `navigate('/vehicle?selected=...')` (push). The `Close` button in `VehicleDetails` calls `setSearchParams({}, { replace: true })`. |
| * Optional hook fired by {@link GenericDataViewPage} after data is loaded | ||
| * and table-logic has produced `dataForTable`. Lets a page reconcile URL | ||
| * state (e.g. `?selected=` deep links) with editor state and pagination | ||
| * without lifting that wiring into the generic page itself. See |
Self-review pass on PR #67. Iter 1 ships only data-independent no-auth tests (`.app-content` visible); /vehicle's deep-link slider, chip filter, pagination jump, and edit-chip behaviour are only verifiable end-to-end. None of that is reachable without a stub for the `vehicleTypes` GraphQL query. `autosys-helpers.ts` already wires this pattern for the /vehicle-type tests but its fixture has 3 vehicles with `transportMode: null` — too lean for pagination (need 10+ rows) and chip-filter (need known + unknown modes). Add a parallel fixture rather than mutate the existing one, since /vehicle-type specs depend on the leaner shape. - `e2e-tests/fixtures/vehicles-list-mock.json` — 3 VehicleTypes with TransportMode variety (`rail`, `bus`, `someUnknownNetexValue` to exercise the `toTransportMode → 'unknown'` boundary), 15 total vehicles (12 + 2 + 1), deterministic ids (`NMR:Vehicle:rail-11` lands on page 2 at `rowsPerPage=10`). - `e2e-tests/no-auth/vehicle-list-helpers.ts` — exports `interceptVehicleListQuery(page)` mirroring the inline pattern in `autosys-helpers.ts` (match `postData.query` substring, fulfill matching requests, continue() everything else). No spec changes yet — commit 2 wires this into a new vehicle.spec.ts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Iter 1 self-review on PR #67 — pin the integration contracts that live behind `useUrlEffect` so future indirection refactors and iter-2 Save wiring have a regression net. Six cases in `e2e-tests/no-auth/vehicle.spec.ts`, exercising the no-auth path with the GraphQL stub from the previous commit: 1. list renders 15 rows, default sort = registrationNumber asc, page 0 contains 2 bus + 8 rail chips (localised via `transportMode.*`). 2. clicking the search filter button + 'Rail' checkbox narrows the total to 12 and hides BUS rows. 3. row action navigates to `/vehicle?selected=NMR:Vehicle:bus-1` and opens the sidebar. 4. **Edit chip toggles to TextFields, TransportMode stays localised.** Asserts `getByLabel('Transport Mode').toHaveValue('Bus')` — **this case fails today** (received `'bus'`, the raw NeTEx enum, instead of the localised `'Bus'` shown in view mode). The next commit routes view + edit modes through one display helper to turn this green. 5. clicking Close drops `?selected=` from the URL and collapses the sidebar. 6. deep-linking `/vehicle?selected=NMR:Vehicle:rail-9` auto-paginates to page 2 (`pagination-displayed-rows` contains '11') and opens the sidebar — exercises the `useVehicleUrlSelection` page-jump path and the load-guard that prevents committing `vehicle=null` while `loading: true`. Tests are independent (each `goto`s fresh) — dropped the `mode: 'serial'` configure so a failure in any case doesn't mask the others. The no-auth project still uses `workers: 1` for shared dev-server safety, so ordering is preserved. Also adds `data-testid="search-filter-button"` on `SearchFilterControl`'s IconButton so the chip-filter popover is addressable without depending on MUI internals. Pure addition; no behaviour change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Edit mode wrote `vehicle[key]` straight into the TextField, so for
`parentTransportMode` the field showed the raw NeTEx enum (`'bus'`,
`'rail'`, `'unknown'`) instead of the localised label that view mode
rendered via `transportModeLabelKey + t()`. Toggling the Edit chip
flipped 'Bus' → 'bus' — visible inconsistency.
Rename `fieldValue` → `fieldDisplay` and parametrise the missing-value
fallback so both branches share one display path:
- view mode: `fieldDisplay(key, '—')` — em-dash for empties (unchanged).
- edit mode: `fieldDisplay(key, '')` — empty input for empties.
TransportMode mapping lives in the helper, so the two modes now agree
by construction. Turns the failing case in `vehicle.spec.ts` green
(`getByLabel('Transport Mode').toHaveValue('Bus')`).
No save-path involvement — fields stay `readOnly` per iter 1.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Iter 1 self-review on PR #67. `fetchVehicles` is the only place that delegates to `fetchVehicleTypes`, flattens `vehicleType.vehicles[]`, and routes `vt.transportMode` through `toTransportMode()` — each load-bearing for the /vehicle list, the TransportMode chip filter (#24), and the no-duplicated-GraphQL claim in the PR description. Yet the function had zero direct coverage. Add `fetchVehicles.test.ts` — pure vitest, mocks `../vehicle-types/fetchVehicleTypes.ts` via `vi.mock`. Mirrors the template of `vehicleSortValue.test.ts` (table-driven via a small `mkVT` factory). Four cases: 1. empty `vehicleTypes` → `[]`. 2. VehicleType with `vehicles: undefined` → zero rows (pins the `?? []` fallback). 3. parent context carries through — `id`, `name.value`, `transportMode` land on every flattened row. 4. unrecognised parent `transportMode` collapses to `'unknown'` via `toTransportMode` (pins the boundary mapping at the call site — complements `transportMode.test.ts` which pins it at the helper). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Iter 1 self-review on PR #67. The null/undefined/empty boundary cases the v1 plan called out for `toTransportMode` were already covered (lines 16-20 of the existing test). The genuine gap is `transportModeFilters` — exported but completely untested, despite the JSDoc stating a deliberate UX contract that `'unknown'` is omitted from the chip filter set (rows with `'unknown'` simply don't match any chip). Add a `transportModeFilters` describe block with two cases: 1. `'unknown'` is not in the filter set (pins the JSDoc'd omission — prevents accidental re-inclusion if someone treats the union and the filter list as the same data). 2. Every known `TransportMode` has a filter entry, and the list length matches — regression net for new NeTEx modes added to the union but forgotten in the filter array. The known-modes array here is the source of truth; if `TransportMode` grows it must be updated, which is exactly the prompt we want a reviewer to see. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
GunnarAtEgde
left a comment
There was a problem hiding this comment.
Fix the routes before merge please.
Other than that:
The
| path="/vehicle-type" | ||
| element={<ProtectedRoute element={<VehicleTypeView />} />} | ||
| /> | ||
| <Route path="/vehicle" element={<ProtectedRoute element={<VehicleView />} />} /> |
There was a problem hiding this comment.
Use plural in routes. It should be /vehicles.
Same goes for the other ones, /vehicle-types and /deck-plans
Then you can have routes like /vehicles/:id.
This is common best practice in route naming
There was a problem hiding this comment.
will fix on the new wider PR,
closing this since newer branch has a lot more on gql + xml
Addresses Gunnar's review on PR #67 — REST URL convention is plural for collections (`/vehicles/:id` reads naturally, `/vehicle/:id` reads as a typo). Renamed across: - src/App.tsx route definitions (/vehicles, /vehicles/new, /vehicle-types, /vehicle-types/new, /vehicle-types/:id, /deck-plans, /deck-plans/:id). - Navigation callsites: components/Menu, NewVehicleFab, VehicleCreatePage, AutosysImportFloatingMenu, deckPlanViewConfig, DeckPlanDetailsView, vehicleTypeViewConfig, vehicleUrlParams.vehicleSelectedHref, Home menu paths, vehicle-type quick-create link. - JSDoc, comments, theme/createThemeFromConfig notes. - e2e specs: every page.goto in auth + no-auth suites. Verified: tsc -b clean, npm run lint 0 errors, npm test 127/127 green, prettier clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
vehicles: /vehicles list+sidebar details (cumulative, supersedes #67)
Summary
/vehicleroute as a conventional Inanna feature dir atsrc/data/vehicles/— list + sidebar details, replacing thepackages/vehicle-details/PoC. Mirrorsvehicle-types/.TransportModeunion + chip filters atsrc/data/netex/transportMode.ts, consumed by/vehiclefrom day one (partial delivery of All entity lists: wire TransportMode chip filter #24)./vehicle?selected=<id>reconciles withEditingContext, sidebar visibility, and pagination (auto-jumps to the page containing the row). Chips on/vehicle-typerows and the per-row icon on/vehicleboth navigate to the same URL contract.Closes iteration 1 of #26. Iter 2 (Save semantics, XML PUT mapping, optimistic update) is tracked on the same issue.
What's in here
src/data/netex/transportMode.ts—TransportModeunion with'unknown'as a first-class member, totaltoTransportMode()boundary mapping,isTransportModeguard,transportModeLabelKey,transportModeSortValue,transportModeFilters. Test file with 8 cases.src/data/vehicles/—vehicleTypes.ts(flatVehicleRowview model),fetchVehicles.ts(thin flatten over../vehicle-types/fetchVehicleTypes— no duplicated GQL),useVehicles.ts,vehicleSortValue.ts(+ test),vehicleViewConfig.tsx,VehicleView.tsx,VehicleDetails.tsx(readonly + Edit chip toggle, no Save button — gated on layout approval).cells/RowClickCell.tsx,cells/TransportModeChip.tsx.useVehicleUrlSelection.tsx— new optionaluseUrlEffectViewConfig field onGenericDataViewPage; stable no-op fallback preserves hook order for pages that don't opt in. Sidebar Close drops?selected=symmetrically.VehicleListCell.tsx(under vehicle-types): chip click navigates to/vehicle?selected=<id>.transportMode.*+ 12×vehicles.*(en + nb).FORK_DECISIONS.md— new ADR-style log; first entry archives the deep-link rationale. Companion toOPEN_QUESTIONS.md.Out of scope
PUT /netex/vehicles/{id}wiring — iter 2 of vehicle list+form #26.vehicles(...)/vehicle(id:)GQL on Sobek — Hathor relies on the flatten until those land.vehicleTypeViewConfig/deckPlanViewConfigto the sharedtransportModeFilters— rest of All entity lists: wire TransportMode chip filter #24.Test plan
npm run lintcleannpm run checkclean (Prettier)npm run buildclean (tsc -b + Vite)npm run test— newtransportMode.test.ts(8 cases) andvehicleSortValue.test.ts(8 cases) pass/vehiclerenders, TransportMode chip filters narrow rows, row click opens sidebar, Edit chip toggles to form view, Close drops?selected=from URL/vehicle-typerow deep-links to/vehicle?selected=<id>and auto-paginates to the correct page🤖 Generated with Claude Code
Self-review pass (commits
8f81e15…e379e8a)Three parallel Explore agents reviewed the diff after open. Most flagged items were intentional iter-1 trade-offs (closure capture for stable edit-mode state), matched existing conventions (no AbortController, hardcoded column
headerLabel), or pre-existed onmain('Show less'in VehicleListCell). Five commits land the surviving items, TDD-shaped:8f81e15e2e: add /vehicle GraphQL stub helper — fixture +interceptVehicleListQuery(mirrorsinterceptVehicleTypesQuery) so /vehicle is reachable without a backend.a3f8641e2e: cover /vehicle list, sidebar, deep-link, chip filter — six cases invehicle.spec.ts, including a failing assertion that toggling Edit keeps the TransportMode label localised ('Bus', not raw'bus').8655ad5vehicles: route view+edit display through one helper — collapses two display paths inVehicleDetails.tsxto a singlefieldDisplay(key, missing). Turns the failing case froma3f8641green.6653afbvehicles: cover fetchVehicles flatten + enrichment — 4 vitest cases mocking the upstreamfetchVehicleTypes. Pins the no-duplicated-GraphQL claim, the'unknown'collapse, and the empty/missing-array boundary.e379e8anetex: pin transportModeFilters contract — two cases pin the JSDoc'd "'unknown'is intentionally omitted" UX rule and a regression net for newTransportModeunion members forgotten in the filter list.After the pass: lint clean, prettier clean,
tsc -bclean, 100 vitest cases pass (was 96), 18 no-auth e2e cases pass on chromium (was 12).Deferred to follow-up issues (verified, not regressions)
(viewConfig.useUrlEffect ?? noopUrlEffect)({...})indirection inGenericDataViewPage— works in practice; cleaner refactor belongs in its own issue.vehicle.spec.tscase 6 (deep-link pagination jump) pins the contract end-to-end.EditorComponentclosure-capturesrow— intentional iter-1 trade-off per JSDoc; iter 2's Save flow will refactor.useVehicleslacksAbortControllerand?filter=refetch — both matchuseVehicleTypes; cross-feature sweep, not iter-1.allDataindex — edge case; needsuseDataViewTableLogicto expose pre-pagination filtered data, separate API change.headerLabel+ pagetitleinvehicleViewConfig— matchesvehicleTypeViewConfig/deckPlanViewConfig; cross-feature i18n sweep.'Show less'/'+N more'inVehicleListCell— pre-existing onmain, not part of this PR.