Currency improvements#12
Conversation
…nce forms, fetching currency settings from the database.
… database migrations, and add new database utility scripts.
…d update DB inspection to focus on `torque_specs`.
There was a problem hiding this comment.
Pull request overview
This PR updates currency handling end-to-end by making currency choices dynamic in forms, storing normalized (base-currency) cost fields in the database, and adjusting dashboard stats/migrations/scripts to support the change.
Changes:
- Pass
currenciesfrom loaders intoAddMotorcycleForm/MaintenanceForm, with a local fallback list when not provided. - Add
normalized_purchase_price/normalized_costcolumns and switch dashboard yearly maintenance totals to use normalized values. - Update Drizzle migration metadata and add helper scripts (backfill/verify/inspect), plus add
tsxfor running TS scripts.
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/verify-stats.ts | New script to compare computed dashboard totals vs a manual normalized sum. |
| scripts/manual-fix.ts | New one-off script to ALTER TABLE and add normalized_cost. |
| scripts/inspect-db.ts | New script to inspect SQLite table columns via PRAGMA. |
| scripts/fix-torque-specs.ts | New script to add missing tool_size column defensively. |
| scripts/backfill-costs.ts | New script to backfill normalized purchase price and maintenance cost fields. |
| package.json | Adds tsx dev dependency to run TS scripts more easily. |
| pnpm-lock.yaml | Lockfile updates for tsx and dependency graph changes. |
| app/utils/home-stats.ts | Attributes manual odo to last activity date; sums maintenance totals using normalizedCost. |
| app/routes/motorcycle.detail.tsx | Loads currencies, passes to dialogs/forms, and normalizes persisted purchase price + maintenance cost. |
| app/routes/home.tsx | Loads currencies for add form; normalizes purchase price on create. |
| app/db/schema.ts | Adds normalizedPurchasePrice and normalizedCost columns to Drizzle schema. |
| app/db/migrations/meta/_journal.json | Rewrites migration journal to a single new migration entry. |
| app/db/migrations/meta/0000_snapshot.json | Updates snapshot to reflect current schema (new columns/FKs/nullability). |
| app/db/migrations/meta/0001_snapshot.json | Deleted old snapshot. |
| app/db/migrations/meta/0002_snapshot.json | Deleted old snapshot. |
| app/db/migrations/0000_nice_sally_floyd.sql | New migration adding the normalized columns via ALTER TABLE. |
| app/db/migrations/0000_initial.sql | Deleted initial schema migration. |
| app/db/migrations/0001_add_torque_end_to_torque_specs.sql | Deleted older incremental migration. |
| app/db/migrations/0002_add_privacy_columns_to_documents.sql | Deleted older incremental migration. |
| app/db/migrations/0003_add_location_id_to_maintenance_records.sql | Deleted older incremental migration. |
| app/db/migrations/0004_drop_image_column.sql | Deleted older incremental migration. |
| app/db/migrations/0005_add_image_column_back.sql | Deleted older incremental migration. |
| app/db/migrations/0006_make_vin_nullable.sql | Deleted older incremental migration. |
| app/db/migrations/0007_add_tool_size_to_torque_specs.sql | Deleted older incremental migration. |
| app/components/maintenance-form.tsx | Accepts currencies prop; renders currency options dynamically with fallback. |
| app/components/maintenance-dialog.tsx | Wires currencies through to MaintenanceForm. |
| app/components/add-motorcycle-form.tsx | Accepts currencies prop; renders currency options dynamically with fallback. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,41 @@ | |||
| import { getDb } from "../app/db/index.ts"; | |||
| import { buildDashboardData } from "../app/utils/home-stats.ts"; | |||
| import { motorcycles, issues, maintenanceRecords, locationRecords, currencySettings } from "../app/db/schema.ts"; | |||
There was a problem hiding this comment.
import { motorcycles, issues, maintenanceRecords, locationRecords, currencySettings } ... is unused in this script, which will fail the repo's ESLint no-unused-vars check. Remove the unused schema imports (or actually use them) to keep lint passing.
| import { motorcycles, issues, maintenanceRecords, locationRecords, currencySettings } from "../app/db/schema.ts"; |
| for (const record of records) { | ||
| if (record.cost) { | ||
| const factor = getFactor(record.currency); | ||
| const normalizedCost = record.cost * factor; | ||
|
|
There was a problem hiding this comment.
Similarly, if (record.cost) will skip 0 and won’t backfill rows where cost is present but zero. Use an explicit null check (e.g., cost != null) to backfill consistently.
| for (const moto of motos) { | ||
| if (moto.purchasePrice) { | ||
| const factor = getFactor(moto.currencyCode); | ||
| const normalizedPrice = moto.purchasePrice * factor; | ||
|
|
There was a problem hiding this comment.
The backfill uses a truthy check (if (moto.purchasePrice)) which will skip valid 0 values and treats null differently than intended. Prefer an explicit null check (e.g., purchasePrice != null) so behavior aligns with the column’s nullable numeric type.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| if (entryYear !== year) { | ||
| return sum; | ||
| } | ||
| return sum + (entry.cost ?? 0); | ||
| return sum + (entry.normalizedCost ?? 0); | ||
| }, 0); |
There was a problem hiding this comment.
This reducer computes totals from normalizedCost, but it only guards on entry.cost == null. For legacy rows with cost set and normalizedCost null (pre-backfill), the total becomes 0. Either guard on normalizedCost or fall back to cost when normalizedCost is null to keep dashboards correct during rollout.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| viscosity: parseString(formData.get("viscosity")), | ||
| inspectionLocation: parseString(formData.get("inspectionLocation")), | ||
| locationId: locationId, | ||
| normalizedCost: (parseNumber(formData.get("cost")) || 0) * getCurrencyFactor(parseString(formData.get("currency"))), | ||
| }; |
There was a problem hiding this comment.
normalizedCost defaults to 0 when the cost field is empty/invalid, so rows can end up with normalized_cost = 0 while cost is NULL. Reuse the already-parsed cost value and only set normalizedCost when cost != null (otherwise store null).
| export default function MotorcycleDetail({ loaderData }: Route.ComponentProps) { | ||
| const { motorcycle, openIssues, maintenanceHistory, nextInspection, lastKnownOdo, insights, userLocations, currentLocationName } = loaderData; | ||
| // Cast loaderData to any because the generated type definition is stale and not including 'currencies' | ||
| const { motorcycle, openIssues, maintenanceHistory, nextInspection, lastKnownOdo, insights, userLocations, currentLocationName, currencies } = loaderData as any; |
There was a problem hiding this comment.
Casting loaderData to any to access currencies bypasses type safety and can mask real loader/component mismatches. Regenerate the route types (or adjust the Route.LoaderData type) so currencies is properly typed instead of using as any.
…zed cost fields to tests.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…dd motorcycle and maintenance forms.
…ts' into currency-improvements
Co-authored-by: mightea <3684954+mightea@users.noreply.github.com>
Co-authored-by: mightea <3684954+mightea@users.noreply.github.com>
…component type usage.
…/model display condition in the maintenance form.
This pull request introduces improvements to currency handling in forms and updates the database schema and migration files to support new fields and relationships. The main changes include making currency selection dynamic in forms, adding normalized cost fields to the database, and updating migration tracking metadata.
Form currency handling improvements:
AddMotorcycleFormandMaintenanceFormcomponents to accept acurrenciesprop and use a fallback list of default currencies if none are provided, making currency selection dynamic and extensible. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]Database schema and migration updates:
normalized_purchase_priceto themotorcyclestable andnormalized_costto themaintenance_recordstable to support storing costs in a normalized format. [1] [2]0000_snapshot.json) to reflect new fields, foreign keys, and changes such as making thevincolumn inmotorcyclesnullable and adding privacy columns todocuments. [1] [2] [3] [4] [5] [6] [7]Migration file cleanup:
Note: The initial schema SQL file was removed, indicating a shift to using migration files and snapshots for schema management.