Repository: gitify-app/gitify
Pull request #793 (2024-02-19 12:54:06)

refactor: simplify notification html url logic
Fetch the most appropriate html url for the notification based off the notification API response.

Order is
* notification last_comment_url
* notification subject url
* discussions specific lookup (not available via the notifications api)
* default, repository url

Removes or simplifies a lot of string manipulation to generate the link, add in comment anchors and referrer ids. 

Note: tests still need to be reworked given the logic changes in helper.ts

setchy: This PR is now ready for review @afonsojramos  @bmulholland.  

In addition to simplifying the html_url logic (no more string manipulation), this now has increased unit test coverage, and in doing so resolved an edge-base bug for Discussions url generation.
bmulholland: Yep, LGTM!

Patch:
diff --git a/main.js b/main.js
index 56ffc84c..a6d61e19 100644
--- a/main.js
+++ b/main.js
@@ -4,7 +4,7 @@ const { autoUpdater } = require('electron-updater');
 const { onFirstRunMaybe } = require('./first-run');
 const path = require('path');
 
-require('@electron/remote/main').initialize()
+require('@electron/remote/main').initialize();
 
 app.setAppUserModelId('com.electron.gitify');
 
@@ -12,7 +12,7 @@ const iconIdle = path.join(
   __dirname,
   'assets',
   'images',
-  'tray-idleTemplate.png'
+  'tray-idleTemplate.png',
 );
 const iconActive = path.join(__dirname, 'assets', 'images', 'tray-active.png');
 
@@ -30,16 +30,6 @@ const browserWindowOpts = {
   },
 };
 
-const delayedHideAppIcon = () => {
-  if (app.dock && app.dock.hide) {
-    // Setting a timeout because the showDockIcon is not currently working
-    // See more at https://github.com/maxogden/menubar/issues/306
-    setTimeout(() => {
-      app.dock.hide();
-    }, 1500);
-  }
-};
-
 app.on('ready', async () => {
   await onFirstRunMaybe();
 });
@@ -49,11 +39,10 @@ const menubarApp = menubar({
   index: `file://${__dirname}/index.html`,
   browserWindow: browserWindowOpts,
   preloadWindow: true,
+  showDockIcon: false,
 });
 
 menubarApp.on('ready', () => {
-  delayedHideAppIcon();
-
   menubarApp.tray.setIgnoreDoubleClickEvents(true);
 
   autoUpdater.checkForUpdatesAndNotify();
@@ -100,7 +89,7 @@ menubarApp.on('ready', () => {
     const trayBounds = menubarApp.tray.getBounds();
     menubarApp.window.setSize(
       browserWindowOpts.width,
-      browserWindowOpts.height
+      browserWindowOpts.height,
     );
     menubarApp.positioner.move('trayCenter', trayBounds);
     menubarApp.window.resizable = false;
diff --git a/src/__mocks__/mock-state.ts b/src/__mocks__/mock-state.ts
index 310581a6..ac5072e4 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 7ce0f56b..228ae155 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/typesGithub.ts b/src/typesGithub.ts
index fe80ff92..bbe02f13 100644
--- a/src/typesGithub.ts
+++ b/src/typesGithub.ts
@@ -8,6 +8,7 @@ export type Reason =
   | 'member_feature_requested'
   | 'mention'
   | 'review_requested'
+  | 'security_advisory_credit'
   | 'security_alert'
   | 'state_change'
   | 'subscribed'
diff --git a/src/utils/github-api.ts b/src/utils/github-api.ts
index f5b45395..10811d5b 100644
--- a/src/utils/github-api.ts
+++ b/src/utils/github-api.ts
@@ -28,6 +28,7 @@ const DESCRIPTIONS = {
     MEMBER_FEATURE_REQUESTED: 'Organization members have requested to enable a feature such as Draft Pull Requests or CoPilot.',
     MENTION: 'You were specifically @mentioned in the content.',
     REVIEW_REQUESTED: "You, or a team you're a member of, were requested to review a pull request.",
+    SECURITY_ADVISORY_CREDIT: "You were credited for contributing to a security advisory.",
     SECURITY_ALERT: 'GitHub discovered a security vulnerability in your repository.',
     STATE_CHANGE: 'You changed the thread state (for example, closing an issue or merging a pull request).',
     SUBSCRIBED: "You're watching the repository.",
@@ -58,6 +59,8 @@ export function formatReason(reason: Reason): {
       return { type: 'Mention', description: DESCRIPTIONS['MENTION'] };
     case 'review_requested':
       return { type: 'Review Requested', description: DESCRIPTIONS['REVIEW_REQUESTED'] };
+    case 'security_advisory_credit':
+      return { type: 'Security Advisory Credit', description: DESCRIPTIONS['SECURITY_ADVISORY_CREDIT'] };
     case 'security_alert':
       return { type: 'Security Alert', description: DESCRIPTIONS['SECURITY_ALERT'] };
     case 'state_change':
diff --git a/src/utils/helpers.ts b/src/utils/helpers.ts
index fb71af3d..a90e4b21 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<string> {
+  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\/(?<id>\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<string> {
+  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);
 }


Test patch:
diff --git a/src/components/NotificationRow.test.tsx b/src/components/NotificationRow.test.tsx
index 06426188..9158a62d 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/__snapshots__/github-api.test.ts.snap b/src/utils/__snapshots__/github-api.test.ts.snap
index 09d81888..758f59fc 100644
--- a/src/utils/__snapshots__/github-api.test.ts.snap
+++ b/src/utils/__snapshots__/github-api.test.ts.snap
@@ -80,34 +80,41 @@ exports[`./utils/github-api.ts should format the notification reason 9`] = `
 `;
 
 exports[`./utils/github-api.ts should format the notification reason 10`] = `
+{
+  "description": "You were credited for contributing to a security advisory.",
+  "type": "Security Advisory Credit",
+}
+`;
+
+exports[`./utils/github-api.ts should format the notification reason 11`] = `
 {
   "description": "GitHub discovered a security vulnerability in your repository.",
   "type": "Security Alert",
 }
 `;
 
-exports[`./utils/github-api.ts should format the notification reason 11`] = `
+exports[`./utils/github-api.ts should format the notification reason 12`] = `
 {
   "description": "You changed the thread state (for example, closing an issue or merging a pull request).",
   "type": "State Change",
 }
 `;
 
-exports[`./utils/github-api.ts should format the notification reason 12`] = `
+exports[`./utils/github-api.ts should format the notification reason 13`] = `
 {
   "description": "You're watching the repository.",
   "type": "Subscribed",
 }
 `;
 
-exports[`./utils/github-api.ts should format the notification reason 13`] = `
+exports[`./utils/github-api.ts should format the notification reason 14`] = `
 {
   "description": "You were on a team that was mentioned.",
   "type": "Team Mention",
 }
 `;
 
-exports[`./utils/github-api.ts should format the notification reason 14`] = `
+exports[`./utils/github-api.ts should format the notification reason 15`] = `
 {
   "description": "The reason for this notification is not supported by the app.",
   "type": "Unknown",
diff --git a/src/utils/github-api.test.ts b/src/utils/github-api.test.ts
index 8a055d37..5ce25403 100644
--- a/src/utils/github-api.test.ts
+++ b/src/utils/github-api.test.ts
@@ -16,6 +16,7 @@ describe('./utils/github-api.ts', () => {
     expect(formatReason('member_feature_requested')).toMatchSnapshot();
     expect(formatReason('mention')).toMatchSnapshot();
     expect(formatReason('review_requested')).toMatchSnapshot();
+    expect(formatReason('security_advisory_credit')).toMatchSnapshot();
     expect(formatReason('security_alert')).toMatchSnapshot();
     expect(formatReason('state_change')).toMatchSnapshot();
     expect(formatReason('subscribed')).toMatchSnapshot();
diff --git a/src/utils/helpers.test.ts b/src/utils/helpers.test.ts
index 7ccfada9..bb0e6645 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/notifications.test.ts b/src/utils/notifications.test.ts
index adb1a45c..08298fee 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)', () => {