Repository: gitify-app/gitify
Pull request #1095 (2024-05-08 13:27:02)

refactor: use URL object instead of strings
Further refactoring to favor using URL objects rather than string manipulation


Patch:
diff --git a/src/utils/api/client.ts b/src/utils/api/client.ts
index e2e476cf..3401851d 100644
--- a/src/utils/api/client.ts
+++ b/src/utils/api/client.ts
@@ -15,13 +15,12 @@ import type {
   RootHypermediaLinks,
   UserDetails,
 } from '../../typesGitHub';
-import { getGitHubAPIBaseUrl } from '../helpers';
 import { apiRequestAuth } from './request';
 
 import { print } from 'graphql/language/printer';
 import Constants from '../constants';
 import { QUERY_SEARCH_DISCUSSIONS } from './graphql/discussions';
-import { formatSearchQueryString } from './utils';
+import { formatSearchQueryString, getGitHubAPIBaseUrl } from './utils';
 
 /**
  * Get Hypermedia links to resources accessible in GitHub's REST API
@@ -32,8 +31,7 @@ export function getRootHypermediaLinks(
   hostname: string,
   token: string,
 ): AxiosPromise<RootHypermediaLinks> {
-  const baseUrl = getGitHubAPIBaseUrl(hostname);
-  const url = new URL(baseUrl);
+  const url = getGitHubAPIBaseUrl(hostname);
   return apiRequestAuth(url.toString(), 'GET', token);
 }
 
@@ -46,8 +44,9 @@ export function getAuthenticatedUser(
   hostname: string,
   token: string,
 ): AxiosPromise<UserDetails> {
-  const baseUrl = getGitHubAPIBaseUrl(hostname);
-  const url = new URL(`${baseUrl}/user`);
+  const url = getGitHubAPIBaseUrl(hostname);
+  url.pathname += 'user';
+
   return apiRequestAuth(url.toString(), 'GET', token);
 }
 
@@ -56,8 +55,9 @@ export function headNotifications(
   hostname: string,
   token: string,
 ): AxiosPromise<void> {
-  const baseUrl = getGitHubAPIBaseUrl(hostname);
-  const url = new URL(`${baseUrl}/notifications`);
+  const url = getGitHubAPIBaseUrl(hostname);
+  url.pathname += 'notifications';
+
   return apiRequestAuth(url.toString(), 'HEAD', token);
 }
 
@@ -71,8 +71,8 @@ export function listNotificationsForAuthenticatedUser(
   token: string,
   settings: SettingsState,
 ): AxiosPromise<Notification[]> {
-  const baseUrl = getGitHubAPIBaseUrl(hostname);
-  const url = new URL(`${baseUrl}/notifications`);
+  const url = getGitHubAPIBaseUrl(hostname);
+  url.pathname += 'notifications';
   url.searchParams.append('participating', String(settings.participating));
 
   return apiRequestAuth(url.toString(), 'GET', token);
@@ -89,8 +89,9 @@ export function markNotificationThreadAsRead(
   hostname: string,
   token: string,
 ): AxiosPromise<void> {
-  const baseUrl = getGitHubAPIBaseUrl(hostname);
-  const url = new URL(`${baseUrl}/notifications/threads/${threadId}`);
+  const url = getGitHubAPIBaseUrl(hostname);
+  url.pathname += `notifications/threads/${threadId}`;
+
   return apiRequestAuth(url.toString(), 'PATCH', token, {});
 }
 
@@ -105,8 +106,9 @@ export function markNotificationThreadAsDone(
   hostname: string,
   token: string,
 ): AxiosPromise<void> {
-  const baseUrl = getGitHubAPIBaseUrl(hostname);
-  const url = new URL(`${baseUrl}/notifications/threads/${threadId}`);
+  const url = getGitHubAPIBaseUrl(hostname);
+  url.pathname += `notifications/threads/${threadId}`;
+
   return apiRequestAuth(url.toString(), 'DELETE', token, {});
 }
 
@@ -120,10 +122,9 @@ export function ignoreNotificationThreadSubscription(
   hostname: string,
   token: string,
 ): AxiosPromise<NotificationThreadSubscription> {
-  const baseUrl = getGitHubAPIBaseUrl(hostname);
-  const url = new URL(
-    `${baseUrl}/notifications/threads/${threadId}/subscription`,
-  );
+  const url = getGitHubAPIBaseUrl(hostname);
+  url.pathname += `notifications/threads/${threadId}/subscription`;
+
   return apiRequestAuth(url.toString(), 'PUT', token, { ignored: true });
 }
 
@@ -140,8 +141,9 @@ export function markRepositoryNotificationsAsRead(
   hostname: string,
   token: string,
 ): AxiosPromise<void> {
-  const baseUrl = getGitHubAPIBaseUrl(hostname);
-  const url = new URL(`${baseUrl}/repos/${repoSlug}/notifications`);
+  const url = getGitHubAPIBaseUrl(hostname);
+  url.pathname += `repos/${repoSlug}/notifications`;
+
   return apiRequestAuth(url.toString(), 'PUT', token, {});
 }
 
diff --git a/src/utils/api/utils.ts b/src/utils/api/utils.ts
index 6a833c9f..954704b0 100644
--- a/src/utils/api/utils.ts
+++ b/src/utils/api/utils.ts
@@ -1,3 +1,16 @@
+import Constants from '../constants';
+import { isEnterpriseHost } from '../helpers';
+
+export function getGitHubAPIBaseUrl(hostname: string): URL {
+  const url = new URL(Constants.GITHUB_API_BASE_URL);
+
+  if (isEnterpriseHost(hostname)) {
+    url.hostname = hostname;
+    url.pathname = '/api/v3/';
+  }
+  return url;
+}
+
 export function formatSearchQueryString(
   repo: string,
   title: string,
diff --git a/src/utils/helpers.ts b/src/utils/helpers.ts
index 970d9d02..b6501bd6 100644
--- a/src/utils/helpers.ts
+++ b/src/utils/helpers.ts
@@ -28,28 +28,6 @@ export function isEnterpriseHost(hostname: string): boolean {
   return !hostname.endsWith(Constants.DEFAULT_AUTH_OPTIONS.hostname);
 }
 
-export function getGitHubAPIBaseUrl(hostname: string): string {
-  const isEnterprise = isEnterpriseHost(hostname);
-  return isEnterprise
-    ? `https://${hostname}/api/v3`
-    : Constants.GITHUB_API_BASE_URL;
-}
-
-export function addNotificationReferrerIdToUrl(
-  url: string,
-  notificationId: string,
-  userId: number,
-): string {
-  const parsedUrl = new URL(url);
-
-  parsedUrl.searchParams.set(
-    'notification_referrer_id',
-    generateNotificationReferrerId(notificationId, userId),
-  );
-
-  return parsedUrl.href;
-}
-
 export function generateNotificationReferrerId(
   notificationId: string,
   userId: number,
@@ -61,7 +39,6 @@ export function generateNotificationReferrerId(
 }
 
 export function getCheckSuiteUrl(notification: Notification): string {
-  let url = `${notification.repository.html_url}/actions`;
   const filters = [];
 
   const checkSuiteAttributes = getCheckSuiteAttributes(notification);
@@ -80,15 +57,10 @@ export function getCheckSuiteUrl(notification: Notification): string {
     filters.push(`branch:${checkSuiteAttributes.branchName}`);
   }
 
-  if (filters.length > 0) {
-    url += `?query=${filters.join('+')}`;
-  }
-
-  return url;
+  return actionsURL(notification.repository.html_url, filters);
 }
 
 export function getWorkflowRunUrl(notification: Notification): string {
-  let url = `${notification.repository.html_url}/actions`;
   const filters = [];
 
   const workflowRunAttributes = getWorkflowRunAttributes(notification);
@@ -97,34 +69,46 @@ export function getWorkflowRunUrl(notification: Notification): string {
     filters.push(`is:${workflowRunAttributes.status}`);
   }
 
+  return actionsURL(notification.repository.html_url, filters);
+}
+
+/**
+ * Construct a GitHub Actions URL for a repository with optional filters.
+ */
+export function actionsURL(repositoryURL: string, filters: string[]): string {
+  const url = new URL(repositoryURL);
+  url.pathname += '/actions';
+
   if (filters.length > 0) {
-    url += `?query=${filters.join('+')}`;
+    url.searchParams.append('query', filters.join('+'));
   }
 
-  return url;
+  // Note: the GitHub Actions UI cannot handle encoded '+' characters.
+  return url.toString().replace(/%2B/g, '+');
 }
 
 async function getDiscussionUrl(
   notification: Notification,
   token: string,
 ): Promise<string> {
-  let url = `${notification.repository.html_url}/discussions`;
+  const url = new URL(notification.repository.html_url);
+  url.pathname += '/discussions';
 
   const discussion = await fetchDiscussion(notification, token);
 
   if (discussion) {
-    url = discussion.url;
+    url.href = discussion.url;
 
     const comments = discussion.comments.nodes;
 
     const latestComment = getLatestDiscussionComment(comments);
 
     if (latestComment) {
-      url += `#discussioncomment-${latestComment.databaseId}`;
+      url.hash = `#discussioncomment-${latestComment.databaseId}`;
     }
   }
 
-  return url;
+  return url.toString();
 }
 
 export async function fetchDiscussion(
@@ -169,36 +153,39 @@ export async function generateGitHubWebUrl(
   notification: Notification,
   accounts: AuthState,
 ): Promise<string> {
-  let url = notification.repository.html_url;
+  const url = new URL(notification.repository.html_url);
   const token = getTokenForHost(notification.hostname, accounts);
 
   if (notification.subject.latest_comment_url) {
-    url = await getHtmlUrl(notification.subject.latest_comment_url, token);
+    url.href = await getHtmlUrl(notification.subject.latest_comment_url, token);
   } else if (notification.subject.url) {
-    url = await getHtmlUrl(notification.subject.url, token);
+    url.href = await getHtmlUrl(notification.subject.url, token);
   } else {
     // Perform any specific notification type handling (only required for a few special notification scenarios)
     switch (notification.subject.type) {
       case 'CheckSuite':
-        url = getCheckSuiteUrl(notification);
+        url.href = getCheckSuiteUrl(notification);
         break;
       case 'Discussion':
-        url = await getDiscussionUrl(notification, token);
+        url.href = await getDiscussionUrl(notification, token);
         break;
       case 'RepositoryInvitation':
-        url = `${notification.repository.html_url}/invitations`;
+        url.pathname += '/invitations';
         break;
       case 'WorkflowRun':
-        url = getWorkflowRunUrl(notification);
+        url.href = getWorkflowRunUrl(notification);
         break;
       default:
         break;
     }
   }
 
-  url = addNotificationReferrerIdToUrl(url, notification.id, accounts.user?.id);
+  url.searchParams.set(
+    'notification_referrer_id',
+    generateNotificationReferrerId(notification.id, accounts.user?.id),
+  );
 
-  return url;
+  return url.toString();
 }
 
 export function formatForDisplay(text: string[]): string {


Test patch:
diff --git a/src/utils/api/client.test.ts b/src/utils/api/client.test.ts
index e0d864e9..58fb9277 100644
--- a/src/utils/api/client.test.ts
+++ b/src/utils/api/client.test.ts
@@ -43,7 +43,7 @@ describe('utils/api/client.ts', () => {
       await getRootHypermediaLinks(mockEnterpriseHostname, mockToken);
 
       expect(axios).toHaveBeenCalledWith({
-        url: 'https://example.com/api/v3',
+        url: 'https://example.com/api/v3/',
         method: 'GET',
         data: {},
       });
diff --git a/src/utils/api/utils.test.ts b/src/utils/api/utils.test.ts
index 001b3f3c..54af2e1e 100644
--- a/src/utils/api/utils.test.ts
+++ b/src/utils/api/utils.test.ts
@@ -1,15 +1,19 @@
-import { addHours, formatSearchQueryString } from './utils';
+import {
+  addHours,
+  formatSearchQueryString,
+  getGitHubAPIBaseUrl,
+} from './utils';
 
 describe('utils/api/utils.ts', () => {
-  describe('addHours', () => {
-    test('adds hours correctly for positive values', () => {
-      const result = addHours('2024-02-20T12:00:00.000Z', 3);
-      expect(result).toBe('2024-02-20T15:00:00.000Z');
+  describe('generateGitHubAPIUrl', () => {
+    it('should generate a GitHub API url - non enterprise', () => {
+      const result = getGitHubAPIBaseUrl('github.com');
+      expect(result.toString()).toBe('https://api.github.com/');
     });
 
-    test('adds hours correctly for negative values', () => {
-      const result = addHours('2024-02-20T12:00:00.000Z', -2);
-      expect(result).toBe('2024-02-20T10:00:00.000Z');
+    it('should generate a GitHub API url - enterprise', () => {
+      const result = getGitHubAPIBaseUrl('github.manos.im');
+      expect(result.toString()).toBe('https://github.manos.im/api/v3/');
     });
   });
 
@@ -26,4 +30,16 @@ describe('utils/api/utils.ts', () => {
       );
     });
   });
+
+  describe('addHours', () => {
+    test('adds hours correctly for positive values', () => {
+      const result = addHours('2024-02-20T12:00:00.000Z', 3);
+      expect(result).toBe('2024-02-20T15:00:00.000Z');
+    });
+
+    test('adds hours correctly for negative values', () => {
+      const result = addHours('2024-02-20T12:00:00.000Z', -2);
+      expect(result).toBe('2024-02-20T10:00:00.000Z');
+    });
+  });
 });
diff --git a/src/utils/helpers.test.ts b/src/utils/helpers.test.ts
index f9482891..1adbdfcb 100644
--- a/src/utils/helpers.test.ts
+++ b/src/utils/helpers.test.ts
@@ -8,11 +8,9 @@ import {
 import type { SubjectType } from '../typesGitHub';
 import * as apiRequests from './api/request';
 import {
-  addNotificationReferrerIdToUrl,
   formatForDisplay,
   generateGitHubWebUrl,
   generateNotificationReferrerId,
-  getGitHubAPIBaseUrl,
   isEnterpriseHost,
   isGitHubLoggedIn,
 } from './helpers';
@@ -39,43 +37,6 @@ describe('utils/helpers.ts', () => {
     });
   });
 
-  describe('addNotificationReferrerIdToUrl', () => {
-    it('should add notification_referrer_id to the URL', () => {
-      // Mock data
-      const url = 'https://github.com/gitify-app/notifications-test';
-      const notificationId = '123';
-      const userId = 456;
-
-      const result = addNotificationReferrerIdToUrl(
-        url,
-        notificationId,
-        userId,
-      );
-
-      expect(result).toEqual(
-        'https://github.com/gitify-app/notifications-test?notification_referrer_id=MDE4Ok5vdGlmaWNhdGlvblRocmVhZDEyMzo0NTY%3D',
-      );
-    });
-
-    it('should add notification_referrer_id to the URL, preserving anchor tags', () => {
-      // Mock data
-      const url =
-        'https://github.com/gitify-app/notifications-test/pull/123#issuecomment-1951055051';
-      const notificationId = '123';
-      const userId = 456;
-
-      const result = addNotificationReferrerIdToUrl(
-        url,
-        notificationId,
-        userId,
-      );
-
-      expect(result).toEqual(
-        'https://github.com/gitify-app/notifications-test/pull/123?notification_referrer_id=MDE4Ok5vdGlmaWNhdGlvblRocmVhZDEyMzo0NTY%3D#issuecomment-1951055051',
-      );
-    });
-  });
-
   describe('generateNotificationReferrerId', () => {
     it('should generate the notification_referrer_id', () => {
       const referrerId = generateNotificationReferrerId(
@@ -88,18 +49,6 @@ describe('utils/helpers.ts', () => {
     });
   });
 
-  describe('generateGitHubAPIUrl', () => {
-    it('should generate a GitHub API url - non enterprise', () => {
-      const result = getGitHubAPIBaseUrl('github.com');
-      expect(result).toBe('https://api.github.com');
-    });
-
-    it('should generate a GitHub API url - enterprise', () => {
-      const result = getGitHubAPIBaseUrl('github.gitify.app');
-      expect(result).toBe('https://github.gitify.app/api/v3');
-    });
-  });
-
   describe('generateGitHubWebUrl', () => {
     const mockedHtmlUrl =
       'https://github.com/gitify-app/notifications-test/issues/785';