From 6491833b7ed7a0f03a83f8e9d95ecbbec1fa4ee2 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sat, 13 Dec 2025 07:59:59 +1000 Subject: [PATCH 1/5] feat(url): improve fallback url handling Signed-off-by: Adam Setch --- src/renderer/utils/helpers.test.ts | 97 +++++++++++++++++++++--------- src/renderer/utils/helpers.ts | 94 ++++++++++++++++------------- 2 files changed, 120 insertions(+), 71 deletions(-) diff --git a/src/renderer/utils/helpers.test.ts b/src/renderer/utils/helpers.test.ts index bf8825bcd..d04d4ceaa 100644 --- a/src/renderer/utils/helpers.test.ts +++ b/src/renderer/utils/helpers.test.ts @@ -273,7 +273,7 @@ describe('renderer/utils/helpers.ts', () => { }); describe('Discussions URLs', () => { - it('when no subject urls and no discussions found via query, default to linking to repository discussions', async () => { + it('when no discussion found via graphql api, default to base repository discussion url', async () => { const subject = { title: 'generate github web url unit tests', url: null, @@ -296,7 +296,11 @@ describe('renderer/utils/helpers.ts', () => { ); }); - it('link to matching discussion and comment hash', async () => { + it('when error fetching discussion via graphql api, default to base repository discussion url', async () => { + const rendererLogErrorSpy = jest + .spyOn(logger, 'rendererLogError') + .mockImplementation(); + const subject = { title: '1.16.0', url: null, @@ -306,7 +310,12 @@ describe('renderer/utils/helpers.ts', () => { apiRequestAuthSpy.mockResolvedValue({ data: { - ...mockGraphQLResponse, + data: null, + errors: [ + { + message: 'Something failed', + }, + ], }, } as AxiosResponse); @@ -317,15 +326,12 @@ describe('renderer/utils/helpers.ts', () => { expect(apiRequestAuthSpy).toHaveBeenCalledTimes(1); expect(result).toBe( - `https://github.com/gitify-app/notifications-test/discussions/612?${mockNotificationReferrer}#discussioncomment-2300902`, + `https://github.com/gitify-app/notifications-test/discussions?${mockNotificationReferrer}`, ); + expect(rendererLogErrorSpy).toHaveBeenCalledTimes(1); }); - it('default to base discussions url when graphql query fails', async () => { - const rendererLogErrorSpy = jest - .spyOn(logger, 'rendererLogError') - .mockImplementation(); - + it('when discussion found via graphql api, link to matching discussion and comment hash', async () => { const subject = { title: '1.16.0', url: null, @@ -333,7 +339,11 @@ describe('renderer/utils/helpers.ts', () => { type: 'Discussion' as SubjectType, }; - apiRequestAuthSpy.mockResolvedValue(null as AxiosResponse); + apiRequestAuthSpy.mockResolvedValue({ + data: { + ...mockGraphQLResponse, + }, + } as AxiosResponse); const result = await generateGitHubWebUrl({ ...mockSingleNotification, @@ -342,9 +352,8 @@ describe('renderer/utils/helpers.ts', () => { expect(apiRequestAuthSpy).toHaveBeenCalledTimes(1); expect(result).toBe( - `https://github.com/gitify-app/notifications-test/discussions?${mockNotificationReferrer}`, + `https://github.com/gitify-app/notifications-test/discussions/612?${mockNotificationReferrer}#discussioncomment-2300902`, ); - expect(rendererLogErrorSpy).toHaveBeenCalledTimes(1); }); }); @@ -447,8 +456,8 @@ describe('renderer/utils/helpers.ts', () => { }); }); - describe('defaults to repository url', () => { - it('defaults when no urls present in notification', async () => { + describe('defaults web urls', () => { + it('issues - defaults when no urls present in notification', async () => { const subject = { title: 'generate github web url unit tests', url: null, @@ -463,7 +472,45 @@ describe('renderer/utils/helpers.ts', () => { expect(apiRequestAuthSpy).toHaveBeenCalledTimes(0); expect(result).toBe( - `${mockSingleNotification.repository.html_url}?${mockNotificationReferrer}`, + `https://github.com/gitify-app/notifications-test/issues?${mockNotificationReferrer}`, + ); + }); + + it('issues - defaults when no urls present in notification', async () => { + const subject = { + title: 'generate github web url unit tests', + url: null, + latest_comment_url: null, + type: 'PullRequest' as SubjectType, + }; + + const result = await generateGitHubWebUrl({ + ...mockSingleNotification, + subject: subject, + }); + + expect(apiRequestAuthSpy).toHaveBeenCalledTimes(0); + expect(result).toBe( + `https://github.com/gitify-app/notifications-test/pulls?${mockNotificationReferrer}`, + ); + }); + + it('other - defaults when no urls present in notification', async () => { + const subject = { + title: 'generate github web url unit tests', + url: null, + latest_comment_url: null, + type: 'Commit' as SubjectType, + }; + + const result = await generateGitHubWebUrl({ + ...mockSingleNotification, + subject: subject, + }); + + expect(apiRequestAuthSpy).toHaveBeenCalledTimes(0); + expect(result).toBe( + `https://github.com/gitify-app/notifications-test?${mockNotificationReferrer}`, ); }); @@ -474,15 +521,12 @@ describe('renderer/utils/helpers.ts', () => { const subject = { title: 'generate github web url unit tests', - url: 'https://api.github.com/repos/gitify-app/notifications-test/issues/1' as Link, - latest_comment_url: - 'https://api.github.com/repos/gitify-app/notifications-test/issues/comments/302888448' as Link, - type: 'Issue' as SubjectType, + url: 'https://api.github.com/repos/gitify-app/notifications-test/discussion/1' as Link, + latest_comment_url: null as Link, + type: 'Discussion' as SubjectType, }; - const mockError = new Error('Test error'); - - apiRequestAuthSpy.mockRejectedValue(mockError); + apiRequestAuthSpy.mockRejectedValue(new Error('Test error')); const result = await generateGitHubWebUrl({ ...mockSingleNotification, @@ -490,15 +534,10 @@ describe('renderer/utils/helpers.ts', () => { }); expect(apiRequestAuthSpy).toHaveBeenCalledTimes(1); - expect(apiRequestAuthSpy).toHaveBeenCalledWith( - subject.latest_comment_url, - 'GET', - mockPersonalAccessTokenAccount.token, - ); expect(result).toBe( - `https://github.com/gitify-app/notifications-test?${mockNotificationReferrer}`, + `https://github.com/gitify-app/notifications-test/discussions?${mockNotificationReferrer}`, ); - expect(rendererLogErrorSpy).toHaveBeenCalledTimes(2); + expect(rendererLogErrorSpy).toHaveBeenCalledTimes(1); }); }); }); diff --git a/src/renderer/utils/helpers.ts b/src/renderer/utils/helpers.ts index 25c87e753..553657248 100644 --- a/src/renderer/utils/helpers.ts +++ b/src/renderer/utils/helpers.ts @@ -6,7 +6,7 @@ import { import { Constants } from '../constants'; import type { Chevron, Hostname, Link } from '../types'; -import type { Notification } from '../typesGitHub'; +import type { Notification, SubjectType } from '../typesGitHub'; import { getHtmlUrl, getLatestDiscussion } from './api/client'; import type { PlatformType } from './auth/types'; import { rendererLogError } from './logger'; @@ -106,48 +106,38 @@ export async function generateGitHubWebUrl( ): Promise { const url = new URL(notification.repository.html_url); - /** - * Discussions - override generic response values for subject and comment urls, - * as we will fetch more specific html urls ourselves below - * See issue #1583: - */ - if (notification.subject.type === 'Discussion') { - notification.subject.url = null; - notification.subject.latest_comment_url = null; - } - try { - if (notification.subject.latest_comment_url) { - url.href = await getHtmlUrl( - notification.subject.latest_comment_url, - notification.account.token, - ); - } else if (notification.subject.url) { - url.href = await getHtmlUrl( - notification.subject.url, - notification.account.token, - ); - } else { - // Perform any specific notification type handling (only required for a few special notification scenarios) - switch (notification.subject.type) { - case 'CheckSuite': - url.href = getCheckSuiteUrl(notification); - break; - case 'Discussion': - url.href = await getDiscussionUrl(notification); - break; - case 'RepositoryInvitation': - url.pathname += '/invitations'; - break; - case 'RepositoryDependabotAlertsThread': - url.pathname += '/security/dependabot'; - break; - case 'WorkflowRun': - url.href = getWorkflowRunUrl(notification); - break; - default: - break; - } + // Perform any specific notification type handling (only required for a few special notification scenarios) + switch (notification.subject.type) { + case 'CheckSuite': + url.href = getCheckSuiteUrl(notification); + break; + case 'Discussion': + url.href = await getDiscussionUrl(notification); + break; + case 'RepositoryInvitation': + url.pathname += '/invitations'; + break; + case 'RepositoryDependabotAlertsThread': + url.pathname += '/security/dependabot'; + break; + case 'WorkflowRun': + url.href = getWorkflowRunUrl(notification); + break; + default: + if (notification.subject.latest_comment_url) { + url.href = await getHtmlUrl( + notification.subject.latest_comment_url, + notification.account.token, + ); + } else if (notification.subject.url) { + url.href = await getHtmlUrl( + notification.subject.url, + notification.account.token, + ); + } else { + url.href = defaultGitHubWebUrl(url, notification.subject.type); + } } } catch (err) { rendererLogError( @@ -156,6 +146,8 @@ export async function generateGitHubWebUrl( err, notification, ); + + url.href = defaultGitHubWebUrl(url, notification.subject.type); } url.searchParams.set( @@ -166,6 +158,24 @@ export async function generateGitHubWebUrl( return url.toString() as Link; } +export function defaultGitHubWebUrl(url: URL, type: SubjectType) { + switch (type) { + case 'Issue': + url.pathname += '/issues'; + break; + case 'Discussion': + url.pathname += '/discussions'; + break; + case 'PullRequest': + url.pathname += '/pulls'; + break; + default: + break; + } + + return url.href; +} + export function getChevronDetails( hasNotifications: boolean, isVisible: boolean, From ba81d1e2cf9d6bdf5494d99b25fc9b1c197d9ca0 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sat, 13 Dec 2025 08:06:16 +1000 Subject: [PATCH 2/5] feat(url): improve fallback url handling Signed-off-by: Adam Setch --- .../utils/api/__mocks__/response-mocks.ts | 2 +- src/renderer/utils/helpers.test.ts | 120 +++++------------- 2 files changed, 33 insertions(+), 89 deletions(-) diff --git a/src/renderer/utils/api/__mocks__/response-mocks.ts b/src/renderer/utils/api/__mocks__/response-mocks.ts index d1db2fb7b..7141171d7 100644 --- a/src/renderer/utils/api/__mocks__/response-mocks.ts +++ b/src/renderer/utils/api/__mocks__/response-mocks.ts @@ -418,7 +418,7 @@ export const mockDiscussionLabels: DiscussionLabels = { ], }; -export const mockGraphQLResponse: GraphQLSearch = { +export const mockSearchDiscussionsGraphQLResponse: GraphQLSearch = { data: { search: { nodes: [ diff --git a/src/renderer/utils/helpers.test.ts b/src/renderer/utils/helpers.test.ts index d04d4ceaa..db927678e 100644 --- a/src/renderer/utils/helpers.test.ts +++ b/src/renderer/utils/helpers.test.ts @@ -4,17 +4,15 @@ import { ChevronRightIcon, } from '@primer/octicons-react'; -import type { AxiosResponse } from 'axios'; - -import { mockPersonalAccessTokenAccount } from '../__mocks__/account-mocks'; import type { Hostname, Link } from '../types'; import type { SubjectType } from '../typesGitHub'; import * as logger from '../utils/logger'; import { - mockGraphQLResponse, + mockSearchDiscussionsGraphQLResponse, mockSingleNotification, } from './api/__mocks__/response-mocks'; -import * as apiRequests from './api/request'; +import * as apiClient from './api/client'; +import * as apiRequest from './api/request'; import { generateGitHubWebUrl, generateNotificationReferrerId, @@ -74,7 +72,10 @@ describe('renderer/utils/helpers.ts', () => { 'https://github.com/gitify-app/notifications-test/issues/785'; const mockNotificationReferrer = 'notification_referrer_id=MDE4Ok5vdGlmaWNhdGlvblRocmVhZDEzODY2MTA5NjoxMjM0NTY3ODk%3D'; - const apiRequestAuthSpy = jest.spyOn(apiRequests, 'apiRequestAuth'); + + const apiRequestAuthSpy = jest.spyOn(apiRequest, 'apiRequestAuth'); + const getHtmlUrlSpy = jest.spyOn(apiClient, 'getHtmlUrl'); + const getLatestDiscussionSpy = jest.spyOn(apiClient, 'getLatestDiscussion'); afterEach(() => { jest.clearAllMocks(); @@ -89,23 +90,14 @@ describe('renderer/utils/helpers.ts', () => { type: 'Issue' as SubjectType, }; - apiRequestAuthSpy.mockResolvedValue({ - data: { - html_url: mockHtmlUrl, - }, - } as AxiosResponse); + getHtmlUrlSpy.mockResolvedValue(mockHtmlUrl); const result = await generateGitHubWebUrl({ ...mockSingleNotification, subject: subject, }); - expect(apiRequestAuthSpy).toHaveBeenCalledTimes(1); - expect(apiRequestAuthSpy).toHaveBeenCalledWith( - subject.latest_comment_url, - 'GET', - mockPersonalAccessTokenAccount.token, - ); + expect(getHtmlUrlSpy).toHaveBeenCalledTimes(1); expect(result).toBe(`${mockHtmlUrl}?${mockNotificationReferrer}`); }); @@ -117,23 +109,14 @@ describe('renderer/utils/helpers.ts', () => { type: 'Issue' as SubjectType, }; - apiRequestAuthSpy.mockResolvedValue({ - data: { - html_url: mockHtmlUrl, - }, - } as AxiosResponse); + getHtmlUrlSpy.mockResolvedValue(mockHtmlUrl); const result = await generateGitHubWebUrl({ ...mockSingleNotification, subject: subject, }); - expect(apiRequestAuthSpy).toHaveBeenCalledTimes(1); - expect(apiRequestAuthSpy).toHaveBeenCalledWith( - subject.url, - 'GET', - mockPersonalAccessTokenAccount.token, - ); + expect(getHtmlUrlSpy).toHaveBeenCalledTimes(1); expect(result).toBe(`${mockHtmlUrl}?${mockNotificationReferrer}`); }); @@ -151,7 +134,7 @@ describe('renderer/utils/helpers.ts', () => { subject: subject, }); - expect(apiRequestAuthSpy).toHaveBeenCalledTimes(0); + expect(getHtmlUrlSpy).toHaveBeenCalledTimes(0); expect(result).toBe( `https://github.com/gitify-app/notifications-test/actions?query=workflow%3A%22Demo%22+is%3Asuccess+branch%3Amain&${mockNotificationReferrer}`, ); @@ -170,7 +153,7 @@ describe('renderer/utils/helpers.ts', () => { subject: subject, }); - expect(apiRequestAuthSpy).toHaveBeenCalledTimes(0); + expect(getHtmlUrlSpy).toHaveBeenCalledTimes(0); expect(result).toBe( `https://github.com/gitify-app/notifications-test/actions?query=workflow%3A%22Demo%22+is%3Afailure+branch%3Amain&${mockNotificationReferrer}`, ); @@ -189,7 +172,7 @@ describe('renderer/utils/helpers.ts', () => { subject: subject, }); - expect(apiRequestAuthSpy).toHaveBeenCalledTimes(0); + expect(getHtmlUrlSpy).toHaveBeenCalledTimes(0); expect(result).toBe( `https://github.com/gitify-app/notifications-test/actions?query=workflow%3A%22Demo%22+is%3Afailure+branch%3Amain&${mockNotificationReferrer}`, ); @@ -208,7 +191,7 @@ describe('renderer/utils/helpers.ts', () => { subject: subject, }); - expect(apiRequestAuthSpy).toHaveBeenCalledTimes(0); + expect(getHtmlUrlSpy).toHaveBeenCalledTimes(0); expect(result).toBe( `https://github.com/gitify-app/notifications-test/actions?query=workflow%3A%22Demo%22+is%3Askipped+branch%3Amain&${mockNotificationReferrer}`, ); @@ -227,7 +210,7 @@ describe('renderer/utils/helpers.ts', () => { subject: subject, }); - expect(apiRequestAuthSpy).toHaveBeenCalledTimes(0); + expect(getHtmlUrlSpy).toHaveBeenCalledTimes(0); expect(result).toBe( `https://github.com/gitify-app/notifications-test/actions?${mockNotificationReferrer}`, ); @@ -246,7 +229,7 @@ describe('renderer/utils/helpers.ts', () => { subject: subject, }); - expect(apiRequestAuthSpy).toHaveBeenCalledTimes(0); + expect(getHtmlUrlSpy).toHaveBeenCalledTimes(0); expect(result).toBe( `https://github.com/gitify-app/notifications-test/actions?query=workflow%3A%22Demo%22+branch%3Amain&${mockNotificationReferrer}`, ); @@ -265,7 +248,7 @@ describe('renderer/utils/helpers.ts', () => { subject: subject, }); - expect(apiRequestAuthSpy).toHaveBeenCalledTimes(0); + expect(getHtmlUrlSpy).toHaveBeenCalledTimes(0); expect(result).toBe( `https://github.com/gitify-app/notifications-test/actions?${mockNotificationReferrer}`, ); @@ -281,56 +264,19 @@ describe('renderer/utils/helpers.ts', () => { type: 'Discussion' as SubjectType, }; - apiRequestAuthSpy.mockResolvedValue({ - data: { data: { search: { nodes: [] } } }, - } as AxiosResponse); + getLatestDiscussionSpy.mockResolvedValue(null); const result = await generateGitHubWebUrl({ ...mockSingleNotification, subject: subject, }); - expect(apiRequestAuthSpy).toHaveBeenCalledTimes(1); + expect(getLatestDiscussionSpy).toHaveBeenCalledTimes(1); expect(result).toBe( `${mockSingleNotification.repository.html_url}/discussions?${mockNotificationReferrer}`, ); }); - it('when error fetching discussion via graphql api, default to base repository discussion url', async () => { - const rendererLogErrorSpy = jest - .spyOn(logger, 'rendererLogError') - .mockImplementation(); - - const subject = { - title: '1.16.0', - url: null, - latest_comment_url: null, - type: 'Discussion' as SubjectType, - }; - - apiRequestAuthSpy.mockResolvedValue({ - data: { - data: null, - errors: [ - { - message: 'Something failed', - }, - ], - }, - } as AxiosResponse); - - const result = await generateGitHubWebUrl({ - ...mockSingleNotification, - subject: subject, - }); - - expect(apiRequestAuthSpy).toHaveBeenCalledTimes(1); - expect(result).toBe( - `https://github.com/gitify-app/notifications-test/discussions?${mockNotificationReferrer}`, - ); - expect(rendererLogErrorSpy).toHaveBeenCalledTimes(1); - }); - it('when discussion found via graphql api, link to matching discussion and comment hash', async () => { const subject = { title: '1.16.0', @@ -339,18 +285,16 @@ describe('renderer/utils/helpers.ts', () => { type: 'Discussion' as SubjectType, }; - apiRequestAuthSpy.mockResolvedValue({ - data: { - ...mockGraphQLResponse, - }, - } as AxiosResponse); + getLatestDiscussionSpy.mockResolvedValue( + mockSearchDiscussionsGraphQLResponse[0], + ); const result = await generateGitHubWebUrl({ ...mockSingleNotification, subject: subject, }); - expect(apiRequestAuthSpy).toHaveBeenCalledTimes(1); + expect(getLatestDiscussionSpy).toHaveBeenCalledTimes(1); expect(result).toBe( `https://github.com/gitify-app/notifications-test/discussions/612?${mockNotificationReferrer}#discussioncomment-2300902`, ); @@ -371,7 +315,7 @@ describe('renderer/utils/helpers.ts', () => { subject: subject, }); - expect(apiRequestAuthSpy).toHaveBeenCalledTimes(0); + expect(getHtmlUrlSpy).toHaveBeenCalledTimes(0); expect(result).toBe( `https://github.com/gitify-app/notifications-test/invitations?${mockNotificationReferrer}`, ); @@ -390,7 +334,7 @@ describe('renderer/utils/helpers.ts', () => { subject: subject, }); - expect(apiRequestAuthSpy).toHaveBeenCalledTimes(0); + expect(getHtmlUrlSpy).toHaveBeenCalledTimes(0); expect(result).toBe( `https://github.com/gitify-app/notifications-test/security/dependabot?${mockNotificationReferrer}`, ); @@ -410,7 +354,7 @@ describe('renderer/utils/helpers.ts', () => { subject: subject, }); - expect(apiRequestAuthSpy).toHaveBeenCalledTimes(0); + expect(getHtmlUrlSpy).toHaveBeenCalledTimes(0); expect(result).toBe( `https://github.com/gitify-app/notifications-test/actions?query=is%3Awaiting&${mockNotificationReferrer}`, ); @@ -430,7 +374,7 @@ describe('renderer/utils/helpers.ts', () => { subject: subject, }); - expect(apiRequestAuthSpy).toHaveBeenCalledTimes(0); + expect(getHtmlUrlSpy).toHaveBeenCalledTimes(0); expect(result).toBe( `https://github.com/gitify-app/notifications-test/actions?${mockNotificationReferrer}`, ); @@ -449,7 +393,7 @@ describe('renderer/utils/helpers.ts', () => { subject: subject, }); - expect(apiRequestAuthSpy).toHaveBeenCalledTimes(0); + expect(getHtmlUrlSpy).toHaveBeenCalledTimes(0); expect(result).toBe( `https://github.com/gitify-app/notifications-test/actions?${mockNotificationReferrer}`, ); @@ -470,7 +414,7 @@ describe('renderer/utils/helpers.ts', () => { subject: subject, }); - expect(apiRequestAuthSpy).toHaveBeenCalledTimes(0); + expect(getHtmlUrlSpy).toHaveBeenCalledTimes(0); expect(result).toBe( `https://github.com/gitify-app/notifications-test/issues?${mockNotificationReferrer}`, ); @@ -489,7 +433,7 @@ describe('renderer/utils/helpers.ts', () => { subject: subject, }); - expect(apiRequestAuthSpy).toHaveBeenCalledTimes(0); + expect(getHtmlUrlSpy).toHaveBeenCalledTimes(0); expect(result).toBe( `https://github.com/gitify-app/notifications-test/pulls?${mockNotificationReferrer}`, ); @@ -508,7 +452,7 @@ describe('renderer/utils/helpers.ts', () => { subject: subject, }); - expect(apiRequestAuthSpy).toHaveBeenCalledTimes(0); + expect(getHtmlUrlSpy).toHaveBeenCalledTimes(0); expect(result).toBe( `https://github.com/gitify-app/notifications-test?${mockNotificationReferrer}`, ); From c6422c4e39b4692a031d6dc3fab0425baf477601 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sat, 13 Dec 2025 08:09:39 +1000 Subject: [PATCH 3/5] feat(url): improve fallback url handling Signed-off-by: Adam Setch --- src/renderer/utils/helpers.test.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/renderer/utils/helpers.test.ts b/src/renderer/utils/helpers.test.ts index db927678e..272c6afb2 100644 --- a/src/renderer/utils/helpers.test.ts +++ b/src/renderer/utils/helpers.test.ts @@ -12,7 +12,6 @@ import { mockSingleNotification, } from './api/__mocks__/response-mocks'; import * as apiClient from './api/client'; -import * as apiRequest from './api/request'; import { generateGitHubWebUrl, generateNotificationReferrerId, @@ -73,7 +72,6 @@ describe('renderer/utils/helpers.ts', () => { const mockNotificationReferrer = 'notification_referrer_id=MDE4Ok5vdGlmaWNhdGlvblRocmVhZDEzODY2MTA5NjoxMjM0NTY3ODk%3D'; - const apiRequestAuthSpy = jest.spyOn(apiRequest, 'apiRequestAuth'); const getHtmlUrlSpy = jest.spyOn(apiClient, 'getHtmlUrl'); const getLatestDiscussionSpy = jest.spyOn(apiClient, 'getLatestDiscussion'); @@ -465,21 +463,21 @@ describe('renderer/utils/helpers.ts', () => { const subject = { title: 'generate github web url unit tests', - url: 'https://api.github.com/repos/gitify-app/notifications-test/discussion/1' as Link, + url: 'https://api.github.com/repos/gitify-app/notifications-test/issues/1' as Link, latest_comment_url: null as Link, - type: 'Discussion' as SubjectType, + type: 'Issue' as SubjectType, }; - apiRequestAuthSpy.mockRejectedValue(new Error('Test error')); + getHtmlUrlSpy.mockRejectedValue(new Error('Test error')); const result = await generateGitHubWebUrl({ ...mockSingleNotification, subject: subject, }); - expect(apiRequestAuthSpy).toHaveBeenCalledTimes(1); + expect(getHtmlUrlSpy).toHaveBeenCalledTimes(0); expect(result).toBe( - `https://github.com/gitify-app/notifications-test/discussions?${mockNotificationReferrer}`, + `https://github.com/gitify-app/notifications-test/issues?${mockNotificationReferrer}`, ); expect(rendererLogErrorSpy).toHaveBeenCalledTimes(1); }); From c273335340c9f7f0a941f3e692422477c61517bc Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sat, 13 Dec 2025 08:14:25 +1000 Subject: [PATCH 4/5] feat(url): improve fallback url handling Signed-off-by: Adam Setch --- .../utils/api/__mocks__/response-mocks.ts | 35 +++++++++---------- src/renderer/utils/api/client.ts | 2 ++ src/renderer/utils/helpers.test.ts | 8 ++--- 3 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/renderer/utils/api/__mocks__/response-mocks.ts b/src/renderer/utils/api/__mocks__/response-mocks.ts index 7141171d7..8130de5a3 100644 --- a/src/renderer/utils/api/__mocks__/response-mocks.ts +++ b/src/renderer/utils/api/__mocks__/response-mocks.ts @@ -418,27 +418,26 @@ export const mockDiscussionLabels: DiscussionLabels = { ], }; +export const mockDiscussion: Discussion = { + number: 123, + title: '1.16.0', + isAnswered: false, + stateReason: 'OPEN', + url: 'https://github.com/gitify-app/notifications-test/discussions/612' as Link, + author: { + login: 'discussion-creator', + url: 'https://github.com/discussion-creator' as Link, + avatar_url: 'https://avatars.githubusercontent.com/u/123456789?v=4' as Link, + type: 'User', + }, + comments: mockDiscussionComments, + labels: mockDiscussionLabels, +}; + export const mockSearchDiscussionsGraphQLResponse: GraphQLSearch = { data: { search: { - nodes: [ - { - number: 123, - title: '1.16.0', - isAnswered: false, - stateReason: 'OPEN', - url: 'https://github.com/gitify-app/notifications-test/discussions/612' as Link, - author: { - login: 'discussion-creator', - url: 'https://github.com/discussion-creator' as Link, - avatar_url: - 'https://avatars.githubusercontent.com/u/123456789?v=4' as Link, - type: 'User', - }, - comments: mockDiscussionComments, - labels: mockDiscussionLabels, - }, - ], + nodes: [mockDiscussion], }, }, }; diff --git a/src/renderer/utils/api/client.ts b/src/renderer/utils/api/client.ts index 0fdfe4920..54985a1ca 100644 --- a/src/renderer/utils/api/client.ts +++ b/src/renderer/utils/api/client.ts @@ -284,4 +284,6 @@ export async function getLatestDiscussion( notification, ); } + + return null; } diff --git a/src/renderer/utils/helpers.test.ts b/src/renderer/utils/helpers.test.ts index 272c6afb2..b065569c5 100644 --- a/src/renderer/utils/helpers.test.ts +++ b/src/renderer/utils/helpers.test.ts @@ -8,7 +8,7 @@ import type { Hostname, Link } from '../types'; import type { SubjectType } from '../typesGitHub'; import * as logger from '../utils/logger'; import { - mockSearchDiscussionsGraphQLResponse, + mockDiscussion, mockSingleNotification, } from './api/__mocks__/response-mocks'; import * as apiClient from './api/client'; @@ -283,9 +283,7 @@ describe('renderer/utils/helpers.ts', () => { type: 'Discussion' as SubjectType, }; - getLatestDiscussionSpy.mockResolvedValue( - mockSearchDiscussionsGraphQLResponse[0], - ); + getLatestDiscussionSpy.mockResolvedValue(mockDiscussion); const result = await generateGitHubWebUrl({ ...mockSingleNotification, @@ -475,7 +473,7 @@ describe('renderer/utils/helpers.ts', () => { subject: subject, }); - expect(getHtmlUrlSpy).toHaveBeenCalledTimes(0); + expect(getHtmlUrlSpy).toHaveBeenCalledTimes(1); expect(result).toBe( `https://github.com/gitify-app/notifications-test/issues?${mockNotificationReferrer}`, ); From 6729fa965a16077a62b977d90c00abe7ec6d59d9 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sat, 13 Dec 2025 08:40:08 +1000 Subject: [PATCH 5/5] feat(url): improve fallback url handling Signed-off-by: Adam Setch --- src/renderer/utils/helpers.test.ts | 35 +++++++++++++++++++++++++++++- src/renderer/utils/helpers.ts | 12 +++++----- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/src/renderer/utils/helpers.test.ts b/src/renderer/utils/helpers.test.ts index b065569c5..91206608d 100644 --- a/src/renderer/utils/helpers.test.ts +++ b/src/renderer/utils/helpers.test.ts @@ -16,6 +16,7 @@ import { generateGitHubWebUrl, generateNotificationReferrerId, getChevronDetails, + getDefaultURLForType, getPlatformFromHostname, isEnterpriseServerHost, } from './helpers'; @@ -416,7 +417,7 @@ describe('renderer/utils/helpers.ts', () => { ); }); - it('issues - defaults when no urls present in notification', async () => { + it('pull requests - defaults when no urls present in notification', async () => { const subject = { title: 'generate github web url unit tests', url: null, @@ -482,6 +483,38 @@ describe('renderer/utils/helpers.ts', () => { }); }); + describe('getDefaultURLForType', () => { + let mockUrl: URL; + + beforeEach(() => { + mockUrl = new URL('https://github.com/gitify-app/notifications-test'); + }); + + it('discussions', () => { + expect(getDefaultURLForType(mockUrl, 'Discussion')).toEqual( + 'https://github.com/gitify-app/notifications-test/discussions', + ); + }); + + it('issues', () => { + expect(getDefaultURLForType(mockUrl, 'Issue')).toEqual( + 'https://github.com/gitify-app/notifications-test/issues', + ); + }); + + it('pull requests', () => { + expect(getDefaultURLForType(mockUrl, 'PullRequest')).toEqual( + 'https://github.com/gitify-app/notifications-test/pulls', + ); + }); + + it('default', () => { + expect(getDefaultURLForType(mockUrl, 'RepositoryInvitation')).toEqual( + 'https://github.com/gitify-app/notifications-test', + ); + }); + }); + describe('getChevronDetails', () => { it('should return correct chevron details', () => { expect(getChevronDetails(true, true, 'account')).toEqual({ diff --git a/src/renderer/utils/helpers.ts b/src/renderer/utils/helpers.ts index 553657248..9071901d4 100644 --- a/src/renderer/utils/helpers.ts +++ b/src/renderer/utils/helpers.ts @@ -136,7 +136,7 @@ export async function generateGitHubWebUrl( notification.account.token, ); } else { - url.href = defaultGitHubWebUrl(url, notification.subject.type); + url.href = getDefaultURLForType(url, notification.subject.type); } } } catch (err) { @@ -147,7 +147,7 @@ export async function generateGitHubWebUrl( notification, ); - url.href = defaultGitHubWebUrl(url, notification.subject.type); + url.href = getDefaultURLForType(url, notification.subject.type); } url.searchParams.set( @@ -158,14 +158,14 @@ export async function generateGitHubWebUrl( return url.toString() as Link; } -export function defaultGitHubWebUrl(url: URL, type: SubjectType) { +export function getDefaultURLForType(url: URL, type: SubjectType) { switch (type) { - case 'Issue': - url.pathname += '/issues'; - break; case 'Discussion': url.pathname += '/discussions'; break; + case 'Issue': + url.pathname += '/issues'; + break; case 'PullRequest': url.pathname += '/pulls'; break;