Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 96 additions & 1 deletion src/renderer/hooks/useNotifications.test.tsx
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -81,6 +83,7 @@ describe('renderer/hooks/useNotifications.ts', () => {
raiseSoundNotificationSpy.mockClear();
raiseNativeNotificationSpy.mockClear();
getAllNotificationsMock.mockReset();
refreshEnrichmentCacheMock.mockReset();
clearServerPollIntervals();

useAccountsStore.setState({ accounts: [mockGitHubCloudAccount] });
Expand Down Expand Up @@ -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 }) => (
<QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
);

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({
Expand Down
32 changes: 31 additions & 1 deletion src/renderer/hooks/useNotifications.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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';
Expand Down Expand Up @@ -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);
Expand Down
48 changes: 48 additions & 0 deletions src/renderer/utils/notifications/notifications.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
enrichNotifications,
getNotificationCount,
getUnreadNotificationCount,
refreshEnrichmentCache,
stabilizeNotificationsOrder,
} from './notifications';

Expand Down Expand Up @@ -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();
}
});
});
});
38 changes: 36 additions & 2 deletions src/renderer/utils/notifications/notifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down