Skip to content

Commit b90814d

Browse files
setchyafonsojramos
andauthored
feat(api): use conditional requests and fetch all inbox notifications (#1414)
* feat: conditional request * feat: conditional request * feat: conditional request * feat: conditional request * rename util fn * Merge branch 'main' into feature/conditional-request Signed-off-by: Adam Setch <[email protected]> * feat: fetch all records Signed-off-by: Adam Setch <[email protected]> * feat: add notification setting to control fetching all Signed-off-by: Adam Setch <[email protected]> * feat: add notification setting to control fetching all Signed-off-by: Adam Setch <[email protected]> * Update src/components/settings/NotificationSettings.tsx Co-authored-by: Afonso Jorge Ramos <[email protected]> * Update src/components/settings/NotificationSettings.tsx Co-authored-by: Afonso Jorge Ramos <[email protected]> --------- Signed-off-by: Adam Setch <[email protected]> Co-authored-by: Afonso Jorge Ramos <[email protected]>
1 parent 7d4c723 commit b90814d

14 files changed

+289
-27
lines changed

src/__mocks__/state-mocks.ts

+1
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ const mockAppearanceSettings = {
8585

8686
const mockNotificationSettings = {
8787
groupBy: GroupBy.REPOSITORY,
88+
fetchAllNotifications: true,
8889
participating: false,
8990
markAsDoneOnOpen: false,
9091
markAsDoneOnUnsubscribe: false,

src/components/settings/NotificationSettings.test.tsx

+26
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,32 @@ describe('routes/components/settings/NotificationSettings.tsx', () => {
3434
expect(updateSetting).toHaveBeenCalledTimes(1);
3535
expect(updateSetting).toHaveBeenCalledWith('groupBy', 'DATE');
3636
});
37+
38+
it('should toggle the fetchAllNotifications checkbox', async () => {
39+
await act(async () => {
40+
render(
41+
<AppContext.Provider
42+
value={{
43+
auth: mockAuth,
44+
settings: mockSettings,
45+
updateSetting,
46+
}}
47+
>
48+
<MemoryRouter>
49+
<NotificationSettings />
50+
</MemoryRouter>
51+
</AppContext.Provider>,
52+
);
53+
});
54+
55+
fireEvent.click(screen.getByLabelText('Fetch all notifications'), {
56+
target: { checked: true },
57+
});
58+
59+
expect(updateSetting).toHaveBeenCalledTimes(1);
60+
expect(updateSetting).toHaveBeenCalledWith('fetchAllNotifications', false);
61+
});
62+
3763
it('should toggle the showOnlyParticipating checkbox', async () => {
3864
await act(async () => {
3965
render(

src/components/settings/NotificationSettings.tsx

+20
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,26 @@ export const NotificationSettings: FC = () => {
2525
updateSetting('groupBy', evt.target.value as GroupBy);
2626
}}
2727
/>
28+
<Checkbox
29+
name="fetchAllNotifications"
30+
label="Fetch all notifications"
31+
checked={settings.fetchAllNotifications}
32+
onChange={(evt) =>
33+
updateSetting('fetchAllNotifications', evt.target.checked)
34+
}
35+
tooltip={
36+
<div>
37+
<div className="pb-3">
38+
When <em>checked</em>, Gitify will fetch <strong>all</strong>{' '}
39+
notifications from your inbox.
40+
</div>
41+
<div>
42+
When <em>unchecked</em>, Gitify will only fetch the first page of
43+
notifications (max 50 records per GitHub account)
44+
</div>
45+
</div>
46+
}
47+
/>
2848
<Checkbox
2949
name="showOnlyParticipating"
3050
label="Show only participating"

src/context/App.tsx

+1
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ const defaultAppearanceSettings = {
6666

6767
const defaultNotificationSettings = {
6868
groupBy: GroupBy.REPOSITORY,
69+
fetchAllNotifications: true,
6970
participating: false,
7071
markAsDoneOnOpen: false,
7172
markAsDoneOnUnsubscribe: false,

src/hooks/useNotifications.test.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ describe('hooks/useNotifications.ts', () => {
298298
});
299299

300300
expect(result.current.globalError).toBe(Errors.BAD_CREDENTIALS);
301-
expect(logErrorSpy).toHaveBeenCalledTimes(2);
301+
expect(logErrorSpy).toHaveBeenCalledTimes(4);
302302
});
303303

304304
it('should fetch notifications with different failures', async () => {
@@ -341,7 +341,7 @@ describe('hooks/useNotifications.ts', () => {
341341
});
342342

343343
expect(result.current.globalError).toBeNull();
344-
expect(logErrorSpy).toHaveBeenCalledTimes(2);
344+
expect(logErrorSpy).toHaveBeenCalledTimes(4);
345345
});
346346
});
347347

src/routes/__snapshots__/Settings.test.tsx.snap

+44
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/types.ts

+1
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ interface AppearanceSettingsState {
7676

7777
interface NotificationSettingsState {
7878
groupBy: GroupBy;
79+
fetchAllNotifications: boolean;
7980
participating: boolean;
8081
markAsDoneOnOpen: boolean;
8182
markAsDoneOnUnsubscribe: boolean;

src/utils/api/__snapshots__/client.test.ts.snap

+21-12
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/utils/api/__snapshots__/request.test.ts.snap

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/utils/api/client.test.ts

+29-4
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,32 @@ describe('utils/api/client.ts', () => {
8383
});
8484

8585
describe('listNotificationsForAuthenticatedUser', () => {
86-
const mockSettings: Partial<SettingsState> = {
87-
participating: true,
88-
};
86+
it('should list notifications for user - github cloud - fetchAllNotifications true', async () => {
87+
const mockSettings: Partial<SettingsState> = {
88+
participating: true,
89+
fetchAllNotifications: true,
90+
};
91+
92+
await listNotificationsForAuthenticatedUser(
93+
mockGitHubCloudAccount,
94+
mockSettings as SettingsState,
95+
);
96+
97+
expect(axios).toHaveBeenCalledWith({
98+
url: 'https://api.github.com/notifications?participating=true',
99+
method: 'GET',
100+
data: {},
101+
});
102+
103+
expect(axios.defaults.headers.common).toMatchSnapshot();
104+
});
105+
106+
it('should list notifications for user - github cloud - fetchAllNotifications false', async () => {
107+
const mockSettings: Partial<SettingsState> = {
108+
participating: true,
109+
fetchAllNotifications: false,
110+
};
89111

90-
it('should list notifications for user - github cloud', async () => {
91112
await listNotificationsForAuthenticatedUser(
92113
mockGitHubCloudAccount,
93114
mockSettings as SettingsState,
@@ -103,6 +124,10 @@ describe('utils/api/client.ts', () => {
103124
});
104125

105126
it('should list notifications for user - github enterprise server', async () => {
127+
const mockSettings: Partial<SettingsState> = {
128+
participating: true,
129+
};
130+
106131
await listNotificationsForAuthenticatedUser(
107132
mockGitHubEnterpriseServerAccount,
108133
mockSettings as SettingsState,

src/utils/api/client.ts

+7-2
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,13 @@ export function listNotificationsForAuthenticatedUser(
6969
const url = getGitHubAPIBaseUrl(account.hostname);
7070
url.pathname += 'notifications';
7171
url.searchParams.append('participating', String(settings.participating));
72-
73-
return apiRequestAuth(url.toString() as Link, 'GET', account.token);
72+
return apiRequestAuth(
73+
url.toString() as Link,
74+
'GET',
75+
account.token,
76+
{},
77+
settings.fetchAllNotifications,
78+
);
7479
}
7580

7681
/**

0 commit comments

Comments
 (0)