diff --git a/src/App.test.tsx b/src/App.test.tsx index 0a770c4c..d3fdcc22 100644 --- a/src/App.test.tsx +++ b/src/App.test.tsx @@ -270,6 +270,42 @@ describe('App', () => { expect(screen.getByTestId('welcome-open-folder')).toHaveTextContent('Open existing vault') }) + it.each([ + ['telemetry-accept', 'Allow anonymous reporting'], + ['telemetry-decline', 'No thanks'], + ])('ignores a remembered default vault after %s when onboarding was never completed', async (buttonTestId) => { + const rememberedDefaultVaultPath = '/Volumes/Jupiter/Workspace/laputa-app/demo-vault-v2' + localStorage.setItem('tolaria_welcome_dismissed', '1') + mockCommandResults.get_default_vault_path = rememberedDefaultVaultPath + mockCommandResults.get_settings = { + auto_pull_interval_minutes: null, + telemetry_consent: null, + crash_reporting_enabled: null, + analytics_enabled: null, + anonymous_id: null, + release_channel: null, + } + mockCommandResults.load_vault_list = { + vaults: [], + active_vault: rememberedDefaultVaultPath, + hidden_defaults: [], + } + mockCommandResults.check_vault_exists = (args?: { path?: string }) => args?.path === rememberedDefaultVaultPath + + render() + + await waitFor(() => { + expect(screen.getByText('Help improve Tolaria')).toBeInTheDocument() + }) + + fireEvent.click(screen.getByTestId(buttonTestId)) + + await waitFor(() => { + expect(screen.getByTestId('welcome-screen')).toBeInTheDocument() + }) + expect(screen.getByTestId('welcome-open-folder')).toHaveTextContent('Open existing vault') + }) + it('renders sidebar with correct default selection (All Notes)', async () => { render() await waitFor(() => { diff --git a/src/App.tsx b/src/App.tsx index 72e95d18..ba8f8508 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -61,7 +61,6 @@ import { useLayoutPanels } from './hooks/useLayoutPanels' import { useConflictFlow } from './hooks/useConflictFlow' import { useAppSave } from './hooks/useAppSave' import { useVaultBridge } from './hooks/useVaultBridge' -import { getAppStorageItem } from './constants/appStorage' import type { CommitDiffRequest } from './hooks/useDiffMode' import { ConflictResolverModal } from './components/ConflictResolverModal' import { ConfirmDeleteDialog } from './components/ConfirmDeleteDialog' @@ -207,7 +206,7 @@ function App() { onSwitch: () => { handleSetSelection(DEFAULT_SELECTION); notes.closeAllTabs() }, onToast: (msg) => setToastMessage(msg), }) - const { allVaults, handleVaultCloned, switchVault } = vaultSwitcher + const { allVaults, defaultPath, handleVaultCloned, selectedVaultPath, switchVault } = vaultSwitcher const rememberOnboardingVaultChoice = useCallback((vaultPath: string) => { if (!vaultPath) return @@ -1019,17 +1018,13 @@ function App() { const shouldResumeFreshStartOnboarding = useMemo(() => { if (onboarding.state.status !== 'ready' || !vaultSwitcher.loaded) return false + const remembersOnlyDefaultVault = selectedVaultPath === null || selectedVaultPath === defaultPath - try { - if (getAppStorageItem('welcomeDismissed') === '1') return false - } catch { - return false - } - - return vaultSwitcher.allVaults.length === 1 + return remembersOnlyDefaultVault + && vaultSwitcher.allVaults.length === 1 && vaultSwitcher.allVaults[0]?.path === vaultSwitcher.vaultPath && onboarding.state.vaultPath === vaultSwitcher.vaultPath - }, [onboarding.state, vaultSwitcher.allVaults, vaultSwitcher.loaded, vaultSwitcher.vaultPath]) + }, [defaultPath, onboarding.state, selectedVaultPath, vaultSwitcher.allVaults, vaultSwitcher.loaded, vaultSwitcher.vaultPath]) // Show loading spinner while checking vault (skip for note windows) if (!noteWindowParams && onboarding.state.status === 'loading') { diff --git a/src/hooks/useVaultSwitcher.test.ts b/src/hooks/useVaultSwitcher.test.ts index f257e1ee..1046b6de 100644 --- a/src/hooks/useVaultSwitcher.test.ts +++ b/src/hooks/useVaultSwitcher.test.ts @@ -148,6 +148,16 @@ describe('useVaultSwitcher', () => { expect(onSwitch).toHaveBeenCalled() }) + it('does not persist the implicit default vault as an active selection', async () => { + const { result } = renderHook(() => useVaultSwitcher({ onSwitch, onToast })) + + await waitFor(() => { expect(result.current.loaded).toBe(true) }) + + expect(result.current.vaultPath).toBe(expectedDefaultVaultPath) + expect(result.current.selectedVaultPath).toBeNull() + expect(mockVaultListStore.active_vault).toBeNull() + }) + it('handles load error gracefully', async () => { const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) mockInvokeFn.mockImplementation((cmd: string) => { @@ -413,6 +423,20 @@ describe('useVaultSwitcher', () => { expect(result.current.vaultPath).toBe(persistedPath) }) + + it('treats a remembered default vault as unselected when it is the only vault', async () => { + mockVaultListStore = { + vaults: [], + active_vault: expectedDefaultVaultPath, + hidden_defaults: [], + } + + const { result } = renderHook(() => useVaultSwitcher({ onSwitch, onToast })) + await waitFor(() => { expect(result.current.loaded).toBe(true) }) + + expect(result.current.vaultPath).toBe(expectedDefaultVaultPath) + expect(result.current.selectedVaultPath).toBeNull() + }) }) describe('isGettingStartedHidden', () => { diff --git a/src/hooks/useVaultSwitcher.ts b/src/hooks/useVaultSwitcher.ts index c7cd0e39..aa93e95e 100644 --- a/src/hooks/useVaultSwitcher.ts +++ b/src/hooks/useVaultSwitcher.ts @@ -31,8 +31,10 @@ interface PersistedVaultState { extraVaults: VaultOption[] hiddenDefaults: string[] loaded: boolean + selectedVaultPath: string | null setExtraVaults: Dispatch> setHiddenDefaults: Dispatch> + setSelectedVaultPath: Dispatch> setVaultPath: Dispatch> vaultPath: string } @@ -48,10 +50,12 @@ interface PersistedVaultStore { extraVaults: VaultOption[] hiddenDefaults: string[] loaded: boolean + selectedVaultPath: string | null setDefaultPath: Dispatch> setExtraVaults: Dispatch> setHiddenDefaults: Dispatch> setLoaded: Dispatch> + setSelectedVaultPath: Dispatch> setVaultPath: Dispatch> vaultPath: string } @@ -77,10 +81,13 @@ interface RemainingVaultOptions { } interface RemoveVaultStateOptions extends RemainingVaultOptions { + selectedVaultPath: string | null onSwitchRef: MutableRefObject<() => void> setExtraVaults: Dispatch> setHiddenDefaults: Dispatch> + setSelectedVaultPath: Dispatch> setVaultPath: Dispatch> + vaultPath: string } interface RemoveVaultActionOptions { @@ -91,10 +98,17 @@ interface RemoveVaultActionOptions { onToastRef: MutableRefObject<(msg: string) => void> setExtraVaults: Dispatch> setHiddenDefaults: Dispatch> + setSelectedVaultPath: Dispatch> setVaultPath: Dispatch> + selectedVaultPath: string | null + vaultPath: string } -function labelFromPath(path: string): string { +interface VaultPathInput { + path: string +} + +function labelFromPath({ path }: VaultPathInput): string { return path.split('/').pop() || 'Local Vault' } @@ -127,22 +141,37 @@ async function loadInitialVaultState() { return { activeVault, hiddenDefaults, resolvedDefaultPath, vaults } } -function buildDefaultVaults(defaultPath: string): VaultOption[] { +function buildDefaultVaults({ defaultPath }: { defaultPath: string }): VaultOption[] { return [{ label: GETTING_STARTED_LABEL, path: defaultPath }] } -function buildVisibleDefaultVaults(defaultVaults: VaultOption[], hiddenDefaults: string[]): VaultOption[] { +function buildVisibleDefaultVaults({ + defaultVaults, + hiddenDefaults, +}: { + defaultVaults: VaultOption[] + hiddenDefaults: string[] +}): VaultOption[] { return defaultVaults.filter(vault => !hiddenDefaults.includes(vault.path)) } -function buildAllVaults(visibleDefaults: VaultOption[], extraVaults: VaultOption[]): VaultOption[] { +function buildAllVaults({ + visibleDefaults, + extraVaults, +}: { + visibleDefaults: VaultOption[] + extraVaults: VaultOption[] +}): VaultOption[] { return [...visibleDefaults, ...extraVaults] } -function applyResolvedDefaultPath( - resolvedDefaultPath: string, - setDefaultPath: Dispatch>, -) { +function applyResolvedDefaultPath({ + resolvedDefaultPath, + setDefaultPath, +}: { + resolvedDefaultPath: string + setDefaultPath: Dispatch> +}) { if (!resolvedDefaultPath) { return } @@ -151,14 +180,35 @@ function applyResolvedDefaultPath( syncDefaultVaultExport(resolvedDefaultPath) } -function applyInitialVaultTarget( +function normalizeInitialSelectedVaultPath( activeVault: string | null, resolvedDefaultPath: string, - setVaultPath: Dispatch>, - onSwitchRef: MutableRefObject<() => void>, -) { + vaults: VaultOption[], +): string | null { + if (!activeVault) { + return null + } + + const isRememberedDefaultOnlySelection = activeVault === resolvedDefaultPath && vaults.length === 0 + return isRememberedDefaultOnlySelection ? null : activeVault +} + +function applyInitialVaultTarget({ + activeVault, + resolvedDefaultPath, + setSelectedVaultPath, + setVaultPath, + onSwitchRef, +}: { + activeVault: string | null + resolvedDefaultPath: string + setSelectedVaultPath: Dispatch> + setVaultPath: Dispatch> + onSwitchRef: MutableRefObject<() => void> +}) { if (activeVault) { setVaultPath(activeVault) + setSelectedVaultPath(activeVault) onSwitchRef.current() return } @@ -174,15 +224,15 @@ function useVaultCollections( extraVaults: VaultOption[], ): VaultCollections { const defaultVaults = useMemo( - () => buildDefaultVaults(defaultPath), + () => buildDefaultVaults({ defaultPath }), [defaultPath], ) const visibleDefaults = useMemo( - () => buildVisibleDefaultVaults(defaultVaults, hiddenDefaults), + () => buildVisibleDefaultVaults({ defaultVaults, hiddenDefaults }), [defaultVaults, hiddenDefaults], ) const allVaults = useMemo( - () => buildAllVaults(visibleDefaults, extraVaults), + () => buildAllVaults({ visibleDefaults, extraVaults }), [extraVaults, visibleDefaults], ) const isGettingStartedHidden = useMemo( @@ -202,6 +252,7 @@ function useLoadPersistedVaultState( setExtraVaults, setHiddenDefaults, setLoaded, + setSelectedVaultPath, setVaultPath, } = store @@ -214,8 +265,14 @@ function useLoadPersistedVaultState( setExtraVaults(vaults) setHiddenDefaults(hidden) - applyResolvedDefaultPath(resolvedDefaultPath, setDefaultPath) - applyInitialVaultTarget(activeVault, resolvedDefaultPath, setVaultPath, onSwitchRef) + applyResolvedDefaultPath({ resolvedDefaultPath, setDefaultPath }) + applyInitialVaultTarget({ + activeVault: normalizeInitialSelectedVaultPath(activeVault, resolvedDefaultPath, vaults), + resolvedDefaultPath, + setSelectedVaultPath, + setVaultPath, + onSwitchRef, + }) }) .catch(err => console.warn('Failed to load vault list:', err)) .finally(() => { @@ -225,23 +282,24 @@ function useLoadPersistedVaultState( }) return () => { cancelled = true } - }, [onSwitchRef, setDefaultPath, setExtraVaults, setHiddenDefaults, setLoaded, setVaultPath]) + }, [onSwitchRef, setDefaultPath, setExtraVaults, setHiddenDefaults, setLoaded, setSelectedVaultPath, setVaultPath]) } function usePersistedVaultStorage(store: PersistedVaultStore) { - const { extraVaults, hiddenDefaults, loaded, vaultPath } = store + const { extraVaults, hiddenDefaults, loaded, selectedVaultPath } = store useEffect(() => { if (!loaded) return - saveVaultList(extraVaults, vaultPath, hiddenDefaults).catch(err => + saveVaultList(extraVaults, selectedVaultPath, hiddenDefaults).catch(err => console.warn('Failed to persist vault list:', err), ) - }, [extraVaults, hiddenDefaults, loaded, vaultPath]) + }, [extraVaults, hiddenDefaults, loaded, selectedVaultPath]) } function usePersistedVaultState(onSwitchRef: MutableRefObject<() => void>): PersistedVaultState { const [vaultPath, setVaultPath] = useState(STATIC_DEFAULT_PATH) + const [selectedVaultPath, setSelectedVaultPath] = useState(null) const [extraVaults, setExtraVaults] = useState([]) const [hiddenDefaults, setHiddenDefaults] = useState([]) const [loaded, setLoaded] = useState(false) @@ -252,10 +310,12 @@ function usePersistedVaultState(onSwitchRef: MutableRefObject<() => void>): Pers extraVaults, hiddenDefaults, loaded, + selectedVaultPath, setDefaultPath, setExtraVaults, setHiddenDefaults, setLoaded, + setSelectedVaultPath, setVaultPath, vaultPath, } @@ -268,8 +328,10 @@ function usePersistedVaultState(onSwitchRef: MutableRefObject<() => void>): Pers extraVaults, hiddenDefaults, loaded, + selectedVaultPath, setExtraVaults, setHiddenDefaults, + setSelectedVaultPath, setVaultPath, vaultPath, } @@ -304,23 +366,34 @@ async function ensureGettingStartedVaultReady(path: string): Promise { } } -function addVaultToList( - setExtraVaults: Dispatch>, - path: string, - label: string, -) { +function addVaultToList({ + setExtraVaults, + path, + label, +}: { + setExtraVaults: Dispatch> + path: string + label: string +}) { setExtraVaults(previousVaults => { const exists = previousVaults.some(vault => vault.path === path) return exists ? previousVaults : [...previousVaults, { label, path, available: true }] }) } -function switchVaultPath( - setVaultPath: Dispatch>, - onSwitchRef: MutableRefObject<() => void>, - path: string, -) { +function switchVaultPath({ + setSelectedVaultPath, + setVaultPath, + onSwitchRef, + path, +}: { + setSelectedVaultPath: Dispatch> + setVaultPath: Dispatch> + onSwitchRef: MutableRefObject<() => void> + path: string +}) { trackEvent('vault_switched') + setSelectedVaultPath(path) setVaultPath(path) onSwitchRef.current() } @@ -349,7 +422,10 @@ function removeVaultFromState({ removedPath, setExtraVaults, setHiddenDefaults, + setSelectedVaultPath, setVaultPath, + selectedVaultPath, + vaultPath, }: RemoveVaultStateOptions) { if (isDefault) { setHiddenDefaults(previousHidden => previousHidden.includes(removedPath) ? previousHidden : [...previousHidden, removedPath]) @@ -357,43 +433,52 @@ function removeVaultFromState({ setExtraVaults(previousVaults => previousVaults.filter(vault => vault.path !== removedPath)) } - setVaultPath(currentPath => { - if (currentPath !== removedPath) { - return currentPath + if (vaultPath !== removedPath) { + if (selectedVaultPath === removedPath) { + setSelectedVaultPath(null) } + return + } - const remainingVaults = listRemainingVaults({ - defaultVaults, - extraVaults, - hiddenDefaults, - isDefault, - removedPath, - }) - if (remainingVaults.length === 0) { - return currentPath - } - - onSwitchRef.current() - return remainingVaults[0].path + const remainingVaults = listRemainingVaults({ + defaultVaults, + extraVaults, + hiddenDefaults, + isDefault, + removedPath, }) + if (remainingVaults.length === 0) { + setSelectedVaultPath(null) + return + } + + const nextPath = remainingVaults[0].path + setSelectedVaultPath(nextPath) + setVaultPath(nextPath) + onSwitchRef.current() } -function getRemovedVaultLabel( - path: string, - defaultVaults: VaultOption[], - extraVaults: VaultOption[], -): string { +function getRemovedVaultLabel({ + path, + defaultVaults, + extraVaults, +}: { + path: string + defaultVaults: VaultOption[] + extraVaults: VaultOption[] +}): string { const removedVault = [...defaultVaults, ...extraVaults].find(vault => vault.path === path) - return removedVault?.label ?? labelFromPath(path) + return removedVault?.label ?? labelFromPath({ path }) } function useSwitchVaultAction( onSwitchRef: MutableRefObject<() => void>, + setSelectedVaultPath: Dispatch>, setVaultPath: Dispatch>, ) { return useCallback((path: string) => { - switchVaultPath(setVaultPath, onSwitchRef, path) - }, [onSwitchRef, setVaultPath]) + switchVaultPath({ setSelectedVaultPath, setVaultPath, onSwitchRef, path }) + }, [onSwitchRef, setSelectedVaultPath, setVaultPath]) } function useVaultClonedAction( @@ -414,7 +499,7 @@ function useOpenLocalFolderAction( const path = await pickFolder('Open vault folder') if (!path) return - const label = labelFromPath(path) + const label = labelFromPath({ path }) addAndSwitch(path, label) onToastRef.current(`Vault "${label}" opened`) }, [addAndSwitch, onToastRef]) @@ -428,7 +513,10 @@ function useRemoveVaultAction({ onToastRef, setExtraVaults, setHiddenDefaults, + setSelectedVaultPath, setVaultPath, + selectedVaultPath, + vaultPath, }: RemoveVaultActionOptions) { return useCallback((path: string) => { const isDefault = defaultVaults.some(vault => vault.path === path) @@ -442,9 +530,12 @@ function useRemoveVaultAction({ removedPath: path, setExtraVaults, setHiddenDefaults, + setSelectedVaultPath, setVaultPath, + selectedVaultPath, + vaultPath, }) - onToastRef.current(`Vault "${getRemovedVaultLabel(path, defaultVaults, extraVaults)}" removed from list`) + onToastRef.current(`Vault "${getRemovedVaultLabel({ path, defaultVaults, extraVaults })}" removed from list`) }, [ defaultVaults, extraVaults, @@ -453,7 +544,10 @@ function useRemoveVaultAction({ onToastRef, setExtraVaults, setHiddenDefaults, + setSelectedVaultPath, setVaultPath, + selectedVaultPath, + vaultPath, ]) } @@ -479,13 +573,16 @@ function useVaultActions({ onToastRef, setExtraVaults, setHiddenDefaults, + selectedVaultPath, + setSelectedVaultPath, setVaultPath, + vaultPath, }: VaultActionOptions) { const addVault = useCallback((path: string, label: string) => { - addVaultToList(setExtraVaults, path, label) + addVaultToList({ setExtraVaults, path, label }) }, [setExtraVaults]) - const switchVault = useSwitchVaultAction(onSwitchRef, setVaultPath) + const switchVault = useSwitchVaultAction(onSwitchRef, setSelectedVaultPath, setVaultPath) const addAndSwitch = useCallback((path: string, label: string) => { addVault(path, label) switchVault(path) @@ -502,7 +599,10 @@ function useVaultActions({ onToastRef, setExtraVaults, setHiddenDefaults, + setSelectedVaultPath, setVaultPath, + selectedVaultPath, + vaultPath, }), restoreGettingStarted: useRestoreGettingStartedAction({ defaultPath, @@ -543,7 +643,14 @@ export function useVaultSwitcher({ onSwitch, onToast }: UseVaultSwitcherOptions) useEffect(() => { onSwitchRef.current = onSwitch; onToastRef.current = onToast }) const persistedState = usePersistedVaultState(onSwitchRef) - const { defaultPath, extraVaults, hiddenDefaults, loaded, vaultPath } = persistedState + const { + defaultPath, + extraVaults, + hiddenDefaults, + loaded, + selectedVaultPath, + vaultPath, + } = persistedState const { allVaults, defaultVaults, isGettingStartedHidden } = useVaultCollections( defaultPath, hiddenDefaults, @@ -559,7 +666,16 @@ export function useVaultSwitcher({ onSwitch, onToast }: UseVaultSwitcherOptions) }) return { - vaultPath, allVaults, switchVault, handleVaultCloned, handleOpenLocalFolder, loaded, - removeVault, restoreGettingStarted, isGettingStartedHidden, + allVaults, + defaultPath, + handleOpenLocalFolder, + handleVaultCloned, + isGettingStartedHidden, + loaded, + removeVault, + restoreGettingStarted, + selectedVaultPath, + switchVault, + vaultPath, } } diff --git a/src/utils/vaultListStore.ts b/src/utils/vaultListStore.ts index 65a7ffee..5842c4d1 100644 --- a/src/utils/vaultListStore.ts +++ b/src/utils/vaultListStore.ts @@ -28,7 +28,7 @@ export async function loadVaultList(): Promise<{ vaults: VaultOption[]; activeVa return { vaults: checked, activeVault: data?.active_vault ?? null, hiddenDefaults: data?.hidden_defaults ?? [] } } -export function saveVaultList(vaults: VaultOption[], activeVault: string, hiddenDefaults: string[] = []): Promise { +export function saveVaultList(vaults: VaultOption[], activeVault: string | null, hiddenDefaults: string[] = []): Promise { const list: PersistedVaultList = { vaults: vaults.map(v => ({ label: v.label, path: v.path })), active_vault: activeVault, diff --git a/tests/smoke/fresh-start-telemetry-onboarding.spec.ts b/tests/smoke/fresh-start-telemetry-onboarding.spec.ts index d08b152f..d309aa12 100644 --- a/tests/smoke/fresh-start-telemetry-onboarding.spec.ts +++ b/tests/smoke/fresh-start-telemetry-onboarding.spec.ts @@ -1,8 +1,20 @@ -import { test, expect } from '@playwright/test' +import { test, expect, type Page } from '@playwright/test' -test('accepting telemetry consent on a fresh start opens the vault choice wizard @smoke', async ({ page }) => { - await page.addInitScript(() => { +const REMEMBERED_DEFAULT_VAULT_PATH = '/Volumes/Jupiter/Workspace/laputa-app/demo-vault-v2' + +async function mockFreshStart( + page: Page, + options: { + activeVault: string | null + checkExistingPath: string + rememberWelcomeDismissal?: boolean + }, +) { + await page.addInitScript((config) => { localStorage.clear() + if (config.rememberWelcomeDismissal) { + localStorage.setItem('tolaria_welcome_dismissed', '1') + } let ref: Record | null = null @@ -21,16 +33,23 @@ test('accepting telemetry consent on a fresh start opens the vault choice wizard }) ref.load_vault_list = () => ({ vaults: [], - active_vault: null, + active_vault: config.activeVault, hidden_defaults: [], }) - ref.get_default_vault_path = () => '/Users/mock/Documents/Getting Started' - ref.check_vault_exists = (args: { path?: string }) => args?.path === '/Users/mock/Documents/Getting Started' + ref.get_default_vault_path = () => config.checkExistingPath + ref.check_vault_exists = (args: { path?: string }) => args?.path === config.checkExistingPath }, get() { return ref }, }) + }, options) +} + +test('accepting telemetry consent on a fresh start opens the vault choice wizard @smoke', async ({ page }) => { + await mockFreshStart(page, { + activeVault: null, + checkExistingPath: '/Users/mock/Documents/Getting Started', }) await page.goto('/', { waitUntil: 'domcontentloaded' }) @@ -42,3 +61,21 @@ test('accepting telemetry consent on a fresh start opens the vault choice wizard await expect(page.getByTestId('welcome-open-folder')).toBeVisible() await expect(page.getByTestId('welcome-create-new')).toBeFocused() }) + +for (const action of ['accept', 'decline'] as const) { + test(`${action} telemetry still resumes onboarding with only a remembered default vault @smoke`, async ({ page }) => { + await mockFreshStart(page, { + activeVault: REMEMBERED_DEFAULT_VAULT_PATH, + checkExistingPath: REMEMBERED_DEFAULT_VAULT_PATH, + rememberWelcomeDismissal: true, + }) + + await page.goto('/', { waitUntil: 'domcontentloaded' }) + + await expect(page.getByText('Help improve Tolaria')).toBeVisible() + await page.getByTestId(`telemetry-${action}`).click() + + await expect(page.getByTestId('welcome-screen')).toBeVisible() + await expect(page.getByTestId('welcome-open-folder')).toBeVisible() + }) +}