From cc9d0dd842dc6f11182fcc02e837cf3290a4f5b2 Mon Sep 17 00:00:00 2001 From: Afonso Jorge Ramos Date: Tue, 12 May 2026 02:27:36 +0100 Subject: [PATCH] refactor(notifications): generate notification URL via the adapter --- .../utils/forges/github/client.test.ts | 13 -------- src/renderer/utils/forges/github/client.ts | 8 ----- src/renderer/utils/forges/github/types.ts | 9 ------ src/renderer/utils/notifications/url.test.ts | 32 ++++++++----------- src/renderer/utils/notifications/url.ts | 21 ++++++------ src/renderer/utils/system/links.test.ts | 2 +- src/renderer/utils/system/links.ts | 4 +-- src/renderer/utils/system/native.test.ts | 6 ++-- src/renderer/utils/system/native.ts | 4 +-- 9 files changed, 31 insertions(+), 68 deletions(-) diff --git a/src/renderer/utils/forges/github/client.test.ts b/src/renderer/utils/forges/github/client.test.ts index 4560a6497..2b74f0f7b 100644 --- a/src/renderer/utils/forges/github/client.test.ts +++ b/src/renderer/utils/forges/github/client.test.ts @@ -18,7 +18,6 @@ import { fetchPullByNumber, getCommit, getCommitComment, - getHtmlUrl, getRelease, ignoreNotificationThreadSubscription, listNotificationsForAuthenticatedUser, @@ -237,18 +236,6 @@ describe('renderer/utils/forges/github/client.ts', () => { }); }); - it('getHtmlUrl - should return the HTML URL', async () => { - await getHtmlUrl( - mockGitHubCloudAccount, - 'https://api.github.com/repos/gitify-app/notifications-test/issues/785' as Link, - ); - - expect(createOctokitClientSpy).toHaveBeenCalledWith(mockGitHubCloudAccount, 'rest'); - expect(mockOctokit.request).toHaveBeenCalledWith('GET {+url}', { - url: 'https://api.github.com/repos/gitify-app/notifications-test/issues/785', - }); - }); - it('getCommit - should fetch commit details', async () => { const commitUrl = 'https://api.github.com/repos/gitify-app/gitify/commits/abc123' as Link; diff --git a/src/renderer/utils/forges/github/client.ts b/src/renderer/utils/forges/github/client.ts index f151b6c5e..546a01a7f 100644 --- a/src/renderer/utils/forges/github/client.ts +++ b/src/renderer/utils/forges/github/client.ts @@ -5,7 +5,6 @@ import type { GetCommitCommentResponse, GetCommitResponse, GetReleaseResponse, - GitHubHtmlUrlResponse, IgnoreNotificationThreadSubscriptionResponse, ListNotificationsForAuthenticatedUserResponse, MarkNotificationThreadAsDoneResponse, @@ -168,13 +167,6 @@ export async function getRelease(account: Account, url: Link): Promise(account, url); } -/** - * Get the `html_url` from the GitHub response - */ -export async function getHtmlUrl(account: Account, url: Link): Promise { - return followUrl(account, url); -} - /** * Follow GitHub Response URL */ diff --git a/src/renderer/utils/forges/github/types.ts b/src/renderer/utils/forges/github/types.ts index fda64ad0f..8ff95f22f 100644 --- a/src/renderer/utils/forges/github/types.ts +++ b/src/renderer/utils/forges/github/types.ts @@ -24,15 +24,6 @@ export type RawGitHubNotification = Endpoints['GET /notifications']['response'][ export type RawUser = components['schemas']['simple-user']; -/** - * Minimal response for endpoints where we're only interested in the `html_url`. - * - * Used when following a notification thread's subject URL or latest comment URL. - */ -export type GitHubHtmlUrlResponse = { - html_url: string; -}; - /** * These API endpoints don't return a response body: * - Endpoints['PATCH /notifications/threads/{thread_id}']['response']['data'] diff --git a/src/renderer/utils/notifications/url.test.ts b/src/renderer/utils/notifications/url.test.ts index be010f122..fe16a616f 100644 --- a/src/renderer/utils/notifications/url.test.ts +++ b/src/renderer/utils/notifications/url.test.ts @@ -3,8 +3,8 @@ import { mockGitifyNotification } from '../../__mocks__/notifications-mocks'; import type { GitifySubject, Link, SubjectType } from '../../types'; -import * as apiClient from '../forges/github/client'; -import { generateGitHubWebUrl, generateNotificationReferrerId } from './url'; +import { githubAdapter } from '../forges/github/adapter'; +import { generateNotificationReferrerId, generateNotificationWebUrl } from './url'; describe('renderer/utils/notifications/url.ts', () => { describe('generateNotificationReferrerId', () => { @@ -14,12 +14,12 @@ describe('renderer/utils/notifications/url.ts', () => { }); }); - describe('generateGitHubWebUrl', () => { + describe('generateNotificationWebUrl', () => { const mockHtmlUrl = 'https://github.com/gitify-app/notifications-test/issues/785' as Link; const mockNotificationReferrer = 'notification_referrer_id=MDE4Ok5vdGlmaWNhdGlvblRocmVhZDEzODY2MTA5NjoxMjM0NTY3ODk%3D'; - const getHtmlUrlSpy = vi.spyOn(apiClient, 'getHtmlUrl'); + const followUrlSpy = vi.spyOn(githubAdapter, 'followUrl'); it('Subject HTML URL: prefer if available from enrichment stage', async () => { const mockSubjectHtmlUrl = 'https://gitify.io/' as Link; @@ -36,12 +36,12 @@ describe('renderer/utils/notifications/url.ts', () => { htmlUrl: mockSubjectHtmlUrl, } as unknown as GitifySubject; - const result = await generateGitHubWebUrl({ + const result = await generateNotificationWebUrl({ ...mockGitifyNotification, subject: subject, }); - expect(getHtmlUrlSpy).toHaveBeenCalledTimes(0); + expect(followUrlSpy).toHaveBeenCalledTimes(0); expect(result).toBe(`${mockSubjectHtmlUrl}?${mockNotificationReferrer}`); }); @@ -60,17 +60,15 @@ describe('renderer/utils/notifications/url.ts', () => { htmlUrl: mockSubjectHtmlUrl, } as unknown as GitifySubject; - getHtmlUrlSpy.mockResolvedValue({ - html_url: mockHtmlUrl, - }); + followUrlSpy.mockResolvedValue({ html_url: mockHtmlUrl }); - const result = await generateGitHubWebUrl({ + const result = await generateNotificationWebUrl({ ...mockGitifyNotification, subject: subject, }); - expect(getHtmlUrlSpy).toHaveBeenCalledTimes(1); - expect(getHtmlUrlSpy).toHaveBeenCalledWith(mockGitHubCloudAccount, mockLatestCommentUrl); + expect(followUrlSpy).toHaveBeenCalledTimes(1); + expect(followUrlSpy).toHaveBeenCalledWith(mockGitHubCloudAccount, mockLatestCommentUrl); expect(result).toBe(`${mockHtmlUrl}?${mockNotificationReferrer}`); }); @@ -88,17 +86,15 @@ describe('renderer/utils/notifications/url.ts', () => { htmlUrl: mockSubjectHtmlUrl, } as unknown as GitifySubject; - getHtmlUrlSpy.mockResolvedValue({ - html_url: mockHtmlUrl, - }); + followUrlSpy.mockResolvedValue({ html_url: mockHtmlUrl }); - const result = await generateGitHubWebUrl({ + const result = await generateNotificationWebUrl({ ...mockGitifyNotification, subject: subject, }); - expect(getHtmlUrlSpy).toHaveBeenCalledTimes(1); - expect(getHtmlUrlSpy).toHaveBeenCalledWith(mockGitHubCloudAccount, mockSubjectUrl); + expect(followUrlSpy).toHaveBeenCalledTimes(1); + expect(followUrlSpy).toHaveBeenCalledWith(mockGitHubCloudAccount, mockSubjectUrl); expect(result).toBe(`${mockHtmlUrl}?${mockNotificationReferrer}`); }); }); diff --git a/src/renderer/utils/notifications/url.ts b/src/renderer/utils/notifications/url.ts index 993d1509e..2b8c9d107 100644 --- a/src/renderer/utils/notifications/url.ts +++ b/src/renderer/utils/notifications/url.ts @@ -1,7 +1,6 @@ import type { GitifyNotification, Link } from '../../types'; import { rendererLogError, toError } from '../core/logger'; -import { getHtmlUrl } from '../forges/github/client'; import { getAdapter } from '../forges/registry'; export function generateNotificationReferrerId(notification: GitifyNotification): string { @@ -9,28 +8,26 @@ export function generateNotificationReferrerId(notification: GitifyNotification) return btoa(raw); } -export async function generateGitHubWebUrl(notification: GitifyNotification): Promise { - const url = new URL(getAdapter(notification.account).getDisplayHelpers(notification).defaultUrl); +export async function generateNotificationWebUrl(notification: GitifyNotification): Promise { + const adapter = getAdapter(notification.account); + const url = new URL(adapter.getDisplayHelpers(notification).defaultUrl); if (notification.subject.htmlUrl) { url.href = notification.subject.htmlUrl; } else { try { - if (notification.subject.latestCommentUrl) { - const response = await getHtmlUrl( + const followTarget = + notification.subject.latestCommentUrl ?? notification.subject.url ?? null; + if (followTarget) { + const response = await adapter.followUrl<{ html_url: string }>( notification.account, - notification.subject.latestCommentUrl, + followTarget, ); - - url.href = response.html_url; - } else if (notification.subject.url) { - const response = await getHtmlUrl(notification.account, notification.subject.url); - url.href = response.html_url; } } catch (err) { rendererLogError( - 'generateGitHubWebUrl', + 'generateNotificationWebUrl', 'Failed to resolve specific notification html url for', toError(err), notification, diff --git a/src/renderer/utils/system/links.test.ts b/src/renderer/utils/system/links.test.ts index 074fb7ab4..5b8bde89f 100644 --- a/src/renderer/utils/system/links.test.ts +++ b/src/renderer/utils/system/links.test.ts @@ -94,7 +94,7 @@ describe('renderer/utils/links.ts', () => { it('openNotification', async () => { const mockNotificationUrl = mockGitifyNotification.repository.htmlUrl; - vi.spyOn(url, 'generateGitHubWebUrl').mockResolvedValue(mockNotificationUrl); + vi.spyOn(url, 'generateNotificationWebUrl').mockResolvedValue(mockNotificationUrl); await openNotification(mockGitifyNotification); diff --git a/src/renderer/utils/system/links.ts b/src/renderer/utils/system/links.ts index 81258f1ca..723c11941 100644 --- a/src/renderer/utils/system/links.ts +++ b/src/renderer/utils/system/links.ts @@ -12,7 +12,7 @@ import type { } from '../../types'; import { getAdapter } from '../forges/registry'; -import { generateGitHubWebUrl } from '../notifications/url'; +import { generateNotificationWebUrl } from '../notifications/url'; import { openExternalLink } from './comms'; export function openGitifyReleaseNotes(version: string) { @@ -63,7 +63,7 @@ export function openRepository(repository: GitifyRepository) { } export async function openNotification(notification: GitifyNotification) { - const url = await generateGitHubWebUrl(notification); + const url = await generateNotificationWebUrl(notification); openExternalLink(url); } diff --git a/src/renderer/utils/system/native.test.ts b/src/renderer/utils/system/native.test.ts index f6a69bac5..99dfd11fc 100644 --- a/src/renderer/utils/system/native.test.ts +++ b/src/renderer/utils/system/native.test.ts @@ -11,7 +11,7 @@ import * as native from './native'; describe('renderer/utils/system/native.ts', () => { const mockHtmlUrl = mockSingleAccountNotifications[0].notifications[0].repository.htmlUrl; - vi.spyOn(url, 'generateGitHubWebUrl').mockImplementation(async () => mockHtmlUrl); + vi.spyOn(url, 'generateNotificationWebUrl').mockImplementation(async () => mockHtmlUrl); it('should raise a native notification for a single new notification', async () => { native.raiseNativeNotification(mockSingleAccountNotifications[0].notifications); @@ -25,7 +25,7 @@ describe('renderer/utils/system/native.ts', () => { expect.stringContaining(mockSingleAccountNotifications[0].notifications[0].subject.title), expect.stringContaining(mockHtmlUrl), ); - expect(url.generateGitHubWebUrl).toHaveBeenCalledTimes(1); + expect(url.generateNotificationWebUrl).toHaveBeenCalledTimes(1); }); it('should raise a native notification for multiple new notifications', async () => { @@ -38,6 +38,6 @@ describe('renderer/utils/system/native.ts', () => { 'You have 2 notifications', undefined, ); - expect(url.generateGitHubWebUrl).toHaveBeenCalledTimes(0); + expect(url.generateNotificationWebUrl).toHaveBeenCalledTimes(0); }); }); diff --git a/src/renderer/utils/system/native.ts b/src/renderer/utils/system/native.ts index 03decaaea..626622674 100644 --- a/src/renderer/utils/system/native.ts +++ b/src/renderer/utils/system/native.ts @@ -2,7 +2,7 @@ import { APPLICATION } from '../../../shared/constants'; import type { GitifyNotification } from '../../types'; -import { generateGitHubWebUrl } from '../notifications/url'; +import { generateNotificationWebUrl } from '../notifications/url'; /** * Raises a native OS notification. @@ -22,7 +22,7 @@ export async function raiseNativeNotification(notifications: GitifyNotification[ const notification = notifications[0]; title = window.gitify.platform.isWindows() ? '' : notification.repository.fullName; body = notification.subject.title; - url = await generateGitHubWebUrl(notification); + url = await generateNotificationWebUrl(notification); } else { title = APPLICATION.NAME; body = `You have ${notifications.length} notifications`;