+
{/* Top bar */}
diff --git a/src/data/defaults.js b/src/data/defaults.js
index ac10699..c993498 100644
--- a/src/data/defaults.js
+++ b/src/data/defaults.js
@@ -26,10 +26,10 @@ export const COINGECKO_TICKERS = {
};
export const DEFAULT_PLATFORMS = {
- cs: { name: "ComputerShare", feePerShare: 0.10, flatFee: 10 },
- gem: { name: "Gemini", feePercent: 0.015 },
- pp: { name: "Paypal", feePercent: 0.02 },
- fidelity: { name: "Fidelity", feePercent: 0 },
+ cs: { name: "ComputerShare", feePerShare: 0.10, flatFee: 10, feePercent: 0 },
+ gem: { name: "Gemini", feePerShare: 0, flatFee: 0, feePercent: 0.015 },
+ pp: { name: "Paypal", feePerShare: 0, flatFee: 0, feePercent: 0.02 },
+ fidelity: { name: "Fidelity", feePerShare: 0, flatFee: 0, feePercent: 0 },
};
export const DEFAULT_TAX_CONFIG = {
@@ -123,7 +123,7 @@ export const DEFAULT_READINESS = {
assetAppreciationRate: 0,
};
-export const SCHEMA_VERSION = 2;
+export const SCHEMA_VERSION = 1;
export function createDefaultState() {
return {
diff --git a/src/lib/__tests__/calculations.test.js b/src/lib/__tests__/calculations.test.js
index e8f0e8c..1cfbdb4 100644
--- a/src/lib/__tests__/calculations.test.js
+++ b/src/lib/__tests__/calculations.test.js
@@ -25,8 +25,9 @@ describe("isLongTerm", () => {
describe("calcFee", () => {
const platforms = {
- cs: { name: "ComputerShare", feePerShare: 0.10, flatFee: 10 },
- gem: { name: "Gemini", feePercent: 0.015 },
+ cs: { name: "ComputerShare", feePerShare: 0.10, flatFee: 10, feePercent: 0 },
+ gem: { name: "Gemini", feePerShare: 0, flatFee: 0, feePercent: 0.015 },
+ combo: { name: "Combo", feePerShare: 0.05, flatFee: 5, feePercent: 0.01 },
};
it("calculates per-share fee + flat fee", () => {
@@ -39,6 +40,12 @@ describe("calcFee", () => {
expect(calcFee(asset, 10000, platforms)).toBe(150);
});
+ it("calculates combined per-share + flat + percentage fee", () => {
+ const asset = { quantity: 200, feeType: "combo" };
+ // 200 * 0.05 + 5 + 0.01 * 8000 = 10 + 5 + 80 = 95
+ expect(calcFee(asset, 8000, platforms)).toBe(95);
+ });
+
it("returns 0 for feeType none", () => {
expect(calcFee({ feeType: "none" }, 5000, platforms)).toBe(0);
});
diff --git a/src/lib/__tests__/storage.test.js b/src/lib/__tests__/storage.test.js
index c9c725d..e0abc28 100644
--- a/src/lib/__tests__/storage.test.js
+++ b/src/lib/__tests__/storage.test.js
@@ -9,13 +9,6 @@ describe("migrateState", () => {
expect(migrateState(state)).toBe(state);
});
- it("migrates v0 state to current version preserving data", () => {
- const result = migrateState({ schemaVersion: 0, purchase: { category: "home" } });
- expect(result.schemaVersion).toBe(SCHEMA_VERSION);
- expect(result.purchase.category).toBe("home");
- expect(result.purchase.carMaintenanceAnnual).toBeNull();
- });
-
it("resets to defaults for empty/unrecognizable state", () => {
const result = migrateState({});
expect(result.schemaVersion).toBe(SCHEMA_VERSION);
@@ -27,30 +20,16 @@ describe("migrateState", () => {
expect(result.dateOfBirth).toEqual({ month: "", year: "" });
});
- it("v1 state: adds carMaintenanceAnnual: null to purchase", () => {
- const v1State = {
- schemaVersion: 1,
- assets: [],
- cashAccounts: [],
- purchase: { category: "home", homePrice: 350000 },
- };
- const result = migrateState(v1State);
+ it("resets unknown older versions to defaults", () => {
+ const result = migrateState({ schemaVersion: 0, assets: [], purchase: {} });
expect(result.schemaVersion).toBe(SCHEMA_VERSION);
- expect(result.purchase.carMaintenanceAnnual).toBeNull();
- expect(result.purchase.homePrice).toBe(350000);
+ expect(Array.isArray(result.assets)).toBe(true);
});
- it("v1 state: other fields preserved intact", () => {
- const v1State = {
- schemaVersion: 1,
- assets: [{ name: "GME", symbol: "GME", quantity: 10 }],
- cashAccounts: [{ name: "Checking", balance: 5000 }],
- purchase: { category: "vehicle", carPrice: 35000 },
- };
- const result = migrateState(v1State);
- expect(result.assets).toHaveLength(1);
- expect(result.assets[0].name).toBe("GME");
- expect(result.cashAccounts[0].balance).toBe(5000);
+ it("returns future-versioned state as-is", () => {
+ const future = { schemaVersion: SCHEMA_VERSION + 1, assets: [{ name: "X" }] };
+ const result = migrateState(future);
+ expect(result).toBe(future);
});
});
diff --git a/src/lib/calculations.js b/src/lib/calculations.js
index b75ce2f..49d0956 100644
--- a/src/lib/calculations.js
+++ b/src/lib/calculations.js
@@ -13,9 +13,7 @@ export function calcFee(asset, grossValue, platforms) {
if (ft === "none" || !ft) return 0;
const plat = platforms[ft];
if (!plat) return 0;
- if (plat.feePerShare != null) return asset.quantity * plat.feePerShare + (plat.flatFee || 0);
- if (plat.feePercent != null) return grossValue * plat.feePercent;
- return 0;
+ return (plat.feePerShare || 0) * asset.quantity + (plat.flatFee || 0) + (plat.feePercent || 0) * grossValue;
}
export function paychecksBefore(sellDate, cashFlowConfig) {
diff --git a/src/lib/storage.js b/src/lib/storage.js
index 6577e68..3237fdd 100644
--- a/src/lib/storage.js
+++ b/src/lib/storage.js
@@ -152,26 +152,12 @@ export function migrateState(state) {
// Already current — return as-is (preserves object reference)
if (state.schemaVersion === SCHEMA_VERSION) return state;
- let data = { ...state };
-
- // v1 → v2: add carMaintenanceAnnual to purchase
- if ((data.schemaVersion ?? 0) < 2) {
- if (data.purchase) {
- if (data.purchase.carMaintenanceAnnual === undefined) {
- data.purchase = { ...data.purchase, carMaintenanceAnnual: null };
- }
- }
- data.schemaVersion = 2;
+ // Future version — return as-is with warning
+ if (state.schemaVersion > SCHEMA_VERSION) {
+ console.warn(`[GreenLight] State has future schema v${state.schemaVersion} (current: ${SCHEMA_VERSION}). Returning as-is.`);
+ return state;
}
- // Safety net: if still not at current version after all migrations
- if (data.schemaVersion !== SCHEMA_VERSION) {
- if (data.schemaVersion > SCHEMA_VERSION) {
- console.warn(`[GreenLight] State has future schema v${data.schemaVersion} (current: ${SCHEMA_VERSION}). Returning as-is.`);
- return data;
- }
- console.error(`[GreenLight] Migration failed: expected v${SCHEMA_VERSION}, got v${data.schemaVersion}. Resetting.`);
- return createDefaultState();
- }
- return data;
+ // Unknown older version — reset
+ return createDefaultState();
}
diff --git a/src/pages/Assets.jsx b/src/pages/Assets.jsx
index 21154bc..4cc9408 100644
--- a/src/pages/Assets.jsx
+++ b/src/pages/Assets.jsx
@@ -185,7 +185,7 @@ export default function Assets({ state, updateState, prices }) {
{plat.name}:
- {plat.feePerShare != null ? `$${plat.feePerShare}/sh + $${plat.flatFee} bulk` : `${(plat.feePercent * 100).toFixed(1)}%`}
+ {[plat.feePerShare ? `$${plat.feePerShare}/sh` : null, plat.flatFee ? `$${plat.flatFee} flat` : null, plat.feePercent ? `${(plat.feePercent * 100).toFixed(1)}%` : null].filter(Boolean).join(" + ") || "Free"}
))}
diff --git a/src/pages/Settings.jsx b/src/pages/Settings.jsx
index 32010bb..62ca3b2 100644
--- a/src/pages/Settings.jsx
+++ b/src/pages/Settings.jsx
@@ -1,6 +1,6 @@
import { useState, useMemo } from "react";
import { colors, styles } from "../theme.js";
-import { createDefaultState, createSeededState } from "../data/defaults.js";
+import { createDefaultState } from "../data/defaults.js";
import { exportState, importState } from "../lib/storage.js";
import { getConsent, setConsent, track } from "../lib/analytics.js";
import { STATE_TAXES } from "../data/stateTaxes.js";
@@ -51,7 +51,7 @@ export default function Settings({ state, updateState, replaceState }) {
if (state.platforms[k]) { setPlatformKeyError(`Key "${k}" already exists.`); return; }
updateState(prev => ({
...prev,
- platforms: { ...prev.platforms, [k]: { name: newPlatformName.trim(), feePercent: 0 } },
+ platforms: { ...prev.platforms, [k]: { name: newPlatformName.trim(), feePerShare: 0, flatFee: 0, feePercent: 0 } },
}));
setNewPlatformKey("");
setNewPlatformName("");
@@ -174,11 +174,6 @@ export default function Settings({ state, updateState, replaceState }) {
}
};
- const handleSeed = () => {
- if (confirm("Replace all data with example data? This will overwrite current data.")) {
- replaceState(createSeededState());
- }
- };
const labelStyle = { fontSize: 13, color: colors.dim, textTransform: "uppercase", letterSpacing: 1, display: "block", marginBottom: 4 };
const inputStyle = styles.input;
@@ -351,34 +346,26 @@ export default function Settings({ state, updateState, replaceState }) {
)}
{Object.entries(state.platforms).map(([key, plat]) => (
-
+
updatePlatform(key, "name", e.target.value)} style={inputStyle} />
- {plat.feePerShare != null ? (
- <>
-
-
- updatePlatform(key, "feePerShare", parseFloat(e.target.value) || 0)} style={inputStyle} />
-
-
-
- updatePlatform(key, "flatFee", parseFloat(e.target.value) || 0)} style={inputStyle} />
-
- >
- ) : (
- <>
-
-
- updatePlatform(key, "feePercent", parseFloat(e.target.value) || 0)} style={inputStyle} />
-
-
{(plat.feePercent * 100).toFixed(1)}%
- >
- )}
+
+
+ updatePlatform(key, "feePerShare", parseFloat(e.target.value) || 0)} style={inputStyle} />
+
+
+
+ updatePlatform(key, "flatFee", parseFloat(e.target.value) || 0)} style={inputStyle} />
+
+
+
+ updatePlatform(key, "feePercent", parseFloat(e.target.value) || 0)} style={inputStyle} />
+
Key: {key}
@@ -491,8 +478,7 @@ export default function Settings({ state, updateState, replaceState }) {
Import
-
-
+
{showExportForm && (
From e09e8468774aee85df15f79b96e259c573399192 Mon Sep 17 00:00:00 2001
From: azywicki <81277290+NimbleEngineer21@users.noreply.github.com>
Date: Wed, 4 Mar 2026 19:16:47 -0500
Subject: [PATCH 2/2] fix: schema v3 migration, calcFee NaN guard, PR review
fixes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Bump SCHEMA_VERSION to 3 with proper migration ladder:
- v1→v2: adds carMaintenanceAnnual to purchase
- v2→v3: normalizes platform fee fields (all three always present)
Addresses PR review findings:
- Guard against NaN when asset.quantity is undefined in calcFee
- Require schemaVersion on import to prevent silent data loss
- Extract feeLabel() helper for fee display in Assets
- Clamp fee inputs to non-negative values in Settings
- Fix inner flex child using height:100vh instead of minHeight:0
- Fix indentation and blank line formatting in Settings
---
src/components/Layout.jsx | 2 +-
src/data/defaults.js | 2 +-
src/lib/__tests__/calculations.test.js | 7 +++-
src/lib/__tests__/storage.test.js | 37 +++++++++++++++++----
src/lib/calculations.js | 10 +++++-
src/lib/storage.js | 46 ++++++++++++++++++++------
src/pages/Assets.jsx | 4 +--
src/pages/Settings.jsx | 9 +++--
8 files changed, 89 insertions(+), 28 deletions(-)
diff --git a/src/components/Layout.jsx b/src/components/Layout.jsx
index 11b93fa..4cec00e 100644
--- a/src/components/Layout.jsx
+++ b/src/components/Layout.jsx
@@ -111,7 +111,7 @@ export default function Layout({ sellDate, onSellDateChange, purchaseDate, onPur
{/* Main content */}
-
+
{/* Top bar */}
diff --git a/src/data/defaults.js b/src/data/defaults.js
index c993498..cd5777b 100644
--- a/src/data/defaults.js
+++ b/src/data/defaults.js
@@ -123,7 +123,7 @@ export const DEFAULT_READINESS = {
assetAppreciationRate: 0,
};
-export const SCHEMA_VERSION = 1;
+export const SCHEMA_VERSION = 3;
export function createDefaultState() {
return {
diff --git a/src/lib/__tests__/calculations.test.js b/src/lib/__tests__/calculations.test.js
index 1cfbdb4..4e1973b 100644
--- a/src/lib/__tests__/calculations.test.js
+++ b/src/lib/__tests__/calculations.test.js
@@ -46,6 +46,11 @@ describe("calcFee", () => {
expect(calcFee(asset, 8000, platforms)).toBe(95);
});
+ it("handles undefined quantity without NaN", () => {
+ const asset = { feeType: "cs" };
+ expect(calcFee(asset, 5000, platforms)).toBe(10); // 0 * 0.10 + 10 + 0 * 5000
+ });
+
it("returns 0 for feeType none", () => {
expect(calcFee({ feeType: "none" }, 5000, platforms)).toBe(0);
});
@@ -186,7 +191,7 @@ describe("calcSummary", () => {
ltcgRate: 0.15, stcgRate: 0.24, niitRate: 0.038, niitApplies: false,
standardDeduction: 31400,
},
- platforms: { gem: { name: "Gemini", feePercent: 0.015 } },
+ platforms: { gem: { name: "Gemini", feePerShare: 0, flatFee: 0, feePercent: 0.015 } },
cashFlow: {
paycheckAmount: 5000, firstPayDate: "2026-03-06", paycheckFrequency: "biweekly",
expenses: [], oneTimeObligations: [],
diff --git a/src/lib/__tests__/storage.test.js b/src/lib/__tests__/storage.test.js
index e0abc28..a21e480 100644
--- a/src/lib/__tests__/storage.test.js
+++ b/src/lib/__tests__/storage.test.js
@@ -20,10 +20,35 @@ describe("migrateState", () => {
expect(result.dateOfBirth).toEqual({ month: "", year: "" });
});
- it("resets unknown older versions to defaults", () => {
- const result = migrateState({ schemaVersion: 0, assets: [], purchase: {} });
+ it("migrates v1 state: adds carMaintenanceAnnual and normalizes platforms", () => {
+ const v1State = {
+ schemaVersion: 1,
+ assets: [{ name: "GME", symbol: "GME", quantity: 10 }],
+ cashAccounts: [{ name: "Checking", balance: 5000 }],
+ purchase: { category: "home", homePrice: 350000 },
+ platforms: { cs: { name: "ComputerShare", feePerShare: 0.10, flatFee: 10 }, gem: { name: "Gemini", feePercent: 0.015 } },
+ };
+ const result = migrateState(v1State);
expect(result.schemaVersion).toBe(SCHEMA_VERSION);
- expect(Array.isArray(result.assets)).toBe(true);
+ expect(result.purchase.carMaintenanceAnnual).toBeNull();
+ expect(result.purchase.homePrice).toBe(350000);
+ expect(result.assets[0].name).toBe("GME");
+ expect(result.cashAccounts[0].balance).toBe(5000);
+ expect(result.platforms.cs).toEqual({ name: "ComputerShare", feePerShare: 0.10, flatFee: 10, feePercent: 0 });
+ expect(result.platforms.gem).toEqual({ name: "Gemini", feePerShare: 0, flatFee: 0, feePercent: 0.015 });
+ });
+
+ it("migrates v2 state: normalizes platforms to three-field format", () => {
+ const v2State = {
+ schemaVersion: 2,
+ assets: [],
+ purchase: { category: "vehicle", carMaintenanceAnnual: 500 },
+ platforms: { pp: { name: "Paypal", feePercent: 0.02 } },
+ };
+ const result = migrateState(v2State);
+ expect(result.schemaVersion).toBe(SCHEMA_VERSION);
+ expect(result.purchase.carMaintenanceAnnual).toBe(500);
+ expect(result.platforms.pp).toEqual({ name: "Paypal", feePerShare: 0, flatFee: 0, feePercent: 0.02 });
});
it("returns future-versioned state as-is", () => {
@@ -42,8 +67,8 @@ describe("validateImport", () => {
})).not.toThrow();
});
- it("accepts old backups without schemaVersion if they have data", () => {
- expect(() => validateImport({ assets: [{ name: "GME", symbol: "GME" }] })).not.toThrow();
+ it("rejects backups without schemaVersion even if they have data", () => {
+ expect(() => validateImport({ assets: [{ name: "GME", symbol: "GME" }] })).toThrow("no schemaVersion");
});
it("rejects non-object inputs", () => {
@@ -54,7 +79,7 @@ describe("validateImport", () => {
});
it("rejects objects that don't look like GreenLight data", () => {
- expect(() => validateImport({ foo: "bar" })).toThrow("doesn't look like a GreenLight backup");
+ expect(() => validateImport({ foo: "bar" })).toThrow("no schemaVersion");
});
it("rejects invalid schemaVersion", () => {
diff --git a/src/lib/calculations.js b/src/lib/calculations.js
index 49d0956..f7c8abd 100644
--- a/src/lib/calculations.js
+++ b/src/lib/calculations.js
@@ -13,7 +13,15 @@ export function calcFee(asset, grossValue, platforms) {
if (ft === "none" || !ft) return 0;
const plat = platforms[ft];
if (!plat) return 0;
- return (plat.feePerShare || 0) * asset.quantity + (plat.flatFee || 0) + (plat.feePercent || 0) * grossValue;
+ return (plat.feePerShare || 0) * (asset.quantity ?? 0) + (plat.flatFee || 0) + (plat.feePercent || 0) * grossValue;
+}
+
+export function feeLabel(plat) {
+ const parts = [];
+ if (plat.feePerShare) parts.push(`$${plat.feePerShare}/sh`);
+ if (plat.flatFee) parts.push(`$${plat.flatFee} flat`);
+ if (plat.feePercent) parts.push(`${(plat.feePercent * 100).toFixed(1)}%`);
+ return parts.join(" + ") || "Free";
}
export function paychecksBefore(sellDate, cashFlowConfig) {
diff --git a/src/lib/storage.js b/src/lib/storage.js
index 3237fdd..cd2fc36 100644
--- a/src/lib/storage.js
+++ b/src/lib/storage.js
@@ -96,18 +96,16 @@ export function validateImport(obj) {
}
// Must have a schema version (any GreenLight export has one)
- if (obj.schemaVersion == null && obj.assets == null && obj.cashAccounts == null) {
- throw new Error("This doesn't look like a GreenLight backup (no schemaVersion or data found)");
+ if (obj.schemaVersion == null) {
+ throw new Error("This doesn't look like a GreenLight backup (no schemaVersion found)");
}
// Schema version sanity check
- if (obj.schemaVersion != null) {
- if (typeof obj.schemaVersion !== "number" || obj.schemaVersion < 1) {
- throw new Error(`Invalid schemaVersion: ${obj.schemaVersion}`);
- }
- if (obj.schemaVersion > SCHEMA_VERSION + 5) {
- throw new Error(`This backup is from a newer version (v${obj.schemaVersion}). Update GreenLight first.`);
- }
+ if (typeof obj.schemaVersion !== "number" || obj.schemaVersion < 1) {
+ throw new Error(`Invalid schemaVersion: ${obj.schemaVersion}`);
+ }
+ if (obj.schemaVersion > SCHEMA_VERSION + 5) {
+ throw new Error(`This backup is from a newer version (v${obj.schemaVersion}). Update GreenLight first.`);
}
// Validate array fields are actually arrays
@@ -158,6 +156,32 @@ export function migrateState(state) {
return state;
}
- // Unknown older version — reset
- return createDefaultState();
+ let data = { ...state };
+
+ // v1 → v2: add carMaintenanceAnnual to purchase
+ if ((data.schemaVersion ?? 0) < 2) {
+ if (data.purchase && data.purchase.carMaintenanceAnnual === undefined) {
+ data.purchase = { ...data.purchase, carMaintenanceAnnual: null };
+ }
+ data.schemaVersion = 2;
+ }
+
+ // v2 → v3: normalize platform fee fields (all three always present)
+ if (data.schemaVersion < 3) {
+ if (data.platforms) {
+ const migrated = {};
+ for (const [key, plat] of Object.entries(data.platforms)) {
+ migrated[key] = {
+ name: plat.name,
+ feePerShare: plat.feePerShare ?? 0,
+ flatFee: plat.flatFee ?? 0,
+ feePercent: plat.feePercent ?? 0,
+ };
+ }
+ data.platforms = migrated;
+ }
+ data.schemaVersion = 3;
+ }
+
+ return data;
}
diff --git a/src/pages/Assets.jsx b/src/pages/Assets.jsx
index 4cc9408..0fa1739 100644
--- a/src/pages/Assets.jsx
+++ b/src/pages/Assets.jsx
@@ -1,6 +1,6 @@
import React, { useState, useMemo } from "react";
import { colors, styles } from "../theme.js";
-import { calcSummary, calcRetirementNet, fmt, fmtQty } from "../lib/calculations.js";
+import { calcSummary, calcRetirementNet, feeLabel, fmt, fmtQty } from "../lib/calculations.js";
import { RETIREMENT_ACCOUNT_TYPES, uuid } from "../data/defaults.js";
const EMPTY_ASSET = {
@@ -185,7 +185,7 @@ export default function Assets({ state, updateState, prices }) {
{plat.name}:
- {[plat.feePerShare ? `$${plat.feePerShare}/sh` : null, plat.flatFee ? `$${plat.flatFee} flat` : null, plat.feePercent ? `${(plat.feePercent * 100).toFixed(1)}%` : null].filter(Boolean).join(" + ") || "Free"}
+ {feeLabel(plat)}
))}
diff --git a/src/pages/Settings.jsx b/src/pages/Settings.jsx
index 62ca3b2..5e7e982 100644
--- a/src/pages/Settings.jsx
+++ b/src/pages/Settings.jsx
@@ -174,7 +174,6 @@ export default function Settings({ state, updateState, replaceState }) {
}
};
-
const labelStyle = { fontSize: 13, color: colors.dim, textTransform: "uppercase", letterSpacing: 1, display: "block", marginBottom: 4 };
const inputStyle = styles.input;
const btnStyle = { ...styles.btn, padding: "8px 18px", fontSize: 15 };
@@ -354,17 +353,17 @@ export default function Settings({ state, updateState, replaceState }) {
updatePlatform(key, "feePerShare", parseFloat(e.target.value) || 0)} style={inputStyle} />
+ onChange={e => updatePlatform(key, "feePerShare", Math.max(0, parseFloat(e.target.value) || 0))} style={inputStyle} />
updatePlatform(key, "flatFee", parseFloat(e.target.value) || 0)} style={inputStyle} />
+ onChange={e => updatePlatform(key, "flatFee", Math.max(0, parseFloat(e.target.value) || 0))} style={inputStyle} />
updatePlatform(key, "feePercent", parseFloat(e.target.value) || 0)} style={inputStyle} />
+ onChange={e => updatePlatform(key, "feePercent", Math.max(0, parseFloat(e.target.value) || 0))} style={inputStyle} />
Key: {key}
@@ -478,7 +477,7 @@ export default function Settings({ state, updateState, replaceState }) {
Import
-
+
{showExportForm && (