egullickson/motovaultpro
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: coerce numeric/decimal columns in repository mappers (#241) #242

Merged
egullickson merged 1 commits from issue-241-numeric-mapper-coercion into main 2026-05-16 02:35:03 +00:00
Conversation 0 Commits 1 Files Changed 5 +10 -12
egullickson commented 2026-05-16 02:06:52 +00:00
Owner
Copy Link

Fixes #241

Summary

node-postgres returns numeric/decimal columns as JavaScript strings, but the TypeScript interfaces for MaintenanceRecord and OwnershipCost declare those fields as number. The mappers were passing the raw row through, so the runtime types didn't match the declared types — forcing every consumer to defensively call Number() or silently break.

Observable symptoms this fixes

  • Maintenance cost column on the vehicle summary screen was empty (worked around in PR #240 with a frontend coercion that this PR removes).
  • OwnershipCostsList was calling value.toLocaleString({ minimumFractionDigits: 2, maximumFractionDigits: 2 }) on a string; strings have a toLocaleString method that ignores the options object, so ownership-costs amounts displayed without proper currency formatting (e.g. "100.5" instead of "100.50").

Changes

Backend

  • backend/src/features/maintenance/data/maintenance.repository.ts — mapMaintenanceRecord coerces row.cost with Number() when non-null, returns undefined otherwise (matches the optional type cost?: number).
  • backend/src/features/ownership-costs/data/ownership-costs.repository.ts — mapRow coerces row.amount with Number() (amount: number is NOT NULL in the DB).

Pattern matches the existing vehicles.repository.ts convention which already coerces purchasePrice, insuranceCost, and registrationCost.

Frontend (remove now-redundant workarounds)

  • frontend/src/features/maintenance/components/MaintenanceRecordsList.tsx — drop Number(record.cost).toFixed(2) and Number(record.odometerReading).toLocaleString(); the values are now real numbers. Tightened the truthiness check to != null so a legitimate 0 still renders.
  • frontend/src/features/vehicles/pages/VehicleDetailPage.tsx and VehicleDetailMobile.tsx — revert the PR #240 cost coercion back to the simple typeof rec.cost === 'number' guard. The backend now honors the type contract, so the workaround is unnecessary.

Scope decisions / what is NOT in this PR

The audit (recorded in the issue) confirmed several other repositories with the same bug — stations.repository.ts, community-stations.repository.ts price/rating fields, and the fuel-logs.repository.ts enhanced methods that return raw rows without a mapper. Those are not addressed here because:

  • They have unclear downstream consumers and need their own investigation.
  • Fixing them in the same PR risks subtle UI breaks (e.g., if a station price is currently rendered as a string somewhere that doesn't tolerate .toFixed()).
  • Each warrants its own focused PR.

Follow-up issues filed: see issue tracker (linked in #241 comment).

Test plan

  • npm run type-check (backend): clean.
  • npm run type-check (frontend): clean.
  • npm run lint (backend, scoped to edited files): 0 errors; 14 baseline any warnings unchanged.
  • npm run lint (frontend, scoped to edited files): 0 errors; 3 baseline any warnings unchanged.
  • npx jest --testPathPattern='(maintenance|ownership-costs)' (backend): 16 passed; 1 pre-existing compile error in maintenance.integration.test.ts unchanged from main.
  • npx jest --testPathPattern='(maintenance|vehicles|ownership-costs)' (frontend): 23 passed.
  • Manual on staging desktop and mobile (320px, 768px, 1920px): visit https://staging.motovaultpro.com/garage/vehicles/01caf9e8-2b9a-4f24-959f-67a3e6bd91b1 and confirm:
    • Maintenance cost shows as a properly formatted dollar amount.
    • Ownership costs show with thousands separators and 2 decimal places (e.g. $1,234.50).
    • Records list filter chips still display cost correctly.

Acceptance criteria (from #241)

  • mapMaintenanceRecord coerces all numeric/decimal fields to number.
  • OwnershipCost.amount coerced.
  • Other repository mappers audited — done; remaining fixes deferred to follow-up issues (see Scope decisions above).
  • Defensive Number() coercions in maintenance frontend components removed.
  • Linting, type-check, and tests at parity with main.
  • Mobile + desktop verification.
Fixes #241 ## Summary `node-postgres` returns `numeric`/`decimal` columns as JavaScript strings, but the TypeScript interfaces for `MaintenanceRecord` and `OwnershipCost` declare those fields as `number`. The mappers were passing the raw row through, so the runtime types didn't match the declared types — forcing every consumer to defensively call `Number()` or silently break. ### Observable symptoms this fixes - Maintenance cost column on the vehicle summary screen was empty (worked around in PR #240 with a frontend coercion that this PR removes). - `OwnershipCostsList` was calling `value.toLocaleString({ minimumFractionDigits: 2, maximumFractionDigits: 2 })` on a string; strings have a `toLocaleString` method that ignores the options object, so ownership-costs amounts displayed without proper currency formatting (e.g. `"100.5"` instead of `"100.50"`). ## Changes ### Backend - `backend/src/features/maintenance/data/maintenance.repository.ts` — `mapMaintenanceRecord` coerces `row.cost` with `Number()` when non-null, returns `undefined` otherwise (matches the optional type `cost?: number`). - `backend/src/features/ownership-costs/data/ownership-costs.repository.ts` — `mapRow` coerces `row.amount` with `Number()` (`amount: number` is NOT NULL in the DB). Pattern matches the existing `vehicles.repository.ts` convention which already coerces `purchasePrice`, `insuranceCost`, and `registrationCost`. ### Frontend (remove now-redundant workarounds) - `frontend/src/features/maintenance/components/MaintenanceRecordsList.tsx` — drop `Number(record.cost).toFixed(2)` and `Number(record.odometerReading).toLocaleString()`; the values are now real numbers. Tightened the truthiness check to `!= null` so a legitimate `0` still renders. - `frontend/src/features/vehicles/pages/VehicleDetailPage.tsx` and `VehicleDetailMobile.tsx` — revert the PR #240 cost coercion back to the simple `typeof rec.cost === 'number'` guard. The backend now honors the type contract, so the workaround is unnecessary. ## Scope decisions / what is NOT in this PR The audit (recorded in the issue) confirmed several other repositories with the same bug — `stations.repository.ts`, `community-stations.repository.ts` price/rating fields, and the `fuel-logs.repository.ts` enhanced methods that return raw rows without a mapper. Those are **not** addressed here because: - They have unclear downstream consumers and need their own investigation. - Fixing them in the same PR risks subtle UI breaks (e.g., if a station price is currently rendered as a string somewhere that doesn't tolerate `.toFixed()`). - Each warrants its own focused PR. Follow-up issues filed: see issue tracker (linked in #241 comment). ## Test plan - [x] `npm run type-check` (backend): clean. - [x] `npm run type-check` (frontend): clean. - [x] `npm run lint` (backend, scoped to edited files): 0 errors; 14 baseline `any` warnings unchanged. - [x] `npm run lint` (frontend, scoped to edited files): 0 errors; 3 baseline `any` warnings unchanged. - [x] `npx jest --testPathPattern='(maintenance|ownership-costs)'` (backend): 16 passed; 1 pre-existing compile error in `maintenance.integration.test.ts` unchanged from `main`. - [x] `npx jest --testPathPattern='(maintenance|vehicles|ownership-costs)'` (frontend): 23 passed. - [ ] Manual on staging desktop and mobile (320px, 768px, 1920px): visit `https://staging.motovaultpro.com/garage/vehicles/01caf9e8-2b9a-4f24-959f-67a3e6bd91b1` and confirm: - Maintenance cost shows as a properly formatted dollar amount. - Ownership costs show with thousands separators and 2 decimal places (e.g. `$1,234.50`). - Records list filter chips still display cost correctly. ## Acceptance criteria (from #241) - [x] `mapMaintenanceRecord` coerces all `numeric`/`decimal` fields to `number`. - [x] `OwnershipCost.amount` coerced. - [ ] Other repository mappers audited — done; remaining fixes deferred to follow-up issues (see Scope decisions above). - [x] Defensive `Number()` coercions in maintenance frontend components removed. - [x] Linting, type-check, and tests at parity with `main`. - [ ] Mobile + desktop verification.
egullickson added 1 commit 2026-05-16 02:06:52 +00:00
fix: coerce numeric/decimal columns in repository mappers (refs #241)
All checks were successful
Deploy to Staging / Build Images (pull_request) Successful in 1m49s
Details
Deploy to Staging / Deploy to Staging (pull_request) Successful in 43s
Details
Deploy to Staging / Verify Staging (pull_request) Successful in 4s
Details
Deploy to Staging / Notify Staging Ready (pull_request) Successful in 3s
Details
Deploy to Staging / Notify Staging Failure (pull_request) Has been skipped
Details
fdc34aee2f 
node-postgres returns numeric/decimal columns as JavaScript strings,
but the TypeScript interfaces for MaintenanceRecord and OwnershipCost
declare numeric fields as number. The mappers were passing values
through raw, breaking type-safe arithmetic and display (e.g. the
amount column on the vehicle summary screen was empty until the
recent frontend workaround in PR #240, and OwnershipCostsList silently
no-ops toLocaleString on the string).

Backend
- mapMaintenanceRecord: coerce cost via Number() when non-null.
- ownership-costs mapRow: coerce amount via Number().

Frontend (remove now-redundant workarounds)
- MaintenanceRecordsList: drop Number() coercion on cost and
  odometerReading; use the number values directly.
- VehicleDetailPage / VehicleDetailMobile: revert the PR #240 cost
  coercion to the simple typeof number guard now that the backend
  honors the type.

Scope notes
- Other repositories with the same pattern (stations, community-stations,
  fuel-logs enhanced methods) are tracked separately because they have
  unclear downstream consumers and warrant their own investigation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
egullickson merged commit 0dd5746f60 into main 2026-05-16 02:35:03 +00:00
egullickson deleted branch issue-241-numeric-mapper-coercion 2026-05-16 02:35:03 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
status
backlog
status/blocked
status
done
status
in-progress
status
ready
status
review
type
bug
type
chore
type
docs
type
feature
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
egullickson
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: egullickson/motovaultpro#242
Delete Branch "issue-241-numeric-mapper-coercion"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?