Skip to content

Commit 7f6f0cb

Browse files
setchysetchy
authored
feat(settings): allow user configurable notification fetch intervals (#2302)
* feat(settings): make fetch interval user configurable Signed-off-by: Adam Setch <[email protected]> * feat(settings): make fetch interval user configurable Signed-off-by: Adam Setch <[email protected]> * feat(settings): make fetch interval user configurable Signed-off-by: Adam Setch <[email protected]> * feat(settings): make fetch interval user configurable Signed-off-by: Adam Setch <[email protected]> --------- Signed-off-by: Adam Setch <[email protected]>
1 parent d4843b1 commit 7f6f0cb

File tree

9 files changed

+422
-23
lines changed

9 files changed

+422
-23
lines changed

‎src/renderer/__mocks__/state-mocks.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ const mockAppearanceSettings: AppearanceSettingsState = {
8989
const mockNotificationSettings: NotificationSettingsState = {
9090
groupBy: GroupBy.REPOSITORY,
9191
fetchType: FetchType.INTERVAL,
92+
fetchInterval: Constants.DEFAULT_FETCH_NOTIFICATIONS_INTERVAL_MS,
9293
fetchAllNotifications: true,
9394
detailedNotifications: true,
9495
showPills: true,

‎src/renderer/components/settings/NotificationSettings.test.tsx‎

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { act, render, screen } from '@testing-library/react';
22
import userEvent from '@testing-library/user-event';
33

44
import { mockAuth, mockSettings } from '../../__mocks__/state-mocks';
5+
import { Constants } from '../../constants';
56
import { AppContext } from '../../context/App';
67
import * as comms from '../../utils/comms';
78
import { NotificationSettings } from './NotificationSettings';
@@ -55,6 +56,161 @@ describe('renderer/components/settings/NotificationSettings.tsx', () => {
5556
expect(updateSetting).toHaveBeenCalledWith('fetchType', 'INACTIVITY');
5657
});
5758

59+
describe('fetch interval settings', () => {
60+
it('should update the fetch interval values when using the buttons', async () => {
61+
await act(async () => {
62+
render(
63+
<AppContext.Provider
64+
value={{
65+
auth: mockAuth,
66+
settings: mockSettings,
67+
updateSetting,
68+
}}
69+
>
70+
<NotificationSettings />
71+
</AppContext.Provider>,
72+
);
73+
});
74+
75+
// Increase fetch interval
76+
await act(async () => {
77+
await userEvent.click(
78+
screen.getByTestId('settings-fetch-interval-increase'),
79+
);
80+
81+
expect(updateSetting).toHaveBeenCalledTimes(1);
82+
expect(updateSetting).toHaveBeenCalledWith('fetchInterval', 120000);
83+
});
84+
85+
await act(async () => {
86+
await userEvent.click(
87+
screen.getByTestId('settings-fetch-interval-increase'),
88+
);
89+
90+
expect(updateSetting).toHaveBeenCalledTimes(2);
91+
expect(updateSetting).toHaveBeenNthCalledWith(
92+
2,
93+
'fetchInterval',
94+
180000,
95+
);
96+
});
97+
98+
// Decrease fetch interval
99+
await act(async () => {
100+
await userEvent.click(
101+
screen.getByTestId('settings-fetch-interval-decrease'),
102+
);
103+
104+
expect(updateSetting).toHaveBeenCalledTimes(3);
105+
expect(updateSetting).toHaveBeenNthCalledWith(
106+
3,
107+
'fetchInterval',
108+
120000,
109+
);
110+
});
111+
112+
// Fetch interval reset
113+
await act(async () => {
114+
await userEvent.click(
115+
screen.getByTestId('settings-fetch-interval-reset'),
116+
);
117+
118+
expect(updateSetting).toHaveBeenCalledTimes(4);
119+
expect(updateSetting).toHaveBeenNthCalledWith(
120+
4,
121+
'fetchInterval',
122+
60000,
123+
);
124+
});
125+
});
126+
127+
it('should prevent going lower than minimum interval', async () => {
128+
await act(async () => {
129+
render(
130+
<AppContext.Provider
131+
value={{
132+
auth: mockAuth,
133+
settings: {
134+
...mockSettings,
135+
fetchInterval:
136+
Constants.MIN_FETCH_NOTIFICATIONS_INTERVAL_MS +
137+
Constants.FETCH_NOTIFICATIONS_INTERVAL_STEP_MS,
138+
},
139+
updateSetting,
140+
}}
141+
>
142+
<NotificationSettings />
143+
</AppContext.Provider>,
144+
);
145+
});
146+
147+
await act(async () => {
148+
await userEvent.click(
149+
screen.getByTestId('settings-fetch-interval-decrease'),
150+
);
151+
152+
expect(updateSetting).toHaveBeenCalledTimes(1);
153+
expect(updateSetting).toHaveBeenNthCalledWith(
154+
1,
155+
'fetchInterval',
156+
60000,
157+
);
158+
});
159+
160+
// Attempt to go below the minimum interval, update settings should not be called
161+
await act(async () => {
162+
await userEvent.click(
163+
screen.getByTestId('settings-fetch-interval-decrease'),
164+
);
165+
166+
expect(updateSetting).toHaveBeenCalledTimes(1);
167+
});
168+
});
169+
170+
it('should prevent going above maximum interval', async () => {
171+
await act(async () => {
172+
render(
173+
<AppContext.Provider
174+
value={{
175+
auth: mockAuth,
176+
settings: {
177+
...mockSettings,
178+
fetchInterval:
179+
Constants.MAX_FETCH_NOTIFICATIONS_INTERVAL_MS -
180+
Constants.FETCH_NOTIFICATIONS_INTERVAL_STEP_MS,
181+
},
182+
updateSetting,
183+
}}
184+
>
185+
<NotificationSettings />
186+
</AppContext.Provider>,
187+
);
188+
});
189+
190+
await act(async () => {
191+
await userEvent.click(
192+
screen.getByTestId('settings-fetch-interval-increase'),
193+
);
194+
195+
expect(updateSetting).toHaveBeenCalledTimes(1);
196+
expect(updateSetting).toHaveBeenNthCalledWith(
197+
1,
198+
'fetchInterval',
199+
3600000,
200+
);
201+
});
202+
203+
// Attempt to go above the maximum interval, update settings should not be called
204+
await act(async () => {
205+
await userEvent.click(
206+
screen.getByTestId('settings-fetch-interval-increase'),
207+
);
208+
209+
expect(updateSetting).toHaveBeenCalledTimes(1);
210+
});
211+
});
212+
});
213+
58214
it('should toggle the fetchAllNotifications checkbox', async () => {
59215
await act(async () => {
60216
render(

‎src/renderer/components/settings/NotificationSettings.tsx‎

Lines changed: 97 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,47 @@
1-
import { type FC, type MouseEvent, useContext } from 'react';
1+
import {
2+
type FC,
3+
type MouseEvent,
4+
useContext,
5+
useEffect,
6+
useState,
7+
} from 'react';
28

39
import {
410
BellIcon,
511
CheckIcon,
612
CommentIcon,
13+
DashIcon,
714
GitPullRequestIcon,
815
IssueOpenedIcon,
916
MilestoneIcon,
17+
PlusIcon,
18+
SyncIcon,
1019
TagIcon,
1120
} from '@primer/octicons-react';
12-
import { Stack, Text } from '@primer/react';
21+
import { Button, ButtonGroup, IconButton, Stack, Text } from '@primer/react';
22+
23+
import { formatDuration, millisecondsToMinutes } from 'date-fns';
1324

1425
import { APPLICATION } from '../../../shared/constants';
1526

27+
import { Constants } from '../../constants';
1628
import { AppContext } from '../../context/App';
1729
import { FetchType, GroupBy, Size } from '../../types';
1830
import { openGitHubParticipatingDocs } from '../../utils/links';
1931
import { Checkbox } from '../fields/Checkbox';
32+
import { FieldLabel } from '../fields/FieldLabel';
2033
import { RadioGroup } from '../fields/RadioGroup';
2134
import { Title } from '../primitives/Title';
2235

2336
export const NotificationSettings: FC = () => {
2437
const { settings, updateSetting } = useContext(AppContext);
38+
const [fetchInterval, setFetchInterval] = useState<number>(
39+
settings.fetchInterval,
40+
);
41+
42+
useEffect(() => {
43+
setFetchInterval(settings.fetchInterval);
44+
}, [settings.fetchInterval]);
2545

2646
return (
2747
<fieldset>
@@ -68,6 +88,81 @@ export const NotificationSettings: FC = () => {
6888
value={settings.fetchType}
6989
/>
7090

91+
<Stack
92+
align="center"
93+
className="text-sm"
94+
direction="horizontal"
95+
gap="condensed"
96+
>
97+
<FieldLabel label="Fetch interval:" name="fetchInterval" />
98+
99+
<ButtonGroup className="ml-2">
100+
<IconButton
101+
aria-label="Decrease fetch interval"
102+
data-testid="settings-fetch-interval-decrease"
103+
icon={DashIcon}
104+
onClick={() => {
105+
const newInterval = Math.max(
106+
fetchInterval -
107+
Constants.FETCH_NOTIFICATIONS_INTERVAL_STEP_MS,
108+
Constants.MIN_FETCH_NOTIFICATIONS_INTERVAL_MS,
109+
);
110+
111+
if (newInterval !== fetchInterval) {
112+
setFetchInterval(newInterval);
113+
updateSetting('fetchInterval', newInterval);
114+
}
115+
}}
116+
size="small"
117+
unsafeDisableTooltip={true}
118+
/>
119+
120+
<Button aria-label="Fetch interval" disabled size="small">
121+
{formatDuration({
122+
minutes: millisecondsToMinutes(fetchInterval),
123+
})}
124+
</Button>
125+
126+
<IconButton
127+
aria-label="Increase fetch interval"
128+
data-testid="settings-fetch-interval-increase"
129+
icon={PlusIcon}
130+
onClick={() => {
131+
const newInterval = Math.min(
132+
fetchInterval +
133+
Constants.FETCH_NOTIFICATIONS_INTERVAL_STEP_MS,
134+
Constants.MAX_FETCH_NOTIFICATIONS_INTERVAL_MS,
135+
);
136+
137+
if (newInterval !== fetchInterval) {
138+
setFetchInterval(newInterval);
139+
updateSetting('fetchInterval', newInterval);
140+
}
141+
}}
142+
size="small"
143+
unsafeDisableTooltip={true}
144+
/>
145+
146+
<IconButton
147+
aria-label="Reset fetch interval"
148+
data-testid="settings-fetch-interval-reset"
149+
icon={SyncIcon}
150+
onClick={() => {
151+
setFetchInterval(
152+
Constants.DEFAULT_FETCH_NOTIFICATIONS_INTERVAL_MS,
153+
);
154+
updateSetting(
155+
'fetchInterval',
156+
Constants.DEFAULT_FETCH_NOTIFICATIONS_INTERVAL_MS,
157+
);
158+
}}
159+
size="small"
160+
unsafeDisableTooltip={true}
161+
variant="danger"
162+
/>
163+
</ButtonGroup>
164+
</Stack>
165+
71166
<Checkbox
72167
checked={settings.fetchAllNotifications}
73168
label="Fetch all notifications"

‎src/renderer/constants.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,10 @@ export const Constants = {
2020

2121
ALL_READ_EMOJIS: ['🎉', '🎊', '🥳', '👏', '🙌', '😎', '🏖️', '🚀', '✨', '🏆'],
2222

23-
FETCH_NOTIFICATIONS_INTERVAL_MS: 60 * 1000, // 1 minute
23+
DEFAULT_FETCH_NOTIFICATIONS_INTERVAL_MS: 60 * 1000, // 1 minute
24+
MIN_FETCH_NOTIFICATIONS_INTERVAL_MS: 60 * 1000, // 1 minute
25+
MAX_FETCH_NOTIFICATIONS_INTERVAL_MS: 60 * 60 * 1000, // 1 hour
26+
FETCH_NOTIFICATIONS_INTERVAL_STEP_MS: 60 * 1000, // 1 minute
2427

2528
REFRESH_ACCOUNTS_INTERVAL_MS: 60 * 60 * 1000, // 1 hour
2629

‎src/renderer/context/App.test.tsx‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,19 +77,25 @@ describe('renderer/context/App.tsx', () => {
7777
);
7878

7979
act(() => {
80-
jest.advanceTimersByTime(Constants.FETCH_NOTIFICATIONS_INTERVAL_MS);
80+
jest.advanceTimersByTime(
81+
Constants.DEFAULT_FETCH_NOTIFICATIONS_INTERVAL_MS,
82+
);
8183
return;
8284
});
8385
expect(fetchNotificationsMock).toHaveBeenCalledTimes(2);
8486

8587
act(() => {
86-
jest.advanceTimersByTime(Constants.FETCH_NOTIFICATIONS_INTERVAL_MS);
88+
jest.advanceTimersByTime(
89+
Constants.DEFAULT_FETCH_NOTIFICATIONS_INTERVAL_MS,
90+
);
8791
return;
8892
});
8993
expect(fetchNotificationsMock).toHaveBeenCalledTimes(3);
9094

9195
act(() => {
92-
jest.advanceTimersByTime(Constants.FETCH_NOTIFICATIONS_INTERVAL_MS);
96+
jest.advanceTimersByTime(
97+
Constants.DEFAULT_FETCH_NOTIFICATIONS_INTERVAL_MS,
98+
);
9399
return;
94100
});
95101
expect(fetchNotificationsMock).toHaveBeenCalledTimes(4);

‎src/renderer/context/App.tsx‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -148,18 +148,14 @@ export const AppProvider = ({ children }: { children: ReactNode }) => {
148148
() => {
149149
fetchNotifications({ auth, settings });
150150
},
151-
settings.fetchType === FetchType.INTERVAL
152-
? Constants.FETCH_NOTIFICATIONS_INTERVAL_MS
153-
: null,
151+
settings.fetchType === FetchType.INTERVAL ? settings.fetchInterval : null,
154152
);
155153

156154
useInactivityTimer(
157155
() => {
158156
fetchNotifications({ auth, settings });
159157
},
160-
settings.fetchType === FetchType.INACTIVITY
161-
? Constants.FETCH_NOTIFICATIONS_INTERVAL_MS
162-
: null,
158+
settings.fetchType === FetchType.INACTIVITY ? settings.fetchInterval : null,
163159
);
164160

165161
useIntervalTimer(() => {

‎src/renderer/context/defaults.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { Constants } from '../constants';
12
import {
23
type AppearanceSettingsState,
34
type AuthState,
@@ -27,6 +28,7 @@ const defaultAppearanceSettings: AppearanceSettingsState = {
2728
const defaultNotificationSettings: NotificationSettingsState = {
2829
groupBy: GroupBy.REPOSITORY,
2930
fetchType: FetchType.INTERVAL,
31+
fetchInterval: Constants.DEFAULT_FETCH_NOTIFICATIONS_INTERVAL_MS,
3032
fetchAllNotifications: true,
3133
detailedNotifications: true,
3234
showPills: true,

0 commit comments

Comments
 (0)