Skip to content

vehicles: /vehicles list+sidebar details (cumulative, supersedes #67)#74

Merged
dynnamitt merged 54 commits into
mainfrom
phase2/issue72-vehicles-gql-cutover
May 18, 2026
Merged

vehicles: /vehicles list+sidebar details (cumulative, supersedes #67)#74
dynnamitt merged 54 commits into
mainfrom
phase2/issue72-vehicles-gql-cutover

Conversation

@dynnamitt
Copy link
Copy Markdown
Contributor

Summary

Cumulative replacement for the iter-1 PR #67. Carries the /vehicle list + sidebar details all the way to a working save round-trip against Sobek's real GraphQL + NeTEx XML endpoints.

  • List side cut over to Sobek's vehicles(...) GraphQL query (no more flatten over vehicle-types). Projection split into src/data/vehicles/projection/ with TransportMode chip filter + OperationalNumber column + whole-row click.
  • Edit side now hydrates from GET /services/vehicles/netex/vehicles/{id} (NeTEx PublicationDelivery) via a new parseVehicleXml that mirrors the existing vehicleToXmlShape write path — read and write are inverse mappers over the same xml/Vehicle.ts domain shape.
  • Save flows through useVehiclePairSavebuildPublicationDeliveryXml → import, with a positive SaveSuccessSnackbar (4s timeout) confirming success. The sidebar stays open after save so the toast gets its full timeout.
  • Editor chrome — NetexId chip with copy + 3D gradient + size scale; EditorRail (collapse · edit → cancel/save); CSS-grid form with FieldRow.

Closes / refs

What's in here

Projection (list side, #72):

  • src/data/vehicles/projection/vehicleGqlShaped.ts, fetchVehicles.ts (Sobek vehicles() GQL, nested transportType), useVehicles.ts, vehicleSortValue.ts, vehicleViewConfig.tsx, useVehicleUrlSelection.tsx.
  • src/graphql/vehicles/queries/fetchVehicles.ts — paged GQL request.
  • src/data/netex/restructNetexId.ts — bridge for nested transportType.id (DB row id → reconstructed NeTEx id; sunsets when sobek#125 ships).
  • OperationalNumber column, whole-row click (no dedicated action column), TransportMode chip filter, pagination + sort.

Form/details (xml side, #26/#69/#73):

  • src/data/vehicles/xml/ — generated Vehicle.ts + VehicleModel.ts (via netex-ts-gen), Vehicle-mapping.ts + VehicleModel-mapping.ts (write-side vehicleToXmlShape), buildVehiclePairXml.ts, parseVehicleImportResponse.ts (+ test), useVehiclePairSave.ts, and the new parseVehicleXml.ts (+ test) + fetchVehicleNetexXml.ts.
  • src/data/vehicles/useVehicle.ts{data, loading, error, refetch}, fetch+parse pipeline.
  • src/data/vehicles/VehicleDetails.tsx, VehicleEditForm.tsx, vehicleFormDefaults.ts — readonly head with NetexId chip, CSS-grid label/input form, EditorRail with collapse/edit/cancel/save, dirty-form beforeunload guard.
  • MISSING_MODEL placeholder — Sobek's streamOneVehicle emits only the Vehicle, not its Model; the form's model slot shows Manufacturer: [missing] until Sobek adds a single-Model export or sobek#126's name field unblocks a richer projection.

Chrome / shared:

  • src/components/sidebar/EditorRail.tsx — collapse + edit → cancel/save segments; right-side flip on list pages.
  • src/data/netex/NetexId.tsx — copy button, 3D gradient, size scale.
  • src/data/netex/publicationDeliveryXml.ts — builder replacing wrapInPublicationDelivery; tested via publicationDeliveryXml.test.ts.
  • src/data/netex/transportMode.tsTransportMode union with 'unknown', toTransportMode() boundary mapping, transportModeFilters.

i18n: transportMode.*, vehicles.* (incl. vehicles.loading, vehicles.saveSuccess), vehicles.field.* in en + nb.

Docs: FORK_DECISIONS.md — ADR-style log seeded with the deep-link rationale and the projection/ + xml/ split decision.

Out of scope (follow-ups)

Test plan

  • npm run lint clean (0 errors; pre-existing react-refresh / exhaustive-deps warnings on unrelated files only).
  • npm run check clean (Prettier).
  • npm run build clean (tsc -b + Vite).
  • npm test — 127 vitest cases pass across 17 files, including the new parseVehicleXml.test.ts (seed-fixture, CompositeFrame, empty RF, multi-Name, round-trip via vehicleToXmlShape).
  • Manual (npm run local against Sobek :37999):
    • /vehicle list renders; TransportMode chip filters narrow rows; OperationalNumber column populated; whole-row click opens sidebar.
    • Sidebar opens with header (TransportType chip + Mode) immediately from the list row; form area shows a brief spinner then populates Name / Description / BuildDate / RegistrationDate / Chassis / etc. from the NeTEx fetch (fields the GQL projection never carried).
    • Network tab: one GET /services/vehicles/netex/vehicles/{id} per slider open.
    • Edit Name → Save → green snackbar, slider stays open, dirty indicator clears, mode reverts to view; close + reopen → name persists.
    • Force a 404 (mangle the URL) → form area shows error string, header strip still rendered.
    • Unknown deep-link /vehicle?selected=does-not-exist shows the existing not-found body.
    • vehicles.saveSuccess shows in nb when locale is switched ("Kjøretøy lagret").

🤖 Generated with Claude Code

dynnamitt and others added 30 commits May 12, 2026 10:31
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>
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>
…onDelivery (#69)

Type-blind, slot-based envelope builder in src/data/netex/. Caller passes
each slot's items pre-shaped via the matching *ToXmlShape(...) function;
builder composes the PublicationDelivery → ResourceFrame envelope as one
object tree and emits via a single XMLBuilder.build pass.

Migrates VehicleTypeDetails save path to the new helper and deletes the
legacy string-template wrapInPublicationDelivery. Envelope defaults
(namespace, version, RF:1, Europe/Oslo) are carried forward verbatim so
the VehicleType save output stays compatible.

Vitest covers slot composition, sibling emission, omitted-empty-slot
behaviour, and envelope defaults.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
#69)

Produced with `netex-ts-gen --schema base.schema.json --dest-dir
src/data/vehicles --collapse-refs --collapse-collections Vehicle VehicleModel`.

Post-generation hand-patches required to satisfy strict tsc + lint:
- rename unused `reshapeComplex` Reshape params in leaf *ToXmlShape
  helpers to `_reshapeComplex` (TS6133 under noUnusedParameters);
- delete unused `childWrapped` helpers from both *-mapping.ts;
- eslint override for `src/data/vehicles/Vehicle*.ts` patterns: turn
  off `@typescript-eslint/no-unused-vars` and `no-explicit-any` since
  both fire on machine-generated emission shapes.

Also: standardise `argsIgnorePattern: '^_'` for the unused-vars rule
globally — common convention for intentionally-unused names.

Upstream fix in netex-typescript-model is the proper resolution; the
hand-patch will need re-applying on every regen until then.

The Vehicle interface exposes `VehicleModelRef?: Ref<'VehicleModel'>`,
the 1-to-1 link the new form populates with `INLINE:VehicleModel:1`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New shared VehicleEditForm component (`src/data/vehicles/`) takes
`{ value, onChange, mode }` and renders Vehicle fields + the
VehicleModel-unique fields (Manufacturer / Range / FullCharge) in one
column. Mode toggles between view (disabled inputs) and edit. Form
holds typed `{ vehicle: Vehicle, model: VehicleModel }` state per
issue #69. Overlap fields (Name, Description, TransportTypeRef,
BrandingRef, ValidBetween) live on Vehicle only; VehicleModel emits
only its unique fields on save.

New VehicleCreatePage hosts the form via GenericDetailsPage chrome at
`/vehicle/new` with `model.$id = 'INLINE:VehicleModel:1'` and
`vehicle.VehicleModelRef = 'INLINE:VehicleModel:1'` preset; server
collapses the inline model on import.

New NewVehicleFab mounts via `GenericDataViewPage`'s `floatingAction`
slot on `/vehicle` and navigates to `/vehicle/new`. Same pattern as
AutosysImportFloatingMenu on `/vehicle-type`.

Save handler is stubbed (throws) in this commit. Commit 5 wires
buildPublicationDeliveryXml + importAsNetexToBackend, after-save
navigation, toast on error, and useBlocker nav guard for dirty form.

NOTE comment in VehicleCreatePage points at issue #68 (pending
`modification="new|revise"` client policy decision).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sidebar swaps its inline 5-field display for the shared VehicleEditForm
introduced in 9808ea0, hydrating `{ vehicle, model }` from the partial
VehicleRow. Only 4 fields persist from the list row today ($id, $version,
RegistrationNumber, TransportTypeRef = parent VehicleType ref); the rest
start blank until the deferred /netex/vehicles/{id} read lands.

VehicleRow-only enrichment (id, version, parent VehicleType name, localised
transport mode) moves into a read-only context panel above the form so
the underlying Vehicle entity (which doesn't carry those fields) doesn't
have to.

VehicleModel side always starts at `{ $id: 'INLINE:VehicleModel:1' }` per
issue #69 — server collapses the inlined model on import. (Edit-vs-create
semantics are uniform under this rule; an existing Vehicle's prior
VehicleModelRef is overwritten with the INLINE placeholder, and server
handles identity.)

E2E: the existing 'Edit chip toggles to TextFields' test was tracking a
regression in the now-removed editable Transport Mode field. Replaced
with a positive test that pins the new contract: Transport Mode renders
localised in the read-only context panel regardless of mode.

Save is still stubbed; commit 5 adds the Save button and wires the
buildPublicationDeliveryXml + importAsNetexToBackend pipeline.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The desktop Sidebar wraps its content in a fixed-height Box with
`overflow: 'hidden'`. With the old 5-field inline display this was fine;
after commit 5fcfac2 swapped in the full VehicleEditForm (~12 inputs +
context panel + chrome), the bottom fields fell off the viewport with
no way to reach them.

Constrain the VehicleDetails outer Box to `height: 100%` and let it
scroll vertically (`overflowY: 'auto'`, `boxSizing: 'border-box'`). The
fix is local to VehicleDetails so it doesn't change the behaviour of
the other consumer of Sidebar (the nav drawer fallback in
SidebarContent → WorkAreaContent).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
End-to-end save for both Vehicle + VehicleModel flows:

- `buildVehiclePairXml(form)` composes the PublicationDelivery via
  `buildPublicationDeliveryXml`, embedding `vehicleToXmlShape(form.vehicle)`
  and `vehicleModelToXmlShape(form.model)` into their slots. One
  XMLBuilder.build over the full tree.
- `parseVehicleImportResponse(body)` extracts `Vehicle/@id` from a Sobek
  `/import` response in either CompositeFrame-wrapped or flat-ResourceFrame
  layouts. Returns null on empty/non-XML/no-Vehicle bodies. Six vitests.
- `useVehiclePairSave()` wraps build → POST → parse: takes the form, returns
  `{ newId: string | null }` on success, sets `error: string | null` on
  failure. Surfaces `saving` for button-disabled state.
- `useDirtyFormBlock(isDirty)` is a thin wrapper around
  `react-router-dom`'s `useBlocker` with a `window.confirm` step; blocks
  cross-pathname navigation only (intra-route `?selected=` toggles are
  allowed without a prompt).

Wiring:
- `VehicleCreatePage` calls `save(form)` then `navigate('/vehicle?selected=
  <newId>')` (D2a, issue #69). When the response doesn't echo a Vehicle id
  the fallback navigates to `/vehicle` (plain list) — the proper D3
  refetch+find-by-RegistrationNumber path will land once a refetch handle
  is available to the create page. MUI Snackbar shows save errors;
  `useDirtyFormBlock` guards nav when the form differs from blank.
- `VehicleDetails` (sidebar) adds an edit-mode-only Save button; success
  closes the sidebar (drops `?selected=`). Save is disabled when the form
  is pristine (dirty check via `JSON.stringify` against the latest
  hydration from `VehicleRow`). MUI Snackbar shows errors; dirty-form
  blocker covers cross-pathname nav. List refetch on edit success is
  deferred — `useVehicles().refetch` is not yet plumbed through
  `EditingContext` to the sidebar.

NOTE comment on each save handler points at issue #68 — the
`modification="new|revise"` client policy is unresolved; handler does not
set the attribute today.

TODO comment on create-flow navigation flags the future `?search=` filter
interaction: a deep-linked new row may be hidden behind an active filter
once filter UX lands (issue #24).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…outer (#69)

`react-router-dom`'s `useBlocker` invariant-throws when not under a data
router. Hathor still mounts the legacy `<BrowserRouter>` component, so
calling `useBlocker` in either the create page or the sidebar crashed
the render — surfacing as 8 failed sidebar e2e tests (chromium + firefox
× 4 specs) where the "Vehicle Details" heading was never rendered.

Rewrite the hook to use a `beforeunload` listener only when dirty.
Modern browsers ignore the custom message and show their own native
dialog; we just need to call preventDefault + set returnValue.

Limitation acknowledged in the JSDoc: in-app React Router navigation is
no longer guarded — only page unloads (tab close, refresh, URL bar). A
proper in-app guard requires migrating from `<BrowserRouter>` to
`createBrowserRouter` so `useBlocker` becomes usable. That's a router
migration, out of scope here.

Verified: e2e no-auth suite 36 passed (was 28 + 8 failed); 113 vitests
green; tsc -b clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit's diff is the /simplify cleanup pass (+93 / -243); the message
below documents the full phase-2 branch since this is the head commit and
is the most visible entry point when reviewing the PR.

## Goal

Single combined Vehicle + VehicleModel details form for Sobek's national
vehicle registry. Save POSTs both entities to /import as a NeTEx
ResourceFrame; the server collapses the 1-to-1 relationship.

## Branch commit ladder

8 commits, each independently reviewable:

  1. 2ce3e37 netex: introduce buildPublicationDeliveryXml, replace
              wrapInPublicationDelivery
     Type-blind, slot-based envelope builder at src/data/netex/.
     Callers pass slot values pre-shaped via the matching *ToXmlShape
     function. One XMLBuilder.build per emission. Replaces the legacy
     string-template wrapInPublicationDelivery. VehicleType save migrated.
     7 vitests cover slot composition, sibling emission, omitted-empty-slot
     behaviour, and envelope defaults.

  2. 3e3e079 vehicles: generate Vehicle + VehicleModel NeTEx types via
              netex-ts-gen
     Produced via:
       netex-ts-gen --schema base.schema.json
                    --dest-dir src/data/vehicles
                    --collapse-refs --collapse-collections
                    Vehicle VehicleModel
     Post-gen hand-patches needed: _ prefix on unused Reshape params
     (TS6133 under tsconfig.app.json's noUnusedParameters); delete unused
     childWrapped helpers; eslint override on src/data/vehicles/Vehicle*.ts
     for the generator's intrinsic 'Extensions?: any' emission. Upstream
     fix in entur/netex-typescript-model tracked separately.

  3. 9808ea0 vehicles: form shell + /vehicle/new create route
     New shared VehicleEditForm (12 TextField rows; view/edit mode),
     VehicleCreatePage hosted on /vehicle/new, NewVehicleFab on /vehicle.
     Save stubbed.

  4. 5fcfac2 vehicles: sidebar VehicleDetails adopts shared VehicleEditForm
     Sidebar swaps its inline 5-field display for VehicleEditForm.
     Hydrates from VehicleRow (4 fields persist; rest start blank).
     Read-only context panel above the form for VehicleRow-only enrichment
     (id, version, parent VehicleType name, localised TransportMode).
     E2E: 1 regression test rewritten (TransportMode is no longer editable;
     test now pins the read-only context contract).

  5. 7a910ed vehicles: sidebar scrolls when VehicleDetails form overflows
     Outer Box constrained to height: 100% + overflowY: auto. The desktop
     Sidebar shell hides overflow; the old 5-field form fit, the 12-field
     form doesn't.

  6. a189a99 vehicles: wire save → /import for create + sidebar edit flows
     End-to-end save:
       buildVehiclePairXml(form)
         → buildPublicationDeliveryXml({
             vehicles:      [vehicleToXmlShape(form.vehicle)],
             vehicleModels: [vehicleModelToXmlShape(form.model)],
           })
         → importAsNetexToBackend(applicationImportBaseUrl, xml, token)
         → parseVehicleImportResponse(body)
         → navigate('/vehicle?selected=<newId>')  (create)  or
           closeSlider()                          (edit)
     New: useVehiclePairSave hook, useDirtyFormBlock hook, parse helper,
     6 vitests for parseVehicleImportResponse.

  7. ff4a73a vehicles: useDirtyFormBlock falls back to beforeunload under
              BrowserRouter
     react-router-dom's useBlocker requires a data router; the app uses
     the legacy <BrowserRouter>. The invariant-throw crashed the sidebar
     on render — caught by 8 e2e failures. Reverted to a beforeunload
     listener; in-app nav guard requires the router migration.

  8. f32256b vehicles: simplify — dedup constants, extract Snackbar,
              tighten save contract  ←  this commit
     Code-review pass: extracted INLINE_MODEL_ID / BLANK_FORM /
     hydrateFromRow to vehicleFormDefaults.ts; extracted <SaveErrorSnackbar>
     (was byte-identical in 2 hosts); useVehiclePairSave.save() no longer
     throws but returns { newId, error } (removes the latent double
     error display + try/catch ceremony); parseVehicleImportResponse
     reuses the existing xmlParser singleton; stripped narrative /
     issue-number / commit-number comments. +93 / -243 net.

## Key design decisions (locked in the /grill-me session)

  - Object-level XML builder; one XMLBuilder.build per emission.
  - Form scope: both create and edit; partial hydration accepted on edit
    (full hydration awaits /netex/vehicles/{id}).
  - State shape: { vehicle: Vehicle, model: VehicleModel } — typed
    netex-ts-gen objects, no flat-union splitter.
  - VehicleModel emits only its unique fields (Manufacturer, Range,
    FullCharge, CustomerServiceContactDetails). Overlap fields (Name,
    Description, TransportTypeRef, BrandingRef, ValidBetween) live on
    Vehicle only.
  - VehicleModel.\$id = constant 'INLINE:VehicleModel:1'. Vehicle's
    VehicleModelRef always points at that constant; the server collapses
    the inline model under the 1-to-1 relationship.
  - Routing: edit in sidebar /vehicle?selected=<id>; create at
    /vehicle/new full-page; shared form component for both.
  - Envelope helper is type-blind; callers pass slot values pre-shaped
    via *ToXmlShape. Adding a new entity type doesn't change the builder.
  - Envelope defaults (namespace, version, RF:1, Europe/Oslo TimeZone)
    carried forward verbatim from the legacy wrapInPublicationDelivery
    to preserve output compatibility on the VehicleType save path.
  - After-save: navigate to /vehicle?selected=<newId>. Parse Vehicle/@id
    from the response body; fall back to /vehicle plain when parse
    returns null.

## Vehicle name collision (resolved by directory scope)

Three Vehicle-named types coexist; none clash because no file imports
more than one:

  - src/data/vehicles/Vehicle.ts
        Full NeTEx entity, netex-ts-gen output. Used by the form,
        mappings, and save path.

  - src/data/vehicle-types/vehicleTypeTypes.ts::Vehicle
        GraphQL wire projection (id, registrationNumber, version)
        returned as VehicleType.vehicles[]. Used by the VehicleType
        details page's nested list.

  - src/data/vehicles/vehicleTypes.ts::VehicleRow
        Flat enriched list-row used by /vehicle list code (9 files).

VehicleRow is still load-bearing today:

  - The GraphQL list still returns the flat projection. Sobek's
    vehicles(filter) GraphQL pushdown (GH #24) hasn't landed; until
    it does, list code cannot fetch full Vehicle entities.

  - hydrateFromRow(row: VehicleRow): VehicleEditFormValue bridges
    list-row → form state with a 4-field mapping (id, version,
    registrationNumber, parentVehicleTypeId → TransportTypeRef). The
    remaining ~10 Vehicle fields start blank in edit mode until
    /netex/vehicles/{id} lands.

  - The sidebar's read-only context panel reads VehicleRow-only
    enrichment fields (parentVehicleTypeName, parentTransportMode)
    that aren't on the Vehicle entity itself.

VehicleRow retires when Sobek exposes a per-id Vehicle read endpoint
that returns the full NeTEx entity. Out of scope for this branch.

## Open follow-ups (not blockers for this PR)

  - #68 — modification=\"new|revise\" client policy. NOTE comment in
    useVehiclePairSave; the handler omits the attribute today.

  - #24 — Sobek vehicles(filter) GraphQL pushdown. When list filtering
    lands, ?search= filter UX may hide the deep-linked new row after
    create save; TODO comment in VehicleCreatePage flags the
    interaction.

  - useBlocker upgrade — requires <BrowserRouter> → createBrowserRouter
    migration. Today's useDirtyFormBlock is beforeunload-only;
    documented in the hook's JSDoc.

  - List refetch after sidebar-edit save — useVehicles().refetch is
    not yet plumbed through EditingContext to the sidebar editor
    closure; today the list shows cached data until the user
    navigates away.

  - /netex/vehicles/{id} read endpoint — unblocks full edit hydration
    (memory: project_vehicle_details_route_hathor).

  - netex-ts-gen post-gen hand-patches (Reshape param naming, dead
    childWrapped emission, Extensions?: any) — file upstream issue on
    entur/netex-typescript-model so future regens don't reintroduce
    them.

## Verification

  - tsc -b clean
  - vitest 113 / 113 green (13 new across two test files)
  - e2e no-auth 36 passed, 2 skipped
  - lint 10 warnings (all pre-existing)

## Files changed across the branch

  21 files, +1,449 / -98 from base (fix/issue26-list-form).
  Generated NeTEx files: 4 (Vehicle.ts, Vehicle-mapping.ts,
  VehicleModel.ts, VehicleModel-mapping.ts — ~630 LOC).
  Hand-written code: ~890 LOC across 17 files.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Inanna's initial scaffold shipped two near-twin list-chrome pages.
The Edit variant (inline-cell-edit mode) has never been imported in
this fork's git history and forced bubbled-up tunables (e.g. the
initial details-pane width) to live in both files.

Delete src/pages/GenericDataEditPage.tsx and record the decision in
FORK_DECISIONS.md. The surviving GenericDataViewPage carries the
DETAILS_PANE_INIT_WIDTH const (1/4 of viewport on first paint).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sidebar/ToggleButton/useResizableSidebar gain an optional
side: 'left' | 'right' (default 'left'). GenericDataViewPage pins
it to 'right' via a bubbled-up DETAILS_PANE_SIDE const and computes
the content-area margin key from it.

Mechanism lifted from theme/extends commit 9b1028e, with theme.sidebarSide
reads replaced by hardcoded props. We deliberately skip the theme key
augmentation, settings selector, and other layout knobs from that branch —
only the side-flip is wanted here.

The resizer math anchors to the right edge (window.innerWidth - clientX)
when side='right', so dragging the handle inward narrows the pane.
ToggleButton's border-radius and chevrons mirror the side. Mobile
Drawer anchors to the chosen side too.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Share `Side = 'left' | 'right'` from Sidebar.tsx; drop the inline
  union duplicated across 4 files.
- Bubble up more tunables:
  - GenericDataViewPage: DETAILS_PANE_WIDTH_FACTOR (0.25),
    DETAILS_PANE_GUTTER_PX (replaces inline `+ 4`), MARGIN_TRANSITION
    (narrowed from broad `margin` to the actually-animated side).
  - useResizableSidebar: MIN_W, MAX_W_RATIO (was inline 100 / 0.8).
- Fix module-load staleness: compute initWidth from
  window.innerWidth inside the component so a viewport resize
  before first list-page mount is honored.
- Extract closeIcon const in Sidebar to mirror ToggleButton's
  collapsedIcon/expandedIcon pattern.
- Drop the WHAT-narrating "Change is here: use editingItem" comment.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
VehicleEditForm originally stacked 12 floating-label TextFields,
burning ~56px per row and forcing the sidebar pane to scroll. Switch
to a label-on-left / input-on-right layout using a single CSS Grid
container with two columns (`minmax(8rem, 12rem) 1fr`) and a tiny
local <FieldRow id label alignTop?> helper that uses
`display: contents` so its <InputLabel> and <TextField> become direct
grid items of the parent.

Column alignment across all rows is guaranteed by the shared grid,
labels associate to inputs via htmlFor/id (native a11y), and
`size="small"` drops each row ~56 -> 40px vertical. Section dividers
and overline headers span both columns via `gridColumn: '1 / -1'`.
Responsive fallback: `xs: '1fr'` collapses to single-column on
narrow viewports.

Also in this pass:
  - Rename the second section header from "Vehicle Model" to "Model".
  - Drop the ShortName field (not authored separately in this UI).
  - Reorder Vehicle fields: Name -> Reg.no -> Op.no -> Chassis ->
    BuildDate -> Reg.date -> Description -> TransportTypeRef.
  - Description is now a regular single-row input (no multiline).

Document the decision in FORK_DECISIONS.md with the alternatives
considered (Stack+Typography, Grid v2, FormControlLabel) and why each
was rejected.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add src/data/netex/NetexId.tsx — a small chip component for NeTEx ids
rendered as CODESPACE:TYPE:**VALUE** (bold VALUE, monospace, outlined),
with an optional nested vN chip for version. Missing or empty
CODESPACE/TYPE segments render as ???; the rightmost segment is always
treated as VALUE so partial ids like `abc-123` show as ???:???:abc-123
and colons inside VALUE are preserved.

VehicleDetails read-only head:
  - Replace the separate ID + Version ContextRow pair with a single
    <NetexId id={vehicle.id} version={vehicle.version} /> chip.
  - Vehicle Type row: render the parent name AND a <NetexId> chip of
    parentVehicleTypeId on the same row, so the canonical NeTEx ref is
    visible without an extra labelled row.
  - ContextRow rewritten with `display: contents` so its label and value
    are direct grid items of a wrapping CSS Grid (same column template
    as the edit form below). value prop widened to ReactNode for the
    Vehicle Type row.
  - Label typography aligned with the form's <InputLabel> styling
    (fontSize 0.875rem, color text.primary).

VehicleEditForm:
  - Export LABEL_COL_MIN / LABEL_COL_MAX / COL_GAP so the read-only head
    can share the same column template.
  - Remove the TransportTypeRef FieldRow — that data is now in the head.
  - Date labels drop the "(YYYY-MM-DD)" suffix; the format is shown as
    ghost placeholder text inside the input via DATE_PLACEHOLDER.

Document the new NetexId component in FORK_DECISIONS.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
VehicleDetails header was a static "Vehicle Details" h6 with the
NetexId chip sitting in the context block below. It now reads as a
live mirror of the form's Vehicle Name field — typing in the Name
input updates the title in place — with the NetexId chip flowing
on the same line, right of the title text. The Edit/View chip stays
at the far right; the title gets noWrap + minWidth: 0 + flex: 1
so long names truncate via ellipsis without pushing the chip out.

Blank-name fallback: when the trimmed Name is empty, the title
renders `[ unnamed ]` with the brackets in the default h6 weight
and only the word `unnamed` styled as a placeholder (italic,
text.disabled, fontWeight 400, slight letter-spacing). Bubbled-up
const: BLANK_NAME = 'unnamed'.

The read-only context block below loses its outer Stack wrapper
and the NetexId entry — it's now just the two-row CSS Grid
(Vehicle Type, Transport Mode) with data-testid="vehicle-context"
moved onto the grid Box itself.

Extract `firstText(arr)` to src/data/netex/multilingualString.ts —
the helper reads NeTEx MultilingualString[] shape and is now used
by both VehicleEditForm and VehicleDetails. Sharing it via the
edit-form export tripped react-refresh/only-export-components;
a dedicated module is the right home for a netex shape helper.

E2E: five `getByRole('heading', { name: 'Vehicle Details' })`
assertions swapped to `getByTestId('vehicle-details-title')`,
since the heading text now varies with the form's Name field.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Vertical rail anchored at the sidebar's content-facing edge. View mode
shows collapse + pen; edit mode shows collapse + cancel + save. Cancel
reverts the form to the hydrated row and returns to view mode, confirming
via the existing discard dialog when dirty. Replaces the standalone
ToggleButton + per-feature Edit chip + Save + Close trio in
VehicleDetails. GenericDataViewPage exposes --sidebar-width and
--app-header-height so the rail can position itself with position:fixed.
e2e selector updated to editor-rail-collapse.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

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>
PR #74's plural rename made the bug visible: the "vehicles" column on
/vehicle-types builds chips that deep-link to /vehicles/:id, and the
nested vehicles[].id from the vehicleTypes(...) GraphQL query is the
bare DB row id (e.g. "42") instead of the full NeTEx id — same default-
fetcher fallback as transportType.id (sobek#125, secondary mention).
The slider on the destination page can't match the bare id against the
fully-formed ids in allData and renders the not-found body.

Workaround mirrors the established transportType.id one in
fetchVehicles.ts: project the nested vehicles[] in fetchVehicleTypes.ts
through restructNetexId(parent.id, 'Vehicle', v.id). The pass-through
guard means call sites stay correct after the Sobek fix lands.

- fetchVehicleTypes.ts: project nested vehicles[].id via restructNetexId.
- restructNetexId.ts: JSDoc enumerates both affected nested fields.
- restructNetexId.test.ts: reverse-direction case (VehicleType → Vehicle).

Refs sobek#125 (confirmed parallel via comment).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@dynnamitt dynnamitt changed the title vehicles: /vehicle list + sidebar details (cumulative, supersedes #67) vehicles: /vehicles list+sidebar details (cumulative, supersedes #67) May 16, 2026
@dynnamitt dynnamitt force-pushed the phase2/issue72-vehicles-gql-cutover branch from 4028e71 to b38438d Compare May 16, 2026 19:37
…to form grid

- NetexId chips revert to MUI's plain outlined Chip surface; the
  CHIP_GRADIENT_LIGHT/DARK + CHIP_SHADOW_LIGHT/DARK constants and
  theme-mode branching go away. Outlined variant is still the default
  so the chip stays distinct from inline text.
- VehicleDetails.tsx — both callsites bumped to size="small". Title
  header now lays out as a 2-col grid (Name | NetexId) matching the
  context strip + EditForm grid template, so all three blocks share
  the same column edges. Name truncates with ellipsis + native title
  tooltip when wider than the label column.

VehicleDetails is the only NetexId consumer in the tree today, so the
gradient went away wholesale rather than behind a flag.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@dynnamitt dynnamitt force-pushed the phase2/issue72-vehicles-gql-cutover branch from b38438d to bd0585b Compare May 16, 2026 19:42
@dynnamitt dynnamitt requested a review from Copilot May 17, 2026 09:11
@dynnamitt
Copy link
Copy Markdown
Contributor Author

dynnamitt commented May 17, 2026

Code review — PR #74

5,308-line diff. Cumulative replacement for the iter-1 PR #67. Carries /vehicles from list → NeTEx-XML hydrated sidebar → save round-trip against real Sobek.

Overview

The PR is well-structured with strong ADR documentation (FORK_DECISIONS.md), thoughtful directory split (projection/xml/), and solid test coverage on the projection + parse paths. The main risks cluster around the useVehicle lifecycle, the MISSING_MODEL placeholder leaking into save payloads, and a parser/builder asymmetry on aliased Ref keys.

BLOCKER

B1. useVehicle infinite spinner when applicationImportBaseUrl is missing. Fixed in 6fd6197. Was: useState(!!id) starts loading=true, the early-return guard if (!applicationImportBaseUrl || !id) return; never flipped it back — sidebar spinner permanent. Now the guard is split: !id stays benign; missing baseUrl sets error + loading=false synchronously, mirroring useVehiclePairSave's message. Regression test at e2e-tests/no-auth/vehicle-loading-bug.spec.ts (config fixture omits the field; self-contained GQL intercept bypasses the broken vehicleTypes-keyed helper).

B2. MISSING_MODEL placeholder is written to Sobek on save. vehicleFormDefaults.ts:22-25 + buildVehiclePairXml.ts:6-11. MISSING_MODEL carries Manufacturer: [{value: '[missing]'}]. buildVehiclePairXml unconditionally serialises both form.vehicle and form.model into the PublicationDelivery payload. As soon as a user dirties any vehicle field and saves, the [missing] Manufacturer goes to Sobek's import. The PR notes describe the placeholder as cosmetic — but the save path doesn't strip it. Either guard the model slot in buildVehiclePairXml (skip when model.$id === INLINE_MODEL_ID), or block save while the model is the placeholder, or skip the vehicleModels slot at the import builder. This is real data corruption risk.

Comm: Acceptable! ; its a speculative TEMP test on backend

MAJOR

M1. parseVehicleXml / Vehicle-mapping aren't round-trip symmetric on aliased Refs. Fixed in 2face28. Was: parser iterated REF_KEYS and read raw[key] under the XML element name (AuthorityRef, CarModelProfileRef) — round-trip dropped the domain→alias mapping. Now: xml/Vehicle-parser.ts keeps a REF_XML_TO_DOMAIN: RefMap keyed by XML element name (AuthorityRef → TransportOrganisationRef, CarModelProfileRef → VehicleModelProfileRef, identity for the rest). TDD red→green: Vehicle-parser.test.ts (renamed from parseVehicleXml.test.ts) gained an explicit round-trip alias test asserting both keys survive encode→decode. Same pattern mirrored to VehicleModel-parser.{ts,test.ts}. Shared helpers extracted into src/data/netex/{parser,mapping}-helpers.ts so both Vehicle and VehicleModel sides go through the same alias-aware code path. Mirror-pair smell tracked separately on entur/netex-typescript-model#49 — codec v2 collapses each parser+mapping pair into one bidirectional Codec<T> and deletes the helpers.

M2. useDirtyFormBlock arms beforeunload on a freshly-loaded vehicle. Fixed in 0a2a4ee. Was: dirty tracking split across three non-atomic state slices (useState(form) + useMemo(initialFormKey) + useEffect(setForm)) — the memo and the effect both fired on xmlVehicle change but committed on different frames, so for one render form !== initialFormKey even with no user edit, arming beforeunload on a fresh page. Now: pure reducer vehicleFormState.tshydrate advances form AND the hydratedKey baseline in a single returned state, so divergence is by construction only ever the result of a user edit action. VehicleDetails switched to useReducer(formReducer, initialFormState); cancel-edit dispatches hydrate instead of mirroring the hydrate logic inline. TDD red→green: 7 cases in vehicleFormState.test.ts pin the M2 invariant plus edit/revert/cancel cycles (no jsdom needed — pure node TypeScript). Companion FORK_DECISIONS update (0fb4ff2) codifies that the form's VehicleEditFormValue carries NeTEx shapes (xml/Vehicle + xml/VehicleModel) — the GQL projection is read-side only — which is what makes the round-trip honest.

M3. useVehicles.refetch doesn't return the promise. Fixed in 3621a98. Was: doFetch was async but the body kicked off fetchVehicles(...).then().catch().finally() and returned — the async resolved while the chain ran detached, so await refetch() couldn't observe new data. Now: extracted projection/fetchVehiclesAndApply.ts as a pure async orchestration (await getAccessToken → await fetchVehicles → setData | setError), and useVehicles.doFetch is a thin try { await fetchVehiclesAndApply(...) } finally { setLoading(false) }. The returned promise covers the full chain. Bonus extraction: translateVehiclesFetchError is now a pure exported function with its own test coverage. TDD red→green: 10 cases in fetchVehiclesAndApply.test.ts — the M3 invariant test uses a deferred fetch promise to assert await applyPromise resolves only after setData is called.

M4. List doesn't refetch after successful save. Fixed in 3621a98. Was: handleSave set savedAt and flipped out of edit mode but didn't refresh useVehicles — the /vehicles row stayed stale until manual reload, even after the green snackbar told the user "saved". Now: extracted commitSave(save, form, onSaved?) — a pure async helper that awaits onSaved on success (skips on error) and returns the save result unchanged. Plumbed a refetch channel end-to-end: UseDataReturn.refetch? (viewConfigTypes.ts) → GenericDataViewPage destructures from useData()useUrlEffect params gain optional refetchuseVehicleUrlSelection forwards into the VehicleDetails closure → VehicleDetails.onSaved prop wires into commitSave. The success snackbar now appears only after the list refetch promise resolves, so the user always sees the fresh row, never the stale one. TDD red→green: 5 cases in commitSave.test.ts — the M4 invariant test uses a deferred onSaved to assert the chain ordering and skip-on-error behaviour.

M5. parseVehicleXml un-trapped throws from fast-xml-parser. parseVehicleXml.ts:57-64. A malformed response (HTML error page from an auth-proxy, byte-truncation) can make parser.parse() throw or return a partial. The exception bubbles to useVehicle.doFetch.catch and surfaces as a raw XMLParserError string in the form area. Wrap parse in try/catch with a friendlier error.

M6. EditorRail discard-then-save unhappy path. EditorRail.tsx handleSaveFromDialog awaits onSave(), then setConfirming(null) regardless. On rejection the dialog closes, snackbar appears, form is still dirty — only path forward is to retry by hand. Either keep the dialog open on rejection, or surface the error inside the dialog.

MINOR

N1. useVehicleUrlSelection cleanup runs on every unmount. useVehicleUrlSelection.tsx:72-76 calls setEditingItem(null) on unmount unconditionally. In StrictMode dev, the mount → unmount → mount cycle clears editor state mid-cycle (benign — the second mount re-commits, since refs reset). Guard with if (lastCommittedIdRef.current) for clarity.

N2. restructNetexId silently degrades on missing donor. Returns the bare value when donor lacks :. Easier to debug downstream regressions if this logs a one-shot warn.

N3. NetexId clipboard write is unguarded. navigator.clipboard is undefined on insecure origins. The "Copied!" check icon may show even when nothing was written. Detect availability, disable button when missing.

N4. MISSING_MODEL text isn't i18n'd. vehicleFormDefaults.ts:5,24. Literal '[missing]' leaks into the form regardless of locale. Wrap at the render site if intended to be user-visible, or use a distinct sentinel and render via t().

N5. getVehicleSortValue exhaustiveness check returns never instead of throwing. Silent failure at runtime if a new key is added without updating the switch. throw new Error(...) is louder.

N6. parseVehicleImportResponse doesn't toArray on ResourceFrame. Sobek currently emits one, but fast-xml-parser arrays it when multiple are present. Mirror parseVehicleXml's toArray usage.

N7. e2e selector scoping. vehicle.spec.ts:46-52 table.getByText('Bus', {exact:true}) isn't scoped to tbody. Currently passes only because the header text is "Transport Mode" — brittle on header copy changes. Tighten to table.locator('tbody').

N8. useResizableSidebar initial width. GenericDataViewPage.tsx:45 reads window.innerWidth at render time; move into a lazy useState(() => ...) initialiser.

N9. EditorRail.tsx (226 lines) and NetexId.tsx (206 lines) are nearing the file-size smell threshold. Both have clean extraction candidates: DiscardDialog out of EditorRail, copy-button out of NetexId.

N10. The two *-mapping.ts files duplicate ~80 lines of helpers. Extract Obj, Reshape, attr, elem, child, mapArr, wrapArr, text, refAttr, strVal into a shared module — verbatim identical between Vehicle-mapping.ts and VehicleModel-mapping.ts.

N11. buildPublicationDeliveryXml slot order relies on object key insertion order. Explicit array of slot keys is safer and self-documenting.

N12. useVehicles.handleRequestSort not memoised. New function each render → DataTableHeader re-renders. Wrap in useCallback.

N13. FORK_DECISIONS rationale for the z-index bump. Claims the bump lifts the rail "with the sidebar's stacking context" — but the rail is position: fixed, so it creates its own viewport-anchored stacking context. The fix is correct empirically (resizer-vs-sidebar paint order), but the explanation is misleading.

PRAISE

  • FORK_DECISIONS.md ADR style is exceptional — the "Alternatives considered" tables alone are worth the PR.
  • useVehicleUrlSelection's lastCommittedIdRef idempotence guard + JSDoc explaining why (closure-remount footgun) is high-quality defensive code.
  • transportMode.ts: 'unknown' sentinel for total typing + regression test that catches forgotten additions is exactly the right shape.
  • fetchVehicles truncation canary (console.warn when content.length < totalElements) — cheap operational signal.
  • Removing GenericDataEditPage.tsx and ToggleButton.tsx with ADR rationale — hygiene most PRs skip.
  • NetexId.copy: 'show' | 'hide' | 'onHover' follows the tri-state-toggle preference established in earlier feedback.
  • E2E test selectors (editor-rail-collapse, vehicle-details-title, vehicle-context) are consistent and resilient.

Suggested gating

  • Must address before merge: B1 ✅, B2 (placeholder corrupting saved data — accepted as speculative TEMP test).
  • Address before next iter: M1 ✅, M2 ✅, M3 ✅, M4 ✅.
  • Defer with tracking issues: M5, M6, N1–N13.

🤖 Generated with Claude Code

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

dynnamitt and others added 2 commits May 17, 2026 12:59
…tuck spinner

Early-return in doFetch left loading=true forever when applicationImportBaseUrl
was undefined (initial useState(!!id) starts true; no setLoading(false) on
the guard path). Split the guard: !id stays benign; missing baseUrl sets
error + loading=false synchronously, mirroring useVehiclePairSave's message.

Adds e2e regression in vehicle-loading-bug.spec.ts using a baseUrl-less
config fixture; self-contained GraphQL intercept matches the new vehicles()
query directly (skips the broken vehicleTypes-keyed helper).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ehicle pair

- Parser fix (M1 from PR #74 review): Vehicle-mapping emits domain
  TransportOrganisationRef as <AuthorityRef> and VehicleModelProfileRef as
  <CarModelProfileRef>; parser now translates XML element name → domain key
  via a RefMap, so the round-trip preserves both. Adds round-trip alias
  cases to Vehicle-parser.test.ts and VehicleModel-parser.test.ts.

- Adds VehicleModel-parser.ts (+ tests) so the Model side has read parity
  with Vehicle whenever Sobek's streamOneVehicle starts shipping the Model.

- Renames parseVehicleXml.{ts,test.ts} → Vehicle-parser.{ts,test.ts} for
  parity with Vehicle-mapping.ts (serializer <-> parser pair).

- Extracts shared XML helpers into netex/ to drop ~140 lines of duplication
  between the Vehicle and VehicleModel mapping/parser files:
    netex/mapping-helpers.ts — attr/elem/refAttr/...
                               + validBetween/keyValue/privateCode/text shapes
                               + baseReshape dispatcher
    netex/parser-helpers.ts  — attr/scalar/ref/textArr/projectValidBetween/...

- Promotes generic NeTEx XML utils out of vehicle-imports/ into netex/:
    netex/xmlUtils.ts — ParsedXml, findResourceFrame, toArray, xmlParser
  vehicle-imports/xmlUtils.ts now keeps only import-specific orchestration
  (mergeResourceFrames, pubDeliverySingleRcFrame, extractVehicleTypeIds).
  Dependency direction is one-way now: netex/ → consumed by vehicles/xml/
  and vehicle-imports/, never the reverse.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dynnamitt and others added 7 commits May 17, 2026 14:17
…lpers

VehicleDetails previously split dirty tracking across three non-atomic
state slices: `useState(form)`, `useMemo(initialFormKey from xmlVehicle)`,
and `useEffect(setForm(hydrate(xmlVehicle)))`. The memo and the effect
both fire on xmlVehicle change but commit on different frames, so for one
render `form !== initialFormKey` even though no user edit has happened —
arming `beforeunload` on a freshly-loaded vehicle.

Extract a pure `vehicleFormState` reducer so `hydrate` advances `form`
AND `hydratedKey` in a single returned state. Divergence between the two
is now the only signal for `isDirty`, and it can only originate from a
user `edit` action.

Refactor VehicleDetails to drive the form via `useReducer(formReducer,
initialFormState)`. Cancel-edit dispatches `hydrate` instead of mirroring
the hydrate logic inline.

Tests (TDD red→green): 7 cases pinning the M2 invariant and the
edit/revert/cancel cycles. No jsdom needed — the reducer is pure node
TypeScript.

Also park NOTE comments on netex/{parser,mapping}-helpers.ts pointing at
entur/netex-typescript-model#49: once the codegen tool emits
bidirectional `Codec<T>` per entity (v2), this hand-written mirror pair
is deleted and consumers swap to `fooCodec.encode/decode`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… edit/post

Codifies the boundary established by the projection/ + xml/ split:
- projection types (VehicleGQLShaped) power search, list, filter, sort.
- Detail / edit / POST flows must round-trip through xml/Vehicle + xml/VehicleModel
  via useVehicle → Vehicle-parser (hydrate) and Vehicle-mapping → /import (save).
- Using a projection row as a write source silently drops NeTEx fields and
  carries display-flattened data the import path doesn't recognise.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…n there

FORK_DECISIONS.md header now identifies the file explicitly as Architecture
Decision Records (ADR) and declares itself the single home for design
rationale. CLAUDE.md's pointer is tightened correspondingly: do not restate
rationale in CLAUDE.md (it describes what the patterns are; FORK_DECISIONS
describes why).

No content shuffled between the two files — a duplication audit didn't
find any actual repetition. The change is purely about making the
contract between the two documents explicit so future updates land in the
right place.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
M3 — useVehicles.doFetch wrapped `fetchVehicles(...).then(...).catch(...).finally(...)`
in an `async` function with no return — the async returned an
already-resolved promise while the chain ran detached. Callers awaiting
`refetch()` couldn't observe the new data.

Extract `fetchVehiclesAndApply` (pure async orchestration) so the await
covers the full chain. `useVehicles.doFetch` is now a thin
`try { await fetchVehiclesAndApply(...) } finally { setLoading(false) }`.
Bonus: `translateVehiclesFetchError` is now an exported pure function
with its own test coverage.

M4 — VehicleDetails.handleSave fired the success snackbar and flipped
out of edit mode without refreshing the list — the `/vehicles` row
stayed stale until manual reload after the save.

Extract `commitSave(save, form, onSaved?)` — pure async helper that
awaits onSaved on success (skips on error) and returns the save result.
Plumb a refetch channel end-to-end:

  UseDataReturn.refetch?      (viewConfigTypes)
    → GenericDataViewPage destructures from useData()
    → useUrlEffect params gain optional refetch
    → useVehicleUrlSelection forwards into VehicleDetails closure
    → VehicleDetails.onSaved prop is wired into commitSave

After a successful save, the success snackbar appears only once the list
refetch promise has resolved — so the user sees the refreshed row, not
a stale one.

TDD: red→green on both. 10 cases pin fetchVehiclesAndApply (M3 await
semantics + error translation), 5 cases pin commitSave (M4 chain
ordering + error skip).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sorting while the sidebar editor held a selection interacted with
useVehicleUrlSelection's URL→editor→setPage cascade, producing a
flicker-then-revert glitch on the column header. UX call: lock sort
on all sortable columns while a row is being edited rather than
chasing the cascade.

When GenericDataViewPage sees an active editingItem, it forwards
sortLocked=true to PageContentComponent. DataTableHeader then
wraps each sortable label in a MUI Tooltip carrying a
LockOutlinedIcon + i18n("data.table.sortLocked"), dims to opacity
0.55, sets cursor:not-allowed, and drops the onClick — the
active-column sort arrow stays visible so the user retains
ordering context.

Generic by design: every projected list (/vehicles, /vehicle-types,
/deck-plans, future ones) inherits the behaviour through
GenericDataViewPage; no per-view wiring.

E2E coverage in vehicle-registration-sort.spec.ts:
parameterized harness runSortStabilityTests({ locked }) emits one
test per sortable column. Sidebar-closed run asserts toggle works
and the order persists across a settle window (anti-flicker).
Sidebar-open run asserts the click is a no-op and the hover
surfaces the lock tooltip. 10/10 green on Chromium and Firefox.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drops the 16px CircularProgress + "Loading vehicle…" caption while
xmlVehicle hydrates from /netex/vehicles/{id}, in favour of a
layout-matching wave-animated skeleton: title row + NetexId pill,
two context rows, seven vehicle field rows, model section divider
and heading, three model rows. Reuses the same grid template
(LABEL_COL_MIN/MAX, COL_GAP, ROW_GAP) so the swap from skeleton
to real form is zero layout shift.

Screen-reader semantics preserved: skeleton container carries
role="status" + aria-label={t('vehicles.loading', 'Loading vehicle…')},
so the previously-visible caption text is now the announced label.

VehicleDetails xmlLoading branch short-circuits to skeleton +
EditorRail (collapse stays live during load); the bottom path
simplifies from a triple xmlLoading/xmlError/form switch to a
plain xmlError-or-form ternary.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Playwright runs have been red on this branch since the issue-#72
GQL cutover (21885b7, May 16) — pre-existing rot, not caused by the
sidebar/sort/visual commits above. Two stale assumptions, three
small fixes:

1. vehicle-list-helpers.ts matched the legacy `vehicleTypes` query
   token. Post-cutover the production query is `Vehicles { vehicles(...) }`
   — no `vehicleTypes` substring. The intercept fell through, the
   dev server has no backend, and the table never rendered. Matcher
   now anchors on `vehicles(` (field name, stable across operation
   renames).

2. vehicles-list-mock.json reshaped from the legacy nested
   `data.vehicleTypes.content[].vehicles[]` envelope to the flat
   `data.vehicles.content[]` shape with `transportType` inlined per
   row — matches `fetchVehicles.ts`'s wire contract. 15 rows
   preserved (12 rail + 2 bus + 1 unknown-mode).

3. Singular `/vehicle` URL regexes in vehicle.spec.ts:103 and
   vehicle-type-filter.spec.ts:121 pluralised to `/vehicles$` and
   `/vehicle-types$` to match the route rename (66de9d7).

Also: vehicle.spec.ts row-action test used `row.getByRole('button')`
to find a per-row action button that no longer exists — clicking now
goes via the whole-row `useVehicleRowClick`. Test clicks the row
directly. Comment paragraph in vehicle-registration-sort.spec.ts
about the shared helper being "still keyed on the legacy
vehicleTypes query" dropped — drift is fixed.

Validation: vehicle.spec.ts + vehicle-type-filter.spec.ts +
vehicle-registration-sort.spec.ts + vehicle-loading-bug.spec.ts now
44/44 pass across Chromium and Firefox.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@GunnarAtEgde GunnarAtEgde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This contains a LOT of AI generated code that is difficult to understand.
I say OK for now and will register some bugs that I have observed

@dynnamitt dynnamitt merged commit 533c7e9 into main May 18, 2026
9 checks passed
@dynnamitt dynnamitt deleted the phase2/issue72-vehicles-gql-cutover branch May 18, 2026 08:49
@dynnamitt
Copy link
Copy Markdown
Contributor Author

4 new issues added catching review "minors" left here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants