From e75114a9437d7add6ccd41c8ac38ae67d7e8cab7 Mon Sep 17 00:00:00 2001
From: Richard R <me@richardr.dev>
Date: Sat, 23 May 2026 17:07:50 -0600
Subject: [PATCH] refactor(onboarding): introduce onboarding state coordinator
 and central registry

Add useOnboardingCoordinator hook and onboarding-state registry to manage
onboarding-related state such as privacy acceptance and first-visit tracking.
Refactor SettingsModal to utilize the new onboarding state abstractions,
removing legacy privacy gating logic and improving maintainability.

Includes unit tests for onboarding-state registry to ensure correctness.
---
 src/components/SettingsModal.tsx      | 119 +++++------------
 src/hooks/useOnboardingCoordinator.ts | 184 ++++++++++++++++++++++++++
 src/lib/shared/onboarding-state.ts    |  45 +++++++
 tests/unit/onboarding-state.spec.ts   |  19 +++
 4 files changed, 282 insertions(+), 85 deletions(-)
 create mode 100644 src/hooks/useOnboardingCoordinator.ts
 create mode 100644 src/lib/shared/onboarding-state.ts
 create mode 100644 tests/unit/onboarding-state.spec.ts

diff --git a/src/components/SettingsModal.tsx b/src/components/SettingsModal.tsx
index 3a25446..6c01f8a 100644
--- a/src/components/SettingsModal.tsx
+++ b/src/components/SettingsModal.tsx
@@ -1,6 +1,6 @@
 'use client';
 
-import { Fragment, useState, useEffect, useCallback, useMemo, useRef } from 'react';
+import { Fragment, useState, useEffect, useCallback, useMemo } from 'react';
 import {
   Dialog,
   DialogPanel,
@@ -34,7 +34,6 @@ import { useRouter } from 'next/navigation';
 import { showPrivacyModal } from '@/components/PrivacyModal';
 import { deleteDocuments, mimeTypeForDoc, uploadDocuments } from '@/lib/client/api/documents';
 import { postChangelogVersionCheck } from '@/lib/client/api/user-state';
-import { scheduleChangelogCheck } from '@/lib/client/changelog-check';
 import { cacheStoredDocumentFromBytes, clearDocumentCache } from '@/lib/client/cache/documents';
 import { clearAllDocumentPreviewCaches, clearInMemoryDocumentPreviewCache } from '@/lib/client/cache/previews';
 import { resolveTtsSettingsViewModel } from '@/lib/client/settings/tts-settings';
@@ -72,6 +71,8 @@ import {
   type ChangelogManifestEntry,
   type ChangelogReleaseBody,
 } from '@/lib/shared/changelog';
+import { useOnboardingCoordinator } from '@/hooks/useOnboardingCoordinator';
+import { ONBOARDING_STATE_REGISTRY } from '@/lib/shared/onboarding-state';
 
 // Hard-coded theme color palettes for the visual theme selector
 type ThemeColorSet = { background: string; base: string; offbase: string; accent: string; secondaryAccent: string; foreground: string; muted: string };
@@ -174,8 +175,6 @@ export function SettingsModal({ className = '' }: { className?: string }) {
   const { progress, setProgress, estimatedTimeRemaining } = useTimeEstimation();
   const { authEnabled, baseUrl: authBaseUrl } = useAuthConfig();
   const { data: session, isPending: isSessionPending } = useAuthSession();
-  const changelogVersionCheckKeyRef = useRef<string | null>(null);
-  const changelogVersionCheckInFlightRef = useRef<string | null>(null);
   const router = useRouter();
   const isBusy = isImportingLibrary;
   const {
@@ -208,46 +207,44 @@ export function SettingsModal({ className = '' }: { className?: string }) {
   const isSharedSelected = Boolean(selectedSharedProvider);
   const selectedProviderOption = ttsProviders.find((p) => p.id === localProviderRef) ?? ttsProviders[0];
 
-  const closeSettingsForPrivacyGate = useCallback(() => {
+  const closeSettings = useCallback(() => {
     setIsOpen(false);
     setIsChangelogOpen(false);
   }, []);
 
-  const canOpenSettings = useCallback(async () => {
-    if (!authEnabled) {
-      return true;
-    }
-    const appConfig = await getAppConfig();
-    return Boolean(appConfig?.privacyAccepted);
-  }, [authEnabled]);
-
-  const openSettings = useCallback(async (options?: { changelog?: boolean }) => {
-    const allowed = await canOpenSettings();
-    if (!allowed) {
-      closeSettingsForPrivacyGate();
-      return;
-    }
+  const openSettings = useCallback((options?: { changelog?: boolean }) => {
     setIsOpen(true);
     setIsChangelogOpen(Boolean(options?.changelog));
-  }, [canOpenSettings, closeSettingsForPrivacyGate]);
+  }, []);
 
-  const checkFirstVist = useCallback(async () => {
-    const allowed = await canOpenSettings();
-    if (!allowed) {
-      return;
-    }
-    const firstVisit = await getFirstVisit();
-    if (!firstVisit) {
-      await setFirstVisit(true);
-      setIsOpen(true);
-    }
-  }, [canOpenSettings, setIsOpen]);
+  const readLocalOnboardingSnapshot = useCallback(async () => {
+    const appConfig = await getAppConfig();
+    const row = appConfig as Record<string, unknown> | null;
+    const privacyKey = ONBOARDING_STATE_REGISTRY.privacyAccepted.localKey;
+    const firstVisitKey = ONBOARDING_STATE_REGISTRY.firstVisitSettingsOpened.localKey;
 
-  useEffect(() => {
-    checkFirstVist().catch((err) => {
-      console.error('First visit check failed:', err);
-    });
-  }, [checkFirstVist]);
+    return {
+      privacyAccepted: privacyKey ? Boolean(row?.[privacyKey]) : false,
+      firstVisitSettingsOpened: firstVisitKey ? Boolean(row?.[firstVisitKey]) : await getFirstVisit(),
+    };
+  }, []);
+
+  const markFirstVisitSettingsOpened = useCallback(async () => {
+    await setFirstVisit(true);
+  }, []);
+
+  const { requestOpenSettings } = useOnboardingCoordinator({
+    authEnabled,
+    isSessionPending,
+    sessionUserId: session?.user?.id,
+    appVersion: runtimeConfig.appVersion,
+    isSettingsOpen: isOpen,
+    readLocalSnapshot: readLocalOnboardingSnapshot,
+    markFirstVisitSettingsOpened,
+    postChangelogVersionCheck: async (currentVersion) => postChangelogVersionCheck(currentVersion),
+    openSettings,
+    closeSettings,
+  });
 
   useEffect(() => {
     setLocalApiKey(apiKey);
@@ -258,54 +255,6 @@ export function SettingsModal({ className = '' }: { className?: string }) {
     setLocalTTSInstructions(ttsInstructions);
   }, [apiKey, baseUrl, providerRef, providerType, ttsModel, ttsInstructions]);
 
-  useEffect(() => {
-    if (!authEnabled) {
-      return;
-    }
-    const onPrivacyAccepted = () => {
-      checkFirstVist().catch((err) => {
-        console.error('First visit check after privacy acceptance failed:', err);
-      });
-    };
-    window.addEventListener('openreader:privacyAccepted', onPrivacyAccepted);
-    return () => {
-      window.removeEventListener('openreader:privacyAccepted', onPrivacyAccepted);
-    };
-  }, [authEnabled, checkFirstVist]);
-
-  useEffect(() => {
-    if (!isOpen) {
-      return;
-    }
-    let cancelled = false;
-    void (async () => {
-      const allowed = await canOpenSettings();
-      if (!allowed && !cancelled) {
-        closeSettingsForPrivacyGate();
-      }
-    })();
-    return () => {
-      cancelled = true;
-    };
-  }, [isOpen, canOpenSettings, closeSettingsForPrivacyGate]);
-
-  useEffect(() => {
-    return scheduleChangelogCheck({
-      authEnabled,
-      isSessionPending,
-      sessionUserId: session?.user?.id,
-      appVersion: runtimeConfig.appVersion,
-      completedRef: changelogVersionCheckKeyRef,
-      inFlightRef: changelogVersionCheckInFlightRef,
-      postCheck: async (currentVersion) => postChangelogVersionCheck(currentVersion),
-      onShouldOpen: () => {
-        void openSettings({ changelog: true });
-      },
-      delayMs: 120,
-      retryDelayMs: 400,
-    });
-  }, [authEnabled, isSessionPending, session?.user?.id, runtimeConfig.appVersion, openSettings]);
-
   useEffect(() => {
     if (!ttsModels.some(m => m.id === modelValue) && modelValue !== '') {
       setCustomModelInput(modelValue);
@@ -571,7 +520,7 @@ export function SettingsModal({ className = '' }: { className?: string }) {
     <>
       <Button
         onClick={() => {
-          void openSettings();
+          void requestOpenSettings();
         }}
         className={`inline-flex items-center py-1 px-2 rounded-md border border-offbase bg-base text-foreground text-xs hover:bg-offbase hover:text-accent transition-transform transition-colors duration-200 ease-out hover:scale-[1.08] ${className}`}
         aria-label="Settings"
diff --git a/src/hooks/useOnboardingCoordinator.ts b/src/hooks/useOnboardingCoordinator.ts
new file mode 100644
index 0000000..70bb3f8
--- /dev/null
+++ b/src/hooks/useOnboardingCoordinator.ts
@@ -0,0 +1,184 @@
+'use client';
+
+import { useCallback, useEffect, useRef } from 'react';
+import {
+  scheduleChangelogCheck,
+  type ChangelogVersionCheckResponse,
+} from '@/lib/client/changelog-check';
+
+type OpenSettingsOptions = {
+  changelog?: boolean;
+};
+
+type LocalOnboardingSnapshot = {
+  privacyAccepted: boolean;
+  firstVisitSettingsOpened: boolean;
+};
+
+type UseOnboardingCoordinatorArgs = {
+  authEnabled: boolean;
+  isSessionPending: boolean;
+  sessionUserId: string | null | undefined;
+  appVersion: string | null | undefined;
+  isSettingsOpen: boolean;
+  readLocalSnapshot: () => Promise<LocalOnboardingSnapshot>;
+  markFirstVisitSettingsOpened: () => Promise<void>;
+  postChangelogVersionCheck: (currentVersion: string) => Promise<ChangelogVersionCheckResponse>;
+  openSettings: (options?: OpenSettingsOptions) => void;
+  closeSettings: () => void;
+};
+
+type UseOnboardingCoordinatorResult = {
+  requestOpenSettings: (options?: OpenSettingsOptions) => Promise<boolean>;
+};
+
+export function useOnboardingCoordinator(
+  args: UseOnboardingCoordinatorArgs,
+): UseOnboardingCoordinatorResult {
+  const {
+    authEnabled,
+    isSessionPending,
+    sessionUserId,
+    appVersion,
+    isSettingsOpen,
+    readLocalSnapshot,
+    markFirstVisitSettingsOpened,
+    postChangelogVersionCheck,
+    openSettings,
+    closeSettings,
+  } = args;
+
+  const pendingChangelogOpenRef = useRef(false);
+  const changelogVersionCheckKeyRef = useRef<string | null>(null);
+  const changelogVersionCheckInFlightRef = useRef<string | null>(null);
+
+  const canOpenSettingsNow = useCallback(async (): Promise<{
+    allowed: boolean;
+    local: LocalOnboardingSnapshot | null;
+  }> => {
+    if (!authEnabled) {
+      return { allowed: true, local: null };
+    }
+
+    const local = await readLocalSnapshot();
+    return {
+      allowed: local.privacyAccepted,
+      local,
+    };
+  }, [authEnabled, readLocalSnapshot]);
+
+  const requestOpenSettings = useCallback(async (options?: OpenSettingsOptions): Promise<boolean> => {
+    const gate = await canOpenSettingsNow();
+    if (!gate.allowed) {
+      if (options?.changelog) {
+        pendingChangelogOpenRef.current = true;
+      }
+      closeSettings();
+      return false;
+    }
+
+    openSettings({ changelog: Boolean(options?.changelog) });
+    return true;
+  }, [canOpenSettingsNow, closeSettings, openSettings]);
+
+  const maybeOpenFirstVisitSettings = useCallback(async () => {
+    const gate = await canOpenSettingsNow();
+    if (!gate.allowed) {
+      return;
+    }
+
+    const firstVisitSettingsOpened = gate.local
+      ? gate.local.firstVisitSettingsOpened
+      : (await readLocalSnapshot()).firstVisitSettingsOpened;
+
+    if (firstVisitSettingsOpened) {
+      return;
+    }
+
+    await markFirstVisitSettingsOpened();
+    openSettings();
+  }, [canOpenSettingsNow, markFirstVisitSettingsOpened, openSettings, readLocalSnapshot]);
+
+  const replayDeferredChangelogOpen = useCallback(async () => {
+    if (!pendingChangelogOpenRef.current) {
+      return;
+    }
+
+    const opened = await requestOpenSettings({ changelog: true });
+    if (opened) {
+      pendingChangelogOpenRef.current = false;
+    }
+  }, [requestOpenSettings]);
+
+  useEffect(() => {
+    maybeOpenFirstVisitSettings().catch((err) => {
+      console.error('First visit settings check failed:', err);
+    });
+  }, [maybeOpenFirstVisitSettings]);
+
+  useEffect(() => {
+    if (!authEnabled) {
+      return;
+    }
+
+    const onPrivacyAccepted = () => {
+      maybeOpenFirstVisitSettings().catch((err) => {
+        console.error('First visit settings check after privacy acceptance failed:', err);
+      });
+      replayDeferredChangelogOpen().catch((err) => {
+        console.error('Deferred changelog open after privacy acceptance failed:', err);
+      });
+    };
+
+    window.addEventListener('openreader:privacyAccepted', onPrivacyAccepted);
+    return () => {
+      window.removeEventListener('openreader:privacyAccepted', onPrivacyAccepted);
+    };
+  }, [authEnabled, maybeOpenFirstVisitSettings, replayDeferredChangelogOpen]);
+
+  useEffect(() => {
+    if (!isSettingsOpen) {
+      return;
+    }
+
+    let cancelled = false;
+    void (async () => {
+      const gate = await canOpenSettingsNow();
+      if (!gate.allowed && !cancelled) {
+        closeSettings();
+      }
+    })();
+
+    return () => {
+      cancelled = true;
+    };
+  }, [canOpenSettingsNow, closeSettings, isSettingsOpen]);
+
+  useEffect(() => {
+    return scheduleChangelogCheck({
+      authEnabled,
+      isSessionPending,
+      sessionUserId,
+      appVersion,
+      completedRef: changelogVersionCheckKeyRef,
+      inFlightRef: changelogVersionCheckInFlightRef,
+      postCheck: postChangelogVersionCheck,
+      onShouldOpen: () => {
+        void requestOpenSettings({ changelog: true });
+      },
+      delayMs: 120,
+      retryDelayMs: 400,
+    });
+  }, [
+    appVersion,
+    authEnabled,
+    isSessionPending,
+    postChangelogVersionCheck,
+    sessionUserId,
+    requestOpenSettings,
+  ]);
+
+  return {
+    requestOpenSettings,
+  };
+}
diff --git a/src/lib/shared/onboarding-state.ts b/src/lib/shared/onboarding-state.ts
new file mode 100644
index 0000000..8b8723d
--- /dev/null
+++ b/src/lib/shared/onboarding-state.ts
@@ -0,0 +1,45 @@
+export type OnboardingStorageScope = 'local-dexie' | 'server-user-preferences' | 'hybrid';
+
+export type OnboardingStateDescriptor = {
+  /**
+   * Where this state is persisted today.
+   * - local-dexie: browser/device-local only
+   * - server-user-preferences: per-user on server
+   * - hybrid: intentionally persisted in both places
+   */
+  scope: OnboardingStorageScope;
+  /**
+   * Optional local key name when state is stored in Dexie app-config.
+   */
+  localKey?: string;
+  /**
+   * Optional server-side key when state is stored in user preferences/meta.
+   */
+  serverKey?: string;
+};
+
+/**
+ * Central registry for onboarding-related state and where each item lives.
+ * Keep this map up to date whenever onboarding persistence changes.
+ */
+export const ONBOARDING_STATE_REGISTRY = {
+  privacyAccepted: {
+    scope: 'local-dexie',
+    localKey: 'privacyAccepted',
+  },
+  firstVisitSettingsOpened: {
+    scope: 'local-dexie',
+    localKey: 'firstVisit',
+  },
+  documentsMigrationPrompted: {
+    scope: 'local-dexie',
+    localKey: 'documentsMigrationPrompted',
+  },
+  changelogLastSeenAppVersion: {
+    scope: 'server-user-preferences',
+    serverKey: '_meta.lastSeenAppVersion',
+  },
+} as const satisfies Record<string, OnboardingStateDescriptor>;
+
+export type OnboardingStateKey = keyof typeof ONBOARDING_STATE_REGISTRY;
+
diff --git a/tests/unit/onboarding-state.spec.ts b/tests/unit/onboarding-state.spec.ts
new file mode 100644
index 0000000..9379c2a
--- /dev/null
+++ b/tests/unit/onboarding-state.spec.ts
@@ -0,0 +1,19 @@
+import { expect, test } from '@playwright/test';
+import { ONBOARDING_STATE_REGISTRY } from '../../src/lib/shared/onboarding-state';
+import { SYNCED_PREFERENCE_KEYS } from '../../src/types/user-state';
+
+test.describe('onboarding state storage scopes', () => {
+  test('keeps local onboarding flags out of synced server preferences', () => {
+    expect(ONBOARDING_STATE_REGISTRY.privacyAccepted.scope).toBe('local-dexie');
+    expect(ONBOARDING_STATE_REGISTRY.firstVisitSettingsOpened.scope).toBe('local-dexie');
+    expect(ONBOARDING_STATE_REGISTRY.documentsMigrationPrompted.scope).toBe('local-dexie');
+    expect(ONBOARDING_STATE_REGISTRY.changelogLastSeenAppVersion.scope).toBe('server-user-preferences');
+
+    const synced = new Set<string>(SYNCED_PREFERENCE_KEYS);
+
+    expect(synced.has(ONBOARDING_STATE_REGISTRY.privacyAccepted.localKey!)).toBe(false);
+    expect(synced.has(ONBOARDING_STATE_REGISTRY.firstVisitSettingsOpened.localKey!)).toBe(false);
+    expect(synced.has(ONBOARDING_STATE_REGISTRY.documentsMigrationPrompted.localKey!)).toBe(false);
+  });
+});
+