fix: rollback failed notification interaction with visual warning

src/renderer/__helpers__/hook-mocks.ts

@@ -41,6 +41,8 @@ function buildNotificationsDefaults(): NotificationsState {
     markNotificationsAsRead: vi.fn(),
     markNotificationsAsDone: vi.fn(),
     unsubscribeNotification: vi.fn(),
+
+    notificationFailures: {},
   };
 }
 

src/renderer/components/notifications/NotificationRow.test.tsx

@@ -8,8 +8,11 @@ import {
 } from '../../__mocks__/notifications-mocks';
 import { mockSettings } from '../../__mocks__/state-mocks';
 
+import { useNotificationActionFailuresStore } from '../../stores';
+
 import { GroupBy } from '../../types';
 
+import { Errors } from '../../utils/core/errors';
 import * as comms from '../../utils/system/comms';
 import * as links from '../../utils/system/links';
 import { NotificationRow, type NotificationRowProps } from './NotificationRow';
@@ -263,4 +266,143 @@ describe('renderer/components/notifications/NotificationRow.tsx', () => {
       expect(screen.queryByTestId('notification-unsubscribe-from-thread')).not.toBeInTheDocument();
     });
   });
+
+  describe('failure recovery', () => {
+    it('shows hover actions in their normal (non-danger) state when there is no recorded failure', () => {
+      const props: NotificationRowProps = {
+        notification: mockGitifyNotification,
+        isRepositoryAnimatingExit: false,
+      };
+
+      renderWithProviders(<NotificationRow {...props} />, {
+        notificationFailures: {},
+      });
+
+      expect(screen.getByTestId('notification-mark-as-read')).toHaveAttribute(
+        'title',
+        'Mark as read',
+      );
+    });
+
+    it('colors the hover actions and explains the failure via their tooltip when the notification has a recorded failure', () => {
+      const props: NotificationRowProps = {
+        notification: mockGitifyNotification,
+        isRepositoryAnimatingExit: false,
+      };
+
+      renderWithProviders(<NotificationRow {...props} />, {
+        notificationFailures: {
+          [mockGitifyNotification.id]: { action: 'markAsRead', error: Errors.ACTION_FORBIDDEN },
+        },
+      });
+
+      const markAsReadButton = screen.getByTestId('notification-mark-as-read');
+
+      // The row's actions remain available - the row is not in a broken state
+      expect(markAsReadButton).toBeInTheDocument();
+      expect(markAsReadButton).toHaveAttribute(
+        'title',
+        expect.stringContaining(Errors.ACTION_FORBIDDEN.title),
+      );
+      expect(markAsReadButton).toHaveAttribute(
+        'title',
+        expect.stringContaining('You can also try opening this notification in the browser.'),
+      );
+    });
+
+    it('re-invokes the same action on click, acting as a retry, when a failure is recorded', async () => {
+      const markNotificationsAsDoneMock = vi.fn();
+
+      const props: NotificationRowProps = {
+        notification: mockGitifyNotification,
+        isRepositoryAnimatingExit: false,
+      };
+
+      renderWithProviders(<NotificationRow {...props} />, {
+        markNotificationsAsDone: markNotificationsAsDoneMock,
+        notificationFailures: {
+          [mockGitifyNotification.id]: { action: 'markAsDone', error: Errors.ACTION_FORBIDDEN },
+        },
+      });
+
+      await userEvent.click(screen.getByTestId('notification-mark-as-done'));
+
+      expect(markNotificationsAsDoneMock).toHaveBeenCalledTimes(1);
+      expect(markNotificationsAsDoneMock).toHaveBeenCalledWith([mockGitifyNotification]);
+    });
+
+    it('does not disable retrying even for a permanently-failing classification like ACTION_FORBIDDEN', async () => {
+      const markNotificationsAsReadMock = vi.fn();
+
+      const props: NotificationRowProps = {
+        notification: mockGitifyNotification,
+        isRepositoryAnimatingExit: false,
+      };
+
+      renderWithProviders(<NotificationRow {...props} />, {
+        markNotificationsAsRead: markNotificationsAsReadMock,
+        notificationFailures: {
+          [mockGitifyNotification.id]: { action: 'markAsRead', error: Errors.ACTION_FORBIDDEN },
+        },
+      });
+
+      const markAsReadButton = screen.getByTestId('notification-mark-as-read');
+      expect(markAsReadButton).toBeEnabled();
+
+      await userEvent.click(markAsReadButton);
+
+      expect(markNotificationsAsReadMock).toHaveBeenCalledTimes(1);
+    });
+
+    it('gives a retry its own exit-animation cycle even though the previous failure is still recorded', async () => {
+      // Regression test: the revert logic reads the *current* failure store
+      // state after each action settles, rather than an effect keyed off a
+      // (possibly stale, still-present-from-the-previous-attempt) failure
+      // map - so a retry always gets to animate out and, if it fails again,
+      // animate back in, instead of being short-circuited immediately.
+      useNotificationActionFailuresStore.getState().setFailure(mockGitifyNotification.id, {
+        action: 'markAsRead',
+        error: Errors.ACTION_FORBIDDEN,
+      });
+
+      let resolveRetry: () => void = () => {};
+      const markNotificationsAsReadMock = vi.fn().mockImplementation(
+        () =>
+          new Promise<void>((resolve) => {
+            resolveRetry = resolve;
+          }),
+      );
+
+      const props: NotificationRowProps = {
+        notification: mockGitifyNotification,
+        isRepositoryAnimatingExit: false,
+      };
+
+      renderWithProviders(<NotificationRow {...props} />, {
+        settings: { ...mockSettings, delayNotificationState: false, fetchReadNotifications: false },
+        markNotificationsAsRead: markNotificationsAsReadMock,
+        notificationFailures: {
+          [mockGitifyNotification.id]: { action: 'markAsRead', error: Errors.ACTION_FORBIDDEN },
+        },
+      });
+
+      await userEvent.click(screen.getByTestId('notification-mark-as-read'));
+
+      // While the retry is still in flight, the row is animating out again -
+      // its hover actions are hidden, exactly like the very first attempt.
+      expect(screen.queryByTestId('notification-mark-as-read')).not.toBeInTheDocument();
+
+      // The retry fails again; the store still has a (new) failure entry for
+      // this notification once the mutation resolves.
+      useNotificationActionFailuresStore.getState().setFailure(mockGitifyNotification.id, {
+        action: 'markAsRead',
+        error: Errors.ACTION_FORBIDDEN,
+      });
+      resolveRetry();
+
+      await screen.findByTestId('notification-mark-as-read');
+
+      useNotificationActionFailuresStore.getState().reset();
+    });
+  });
 });

src/renderer/components/notifications/NotificationRow.tsx

@@ -4,7 +4,7 @@ import { BellSlashIcon, CheckIcon, ReadIcon } from '@primer/octicons-react';
 import { Stack, Text, Tooltip } from '@primer/react';
 
 import { useNotifications } from '../../hooks/useNotifications';
-import { useSettingsStore } from '../../stores';
+import { useNotificationActionFailuresStore, useSettingsStore } from '../../stores';
 
 import { HoverButton } from '../primitives/HoverButton';
 import { HoverGroup } from '../primitives/HoverGroup';
@@ -32,8 +32,12 @@ export const NotificationRow: FC<NotificationRowProps> = ({
   notification,
   isRepositoryAnimatingExit,
 }: NotificationRowProps) => {
-  const { markNotificationsAsRead, markNotificationsAsDone, unsubscribeNotification } =
-    useNotifications();
+  const {
+    markNotificationsAsRead,
+    markNotificationsAsDone,
+    unsubscribeNotification,
+    notificationFailures,
+  } = useNotifications();
 
   const markAsDoneOnOpen = useSettingsStore((s) => s.markAsDoneOnOpen);
   const wrapNotificationTitle = useSettingsStore((s) => s.wrapNotificationTitle);
@@ -43,31 +47,47 @@ export const NotificationRow: FC<NotificationRowProps> = ({
 
   const shouldAnimateExit = shouldRemoveNotificationsFromState();
 
-  const actionNotificationInteraction = () => {
+  const failure = notificationFailures[notification.id];
+
+  // Explains the failed action and suggests the browser as a fallback,
+  // rather than a dedicated retry control - clicking the (now red) hover
+  // action again re-attempts it. Phrased as "You can also..." rather than
+  // "...instead", since some descriptions already suggest waiting/retrying
+  // (e.g. `RATE_LIMITED`), which "instead" would read as contradicting.
+  const failureTooltip = failure
+    ? `${failure.error.title}: ${failure.error.descriptions.join(' ')} You can also try opening this notification in the browser.`
+    : undefined;
+
+  // Starts the exit animation immediately, then reverts it if this specific
+  // action failed, checked directly against the failure store once it
+  // settles. Checking a stale value (e.g. via an effect watching the failure
+  // map) would wrongly revert a retry's animation using the previous
+  // attempt's still-present entry.
+  const runAction = async (action: () => Promise<void>) => {
     setShouldAnimateNotificationExit(shouldAnimateExit);
-    openNotification(notification);
 
-    if (markAsDoneOnOpen) {
-      markNotificationsAsDone([notification]);
-    } else {
-      markNotificationsAsRead([notification]);
+    await action();
+
+    if (useNotificationActionFailuresStore.getState().failures[notification.id]) {
+      setShouldAnimateNotificationExit(false);
     }
   };
 
-  const actionMarkAsDone = () => {
-    setShouldAnimateNotificationExit(shouldAnimateExit);
-    markNotificationsAsDone([notification]);
-  };
+  const actionNotificationInteraction = () => {
+    openNotification(notification);
 
-  const actionMarkAsRead = () => {
-    setShouldAnimateNotificationExit(shouldAnimateExit);
-    markNotificationsAsRead([notification]);
+    runAction(() =>
+      markAsDoneOnOpen
+        ? markNotificationsAsDone([notification])
+        : markNotificationsAsRead([notification]),
+    );
   };
 
-  const actionUnsubscribeFromThread = () => {
-    setShouldAnimateNotificationExit(shouldAnimateExit);
-    unsubscribeNotification(notification);
-  };
+  const actionMarkAsDone = () => runAction(() => markNotificationsAsDone([notification]));
+
+  const actionMarkAsRead = () => runAction(() => markNotificationsAsRead([notification]));
+
+  const actionUnsubscribeFromThread = () => runAction(() => unsubscribeNotification(notification));
 
   const NotificationIcon = notification.display.icon.type;
   const isNotificationRead = !notification.unread;
@@ -134,24 +154,27 @@ export const NotificationRow: FC<NotificationRowProps> = ({
             action={actionMarkAsRead}
             enabled={!isNotificationRead}
             icon={ReadIcon}
-            label="Mark as read"
+            label={failureTooltip ?? 'Mark as read'}
             testid="notification-mark-as-read"
+            variant={failure ? 'danger' : 'invisible'}
           />
 
           <HoverButton
             action={actionMarkAsDone}
             enabled={isMarkAsDoneFeatureSupported(notification.account) && notification.unread}
             icon={CheckIcon}
-            label="Mark as done"
+            label={failureTooltip ?? 'Mark as done'}
             testid="notification-mark-as-done"
+            variant={failure ? 'danger' : 'invisible'}
           />
 
           <HoverButton
             action={actionUnsubscribeFromThread}
             enabled={isUnsubscribeThreadSupported(notification.account)}
             icon={BellSlashIcon}
-            label="Unsubscribe from thread"
+            label={failureTooltip ?? 'Unsubscribe from thread'}
             testid="notification-unsubscribe-from-thread"
+            variant={failure ? 'danger' : 'invisible'}
           />
         </HoverGroup>
       )}

src/renderer/components/notifications/RepositoryNotifications.test.tsx

@@ -5,6 +5,8 @@ import { renderWithProviders } from '../../__helpers__/test-utils';
 import { mockGitHubCloudGitifyNotifications } from '../../__mocks__/notifications-mocks';
 import { mockSettings } from '../../__mocks__/state-mocks';
 
+import { useNotificationActionFailuresStore } from '../../stores';
+
 import type { Link } from '../../types';
 
 import * as comms from '../../utils/system/comms';
@@ -124,4 +126,42 @@ describe('renderer/components/notifications/RepositoryNotifications.tsx', () =>
     const tree = renderWithProviders(<RepositoryNotifications {...props} />);
     expect(tree.container).toMatchSnapshot();
   });
+
+  describe('partial bulk failure', () => {
+    afterEach(() => {
+      useNotificationActionFailuresStore.getState().reset();
+    });
+
+    it('reverts the group exit animation when a notification within the bulk action failed', async () => {
+      const props: RepositoryNotificationsProps = {
+        repoName: 'gitify-app/notifications-test',
+        repoNotifications: mockGitHubCloudGitifyNotifications,
+      };
+
+      const [, secondNotification] = mockGitHubCloudGitifyNotifications;
+
+      // Simulate the mutation reconciliation that records a failure in the
+      // real (non-mocked) failure store, since `runGroupAction` reads
+      // directly from it rather than through the mocked `useNotifications`
+      // hook.
+      const markNotificationsAsReadWithFailure = vi.fn().mockImplementation(async () => {
+        useNotificationActionFailuresStore.getState().setFailure(secondNotification.id, {
+          action: 'markAsRead',
+          error: { title: 'Action Forbidden', descriptions: [], emojis: [] },
+        });
+      });
+
+      renderWithProviders(<RepositoryNotifications {...props} />, {
+        settings: { ...mockSettings },
+        markNotificationsAsRead: markNotificationsAsReadWithFailure,
+      });
+
+      await userEvent.click(screen.getByTestId('repository-mark-as-read'));
+
+      // Since one of this group's notifications has a recorded failure, the
+      // repository row's own exit animation is reverted - its hover actions
+      // remain reachable rather than staying hidden.
+      expect(screen.getByTestId('repository-mark-as-read')).toBeInTheDocument();
+    });
+  });
 });

src/renderer/components/notifications/RepositoryNotifications.tsx

@@ -4,6 +4,7 @@ import { CheckIcon, ReadIcon } from '@primer/octicons-react';
 import { Button, Stack } from '@primer/react';
 
 import { useNotifications } from '../../hooks/useNotifications';
+import { useNotificationActionFailuresStore } from '../../stores';
 
 import { HoverButton } from '../primitives/HoverButton';
 import { HoverGroup } from '../primitives/HoverGroup';
@@ -39,16 +40,28 @@ export const RepositoryNotifications: FC<RepositoryNotificationsProps> = ({
     openRepository(repoNotifications[0].repository);
   };
 
-  const actionMarkAsDone = () => {
+  // Starts the group's exit animation immediately, then reverts it if any
+  // notification in this bulk action failed, checked directly against the
+  // failure store once it settles (see `NotificationRow`'s `runAction` for
+  // why not a stale-state effect). There is no group-level rollup indicator;
+  // only the specific failed row(s) recolor their own hover actions.
+  const runGroupAction = async (action: () => Promise<void>) => {
     setShouldAnimateRepositoryExit(shouldAnimateExit);
-    markNotificationsAsDone(repoNotifications);
-  };
 
-  const actionMarkAsRead = () => {
-    setShouldAnimateRepositoryExit(shouldAnimateExit);
-    markNotificationsAsRead(repoNotifications);
+    await action();
+
+    const { failures } = useNotificationActionFailuresStore.getState();
+    const hasFailure = repoNotifications.some((notification) => failures[notification.id]);
+
+    if (hasFailure) {
+      setShouldAnimateRepositoryExit(false);
+    }
   };
 
+  const actionMarkAsDone = () => runGroupAction(() => markNotificationsAsDone(repoNotifications));
+
+  const actionMarkAsRead = () => runGroupAction(() => markNotificationsAsRead(repoNotifications));
+
   const actionToggleRepositoryNotifications = () => {
     setIsRepositoryNotificationsVisible(!isRepositoryNotificationsVisible);
   };