feat: fetch notifications on navigation (#1305)

Co-authored-by: Afonso Jorge Ramos <[email protected]>

Author: setchy

src/components/Header.test.tsx

@@ -1,4 +1,5 @@
 import { fireEvent, render, screen } from '@testing-library/react';
+import { AppContext } from '../context/App';
 import { Header } from './Header';
 
 const mockNavigate = jest.fn();
@@ -8,6 +9,12 @@ jest.mock('react-router-dom', () => ({
 }));
 
 describe('components/Header.tsx', () => {
+  const fetchNotifications = jest.fn();
+
+  afterEach(() => {
+    jest.resetAllMocks();
+  });
+
   it('should render itself & its children', () => {
     const tree = render(<Header>Test Header</Header>);
 
@@ -18,9 +25,26 @@ describe('components/Header.tsx', () => {
     render(<Header>Test Header</Header>);
 
     fireEvent.click(screen.getByLabelText('Go Back'));
-    expect(mockNavigate).toHaveBeenNthCalledWith(1, -1);
 
     expect(mockNavigate).toHaveBeenCalledTimes(1);
     expect(mockNavigate).toHaveBeenCalledWith(-1);
   });
+
+  it('should navigate back and fetch notifications', () => {
+    render(
+      <AppContext.Provider
+        value={{
+          fetchNotifications,
+        }}
+      >
+        <Header fetchOnBack={true}>Test Header</Header>
+      </AppContext.Provider>,
+    );
+
+    fireEvent.click(screen.getByLabelText('Go Back'));
+
+    expect(mockNavigate).toHaveBeenCalledTimes(1);
+    expect(mockNavigate).toHaveBeenCalledWith(-1);
+    expect(fetchNotifications).toHaveBeenCalledTimes(1);
+  });
 });

src/components/Header.tsx

@@ -1,21 +1,34 @@
 import { ArrowLeftIcon } from '@primer/octicons-react';
-import type { FC, ReactNode } from 'react';
+import { type FC, type ReactNode, useContext } from 'react';
 import { useNavigate } from 'react-router-dom';
+import { AppContext } from '../context/App';
 import { Size } from '../types';
 
 interface IHeader {
   children: ReactNode;
+  fetchOnBack?: boolean;
 }
 
-export const Header: FC<IHeader> = ({ children }: IHeader) => {
+export const Header: FC<IHeader> = ({
+  children,
+  fetchOnBack = false,
+}: IHeader) => {
   const navigate = useNavigate();
+
+  const { fetchNotifications } = useContext(AppContext);
+
   return (
     <div className="mx-8 mt-2 flex items-center justify-between py-2">
       <button
         type="button"
         className="focus:outline-none"
         title="Go Back"
-        onClick={() => navigate(-1)}
+        onClick={() => {
+          navigate(-1);
+          if (fetchOnBack) {
+            fetchNotifications();
+          }
+        }}
       >
         <ArrowLeftIcon
           size={Size.XLARGE}

src/components/Sidebar.test.tsx

@@ -15,6 +15,7 @@ jest.mock('react-router-dom', () => ({
 
 describe('components/Sidebar.tsx', () => {
   const fetchNotifications = jest.fn();
+
   const openExternalLinkMock = jest.spyOn(comms, 'openExternalLink');
 
   afterEach(() => {
@@ -230,7 +231,9 @@ describe('components/Sidebar.tsx', () => {
 
     it('go to the home if settings path already shown', () => {
       render(
-        <AppContext.Provider value={{ isLoggedIn: true, notifications: [] }}>
+        <AppContext.Provider
+          value={{ isLoggedIn: true, notifications: [], fetchNotifications }}
+        >
           <MemoryRouter initialEntries={['/settings']}>
             <Sidebar />
           </MemoryRouter>
@@ -239,6 +242,7 @@ describe('components/Sidebar.tsx', () => {
 
       fireEvent.click(screen.getByTitle('Settings'));
 
+      expect(fetchNotifications).toHaveBeenCalledTimes(1);
       expect(mockNavigate).toHaveBeenCalledWith('/', { replace: true });
     });
   });

src/components/Sidebar.tsx

@@ -31,6 +31,7 @@ export const Sidebar: FC = () => {
   const toggleSettings = () => {
     if (location.pathname.startsWith('/settings')) {
       navigate('/', { replace: true });
+      fetchNotifications();
     } else {
       navigate('/settings');
     }

src/context/App.tsx

@@ -122,16 +122,10 @@ export const AppProvider = ({ children }: { children: ReactNode }) => {
     setTheme(settings.theme);
   }, [settings.theme]);
 
-  // biome-ignore lint/correctness/useExhaustiveDependencies: We only want fetchNotifications to be called for certain account or setting changes.
+  // biome-ignore lint/correctness/useExhaustiveDependencies: We only want fetchNotifications to be called for account changes
   useEffect(() => {
     fetchNotifications({ auth, settings });
-  }, [
-    settings.participating,
-    settings.showBots,
-    settings.detailedNotifications,
-    settings.delayNotificationState,
-    auth.accounts.length,
-  ]);
+  }, [auth.accounts]);
 
   useInterval(() => {
     fetchNotifications({ auth, settings });

src/routes/Settings.test.tsx

@@ -15,6 +15,7 @@ jest.mock('react-router-dom', () => ({
 describe('routes/Settings.tsx', () => {
   let originalPlatform: NodeJS.Platform;
   const updateSetting = jest.fn();
+  const fetchNotifications = jest.fn();
 
   beforeAll(() => {
     // Save the original platform value
@@ -62,6 +63,7 @@ describe('routes/Settings.tsx', () => {
             value={{
               auth: mockAuth,
               settings: mockSettings,
+              fetchNotifications,
             }}
           >
             <MemoryRouter>
@@ -72,6 +74,7 @@ describe('routes/Settings.tsx', () => {
       });
 
       fireEvent.click(screen.getByLabelText('Go Back'));
+      expect(fetchNotifications).toHaveBeenCalledTimes(1);
       expect(mockNavigate).toHaveBeenNthCalledWith(1, -1);
     });
   });

src/routes/Settings.tsx

@@ -56,7 +56,7 @@ export const SettingsRoute: FC = () => {
 
   return (
     <div className="flex h-screen flex-col" data-testid="settings">
-      <Header>Settings</Header>
+      <Header fetchOnBack={true}>Settings</Header>
       <div className="flex-grow overflow-x-auto px-8">
         <fieldset className="mb-3">
           <legend id="appearance" className="mb-1 mt-2 font-semibold">