From 51e1f4d3752f907d7dfef35167587154e274598a Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Tue, 13 Oct 2026 23:55:12 -0400 Subject: [PATCH 1/4] refactor(renderer): codify module boundaries and decompose notification state Make the renderer's architecture explicit and enforced, then split the single notification hook into focused modules. - Add ARCHITECTURE.md documenting the one-way layer direction, state ownership per kind, and the single-host side-effect pattern; point to it from CONTRIBUTING.md - Enforce the layer direction with oxlint no-restricted-imports overrides, treating `utils` and `stores` as peer domain layers - Move all icon components into a leaf `src/renderer/icons/` so lower layers can import them without an exception - Fix the non-reactive `hasMultipleAccounts` selector in AppearanceSettings - Add pure `utils/notifications/status.ts` derivation and consume it from the route and the hook - Split `useNotifications` into query / mutations / effects / host hooks and remove the `withSideEffects` flag; the public reader API is unchanged Verified with tsc, oxlint/format checks, and the full test suite. --- ARCHITECTURE.md | 79 ++ CONTRIBUTING.md | 1 + src/renderer/__helpers__/vitest.setup.ts | 11 + src/renderer/components/GlobalEffects.tsx | 4 +- src/renderer/components/Sidebar.tsx | 2 +- .../settings/AppearanceSettings.test.tsx | 25 +- .../settings/AppearanceSettings.tsx | 4 +- .../components/settings/SystemSettings.tsx | 4 +- .../notifications/useNotificationEffects.ts | 170 +++++ .../notifications/useNotificationMutations.ts | 319 ++++++++ .../notifications/useNotificationsHost.ts | 31 + .../notifications/useNotificationsQuery.ts | 214 ++++++ src/renderer/hooks/useNotifications.test.tsx | 16 +- src/renderer/hooks/useNotifications.ts | 678 ++---------------- .../{components => }/icons/BitbucketIcon.tsx | 0 .../{components => }/icons/GitLabIcon.tsx | 0 .../{components => }/icons/LogoIcon.test.tsx | 6 +- .../{components => }/icons/LogoIcon.tsx | 4 +- .../icons/ScopeStatusIcon.test.tsx | 4 +- .../icons/ScopeStatusIcon.tsx | 0 .../{components => }/icons/VolumeDownIcon.tsx | 0 .../{components => }/icons/VolumeUpIcon.tsx | 0 .../__snapshots__/LogoIcon.test.tsx.snap | 10 +- .../ScopeStatusIcon.test.tsx.snap | 8 +- src/renderer/routes/AccountScopes.tsx | 2 +- src/renderer/routes/Login.tsx | 2 +- src/renderer/routes/Notifications.tsx | 7 +- .../utils/forges/bitbucket/adapter.test.ts | 3 +- .../utils/forges/bitbucket/adapter.ts | 3 +- .../utils/forges/gitlab/adapter.test.ts | 3 +- src/renderer/utils/forges/gitlab/adapter.ts | 3 +- .../utils/notifications/status.test.ts | 93 +++ src/renderer/utils/notifications/status.ts | 87 +++ src/renderer/utils/ui/icons.ts | 5 +- vite.config.mts | 74 ++ 35 files changed, 1187 insertions(+), 685 deletions(-) create mode 100644 ARCHITECTURE.md create mode 100644 src/renderer/hooks/notifications/useNotificationEffects.ts create mode 100644 src/renderer/hooks/notifications/useNotificationMutations.ts create mode 100644 src/renderer/hooks/notifications/useNotificationsHost.ts create mode 100644 src/renderer/hooks/notifications/useNotificationsQuery.ts rename src/renderer/{components => }/icons/BitbucketIcon.tsx (100%) rename src/renderer/{components => }/icons/GitLabIcon.tsx (100%) rename src/renderer/{components => }/icons/LogoIcon.test.tsx (88%) rename src/renderer/{components => }/icons/LogoIcon.tsx (96%) rename src/renderer/{components => }/icons/ScopeStatusIcon.test.tsx (95%) rename src/renderer/{components => }/icons/ScopeStatusIcon.tsx (100%) rename src/renderer/{components => }/icons/VolumeDownIcon.tsx (100%) rename src/renderer/{components => }/icons/VolumeUpIcon.tsx (100%) rename src/renderer/{components => }/icons/__snapshots__/LogoIcon.test.tsx.snap (96%) rename src/renderer/{components => }/icons/__snapshots__/ScopeStatusIcon.test.tsx.snap (81%) create mode 100644 src/renderer/utils/notifications/status.test.ts create mode 100644 src/renderer/utils/notifications/status.ts diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md new file mode 100644 index 000000000..117ac31c9 --- /dev/null +++ b/ARCHITECTURE.md @@ -0,0 +1,79 @@ +# Architecture + +Gitify is an Electron menubar app that monitors notifications across supported Git forges. This document describes how the code is organised so the structure stays coherent as the app grows. It is the reference for where new code belongs. + +## Processes + +Gitify runs as three Electron processes plus a small shared module: + +| Location | Role | +| --------------- | -------------------------------------------------------------------------------- | +| `src/main/` | Electron main process: window/menubar lifecycle, tray, handlers, updater | +| `src/preload/` | The only bridge between renderer and main, exposed as a typed `window.gitify` | +| `src/shared/` | Code imported by **both** processes (constants, events, logger, platform, theme) | +| `src/renderer/` | The React application | + +The renderer never reaches into the main process directly: it goes through `window.gitify`. A capability that requires Node or Electron APIs belongs in the main process behind an IPC handler. + +## Renderer layers + +The renderer is layered. **Imports flow in one direction only** — a module may import from its own layer or any layer below it, never above: + +``` + types / constants / icons (leaf: shapes, values, presentational icons) + | + +----+----+ + | | + utils stores peer domain layers +(domain (client state, + logic) Zustand) + | | + +----+----+ + | + hooks orchestration over stores, query, and utils + | + components presentational React components + | + routes screens + | + App.tsx / index.tsx composition root - may import any layer +``` + +- `utils/` is split by concern: `api`, `auth`, `core`, `forges`, `notifications`, `system`, `ui`. +- `stores/` holds client state (Zustand): persisted `accounts` and `settings`, session `filters` and notification action failures. +- `icons/` holds presentational icon components (brand marks and UI glyphs). It is a **leaf**, below `utils`, so both `utils` and `components` may import it — e.g. a forge adapter declares its brand mark from here. +- `utils` and `stores` are **peers** — they may reference each other. Neither may import `hooks`, `components`, or `routes`. +- `components/` is grouped by feature (`notifications`, `filters`, `settings`, `login`, `fields`, `avatars`, `layout`, `metrics`, `primitives`). +- The composition root (`App.tsx`, `index.tsx`) is the only place allowed to wire every layer together. + +## State ownership + +Every kind of renderer state has exactly one owner. Put new state with the mechanism that owns its kind: + +| Kind of state | Owner | Examples | +| ---------------------- | ---------------------------------------------------- | -------------------------------------------------------- | +| Server / async data | TanStack Query | the notification list, periodic account refreshes | +| Persisted client state | Zustand with `persist` | `accounts`, `settings` | +| Session client state | Zustand without `persist` | `filters`, notification action failures | +| Derived values | A store selector/action, or a pure `utils/` function | `isLoggedIn`, `hasMultipleAccounts`, notification status | +| Ephemeral view state | React `useState` / `useMemo` | row expand/collapse, exit animations | + +Rules: + +- **Application state is never owned by a React context.** `AppContext` was retired in favour of hooks and stores; do not reintroduce a context provider to hold app state. +- **Derived values are not stored.** If a value can be computed from existing state, expose it as a selector or a pure function instead of adding new state. Derived predicates are read reactively (invoke the store accessor inside the selector) so consumers update when their source changes. +- Query state is server state: never mirror it into a store. + +## Side-effect ownership + +App-level side effects are owned by a single host. `GlobalEffects` mounts exactly once (inside the query and theme providers) and owns theme application, tray updates, global shortcut registration, periodic account refreshes, and notification polling and alerts. + +Consumers read the shared query cache and stores, and **do not register their own** polling, alerts, wake, or focus listeners. This keeps background work (for example, firing one poll per interval) from multiplying with the number of mounted components. When you add a new background effect, add it to the host, not to a leaf component. + +## Forge adapters + +Multi-forge support is adapter-based. Each forge implements the `ForgeAdapter` interface in `src/renderer/utils/forges/`, and shared code routes through `getAdapter(forge)` (forge-wide members) or `getAccountAdapter(account)` (account-scoped operations). Shared code never imports forge-specific modules directly, and there is no forge-specific branching outside an adapter module. See `CONTRIBUTING.md` for the conditions for adding a forge. + +## Enforcement + +The one-way module dependency direction is checked automatically: `vite.config.mts` defines per-layer `no-restricted-imports` overrides for the renderer, and `pnpm check` fails on a violation. The enforced rules are: `utils` and `stores` must not import `hooks`, `components`, or `routes`; `hooks` must not import `components` or `routes`; and `components` must not import `routes`. Any future exception must be argued and documented here rather than silently allow-listed. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d04bac4bb..38c935d3f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -89,6 +89,7 @@ the macOS native vibrancy material, which Chromium cannot render. - Linting and formatting are configured in `vite.config.mts` (the `lint` and `fmt` blocks). Please run `pnpm check` before submitting a PR. - Follow existing file and folder naming conventions. +- See [ARCHITECTURE.md](./ARCHITECTURE.md) for the renderer's module layering, state ownership, and side-effect conventions. - Keep commit messages clear and descriptive. ### How to Report Bugs or Request Features diff --git a/src/renderer/__helpers__/vitest.setup.ts b/src/renderer/__helpers__/vitest.setup.ts index 88ed7354e..8245e34a2 100644 --- a/src/renderer/__helpers__/vitest.setup.ts +++ b/src/renderer/__helpers__/vitest.setup.ts @@ -36,6 +36,17 @@ vi.mock('../hooks/useNotifications', async () => { }; }); +vi.mock('../hooks/notifications/useNotificationsHost', async () => { + const actual = await vi.importActual< + typeof import('../hooks/notifications/useNotificationsHost') + >('../hooks/notifications/useNotificationsHost'); + const { getMockedNotificationsState } = await import('./hook-mocks'); + return { + ...actual, + useNotificationsHost: () => getMockedNotificationsState(), + }; +}); + vi.mock('../hooks/useLogins', async () => { const actual = await vi.importActual('../hooks/useLogins'); const { getMockedLoginsState } = await import('./hook-mocks'); diff --git a/src/renderer/components/GlobalEffects.tsx b/src/renderer/components/GlobalEffects.tsx index 8a154ba05..2114906d7 100644 --- a/src/renderer/components/GlobalEffects.tsx +++ b/src/renderer/components/GlobalEffects.tsx @@ -2,9 +2,9 @@ import { type FC, useEffect } from 'react'; import { useQueryClient } from '@tanstack/react-query'; +import { useNotificationsHost } from '../hooks/notifications/useNotificationsHost'; import { useAccounts } from '../hooks/useAccounts'; import { useAppearance } from '../hooks/useAppearance'; -import { useNotifications } from '../hooks/useNotifications'; import { useOnlineStatus } from '../hooks/useOnlineStatus'; import { useShortcutRegistration, @@ -40,7 +40,7 @@ export const GlobalEffects: FC = () => { // Notification fetching side effects (sound / native alerts, interval/focus/reconnect // polling, and the system-wake refetch) - const { status, notificationCount } = useNotifications({ withSideEffects: true }); + const { status, notificationCount } = useNotificationsHost(); // Periodic account refreshes (startup + hourly interval) useAccounts(); diff --git a/src/renderer/components/Sidebar.tsx b/src/renderer/components/Sidebar.tsx index 6bafb3bba..2e3c38941 100644 --- a/src/renderer/components/Sidebar.tsx +++ b/src/renderer/components/Sidebar.tsx @@ -19,7 +19,7 @@ import { useNotifications } from '../hooks/useNotifications'; import { useShortcutActions } from '../hooks/useShortcutActions'; import { useAccountsStore, useFiltersStore, useSettingsStore } from '../stores'; -import { LogoIcon } from './icons/LogoIcon'; +import { LogoIcon } from '../icons/LogoIcon'; export const Sidebar: FC = () => { const { status, notificationCount, hasUnreadNotifications, isFetching } = useNotifications(); diff --git a/src/renderer/components/settings/AppearanceSettings.test.tsx b/src/renderer/components/settings/AppearanceSettings.test.tsx index 29935a725..d776e334b 100644 --- a/src/renderer/components/settings/AppearanceSettings.test.tsx +++ b/src/renderer/components/settings/AppearanceSettings.test.tsx @@ -2,9 +2,12 @@ import { act, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { renderWithProviders } from '../../__helpers__/test-utils'; -import { mockGitHubAppAccount } from '../../__mocks__/account-mocks'; +import { + mockGitHubAppAccount, + mockGitHubEnterpriseServerAccount, +} from '../../__mocks__/account-mocks'; -import { useSettingsStore } from '../../stores'; +import { useAccountsStore, useSettingsStore } from '../../stores'; import { DesignLanguage } from '../../types'; @@ -128,4 +131,22 @@ describe('renderer/components/settings/AppearanceSettings.tsx', () => { expect(screen.queryByTestId('checkbox-showStatusIconColors')).not.toBeInTheDocument(); }); + + it('reactively hides the account header checkbox when a second account is added', async () => { + await act(async () => { + renderWithProviders(, { + accounts: [mockGitHubAppAccount], + }); + }); + + expect(screen.getByTestId('checkbox-showAccountHeader')).toBeInTheDocument(); + + await act(async () => { + useAccountsStore.setState({ + accounts: [mockGitHubAppAccount, mockGitHubEnterpriseServerAccount], + }); + }); + + expect(screen.queryByTestId('checkbox-showAccountHeader')).not.toBeInTheDocument(); + }); }); diff --git a/src/renderer/components/settings/AppearanceSettings.tsx b/src/renderer/components/settings/AppearanceSettings.tsx index fcd76447f..20399fe54 100644 --- a/src/renderer/components/settings/AppearanceSettings.tsx +++ b/src/renderer/components/settings/AppearanceSettings.tsx @@ -22,7 +22,7 @@ import { export const AppearanceSettings: FC = () => { // Account store values - const hasMultipleAccounts = useAccountsStore((s) => s.hasMultipleAccounts); + const hasMultipleAccounts = useAccountsStore((s) => s.hasMultipleAccounts()); // Setting store actions const toggleSetting = useSettingsStore((s) => s.toggleSetting); @@ -174,7 +174,7 @@ export const AppearanceSettings: FC = () => { notifications list. } - visible={!hasMultipleAccounts()} + visible={!hasMultipleAccounts} /> void; + fetchIntervalMs: number; +} + +/** + * The side-effect half of notification state: sound and native alerts for new + * notifications, refetch on system wake and window focus, and pruning of + * session-local failure entries. + * + * Kept separate from the query and mutations so the side effects can be owned + * by a single host rather than repeated per consumer. + */ +export const useNotificationEffects = ({ + enabled, + notifications, + isLoading, + isError, + notificationsQueryKey, + refetch, + fetchIntervalMs, +}: NotificationEffectsInput): void => { + const queryClient = useQueryClient(); + const previousNotificationsRef = useRef([]); + + const playSoundNewNotifications = useSettingsStore((s) => s.playSound); + const showSystemNotifications = useSettingsStore((s) => s.showNotifications); + const notificationVolume = useSettingsStore((s) => s.notificationVolume); + + // Refetch notifications immediately when the system wakes from sleep or the + // user unlocks their screen, without waiting for the next poll interval. + // Owned by the singleton side-effects host: every consumer registering a + // wake listener would both multiply refetches and leak ipcRenderer + // listeners on unmount. + useEffect(() => { + if (!enabled) { + return; + } + + return window.gitify.onSystemWake(() => { + refetch(); + }); + }, [enabled, 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 (!enabled) { + return; + } + + return focusManager.subscribe((focused) => { + if (focused && refreshEnrichmentCache(fetchIntervalMs)) { + refetch(); + } + }); + }, [enabled, fetchIntervalMs, refetch]); + + // Handle sound and native notifications when new notifications arrive + useEffect(() => { + if (!enabled || isLoading || isError || notifications.length === 0) { + return; + } + + const allAccountsHaveErrors = doesAllAccountsHaveErrors(notifications); + if (allAccountsHaveErrors) { + return; + } + + const unfilteredNotifications = + queryClient.getQueryData(notificationsQueryKey) || []; + + const diffNotifications = getNewNotifications( + previousNotificationsRef.current, + unfilteredNotifications, + ); + + if (diffNotifications.length > 0) { + // Apply filters to new notifications so only filtered notifications trigger alerts + const filteredDiffNotifications = filterDetailedNotifications( + filterBaseNotifications(diffNotifications), + ) as GitifyNotification[]; + + if (filteredDiffNotifications.length > 0) { + if (playSoundNewNotifications) { + void raiseSoundNotification(notificationVolume); + } + + if (showSystemNotifications) { + void raiseNativeNotification(filteredDiffNotifications); + } + } + } + + previousNotificationsRef.current = unfilteredNotifications; + }, [ + enabled, + notifications, + isLoading, + isError, + playSoundNewNotifications, + showSystemNotifications, + notificationVolume, + queryClient, + notificationsQueryKey, + ]); + + // Session-local failure entries are independent of the notifications + // cache, so they must be pruned separately once a notification no longer + // appears in the (unfiltered) list - e.g. actioned successfully elsewhere, + // or its account was removed. Owned by the singleton side-effects host so + // it runs once per notifications update rather than once per mounted + // row/consumer. + useEffect(() => { + if (!enabled) { + return; + } + + const unfilteredNotifications = + queryClient.getQueryData(notificationsQueryKey) || []; + + const currentNotificationKeys = unfilteredNotifications.flatMap((accountNotifications) => + accountNotifications.notifications.map((notification) => + getNotificationFailureKey(notification.account, notification.id), + ), + ); + + useNotificationActionFailuresStore.getState().pruneFailures(currentNotificationKeys); + }, [enabled, notifications, queryClient, notificationsQueryKey]); +}; diff --git a/src/renderer/hooks/notifications/useNotificationMutations.ts b/src/renderer/hooks/notifications/useNotificationMutations.ts new file mode 100644 index 000000000..81cce585a --- /dev/null +++ b/src/renderer/hooks/notifications/useNotificationMutations.ts @@ -0,0 +1,319 @@ +import { useCallback } from 'react'; + +import { type QueryKey, useMutation, useQueryClient } from '@tanstack/react-query'; + +import { + getNotificationFailureKey, + type NotificationFailedActionType, + useNotificationActionFailuresStore, + useSettingsStore, +} from '../../stores'; + +import type { AccountNotifications, GitifyNotification } from '../../types'; + +import { + isMarkAsDoneFeatureSupported, + isUnsubscribeThreadSupported, +} from '../../utils/api/features'; +import { notificationsKeys } from '../../utils/api/queryKeys'; +import { rendererLogError, toError } from '../../utils/core/logger'; +import { getAccountAdapter } from '../../utils/forges/registry'; +import { + type FailedNotificationAction, + type NotificationQuerySnapshot, + restoreFailedNotifications, + settleNotificationActions, +} from '../../utils/notifications/mutations'; +import { removeNotificationsForAccount } from '../../utils/notifications/remove'; + +export interface NotificationMutations { + markNotificationsAsRead: (notifications: GitifyNotification[]) => Promise; + markNotificationsAsDone: (notifications: GitifyNotification[]) => Promise; + unsubscribeNotification: (notification: GitifyNotification) => Promise; +} + +/** + * The write side of notification state: mark-as-read, mark-as-done, and + * unsubscribe, each reconciling per-notification failures against the query + * cache and the session-local failure map. + * + * Takes the caller's `notificationsQueryKey` so cache updates target the same + * key the reader observes. Side effects live in the host hook, not here. + */ +export const useNotificationMutations = ( + notificationsQueryKey: QueryKey, +): NotificationMutations => { + const queryClient = useQueryClient(); + + const markAsDoneOnUnsubscribe = useSettingsStore((s) => s.markAsDoneOnUnsubscribe); + + // Shared by all three mutations' `onSuccess`. Records each failure's + // classified error in the session-local failure map and logs it; also + // re-applies `snapshot` for any failed notification missing from the + // cache, as a defensive guard (these mutations don't remove notifications + // until they succeed, so this is normally a no-op). + const reconcileFailedNotifications = useCallback( + ( + failed: FailedNotificationAction[], + snapshot: NotificationQuerySnapshot | undefined, + action: NotificationFailedActionType, + context: string, + ) => { + if (failed.length === 0) { + return; + } + + const failedNotifications = failed.map((f) => f.notification); + + if (snapshot) { + for (const [queryKey, snapshotData] of snapshot) { + queryClient.setQueryData(queryKey, (existing) => + restoreFailedNotifications(failedNotifications, snapshotData ?? [], existing ?? []), + ); + } + } + + for (const { notification, error, rawError } of failed) { + const notificationKey = getNotificationFailureKey(notification.account, notification.id); + useNotificationActionFailuresStore + .getState() + .setFailure(notificationKey, { action, error }); + rendererLogError( + context, + `Error occurred while processing notification ${notification.id}`, + rawError, + ); + } + }, + [queryClient], + ); + + // Full-cache restore for `onError`, covering the rare case where a + // mutation function throws directly (e.g. a bug) rather than resolving via + // `settleNotificationActions`. Per-notification failures are handled by + // `reconcileFailedNotifications` above instead. + const restoreSnapshot = useCallback( + (snapshot?: NotificationQuerySnapshot) => { + if (!snapshot) { + return; + } + + for (const [queryKey, data] of snapshot) { + queryClient.setQueryData(queryKey, data); + } + }, + [queryClient], + ); + + const snapshotNotifications = useCallback(async () => { + // Prevent an older in-flight poll from overwriting the mutation's eventual cache update. + await queryClient.cancelQueries({ queryKey: notificationsKeys.all }); + + const snapshot = queryClient.getQueriesData({ + queryKey: notificationsKeys.all, + }); + + return { snapshot }; + }, [queryClient]); + + const createMutationErrorHandler = useCallback( + (logContext: string, message: string) => + (err: Error, _variables: unknown, context?: { snapshot: NotificationQuerySnapshot }) => { + restoreSnapshot(context?.snapshot); + rendererLogError(logContext, message, toError(err)); + }, + [restoreSnapshot], + ); + + const markNotificationsAsReadMutation = useMutation({ + mutationFn: async ({ readNotifications }: { readNotifications: GitifyNotification[] }) => { + return await settleNotificationActions(readNotifications, (notification) => + getAccountAdapter(notification.account).markThreadAsRead(notification.id), + ); + }, + + onMutate: snapshotNotifications, + + onSuccess: ({ succeeded, failed }, _variables, context) => { + // Cache removal happens here (once the request resolves) rather than + // optimistically in `onMutate`, so the row's exit animation - started + // synchronously on click - has time to play before the notification + // disappears from the list. + if (succeeded.length > 0) { + queryClient.setQueryData(notificationsQueryKey, (existing) => + removeNotificationsForAccount(succeeded[0].account, succeeded, existing ?? []), + ); + } + + useNotificationActionFailuresStore + .getState() + .clearFailures( + succeeded.map((notification) => + getNotificationFailureKey(notification.account, notification.id), + ), + ); + + reconcileFailedNotifications( + failed, + context?.snapshot, + 'markAsRead', + 'markNotificationsAsRead', + ); + }, + + onError: createMutationErrorHandler( + 'markNotificationsAsRead', + 'Error occurred while marking notifications as read', + ), + }); + + const markNotificationsAsDoneMutation = useMutation({ + mutationFn: async ({ doneNotifications }: { doneNotifications: GitifyNotification[] }) => { + const account = doneNotifications[0].account; + + // Forges that don't support a distinct "done" state fall back to + // marking as read so the user-visible action still removes the thread. + if (!isMarkAsDoneFeatureSupported(account)) { + return await markNotificationsAsReadMutation.mutateAsync({ + readNotifications: doneNotifications, + }); + } + + return await settleNotificationActions(doneNotifications, (notification) => + getAccountAdapter(notification.account).markThreadAsDone(notification.id), + ); + }, + + onMutate: snapshotNotifications, + + onSuccess: ({ succeeded, failed }, { doneNotifications }, context) => { + // The mark-as-read fallback (for forges without a distinct "done" + // state) already updated the cache via its own mutation/onSuccess. + if (succeeded.length > 0 && isMarkAsDoneFeatureSupported(doneNotifications[0].account)) { + queryClient.setQueryData(notificationsQueryKey, (existing) => + removeNotificationsForAccount(succeeded[0].account, succeeded, existing ?? []), + ); + } + + useNotificationActionFailuresStore + .getState() + .clearFailures( + succeeded.map((notification) => + getNotificationFailureKey(notification.account, notification.id), + ), + ); + + if (isMarkAsDoneFeatureSupported(doneNotifications[0].account)) { + reconcileFailedNotifications( + failed, + context?.snapshot, + 'markAsDone', + 'markNotificationsAsDone', + ); + } + }, + + onError: createMutationErrorHandler( + 'markNotificationsAsDone', + 'Error occurred while marking notifications as done', + ), + }); + + const unsubscribeNotificationMutation = useMutation({ + mutationFn: async ({ notification }: { notification: GitifyNotification }) => { + // Forges without thread-subscription support cannot unsubscribe; the UI + // already hides the action, but treat duplicate calls as no-ops. + if (!isUnsubscribeThreadSupported(notification.account)) { + return { succeeded: [notification], failed: [] }; + } + + const result = await settleNotificationActions([notification], (n) => + getAccountAdapter(n.account).unsubscribeThread(n.id), + ); + + if (result.failed.length > 0) { + return result; + } + + const followUp = markAsDoneOnUnsubscribe + ? await markNotificationsAsDoneMutation.mutateAsync({ + doneNotifications: [notification], + }) + : await markNotificationsAsReadMutation.mutateAsync({ + readNotifications: [notification], + }); + + return followUp.failed.length > 0 ? followUp : result; + }, + + onMutate: snapshotNotifications, + + onSuccess: ({ succeeded, failed }, _variables, context) => { + useNotificationActionFailuresStore + .getState() + .clearFailures( + succeeded.map((notification) => + getNotificationFailureKey(notification.account, notification.id), + ), + ); + + reconcileFailedNotifications( + failed, + context?.snapshot, + 'unsubscribe', + 'unsubscribeNotification', + ); + }, + + onError: createMutationErrorHandler( + 'unsubscribeNotification', + 'Error occurred while unsubscribing from notification thread', + ), + }); + + const markNotificationsAsRead = useCallback( + async (readNotifications: GitifyNotification[]) => { + try { + const { failed } = await markNotificationsAsReadMutation.mutateAsync({ + readNotifications, + }); + return failed.length === 0; + } catch { + return false; + } + }, + [markNotificationsAsReadMutation], + ); + + const markNotificationsAsDone = useCallback( + async (doneNotifications: GitifyNotification[]) => { + try { + const { failed } = await markNotificationsAsDoneMutation.mutateAsync({ + doneNotifications, + }); + return failed.length === 0; + } catch { + return false; + } + }, + [markNotificationsAsDoneMutation], + ); + + const unsubscribeNotification = useCallback( + async (notification: GitifyNotification) => { + try { + const { failed } = await unsubscribeNotificationMutation.mutateAsync({ notification }); + return failed.length === 0; + } catch { + return false; + } + }, + [unsubscribeNotificationMutation], + ); + + return { + markNotificationsAsRead, + markNotificationsAsDone, + unsubscribeNotification, + }; +}; diff --git a/src/renderer/hooks/notifications/useNotificationsHost.ts b/src/renderer/hooks/notifications/useNotificationsHost.ts new file mode 100644 index 000000000..3de9f579c --- /dev/null +++ b/src/renderer/hooks/notifications/useNotificationsHost.ts @@ -0,0 +1,31 @@ +import { buildNotificationsState, type NotificationsState } from '../useNotifications'; +import { useNotificationEffects } from './useNotificationEffects'; +import { useNotificationMutations } from './useNotificationMutations'; +import { useNotificationsQuery } from './useNotificationsQuery'; + +/** + * The single app-level notification host. + * + * Owns the polling observer and every notification side effect (sound and + * native alerts, wake and focus refetch, and failure pruning). Mount it exactly + * once, from `GlobalEffects`. Every other consumer uses the side-effect-free + * `useNotifications` and shares the same query cache. + * + * @returns The same reader/actions surface as `useNotifications`. + */ +export const useNotificationsHost = (): NotificationsState => { + const query = useNotificationsQuery({ poll: true }); + const mutations = useNotificationMutations(query.notificationsQueryKey); + + useNotificationEffects({ + enabled: true, + notifications: query.notifications, + isLoading: query.isLoading, + isError: query.isError, + notificationsQueryKey: query.notificationsQueryKey, + refetch: query.refetch, + fetchIntervalMs: query.fetchIntervalMs, + }); + + return buildNotificationsState(query, mutations); +}; diff --git a/src/renderer/hooks/notifications/useNotificationsQuery.ts b/src/renderer/hooks/notifications/useNotificationsQuery.ts new file mode 100644 index 000000000..8810a1e0f --- /dev/null +++ b/src/renderer/hooks/notifications/useNotificationsQuery.ts @@ -0,0 +1,214 @@ +import { useCallback, useMemo } from 'react'; + +import { keepPreviousData, type QueryKey, useQuery, useQueryClient } from '@tanstack/react-query'; + +import { Constants } from '../../constants'; + +import { useAccountsStore, useFiltersStore, useSettingsStore } from '../../stores'; + +import type { + Account, + AccountNotifications, + GitifyError, + GitifyNotification, + Status, +} from '../../types'; + +import { notificationsKeys } from '../../utils/api/queryKeys'; +import { getAccountUUID } from '../../utils/auth/utils'; +import { + filterBaseNotifications, + filterDetailedNotifications, +} from '../../utils/notifications/filters/filter'; +import { + getAllNotifications, + getNotificationCount, + getUnreadNotificationCount, +} from '../../utils/notifications/notifications'; +import { computeRefetchIntervalMs } from '../../utils/notifications/pollInterval'; +import { deriveGlobalError, deriveStatus } from '../../utils/notifications/status'; + +export interface NotificationsQueryResult { + notifications: AccountNotifications[]; + isLoading: boolean; + isError: boolean; + isFetching: boolean; + notificationCount: number; + unreadNotificationCount: number; + hasNotifications: boolean; + hasUnreadNotifications: boolean; + status: Status; + globalError: GitifyError | undefined; + notificationsQueryKey: QueryKey; + fetchIntervalMs: number; + refetch: () => void; + refetchNotifications: () => Promise; + removeAccountNotifications: (account: Account) => void; +} + +/** + * The read side of notification state: the TanStack Query observer (optionally + * polling), the filter projection, derived counts, and the derived status and + * global error. + * + * `poll` distinguishes the single app-level host observer - which schedules the + * poll interval - from the many read-only consumers, which share the same cache + * entry but schedule nothing. See `useNotificationsHost`. + */ +export const useNotificationsQuery = ({ poll }: { poll: boolean }): NotificationsQueryResult => { + const queryClient = useQueryClient(); + + // Account store values + const accounts = useAccountsStore((s) => s.accounts); + + // Filter store values + const includeSearchTokens = useFiltersStore((s) => s.includeSearchTokens); + const excludeSearchTokens = useFiltersStore((s) => s.excludeSearchTokens); + const filterAccounts = useFiltersStore((s) => s.accounts); + const userTypes = useFiltersStore((s) => s.userTypes); + const subjectTypes = useFiltersStore((s) => s.subjectTypes); + const states = useFiltersStore((s) => s.states); + const reasons = useFiltersStore((s) => s.reasons); + const reviewRequestTypes = useFiltersStore((s) => s.reviewRequestTypes); + + // Setting store values + const fetchReadNotifications = useSettingsStore((s) => s.fetchReadNotifications); + const fetchParticipatingNotifications = useSettingsStore((s) => s.participating); + const fetchIntervalMs = useSettingsStore((s) => s.fetchInterval); + + // Query key excludes filters: the cache holds unfiltered data and filter + // changes re-run the select below, applying instantly without refetching + const notificationsQueryKey = useMemo( + () => + notificationsKeys.list( + accounts.map(getAccountUUID), + fetchReadNotifications, + fetchParticipatingNotifications, + ), + [accounts, fetchReadNotifications, fetchParticipatingNotifications], + ); + + // Create select function that depends on filter state + const selectFilteredNotifications = useMemo( + () => (data: AccountNotifications[]) => + data.map((accountNotifications) => ({ + ...accountNotifications, + notifications: filterDetailedNotifications( + filterBaseNotifications(accountNotifications.notifications), + ) as GitifyNotification[], + })), + // oxlint-disable-next-line react/exhaustive-deps -- Recreate the selection function on filter store changes + [ + includeSearchTokens, + excludeSearchTokens, + filterAccounts, + userTypes, + subjectTypes, + states, + reasons, + reviewRequestTypes, + ], + ); + + // Query for fetching notifications - TanStack Query handles polling and refetching + const { + data: notifications = [], + isLoading, + isFetching, + isError, + isPaused, + refetch, + } = useQuery({ + queryKey: notificationsQueryKey, + + queryFn: async () => { + return await getAllNotifications(); + }, + + // Apply filters as a transformation on the cached data + // This allows filter changes to instantly update without refetching + select: selectFilteredNotifications, + + placeholderData: keepPreviousData, + + // Fresh for at least one poll cycle at the fastest possible interval, so + // data isn't treated as stale before a refetch could realistically have + // occurred. Uses the static floor rather than the live interval because + // the latter stretches with `X-Poll-Interval`; pinning staleTime to it + // would suppress mount/focus/reconnect refetches for as long as a forge + // asks Gitify to back off. Only those refetches are stale-gated - the + // poll itself fires on `refetchInterval` regardless. + staleTime: Constants.MIN_FETCH_NOTIFICATIONS_INTERVAL_MS, + + // Only the singleton host observer polls. Other consumers share the cached + // data and would otherwise each schedule their own refetch timer. The + // interval is re-evaluated after each fetch so it stretches to the latest + // server-recommended minimum (`X-Poll-Interval`) when required. + refetchInterval: poll ? () => computeRefetchIntervalMs(fetchIntervalMs, accounts) : false, + refetchOnMount: poll, + refetchOnReconnect: poll, + refetchOnWindowFocus: poll, + }); + + const notificationCount = getNotificationCount(notifications); + + const unreadNotificationCount = getUnreadNotificationCount(notifications); + + const hasNotifications = useMemo(() => notificationCount > 0, [notificationCount]); + + const hasUnreadNotifications = useMemo( + () => unreadNotificationCount > 0, + [unreadNotificationCount], + ); + + // 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( + () => deriveStatus({ isLoading, isPaused, isError, notifications }), + [isLoading, isPaused, isError, notifications], + ); + + const globalError: GitifyError | undefined = useMemo( + () => deriveGlobalError({ isPaused, notifications }), + [isPaused, notifications], + ); + + const refetchNotifications = useCallback(async () => { + await refetch(); + }, [refetch]); + + const removeAccountNotifications = useCallback( + (account: Account) => { + const accountUUID = getAccountUUID(account); + + queryClient.setQueryData( + notificationsQueryKey, + (existing) => + existing?.filter( + (accountNotifications) => getAccountUUID(accountNotifications.account) !== accountUUID, + ) ?? [], + ); + }, + [queryClient, notificationsQueryKey], + ); + + return { + notifications, + isLoading, + isError, + isFetching, + notificationCount, + unreadNotificationCount, + hasNotifications, + hasUnreadNotifications, + status, + globalError, + notificationsQueryKey, + fetchIntervalMs, + refetch, + refetchNotifications, + removeAccountNotifications, + }; +}; diff --git a/src/renderer/hooks/useNotifications.test.tsx b/src/renderer/hooks/useNotifications.test.tsx index 14116b4a4..362264d4a 100644 --- a/src/renderer/hooks/useNotifications.test.tsx +++ b/src/renderer/hooks/useNotifications.test.tsx @@ -34,10 +34,12 @@ import { } from '../utils/notifications/pollInterval'; import * as audio from '../utils/system/audio'; import * as native from '../utils/system/native'; +import { useNotificationsHost } from './notifications/useNotificationsHost'; import { useNotifications } from './useNotifications'; // Exercise the real hook implementation (globally mocked in vitest.setup) vi.unmock('./useNotifications'); +vi.unmock('./notifications/useNotificationsHost'); vi.mock('../utils/notifications/notifications', async () => { const actual = await vi.importActual( @@ -70,7 +72,7 @@ const createWrapper = () => { }; const renderNotificationsHook = () => - renderHook(() => useNotifications({ withSideEffects: true }), { wrapper: createWrapper() }); + renderHook(() => useNotificationsHost(), { wrapper: createWrapper() }); describe('renderer/hooks/useNotifications.ts', () => { const githubAdapter = getAdapter('github'); @@ -278,7 +280,7 @@ describe('renderer/hooks/useNotifications.ts', () => { const wrapper = createWrapper(); // Singleton side-effects host (GlobalEffects) - renderHook(() => useNotifications({ withSideEffects: true }), { wrapper }); + renderHook(() => useNotificationsHost(), { wrapper }); // Plain consumers (notification rows, repo groups, sidebar, etc.) // mount staggered over time, like real components rendering across @@ -340,7 +342,7 @@ describe('renderer/hooks/useNotifications.ts', () => { fetchInterval: 1000, }); - renderHook(() => useNotifications({ withSideEffects: true }), { + renderHook(() => useNotificationsHost(), { wrapper: createWrapper(), }); @@ -385,12 +387,12 @@ describe('renderer/hooks/useNotifications.ts', () => { {children} ); - renderHook(() => useNotifications({ withSideEffects: true }), { wrapper }); + renderHook(() => useNotificationsHost(), { wrapper }); await waitFor(() => expect(getAllNotificationsMock).toHaveBeenCalledTimes(1)); // `refetchOnMount` is stale-gated, so a second host mounting inside the // staleTime window shares the cached data rather than refetching. - renderHook(() => useNotifications({ withSideEffects: true }), { wrapper }); + renderHook(() => useNotificationsHost(), { wrapper }); await act(async () => { await Promise.resolve(); @@ -450,7 +452,7 @@ describe('renderer/hooks/useNotifications.ts', () => { {children} ); - renderHook(() => useNotifications({ withSideEffects: true }), { wrapper }); + renderHook(() => useNotificationsHost(), { wrapper }); await waitFor(() => expect(getAllNotificationsMock).toHaveBeenCalledTimes(1)); try { @@ -832,7 +834,7 @@ describe('renderer/hooks/useNotifications.ts', () => { // listener per mounted row and multiply wake-triggered refetches. expect(vi.mocked(window.gitify.onSystemWake)).not.toHaveBeenCalled(); - renderHook(() => useNotifications({ withSideEffects: true }), { wrapper }); + renderHook(() => useNotificationsHost(), { wrapper }); expect(vi.mocked(window.gitify.onSystemWake)).toHaveBeenCalledTimes(1); }); diff --git a/src/renderer/hooks/useNotifications.ts b/src/renderer/hooks/useNotifications.ts index bb6af5e67..66fdc744d 100644 --- a/src/renderer/hooks/useNotifications.ts +++ b/src/renderer/hooks/useNotifications.ts @@ -1,24 +1,3 @@ -import { useCallback, useEffect, useMemo, useRef } from 'react'; - -import { - focusManager, - keepPreviousData, - useMutation, - useQuery, - useQueryClient, -} from '@tanstack/react-query'; - -import { Constants } from '../constants'; - -import { - type NotificationFailedActionType, - getNotificationFailureKey, - useAccountsStore, - useFiltersStore, - useNotificationActionFailuresStore, - useSettingsStore, -} from '../stores'; - import type { Account, AccountNotifications, @@ -27,35 +6,16 @@ import type { Status, } from '../types'; -import { isMarkAsDoneFeatureSupported, isUnsubscribeThreadSupported } from '../utils/api/features'; -import { notificationsKeys } from '../utils/api/queryKeys'; -import { getAccountUUID } from '../utils/auth/utils'; -import { areAllAccountErrorsSame, doesAllAccountsHaveErrors, Errors } from '../utils/core/errors'; -import { rendererLogError, toError } from '../utils/core/logger'; -import { getAccountAdapter } from '../utils/forges/registry'; import { - filterBaseNotifications, - filterDetailedNotifications, -} from '../utils/notifications/filters/filter'; + type NotificationMutations, + useNotificationMutations, +} from './notifications/useNotificationMutations'; import { - type FailedNotificationAction, - restoreFailedNotifications, - settleNotificationActions, - type NotificationQuerySnapshot, -} from '../utils/notifications/mutations'; -import { - getAllNotifications, - getNotificationCount, - getUnreadNotificationCount, - refreshEnrichmentCache, -} from '../utils/notifications/notifications'; -import { computeRefetchIntervalMs } from '../utils/notifications/pollInterval'; -import { removeNotificationsForAccount } from '../utils/notifications/remove'; -import { getNewNotifications } from '../utils/notifications/utils'; -import { raiseSoundNotification } from '../utils/system/audio'; -import { raiseNativeNotification } from '../utils/system/native'; + type NotificationsQueryResult, + useNotificationsQuery, +} from './notifications/useNotificationsQuery'; -interface NotificationsState { +export interface NotificationsState { status: Status; globalError: GitifyError | undefined; @@ -76,602 +36,46 @@ interface NotificationsState { unsubscribeNotification: (notification: GitifyNotification) => Promise; } -interface UseNotificationsOptions { - /** - * Run the singleton side effects (sound/native alerts for new notifications - * and interval/focus/reconnect polling). Only the app-level effects host - * should enable this; all other consumers share the query cache without - * them. - * - * Polling is owned by the singleton host because TanStack Query schedules - * `refetchInterval` per mounted observer, not per query. Every consumer - * (each notification row, repo group, sidebar, etc.) would otherwise run - * its own staggered poll timer, multiplying API requests. - */ - withSideEffects?: boolean; -} - /** - * Hook that manages all notification state and actions for the application. - * - * Fetching, polling and caching are handled by TanStack Query; account and - * settings state are read from the Zustand stores. Safe to use from any - * component - all consumers share the same query cache. - * - * @returns The current notifications state and action callbacks. + * Assemble the public notification reader/actions surface from the query and + * mutation halves. Shared by the reader hook and the host hook so both expose + * an identical API. */ -export const useNotifications = ({ - withSideEffects = false, -}: UseNotificationsOptions = {}): NotificationsState => { - const queryClient = useQueryClient(); - const previousNotificationsRef = useRef([]); - - // Account store values - const accounts = useAccountsStore((s) => s.accounts); - - // Filter store values - const includeSearchTokens = useFiltersStore((s) => s.includeSearchTokens); - const excludeSearchTokens = useFiltersStore((s) => s.excludeSearchTokens); - const filterAccounts = useFiltersStore((s) => s.accounts); - const userTypes = useFiltersStore((s) => s.userTypes); - const subjectTypes = useFiltersStore((s) => s.subjectTypes); - const states = useFiltersStore((s) => s.states); - const reasons = useFiltersStore((s) => s.reasons); - const reviewRequestTypes = useFiltersStore((s) => s.reviewRequestTypes); - - // Setting store values - const fetchReadNotifications = useSettingsStore((s) => s.fetchReadNotifications); - const fetchParticipatingNotifications = useSettingsStore((s) => s.participating); - const fetchIntervalMs = useSettingsStore((s) => s.fetchInterval); - const markAsDoneOnUnsubscribe = useSettingsStore((s) => s.markAsDoneOnUnsubscribe); - const playSoundNewNotifications = useSettingsStore((s) => s.playSound); - const showSystemNotifications = useSettingsStore((s) => s.showNotifications); - const notificationVolume = useSettingsStore((s) => s.notificationVolume); - - // Query key excludes filters: the cache holds unfiltered data and filter - // changes re-run the select below, applying instantly without refetching - const notificationsQueryKey = useMemo( - () => - notificationsKeys.list( - accounts.map(getAccountUUID), - fetchReadNotifications, - fetchParticipatingNotifications, - ), - [accounts, fetchReadNotifications, fetchParticipatingNotifications], - ); - - // Create select function that depends on filter state - const selectFilteredNotifications = useMemo( - () => (data: AccountNotifications[]) => - data.map((accountNotifications) => ({ - ...accountNotifications, - notifications: filterDetailedNotifications( - filterBaseNotifications(accountNotifications.notifications), - ) as GitifyNotification[], - })), - // oxlint-disable-next-line react/exhaustive-deps -- Recreate the selection function on filter store changes - [ - includeSearchTokens, - excludeSearchTokens, - filterAccounts, - userTypes, - subjectTypes, - states, - reasons, - reviewRequestTypes, - ], - ); - - // Query for fetching notifications - TanStack Query handles polling and refetching - const { - data: notifications = [], - isLoading, - isFetching, - isError, - isPaused, - refetch, - } = useQuery({ - queryKey: notificationsQueryKey, - - queryFn: async () => { - return await getAllNotifications(); - }, - - // Apply filters as a transformation on the cached data - // This allows filter changes to instantly update without refetching - select: selectFilteredNotifications, - - placeholderData: keepPreviousData, - - // Fresh for at least one poll cycle at the fastest possible interval, so - // data isn't treated as stale before a refetch could realistically have - // occurred. Uses the static floor rather than the live interval because - // the latter stretches with `X-Poll-Interval`; pinning staleTime to it - // would suppress mount/focus/reconnect refetches for as long as a forge - // asks Gitify to back off. Only those refetches are stale-gated - the - // poll itself fires on `refetchInterval` regardless. - staleTime: Constants.MIN_FETCH_NOTIFICATIONS_INTERVAL_MS, - - // Only the singleton side-effects host polls. Other consumers share the - // cached data and would otherwise each schedule their own refetch timer. - // The interval is re-evaluated after each fetch so it stretches to the - // latest server-recommended minimum (`X-Poll-Interval`) when required. - refetchInterval: withSideEffects - ? () => computeRefetchIntervalMs(fetchIntervalMs, accounts) - : false, - refetchOnMount: withSideEffects, - refetchOnReconnect: withSideEffects, - refetchOnWindowFocus: withSideEffects, - }); - - const notificationCount = getNotificationCount(notifications); - - const unreadNotificationCount = getUnreadNotificationCount(notifications); - - const hasNotifications = useMemo(() => notificationCount > 0, [notificationCount]); - - const hasUnreadNotifications = useMemo( - () => unreadNotificationCount > 0, - [unreadNotificationCount], - ); - - // 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) { - return 'loading'; - } - - // Check if paused due to offline state first (instant detection) - if (isPaused) { - return 'error'; - } - - if (isError) { - return 'error'; - } - - // Set error status if all accounts have errors - if (doesAllAccountsHaveErrors(notifications)) { - return 'error'; - } - - return 'success'; - }, [isLoading, isPaused, isError, notifications]); - - const globalError: GitifyError | undefined = useMemo(() => { - // If paused due to offline, show network error - if (isPaused) { - return Errors.OFFLINE; - } - - if (notifications.length === 0) { - return undefined; - } - - // Set Global Error if all accounts have the same error - const allAccountsHaveErrors = doesAllAccountsHaveErrors(notifications); - const allAccountErrorsAreSame = areAllAccountErrorsSame(notifications); - - if (allAccountsHaveErrors && allAccountErrorsAreSame) { - return notifications[0].error ?? undefined; - } - - return undefined; - }, [isPaused, notifications]); - - const refetchNotifications = useCallback(async () => { - await refetch(); - }, [refetch]); - - // Refetch notifications immediately when the system wakes from sleep or the - // user unlocks their screen, without waiting for the next poll interval. - // Owned by the singleton side-effects host: every consumer registering a - // wake listener would both multiply refetches and leak ipcRenderer - // listeners on unmount. - useEffect(() => { - if (!withSideEffects) { - return; - } - - return window.gitify.onSystemWake(() => { - refetch(); - }); - }, [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( - (account: Account) => { - const accountUUID = getAccountUUID(account); - - queryClient.setQueryData( - notificationsQueryKey, - (existing) => - existing?.filter( - (accountNotifications) => getAccountUUID(accountNotifications.account) !== accountUUID, - ) ?? [], - ); - }, - [queryClient, notificationsQueryKey], - ); - - // Handle sound and native notifications when new notifications arrive - useEffect(() => { - if (!withSideEffects || isLoading || isError || notifications.length === 0) { - return; - } - - const allAccountsHaveErrors = doesAllAccountsHaveErrors(notifications); - if (allAccountsHaveErrors) { - return; - } - - const unfilteredNotifications = - queryClient.getQueryData(notificationsQueryKey) || []; - - const diffNotifications = getNewNotifications( - previousNotificationsRef.current, - unfilteredNotifications, - ); +export const buildNotificationsState = ( + query: NotificationsQueryResult, + mutations: NotificationMutations, +): NotificationsState => ({ + status: query.status, + globalError: query.globalError, - if (diffNotifications.length > 0) { - // Apply filters to new notifications so only filtered notifications trigger alerts - const filteredDiffNotifications = filterDetailedNotifications( - filterBaseNotifications(diffNotifications), - ) as GitifyNotification[]; + isFetching: query.isFetching, - if (filteredDiffNotifications.length > 0) { - if (playSoundNewNotifications) { - void raiseSoundNotification(notificationVolume); - } + notifications: query.notifications, + notificationCount: query.notificationCount, + unreadNotificationCount: query.unreadNotificationCount, + hasNotifications: query.hasNotifications, + hasUnreadNotifications: query.hasUnreadNotifications, - if (showSystemNotifications) { - void raiseNativeNotification(filteredDiffNotifications); - } - } - } + refetchNotifications: query.refetchNotifications, + removeAccountNotifications: query.removeAccountNotifications, - previousNotificationsRef.current = unfilteredNotifications; - }, [ - withSideEffects, - notifications, - isLoading, - isError, - playSoundNewNotifications, - showSystemNotifications, - notificationVolume, - queryClient, - notificationsQueryKey, - ]); + markNotificationsAsRead: mutations.markNotificationsAsRead, + markNotificationsAsDone: mutations.markNotificationsAsDone, + unsubscribeNotification: mutations.unsubscribeNotification, +}); - // Session-local failure entries are independent of the notifications - // cache, so they must be pruned separately once a notification no longer - // appears in the (unfiltered) list - e.g. actioned successfully elsewhere, - // or its account was removed. Owned by the singleton side-effects host so - // it runs once per notifications update rather than once per mounted - // row/consumer. - useEffect(() => { - if (!withSideEffects) { - return; - } - - const unfilteredNotifications = - queryClient.getQueryData(notificationsQueryKey) || []; - - const currentNotificationKeys = unfilteredNotifications.flatMap((accountNotifications) => - accountNotifications.notifications.map((notification) => - getNotificationFailureKey(notification.account, notification.id), - ), - ); - - useNotificationActionFailuresStore.getState().pruneFailures(currentNotificationKeys); - }, [withSideEffects, notifications, queryClient, notificationsQueryKey]); - - // Shared by all three mutations' `onSuccess`. Records each failure's - // classified error in the session-local failure map and logs it; also - // re-applies `snapshot` for any failed notification missing from the - // cache, as a defensive guard (these mutations don't remove notifications - // until they succeed, so this is normally a no-op). - const reconcileFailedNotifications = useCallback( - ( - failed: FailedNotificationAction[], - snapshot: NotificationQuerySnapshot | undefined, - action: NotificationFailedActionType, - context: string, - ) => { - if (failed.length === 0) { - return; - } - - const failedNotifications = failed.map((f) => f.notification); - - if (snapshot) { - for (const [queryKey, snapshotData] of snapshot) { - queryClient.setQueryData(queryKey, (existing) => - restoreFailedNotifications(failedNotifications, snapshotData ?? [], existing ?? []), - ); - } - } - - for (const { notification, error, rawError } of failed) { - const notificationKey = getNotificationFailureKey(notification.account, notification.id); - useNotificationActionFailuresStore - .getState() - .setFailure(notificationKey, { action, error }); - rendererLogError( - context, - `Error occurred while processing notification ${notification.id}`, - rawError, - ); - } - }, - [queryClient], - ); - - // Full-cache restore for `onError`, covering the rare case where a - // mutation function throws directly (e.g. a bug) rather than resolving via - // `settleNotificationActions`. Per-notification failures are handled by - // `reconcileFailedNotifications` above instead. - const restoreSnapshot = useCallback( - (snapshot?: NotificationQuerySnapshot) => { - if (!snapshot) { - return; - } - - for (const [queryKey, data] of snapshot) { - queryClient.setQueryData(queryKey, data); - } - }, - [queryClient], - ); - - const snapshotNotifications = useCallback(async () => { - // Prevent an older in-flight poll from overwriting the mutation's eventual cache update. - await queryClient.cancelQueries({ queryKey: notificationsKeys.all }); - - const snapshot = queryClient.getQueriesData({ - queryKey: notificationsKeys.all, - }); - - return { snapshot }; - }, [queryClient]); - - const createMutationErrorHandler = useCallback( - (logContext: string, message: string) => - (err: Error, _variables: unknown, context?: { snapshot: NotificationQuerySnapshot }) => { - restoreSnapshot(context?.snapshot); - rendererLogError(logContext, message, toError(err)); - }, - [restoreSnapshot], - ); - - const markNotificationsAsReadMutation = useMutation({ - mutationFn: async ({ readNotifications }: { readNotifications: GitifyNotification[] }) => { - return await settleNotificationActions(readNotifications, (notification) => - getAccountAdapter(notification.account).markThreadAsRead(notification.id), - ); - }, - - onMutate: snapshotNotifications, - - onSuccess: ({ succeeded, failed }, _variables, context) => { - // Cache removal happens here (once the request resolves) rather than - // optimistically in `onMutate`, so the row's exit animation - started - // synchronously on click - has time to play before the notification - // disappears from the list. - if (succeeded.length > 0) { - queryClient.setQueryData(notificationsQueryKey, (existing) => - removeNotificationsForAccount(succeeded[0].account, succeeded, existing ?? []), - ); - } - - useNotificationActionFailuresStore - .getState() - .clearFailures( - succeeded.map((notification) => - getNotificationFailureKey(notification.account, notification.id), - ), - ); - - reconcileFailedNotifications( - failed, - context?.snapshot, - 'markAsRead', - 'markNotificationsAsRead', - ); - }, - - onError: createMutationErrorHandler( - 'markNotificationsAsRead', - 'Error occurred while marking notifications as read', - ), - }); - - const markNotificationsAsDoneMutation = useMutation({ - mutationFn: async ({ doneNotifications }: { doneNotifications: GitifyNotification[] }) => { - const account = doneNotifications[0].account; - - // Forges that don't support a distinct "done" state fall back to - // marking as read so the user-visible action still removes the thread. - if (!isMarkAsDoneFeatureSupported(account)) { - return await markNotificationsAsReadMutation.mutateAsync({ - readNotifications: doneNotifications, - }); - } - - return await settleNotificationActions(doneNotifications, (notification) => - getAccountAdapter(notification.account).markThreadAsDone(notification.id), - ); - }, - - onMutate: snapshotNotifications, - - onSuccess: ({ succeeded, failed }, { doneNotifications }, context) => { - // The mark-as-read fallback (for forges without a distinct "done" - // state) already updated the cache via its own mutation/onSuccess. - if (succeeded.length > 0 && isMarkAsDoneFeatureSupported(doneNotifications[0].account)) { - queryClient.setQueryData(notificationsQueryKey, (existing) => - removeNotificationsForAccount(succeeded[0].account, succeeded, existing ?? []), - ); - } - - useNotificationActionFailuresStore - .getState() - .clearFailures( - succeeded.map((notification) => - getNotificationFailureKey(notification.account, notification.id), - ), - ); - - if (isMarkAsDoneFeatureSupported(doneNotifications[0].account)) { - reconcileFailedNotifications( - failed, - context?.snapshot, - 'markAsDone', - 'markNotificationsAsDone', - ); - } - }, - - onError: createMutationErrorHandler( - 'markNotificationsAsDone', - 'Error occurred while marking notifications as done', - ), - }); - - const unsubscribeNotificationMutation = useMutation({ - mutationFn: async ({ notification }: { notification: GitifyNotification }) => { - // Forges without thread-subscription support cannot unsubscribe; the UI - // already hides the action, but treat duplicate calls as no-ops. - if (!isUnsubscribeThreadSupported(notification.account)) { - return { succeeded: [notification], failed: [] }; - } - - const result = await settleNotificationActions([notification], (n) => - getAccountAdapter(n.account).unsubscribeThread(n.id), - ); - - if (result.failed.length > 0) { - return result; - } - - const followUp = markAsDoneOnUnsubscribe - ? await markNotificationsAsDoneMutation.mutateAsync({ - doneNotifications: [notification], - }) - : await markNotificationsAsReadMutation.mutateAsync({ - readNotifications: [notification], - }); - - return followUp.failed.length > 0 ? followUp : result; - }, - - onMutate: snapshotNotifications, - - onSuccess: ({ succeeded, failed }, _variables, context) => { - useNotificationActionFailuresStore - .getState() - .clearFailures( - succeeded.map((notification) => - getNotificationFailureKey(notification.account, notification.id), - ), - ); - - reconcileFailedNotifications( - failed, - context?.snapshot, - 'unsubscribe', - 'unsubscribeNotification', - ); - }, - - onError: createMutationErrorHandler( - 'unsubscribeNotification', - 'Error occurred while unsubscribing from notification thread', - ), - }); - - const markNotificationsAsRead = useCallback( - async (readNotifications: GitifyNotification[]) => { - try { - const { failed } = await markNotificationsAsReadMutation.mutateAsync({ - readNotifications, - }); - return failed.length === 0; - } catch { - return false; - } - }, - [markNotificationsAsReadMutation], - ); - - const markNotificationsAsDone = useCallback( - async (doneNotifications: GitifyNotification[]) => { - try { - const { failed } = await markNotificationsAsDoneMutation.mutateAsync({ - doneNotifications, - }); - return failed.length === 0; - } catch { - return false; - } - }, - [markNotificationsAsDoneMutation], - ); - - const unsubscribeNotification = useCallback( - async (notification: GitifyNotification) => { - try { - const { failed } = await unsubscribeNotificationMutation.mutateAsync({ notification }); - return failed.length === 0; - } catch { - return false; - } - }, - [unsubscribeNotificationMutation], - ); - - return { - status, - globalError, - - isFetching, - - notifications, - notificationCount, - unreadNotificationCount, - hasNotifications, - hasUnreadNotifications, - - refetchNotifications, - removeAccountNotifications, +/** + * Hook that exposes notification state and actions for the application. + * + * Side-effect free: it does not poll, raise alerts, or register wake/focus + * listeners, and shares the query cache with every other consumer. The + * app-level host (`useNotificationsHost`) owns those side effects. + * + * @returns The current notifications state and action callbacks. + */ +export const useNotifications = (): NotificationsState => { + const query = useNotificationsQuery({ poll: false }); + const mutations = useNotificationMutations(query.notificationsQueryKey); - markNotificationsAsRead, - markNotificationsAsDone, - unsubscribeNotification, - }; + return buildNotificationsState(query, mutations); }; diff --git a/src/renderer/components/icons/BitbucketIcon.tsx b/src/renderer/icons/BitbucketIcon.tsx similarity index 100% rename from src/renderer/components/icons/BitbucketIcon.tsx rename to src/renderer/icons/BitbucketIcon.tsx diff --git a/src/renderer/components/icons/GitLabIcon.tsx b/src/renderer/icons/GitLabIcon.tsx similarity index 100% rename from src/renderer/components/icons/GitLabIcon.tsx rename to src/renderer/icons/GitLabIcon.tsx diff --git a/src/renderer/components/icons/LogoIcon.test.tsx b/src/renderer/icons/LogoIcon.test.tsx similarity index 88% rename from src/renderer/components/icons/LogoIcon.test.tsx rename to src/renderer/icons/LogoIcon.test.tsx index b8eccb010..964fa1d11 100644 --- a/src/renderer/components/icons/LogoIcon.test.tsx +++ b/src/renderer/icons/LogoIcon.test.tsx @@ -1,13 +1,13 @@ import { screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import { renderWithProviders } from '../../__helpers__/test-utils'; +import { renderWithProviders } from '../__helpers__/test-utils'; -import { Size } from '../../types'; +import { Size } from '../types'; import { LogoIcon } from './LogoIcon'; -describe('renderer/components/icons/LogoIcon.tsx', () => { +describe('renderer/icons/LogoIcon.tsx', () => { it('renders correctly (light)', () => { const tree = renderWithProviders(); diff --git a/src/renderer/components/icons/LogoIcon.tsx b/src/renderer/icons/LogoIcon.tsx similarity index 96% rename from src/renderer/components/icons/LogoIcon.tsx rename to src/renderer/icons/LogoIcon.tsx index 25c42fae0..9ccda8052 100644 --- a/src/renderer/components/icons/LogoIcon.tsx +++ b/src/renderer/icons/LogoIcon.tsx @@ -2,9 +2,9 @@ import type { FC } from 'react'; import { cn } from 'cn'; -import { APPLICATION } from '../../../shared/constants'; +import { APPLICATION } from '../../shared/constants'; -import { Size } from '../../types'; +import { Size } from '../types'; interface LogoIconProps { isDark?: boolean; diff --git a/src/renderer/components/icons/ScopeStatusIcon.test.tsx b/src/renderer/icons/ScopeStatusIcon.test.tsx similarity index 95% rename from src/renderer/components/icons/ScopeStatusIcon.test.tsx rename to src/renderer/icons/ScopeStatusIcon.test.tsx index 50dd7647a..a924dc4b3 100644 --- a/src/renderer/components/icons/ScopeStatusIcon.test.tsx +++ b/src/renderer/icons/ScopeStatusIcon.test.tsx @@ -1,10 +1,10 @@ import { screen } from '@testing-library/react'; -import { renderWithProviders } from '../../__helpers__/test-utils'; +import { renderWithProviders } from '../__helpers__/test-utils'; import { ScopeStatusIcon } from './ScopeStatusIcon'; -describe('renderer/components/icons/ScopeStatusIcon.tsx', () => { +describe('renderer/icons/ScopeStatusIcon.tsx', () => { describe('granted state', () => { it('renders a success icon when granted', () => { const tree = renderWithProviders(); diff --git a/src/renderer/components/icons/ScopeStatusIcon.tsx b/src/renderer/icons/ScopeStatusIcon.tsx similarity index 100% rename from src/renderer/components/icons/ScopeStatusIcon.tsx rename to src/renderer/icons/ScopeStatusIcon.tsx diff --git a/src/renderer/components/icons/VolumeDownIcon.tsx b/src/renderer/icons/VolumeDownIcon.tsx similarity index 100% rename from src/renderer/components/icons/VolumeDownIcon.tsx rename to src/renderer/icons/VolumeDownIcon.tsx diff --git a/src/renderer/components/icons/VolumeUpIcon.tsx b/src/renderer/icons/VolumeUpIcon.tsx similarity index 100% rename from src/renderer/components/icons/VolumeUpIcon.tsx rename to src/renderer/icons/VolumeUpIcon.tsx diff --git a/src/renderer/components/icons/__snapshots__/LogoIcon.test.tsx.snap b/src/renderer/icons/__snapshots__/LogoIcon.test.tsx.snap similarity index 96% rename from src/renderer/components/icons/__snapshots__/LogoIcon.test.tsx.snap rename to src/renderer/icons/__snapshots__/LogoIcon.test.tsx.snap index 09fe79ad6..671479a4f 100644 --- a/src/renderer/components/icons/__snapshots__/LogoIcon.test.tsx.snap +++ b/src/renderer/icons/__snapshots__/LogoIcon.test.tsx.snap @@ -1,6 +1,6 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html -exports[`renderer/components/icons/LogoIcon.tsx > renders correctly (dark) 1`] = ` +exports[`renderer/icons/LogoIcon.tsx > renders correctly (dark) 1`] = `
`; -exports[`renderer/components/icons/LogoIcon.tsx > renders correctly (light) 1`] = ` +exports[`renderer/icons/LogoIcon.tsx > renders correctly (light) 1`] = `
`; -exports[`renderer/components/icons/LogoIcon.tsx > should render large size 1`] = ` +exports[`renderer/icons/LogoIcon.tsx > should render large size 1`] = `
`; -exports[`renderer/components/icons/LogoIcon.tsx > should render medium size 1`] = ` +exports[`renderer/icons/LogoIcon.tsx > should render medium size 1`] = `
`; -exports[`renderer/components/icons/LogoIcon.tsx > should render small size 1`] = ` +exports[`renderer/icons/LogoIcon.tsx > should render small size 1`] = `
`; -exports[`renderer/components/icons/ScopeStatusIcon.tsx > missing state > renders a danger icon when not granted 1`] = ` +exports[`renderer/icons/ScopeStatusIcon.tsx > missing state > renders a danger icon when not granted 1`] = `
`; -exports[`renderer/components/icons/ScopeStatusIcon.tsx > notApplicable state > renders a dash icon when notApplicable 1`] = ` +exports[`renderer/icons/ScopeStatusIcon.tsx > notApplicable state > renders a dash icon when notApplicable 1`] = `
`; -exports[`renderer/components/icons/ScopeStatusIcon.tsx > size prop > renders correctly with a custom size 1`] = ` +exports[`renderer/icons/ScopeStatusIcon.tsx > size prop > renders correctly with a custom size 1`] = `