diff --git a/src/renderer/hooks/useNotifications.test.tsx b/src/renderer/hooks/useNotifications.test.tsx index 6812a3e99..6e37269e4 100644 --- a/src/renderer/hooks/useNotifications.test.tsx +++ b/src/renderer/hooks/useNotifications.test.tsx @@ -1,7 +1,7 @@ import { act, renderHook, waitFor } from '@testing-library/react'; import type { ReactNode } from 'react'; -import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { focusManager, QueryClient, QueryClientProvider } from '@tanstack/react-query'; import { mockGitHubCloudAccount, @@ -39,10 +39,12 @@ vi.mock('../utils/notifications/notifications', async () => { return { ...actual, getAllNotifications: vi.fn(), + refreshEnrichmentCache: vi.fn(), }; }); const getAllNotificationsMock = vi.mocked(notificationsUtils.getAllNotifications); +const refreshEnrichmentCacheMock = vi.mocked(notificationsUtils.refreshEnrichmentCache); const createWrapper = () => { const queryClient = new QueryClient({ @@ -81,6 +83,7 @@ describe('renderer/hooks/useNotifications.ts', () => { raiseSoundNotificationSpy.mockClear(); raiseNativeNotificationSpy.mockClear(); getAllNotificationsMock.mockReset(); + refreshEnrichmentCacheMock.mockReset(); clearServerPollIntervals(); useAccountsStore.setState({ accounts: [mockGitHubCloudAccount] }); @@ -283,6 +286,98 @@ describe('renderer/hooks/useNotifications.ts', () => { }); }); + describe('window focus', () => { + it('refreshes the enrichment cache when the window regains focus', async () => { + getAllNotificationsMock.mockResolvedValue(mockSingleAccountNotifications); + refreshEnrichmentCacheMock.mockReturnValue(false); + + useSettingsStore.setState({ fetchInterval: 1234 }); + + renderNotificationsHook(); + await waitFor(() => expect(getAllNotificationsMock).toHaveBeenCalled()); + + try { + await act(async () => { + focusManager.setFocused(false); + }); + expect(refreshEnrichmentCacheMock).not.toHaveBeenCalled(); + + await act(async () => { + focusManager.setFocused(true); + }); + + // Throttled to the configured fetch interval + expect(refreshEnrichmentCacheMock).toHaveBeenCalledWith(1234); + } finally { + await act(async () => { + focusManager.setFocused(undefined); + }); + } + }); + + it('refetches immediately when the cache is cleared on focus, even if the query is fresh', async () => { + getAllNotificationsMock.mockResolvedValue(mockSingleAccountNotifications); + refreshEnrichmentCacheMock.mockReturnValue(true); + + // A never-stale query suppresses the stale-gated `refetchOnWindowFocus` + // refetch, so any second fetch can only come from the explicit refetch + // issued when the enrichment cache is cleared. + const queryClient = new QueryClient({ + defaultOptions: { + queries: { + retry: false, + refetchOnWindowFocus: false, + refetchInterval: false, + staleTime: Number.POSITIVE_INFINITY, + }, + }, + }); + const wrapper = ({ children }: { children: ReactNode }) => ( + {children} + ); + + renderHook(() => useNotifications({ withSideEffects: true }), { wrapper }); + await waitFor(() => expect(getAllNotificationsMock).toHaveBeenCalledTimes(1)); + + try { + await act(async () => { + focusManager.setFocused(false); + }); + await act(async () => { + focusManager.setFocused(true); + }); + + await waitFor(() => expect(getAllNotificationsMock).toHaveBeenCalledTimes(2)); + } finally { + await act(async () => { + focusManager.setFocused(undefined); + }); + } + }); + + it('plain consumers do not refresh the enrichment cache on focus', async () => { + getAllNotificationsMock.mockResolvedValue(mockSingleAccountNotifications); + + renderHook(() => useNotifications(), { wrapper: createWrapper() }); + await waitFor(() => expect(getAllNotificationsMock).toHaveBeenCalled()); + + try { + await act(async () => { + focusManager.setFocused(false); + }); + await act(async () => { + focusManager.setFocused(true); + }); + + expect(refreshEnrichmentCacheMock).not.toHaveBeenCalled(); + } finally { + await act(async () => { + focusManager.setFocused(undefined); + }); + } + }); + }); + describe('sound and native notifications', () => { it('raises sound and native notifications for new notifications', async () => { useSettingsStore.setState({ diff --git a/src/renderer/hooks/useNotifications.ts b/src/renderer/hooks/useNotifications.ts index 3da487ed3..57affdc60 100644 --- a/src/renderer/hooks/useNotifications.ts +++ b/src/renderer/hooks/useNotifications.ts @@ -1,6 +1,12 @@ import { useCallback, useEffect, useMemo, useRef } from 'react'; -import { keepPreviousData, useMutation, useQuery, useQueryClient } from '@tanstack/react-query'; +import { + focusManager, + keepPreviousData, + useMutation, + useQuery, + useQueryClient, +} from '@tanstack/react-query'; import { useAccountsStore, useFiltersStore, useSettingsStore } from '../stores'; @@ -26,6 +32,7 @@ import { getAllNotifications, getNotificationCount, getUnreadNotificationCount, + refreshEnrichmentCache, } from '../utils/notifications/notifications'; import { computeRefetchIntervalMs } from '../utils/notifications/pollInterval'; import { removeNotificationsForAccount } from '../utils/notifications/remove'; @@ -244,6 +251,29 @@ export const useNotifications = ({ }); }, [withSideEffects, refetch]); + // Enriched subject details are cached per `updatedAt`, which label, + // milestone, and reaction changes never bump, so they can drift while the + // app polls in the background. Drop the cache when the window regains focus + // and refetch, so the inbox the user actually looks at re-enriches at that + // moment. The refetch is explicit because the stale-gated focus refetch + // (`refetchOnWindowFocus` above) does not fire within `staleTime`; when it + // does fire too, TanStack dedupes the concurrent fetches. Clearing is + // synchronous within the focus dispatch while any refetch reads the cache + // only after its own awaits, so the clear always lands first. Throttled to + // the fetch interval to avoid re-enrichment bursts from rapid open/close + // cycles. + useEffect(() => { + if (!withSideEffects) { + return; + } + + return focusManager.subscribe((focused) => { + if (focused && refreshEnrichmentCache(fetchIntervalMs)) { + refetch(); + } + }); + }, [withSideEffects, fetchIntervalMs, refetch]); + const removeAccountNotifications = useCallback( async (account: Account) => { const accountUUID = getAccountUUID(account); diff --git a/src/renderer/utils/notifications/notifications.test.ts b/src/renderer/utils/notifications/notifications.test.ts index 5cab69243..0a74af30d 100644 --- a/src/renderer/utils/notifications/notifications.test.ts +++ b/src/renderer/utils/notifications/notifications.test.ts @@ -24,6 +24,7 @@ import { enrichNotifications, getNotificationCount, getUnreadNotificationCount, + refreshEnrichmentCache, stabilizeNotificationsOrder, } from './notifications'; @@ -313,4 +314,51 @@ describe('renderer/utils/notifications/notifications.ts', () => { expect(fetchNotificationDetailsForListSpy).toHaveBeenCalledTimes(2); }); }); + + describe('refreshEnrichmentCache', () => { + it('should clear cached subjects and throttle repeated refreshes', async () => { + vi.useFakeTimers(); + try { + const fetchNotificationDetailsForListSpy = vi.mocked( + apiClient.fetchNotificationDetailsForList, + ); + vi.mocked(apiClient.fetchIssueByNumber).mockResolvedValue({ repository: {} } as never); + fetchNotificationDetailsForListSpy.mockResolvedValue(new Map()); + + useSettingsStore.setState({ detailedNotifications: true }); + + const notification = { + ...(mockPartialGitifyNotification({ + title: 'Issue #1', + type: 'Issue', + url: 'https://api.github.com/repos/gitify-app/notifications-test/issues/1' as Link, + }) as GitifyNotification), + id: '1', + updatedAt: '2026-01-01T00:00:00Z', + }; + + await enrichNotifications([notification]); + expect(fetchNotificationDetailsForListSpy).toHaveBeenCalledTimes(1); + + // A refresh clears the cache, so the next poll re-fetches. + expect(refreshEnrichmentCache(60000)).toBe(true); + await enrichNotifications([notification]); + expect(fetchNotificationDetailsForListSpy).toHaveBeenCalledTimes(2); + + // A second refresh within the throttle window is a no-op; the cache + // entry from the previous poll is still served. + expect(refreshEnrichmentCache(60000)).toBe(false); + await enrichNotifications([notification]); + expect(fetchNotificationDetailsForListSpy).toHaveBeenCalledTimes(2); + + // Once the throttle window has passed, refreshing clears again. + vi.advanceTimersByTime(60001); + expect(refreshEnrichmentCache(60000)).toBe(true); + await enrichNotifications([notification]); + expect(fetchNotificationDetailsForListSpy).toHaveBeenCalledTimes(3); + } finally { + vi.useRealTimers(); + } + }); + }); }); diff --git a/src/renderer/utils/notifications/notifications.ts b/src/renderer/utils/notifications/notifications.ts index e7d53a89d..9e8ced7d6 100644 --- a/src/renderer/utils/notifications/notifications.ts +++ b/src/renderer/utils/notifications/notifications.ts @@ -217,11 +217,45 @@ function enrichmentCacheKey(notification: RawGitifyNotification): string { } /** - * Clear the enrichment cache. Intended for use in tests to keep the - * module-level cache from leaking state across cases. + * Timestamp of the last {@link refreshEnrichmentCache} that actually cleared + * the cache, used to throttle focus-triggered refreshes. + */ +let lastEnrichmentCacheRefreshAt = 0; + +/** + * Drop all cached subjects so the next enrichment pass re-fetches every + * notification. Called when the window regains focus: some subject changes + * (labels, milestones, reactions) never bump a notification's `updatedAt`, + * so cached details can drift while the app polls in the background. Clearing + * at the moment the user looks at the inbox bounds that drift to the moment + * of viewing. + * + * Throttled so rapid window open/close cycles cost at most one extra + * enrichment pass per `throttleMs`. + * + * @param throttleMs - Minimum time between cache-clearing refreshes. + * @returns Whether the cache was cleared (`false` when throttled), so callers + * can trigger an immediate re-enrichment pass only when needed. + */ +export function refreshEnrichmentCache(throttleMs: number): boolean { + const now = Date.now(); + if (now - lastEnrichmentCacheRefreshAt < throttleMs) { + return false; + } + + lastEnrichmentCacheRefreshAt = now; + enrichmentCache.clear(); + + return true; +} + +/** + * Clear the enrichment cache and its refresh throttle. Intended for use in + * tests to keep the module-level state from leaking across cases. */ export function clearEnrichmentCache(): void { enrichmentCache.clear(); + lastEnrichmentCacheRefreshAt = 0; } /**