feat(refresh): reset interval and disable manual refresh (#1014)

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

Author: setchy

src/components/Sidebar.test.tsx

@@ -1,10 +1,11 @@
-import { fireEvent, render, screen } from '@testing-library/react';
+import { act, fireEvent, render, screen } from '@testing-library/react';
 import { MemoryRouter } from 'react-router-dom';
 import * as TestRenderer from 'react-test-renderer';
 const { shell, ipcRenderer } = require('electron');
 import { mockSettings } from '../__mocks__/mock-state';
 import { mockedAccountNotifications } from '../__mocks__/mockedData';
 import { AppContext } from '../context/App';
+import Constants from '../utils/constants';
 import { Sidebar } from './Sidebar';
 
 const mockNavigate = jest.fn();
@@ -19,12 +20,15 @@ describe('components/Sidebar.tsx', () => {
   beforeEach(() => {
     fetchNotifications.mockReset();
 
+    jest.useFakeTimers();
+
     jest.spyOn(ipcRenderer, 'send');
     jest.spyOn(shell, 'openExternal');
     jest.spyOn(window, 'clearInterval');
   });
 
   afterEach(() => {
+    jest.clearAllTimers();
     jest.clearAllMocks();
   });
 
@@ -57,6 +61,37 @@ describe('components/Sidebar.tsx', () => {
     expect(tree).toMatchSnapshot();
   });
 
+  it('should fetch notifications every minute', async () => {
+    render(
+      <AppContext.Provider
+        value={{ isLoggedIn: true, notifications: [], fetchNotifications }}
+      >
+        <MemoryRouter>
+          <Sidebar />
+        </MemoryRouter>
+      </AppContext.Provider>,
+    );
+    fetchNotifications.mockReset();
+
+    act(() => {
+      jest.advanceTimersByTime(Constants.FETCH_INTERVAL);
+      return;
+    });
+    expect(fetchNotifications).toHaveBeenCalledTimes(1);
+
+    act(() => {
+      jest.advanceTimersByTime(Constants.FETCH_INTERVAL);
+      return;
+    });
+    expect(fetchNotifications).toHaveBeenCalledTimes(2);
+
+    act(() => {
+      jest.advanceTimersByTime(Constants.FETCH_INTERVAL);
+      return;
+    });
+    expect(fetchNotifications).toHaveBeenCalledTimes(3);
+  });
+
   it('should refresh the notifications', () => {
     render(
       <AppContext.Provider
@@ -68,8 +103,19 @@ describe('components/Sidebar.tsx', () => {
       </AppContext.Provider>,
     );
     fetchNotifications.mockReset();
-    fireEvent.click(screen.getByTitle('Refresh Notifications'));
+
+    const enabledRefreshButton = 'Refresh Notifications';
+
+    fireEvent.click(screen.getByTitle(enabledRefreshButton));
+
     expect(fetchNotifications).toHaveBeenCalledTimes(1);
+
+    act(() => {
+      jest.advanceTimersByTime(Constants.FETCH_INTERVAL);
+      return;
+    });
+
+    expect(fetchNotifications).toHaveBeenCalledTimes(2);
   });
 
   it('go to the settings route', () => {

src/components/Sidebar.tsx

@@ -6,7 +6,14 @@ import {
 } from '@primer/octicons-react';
 import { ipcRenderer } from 'electron';
 
-import { type FC, useCallback, useContext, useMemo } from 'react';
+import {
+  type FC,
+  useCallback,
+  useContext,
+  useEffect,
+  useMemo,
+  useRef,
+} from 'react';
 import { useLocation, useNavigate } from 'react-router-dom';
 
 import { Logo } from '../components/Logo';
@@ -22,6 +29,38 @@ export const Sidebar: FC = () => {
   const { notifications, fetchNotifications, isLoggedIn, isFetching } =
     useContext(AppContext);
 
+  const useFetchInterval = (callback, delay: number) => {
+    const savedCallback = useRef(callback);
+    const intervalRef = useRef(null);
+
+    useEffect(() => {
+      savedCallback.current = callback;
+    }, [callback]);
+
+    useEffect(() => {
+      if (delay !== null) {
+        const id = setInterval(savedCallback.current, delay);
+        intervalRef.current = id;
+        return () => clearInterval(id);
+      }
+    }, [delay]);
+
+    const resetFetchInterval = useCallback(() => {
+      if (intervalRef.current !== null) {
+        clearInterval(intervalRef.current);
+        intervalRef.current = setInterval(savedCallback.current, delay);
+      }
+    }, [delay]);
+
+    return { resetFetchInterval };
+  };
+
+  const { resetFetchInterval } = useFetchInterval(() => {
+    if (isLoggedIn) {
+      fetchNotifications();
+    }
+  }, Constants.FETCH_INTERVAL);
+
   const onOpenBrowser = useCallback(() => {
     openExternalLink(`https://github.com/${Constants.REPO_SLUG}`);
   }, []);
@@ -80,6 +119,7 @@ export const Sidebar: FC = () => {
               onClick={() => {
                 navigate('/', { replace: true });
                 fetchNotifications();
+                resetFetchInterval();
               }}
               disabled={isFetching}
             >

src/context/App.test.tsx

@@ -58,36 +58,6 @@ describe('context/App.tsx', () => {
       });
     });
 
-    it('fetch notifications every minute', async () => {
-      customRender(null);
-
-      // Wait for the useEffects, for settings.participating and accounts, to run.
-      // Those aren't what we're testing
-      await waitFor(() =>
-        expect(fetchNotificationsMock).toHaveBeenCalledTimes(1),
-      );
-
-      fetchNotificationsMock.mockReset();
-
-      act(() => {
-        jest.advanceTimersByTime(60000);
-        return;
-      });
-      expect(fetchNotificationsMock).toHaveBeenCalledTimes(1);
-
-      act(() => {
-        jest.advanceTimersByTime(60000);
-        return;
-      });
-      expect(fetchNotificationsMock).toHaveBeenCalledTimes(2);
-
-      act(() => {
-        jest.advanceTimersByTime(60000);
-        return;
-      });
-      expect(fetchNotificationsMock).toHaveBeenCalledTimes(3);
-    });
-
     it('should call fetchNotifications', async () => {
       const TestComponent = () => {
         const { fetchNotifications } = useContext(AppContext);

src/context/App.tsx

@@ -7,7 +7,6 @@ import {
   useState,
 } from 'react';
 
-import { useInterval } from '../hooks/useInterval';
 import { useNotifications } from '../hooks/useNotifications';
 import {
   type AccountNotifications,
@@ -111,10 +110,6 @@ export const AppProvider = ({ children }: { children: ReactNode }) => {
     accounts.enterpriseAccounts.length,
   ]);
 
-  useInterval(() => {
-    fetchNotifications(accounts, settings);
-  }, 60000);
-
   // biome-ignore lint/correctness/useExhaustiveDependencies: We need to update tray title when settings or notifications changes.
   useEffect(() => {
     const count = getNotificationCount(notifications);

src/utils/constants.ts

@@ -16,6 +16,8 @@ export const Constants = {
   STORAGE_KEY: 'gitify-storage',
 
   ALLREAD_EMOJIS: ['😉', '🎉', '🐯', '🙈', '🎈', '🎊', '👏', '🎪', '🍝'],
+
+  FETCH_INTERVAL: 60000,
 };
 
 export const Errors: Record<ErrorType, GitifyError> = {