From b3bd8dee64043d63462ad83045f7cd80eda0569e Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sat, 27 Jul 2024 03:58:16 +1000 Subject: [PATCH 01/11] feat: conditional request --- src/utils/api/request.ts | 69 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 65 insertions(+), 4 deletions(-) diff --git a/src/utils/api/request.ts b/src/utils/api/request.ts index 03a71b175..904665f2a 100644 --- a/src/utils/api/request.ts +++ b/src/utils/api/request.ts @@ -1,4 +1,8 @@ -import axios, { type AxiosPromise, type Method } from 'axios'; +import axios, { + type AxiosResponse, + type AxiosPromise, + type Method, +} from 'axios'; import type { Link, Token } from '../../types'; export function apiRequest( @@ -12,7 +16,7 @@ export function apiRequest( return axios({ method, url, data }); } -export function apiRequestAuth( +export async function apiRequestAuth( url: Link, method: Method, token: Token, @@ -20,7 +24,64 @@ export function apiRequestAuth( ): AxiosPromise | null { axios.defaults.headers.common.Accept = 'application/json'; axios.defaults.headers.common.Authorization = `token ${token}`; - axios.defaults.headers.common['Cache-Control'] = 'no-cache'; axios.defaults.headers.common['Content-Type'] = 'application/json'; - return axios({ method, url, data }); + axios.defaults.headers.common['Cache-Control'] = shouldRequestWithNoCache(url) + ? 'no-cache' + : ''; + + let response: AxiosResponse | null = null; + let combinedData: any[] = []; // Array to accumulate data from each response + + try { + let nextUrl: string | null = url; + + // Loop to handle pagination + while (nextUrl) { + response = await axios({ method, url: nextUrl, data }); + combinedData = combinedData.concat(response.data); // Accumulate data + + // Check if there are additional pages + const linkHeader = response.headers.link; + if (hasNextLink(linkHeader)) { + nextUrl = parseNextUrl(linkHeader); + } else { + nextUrl = null; + } + } + } catch (error) { + console.error('API request failed:', error); + return null; + } + + // Return the combined data as part of the response object + return { + ...response, + data: combinedData, + } as AxiosResponse; + + // if (url.includes("/notifications?")) { + // console.log("ADAM ", JSON.stringify(response)); + // } + + // return response; +} + +function shouldRequestWithNoCache(url: string) { + const parsedUrl = new URL(url); + + switch (parsedUrl.pathname) { + case 'notifications': + return true; + default: + return false; + } +} + +function hasNextLink(linkHeader: string) { + return linkHeader?.includes('rel="next"'); +} +// Function to parse the next URL from the "Link" header +function parseNextUrl(linkHeader: string): string | null { + const matches = linkHeader.match(/<([^>]+)>;\s*rel="next"/); + return matches ? matches[1] : null; } From 99ad1f37be6913bbc1284fcd021618b62a46f4b6 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sat, 27 Jul 2024 05:31:53 +1000 Subject: [PATCH 02/11] feat: conditional request --- .../api/__snapshots__/client.test.ts.snap | 24 ++-- .../api/__snapshots__/request.test.ts.snap | 4 +- src/utils/api/client.ts | 7 +- src/utils/api/request.ts | 120 ++++++++---------- src/utils/api/utils.ts | 7 + 5 files changed, 77 insertions(+), 85 deletions(-) diff --git a/src/utils/api/__snapshots__/client.test.ts.snap b/src/utils/api/__snapshots__/client.test.ts.snap index 7f0db3198..54eadc847 100644 --- a/src/utils/api/__snapshots__/client.test.ts.snap +++ b/src/utils/api/__snapshots__/client.test.ts.snap @@ -4,7 +4,7 @@ exports[`utils/api/client.ts getAuthenticatedUser should fetch authenticated use { "Accept": "application/json", "Authorization": "token token-123-456", - "Cache-Control": "no-cache", + "Cache-Control": "", "Content-Type": "application/json", } `; @@ -13,7 +13,7 @@ exports[`utils/api/client.ts getAuthenticatedUser should fetch authenticated use { "Accept": "application/json", "Authorization": "token token-123-456", - "Cache-Control": "no-cache", + "Cache-Control": "", "Content-Type": "application/json", } `; @@ -22,7 +22,7 @@ exports[`utils/api/client.ts headNotifications should fetch notifications head - { "Accept": "application/json", "Authorization": "token token-123-456", - "Cache-Control": "no-cache", + "Cache-Control": "", "Content-Type": "application/json", } `; @@ -40,7 +40,7 @@ exports[`utils/api/client.ts ignoreNotificationThreadSubscription should ignore { "Accept": "application/json", "Authorization": "token token-123-456", - "Cache-Control": "no-cache", + "Cache-Control": "", "Content-Type": "application/json", } `; @@ -49,7 +49,7 @@ exports[`utils/api/client.ts ignoreNotificationThreadSubscription should ignore { "Accept": "application/json", "Authorization": "token token-123-456", - "Cache-Control": "no-cache", + "Cache-Control": "", "Content-Type": "application/json", } `; @@ -67,7 +67,7 @@ exports[`utils/api/client.ts listNotificationsForAuthenticatedUser should list n { "Accept": "application/json", "Authorization": "token 1234568790", - "Cache-Control": "no-cache", + "Cache-Control": "", "Content-Type": "application/json", } `; @@ -76,7 +76,7 @@ exports[`utils/api/client.ts markNotificationThreadAsDone should mark notificati { "Accept": "application/json", "Authorization": "token token-123-456", - "Cache-Control": "no-cache", + "Cache-Control": "", "Content-Type": "application/json", } `; @@ -85,7 +85,7 @@ exports[`utils/api/client.ts markNotificationThreadAsDone should mark notificati { "Accept": "application/json", "Authorization": "token token-123-456", - "Cache-Control": "no-cache", + "Cache-Control": "", "Content-Type": "application/json", } `; @@ -94,7 +94,7 @@ exports[`utils/api/client.ts markNotificationThreadAsRead should mark notificati { "Accept": "application/json", "Authorization": "token token-123-456", - "Cache-Control": "no-cache", + "Cache-Control": "", "Content-Type": "application/json", } `; @@ -103,7 +103,7 @@ exports[`utils/api/client.ts markNotificationThreadAsRead should mark notificati { "Accept": "application/json", "Authorization": "token token-123-456", - "Cache-Control": "no-cache", + "Cache-Control": "", "Content-Type": "application/json", } `; @@ -112,7 +112,7 @@ exports[`utils/api/client.ts markRepositoryNotificationsAsRead should mark repos { "Accept": "application/json", "Authorization": "token token-123-456", - "Cache-Control": "no-cache", + "Cache-Control": "", "Content-Type": "application/json", } `; @@ -121,7 +121,7 @@ exports[`utils/api/client.ts markRepositoryNotificationsAsRead should mark repos { "Accept": "application/json", "Authorization": "token token-123-456", - "Cache-Control": "no-cache", + "Cache-Control": "", "Content-Type": "application/json", } `; diff --git a/src/utils/api/__snapshots__/request.test.ts.snap b/src/utils/api/__snapshots__/request.test.ts.snap index bc9cdb84a..a333fa77e 100644 --- a/src/utils/api/__snapshots__/request.test.ts.snap +++ b/src/utils/api/__snapshots__/request.test.ts.snap @@ -4,7 +4,7 @@ exports[`apiRequestAuth should make an authenticated request with the correct pa { "Accept": "application/json", "Authorization": "token yourAuthToken", - "Cache-Control": "no-cache", + "Cache-Control": "", "Content-Type": "application/json", } `; @@ -13,7 +13,7 @@ exports[`apiRequestAuth should make an authenticated request with the correct pa { "Accept": "application/json", "Authorization": "token yourAuthToken", - "Cache-Control": "no-cache", + "Cache-Control": "", "Content-Type": "application/json", } `; diff --git a/src/utils/api/client.ts b/src/utils/api/client.ts index c558b24bb..ca1511221 100644 --- a/src/utils/api/client.ts +++ b/src/utils/api/client.ts @@ -65,8 +65,7 @@ export function listNotificationsForAuthenticatedUser( const url = getGitHubAPIBaseUrl(account.hostname); url.pathname += 'notifications'; url.searchParams.append('participating', String(settings.participating)); - - return apiRequestAuth(url.toString() as Link, 'GET', account.token); + return apiRequestAuth(url.toString() as Link, 'GET', account.token, {}, true); } /** @@ -271,5 +270,7 @@ export async function getLatestDiscussion( (discussion) => discussion.title === notification.subject.title, )[0] ?? null ); - } catch (err) {} + } catch (err) { + log.error('Failed to get latest discussion'); + } } diff --git a/src/utils/api/request.ts b/src/utils/api/request.ts index 904665f2a..97411e9d5 100644 --- a/src/utils/api/request.ts +++ b/src/utils/api/request.ts @@ -1,87 +1,71 @@ import axios, { - type AxiosResponse, - type AxiosPromise, - type Method, -} from 'axios'; -import type { Link, Token } from '../../types'; + type AxiosResponse, + type AxiosPromise, + type Method, +} from "axios"; +import log from "electron-log"; +import type { Link, Token } from "../../types"; +import { parseNextUrl } from "./utils"; export function apiRequest( - url: Link, - method: Method, - data = {}, + url: Link, + method: Method, + data = {}, ): AxiosPromise | null { - axios.defaults.headers.common.Accept = 'application/json'; - axios.defaults.headers.common['Content-Type'] = 'application/json'; - axios.defaults.headers.common['Cache-Control'] = 'no-cache'; - return axios({ method, url, data }); + axios.defaults.headers.common.Accept = "application/json"; + axios.defaults.headers.common["Content-Type"] = "application/json"; + axios.defaults.headers.common["Cache-Control"] = "no-cache"; + return axios({ method, url, data }); } export async function apiRequestAuth( - url: Link, - method: Method, - token: Token, - data = {}, + url: Link, + method: Method, + token: Token, + data = {}, + paginated = false, ): AxiosPromise | null { - axios.defaults.headers.common.Accept = 'application/json'; - axios.defaults.headers.common.Authorization = `token ${token}`; - axios.defaults.headers.common['Content-Type'] = 'application/json'; - axios.defaults.headers.common['Cache-Control'] = shouldRequestWithNoCache(url) - ? 'no-cache' - : ''; + axios.defaults.headers.common.Accept = "application/json"; + axios.defaults.headers.common.Authorization = `token ${token}`; + axios.defaults.headers.common["Content-Type"] = "application/json"; + axios.defaults.headers.common["Cache-Control"] = shouldRequestWithNoCache(url) + ? "no-cache" + : ""; - let response: AxiosResponse | null = null; - let combinedData: any[] = []; // Array to accumulate data from each response + if (!paginated) { + return axios({ method, url, data }); + } - try { - let nextUrl: string | null = url; + let response: AxiosResponse | null = null; + let combinedData = []; - // Loop to handle pagination - while (nextUrl) { - response = await axios({ method, url: nextUrl, data }); - combinedData = combinedData.concat(response.data); // Accumulate data + try { + let nextUrl: string | null = url; - // Check if there are additional pages - const linkHeader = response.headers.link; - if (hasNextLink(linkHeader)) { - nextUrl = parseNextUrl(linkHeader); - } else { - nextUrl = null; - } - } - } catch (error) { - console.error('API request failed:', error); - return null; - } + while (nextUrl) { + response = await axios({ method, url: nextUrl, data }); + combinedData = combinedData.concat(response.data); // Accumulate data - // Return the combined data as part of the response object - return { - ...response, - data: combinedData, - } as AxiosResponse; + nextUrl = parseNextUrl(response); + } + } catch (error) { + log.error("API request failed:", error); + return null; + } - // if (url.includes("/notifications?")) { - // console.log("ADAM ", JSON.stringify(response)); - // } - - // return response; + return { + ...response, + data: combinedData, + } as AxiosResponse; } function shouldRequestWithNoCache(url: string) { - const parsedUrl = new URL(url); - - switch (parsedUrl.pathname) { - case 'notifications': - return true; - default: - return false; - } -} + const parsedUrl = new URL(url); -function hasNextLink(linkHeader: string) { - return linkHeader?.includes('rel="next"'); -} -// Function to parse the next URL from the "Link" header -function parseNextUrl(linkHeader: string): string | null { - const matches = linkHeader.match(/<([^>]+)>;\s*rel="next"/); - return matches ? matches[1] : null; + switch (parsedUrl.pathname) { + case "/notifications": + return true; + default: + return false; + } } diff --git a/src/utils/api/utils.ts b/src/utils/api/utils.ts index 52fdf25fb..6173f5f69 100644 --- a/src/utils/api/utils.ts +++ b/src/utils/api/utils.ts @@ -1,3 +1,4 @@ +import type { AxiosResponse } from 'axios'; import type { Hostname } from '../../types'; import Constants from '../constants'; import { isEnterpriseHost } from '../helpers'; @@ -22,3 +23,9 @@ export function getGitHubGraphQLUrl(hostname: Hostname): URL { return url; } + +export function parseNextUrl(response: AxiosResponse): string | null { + const linkHeader = response.headers.link; + const matches = linkHeader?.match(/<([^>]+)>;\s*rel="next"/); + return matches ? matches[1] : null; +} From 4a2d7e333c65ad976ec091094732c0b22d7056e9 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sat, 27 Jul 2024 05:41:21 +1000 Subject: [PATCH 03/11] feat: conditional request --- src/utils/api/request.ts | 104 +++++++++++++++++++-------------------- 1 file changed, 52 insertions(+), 52 deletions(-) diff --git a/src/utils/api/request.ts b/src/utils/api/request.ts index 97411e9d5..8ae23823e 100644 --- a/src/utils/api/request.ts +++ b/src/utils/api/request.ts @@ -1,71 +1,71 @@ import axios, { - type AxiosResponse, - type AxiosPromise, - type Method, -} from "axios"; -import log from "electron-log"; -import type { Link, Token } from "../../types"; -import { parseNextUrl } from "./utils"; + type AxiosResponse, + type AxiosPromise, + type Method, +} from 'axios'; +import log from 'electron-log'; +import type { Link, Token } from '../../types'; +import { parseNextUrl } from './utils'; export function apiRequest( - url: Link, - method: Method, - data = {}, + url: Link, + method: Method, + data = {}, ): AxiosPromise | null { - axios.defaults.headers.common.Accept = "application/json"; - axios.defaults.headers.common["Content-Type"] = "application/json"; - axios.defaults.headers.common["Cache-Control"] = "no-cache"; - return axios({ method, url, data }); + axios.defaults.headers.common.Accept = 'application/json'; + axios.defaults.headers.common['Content-Type'] = 'application/json'; + axios.defaults.headers.common['Cache-Control'] = 'no-cache'; + return axios({ method, url, data }); } export async function apiRequestAuth( - url: Link, - method: Method, - token: Token, - data = {}, - paginated = false, + url: Link, + method: Method, + token: Token, + data = {}, + paginated = false, ): AxiosPromise | null { - axios.defaults.headers.common.Accept = "application/json"; - axios.defaults.headers.common.Authorization = `token ${token}`; - axios.defaults.headers.common["Content-Type"] = "application/json"; - axios.defaults.headers.common["Cache-Control"] = shouldRequestWithNoCache(url) - ? "no-cache" - : ""; + axios.defaults.headers.common.Accept = 'application/json'; + axios.defaults.headers.common.Authorization = `token ${token}`; + axios.defaults.headers.common['Content-Type'] = 'application/json'; + axios.defaults.headers.common['Cache-Control'] = shouldRequestWithNoCache(url) + ? 'no-cache' + : ''; - if (!paginated) { - return axios({ method, url, data }); - } + if (!paginated) { + return axios({ method, url, data }); + } - let response: AxiosResponse | null = null; - let combinedData = []; + let response: AxiosResponse | null = null; + let combinedData = []; - try { - let nextUrl: string | null = url; + try { + let nextUrl: string | null = url; - while (nextUrl) { - response = await axios({ method, url: nextUrl, data }); - combinedData = combinedData.concat(response.data); // Accumulate data + while (nextUrl) { + response = await axios({ method, url: nextUrl, data }); + combinedData = combinedData.concat(response.data); // Accumulate data - nextUrl = parseNextUrl(response); - } - } catch (error) { - log.error("API request failed:", error); - return null; - } + nextUrl = parseNextUrl(response); + } + } catch (error) { + log.error('API request failed:', error); + return null; + } - return { - ...response, - data: combinedData, - } as AxiosResponse; + return { + ...response, + data: combinedData, + } as AxiosResponse; } function shouldRequestWithNoCache(url: string) { - const parsedUrl = new URL(url); + const parsedUrl = new URL(url); - switch (parsedUrl.pathname) { - case "/notifications": - return true; - default: - return false; - } + switch (parsedUrl.pathname) { + case '/notifications': + return true; + default: + return false; + } } From bd168d058b9a5e453a4e146caee76da9e3867b30 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sat, 27 Jul 2024 05:46:35 +1000 Subject: [PATCH 04/11] feat: conditional request --- src/utils/api/utils.test.ts | 42 ++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/src/utils/api/utils.test.ts b/src/utils/api/utils.test.ts index e6d488af5..3613f9573 100644 --- a/src/utils/api/utils.test.ts +++ b/src/utils/api/utils.test.ts @@ -1,5 +1,10 @@ +import type { AxiosResponse } from 'axios'; import type { Hostname } from '../../types'; -import { getGitHubAPIBaseUrl, getGitHubGraphQLUrl } from './utils'; +import { + getGitHubAPIBaseUrl, + getGitHubGraphQLUrl, + parseNextUrl, +} from './utils'; describe('utils/api/utils.ts', () => { describe('getGitHubAPIBaseUrl', () => { @@ -25,4 +30,39 @@ describe('utils/api/utils.ts', () => { expect(result.toString()).toBe('https://github.gitify.io/api/graphql'); }); }); + + describe('parseNextUrl', () => { + it('should parse next url from link header', () => { + const mockResponse = { + headers: { + link: '; rel="next", ; rel="last"', + }, + }; + + const result = parseNextUrl(mockResponse as unknown as AxiosResponse); + expect(result.toString()).toBe( + 'https://api.github.com/notifications?participating=false&page=2', + ); + }); + + it('should return null if no next url in link header', () => { + const mockResponse = { + headers: { + link: '; rel="last"', + }, + }; + + const result = parseNextUrl(mockResponse as unknown as AxiosResponse); + expect(result).toBeNull(); + }); + + it('should return null if no link header exists', () => { + const mockResponse = { + headers: {}, + }; + + const result = parseNextUrl(mockResponse as unknown as AxiosResponse); + expect(result).toBeNull(); + }); + }); }); From 71f4316afe74bb83b1a9e62b61f0750cff753c99 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Tue, 30 Jul 2024 00:49:16 +1000 Subject: [PATCH 05/11] rename util fn --- src/utils/api/request.ts | 4 ++-- src/utils/api/utils.test.ts | 16 +++++++++++----- src/utils/api/utils.ts | 4 +++- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/utils/api/request.ts b/src/utils/api/request.ts index 8ae23823e..ba0a260ad 100644 --- a/src/utils/api/request.ts +++ b/src/utils/api/request.ts @@ -5,7 +5,7 @@ import axios, { } from 'axios'; import log from 'electron-log'; import type { Link, Token } from '../../types'; -import { parseNextUrl } from './utils'; +import { getNextURLFromLinkHeader } from './utils'; export function apiRequest( url: Link, @@ -46,7 +46,7 @@ export async function apiRequestAuth( response = await axios({ method, url: nextUrl, data }); combinedData = combinedData.concat(response.data); // Accumulate data - nextUrl = parseNextUrl(response); + nextUrl = getNextURLFromLinkHeader(response); } } catch (error) { log.error('API request failed:', error); diff --git a/src/utils/api/utils.test.ts b/src/utils/api/utils.test.ts index 3613f9573..25494a18b 100644 --- a/src/utils/api/utils.test.ts +++ b/src/utils/api/utils.test.ts @@ -3,7 +3,7 @@ import type { Hostname } from '../../types'; import { getGitHubAPIBaseUrl, getGitHubGraphQLUrl, - parseNextUrl, + getNextURLFromLinkHeader, } from './utils'; describe('utils/api/utils.ts', () => { @@ -31,7 +31,7 @@ describe('utils/api/utils.ts', () => { }); }); - describe('parseNextUrl', () => { + describe('getNextURLFromLinkHeader', () => { it('should parse next url from link header', () => { const mockResponse = { headers: { @@ -39,7 +39,9 @@ describe('utils/api/utils.ts', () => { }, }; - const result = parseNextUrl(mockResponse as unknown as AxiosResponse); + const result = getNextURLFromLinkHeader( + mockResponse as unknown as AxiosResponse, + ); expect(result.toString()).toBe( 'https://api.github.com/notifications?participating=false&page=2', ); @@ -52,7 +54,9 @@ describe('utils/api/utils.ts', () => { }, }; - const result = parseNextUrl(mockResponse as unknown as AxiosResponse); + const result = getNextURLFromLinkHeader( + mockResponse as unknown as AxiosResponse, + ); expect(result).toBeNull(); }); @@ -61,7 +65,9 @@ describe('utils/api/utils.ts', () => { headers: {}, }; - const result = parseNextUrl(mockResponse as unknown as AxiosResponse); + const result = getNextURLFromLinkHeader( + mockResponse as unknown as AxiosResponse, + ); expect(result).toBeNull(); }); }); diff --git a/src/utils/api/utils.ts b/src/utils/api/utils.ts index 20aebfa1f..fc2afa193 100644 --- a/src/utils/api/utils.ts +++ b/src/utils/api/utils.ts @@ -24,7 +24,9 @@ export function getGitHubGraphQLUrl(hostname: Hostname): URL { return url; } -export function parseNextUrl(response: AxiosResponse): string | null { +export function getNextURLFromLinkHeader( + response: AxiosResponse, +): string | null { const linkHeader = response.headers.link; const matches = linkHeader?.match(/<([^>]+)>;\s*rel="next"/); return matches ? matches[1] : null; From b64b230434e4699ba505e60154a6e127ed12a988 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Fri, 6 Sep 2024 13:12:02 -0400 Subject: [PATCH 06/11] Merge branch 'main' into feature/conditional-request Signed-off-by: Adam Setch --- src/utils/api/__snapshots__/client.test.ts.snap | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils/api/__snapshots__/client.test.ts.snap b/src/utils/api/__snapshots__/client.test.ts.snap index 41024d2a6..920d01e18 100644 --- a/src/utils/api/__snapshots__/client.test.ts.snap +++ b/src/utils/api/__snapshots__/client.test.ts.snap @@ -67,7 +67,7 @@ exports[`utils/api/client.ts listNotificationsForAuthenticatedUser should list n { "Accept": "application/json", "Authorization": "token 1234568790", - "Cache-Control": "no-cache", + "Cache-Control": "", "Content-Type": "application/json", } `; @@ -76,7 +76,7 @@ exports[`utils/api/client.ts markNotificationThreadAsDone should mark notificati { "Accept": "application/json", "Authorization": "token token-123-456", - "Cache-Control": "no-cache", + "Cache-Control": "", "Content-Type": "application/json", } `; From 21c87e3b8d83541465d9bd446f49a588e84e8328 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Fri, 6 Sep 2024 15:41:56 -0400 Subject: [PATCH 07/11] feat: fetch all records Signed-off-by: Adam Setch --- src/hooks/useNotifications.test.ts | 4 ++-- src/utils/api/request.ts | 36 +++++++++++++++++++++++++++--- 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/src/hooks/useNotifications.test.ts b/src/hooks/useNotifications.test.ts index d825cebe2..ad41dc3d5 100644 --- a/src/hooks/useNotifications.test.ts +++ b/src/hooks/useNotifications.test.ts @@ -298,7 +298,7 @@ describe('hooks/useNotifications.ts', () => { }); expect(result.current.globalError).toBe(Errors.BAD_CREDENTIALS); - expect(logErrorSpy).toHaveBeenCalledTimes(2); + expect(logErrorSpy).toHaveBeenCalledTimes(4); }); it('should fetch notifications with different failures', async () => { @@ -341,7 +341,7 @@ describe('hooks/useNotifications.ts', () => { }); expect(result.current.globalError).toBeNull(); - expect(logErrorSpy).toHaveBeenCalledTimes(2); + expect(logErrorSpy).toHaveBeenCalledTimes(4); }); }); diff --git a/src/utils/api/request.ts b/src/utils/api/request.ts index ba0a260ad..f469cdfcb 100644 --- a/src/utils/api/request.ts +++ b/src/utils/api/request.ts @@ -7,6 +7,14 @@ import log from 'electron-log'; import type { Link, Token } from '../../types'; import { getNextURLFromLinkHeader } from './utils'; +/** + * Perform an unauthenticated API request + * + * @param url + * @param method + * @param data + * @returns + */ export function apiRequest( url: Link, method: Method, @@ -18,12 +26,22 @@ export function apiRequest( return axios({ method, url, data }); } +/** + * Perform an authenticated API request + * + * @param url + * @param method + * @param token + * @param data + * @param fetchAllRecords whether to fetch all records or just the first page + * @returns + */ export async function apiRequestAuth( url: Link, method: Method, token: Token, data = {}, - paginated = false, + fetchAllRecords = false, ): AxiosPromise | null { axios.defaults.headers.common.Accept = 'application/json'; axios.defaults.headers.common.Authorization = `token ${token}`; @@ -32,7 +50,7 @@ export async function apiRequestAuth( ? 'no-cache' : ''; - if (!paginated) { + if (!fetchAllRecords) { return axios({ method, url, data }); } @@ -44,13 +62,19 @@ export async function apiRequestAuth( while (nextUrl) { response = await axios({ method, url: nextUrl, data }); + + // If no data is returned, break the loop + if (!response?.data) { + break; + } + combinedData = combinedData.concat(response.data); // Accumulate data nextUrl = getNextURLFromLinkHeader(response); } } catch (error) { log.error('API request failed:', error); - return null; + throw error; } return { @@ -59,6 +83,12 @@ export async function apiRequestAuth( } as AxiosResponse; } +/** + * Return true if the request should be made with no-cache + * + * @param url + * @returns boolean + */ function shouldRequestWithNoCache(url: string) { const parsedUrl = new URL(url); From 15e8eae7cd1b0743b5af22b9e7a167db0bdc5fcc Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Mon, 9 Sep 2024 14:37:28 -0400 Subject: [PATCH 08/11] feat: add notification setting to control fetching all Signed-off-by: Adam Setch --- src/__mocks__/state-mocks.ts | 1 + .../settings/NotificationSettings.test.tsx | 26 +++++++++++ .../settings/NotificationSettings.tsx | 20 +++++++++ src/context/App.tsx | 1 + .../__snapshots__/Settings.test.tsx.snap | 44 +++++++++++++++++++ src/types.ts | 1 + src/utils/api/client.ts | 8 +++- 7 files changed, 100 insertions(+), 1 deletion(-) diff --git a/src/__mocks__/state-mocks.ts b/src/__mocks__/state-mocks.ts index 6434f25ff..d625cf808 100644 --- a/src/__mocks__/state-mocks.ts +++ b/src/__mocks__/state-mocks.ts @@ -85,6 +85,7 @@ const mockAppearanceSettings = { const mockNotificationSettings = { groupBy: GroupBy.REPOSITORY, + fetchAllNotifications: true, participating: false, markAsDoneOnOpen: false, markAsDoneOnUnsubscribe: false, diff --git a/src/components/settings/NotificationSettings.test.tsx b/src/components/settings/NotificationSettings.test.tsx index 7b740125f..3de17e1c0 100644 --- a/src/components/settings/NotificationSettings.test.tsx +++ b/src/components/settings/NotificationSettings.test.tsx @@ -34,6 +34,32 @@ describe('routes/components/settings/NotificationSettings.tsx', () => { expect(updateSetting).toHaveBeenCalledTimes(1); expect(updateSetting).toHaveBeenCalledWith('groupBy', 'DATE'); }); + + it('should toggle the fetchAllNotifications checkbox', async () => { + await act(async () => { + render( + + + + + , + ); + }); + + fireEvent.click(screen.getByLabelText('Fetch all notifications'), { + target: { checked: true }, + }); + + expect(updateSetting).toHaveBeenCalledTimes(1); + expect(updateSetting).toHaveBeenCalledWith('fetchAllNotifications', false); + }); + it('should toggle the showOnlyParticipating checkbox', async () => { await act(async () => { render( diff --git a/src/components/settings/NotificationSettings.tsx b/src/components/settings/NotificationSettings.tsx index 329ea9994..b486dedc1 100644 --- a/src/components/settings/NotificationSettings.tsx +++ b/src/components/settings/NotificationSettings.tsx @@ -25,6 +25,26 @@ export const NotificationSettings: FC = () => { updateSetting('groupBy', evt.target.value as GroupBy); }} /> + + updateSetting('fetchAllNotifications', evt.target.checked) + } + tooltip={ +
+
+ When checked, Gitify will fetch all{' '} + notifications from your inbox. +
+
+ When unhecked, Gitify will only fetch the first page of + notifications (max 50 records pet GitHub account) +
+
+ } + /> +
+
+ +
+ +
+
+
diff --git a/src/types.ts b/src/types.ts index 58dbe4cd9..0b96c3471 100644 --- a/src/types.ts +++ b/src/types.ts @@ -76,6 +76,7 @@ interface AppearanceSettingsState { interface NotificationSettingsState { groupBy: GroupBy; + fetchAllNotifications: boolean; participating: boolean; markAsDoneOnOpen: boolean; markAsDoneOnUnsubscribe: boolean; diff --git a/src/utils/api/client.ts b/src/utils/api/client.ts index fccd96a46..54115c276 100644 --- a/src/utils/api/client.ts +++ b/src/utils/api/client.ts @@ -69,7 +69,13 @@ export function listNotificationsForAuthenticatedUser( const url = getGitHubAPIBaseUrl(account.hostname); url.pathname += 'notifications'; url.searchParams.append('participating', String(settings.participating)); - return apiRequestAuth(url.toString() as Link, 'GET', account.token, {}, true); + return apiRequestAuth( + url.toString() as Link, + 'GET', + account.token, + {}, + settings.fetchAllNotifications, + ); } /** From 552420e1f182ff05a6e74f74abc9045ba62a98a0 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Mon, 9 Sep 2024 15:24:22 -0400 Subject: [PATCH 09/11] feat: add notification setting to control fetching all Signed-off-by: Adam Setch --- .../api/__snapshots__/client.test.ts.snap | 11 ++++++- src/utils/api/client.test.ts | 33 ++++++++++++++++--- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/utils/api/__snapshots__/client.test.ts.snap b/src/utils/api/__snapshots__/client.test.ts.snap index 920d01e18..5e586536a 100644 --- a/src/utils/api/__snapshots__/client.test.ts.snap +++ b/src/utils/api/__snapshots__/client.test.ts.snap @@ -54,7 +54,16 @@ exports[`utils/api/client.ts ignoreNotificationThreadSubscription should ignore } `; -exports[`utils/api/client.ts listNotificationsForAuthenticatedUser should list notifications for user - github cloud 1`] = ` +exports[`utils/api/client.ts listNotificationsForAuthenticatedUser should list notifications for user - github cloud - fetchAllNotifications false 1`] = ` +{ + "Accept": "application/json", + "Authorization": "token token-123-456", + "Cache-Control": "no-cache", + "Content-Type": "application/json", +} +`; + +exports[`utils/api/client.ts listNotificationsForAuthenticatedUser should list notifications for user - github cloud - fetchAllNotifications true 1`] = ` { "Accept": "application/json", "Authorization": "token token-123-456", diff --git a/src/utils/api/client.test.ts b/src/utils/api/client.test.ts index 7f6ac0c7a..0581106cf 100644 --- a/src/utils/api/client.test.ts +++ b/src/utils/api/client.test.ts @@ -83,11 +83,32 @@ describe('utils/api/client.ts', () => { }); describe('listNotificationsForAuthenticatedUser', () => { - const mockSettings: Partial = { - participating: true, - }; + it('should list notifications for user - github cloud - fetchAllNotifications true', async () => { + const mockSettings: Partial = { + participating: true, + fetchAllNotifications: true, + }; + + await listNotificationsForAuthenticatedUser( + mockGitHubCloudAccount, + mockSettings as SettingsState, + ); + + expect(axios).toHaveBeenCalledWith({ + url: 'https://api.github.com/notifications?participating=true', + method: 'GET', + data: {}, + }); + + expect(axios.defaults.headers.common).toMatchSnapshot(); + }); + + it('should list notifications for user - github cloud - fetchAllNotifications false', async () => { + const mockSettings: Partial = { + participating: true, + fetchAllNotifications: false, + }; - it('should list notifications for user - github cloud', async () => { await listNotificationsForAuthenticatedUser( mockGitHubCloudAccount, mockSettings as SettingsState, @@ -103,6 +124,10 @@ describe('utils/api/client.ts', () => { }); it('should list notifications for user - github enterprise server', async () => { + const mockSettings: Partial = { + participating: true, + }; + await listNotificationsForAuthenticatedUser( mockGitHubEnterpriseServerAccount, mockSettings as SettingsState, From f10dc04ee2000d2c6340c5b9a0dc6d4ea4c62913 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Fri, 13 Sep 2024 08:35:50 -0400 Subject: [PATCH 10/11] Update src/components/settings/NotificationSettings.tsx Co-authored-by: Afonso Jorge Ramos --- src/components/settings/NotificationSettings.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/settings/NotificationSettings.tsx b/src/components/settings/NotificationSettings.tsx index b486dedc1..b77c9b20c 100644 --- a/src/components/settings/NotificationSettings.tsx +++ b/src/components/settings/NotificationSettings.tsx @@ -40,7 +40,7 @@ export const NotificationSettings: FC = () => {
When unhecked, Gitify will only fetch the first page of - notifications (max 50 records pet GitHub account) + notifications (max 50 records per GitHub account)
} From 994b0725567ba28218302cdfd9ce0b2d550bf819 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Fri, 13 Sep 2024 08:35:59 -0400 Subject: [PATCH 11/11] Update src/components/settings/NotificationSettings.tsx Co-authored-by: Afonso Jorge Ramos --- src/components/settings/NotificationSettings.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/settings/NotificationSettings.tsx b/src/components/settings/NotificationSettings.tsx index b77c9b20c..056a85b9b 100644 --- a/src/components/settings/NotificationSettings.tsx +++ b/src/components/settings/NotificationSettings.tsx @@ -39,7 +39,7 @@ export const NotificationSettings: FC = () => { notifications from your inbox.
- When unhecked, Gitify will only fetch the first page of + When unchecked, Gitify will only fetch the first page of notifications (max 50 records per GitHub account)