fix: coerce decimals in fuel-logs enhanced repository methods (#244) #245

egullickson · 2026-05-16T02:47:19Z

egullickson commented

2026-05-16 02:47:19 +00:00

Fixes #244

Summary

The enhanced API path on FuelLogsRepository returned raw pg rows straight to the service layer. node-postgres returns DECIMAL columns as strings, so fuel_units, cost_per_unit, total_cost, gallons, and price_per_gallon arrived as strings throughout the enhanced code path even though every TypeScript signature in the chain declared number. The service compensated with scattered Number() coercion in toEnhancedResponse and getVehicleStats, and EfficiencyCalculationService silently leaned on JS coercion in division — works today but fragile.

Pre-fix audit (full audit in #244 comment)

Critical findings before touching code:

7 repository methods return raw rows: createEnhanced, findByVehicleIdEnhanced, findByUserIdEnhanced, findByIdEnhanced, getPreviousLogByOdometer, getLatestLogForVehicle, updateEnhanced. The earlier issue body only named 3 — the audit caught the rest.
Service callers access fields as snake_case (r.fuel_units, r.total_cost) on the raw rows. A mapRow-style camelCase mapper would silently break every caller. The right shape is a mapper that coerces values but preserves snake_case keys.
No frontend code does string operations (.includes, .split) on these fields — only typeof === 'number' defensive checks and arithmetic. Coercing to numbers is safe across all consumers.
EfficiencyCalculationService.calculateEfficiency(distance, fuelUnits) does distance / fuelUnits with operand types declared number but actually receiving strings. Worked via JS coercion; will still work cleanly with numbers.

Changes

`backend/src/features/fuel-logs/data/fuel-logs.repository.ts`

Add private mapEnhancedRow(row) that coerces fuel_units, cost_per_unit, total_cost, gallons, price_per_gallon from string to number via parseFloat when non-null, preserving snake_case keys for the existing service-layer convention.
Apply it in all 7 enhanced read/write paths.

`backend/src/features/fuel-logs/domain/fuel-logs.service.ts`

getVehicleStats: drop Number(r.fuel_units) and Number(r.total_cost) wrappers in the reduces — the values are now real numbers.
toEnhancedResponse: replace Number(row.fuel_units), Number(row.cost_per_unit), Number(row.total_cost) with direct access plus ?? 0 to preserve the existing null-tolerance semantics (Number(null) === 0).

What I deliberately did NOT change

trip_distance and odometer are INTEGER columns; node-postgres already returns those as numbers. The Number(cur.trip_distance) and Number(cur.odometer) calls in getVehicleStats (lines 246-249) were always belt-and-suspenders no-ops. Removing them isn't part of this bug — left alone to limit blast radius.
Frontend defensive typeof === 'number' checks in FuelStatsCard, CostCalculator, and FuelLogForm are part of a broader convention (input fields, form parsing) and not tied to this specific bug. Left untouched.
Return type signatures on enhanced methods remain Promise<any>. Introducing a typed EnhancedFuelLogRow interface is a larger cleanup with its own review surface and belongs in a separate refactor.

Test plan

npm run type-check (backend): clean.
npm run lint (backend, scoped to edited files): 0 errors, 18 baseline any warnings unchanged.
npx jest --testPathPattern='fuel-logs': 2 suites failed with Configuration file not found at /app/config/production.yml — identical to baseline on main. Pre-existing config-loader path issue; not introduced by this PR.
Manual on staging desktop and mobile: create / view / edit a fuel log; verify totalCost, fuelUnits, costPerUnit, efficiency display as numbers and arithmetic in vehicle stats (/garage/vehicles/:id/stats if surfaced) is correct.

Acceptance criteria (from #244)

Audit every call site of the enhanced methods and consumers of their return values — done; recorded above.
Decide whether to extend mapRow or add a sibling mapEnhancedRow — added mapEnhancedRow to preserve snake_case convention.
Replace raw row returns in createEnhanced, findByVehicleIdEnhanced, updateEnhanced (and 4 more found in the audit) with the mapper.
Remove defensive Number() calls in consumers that become unnecessary.
Check whether internal arithmetic was relying on broken behavior — yes, JS auto-coercion was carrying it; now operating on real numbers. No silent miscalculations were observed in the audit (everything was either already wrapped in Number() or relied on / which auto-coerces).
Linting, type-check, and tests at parity with main.
Mobile + desktop verification.

Fixes #244 ## Summary The enhanced API path on `FuelLogsRepository` returned raw pg rows straight to the service layer. `node-postgres` returns `DECIMAL` columns as strings, so `fuel_units`, `cost_per_unit`, `total_cost`, `gallons`, and `price_per_gallon` arrived as strings throughout the enhanced code path even though every TypeScript signature in the chain declared `number`. The service compensated with scattered `Number()` coercion in `toEnhancedResponse` and `getVehicleStats`, and `EfficiencyCalculationService` silently leaned on JS coercion in division — works today but fragile. ## Pre-fix audit (full audit in #244 comment) Critical findings before touching code: - **7 repository methods** return raw rows: `createEnhanced`, `findByVehicleIdEnhanced`, `findByUserIdEnhanced`, `findByIdEnhanced`, `getPreviousLogByOdometer`, `getLatestLogForVehicle`, `updateEnhanced`. The earlier issue body only named 3 — the audit caught the rest. - **Service callers access fields as snake_case** (`r.fuel_units`, `r.total_cost`) on the raw rows. A `mapRow`-style camelCase mapper would silently break every caller. The right shape is a mapper that **coerces values but preserves snake_case keys**. - No frontend code does string operations (`.includes`, `.split`) on these fields — only `typeof === 'number'` defensive checks and arithmetic. Coercing to numbers is safe across all consumers. - `EfficiencyCalculationService.calculateEfficiency(distance, fuelUnits)` does `distance / fuelUnits` with operand types declared `number` but actually receiving strings. Worked via JS coercion; will still work cleanly with numbers. ## Changes ### `backend/src/features/fuel-logs/data/fuel-logs.repository.ts` - Add private `mapEnhancedRow(row)` that coerces `fuel_units`, `cost_per_unit`, `total_cost`, `gallons`, `price_per_gallon` from string to number via `parseFloat` when non-null, preserving snake_case keys for the existing service-layer convention. - Apply it in all 7 enhanced read/write paths. ### `backend/src/features/fuel-logs/domain/fuel-logs.service.ts` - `getVehicleStats`: drop `Number(r.fuel_units)` and `Number(r.total_cost)` wrappers in the reduces — the values are now real numbers. - `toEnhancedResponse`: replace `Number(row.fuel_units)`, `Number(row.cost_per_unit)`, `Number(row.total_cost)` with direct access plus `?? 0` to preserve the existing null-tolerance semantics (`Number(null) === 0`). ### What I deliberately did NOT change - `trip_distance` and `odometer` are `INTEGER` columns; node-postgres already returns those as numbers. The `Number(cur.trip_distance)` and `Number(cur.odometer)` calls in `getVehicleStats` (lines 246-249) were always belt-and-suspenders no-ops. Removing them isn't part of this bug — left alone to limit blast radius. - Frontend defensive `typeof === 'number'` checks in `FuelStatsCard`, `CostCalculator`, and `FuelLogForm` are part of a broader convention (input fields, form parsing) and not tied to this specific bug. Left untouched. - Return type signatures on enhanced methods remain `Promise<any>`. Introducing a typed `EnhancedFuelLogRow` interface is a larger cleanup with its own review surface and belongs in a separate refactor. ## Test plan - [x] `npm run type-check` (backend): clean. - [x] `npm run lint` (backend, scoped to edited files): 0 errors, 18 baseline `any` warnings unchanged. - [x] `npx jest --testPathPattern='fuel-logs'`: 2 suites failed with `Configuration file not found at /app/config/production.yml` — **identical to baseline on `main`**. Pre-existing config-loader path issue; not introduced by this PR. - [ ] Manual on staging desktop and mobile: create / view / edit a fuel log; verify `totalCost`, `fuelUnits`, `costPerUnit`, `efficiency` display as numbers and arithmetic in vehicle stats (`/garage/vehicles/:id/stats` if surfaced) is correct. ## Acceptance criteria (from #244) - [x] Audit every call site of the enhanced methods and consumers of their return values — done; recorded above. - [x] Decide whether to extend `mapRow` or add a sibling `mapEnhancedRow` — added `mapEnhancedRow` to preserve snake_case convention. - [x] Replace raw row returns in `createEnhanced`, `findByVehicleIdEnhanced`, `updateEnhanced` (and 4 more found in the audit) with the mapper. - [x] Remove defensive `Number()` calls in consumers that become unnecessary. - [x] Check whether internal arithmetic was relying on broken behavior — yes, JS auto-coercion was carrying it; now operating on real numbers. No silent miscalculations were observed in the audit (everything was either already wrapped in `Number()` or relied on `/` which auto-coerces). - [x] Linting, type-check, and tests at parity with `main`. - [ ] Mobile + desktop verification.

egullickson added 1 commit 2026-05-16 02:47:20 +00:00

fix: coerce decimals in fuel-logs enhanced repository methods (refs #244 )

Deploy to Staging / Build Images (pull_request) Successful in 35s

Details

Deploy to Staging / Deploy to Staging (pull_request) Successful in 43s

Details

Deploy to Staging / Verify Staging (pull_request) Successful in 3s

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

0d90829d31

The enhanced API path on FuelLogsRepository returned raw pg rows
straight to the service layer, so DECIMAL columns (fuel_units,
cost_per_unit, total_cost, gallons, price_per_gallon) arrived as
strings instead of numbers. The service layer compensated with
scattered Number() coercion in toEnhancedResponse and getVehicleStats,
and EfficiencyCalculationService silently leaned on JS string-to-
number coercion in division. Type signatures across the stack
declared number and the runtime delivered string.

Add a private mapEnhancedRow that coerces all DECIMAL columns with
parseFloat while preserving snake_case keys (the convention the
service layer already uses to access these rows). Apply it in every
enhanced read/write path: createEnhanced, findByVehicleIdEnhanced,
findByUserIdEnhanced, findByIdEnhanced, getPreviousLogByOdometer,
getLatestLogForVehicle, updateEnhanced. Drop the now-redundant
Number() wrappers in toEnhancedResponse and getVehicleStats.

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

egullickson merged commit 77aeba0102 into main

2026-05-16 02:58:52 +00:00

egullickson deleted branch issue-244-fuel-logs-enhanced-mapper

2026-05-16 02:58:52 +00:00

egullickson referenced this issue from a commit

2026-05-16 02:58:53 +00:00

Merge pull request 'fix: coerce decimals in fuel-logs enhanced repository methods (#244)' (#245) from issue-244-fuel-logs-enhanced-mapper into main

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: egullickson/motovaultpro#245