diff --git a/src/renderer/__helpers__/hook-mocks.ts b/src/renderer/__helpers__/hook-mocks.ts index 95016f9ef..631795783 100644 --- a/src/renderer/__helpers__/hook-mocks.ts +++ b/src/renderer/__helpers__/hook-mocks.ts @@ -29,6 +29,8 @@ function buildNotificationsDefaults(): NotificationsState { status: 'success', globalError: undefined, + isFetching: false, + notifications: [], notificationCount: 0, unreadNotificationCount: 0, diff --git a/src/renderer/components/Sidebar.test.tsx b/src/renderer/components/Sidebar.test.tsx index 96becd089..c2fd342ef 100644 --- a/src/renderer/components/Sidebar.test.tsx +++ b/src/renderer/components/Sidebar.test.tsx @@ -188,6 +188,26 @@ describe('renderer/components/Sidebar.tsx', () => { expect(fetchNotificationsMock).not.toHaveBeenCalled(); }); + + it('animates the refresh icon while a background fetch is in flight, regardless of settled status', () => { + renderWithProviders(, { + accounts: [mockGitHubCloudAccount], + status: 'error', + isFetching: true, + }); + + expect(screen.getByTestId('sidebar-refresh')).toHaveClass('animate-spin'); + }); + + it('does not animate the refresh icon when settled and no fetch is in flight', () => { + renderWithProviders(, { + accounts: [mockGitHubCloudAccount], + status: 'error', + isFetching: false, + }); + + expect(screen.getByTestId('sidebar-refresh')).not.toHaveClass('animate-spin'); + }); }); describe('Settings', () => { diff --git a/src/renderer/components/Sidebar.tsx b/src/renderer/components/Sidebar.tsx index f65571e2f..03d3b38ff 100644 --- a/src/renderer/components/Sidebar.tsx +++ b/src/renderer/components/Sidebar.tsx @@ -22,7 +22,7 @@ import { useAccountsStore, useFiltersStore, useSettingsStore } from '../stores'; import { LogoIcon } from './icons/LogoIcon'; export const Sidebar: FC = () => { - const { status, notificationCount, hasUnreadNotifications } = useNotifications(); + const { status, notificationCount, hasUnreadNotifications, isFetching } = useNotifications(); const { shortcuts } = useShortcutActions(); @@ -124,7 +124,7 @@ export const Sidebar: FC = () => { <> { }); }); + describe('status stability during background refetches', () => { + it('keeps status as error while a retry during an ongoing outage is in flight', async () => { + let rejectRetry: (() => void) | undefined; + getAllNotificationsMock + .mockResolvedValueOnce(mockSingleAccountNotifications) + .mockRejectedValueOnce(new Error('network error')) + .mockImplementationOnce( + () => + new Promise((_resolve, reject) => { + rejectRetry = () => reject(new Error('network error')); + }), + ); + + const { result } = renderNotificationsHook(); + + // Establish a prior successful fetch, then let the next poll fail - + // mirroring an outage starting after notifications had already loaded. + await waitFor(() => expect(result.current.status).toBe('success')); + + act(() => { + result.current.refetchNotifications(); + }); + await waitFor(() => expect(result.current.status).toBe('error')); + expect(result.current.isFetching).toBe(false); + + // Trigger a retry that stays unsettled. + act(() => { + result.current.refetchNotifications(); + }); + + await waitFor(() => expect(result.current.isFetching).toBe(true)); + expect(result.current.status).toBe('error'); + + await act(async () => { + rejectRetry?.(); + }); + + await waitFor(() => expect(result.current.isFetching).toBe(false)); + expect(result.current.status).toBe('error'); + }); + + it('keeps status as success while a background refetch of loaded data is in flight', async () => { + let resolveRefetch: ((data: AccountNotifications[]) => void) | undefined; + getAllNotificationsMock + .mockResolvedValueOnce(mockSingleAccountNotifications) + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveRefetch = resolve; + }), + ); + + const { result } = renderNotificationsHook(); + + await waitFor(() => expect(result.current.status).toBe('success')); + expect(result.current.isFetching).toBe(false); + + // Trigger a background refetch that stays unsettled. + act(() => { + result.current.refetchNotifications(); + }); + + await waitFor(() => expect(result.current.isFetching).toBe(true)); + expect(result.current.status).toBe('success'); + + await act(async () => { + resolveRefetch?.(mockSingleAccountNotifications); + }); + + await waitFor(() => expect(result.current.isFetching).toBe(false)); + expect(result.current.status).toBe('success'); + }); + }); + describe('polling', () => { it('only polls once per interval, regardless of consumer count', async () => { vi.useFakeTimers(); diff --git a/src/renderer/hooks/useNotifications.ts b/src/renderer/hooks/useNotifications.ts index 5cfd87f01..a2164a075 100644 --- a/src/renderer/hooks/useNotifications.ts +++ b/src/renderer/hooks/useNotifications.ts @@ -59,6 +59,9 @@ interface NotificationsState { status: Status; globalError: GitifyError | undefined; + /** Whether a fetch (initial, background poll, or manual refetch) is currently in flight. */ + isFetching: boolean; + notifications: AccountNotifications[]; notificationCount: number; unreadNotificationCount: number; @@ -212,9 +215,12 @@ export const useNotifications = ({ [unreadNotificationCount], ); - // Determine status and globalError from query state + // Determine status and globalError from query state. Only the initial + // fetch (no settled success or error yet) reports 'loading' - background + // refetches of already-settled data keep their settled status so + // consumers like the tray icon don't flicker on every poll/retry. const status: Status = useMemo(() => { - if (isLoading || isFetching) { + if (isLoading) { return 'loading'; } @@ -233,7 +239,7 @@ export const useNotifications = ({ } return 'success'; - }, [isLoading, isFetching, isPaused, isError, notifications]); + }, [isLoading, isPaused, isError, notifications]); const globalError: GitifyError | undefined = useMemo(() => { // If paused due to offline, show network error @@ -634,6 +640,8 @@ export const useNotifications = ({ status, globalError, + isFetching, + notifications, notificationCount, unreadNotificationCount,