diff --git a/src/__mocks__/mock-state.ts b/src/__mocks__/mock-state.ts index 310581a67..ac5072e48 100644 --- a/src/__mocks__/mock-state.ts +++ b/src/__mocks__/mock-state.ts @@ -1,14 +1,9 @@ import { Appearance, AuthState, SettingsState } from '../types'; -import { mockedUser } from './mockedData'; +import { mockedUser, mockedEnterpriseAccounts } from './mockedData'; export const mockAccounts: AuthState = { token: 'token-123-456', - enterpriseAccounts: [ - { - token: 'token-gitify-123-456', - hostname: 'github.gitify.io', - }, - ], + enterpriseAccounts: mockedEnterpriseAccounts, user: mockedUser, }; diff --git a/src/__mocks__/mockedData.ts b/src/__mocks__/mockedData.ts index 7ce0f56b2..228ae1558 100644 --- a/src/__mocks__/mockedData.ts +++ b/src/__mocks__/mockedData.ts @@ -383,6 +383,108 @@ export const mockedGraphQLResponse: GraphQLSearch = { }, }, }, + { + node: { + viewerSubscription: 'IGNORED', + title: '1.16.0', + url: 'https://github.com/manosim/notifications-test/discussions/612', + comments: { + edges: [ + { + node: { + databaseId: 2215656, + createdAt: '2022-02-20T18:33:39Z', + replies: { + edges: [], + }, + }, + }, + { + node: { + databaseId: 2217789, + createdAt: '2022-02-21T03:30:42Z', + replies: { + edges: [], + }, + }, + }, + { + node: { + databaseId: 2223243, + createdAt: '2022-02-21T18:26:27Z', + replies: { + edges: [ + { + node: { + databaseId: 2232922, + createdAt: '2022-02-23T00:57:58Z', + }, + }, + ], + }, + }, + }, + { + node: { + databaseId: 2232921, + createdAt: '2022-02-23T00:57:49Z', + replies: { + edges: [], + }, + }, + }, + { + node: { + databaseId: 2258799, + createdAt: '2022-02-27T01:22:20Z', + replies: { + edges: [ + { + node: { + databaseId: 2300902, + createdAt: '2022-03-05T17:43:52Z', + }, + }, + ], + }, + }, + }, + { + node: { + databaseId: 2297637, + createdAt: '2022-03-04T20:39:44Z', + replies: { + edges: [ + { + node: { + databaseId: 2300893, + createdAt: '2022-03-05T17:41:04Z', + }, + }, + ], + }, + }, + }, + { + node: { + databaseId: 2299763, + createdAt: '2022-03-05T11:05:42Z', + replies: { + edges: [ + { + node: { + databaseId: 2300895, + createdAt: '2022-03-05T17:41:44Z', + }, + }, + ], + }, + }, + }, + ], + }, + }, + }, ], }, }, diff --git a/src/components/NotificationRow.test.tsx b/src/components/NotificationRow.test.tsx index 064261881..9158a62d6 100644 --- a/src/components/NotificationRow.test.tsx +++ b/src/components/NotificationRow.test.tsx @@ -2,7 +2,7 @@ import * as React from 'react'; import * as TestRenderer from 'react-test-renderer'; import { fireEvent, render } from '@testing-library/react'; -const { shell } = require('electron'); +import * as helpers from '../utils/helpers'; import { AppContext } from '../context/App'; import { mockedSingleNotification } from '../__mocks__/mockedData'; @@ -11,7 +11,7 @@ import { mockAccounts, mockSettings } from '../__mocks__/mock-state'; describe('components/Notification.js', () => { beforeEach(() => { - jest.spyOn(shell, 'openExternal'); + jest.spyOn(helpers, 'openInBrowser'); }); afterEach(() => { @@ -51,7 +51,7 @@ describe('components/Notification.js', () => { ); fireEvent.click(getByRole('main')); - expect(shell.openExternal).toHaveBeenCalledTimes(1); + expect(helpers.openInBrowser).toHaveBeenCalledTimes(1); expect(removeNotificationFromState).toHaveBeenCalledTimes(1); }); @@ -76,7 +76,7 @@ describe('components/Notification.js', () => { ); fireEvent.click(getByRole('main')); - expect(shell.openExternal).toHaveBeenCalledTimes(1); + expect(helpers.openInBrowser).toHaveBeenCalledTimes(1); expect(markNotificationDone).toHaveBeenCalledTimes(1); }); diff --git a/src/utils/helpers.test.ts b/src/utils/helpers.test.ts index 7ccfada94..bb0e6645d 100644 --- a/src/utils/helpers.test.ts +++ b/src/utils/helpers.test.ts @@ -1,29 +1,22 @@ import { - generateGitHubWebUrl, generateGitHubAPIUrl, generateNotificationReferrerId, - getCommentId, - getLatestDiscussionCommentId, isEnterpriseHost, addHours, formatSearchQueryString, + addNotificationReferrerIdToUrl, + getHtmlUrl, + generateGitHubWebUrl, } from './helpers'; import { + mockedGraphQLResponse, mockedSingleNotification, mockedUser, - mockedGraphQLResponse, } from '../__mocks__/mockedData'; - -const URL = { - normal: { - api: 'https://api.github.com/repos/myuser/notifications-test', - default: 'https://github.com/myuser/notifications-test', - }, - enterprise: { - api: 'https://github.gitify.io/api/v3/repos/myorg/notifications-test', - default: 'https://github.gitify.io/myorg/notifications-test', - }, -}; +import * as apiRequests from './api-requests'; +import { AxiosPromise, AxiosResponse } from 'axios'; +import { mockAccounts } from '../__mocks__/mock-state'; +import { SubjectType } from '../typesGithub'; describe('utils/helpers.ts', () => { describe('isEnterpriseHost', () => { @@ -38,104 +31,53 @@ describe('utils/helpers.ts', () => { }); }); - describe('generateNotificationReferrerId', () => { - it('should generate the notification_referrer_id', () => { - const referrerId = generateNotificationReferrerId( - mockedSingleNotification.id, - mockedUser.id, + describe('addNotificationReferrerIdToUrl', () => { + it('should add notification_referrer_id to the URL', () => { + // Mock data + const url = 'https://github.com/some/repo'; + const notificationId = '123'; + const userId = 456; + + const result = addNotificationReferrerIdToUrl( + url, + notificationId, + userId, ); - expect(referrerId).toBe( - 'notification_referrer_id=MDE4Ok5vdGlmaWNhdGlvblRocmVhZDEzODY2MTA5NjoxMjM0NTY3ODk=', + + expect(result).toEqual( + 'https://github.com/some/repo?notification_referrer_id=MDE4Ok5vdGlmaWNhdGlvblRocmVhZDEyMzo0NTY%3D', + ); + }); + + it('should add notification_referrer_id to the URL, preserving anchor tags', () => { + // Mock data + const url = + 'https://github.com/some/repo/pull/123#issuecomment-1951055051'; + const notificationId = '123'; + const userId = 456; + + const result = addNotificationReferrerIdToUrl( + url, + notificationId, + userId, + ); + + expect(result).toEqual( + 'https://github.com/some/repo/pull/123?notification_referrer_id=MDE4Ok5vdGlmaWNhdGlvblRocmVhZDEyMzo0NTY%3D#issuecomment-1951055051', ); }); }); - describe('generateGitHubWebUrl', () => { - let notificationReferrerId; - beforeAll(() => { - notificationReferrerId = generateNotificationReferrerId( + describe('generateNotificationReferrerId', () => { + it('should generate the notification_referrer_id', () => { + const referrerId = generateNotificationReferrerId( mockedSingleNotification.id, mockedUser.id, ); + expect(referrerId).toBe( + 'MDE4Ok5vdGlmaWNhdGlvblRocmVhZDEzODY2MTA5NjoxMjM0NTY3ODk=', + ); }); - - it('should generate the GitHub url - non enterprise - (issue)', () => - testGenerateUrl( - `${URL.normal.api}/issues/3`, - `${URL.normal.default}/issues/3?${notificationReferrerId}`, - )); - - it('should generate the GitHub url - non enterprise - (pull request)', () => - testGenerateUrl( - `${URL.normal.api}/pulls/123`, - `${URL.normal.default}/pull/123?${notificationReferrerId}`, - )); - - it('should generate the GitHub url - non enterprise - (release)', () => - testGenerateUrl( - `${URL.normal.api}/releases/3988077`, - `${URL.normal.default}/releases/3988077?${notificationReferrerId}`, - )); - - it('should generate the GitHub url - non enterprise - (discussion)', () => - testGenerateUrl( - `${URL.normal.api}/discussions/630`, - `${URL.normal.default}/discussions/630?${notificationReferrerId}`, - )); - - it('should generate the GitHub url - enterprise - (issue)', () => - testGenerateUrl( - `${URL.enterprise.api}/issues/123`, - `${URL.enterprise.default}/issues/123?${notificationReferrerId}`, - )); - - it('should generate the GitHub url - enterprise - (pull request)', () => - testGenerateUrl( - `${URL.enterprise.api}/pulls/3`, - `${URL.enterprise.default}/pull/3?${notificationReferrerId}`, - )); - - it('should generate the GitHub url - enterprise - (release)', () => - testGenerateUrl( - `${URL.enterprise.api}/releases/1`, - `${URL.enterprise.default}/releases/1?${notificationReferrerId}`, - )); - - it('should generate the GitHub url - enterprise - (discussion)', () => - testGenerateUrl( - `${URL.enterprise.api}/discussions/343`, - `${URL.enterprise.default}/discussions/343?${notificationReferrerId}`, - )); - - it('should generate the GitHub issue url with correct commentId', () => - testGenerateUrl( - `${URL.normal.api}/issues/5`, - `${URL.normal.default}/issues/5?${notificationReferrerId}#issuecomment-1059824632`, - '#issuecomment-' + - getCommentId(`${URL.normal.api}/issues/comments/1059824632`), - )); - - it('should generate the GitHub discussion url with correct commentId', () => - testGenerateUrl( - `${URL.normal.api}/discussions/75`, - `${URL.normal.default}/discussions/75?${notificationReferrerId}#discussioncomment-2300902`, - '#discussioncomment-' + - getLatestDiscussionCommentId( - mockedGraphQLResponse.data.data.search.edges[0].node.comments.edges, - ), - )); - - function testGenerateUrl(apiUrl, ExpectedResult, comment?) { - const notif = { ...mockedSingleNotification, subject: { url: apiUrl } }; - expect( - generateGitHubWebUrl( - notif.subject.url, - notif.id, - mockedUser.id, - comment, - ), - ).toBe(ExpectedResult); - } }); describe('generateGitHubAPIUrl', () => { @@ -176,4 +118,198 @@ describe('utils/helpers.ts', () => { ); }); }); + + describe('getHtmlUrl', () => { + let apiRequestAuthMock; + + beforeEach(() => { + apiRequestAuthMock = jest.spyOn(apiRequests, 'apiRequestAuth'); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should return the HTML URL', async () => { + const requestPromise = new Promise((resolve) => + resolve({ + data: { + html_url: 'https://github.com/gitify-app/gitify/issues/785', + }, + } as AxiosResponse), + ) as AxiosPromise; + + apiRequestAuthMock.mockResolvedValue(requestPromise); + + const result = await getHtmlUrl( + 'https://api.github.com/repos/gitify-app/gitify/issues/785', + '123', + ); + expect(result).toBe('https://github.com/gitify-app/gitify/issues/785'); + }); + }); + + describe('generateGitHubWebUrl', () => { + const mockedHtmlUrl = 'https://github.com/gitify-app/gitify/issues/785'; + const mockedNotificationReferrer = + '?notification_referrer_id=MDE4Ok5vdGlmaWNhdGlvblRocmVhZDEzODY2MTA5NjoxMjM0NTY3ODk%3D'; + const apiRequestAuthMock = jest.spyOn(apiRequests, 'apiRequestAuth'); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('Subject Latest Comment Url: when not null, fetch lastest comment html url', async () => { + const subject = { + title: 'generate github web url unit tests', + url: 'https://api.github.com/repos/gitify-app/notifications-test/issues/1', + latest_comment_url: + 'https://api.github.com/repos/gitify-app/notifications-test/issues/comments/302888448', + type: 'Issue' as SubjectType, + }; + + const requestPromise = new Promise((resolve) => + resolve({ + data: { + html_url: mockedHtmlUrl, + }, + } as AxiosResponse), + ) as AxiosPromise; + + apiRequestAuthMock.mockResolvedValue(requestPromise); + + const result = await generateGitHubWebUrl( + { + ...mockedSingleNotification, + subject: subject, + }, + mockAccounts, + ); + + expect(apiRequestAuthMock).toHaveBeenCalledTimes(1); + expect(apiRequestAuthMock).toHaveBeenCalledWith( + subject.latest_comment_url, + 'GET', + mockAccounts.token, + ); + expect(result).toBe(`${mockedHtmlUrl}${mockedNotificationReferrer}`); + }); + + it('Subject Url: when no latest comment url available, fetch subject html url', async () => { + const subject = { + title: 'generate github web url unit tests', + url: 'https://api.github.com/repos/gitify-app/notifications-test/issues/1', + latest_comment_url: null, + type: 'Issue' as SubjectType, + }; + + const requestPromise = new Promise((resolve) => + resolve({ + data: { + html_url: mockedHtmlUrl, + }, + } as AxiosResponse), + ) as AxiosPromise; + + apiRequestAuthMock.mockResolvedValue(requestPromise); + + const result = await generateGitHubWebUrl( + { + ...mockedSingleNotification, + subject: subject, + }, + mockAccounts, + ); + + expect(apiRequestAuthMock).toHaveBeenCalledTimes(1); + expect(apiRequestAuthMock).toHaveBeenCalledWith( + subject.url, + 'GET', + mockAccounts.token, + ); + expect(result).toBe(`${mockedHtmlUrl}${mockedNotificationReferrer}`); + }); + + it('Discussions: when no subject urls and no discussions found via query, default to linking to repository discussions', async () => { + const subject = { + title: 'generate github web url unit tests', + url: null, + latest_comment_url: null, + type: 'Discussion' as SubjectType, + }; + + const requestPromise = new Promise((resolve) => + resolve({ + data: {}, + } as AxiosResponse), + ) as AxiosPromise; + + apiRequestAuthMock.mockResolvedValue(requestPromise); + + const result = await generateGitHubWebUrl( + { + ...mockedSingleNotification, + subject: subject, + }, + mockAccounts, + ); + + expect(apiRequestAuthMock).toHaveBeenCalledTimes(1); + expect(result).toBe( + `${mockedSingleNotification.repository.html_url}/discussions${mockedNotificationReferrer}`, + ); + }); + + it('Discussions: when no subject urls and no discussions found via query, default to linking to repository discussions', async () => { + const subject = { + title: '1.16.0', + url: null, + latest_comment_url: null, + type: 'Discussion' as SubjectType, + }; + + // const latestDiscussionCommentId = 12345; + + const requestPromise = new Promise((resolve) => + resolve(mockedGraphQLResponse as AxiosResponse), + ) as AxiosPromise; + + apiRequestAuthMock.mockResolvedValue(requestPromise); + + const result = await generateGitHubWebUrl( + { + ...mockedSingleNotification, + subject: subject, + }, + mockAccounts, + ); + + expect(apiRequestAuthMock).toHaveBeenCalledTimes(1); + expect(result).toBe( + 'https://github.com/manosim/notifications-test/discussions/612?notification_referrer_id=MDE4Ok5vdGlmaWNhdGlvblRocmVhZDEzODY2MTA5NjoxMjM0NTY3ODk%3D#discussioncomment-2300902', + ); + }); + + it('defaults to repository url', async () => { + const subject = { + title: 'generate github web url unit tests', + url: null, + latest_comment_url: null, + type: 'Issue' as SubjectType, + }; + + const result = await generateGitHubWebUrl( + { + ...mockedSingleNotification, + subject: subject, + }, + mockAccounts, + ); + + expect(apiRequestAuthMock).toHaveBeenCalledTimes(0); + expect(result).toBe( + `${mockedSingleNotification.repository.html_url}${mockedNotificationReferrer}`, + ); + }); + }); }); diff --git a/src/utils/helpers.ts b/src/utils/helpers.ts index fb71af3de..a90e4b212 100644 --- a/src/utils/helpers.ts +++ b/src/utils/helpers.ts @@ -26,43 +26,29 @@ export function generateGitHubAPIUrl(hostname) { : `https://api.${hostname}/`; } -export function generateNotificationReferrerId( +export function addNotificationReferrerIdToUrl( + url: string, notificationId: string, userId: number, -) { - const buffer = Buffer.from( - `018:NotificationThread${notificationId}:${userId}`, +): string { + const parsedUrl = new URL(url); + + parsedUrl.searchParams.set( + 'notification_referrer_id', + generateNotificationReferrerId(notificationId, userId), ); - return `notification_referrer_id=${buffer.toString('base64')}`; + + return parsedUrl.href; } -export function generateGitHubWebUrl( - url: string, +export function generateNotificationReferrerId( notificationId: string, - userId?: number, - comment: string = '', -) { - const { hostname } = new URL(url); - const isEnterprise = isEnterpriseHost(hostname); - - let newUrl: string = isEnterprise - ? url.replace(`${hostname}/api/v3/repos`, hostname) - : url.replace('api.github.com/repos', 'github.com'); - - if (newUrl.indexOf('/pulls/') !== -1) { - newUrl = newUrl.replace('/pulls/', '/pull/'); - } - - if (userId) { - const notificationReferrerId = generateNotificationReferrerId( - notificationId, - userId, - ); - - return `${newUrl}?${notificationReferrerId}${comment}`; - } - - return newUrl + comment; + userId: number, +): string { + const buffer = Buffer.from( + `018:NotificationThread${notificationId}:${userId}`, + ); + return buffer.toString('base64'); } export function addHours(date: string, hours: number): string { @@ -77,21 +63,18 @@ export function formatSearchQueryString( return `${title} in:title repo:${repo} updated:>${addHours(lastUpdated, -2)}`; } -export async function getReleaseTagWebUrl( - notification: Notification, - token: string, -) { - const response = await apiRequestAuth(notification.subject.url, 'GET', token); +export async function getHtmlUrl(url: string, token: string) { + const response = await apiRequestAuth(url, 'GET', token); - return { - url: response.data.html_url, - }; + return response.data.html_url; } async function getDiscussionUrl( notification: Notification, token: string, -): Promise<{ url: string; latestCommentId: string | number }> { +): Promise { + let url = `${notification.repository.html_url}/discussions`; + const response: GraphQLSearch = await apiRequestAuth( `https://api.github.com/graphql`, 'POST', @@ -141,17 +124,19 @@ async function getDiscussionUrl( (edge) => edge.node.viewerSubscription === 'SUBSCRIBED', ); - let comments = edges[0]?.node.comments.edges; + if (edges[0]) { + url = edges[0].node.url; + + let comments = edges[0]?.node.comments.edges; - let latestCommentId: string | number; - if (comments?.length) { - latestCommentId = getLatestDiscussionCommentId(comments); + let latestCommentId: string | number; + if (comments?.length) { + latestCommentId = getLatestDiscussionCommentId(comments); + url += `#discussioncomment-${latestCommentId}`; + } } - return { - url: edges[0]?.node.url, - latestCommentId, - }; + return url; } export const getLatestDiscussionCommentId = ( @@ -163,49 +148,41 @@ export const getLatestDiscussionCommentId = ( .reduce((a, b) => (a.node.createdAt > b.node.createdAt ? a : b))?.node .databaseId; -export const getCommentId = (url: string) => - /comments\/(?\d+)/g.exec(url)?.groups?.id; - -export async function openInBrowser( +export async function generateGitHubWebUrl( notification: Notification, accounts: AuthState, -) { - if (notification.subject.type === 'Release') { - getReleaseTagWebUrl(notification, accounts.token).then(({ url }) => - openExternalLink( - generateGitHubWebUrl( - url, - notification.id, - accounts.user?.id, - undefined, - ), - ), - ); - } else if (notification.subject.type === 'Discussion') { - getDiscussionUrl(notification, accounts.token).then( - ({ url, latestCommentId }) => - openExternalLink( - generateGitHubWebUrl( - url || `${notification.repository.url}/discussions`, - notification.id, - accounts.user?.id, - latestCommentId - ? '#discussioncomment-' + latestCommentId - : undefined, - ), - ), - ); - } else if (notification.subject.url) { - const latestCommentId = getCommentId( +): Promise { + let url; + + if (notification.subject.latest_comment_url) { + url = await getHtmlUrl( notification.subject.latest_comment_url, + accounts.token, ); - openExternalLink( - generateGitHubWebUrl( - notification.subject.url, - notification.id, - accounts.user?.id, - latestCommentId ? '#issuecomment-' + latestCommentId : undefined, - ), - ); + } else if (notification.subject.url) { + url = await getHtmlUrl(notification.subject.url, accounts.token); + } else { + // Perform any specific notification type handling (only required for a few special notification scenarios) + switch (notification.subject.type) { + case 'Discussion': + url = await getDiscussionUrl(notification, accounts.token); + break; + default: + url = notification.repository.html_url; + break; + } } + + url = addNotificationReferrerIdToUrl(url, notification.id, accounts.user?.id); + + return url; +} + +export async function openInBrowser( + notification: Notification, + accounts: AuthState, +) { + const url = await generateGitHubWebUrl(notification, accounts); + + openExternalLink(url); } diff --git a/src/utils/notifications.test.ts b/src/utils/notifications.test.ts index adb1a45ca..08298feef 100644 --- a/src/utils/notifications.test.ts +++ b/src/utils/notifications.test.ts @@ -1,17 +1,15 @@ import { ipcRenderer } from 'electron'; -import { generateGitHubWebUrl, getCommentId } from './helpers'; import { mockedAccountNotifications, mockedGithubNotifications, mockedSingleAccountNotifications, - mockedUser, } from '../__mocks__/mockedData'; import { mockAccounts } from '../__mocks__/mock-state'; -import * as comms from './comms'; import * as notificationsHelpers from './notifications'; import { SettingsState } from '../types'; import { defaultSettings } from '../context/App'; +import * as helpers from './helpers'; describe('utils/notifications.ts', () => { afterEach(() => { @@ -107,7 +105,7 @@ describe('utils/notifications.ts', () => { }); it('should click on a native notification (with 1 notification)', () => { - jest.spyOn(comms, 'openExternalLink'); + jest.spyOn(helpers, 'openInBrowser'); const nativeNotification: Notification = notificationsHelpers.raiseNativeNotification( @@ -116,15 +114,12 @@ describe('utils/notifications.ts', () => { ); nativeNotification.onclick(null); - const notif = mockedGithubNotifications[0]; - const newUrl = generateGitHubWebUrl( - notif.subject.url, - notif.id, - mockedUser.id, - '#issuecomment-' + getCommentId(notif.subject.latest_comment_url), + expect(helpers.openInBrowser).toHaveBeenCalledTimes(1); + expect(helpers.openInBrowser).toHaveBeenLastCalledWith( + mockedGithubNotifications[0], + mockAccounts, ); - expect(comms.openExternalLink).toHaveBeenCalledTimes(1); - expect(comms.openExternalLink).toHaveBeenCalledWith(newUrl); + expect(ipcRenderer.send).toHaveBeenCalledWith('hide-window'); }); it('should click on a native notification (with more than 1 notification)', () => {