From 417ae9f9ef6712a23a00f10bb2039a9e0d31f8ce Mon Sep 17 00:00:00 2001 From: Larry Lobsterman Date: Fri, 14 Aug 2026 01:16:28 -0400 Subject: [PATCH] feat(#96): remove legacy saved-data persistence bypasses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Delete orphaned syncEngine.js (replaced by syncCoordinator.js) and its tests. Strip all legacy sync functions from localStore.js — only getCustomSpecies/setCustomSpecies remain for the device-local species path. Remove the lifecycleFlag and its guard branches in DataContext.jsx; the identity-scoped lifecycle is now the single supported read/write path. Co-Authored-By: Claude Sonnet 4.6 --- app/src/context/DataContext.jsx | 10 - app/src/lib/apiClient.js | 6 +- app/src/lib/lifecycleFlag.js | 51 --- app/src/lib/lifecycleFlag.test.js | 90 ----- app/src/lib/localStore.js | 166 --------- app/src/lib/localStore.test.js | 241 -------------- app/src/lib/syncEngine.js | 200 ----------- app/src/lib/syncEngine.test.js | 536 ------------------------------ 8 files changed, 3 insertions(+), 1297 deletions(-) delete mode 100644 app/src/lib/lifecycleFlag.js delete mode 100644 app/src/lib/lifecycleFlag.test.js delete mode 100644 app/src/lib/localStore.test.js delete mode 100644 app/src/lib/syncEngine.js delete mode 100644 app/src/lib/syncEngine.test.js diff --git a/app/src/context/DataContext.jsx b/app/src/context/DataContext.jsx index 282c540..866d36f 100644 --- a/app/src/context/DataContext.jsx +++ b/app/src/context/DataContext.jsx @@ -12,15 +12,12 @@ import ConflictResolutionModal from '../components/ConflictResolutionModal'; import PreviewPublishModal from '../components/PreviewPublishModal'; import RecoveryModal from '../components/RecoveryModal'; import { apiClient } from '../lib/apiClient'; -import { isLifecycleEnabled } from '../lib/lifecycleFlag'; import { trackGuestAdoption, trackPendingAge } from '../lib/lifecycleTelemetry'; const DataContext = createContext(null); export function DataProvider({ children }) { const { user, logout } = useAuth(); - // Evaluated once at mount; rollback by setting localStorage lifecycle_override=false. - const lifecycleEnabledRef = useRef(isLifecycleEnabled()); const [savedCalcs, setSavedCalcs] = useState([]); const [customYields, setCustomYields] = useState([]); const [customSpecies, setCustomSpeciesState] = useState({}); @@ -85,13 +82,7 @@ export function DataProvider({ children }) { const coordinator = useMemo(() => createSyncCoordinator(repo), [repo]); // Load from IndexedDB when scope changes. - // When lifecycle is disabled (emergency rollback), skip IndexedDB — data loads on sync. useEffect(() => { - if (!lifecycleEnabledRef.current) { - loadedScopeRef.current = scope; - setDataLoaded(true); - return; - } let cancelled = false; const loadingScope = scope; loadedScopeRef.current = null; @@ -184,7 +175,6 @@ export function DataProvider({ children }) { }, [user]); const triggerSync = useCallback(async () => { - if (!lifecycleEnabledRef.current) return; if (!hasAuthCredential(user) || !navigator.onLine) return; if (signingOutRef.current) return; const gen = syncGenRef.current; diff --git a/app/src/lib/apiClient.js b/app/src/lib/apiClient.js index 4112242..bde3650 100644 --- a/app/src/lib/apiClient.js +++ b/app/src/lib/apiClient.js @@ -27,7 +27,7 @@ * listUserDataRaw(extraHeaders) → Promise * * Methods suffixed *Raw return the raw Response so callers can inspect - * status codes (e.g. syncEngine needs to detect 401 and 404 without throwing). + * status codes (e.g. syncCoordinator needs to detect 401 and 404 without throwing). */ // --------------------------------------------------------------------------- @@ -333,9 +333,9 @@ export function createApiClient(options = {}) { } // --------------------- - // Sync-engine raw API + // Sync coordinator raw API // --------------------- - // These return the raw Response so syncEngine can inspect status codes + // These return the raw Response so syncCoordinator can inspect status codes // (401 to break the loop, 404 as an acceptable delete outcome). /** diff --git a/app/src/lib/lifecycleFlag.js b/app/src/lib/lifecycleFlag.js deleted file mode 100644 index 013ccd9..0000000 --- a/app/src/lib/lifecycleFlag.js +++ /dev/null @@ -1,51 +0,0 @@ -/** - * Feature flag controlling whether the identity-scoped lifecycle - * (LocalRepository + SyncCoordinator) is active for the current client. - * - * Precedence (highest to lowest): - * 1. localStorage override — for internal testing / emergency rollback - * 2. VITE_LIFECYCLE_ENABLED — deployment-time flag - * 3. default: true — lifecycle on by default - * - * Rollback: set localStorage.setItem('lifecycle_override', 'false') or - * redeploy with VITE_LIFECYCLE_ENABLED=false. - */ - -const LS_OVERRIDE_KEY = 'lifecycle_override'; - -export function isLifecycleEnabled() { - try { - const override = localStorage.getItem(LS_OVERRIDE_KEY); - if (override !== null) return override !== 'false'; - } catch { - // localStorage unavailable (SSR, privacy mode) — fall through - } - - const envFlag = import.meta.env.VITE_LIFECYCLE_ENABLED; - if (envFlag !== undefined) return envFlag !== 'false'; - - return true; -} - -/** - * Force-enable the lifecycle for internal users regardless of env flag. - * Does NOT override a 'false' localStorage value — a deliberate rollback wins. - */ -export function enableLifecycleForSession() { - try { - const existing = localStorage.getItem(LS_OVERRIDE_KEY); - if (existing === 'false') return; // respect rollback - localStorage.setItem(LS_OVERRIDE_KEY, 'true'); - } catch { - // ignore - } -} - -/** Emergency rollback: disable lifecycle for this browser session. */ -export function disableLifecycleForSession() { - try { - localStorage.setItem(LS_OVERRIDE_KEY, 'false'); - } catch { - // ignore - } -} diff --git a/app/src/lib/lifecycleFlag.test.js b/app/src/lib/lifecycleFlag.test.js deleted file mode 100644 index bf91b45..0000000 --- a/app/src/lib/lifecycleFlag.test.js +++ /dev/null @@ -1,90 +0,0 @@ -import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; - -// Provide a localStorage stub before importing the module. -const lsStore = {}; -const localStorageMock = { - getItem: vi.fn((k) => lsStore[k] ?? null), - setItem: vi.fn((k, v) => { lsStore[k] = String(v); }), - removeItem: vi.fn((k) => { delete lsStore[k]; }), - clear: vi.fn(() => { Object.keys(lsStore).forEach((k) => delete lsStore[k]); }), -}; -Object.defineProperty(globalThis, 'localStorage', { value: localStorageMock, configurable: true }); - -// Import after stubbing so the module's try/catch sees the stub. -const { isLifecycleEnabled, enableLifecycleForSession, disableLifecycleForSession } = - await import('./lifecycleFlag'); - -describe('lifecycleFlag', () => { - beforeEach(() => { - localStorageMock.clear(); - vi.clearAllMocks(); - vi.unstubAllEnvs(); - }); - - afterEach(() => { - vi.unstubAllEnvs(); - }); - - describe('isLifecycleEnabled', () => { - it('defaults to true when no flag or override is set', () => { - expect(isLifecycleEnabled()).toBe(true); - }); - - it('returns true when VITE_LIFECYCLE_ENABLED=true', () => { - vi.stubEnv('VITE_LIFECYCLE_ENABLED', 'true'); - expect(isLifecycleEnabled()).toBe(true); - }); - - it('returns false when VITE_LIFECYCLE_ENABLED=false', () => { - vi.stubEnv('VITE_LIFECYCLE_ENABLED', 'false'); - expect(isLifecycleEnabled()).toBe(false); - }); - - it('localStorage override=true beats env flag=false', () => { - vi.stubEnv('VITE_LIFECYCLE_ENABLED', 'false'); - localStorageMock.setItem('lifecycle_override', 'true'); - expect(isLifecycleEnabled()).toBe(true); - }); - - it('localStorage override=false beats env flag=true', () => { - vi.stubEnv('VITE_LIFECYCLE_ENABLED', 'true'); - localStorageMock.setItem('lifecycle_override', 'false'); - expect(isLifecycleEnabled()).toBe(false); - }); - - it('localStorage override=false beats default true', () => { - localStorageMock.setItem('lifecycle_override', 'false'); - expect(isLifecycleEnabled()).toBe(false); - }); - }); - - describe('enableLifecycleForSession', () => { - it('sets override to true', () => { - enableLifecycleForSession(); - expect(localStorageMock.setItem).toHaveBeenCalledWith('lifecycle_override', 'true'); - }); - - it('does NOT override a deliberate rollback (false)', () => { - localStorageMock.setItem('lifecycle_override', 'false'); - vi.clearAllMocks(); - enableLifecycleForSession(); - // setItem should NOT have been called again after the guard - expect(localStorageMock.setItem).not.toHaveBeenCalled(); - }); - }); - - describe('disableLifecycleForSession', () => { - it('sets override to false', () => { - disableLifecycleForSession(); - expect(localStorageMock.setItem).toHaveBeenCalledWith('lifecycle_override', 'false'); - }); - - it('overrides a previous enable', () => { - enableLifecycleForSession(); - disableLifecycleForSession(); - // Last call is the disable - const calls = localStorageMock.setItem.mock.calls; - expect(calls[calls.length - 1]).toEqual(['lifecycle_override', 'false']); - }); - }); -}); diff --git a/app/src/lib/localStore.js b/app/src/lib/localStore.js index d127920..7c41ddc 100644 --- a/app/src/lib/localStore.js +++ b/app/src/lib/localStore.js @@ -1,109 +1,7 @@ import { get, set } from 'idb-keyval'; -// Keys for our three stores -const SAVED_CALCS_KEY = 'fish-calc-saved-calcs'; -const CUSTOM_YIELDS_KEY = 'fish-calc-custom-yields'; const CUSTOM_SPECIES_KEY = 'fish-calc-custom-species'; -// --- Generic synced store factory --- - -function createSyncedStore(storageKey) { - const getAll = async () => (await get(storageKey)) || []; - - const add = async (item) => { - const items = await getAll(); - const newItem = { - ...item, - id: item.id || crypto.randomUUID(), - syncStatus: 'local', - updatedAt: new Date().toISOString(), - createdAt: item.createdAt || new Date().toISOString(), - }; - items.push(newItem); - await set(storageKey, items); - return newItem; - }; - - const update = async (id, data) => { - const items = await getAll(); - const index = items.findIndex((i) => i.id === id); - if (index === -1) return null; - items[index] = { - ...items[index], - ...data, - syncStatus: items[index].syncStatus === 'synced' ? 'local' : items[index].syncStatus, - updatedAt: new Date().toISOString(), - }; - await set(storageKey, items); - return items[index]; - }; - - const remove = async (id) => { - const items = await getAll(); - const item = items.find((i) => i.id === id); - if (!item) return; - - if (item.syncStatus === 'synced') { - item.syncStatus = 'pending-delete'; - item.updatedAt = new Date().toISOString(); - await set(storageKey, items); - } else { - await set(storageKey, items.filter((i) => i.id !== id)); - } - }; - - const markSynced = async (id, serverId, serverRevision) => { - const items = await getAll(); - const item = items.find((i) => i.id === id); - if (item) { - item.syncStatus = 'synced'; - if (serverId) item.serverId = serverId; - if (serverRevision != null) item.serverRevision = serverRevision; - await set(storageKey, items); - } - }; - - const removeSyncedDelete = async (id) => { - const items = await getAll(); - await set(storageKey, items.filter((i) => i.id !== id)); - }; - - return { getAll, add, update, remove, markSynced, removeSyncedDelete }; -} - -// --- Store instances --- - -const calcsStore = createSyncedStore(SAVED_CALCS_KEY); -const yieldsStore = createSyncedStore(CUSTOM_YIELDS_KEY); - -// --- Named exports for backward compatibility --- - -export const getSavedCalcs = calcsStore.getAll; -export const addSavedCalc = calcsStore.add; -export const deleteSavedCalc = calcsStore.remove; - -export const getCustomYields = yieldsStore.getAll; -export const addCustomYield = yieldsStore.add; -export const updateCustomYield = yieldsStore.update; -export const deleteCustomYield = yieldsStore.remove; - -// --- Sync helpers (typed exports instead of string dispatch) --- - -export const markCalcSynced = calcsStore.markSynced; -export const markYieldSynced = yieldsStore.markSynced; -export const removeCalcSyncedDelete = calcsStore.removeSyncedDelete; -export const removeYieldSyncedDelete = yieldsStore.removeSyncedDelete; - -export async function getAllPendingSync() { - const [calcs, yields] = await Promise.all([calcsStore.getAll(), yieldsStore.getAll()]); - return { - calcs: calcs.filter((c) => c.syncStatus === 'local' || c.syncStatus === 'pending-delete'), - yields: yields.filter((y) => y.syncStatus === 'local' || y.syncStatus === 'pending-delete'), - }; -} - -// --- Custom Species (different data model, no sync) --- - export async function getCustomSpecies() { return (await get(CUSTOM_SPECIES_KEY)) || {}; } @@ -111,67 +9,3 @@ export async function getCustomSpecies() { export async function setCustomSpecies(data) { await set(CUSTOM_SPECIES_KEY, data); } - -// --- Bulk merge operations (keep separate due to different field mappings) --- - -export async function mergeSyncedCalcs(serverCalcs) { - const local = await calcsStore.getAll(); - // Stringify all local serverIds for type-safe comparison (server returns ints, - // local may store them as numbers or strings depending on how they were saved). - // Also track items that are pending-delete so we never resurrect a tombstone. - const localServerIds = new Set(local.filter((c) => c.serverId).map((c) => String(c.serverId))); - const pendingDeleteIds = new Set( - local.filter((c) => c.syncStatus === 'pending-delete' && c.serverId).map((c) => String(c.serverId)) - ); - - for (const sc of serverCalcs) { - const sid = String(sc.id); - // Skip if already tracked locally (any syncStatus) — especially pending-delete - // tombstones, which must not be resurrected by a pull from the server. - if (localServerIds.has(sid) || pendingDeleteIds.has(sid)) continue; - - local.push({ - ...sc, - serverId: sc.id, - id: crypto.randomUUID(), - syncStatus: 'synced', - updatedAt: sc.created_at || new Date().toISOString(), - createdAt: sc.created_at || new Date().toISOString(), - }); - } - await set(SAVED_CALCS_KEY, local); -} - -export async function mergeSyncedYields(serverYields) { - const local = await yieldsStore.getAll(); - - for (const sy of serverYields) { - const existing = local.find((y) => String(y.serverId) === String(sy.id)); - if (existing?.syncStatus === 'synced') { - // Refresh fields and revision for synced records so future update/delete - // sends the correct expected_revision. Leave pending-delete and local-edit - // records alone — they represent in-flight changes and must not be clobbered. - Object.assign(existing, { - species: sy.species, - product: sy.product, - yield: sy.yield, - source: sy.source || 'User Input', - serverRevision: sy.revision ?? null, - }); - } else if (!existing) { - local.push({ - species: sy.species, - product: sy.product, - yield: sy.yield, - source: sy.source || 'User Input', - serverId: sy.id, - serverRevision: sy.revision ?? null, - id: crypto.randomUUID(), - syncStatus: 'synced', - updatedAt: new Date().toISOString(), - }); - } - // else: pending-delete tombstone or local edit — leave unchanged - } - await set(CUSTOM_YIELDS_KEY, local); -} diff --git a/app/src/lib/localStore.test.js b/app/src/lib/localStore.test.js deleted file mode 100644 index c9e29f2..0000000 --- a/app/src/lib/localStore.test.js +++ /dev/null @@ -1,241 +0,0 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { get, set } from 'idb-keyval'; -import { mergeSyncedYields, mergeSyncedCalcs, markYieldSynced } from './localStore'; - -vi.mock('idb-keyval', () => ({ - get: vi.fn(async () => []), - set: vi.fn(async () => undefined), -})); - -describe('mergeSyncedYields', () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - - it('stores pulled custom yields using the server yield field', async () => { - await mergeSyncedYields([ - { - id: 12, - species: 'Pacific Halibut', - product: 'Round → Skinless Fillet', - yield: 48, - source: 'User Input', - }, - ]); - - expect(set).toHaveBeenCalledWith( - 'fish-calc-custom-yields', - expect.arrayContaining([ - expect.objectContaining({ - species: 'Pacific Halibut', - product: 'Round → Skinless Fillet', - yield: 48, - source: 'User Input', - serverId: 12, - syncStatus: 'synced', - }), - ]) - ); - }); - - it('stores serverRevision from pulled yields so revision guards work on update/delete', async () => { - await mergeSyncedYields([ - { id: 5, species: 'Cod', product: 'Fillet', yield: 55, source: 'User Input', revision: 3 }, - ]); - - const saved = set.mock.calls[0][1]; - const entry = saved.find((y) => String(y.serverId) === '5'); - expect(entry).toBeDefined(); - expect(entry.serverRevision).toBe(3); - }); - - it('does not resurrect a yield that is pending-delete (tombstoned locally)', async () => { - // Local store has a pending-delete tombstone for server id 12 - get.mockResolvedValue([ - { - id: 'local-uuid', - serverId: 12, - syncStatus: 'pending-delete', - species: 'Pacific Halibut', - product: 'Round → Skinless Fillet', - yield: 48, - }, - ]); - - // Server returns the same item (delete not yet confirmed server-side) - await mergeSyncedYields([ - { id: 12, species: 'Pacific Halibut', product: 'Round → Skinless Fillet', yield: 48 }, - ]); - - // The tombstoned item must NOT gain a duplicate in the store - const saved = set.mock.calls[0][1]; - const copies = saved.filter((i) => String(i.serverId) === '12'); - expect(copies).toHaveLength(1); - expect(copies[0].syncStatus).toBe('pending-delete'); - }); - - it('refreshes serverRevision for an existing synced yield on pull', async () => { - get.mockResolvedValue([ - { - id: 'local-uuid', - serverId: 7, - syncStatus: 'synced', - species: 'Cod', - product: 'Fillet', - yield: 55, - serverRevision: 1, - }, - ]); - - await mergeSyncedYields([ - { id: 7, species: 'Cod', product: 'Fillet', yield: 55, source: 'User Input', revision: 5 }, - ]); - - const saved = set.mock.calls[0][1]; - const copies = saved.filter((i) => String(i.serverId) === '7'); - expect(copies).toHaveLength(1); - expect(copies[0].serverRevision).toBe(5); - expect(copies[0].syncStatus).toBe('synced'); - }); - - it('does not overwrite a local-edit record during a pull', async () => { - get.mockResolvedValue([ - { - id: 'local-uuid', - serverId: 7, - syncStatus: 'local', - species: 'Cod', - product: 'Fillet', - yield: 60, - serverRevision: 1, - }, - ]); - - await mergeSyncedYields([ - { id: 7, species: 'Cod', product: 'Fillet', yield: 55, source: 'User Input', revision: 1 }, - ]); - - const saved = set.mock.calls[0][1]; - const entry = saved.find((i) => String(i.serverId) === '7'); - expect(entry.syncStatus).toBe('local'); - expect(entry.yield).toBe(60); - }); -}); - -describe('markYieldSynced', () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - - it('stores serverRevision so the sync engine can send it on future update/delete', async () => { - get.mockResolvedValue([ - { id: 'local-uuid', syncStatus: 'local', species: 'Salmon', product: 'Fillet', yield: 60 }, - ]); - - await markYieldSynced('local-uuid', 99, 2); - - const saved = set.mock.calls[0][1]; - const entry = saved.find((y) => y.id === 'local-uuid'); - expect(entry.serverId).toBe(99); - expect(entry.serverRevision).toBe(2); - expect(entry.syncStatus).toBe('synced'); - }); -}); - -describe('mergeSyncedCalcs', () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - - it('adds a new server calc that is not in the local store', async () => { - get.mockResolvedValue([]); - - await mergeSyncedCalcs([ - { - id: 7, - name: 'Cod Fillet', - species: 'Pacific Cod', - product: 'Round → Skinless Fillets', - cost: 4.5, - yield: 42, - result: 10.71, - created_at: '2026-06-01T00:00:00Z', - }, - ]); - - expect(set).toHaveBeenCalledWith( - 'fish-calc-saved-calcs', - expect.arrayContaining([ - expect.objectContaining({ - serverId: 7, - syncStatus: 'synced', - species: 'Pacific Cod', - }), - ]) - ); - }); - - it('does not duplicate a calc already tracked by serverId (number vs string)', async () => { - // serverId stored as number (common when restored from server JSON) - get.mockResolvedValue([ - { - id: 'local-uuid', - serverId: 7, - syncStatus: 'synced', - species: 'Pacific Cod', - product: 'Round → Skinless Fillets', - yield: 42, - result: 10.71, - }, - ]); - - // Server returns same id (integer from JSON) - await mergeSyncedCalcs([ - { id: 7, species: 'Pacific Cod', product: 'Round → Skinless Fillets', yield: 42, result: 10.71 }, - ]); - - const saved = set.mock.calls[0][1]; - const copies = saved.filter((i) => String(i.serverId) === '7'); - expect(copies).toHaveLength(1); - }); - - it('does not resurrect a calc that is pending-delete (tombstoned locally)', async () => { - // Local store has a pending-delete tombstone for server id 42 - get.mockResolvedValue([ - { - id: 'local-uuid', - serverId: 42, - syncStatus: 'pending-delete', - species: 'Pink Salmon', - product: 'Round → Skinless Fillet', - yield: 38, - result: 6.58, - }, - ]); - - // Server still returns the item (delete push hasn't happened yet) - await mergeSyncedCalcs([ - { id: 42, species: 'Pink Salmon', product: 'Round → Skinless Fillet', yield: 38, result: 6.58 }, - ]); - - // Store must still contain exactly one entry for server id 42, still tombstoned - const saved = set.mock.calls[0][1]; - const copies = saved.filter((i) => String(i.serverId) === '42'); - expect(copies).toHaveLength(1); - expect(copies[0].syncStatus).toBe('pending-delete'); - }); - - it('second device pull: does not re-add a calc whose serverId has no local record', async () => { - // Fresh device — local store is empty - get.mockResolvedValue([]); - - await mergeSyncedCalcs([ - { id: 99, species: 'Chinook Salmon', product: 'Round → D/H-On', yield: 91, result: 2.2 }, - ]); - - const saved = set.mock.calls[0][1]; - expect(saved).toHaveLength(1); - expect(saved[0].serverId).toBe(99); - expect(saved[0].syncStatus).toBe('synced'); - }); -}); diff --git a/app/src/lib/syncEngine.js b/app/src/lib/syncEngine.js deleted file mode 100644 index 791175e..0000000 --- a/app/src/lib/syncEngine.js +++ /dev/null @@ -1,200 +0,0 @@ -import { getAuthHeaders, hasAuthCredential } from './authHeaders'; -import { apiClient } from './apiClient'; -import { - getAllPendingSync, - markCalcSynced, - markYieldSynced, - removeCalcSyncedDelete, - removeYieldSyncedDelete, - mergeSyncedCalcs, - mergeSyncedYields, -} from './localStore'; - -/** - * Sync all pending local changes to the server, then pull latest. - * @param {Object} user - Current authenticated user session - * @returns {Promise<{pushed: number, pulled: number, errors: number, errorDetails: Array}>} - */ -export async function syncAll(user) { - const stats = { pushed: 0, pulled: 0, errors: 0, errorDetails: [] }; - if (!hasAuthCredential(user)) return stats; - - const headers = await getAuthHeaders(user, { 'Content-Type': 'application/json' }); - if (!headers.Authorization) { - stats.errors++; - stats.errorDetails.push({ - type: 'auth', - isAuthError: true, - message: 'Missing authentication headers', - }); - return stats; - } - - // --- Push local changes --- - const pending = await getAllPendingSync(); - - // Push new/updated calcs - for (const calc of pending.calcs.filter((c) => c.syncStatus === 'local')) { - try { - const res = await apiClient.saveCalcRaw({ - name: calc.name || '', - species: calc.species, - product: calc.product, - cost: calc.cost, - yield: calc.yield, - result: calc.result, - // Stable local UUID used as clientId for idempotent retry. - client_id: calc.id, - }, headers); - if (res.ok) { - const data = await res.json(); - await markCalcSynced(calc.id, data.id); - stats.pushed++; - } else { - stats.errors++; - stats.errorDetails.push({ type: 'push-calc', id: calc.id, status: res.status, isAuthError: res.status === 401 }); - if (res.status === 401) break; - } - } catch (err) { - stats.errors++; - stats.errorDetails.push({ type: 'push-calc', id: calc.id, message: err.message }); - } - } - - // Push deleted calcs - for (const calc of pending.calcs.filter((c) => c.syncStatus === 'pending-delete')) { - try { - if (calc.serverId) { - const res = await apiClient.deleteCalcRaw(calc.serverId, headers); - if (res.ok || res.status === 404) { - await removeCalcSyncedDelete(calc.id); - stats.pushed++; - } else { - stats.errors++; - stats.errorDetails.push({ type: 'delete-calc', id: calc.id, status: res.status, isAuthError: res.status === 401 }); - if (res.status === 401) break; - } - } else { - await removeCalcSyncedDelete(calc.id); - stats.pushed++; - } - } catch (err) { - stats.errors++; - stats.errorDetails.push({ type: 'delete-calc', id: calc.id, message: err.message }); - } - } - - // Push new/updated yields - for (const yld of pending.yields.filter((y) => y.syncStatus === 'local')) { - try { - let res; - if (yld.serverId) { - // Edited synced yield — use PUT so the server applies the field changes. - // A POST with the same client_id would return the existing row unchanged. - res = await apiClient.updateUserDataRaw(yld.serverId, { - species: yld.species, - product: yld.product, - yield: yld.yield, - source: yld.source || 'User Input', - expected_revision: yld.serverRevision, - }, headers); - } else { - // New yield — use POST with idempotent client_id. - res = await apiClient.createUserDataRaw({ - species: yld.species, - product: yld.product, - yield: yld.yield, - source: yld.source || 'User Input', - client_id: yld.id, - }, headers); - } - if (res.ok) { - const data = await res.json(); - if (yld.serverId) { - // Edited synced yield — PUT already applied the current fields. - await markYieldSynced(yld.id, data.id ?? yld.serverId, data.revision); - } else { - // New yield — the POST may have been an idempotent retry that returned - // the original row while the user had already edited the local fields. - // A reconciling PUT ensures the server always holds the current values. - const putRes = await apiClient.updateUserDataRaw(data.id, { - species: yld.species, - product: yld.product, - yield: yld.yield, - source: yld.source || 'User Input', - expected_revision: data.revision, - }, headers); - if (!putRes.ok) { - // Reconciling PUT failed — leave yield pending so it is retried. - stats.errors++; - stats.errorDetails.push({ type: 'push-yield', id: yld.id, status: putRes.status, isAuthError: putRes.status === 401 }); - if (putRes.status === 401) break; - continue; - } - const finalRevision = (await putRes.json()).revision; - await markYieldSynced(yld.id, data.id, finalRevision); - } - stats.pushed++; - } else { - stats.errors++; - stats.errorDetails.push({ type: 'push-yield', id: yld.id, status: res.status, isAuthError: res.status === 401 }); - if (res.status === 401) break; - } - } catch (err) { - stats.errors++; - stats.errorDetails.push({ type: 'push-yield', id: yld.id, message: err.message }); - } - } - - // Push deleted yields - for (const yld of pending.yields.filter((y) => y.syncStatus === 'pending-delete')) { - try { - if (yld.serverId) { - const res = await apiClient.deleteUserDataRaw(yld.serverId, yld.serverRevision, headers); - if (res.ok || res.status === 404) { - await removeYieldSyncedDelete(yld.id); - stats.pushed++; - } else { - stats.errors++; - stats.errorDetails.push({ type: 'delete-yield', id: yld.id, status: res.status, isAuthError: res.status === 401 }); - if (res.status === 401) break; - } - } else { - await removeYieldSyncedDelete(yld.id); - stats.pushed++; - } - } catch (err) { - stats.errors++; - stats.errorDetails.push({ type: 'delete-yield', id: yld.id, message: err.message }); - } - } - - // --- Pull server data --- - try { - const res = await apiClient.listSavedCalcsRaw(headers); - if (res.ok) { - const serverCalcs = await res.json(); - if (Array.isArray(serverCalcs)) { - await mergeSyncedCalcs(serverCalcs); - stats.pulled += serverCalcs.length; - } - } - } catch { - // Pull errors are non-critical — local data is still intact - } - - try { - const res = await apiClient.listUserDataRaw(headers); - if (res.ok) { - const serverYields = await res.json(); - if (Array.isArray(serverYields)) { - await mergeSyncedYields(serverYields); - stats.pulled += serverYields.length; - } - } - } catch { - // Pull errors are non-critical — local data is still intact - } - - return stats; -} diff --git a/app/src/lib/syncEngine.test.js b/app/src/lib/syncEngine.test.js deleted file mode 100644 index e5a66ea..0000000 --- a/app/src/lib/syncEngine.test.js +++ /dev/null @@ -1,536 +0,0 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { syncAll } from './syncEngine'; -import { getAuthHeaders, hasAuthCredential } from './authHeaders'; -import { - getAllPendingSync, - markCalcSynced, - markYieldSynced, - removeCalcSyncedDelete, - mergeSyncedCalcs, - mergeSyncedYields, -} from './localStore'; - -vi.mock('./localStore', () => ({ - getAllPendingSync: vi.fn(), - markCalcSynced: vi.fn(), - markYieldSynced: vi.fn(), - removeCalcSyncedDelete: vi.fn(), - removeYieldSyncedDelete: vi.fn(), - mergeSyncedCalcs: vi.fn(), - mergeSyncedYields: vi.fn(), -})); - -vi.mock('./authHeaders', () => ({ - getAuthHeaders: vi.fn(), - hasAuthCredential: vi.fn(), -})); - -// Mock apiClient so tests control HTTP responses without a live network. -// Each *Raw method returns a fake Response; the non-raw methods aren't used -// by syncEngine so they're left as no-ops. -vi.mock('./apiClient', () => { - const saveCalcRaw = vi.fn(); - const deleteCalcRaw = vi.fn(); - const createUserDataRaw = vi.fn(); - const updateUserDataRaw = vi.fn(); - const deleteUserDataRaw = vi.fn(); - const listSavedCalcsRaw = vi.fn(); - const listUserDataRaw = vi.fn(); - return { - apiClient: { - saveCalcRaw, - deleteCalcRaw, - createUserDataRaw, - updateUserDataRaw, - deleteUserDataRaw, - listSavedCalcsRaw, - listUserDataRaw, - }, - }; -}); - -import { apiClient } from './apiClient'; - -// --------------------------------------------------------------------------- -// Helpers -// --------------------------------------------------------------------------- - -/** Build a fake Response-like object (same helper style as apiClient.test.js). */ -function fakeResponse({ ok, status, body }) { - return { - ok, - status, - json: async () => body, - text: async () => (typeof body === 'string' ? body : JSON.stringify(body)), - headers: { get: () => null }, - }; -} - -const passwordUser = { username: 'processor', authProvider: 'password' }; -const AUTH_HEADERS = { - 'Content-Type': 'application/json', - Authorization: 'Bearer jwt-token', -}; - -describe('syncAll', () => { - beforeEach(() => { - vi.clearAllMocks(); - hasAuthCredential.mockReturnValue(true); - getAuthHeaders.mockResolvedValue(AUTH_HEADERS); - - // Default: push succeeds with id, pull returns empty arrays - apiClient.saveCalcRaw.mockResolvedValue(fakeResponse({ ok: true, status: 201, body: { id: 'server-id' } })); - apiClient.deleteCalcRaw.mockResolvedValue(fakeResponse({ ok: true, status: 200, body: {} })); - apiClient.createUserDataRaw.mockResolvedValue(fakeResponse({ ok: true, status: 201, body: { id: 'server-id', revision: 1 } })); - apiClient.updateUserDataRaw.mockResolvedValue(fakeResponse({ ok: true, status: 200, body: { id: 'server-id', revision: 2 } })); - apiClient.deleteUserDataRaw.mockResolvedValue(fakeResponse({ ok: true, status: 200, body: {} })); - apiClient.listSavedCalcsRaw.mockResolvedValue(fakeResponse({ ok: true, status: 200, body: [] })); - apiClient.listUserDataRaw.mockResolvedValue(fakeResponse({ ok: true, status: 200, body: [] })); - }); - - it('sends saved calculations with the yield field the server accepts', async () => { - getAllPendingSync.mockResolvedValue({ - calcs: [ - { - id: 'local-calc-id', - syncStatus: 'local', - name: 'Cod Round to Fillet', - species: 'Pacific Cod', - product: 'Round → Skinless Fillets', - mode: 'cost', - cost: 4.5, - target_weight: 0, - yield: 42, - result: 10.71, - }, - ], - yields: [], - }); - - await syncAll(passwordUser); - - expect(apiClient.saveCalcRaw).toHaveBeenCalledTimes(1); - const [body] = apiClient.saveCalcRaw.mock.calls[0]; - expect(body).toMatchObject({ - species: 'Pacific Cod', - product: 'Round → Skinless Fillets', - yield: 42, - }); - expect(body).not.toHaveProperty('yield_value'); - expect(markCalcSynced).toHaveBeenCalledWith('local-calc-id', 'server-id'); - }); - - it('sends the local record id as client_id for idempotent retry', async () => { - getAllPendingSync.mockResolvedValue({ - calcs: [ - { - id: 'stable-uuid-123', - syncStatus: 'local', - name: 'Halibut Fillet', - species: 'Pacific Halibut', - product: 'Skinless Fillet', - cost: 3.5, - yield: 55, - result: 6.36, - }, - ], - yields: [], - }); - - await syncAll(passwordUser); - - const [body] = apiClient.saveCalcRaw.mock.calls[0]; - expect(body.client_id).toBe('stable-uuid-123'); - }); - - it('omits saved calculation fields that the remote schema does not store', async () => { - getAllPendingSync.mockResolvedValue({ - calcs: [ - { - id: 'local-weight-calc-id', - syncStatus: 'local', - name: 'Target halibut output', - species: 'Pacific Halibut', - product: 'Round → Skinless Fillet', - mode: 'weight', - cost: 0, - target_weight: 100, - yield: 48, - result: 208.33, - }, - ], - yields: [], - }); - - await syncAll(passwordUser); - - expect(apiClient.saveCalcRaw).toHaveBeenCalledTimes(1); - const [body] = apiClient.saveCalcRaw.mock.calls[0]; - expect(body).not.toHaveProperty('mode'); - expect(body).not.toHaveProperty('target_weight'); - }); - - it('sends the local record id as client_id for idempotent yield push', async () => { - getAllPendingSync.mockResolvedValue({ - calcs: [], - yields: [ - { - id: 'stable-yield-uuid', - syncStatus: 'local', - species: 'Pacific Halibut', - product: 'Round → Skinless Fillet', - yield: 48, - source: 'User Input', - }, - ], - }); - - await syncAll(passwordUser); - - expect(apiClient.createUserDataRaw).toHaveBeenCalledTimes(1); - const [body] = apiClient.createUserDataRaw.mock.calls[0]; - expect(body.client_id).toBe('stable-yield-uuid'); - }); - - it('sends custom yields with the yield field the server accepts', async () => { - getAllPendingSync.mockResolvedValue({ - calcs: [], - yields: [ - { - id: 'local-yield-id', - syncStatus: 'local', - species: 'Pacific Halibut', - product: 'Round → Skinless Fillet', - yield: 48, - source: 'User Input', - }, - ], - }); - - await syncAll(passwordUser); - - expect(apiClient.createUserDataRaw).toHaveBeenCalledTimes(1); - const [body] = apiClient.createUserDataRaw.mock.calls[0]; - expect(body).toMatchObject({ - species: 'Pacific Halibut', - product: 'Round → Skinless Fillet', - yield: 48, - source: 'User Input', - }); - expect(body).not.toHaveProperty('yield_percentage'); - // After POST a reconciling PUT is issued to ensure the server holds current - // local fields (guards against idempotent-retry lost-response edits). - expect(apiClient.updateUserDataRaw).toHaveBeenCalledTimes(1); - const [putServerId, putBody] = apiClient.updateUserDataRaw.mock.calls[0]; - expect(putServerId).toBe('server-id'); - expect(putBody).toMatchObject({ species: 'Pacific Halibut', yield: 48, expected_revision: 1 }); - // markYieldSynced uses the PUT response revision (2), not the POST revision (1). - expect(markYieldSynced).toHaveBeenCalledWith('local-yield-id', 'server-id', 2); - }); - - it('reconciles local edits after an idempotent POST retry (lost-response scenario)', async () => { - // Simulate: user edited yield between the original POST (response lost) and - // this retry. The POST returns the original row; the reconciling PUT must - // send the current edited fields so the server is up to date. - getAllPendingSync.mockResolvedValue({ - calcs: [], - yields: [ - { - id: 'edited-yield-id', - syncStatus: 'local', - // serverId is null because the original POST response was lost - species: 'Pacific Halibut', - product: 'Round → Skinless Fillet', - yield: 55, // edited from the original 48 - source: 'User Input', - }, - ], - }); - // POST returns existing row (idempotent — server still has original values) - apiClient.createUserDataRaw.mockResolvedValue( - fakeResponse({ ok: true, status: 200, body: { id: 'server-id', revision: 1 } }) - ); - // PUT applies the edited fields - apiClient.updateUserDataRaw.mockResolvedValue( - fakeResponse({ ok: true, status: 200, body: { id: 'server-id', revision: 2 } }) - ); - - await syncAll(passwordUser); - - expect(apiClient.createUserDataRaw).toHaveBeenCalledTimes(1); - expect(apiClient.updateUserDataRaw).toHaveBeenCalledTimes(1); - const [putServerId, putBody] = apiClient.updateUserDataRaw.mock.calls[0]; - expect(putServerId).toBe('server-id'); - expect(putBody).toMatchObject({ yield: 55, expected_revision: 1 }); - expect(markYieldSynced).toHaveBeenCalledWith('edited-yield-id', 'server-id', 2); - }); - - it('uses PUT for a synced yield edited locally (serverId set)', async () => { - getAllPendingSync.mockResolvedValue({ - calcs: [], - yields: [ - { - id: 'local-yield-id', - syncStatus: 'local', - serverId: 77, - serverRevision: 3, - species: 'Pacific Halibut', - product: 'Round → Skinless Fillet', - yield: 52, - source: 'User Input', - }, - ], - }); - - await syncAll(passwordUser); - - expect(apiClient.updateUserDataRaw).toHaveBeenCalledTimes(1); - expect(apiClient.createUserDataRaw).not.toHaveBeenCalled(); - const [serverId, body] = apiClient.updateUserDataRaw.mock.calls[0]; - expect(serverId).toBe(77); - expect(body).toMatchObject({ species: 'Pacific Halibut', yield: 52, expected_revision: 3 }); - expect(markYieldSynced).toHaveBeenCalledWith('local-yield-id', 'server-id', 2); - }); - - it('marks pushed records as synced only after successful server responses', async () => { - getAllPendingSync.mockResolvedValue({ - calcs: [ - { - id: 'local-calc-id', - syncStatus: 'local', - species: 'Pacific Cod', - product: 'Round → Skinless Fillets', - yield: 42, - result: 10.71, - }, - ], - yields: [ - { - id: 'local-yield-id', - syncStatus: 'local', - species: 'Pacific Halibut', - product: 'Round → Skinless Fillet', - yield: 48, - }, - ], - }); - - await syncAll(passwordUser); - - expect(markCalcSynced).toHaveBeenCalledWith('local-calc-id', 'server-id'); - // New yield sync always issues a reconciling PUT; markYieldSynced gets the - // PUT response revision (2) not the POST revision (1). - expect(markYieldSynced).toHaveBeenCalledWith('local-yield-id', 'server-id', 2); - }); - - it('uses shared auth headers so Firebase sessions can sync without a legacy JWT', async () => { - const firebaseUser = { username: 'firebase-user', authProvider: 'firebase', getIdToken: vi.fn() }; - const firebaseHeaders = { - 'Content-Type': 'application/json', - Authorization: 'Bearer firebase-id-token', - }; - getAuthHeaders.mockResolvedValue(firebaseHeaders); - getAllPendingSync.mockResolvedValue({ - calcs: [ - { - id: 'local-calc-id', - syncStatus: 'local', - species: 'Pacific Cod', - product: 'Round → Skinless Fillets', - yield: 42, - result: 10.71, - }, - ], - yields: [], - }); - - await syncAll(firebaseUser); - - expect(getAuthHeaders).toHaveBeenCalledWith(firebaseUser, { 'Content-Type': 'application/json' }); - // extraHeaders passed to saveCalcRaw must include the Firebase ID token - const [, extraHeaders] = apiClient.saveCalcRaw.mock.calls[0]; - expect(extraHeaders).toMatchObject({ - 'Content-Type': 'application/json', - Authorization: 'Bearer firebase-id-token', - }); - }); - - it('classifies auth failures without marking records as synced', async () => { - getAllPendingSync.mockResolvedValue({ - calcs: [ - { - id: 'local-calc-id', - syncStatus: 'local', - species: 'Pacific Cod', - product: 'Round → Skinless Fillets', - yield: 42, - result: 10.71, - }, - ], - yields: [], - }); - apiClient.saveCalcRaw.mockResolvedValue( - fakeResponse({ ok: false, status: 401, body: { error: 'Unauthorized' } }) - ); - - const stats = await syncAll(passwordUser); - - expect(stats.errors).toBe(1); - expect(stats.errorDetails).toContainEqual({ - type: 'push-calc', - id: 'local-calc-id', - status: 401, - isAuthError: true, - }); - expect(markCalcSynced).not.toHaveBeenCalled(); - expect(markYieldSynced).not.toHaveBeenCalled(); - }); - - it('reports an auth error when shared headers contain no usable credential', async () => { - getAuthHeaders.mockResolvedValue({ 'Content-Type': 'application/json' }); - getAllPendingSync.mockResolvedValue({ - calcs: [ - { - id: 'local-calc-id', - syncStatus: 'local', - species: 'Pacific Cod', - product: 'Round → Skinless Fillets', - yield: 42, - result: 10.71, - }, - ], - yields: [], - }); - - const stats = await syncAll(passwordUser); - - expect(stats).toMatchObject({ - errors: 1, - errorDetails: [ - { - type: 'auth', - isAuthError: true, - message: 'Missing authentication headers', - }, - ], - }); - expect(apiClient.saveCalcRaw).not.toHaveBeenCalled(); - expect(markCalcSynced).not.toHaveBeenCalled(); - expect(markYieldSynced).not.toHaveBeenCalled(); - }); - - it('passes pulled custom yields with the server yield field intact', async () => { - getAllPendingSync.mockResolvedValue({ calcs: [], yields: [] }); - const yieldRow = { - id: 12, - species: 'Pacific Halibut', - product: 'Round → Skinless Fillet', - yield: 48, - source: 'User Input', - }; - apiClient.listUserDataRaw.mockResolvedValue( - fakeResponse({ ok: true, status: 200, body: [yieldRow] }) - ); - - await syncAll(passwordUser); - - expect(mergeSyncedYields).toHaveBeenCalledWith([yieldRow]); - }); - - // --- Delete round-trip: issue #24 --- - - it('successful delete clears the tombstone from local store', async () => { - getAllPendingSync.mockResolvedValue({ - calcs: [ - { - id: 'local-calc-id', - serverId: 42, - syncStatus: 'pending-delete', - }, - ], - yields: [], - }); - apiClient.deleteCalcRaw.mockResolvedValue(fakeResponse({ ok: true, status: 200, body: {} })); - - await syncAll(passwordUser); - - expect(apiClient.deleteCalcRaw).toHaveBeenCalledWith(42, AUTH_HEADERS); - expect(removeCalcSyncedDelete).toHaveBeenCalledWith('local-calc-id'); - }); - - it('404 on delete is treated as already-deleted and clears the tombstone', async () => { - getAllPendingSync.mockResolvedValue({ - calcs: [ - { - id: 'local-calc-id', - serverId: 99, - syncStatus: 'pending-delete', - }, - ], - yields: [], - }); - apiClient.deleteCalcRaw.mockResolvedValue(fakeResponse({ ok: false, status: 404, body: { error: 'Not found' } })); - - const stats = await syncAll(passwordUser); - - // 404 = already gone from server, tombstone must be cleared, not counted as error - expect(removeCalcSyncedDelete).toHaveBeenCalledWith('local-calc-id'); - expect(stats.errors).toBe(0); - }); - - it('failed delete (5xx) leaves the tombstone intact for retry', async () => { - getAllPendingSync.mockResolvedValue({ - calcs: [ - { - id: 'local-calc-id', - serverId: 55, - syncStatus: 'pending-delete', - }, - ], - yields: [], - }); - apiClient.deleteCalcRaw.mockResolvedValue(fakeResponse({ ok: false, status: 500, body: { error: 'Server error' } })); - - const stats = await syncAll(passwordUser); - - expect(removeCalcSyncedDelete).not.toHaveBeenCalled(); - expect(stats.errors).toBe(1); - expect(stats.errorDetails[0]).toMatchObject({ type: 'delete-calc', id: 'local-calc-id', status: 500 }); - }); - - it('calc with no serverId (never synced) is immediately removed without an HTTP call', async () => { - getAllPendingSync.mockResolvedValue({ - calcs: [ - { - id: 'local-only-id', - serverId: undefined, - syncStatus: 'pending-delete', - }, - ], - yields: [], - }); - - const stats = await syncAll(passwordUser); - - expect(apiClient.deleteCalcRaw).not.toHaveBeenCalled(); - expect(removeCalcSyncedDelete).toHaveBeenCalledWith('local-only-id'); - expect(stats.pushed).toBe(1); - expect(stats.errors).toBe(0); - }); - - it('pull does not resurrect calcs that were just deleted (mergeSyncedCalcs called with server list)', async () => { - getAllPendingSync.mockResolvedValue({ calcs: [], yields: [] }); - const serverCalcs = [ - { id: 7, species: 'Pacific Cod', product: 'Round → Skinless Fillets', yield: 42, result: 10.71 }, - ]; - apiClient.listSavedCalcsRaw.mockResolvedValue( - fakeResponse({ ok: true, status: 200, body: serverCalcs }) - ); - - await syncAll(passwordUser); - - // syncEngine must pass the raw server list to mergeSyncedCalcs which - // is responsible for skipping tombstoned items - expect(mergeSyncedCalcs).toHaveBeenCalledWith(serverCalcs); - }); -});