Skip to content

Fix driver route export import#213

Open
hir-al-14 wants to merge 4 commits into
benevolentbandwidth:mainfrom
hir-al-14:fix-driver-route-export-import
Open

Fix driver route export import#213
hir-al-14 wants to merge 4 commits into
benevolentbandwidth:mainfrom
hir-al-14:fix-driver-route-export-import

Conversation

@hir-al-14

Copy link
Copy Markdown
Contributor

Summary

  • Driver Assist can now import the exact JSON file exported from a single route on the Results page.
  • Existing Driver Assist upload formats still work.

Motivation

  • A route exported from Results should be usable by the driver without manual editing.
  • Before this change, Results exported a Route JSON shape, but Driver Assist only accepted session-save or raw session data JSON.

Changes

  • Frontend:

    • Added support for Results page route export JSON in the Driver Assist importer.
    • Maps exported route stops into Driver Assist stops:
      • sequence -> route order
      • addresseeName -> customer name
      • phoneNumber -> phone number
      • capacityUsed -> package count
      • note -> notes
      • lat / lng -> navigation coordinates
    • Updated /upload-route and /driver_assist to use the same Driver Assist route parser.
    • Kept existing session-save envelope and raw session data imports working.
    • Added regression coverage for importing a Results route export.
  • Backend:

    • No backend changes.

Validation

Frontend

  • npm.cmd --prefix app/ui run lint
  • npm.cmd --prefix app/ui run format:check
  • npm.cmd --prefix app/ui run typecheck
  • npm.cmd --prefix app/ui run test
  • npm.cmd --prefix app/ui run build
  • npm.cmd --prefix app/mobile run lint
  • npm.cmd --prefix app/mobile run typecheck

Backend

  • cmake --preset dev
  • .github/scripts/check-backend-static.sh build/dev
  • cmake --build --preset dev --parallel
  • ctest --preset dev --output-on-failure --no-tests=error -LE 'e2e|docker'

Risk

  • Low risk.
  • Change is limited to Driver Assist route import parsing.
  • Existing accepted upload formats are preserved.
  • Results export format is not changed.

Rollout and Recovery

  • Roll out normally with the frontend deploy.
  • If anything looks wrong, revert this PR. Existing session-save and raw session imports should return to the old behavior.

High-Signal PR Checklist

  • The summary states the user-visible or operational outcome, not just file names.
  • The motivation explains why this change is needed now.
  • The change list separates frontend, backend, infrastructure, and documentation work where applicable.
  • Validation includes exact commands run, relevant output, and any checks intentionally skipped.
  • Risks, migrations, feature flags, and rollback steps are called out when relevant.
  • Screenshots or request/response examples are included for UI and API behavior changes.

Related Issues

#199

  • Exporting a route from Results and uploading that file to Driver Assist populates the route.
  • Stops keep the correct order, address, recipient name, phone number, package count, notes, and coordinates.
  • Navigation uses exported lat / lng.
  • Existing session-save and raw session data uploads still work.
  • Invalid/unrecognized JSON still shows a validation error.
  • Regression test added for importing a Results route export.

@markboenigk markboenigk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Code Review

This PR adds loadDriverRouteFromText to accept both Results-page export JSON and the existing session save format, removes the inline upload UI from /driver_assist, and routes import errors back via sessionStorage. Clean frontend-only change — here are a few things worth addressing before merge.


Bug: capacityUsed: 0 silently rejects valid Results exports

app/ui/src/lib/driver-route/importSession.ts line 47:

capacityUsed: z.number().positive().optional(),

z.number().positive() excludes zero. If the Results page ever exports a stop with capacityUsed: 0 (e.g. a mandatory waypoint or pickup with nothing to drop off), resultsRouteSchema.safeParse() fails, the code falls through to session parsing which also fails, and the driver sees "This file is not a recognized route or session JSON file" — no indication of the actual problem.

Fix: change .positive().nonnegative(). Also revisit whether the ?? 1 default on line 183 should become ?? 0 (0-capacity stop vs. unspecified-capacity stop are different things).


Architecture: double validation makes the driver_assist error path dead code

upload-route/page.tsx now calls parseRouteUploadText(text) (result discarded) before storing to sessionStorage. Then driver_assist/page.tsx calls loadDriverRouteFromText again on the same bytes. Since both call the same function with the same input, anything that passes upload-route will pass driver_assist — the catch / sessionStorage.setItem(ROUTE_UPLOAD_ERROR_KEY, ...) / router.replace block in driver_assist is unreachable through the normal UI flow.

Worth picking one owner: pre-validate at upload and simplify/remove the driver_assist catch, or drop the upload-time pre-validation and let driver_assist remain the sole error handler. Right now both live on and the dead path will silently rot.


Architecture: sessionStorage for cross-page error passback is fragile

Writing an error key in driver_assist and reading it back in upload-route's mount effect breaks silently if sessionStorage is unavailable (strict private-browsing settings, storage quota hit) or cleared mid-navigation. The Next.js convention for this pattern is a query param:

router.replace(`/upload-route?error=${encodeURIComponent(message)}`);

Survives storage resets, shows up in the address bar, and eliminates the ROUTE_UPLOAD_ERROR_KEY constant being imported from an upload-route internal module into driver_assist.


Cleanup: queueMicrotask wrapping setState in useEffect is unnecessary

React 18 automatically batches all synchronous state updates within an effect. The queueMicrotask calls on lines 74 and 100 of driver_assist/page.tsx add an extra microtask turn with no batching benefit — they just delay the loading → route transition by one tick and make the timing harder to follow. Calling the three setters directly is equivalent and clearer.


Cleanup: parseRouteUploadText is a pure pass-through

routeUploadValidation.ts exports one constant and one function that is literally return loadDriverRouteFromText(text). The module name implies validation responsibility that lives entirely in importSession.ts. Callers could import loadDriverRouteFromText directly; ROUTE_UPLOAD_ERROR_KEY could move to a shared constants file to remove the driver_assist → upload-route import dependency.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants