From b40ba6510ef5de3432202eb623f3b7321d0d2649 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 22:09:15 +0000 Subject: [PATCH 1/5] docs: record that the calculator works offline The "Offline behavior" follow-up was fixed by #113 (fishDataShape gives the bundled yields from/to states). Browser-checked against the production build: API failing, API hanging, fully offline via the service worker, and a late API answer all keep the calculator working. Note the remaining gap: a signed-in user's own yields are not shown offline. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01ErLEY8gicR3Wg7Q1yXAVaC --- docs/DESIGN_SYSTEM.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/docs/DESIGN_SYSTEM.md b/docs/DESIGN_SYSTEM.md index 7bcac39..c5fd519 100644 --- a/docs/DESIGN_SYSTEM.md +++ b/docs/DESIGN_SYSTEM.md @@ -51,6 +51,10 @@ Chosen from the three mock-ups in `docs/design-mockups/` (direction A). - **Number boxes read what people type** (`app/src/lib/numberInput.js`): "$4.50", "1,000", "42%" and "4,50" all work. Text that isn't a number marks that box as invalid and the bar says to use numbers; it never counts as 0, because a wrong answer is worse than none. +- **Works with no signal.** The calculator starts from the bundled reference yields + (`FISH_DATA_V3`, given `from`/`to` by `app/src/lib/fishDataShape.js`) and the service worker serves + the app, so it works on a boat with no connection. `/api/fish-data` replaces that data only when it + sends usable yields, and what is already picked stays picked. - **Nothing hides behind the bar.** The calculator sets the page's `scroll-padding-bottom` to the bar's height, so whatever you Tab to scrolls into view above it (WCAG 2.4.11). @@ -70,6 +74,6 @@ Chosen from the three mock-ups in `docs/design-mockups/` (direction A). and the font link in `index.html`. - **Unused components** `Footer.jsx` and `InstallPrompt.jsx` are not rendered anywhere and use tokens that no longer exist (`bg-navy`, `text-teal`, `bg-rust`). Restyle before wiring them in, or delete. -- **Offline behavior.** `Calculator` seeds its data from `FISH_DATA_V3`, whose conversions have no - `from`/`to` fields (they are derived server-side by `/api/fish-data`). If that request fails, the - "What you have" list is empty and the calculator cannot be used, which matters on a boat with poor signal. +- **Your own yields offline.** The calculator reads a signed-in user's custom yields only from + `/api/user-data`, not from the copy `DataContext` keeps on the device, so they are missing offline + (the reference yields still work). From 6cff891c413567d924b0fd971038976910e4825e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 23:02:39 +0000 Subject: [PATCH 2/5] fix: show custom yields in the calculator offline The calculator fetched a signed-in user's custom yields straight from /api/user-data, so they vanished with no signal. It now reads the copy DataContext keeps on the device, merged by the existing mergeFishData. That copy never took edits or deletions made on another device, which the direct fetch had hidden. The sync's pull now updates a synced yield whose server values differ (compared by value, since Neon sends "48.00") and drops synced yields the server no longer lists. Records with unpushed local changes or conflicts are left alone. Interim until ADR 0001 replaces the sync engine with Firestore. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01ErLEY8gicR3Wg7Q1yXAVaC --- app/src/components/Calculator.jsx | 33 +++------------- app/src/lib/localRepository.js | 49 +++++++++++++++-------- app/src/lib/localRepository.test.js | 60 +++++++++++++++++++++++++++++ docs/DESIGN_SYSTEM.md | 7 ++-- 4 files changed, 101 insertions(+), 48 deletions(-) diff --git a/app/src/components/Calculator.jsx b/app/src/components/Calculator.jsx index 6808873..a12da46 100644 --- a/app/src/components/Calculator.jsx +++ b/app/src/components/Calculator.jsx @@ -3,10 +3,12 @@ import { Link } from 'react-router-dom'; import { ACRONYMS, FISH_DATA_V3, PROFILES_DATA } from '../data/fish_data_v3'; import { Calculator as CalcIcon, Save, HelpCircle, Download, ChevronDown } from 'lucide-react'; import { useAuth } from '../context/AuthContext'; +import { useData } from '../context/DataContext'; import { apiUrl } from '../config/api'; import { calculate } from '../lib/calcEngine'; import { parseAmount } from '../lib/numberInput'; import { withConversionStates, hasUsableConversions, parseYieldPercent } from '../lib/fishDataShape'; +import { mergeFishData } from '../lib/fishDataMerge'; /** * Help bubble that works for mouse (hover), keyboard (focus) and touch (tap). @@ -232,6 +234,8 @@ const TO_LIMIT = 6; const Calculator = () => { const { user, getAuthHeaders } = useAuth(); + // Custom yields kept on this device and synced when there is signal, so they work offline too + const { customYields } = useData(); const [mode, setMode] = useState('cost'); const [targetWeight, setTargetWeight] = useState(''); const [species, setSpecies] = useState(''); @@ -249,7 +253,6 @@ const Calculator = () => { const [announcement, setAnnouncement] = useState(''); const dockRef = useRef(null); - const [customData, setCustomData] = useState({}); const [_history, setHistory] = useState([]); const [publicHistory, setPublicHistory] = useState([]); @@ -279,24 +282,6 @@ const Calculator = () => { useEffect(() => { if (user) { getAuthHeaders().then(headers => { - fetch(apiUrl('/api/user-data'), { headers }) - .then(res => res.json()) - .then(data => { - if (Array.isArray(data)) { - const mapped = {}; - data.forEach(item => { - if (!mapped[item.species]) mapped[item.species] = { conversions: {} }; - mapped[item.species].conversions[`Custom: ${item.product}`] = { - yield: parseFloat(item.yield), - from: 'Custom', - to: item.product - }; - }); - setCustomData(mapped); - } - }) - .catch(() => {}); - fetch(apiUrl('/api/saved-calcs'), { headers }) .then(res => res.json()) .then(data => setHistory(data)) @@ -304,19 +289,11 @@ const Calculator = () => { }); } else { // eslint-disable-next-line react-hooks/set-state-in-effect - setCustomData({}); setHistory([]); } }, [user, getAuthHeaders]); - const combinedData = useMemo(() => { - const merged = { ...fishData }; - Object.keys(customData).forEach(sp => { - if (!merged[sp]) merged[sp] = customData[sp]; - else merged[sp] = { ...merged[sp], conversions: { ...merged[sp].conversions, ...customData[sp].conversions } }; - }); - return merged; - }, [fishData, customData]); + const combinedData = useMemo(() => mergeFishData(fishData, customYields), [fishData, customYields]); const speciesList = Object.keys(combinedData).sort(); diff --git a/app/src/lib/localRepository.js b/app/src/lib/localRepository.js index 93acbd1..b9f6925 100644 --- a/app/src/lib/localRepository.js +++ b/app/src/lib/localRepository.js @@ -38,6 +38,18 @@ function now() { return new Date().toISOString(); } +// Compares by value: the server sends yield as a decimal string ("48.00") and may include +// fields a record made on this device never set (is_shared). +function sameYieldValues(local, server) { + return ( + local.species === server.species && + local.product === server.product && + Number(local.yield) === Number(server.yield) && + (local.source || 'User Input') === server.source && + Boolean(local.is_shared) === Boolean(server.is_shared) + ); +} + function makeRecord(data, scope) { const ts = now(); return { @@ -515,16 +527,19 @@ class LocalRepository { ); for (const sy of serverYields) { + const server = { + species: sy.species, + product: sy.product, + yield: sy.yield, + source: sy.source || 'User Input', + is_shared: sy.is_shared ?? false, + }; const localId = byServerId.get(String(sy.id)); if (!localId) { // New from server — insert as synced. const ts = now(); all.push({ - species: sy.species, - product: sy.product, - yield: sy.yield, - source: sy.source || 'User Input', - is_shared: sy.is_shared ?? false, + ...server, id: crypto.randomUUID(), scope: this._scope, serverId: sy.id, @@ -544,21 +559,23 @@ class LocalRepository { if (rec.syncStatus === 'conflicted' || rec.syncStatus === 'conflict-delete') { all[idx] = { ...rec, - conflictServer: { - serverId: sy.id, - serverRevision: sy.revision, - species: sy.species, - product: sy.product, - yield: sy.yield, - source: sy.source || 'User Input', - is_shared: sy.is_shared ?? false, - }, + conflictServer: { serverId: sy.id, serverRevision: sy.revision, ...server }, }; + } else if (rec.syncStatus === 'synced' && !sameYieldValues(rec, server)) { + // Edited on another device: a synced record is a copy of the server's, so take its version. + all[idx] = { ...rec, ...server, serverRevision: sy.revision, updatedAt: now() }; } - // pending-delete, local, synced: no change needed + // pending-delete, local: the local change is still to be pushed, so keep it } - await this._set(key, all); + // A synced record the server no longer lists was deleted on another device. Records with + // local changes (local, pending-delete, conflicts) stay for the push and conflict flows. + const onServer = new Set(serverYields.map((sy) => String(sy.id))); + const kept = all.filter( + (y) => !(y.syncStatus === 'synced' && y.serverId != null && !onServer.has(String(y.serverId))) + ); + + await this._set(key, kept); }); } diff --git a/app/src/lib/localRepository.test.js b/app/src/lib/localRepository.test.js index e8a1c4d..0a77f55 100644 --- a/app/src/lib/localRepository.test.js +++ b/app/src/lib/localRepository.test.js @@ -411,6 +411,66 @@ describe('mergeServerYields', () => { const pending = await repo.getPendingSync(); expect(pending.yields[0].syncStatus).toBe('pending-delete'); }); + + it('takes the server version of a synced yield edited on another device', async () => { + const rec = await repo.addYield({ species: 'Halibut', product: 'Skinless Fillet', yield: 48 }); + await repo.markYieldSynced(rec.id, 12, 1); + + await repo.mergeServerYields([ + { id: 12, revision: 2, species: 'Halibut', product: 'Skin-On Fillet', yield: 52, source: 'Scale', is_shared: false }, + ]); + + const yields = await repo.getYields(); + expect(yields).toHaveLength(1); + expect(yields[0]).toMatchObject({ + id: rec.id, serverId: 12, serverRevision: 2, syncStatus: 'synced', + product: 'Skin-On Fillet', yield: 52, source: 'Scale', + }); + }); + + it('leaves a synced yield alone when the server copy only looks different', async () => { + const rec = await repo.addYield({ species: 'Halibut', product: 'Skinless Fillet', yield: 48, source: 'User Input' }); + await repo.markYieldSynced(rec.id, 12); // a create reply carries no revision + const [before] = await repo.getYields(); + + await repo.mergeServerYields([ + { id: 12, revision: 1, species: 'Halibut', product: 'Skinless Fillet', yield: '48.00', source: 'User Input', is_shared: false }, + ]); + + const [after] = await repo.getYields(); + expect(after).toEqual(before); + }); + + it('drops a synced yield the server no longer has (deleted on another device)', async () => { + const gone = await repo.addYield({ species: 'Halibut', product: 'Skinless Fillet', yield: 48 }); + await repo.markYieldSynced(gone.id, 12, 1); + const kept = await repo.addYield({ species: 'Cod', product: 'Fillet', yield: 40 }); + await repo.markYieldSynced(kept.id, 13, 1); + + await repo.mergeServerYields([{ id: 13, revision: 1, species: 'Cod', product: 'Fillet', yield: 40 }]); + + const yields = await repo.getYields(); + expect(yields.map((y) => y.serverId)).toEqual([13]); + }); + + it('keeps unsynced, deleting and conflicted yields the server does not list', async () => { + await repo.addYield({ species: 'Cod', product: 'Fillet', yield: 40 }); // added offline + const edited = await repo.addYield({ species: 'Halibut', product: 'Skinless Fillet', yield: 48 }); + await repo.markYieldSynced(edited.id, 12, 1); + await repo.updateYield(edited.id, { yield: 50 }); // edited offline, not pushed yet + const deleting = await repo.addYield({ species: 'Pollock', product: 'Fillet', yield: 30 }); + await repo.markYieldSynced(deleting.id, 14, 1); + await repo.removeYield(deleting.id); + const conflicted = await repo.addYield({ species: 'Sole', product: 'Fillet', yield: 35 }); + await repo.markYieldSynced(conflicted.id, 15, 1); + await repo.updateYield(conflicted.id, { yield: 36 }); + await repo.markYieldConflicted(conflicted.id); + + await repo.mergeServerYields([]); + + expect((await repo.getYields()).map((y) => y.species).sort()).toEqual(['Cod', 'Halibut', 'Sole']); + expect((await repo.getPendingSync()).yields.map((y) => y.species)).toContain('Pollock'); + }); }); // ---- removeCalcTombstone / removeYieldTombstone ---- diff --git a/docs/DESIGN_SYSTEM.md b/docs/DESIGN_SYSTEM.md index c5fd519..c2a783f 100644 --- a/docs/DESIGN_SYSTEM.md +++ b/docs/DESIGN_SYSTEM.md @@ -54,7 +54,9 @@ Chosen from the three mock-ups in `docs/design-mockups/` (direction A). - **Works with no signal.** The calculator starts from the bundled reference yields (`FISH_DATA_V3`, given `from`/`to` by `app/src/lib/fishDataShape.js`) and the service worker serves the app, so it works on a boat with no connection. `/api/fish-data` replaces that data only when it - sends usable yields, and what is already picked stays picked. + sends usable yields, and what is already picked stays picked. A signed-in person's custom yields come + from the copy `DataContext` keeps on the device (merged by `app/src/lib/fishDataMerge.js`), which the + sync refreshes when there is signal, so they work offline too. - **Nothing hides behind the bar.** The calculator sets the page's `scroll-padding-bottom` to the bar's height, so whatever you Tab to scrolls into view above it (WCAG 2.4.11). @@ -74,6 +76,3 @@ Chosen from the three mock-ups in `docs/design-mockups/` (direction A). and the font link in `index.html`. - **Unused components** `Footer.jsx` and `InstallPrompt.jsx` are not rendered anywhere and use tokens that no longer exist (`bg-navy`, `text-teal`, `bg-rust`). Restyle before wiring them in, or delete. -- **Your own yields offline.** The calculator reads a signed-in user's custom yields only from - `/api/user-data`, not from the copy `DataContext` keeps on the device, so they are missing offline - (the reference yields still work). From ee8cbda1413341b84f9a9f0e045d58b25f4e963a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 23:22:26 +0000 Subject: [PATCH 3/5] fix: track a newer yield revision when values are unchanged A synced yield edited elsewhere and then restored kept its old revision, so the next local edit was pushed with a stale expected_revision and hit a false 409 conflict. Take the server revision on its own, keeping the local values and updatedAt. Raised by CodeRabbit on #135. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01ErLEY8gicR3Wg7Q1yXAVaC --- app/src/lib/localRepository.js | 11 ++++++++--- app/src/lib/localRepository.test.js | 21 ++++++++++++++++++++- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/app/src/lib/localRepository.js b/app/src/lib/localRepository.js index b9f6925..f8f566e 100644 --- a/app/src/lib/localRepository.js +++ b/app/src/lib/localRepository.js @@ -561,9 +561,14 @@ class LocalRepository { ...rec, conflictServer: { serverId: sy.id, serverRevision: sy.revision, ...server }, }; - } else if (rec.syncStatus === 'synced' && !sameYieldValues(rec, server)) { - // Edited on another device: a synced record is a copy of the server's, so take its version. - all[idx] = { ...rec, ...server, serverRevision: sy.revision, updatedAt: now() }; + } else if (rec.syncStatus === 'synced') { + if (!sameYieldValues(rec, server)) { + // Edited on another device: a synced record is a copy of the server's, so take its version. + all[idx] = { ...rec, ...server, serverRevision: sy.revision, updatedAt: now() }; + } else if (sy.revision != null && rec.serverRevision !== sy.revision) { + // Same values at a newer revision: track it, or the next local edit is a false conflict. + all[idx] = { ...rec, serverRevision: sy.revision }; + } } // pending-delete, local: the local change is still to be pushed, so keep it } diff --git a/app/src/lib/localRepository.test.js b/app/src/lib/localRepository.test.js index 0a77f55..eb76f9b 100644 --- a/app/src/lib/localRepository.test.js +++ b/app/src/lib/localRepository.test.js @@ -430,7 +430,7 @@ describe('mergeServerYields', () => { it('leaves a synced yield alone when the server copy only looks different', async () => { const rec = await repo.addYield({ species: 'Halibut', product: 'Skinless Fillet', yield: 48, source: 'User Input' }); - await repo.markYieldSynced(rec.id, 12); // a create reply carries no revision + await repo.markYieldSynced(rec.id, 12, 1); const [before] = await repo.getYields(); await repo.mergeServerYields([ @@ -441,6 +441,25 @@ describe('mergeServerYields', () => { expect(after).toEqual(before); }); + it('tracks a newer server revision even when the values are unchanged', async () => { + // Another device edited the yield and then put the old values back + const rec = await repo.addYield({ species: 'Halibut', product: 'Skinless Fillet', yield: 48, source: 'User Input' }); + await repo.markYieldSynced(rec.id, 12, 1); + const [before] = await repo.getYields(); + + await repo.mergeServerYields([ + { id: 12, revision: 3, species: 'Halibut', product: 'Skinless Fillet', yield: '48.00', source: 'User Input', is_shared: false }, + ]); + + const [after] = await repo.getYields(); + expect(after).toEqual({ ...before, serverRevision: 3 }); + + // so the next local edit is pushed against the current revision, not a false conflict + await repo.updateYield(rec.id, { yield: 50 }); + const pending = await repo.getPendingSync(); + expect(pending.yields[0].serverRevision).toBe(3); + }); + it('drops a synced yield the server no longer has (deleted on another device)', async () => { const gone = await repo.addYield({ species: 'Halibut', product: 'Skinless Fillet', yield: 48 }); await repo.markYieldSynced(gone.id, 12, 1); From e15b8806e7b8b66061222bc801ec9fd62cea3eba Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 23:30:06 +0000 Subject: [PATCH 4/5] fix: scope, refresh and follow custom yields in the calculator Addresses the Codex review on #135: - Password logins set the account id from the JWT (new lib/legacyJwt), so their synced yields go to the account's scope, not the guest one. The calculator also shows custom yields only to a signed-in person. - If site storage can't be read, DataContext reads the account's yields from the server into memory, so they still show online. - Opening the calculator pulls again, as its own fetch used to. - When a sync changes the chosen conversion, the yield box follows it unless it was typed by hand; a deleted conversion is unpicked. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01ErLEY8gicR3Wg7Q1yXAVaC --- app/src/components/Calculator.jsx | 26 ++++++++++++++++++++-- app/src/context/AuthContext.jsx | 36 ++++++------------------------ app/src/context/DataContext.jsx | 36 ++++++++++++++++++++++++------ app/src/lib/legacyJwt.js | 37 +++++++++++++++++++++++++++++++ app/src/lib/legacyJwt.test.js | 34 ++++++++++++++++++++++++++++ docs/DESIGN_SYSTEM.md | 3 ++- 6 files changed, 133 insertions(+), 39 deletions(-) create mode 100644 app/src/lib/legacyJwt.js create mode 100644 app/src/lib/legacyJwt.test.js diff --git a/app/src/components/Calculator.jsx b/app/src/components/Calculator.jsx index 4ee5372..4ae0109 100644 --- a/app/src/components/Calculator.jsx +++ b/app/src/components/Calculator.jsx @@ -232,11 +232,12 @@ const StepHeading = ({ number, children, id }) => ( ); const TO_LIMIT = 6; +const NO_YIELDS = []; const Calculator = () => { const { user, getAuthHeaders } = useAuth(); // Custom yields kept on this device and synced when there is signal, so they work offline too - const { customYields } = useData(); + const { customYields, dataLoaded, retrySync } = useData(); const [mode, setMode] = useState('cost'); const [targetWeight, setTargetWeight] = useState(''); const [species, setSpecies] = useState(''); @@ -294,7 +295,15 @@ const Calculator = () => { } }, [user, getAuthHeaders]); - const combinedData = useMemo(() => mergeFishData(fishData, customYields), [fishData, customYields]); + // The provider pulls once when it loads; opening the calculator again pulls too, so a yield + // edited on another device shows up here as it did when this page fetched its own copy + useEffect(() => { + if (dataLoaded) retrySync(); + }, [dataLoaded, retrySync]); + + // Only a signed-in person's own yields: the guest scope can hold records no one here owns + const myYields = user ? customYields : NO_YIELDS; + const combinedData = useMemo(() => mergeFishData(fishData, myYields), [fishData, myYields]); const speciesList = Object.keys(combinedData).sort(); @@ -321,6 +330,19 @@ const Calculator = () => { ); }, [species, fromState, toState, combinedData]); + // When a sync changes the chosen conversion (edited or deleted on another device), follow it, + // unless the yield was typed in by hand + const conversionYield = currentConversion ? String(currentConversion.yield) : null; + const [followedYield, setFollowedYield] = useState(conversionYield); + if (conversionYield !== followedYield) { + setFollowedYield(conversionYield); + if (conversionYield === null && toState) { + setToState(''); setYieldPercent(''); + } else if (conversionYield !== null && yieldPercent === followedYield) { + setYieldPercent(conversionYield); + } + } + const profile = species ? profilesData[species] : null; const scientificName = species && combinedData[species] ? combinedData[species].scientific_name : null; const yieldRange = currentConversion?.range || null; diff --git a/app/src/context/AuthContext.jsx b/app/src/context/AuthContext.jsx index 09d4902..751a701 100644 --- a/app/src/context/AuthContext.jsx +++ b/app/src/context/AuthContext.jsx @@ -1,6 +1,7 @@ import React, { createContext, useContext, useState, useCallback, useLayoutEffect, useEffect } from 'react'; import { apiUrl } from '../config/api'; import { getAuthHeaders as getAuthHeadersFn } from '../lib/authHeaders'; +import { decodeJwtPayload, legacyUserFromToken } from '../lib/legacyJwt'; import { clearFirebaseSession, createGoogleAuthUri, @@ -27,23 +28,6 @@ const defaultAuthApi = { signUpWithEmailPassword, }; -function decodeBase64UrlJson(value) { - try { - let base64 = value.replace(/-/g, '+').replace(/_/g, '/'); - const padding = base64.length % 4; - if (padding) { - base64 += '='.repeat(4 - padding); - } - - const binary = globalThis.atob(base64); - const bytes = Uint8Array.from(binary, (c) => c.charCodeAt(0)); - const text = new TextDecoder().decode(bytes); - return JSON.parse(text); - } catch { - return null; - } -} - function loadLegacyJwtSession(storage = globalThis.localStorage) { if (!storage || typeof storage.getItem !== 'function') { return null; @@ -54,8 +38,7 @@ function loadLegacyJwtSession(storage = globalThis.localStorage) { return null; } - const [, encodedPayload] = storedToken.split('.'); - const payload = encodedPayload ? decodeBase64UrlJson(encodedPayload) : null; + const payload = decodeJwtPayload(storedToken); if (!payload?.username) { storage.removeItem?.('token'); return null; @@ -69,12 +52,7 @@ function loadLegacyJwtSession(storage = globalThis.localStorage) { return { token: storedToken, - user: { - id: payload.id, - username: payload.username, - email: payload.email || null, - authProvider: 'password', - }, + user: legacyUserFromToken(storedToken), }; } @@ -125,8 +103,7 @@ export const AuthProvider = ({ children, authApi = defaultAuthApi }) => { // as logged in while all protected requests return 401. useEffect(() => { if (!token) return; - const [, encodedPayload] = token.split('.'); - const payload = encodedPayload ? decodeBase64UrlJson(encodedPayload) : null; + const payload = decodeJwtPayload(token); if (!payload?.exp) return; let timerId; function scheduleLogout() { @@ -183,7 +160,7 @@ export const AuthProvider = ({ children, authApi = defaultAuthApi }) => { authApi.clearFirebaseSession(); globalThis.localStorage?.setItem('token', legacyData.token); setToken(legacyData.token); - setUser({ username: legacyData.username, authProvider: 'password' }); + setUser(legacyUserFromToken(legacyData.token) ?? { username: legacyData.username, authProvider: 'password' }); return true; } const response = await globalThis.fetch(apiUrl('/api/login'), { @@ -201,7 +178,8 @@ export const AuthProvider = ({ children, authApi = defaultAuthApi }) => { authApi.clearFirebaseSession(); globalThis.localStorage?.setItem('token', data.token); setToken(data.token); - setUser({ username: data.username, authProvider: 'password' }); + // The id in the token gives the account its own storage scope (see legacyUserFromToken) + setUser(legacyUserFromToken(data.token) ?? { username: data.username, authProvider: 'password' }); return true; }; diff --git a/app/src/context/DataContext.jsx b/app/src/context/DataContext.jsx index 74fb822..b6da423 100644 --- a/app/src/context/DataContext.jsx +++ b/app/src/context/DataContext.jsx @@ -25,6 +25,7 @@ export function DataProvider({ children }) { const [customSpecies, setCustomSpeciesState] = useState({}); const [isOnline, setIsOnline] = useState(navigator.onLine); const [dataLoaded, setDataLoaded] = useState(false); + const [storageFailed, setStorageFailed] = useState(false); // 'idle' | 'syncing' | 'synced' | 'offline' | 'pending' | 'error' | 'conflict' const [syncStatus, setSyncStatus] = useState('idle'); const [syncError, setSyncError] = useState(null); // null | 'auth' | 'network' @@ -91,12 +92,19 @@ export function DataProvider({ children }) { loadedScopeRef.current = null; async function loadData() { setDataLoaded(false); - const [calcs, yields, species, conflicts] = await Promise.all([ - repo.getCalcs(), - repo.getYields(), - getCustomSpecies(), - repo.getConflictedYields(), - ]); + let calcs, yields, species, conflicts; + try { + [calcs, yields, species, conflicts] = await Promise.all([ + repo.getCalcs(), + repo.getYields(), + getCustomSpecies(), + repo.getConflictedYields(), + ]); + } catch { + // Site storage blocked or broken: nothing can be kept on this device. + if (!cancelled) setStorageFailed(true); + return; + } if (!cancelled) { setSavedCalcs(calcs); setCustomYields(yields); @@ -110,6 +118,20 @@ export function DataProvider({ children }) { return () => { cancelled = true; }; }, [repo, scope]); + // Without on-device storage there is nothing for the sync to fill, so read the account's custom + // yields from the server instead and hold them in memory, tagged with the account they belong to. + const [serverYields, setServerYields] = useState({ uid: null, rows: [] }); + useEffect(() => { + if (!storageFailed || !uid || !isOnline) return; + let cancelled = false; + getAuthHeaders() + .then((headers) => apiClient.listUserDataRaw(headers)) + .then((res) => (res.ok ? res.json() : null)) + .then((rows) => { if (!cancelled && Array.isArray(rows)) setServerYields({ uid, rows }); }) + .catch(() => {}); + return () => { cancelled = true; }; + }, [storageFailed, uid, isOnline, getAuthHeaders]); + // One-time legacy migration + recovery check. Runs once per session. useEffect(() => { if (recoveryCheckedRef.current) return; @@ -632,7 +654,7 @@ export function DataProvider({ children }) { const value = { savedCalcs: scopeReady ? savedCalcs : [], - customYields: scopeReady ? customYields : [], + customYields: scopeReady ? customYields : (storageFailed && serverYields.uid === uid ? serverYields.rows : []), customSpecies, conflictedYields: scopeReady ? conflictedYields : [], isOnline, diff --git a/app/src/lib/legacyJwt.js b/app/src/lib/legacyJwt.js new file mode 100644 index 0000000..52676f0 --- /dev/null +++ b/app/src/lib/legacyJwt.js @@ -0,0 +1,37 @@ +// Legacy password sessions (retired by ADR 0001) keep the account in a JWT. The client only +// reads it for display and for choosing where data is stored; the server verifies every token. + +function decodeBase64UrlJson(value) { + try { + let base64 = value.replace(/-/g, '+').replace(/_/g, '/'); + const padding = base64.length % 4; + if (padding) { + base64 += '='.repeat(4 - padding); + } + + const binary = globalThis.atob(base64); + const bytes = Uint8Array.from(binary, (c) => c.charCodeAt(0)); + const text = new TextDecoder().decode(bytes); + return JSON.parse(text); + } catch { + return null; + } +} + +export function decodeJwtPayload(token) { + const [, encodedPayload] = String(token ?? '').split('.'); + return encodedPayload ? decodeBase64UrlJson(encodedPayload) : null; +} + +// The signed-in user a legacy JWT stands for. The id matters: DataContext keys the account's +// on-device data by it, so without one the account's synced yields would land in the guest scope. +export function legacyUserFromToken(token) { + const payload = decodeJwtPayload(token); + if (!payload?.username) return null; + return { + id: payload.id, + username: payload.username, + email: payload.email || null, + authProvider: 'password', + }; +} diff --git a/app/src/lib/legacyJwt.test.js b/app/src/lib/legacyJwt.test.js new file mode 100644 index 0000000..6b0b448 --- /dev/null +++ b/app/src/lib/legacyJwt.test.js @@ -0,0 +1,34 @@ +import { Buffer } from 'node:buffer'; +import { describe, expect, it } from 'vitest'; +import { decodeJwtPayload, legacyUserFromToken } from './legacyJwt'; + +const b64 = (value) => Buffer.from(JSON.stringify(value)).toString('base64url'); +const jwt = (payload) => `${b64({ alg: 'HS256', typ: 'JWT' })}.${b64(payload)}.signature`; + +describe('decodeJwtPayload', () => { + it('reads the payload of a JWT', () => { + expect(decodeJwtPayload(jwt({ id: 7, username: 'skipper' }))).toEqual({ id: 7, username: 'skipper' }); + }); + + it('returns null for anything that is not a JWT', () => { + expect(decodeJwtPayload('not-a-jwt')).toBeNull(); + expect(decodeJwtPayload('a.%%%.c')).toBeNull(); + expect(decodeJwtPayload(null)).toBeNull(); + }); +}); + +describe('legacyUserFromToken', () => { + it('includes the account id, so the account gets its own storage scope', () => { + expect(legacyUserFromToken(jwt({ id: 7, username: 'skipper', exp: 9999999999 }))).toEqual({ + id: 7, + username: 'skipper', + email: null, + authProvider: 'password', + }); + }); + + it('returns null when the token names no user', () => { + expect(legacyUserFromToken(jwt({ id: 7 }))).toBeNull(); + expect(legacyUserFromToken('garbage')).toBeNull(); + }); +}); diff --git a/docs/DESIGN_SYSTEM.md b/docs/DESIGN_SYSTEM.md index c2a783f..c2ffd9a 100644 --- a/docs/DESIGN_SYSTEM.md +++ b/docs/DESIGN_SYSTEM.md @@ -56,7 +56,8 @@ Chosen from the three mock-ups in `docs/design-mockups/` (direction A). the app, so it works on a boat with no connection. `/api/fish-data` replaces that data only when it sends usable yields, and what is already picked stays picked. A signed-in person's custom yields come from the copy `DataContext` keeps on the device (merged by `app/src/lib/fishDataMerge.js`), which the - sync refreshes when there is signal, so they work offline too. + sync refreshes when there is signal (and each time the calculator opens), so they work offline too. + If a sync changes the yield you picked, the yield box follows it, unless you typed your own. - **Nothing hides behind the bar.** The calculator sets the page's `scroll-padding-bottom` to the bar's height, so whatever you Tab to scrolls into view above it (WCAG 2.4.11). From 07fd054b1010ad525df5fa1a54295cba19ace953 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 23:42:25 +0000 Subject: [PATCH 5/5] fix: keep edits made during a yield push and refresh fallback yields Addresses the second Codex review on #135: - markYieldSynced gets the record as it was pushed. If it was edited or deleted again while the request was out, it keeps the newer change queued (taking only the server id and revision), so the pull can't revert the edit or bring a deleted yield back. - My Data: saving an edit to a yield deleted on another device now says so and switches to adding it back, instead of reporting success and dropping the edit. - refreshCustomYields() syncs, or refetches the in-memory server copy when storage is blocked; the calculator calls it each time it opens. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01ErLEY8gicR3Wg7Q1yXAVaC --- app/src/components/Calculator.jsx | 6 ++--- app/src/components/DataManagement.jsx | 8 +++++- app/src/context/DataContext.jsx | 11 +++++++- app/src/lib/localRepository.js | 26 +++++++++++-------- app/src/lib/syncCoordinator.js | 4 +-- app/src/lib/syncCoordinator.test.js | 37 +++++++++++++++++++++++++++ 6 files changed, 75 insertions(+), 17 deletions(-) diff --git a/app/src/components/Calculator.jsx b/app/src/components/Calculator.jsx index 4ae0109..49f6e16 100644 --- a/app/src/components/Calculator.jsx +++ b/app/src/components/Calculator.jsx @@ -237,7 +237,7 @@ const NO_YIELDS = []; const Calculator = () => { const { user, getAuthHeaders } = useAuth(); // Custom yields kept on this device and synced when there is signal, so they work offline too - const { customYields, dataLoaded, retrySync } = useData(); + const { customYields, refreshCustomYields } = useData(); const [mode, setMode] = useState('cost'); const [targetWeight, setTargetWeight] = useState(''); const [species, setSpecies] = useState(''); @@ -298,8 +298,8 @@ const Calculator = () => { // The provider pulls once when it loads; opening the calculator again pulls too, so a yield // edited on another device shows up here as it did when this page fetched its own copy useEffect(() => { - if (dataLoaded) retrySync(); - }, [dataLoaded, retrySync]); + refreshCustomYields(); + }, [refreshCustomYields]); // Only a signed-in person's own yields: the guest scope can hold records no one here owns const myYields = user ? customYields : NO_YIELDS; diff --git a/app/src/components/DataManagement.jsx b/app/src/components/DataManagement.jsx index e6bbe3a..b2c44a9 100644 --- a/app/src/components/DataManagement.jsx +++ b/app/src/components/DataManagement.jsx @@ -61,12 +61,18 @@ const DataManagement = () => { e.preventDefault(); try { if (editingId) { - await updateYield(editingId, { + const updated = await updateYield(editingId, { species: formData.species, product: formData.product, yield: parseFloat(formData.yield), source: formData.source, }); + if (!updated) { + // Deleted on another device while open here: keep what was typed and let them add it back + setEditingId(null); + setStatus({ type: 'error', message: 'This yield was deleted on another device. Save to add it back.' }); + return; + } setStatus({ type: 'success', message: 'Updated successfully!' }); } else { await addYield({ diff --git a/app/src/context/DataContext.jsx b/app/src/context/DataContext.jsx index b6da423..31992a5 100644 --- a/app/src/context/DataContext.jsx +++ b/app/src/context/DataContext.jsx @@ -121,6 +121,7 @@ export function DataProvider({ children }) { // Without on-device storage there is nothing for the sync to fill, so read the account's custom // yields from the server instead and hold them in memory, tagged with the account they belong to. const [serverYields, setServerYields] = useState({ uid: null, rows: [] }); + const [serverYieldsWanted, setServerYieldsWanted] = useState(0); // bumped to fetch them again useEffect(() => { if (!storageFailed || !uid || !isOnline) return; let cancelled = false; @@ -130,7 +131,7 @@ export function DataProvider({ children }) { .then((rows) => { if (!cancelled && Array.isArray(rows)) setServerYields({ uid, rows }); }) .catch(() => {}); return () => { cancelled = true; }; - }, [storageFailed, uid, isOnline, getAuthHeaders]); + }, [storageFailed, uid, isOnline, getAuthHeaders, serverYieldsWanted]); // One-time legacy migration + recovery check. Runs once per session. useEffect(() => { @@ -648,6 +649,13 @@ export function DataProvider({ children }) { return triggerSync(); }, [user, triggerSync]); + // Get the account's latest custom yields: through the sync, or straight from the server when + // this device can't store them. The calculator calls this each time it opens. + const refreshCustomYields = useCallback(() => { + if (storageFailed) setServerYieldsWanted((n) => n + 1); + else if (dataLoaded) retrySync(); + }, [storageFailed, dataLoaded, retrySync]); + // Gate account data so consumers never see the previous scope's records // during the render cycle between a uid change and the clearing effect. const scopeReady = loadedScopeRef.current === scope; @@ -670,6 +678,7 @@ export function DataProvider({ children }) { removeYield, updateCustomSpecies, retrySync, + refreshCustomYields, signOut, requestPublish, confirmPublish, diff --git a/app/src/lib/localRepository.js b/app/src/lib/localRepository.js index f8f566e..8bb0efb 100644 --- a/app/src/lib/localRepository.js +++ b/app/src/lib/localRepository.js @@ -38,15 +38,15 @@ function now() { return new Date().toISOString(); } -// Compares by value: the server sends yield as a decimal string ("48.00") and may include -// fields a record made on this device never set (is_shared). -function sameYieldValues(local, server) { +// Compares by value: the server sends yield as a decimal string ("48.00"), and a record made on +// this device may leave out fields the server fills in (source, is_shared). +function sameYieldValues(a, b) { return ( - local.species === server.species && - local.product === server.product && - Number(local.yield) === Number(server.yield) && - (local.source || 'User Input') === server.source && - Boolean(local.is_shared) === Boolean(server.is_shared) + a.species === b.species && + a.product === b.product && + Number(a.yield) === Number(b.yield) && + (a.source || 'User Input') === (b.source || 'User Input') && + Boolean(a.is_shared) === Boolean(b.is_shared) ); } @@ -497,13 +497,19 @@ class LocalRepository { }); } - async markYieldSynced(id, serverId, serverRevision) { + // `pushed` is the record as it was sent. If it was edited or deleted again while the request was + // out, only the server ids are taken and the newer change stays queued for the next push. + async markYieldSynced(id, serverId, serverRevision, pushed) { const key = idbKey(this._scope, 'yields'); return this._withLock(key, async () => { const all = (await this._get(key)) || []; const idx = all.findIndex((y) => y.id === id); if (idx === -1) return; - all[idx] = { ...all[idx], syncStatus: 'synced', serverId, serverRevision, updatedAt: now() }; + const rec = all[idx]; + const changedSince = pushed && (rec.syncStatus !== pushed.syncStatus || !sameYieldValues(rec, pushed)); + all[idx] = changedSince + ? { ...rec, serverId, serverRevision } + : { ...rec, syncStatus: 'synced', serverId, serverRevision, updatedAt: now() }; await this._set(key, all); }); } diff --git a/app/src/lib/syncCoordinator.js b/app/src/lib/syncCoordinator.js index 204c8fd..b56d171 100644 --- a/app/src/lib/syncCoordinator.js +++ b/app/src/lib/syncCoordinator.js @@ -170,7 +170,7 @@ export function createSyncCoordinator(repo, client = defaultApiClient, telemetry if (res.ok) { const data = await res.json(); if (yld.serverId) { - await repo.markYieldSynced(yld.id, data.id ?? yld.serverId, data.revision); + await repo.markYieldSynced(yld.id, data.id ?? yld.serverId, data.revision, yld); } else { // Reconciling PUT: ensure server holds current field values after idempotent POST. const putRes = await client.updateUserDataRaw(data.id, { @@ -190,7 +190,7 @@ export function createSyncCoordinator(repo, client = defaultApiClient, telemetry continue; } const finalRevision = (await putRes.json()).revision; - await repo.markYieldSynced(yld.id, data.id, finalRevision); + await repo.markYieldSynced(yld.id, data.id, finalRevision, yld); } stats.pushed++; } else { diff --git a/app/src/lib/syncCoordinator.test.js b/app/src/lib/syncCoordinator.test.js index 1016106..8114e08 100644 --- a/app/src/lib/syncCoordinator.test.js +++ b/app/src/lib/syncCoordinator.test.js @@ -168,6 +168,43 @@ describe('createSyncCoordinator', () => { ); }); + it('keeps an edit made while the earlier push of the same yield was in flight', async () => { + const yld = await repo.addYield({ species: 'Cod', product: 'Fillet', yield: 55 }); + await repo.markYieldSynced(yld.id, 'srv-race', 1); + await repo.updateYield(yld.id, { yield: 58 }); + client.updateUserDataRaw = vi.fn(async () => { + await repo.updateYield(yld.id, { yield: 60 }); // edited again while the PUT is out + return fakeRes({ body: { id: 'srv-race', revision: 2 } }); + }); + client.listUserDataRaw = vi.fn(async () => + fakeRes({ body: [{ id: 'srv-race', revision: 2, species: 'Cod', product: 'Fillet', yield: 58 }] }) + ); + + await createSyncCoordinator(repo, client).sync(AUTH_USER); + + const [rec] = await repo.getYields(); + expect(rec).toMatchObject({ yield: 60, syncStatus: 'local', serverRevision: 2 }); + }); + + it('keeps a delete made while the earlier push of the same yield was in flight', async () => { + const yld = await repo.addYield({ species: 'Cod', product: 'Fillet', yield: 55 }); + await repo.markYieldSynced(yld.id, 'srv-race', 1); + await repo.updateYield(yld.id, { yield: 58 }); + client.updateUserDataRaw = vi.fn(async () => { + await repo.removeYield(yld.id); // deleted while the PUT is out + return fakeRes({ body: { id: 'srv-race', revision: 2 } }); + }); + client.listUserDataRaw = vi.fn(async () => + fakeRes({ body: [{ id: 'srv-race', revision: 2, species: 'Cod', product: 'Fillet', yield: 58 }] }) + ); + + await createSyncCoordinator(repo, client).sync(AUTH_USER); + + expect(await repo.getYields()).toHaveLength(0); + const pending = await repo.getPendingSync(); + expect(pending.yields[0]).toMatchObject({ syncStatus: 'pending-delete', serverRevision: 2 }); + }); + it('tracks a 409 conflict: increments conflicts (not errors), moves record to conflicted state', async () => { const yld = await repo.addYield({ species: 'Sole', product: 'Fillet', yield: 42 }); await repo.markYieldSynced(yld.id, 'srv-yld-2', 1);