feat: use notification_referrer_id for better UX (#519)

codebytere · web-flow · commit c3683dcfd84a · 2021-10-28T22:13:37.000+02:00
diff --git a/src/__mocks__/mock-state.ts b/src/__mocks__/mock-state.ts
@@ -1,4 +1,5 @@
 import { Appearance, AuthState, SettingsState } from '../types';
+import { mockedUser } from './mockedData';
 
 export const mockAccounts: AuthState = {
   token: 'token-123-456',
@@ -8,6 +9,7 @@ export const mockAccounts: AuthState = {
       hostname: 'github.gitify.io',
     },
   ],
+  user: mockedUser,
 };
 
 export const mockSettings: SettingsState = {
diff --git a/src/__mocks__/mockedData.ts b/src/__mocks__/mockedData.ts
@@ -1,5 +1,5 @@
 import { AccountNotifications, EnterpriseAccount } from '../types';
-import { Notification, Repository } from '../typesGithub';
+import { Notification, Repository, User } from '../typesGithub';
 
 export const mockedEnterpriseAccounts: EnterpriseAccount[] = [
   {
@@ -8,6 +8,12 @@ export const mockedEnterpriseAccounts: EnterpriseAccount[] = [
   },
 ];
 
+export const mockedUser: User = {
+  login: 'octocat',
+  name: 'Mona Lisa Octocat',
+  id: 123456789,
+};
+
 // prettier-ignore
 export const mockedSingleNotification: Notification = {
   id: '138661096',
diff --git a/src/components/NotificationRow.test.tsx b/src/components/NotificationRow.test.tsx
@@ -7,7 +7,7 @@ const { shell } = require('electron');
 import { AppContext } from '../context/App';
 import { mockedSingleNotification } from '../__mocks__/mockedData';
 import { NotificationRow } from './NotificationRow';
-import { mockSettings } from '../__mocks__/mock-state';
+import { mockAccounts, mockSettings } from '../__mocks__/mock-state';
 
 describe('components/Notification.js', () => {
   beforeEach(() => {
@@ -39,6 +39,7 @@ describe('components/Notification.js', () => {
         value={{
           settings: { ...mockSettings, markOnClick: true },
           markNotification,
+          accounts: mockAccounts,
         }}
       >
         <NotificationRow {...props} />
@@ -62,6 +63,7 @@ describe('components/Notification.js', () => {
         value={{
           settings: { ...mockSettings, markOnClick: true },
           markNotification,
+          accounts: mockAccounts,
         }}
       >
         <NotificationRow {...props} />
@@ -83,7 +85,10 @@ describe('components/Notification.js', () => {
 
     const { getByTitle } = render(
       <AppContext.Provider
-        value={{ settings: { ...mockSettings, markOnClick: false } }}
+        value={{
+          settings: { ...mockSettings, markOnClick: false },
+          accounts: mockAccounts,
+        }}
       >
         <AppContext.Provider value={{ markNotification }}>
           <NotificationRow {...props} />
diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx
@@ -18,7 +18,7 @@ export const NotificationRow: React.FC<IProps> = ({
   notification,
   hostname,
 }) => {
-  const { settings } = useContext(AppContext);
+  const { settings, accounts } = useContext(AppContext);
   const { markNotification, unsubscribeNotification } = useContext(AppContext);
 
   const pressTitle = useCallback(() => {
@@ -32,7 +32,11 @@ export const NotificationRow: React.FC<IProps> = ({
   const openBrowser = useCallback(() => {
     // Some Notification types from GitHub are missing urls in their subjects.
     if (notification.subject.url) {
-      const url = generateGitHubWebUrl(notification.subject.url);
+      const url = generateGitHubWebUrl(
+        notification.subject.url,
+        notification.id,
+        accounts.user?.id
+      );
       shell.openExternal(url);
     }
   }, [notification]);
diff --git a/src/context/App.test.tsx b/src/context/App.test.tsx
@@ -105,7 +105,7 @@ describe('context/App.tsx', () => {
 
       expect(markNotificationMock).toHaveBeenCalledTimes(1);
       expect(markNotificationMock).toHaveBeenCalledWith(
-        { enterpriseAccounts: [], token: null },
+        { enterpriseAccounts: [], token: null, user: null },
         '123-456',
         'github.com'
       );
@@ -132,7 +132,7 @@ describe('context/App.tsx', () => {
 
       expect(unsubscribeNotificationMock).toHaveBeenCalledTimes(1);
       expect(unsubscribeNotificationMock).toHaveBeenCalledWith(
-        { enterpriseAccounts: [], token: null },
+        { enterpriseAccounts: [], token: null, user: null },
         '123-456',
         'github.com'
       );
@@ -161,7 +161,7 @@ describe('context/App.tsx', () => {
 
       expect(markRepoNotificationsMock).toHaveBeenCalledTimes(1);
       expect(markRepoNotificationsMock).toHaveBeenCalledWith(
-        { enterpriseAccounts: [], token: null },
+        { enterpriseAccounts: [], token: null, user: null },
         'manosim/gitify',
         'github.com'
       );
@@ -192,12 +192,17 @@ describe('context/App.tsx', () => {
         expect(fetchNotificationsMock).toHaveBeenCalledTimes(2)
       );
 
-      expect(apiRequestAuthMock).toHaveBeenCalledTimes(1);
+      expect(apiRequestAuthMock).toHaveBeenCalledTimes(2);
       expect(apiRequestAuthMock).toHaveBeenCalledWith(
         'https://api.github.com/notifications',
         'HEAD',
         '123-456'
       );
+      expect(apiRequestAuthMock).toHaveBeenCalledWith(
+        'https://api.github.com/user',
+        'GET',
+        '123-456'
+      );
     });
   });
 
@@ -240,7 +245,7 @@ describe('context/App.tsx', () => {
 
     expect(saveStateMock).toHaveBeenCalled();
     expect(saveStateMock).toHaveBeenCalledWith(
-      { enterpriseAccounts: [], token: null },
+      { enterpriseAccounts: [], token: null, user: null },
       {
         appearance: 'SYSTEM',
         markOnClick: false,
@@ -276,7 +281,7 @@ describe('context/App.tsx', () => {
     expect(setAutoLaunchMock).toHaveBeenCalledWith(true);
     expect(saveStateMock).toHaveBeenCalled();
     expect(saveStateMock).toHaveBeenCalledWith(
-      { enterpriseAccounts: [], token: null },
+      { enterpriseAccounts: [], token: null, user: null },
       {
         appearance: 'SYSTEM',
         markOnClick: false,
diff --git a/src/context/App.tsx b/src/context/App.tsx
@@ -15,16 +15,18 @@ import {
   SettingsState,
 } from '../types';
 import { apiRequestAuth } from '../utils/api-requests';
-import { addAccount, authGitHub, getToken } from '../utils/auth';
+import { addAccount, authGitHub, getToken, getUserData } from '../utils/auth';
 import { clearState, loadState, saveState } from '../utils/storage';
 import { setAppearance } from '../utils/appearance';
 import { setAutoLaunch } from '../utils/comms';
 import { useInterval } from '../hooks/useInterval';
 import { useNotifications } from '../hooks/useNotifications';
+import Constants from '../utils/constants';
 
 const defaultAccounts: AuthState = {
   token: null,
   enterpriseAccounts: [],
+  user: null,
 };
 
 export const defaultSettings: SettingsState = {
@@ -111,8 +113,11 @@ export const AppProvider = ({ children }: { children: React.ReactNode }) => {
   const login = useCallback(async () => {
     const { authCode } = await authGitHub();
     const { token } = await getToken(authCode);
-    setAccounts({ ...accounts, token });
-    saveState({ ...accounts, token }, settings);
+    const user = await getUserData(token);
+    const hostname = Constants.DEFAULT_AUTH_OPTIONS.hostname;
+    const updatedAccounts = addAccount(accounts, token, hostname, user);
+    setAccounts(updatedAccounts);
+    saveState(updatedAccounts, settings);
   }, [accounts, settings]);
 
   const loginEnterprise = useCallback(
@@ -133,7 +138,8 @@ export const AppProvider = ({ children }: { children: React.ReactNode }) => {
         'HEAD',
         token
       );
-      const updatedAccounts = addAccount(accounts, token, hostname);
+      const user = await getUserData(token);
+      const updatedAccounts = addAccount(accounts, token, hostname, user);
       setAccounts(updatedAccounts);
       saveState(updatedAccounts, settings);
     },
diff --git a/src/hooks/useNotifications.test.ts b/src/hooks/useNotifications.test.ts
@@ -5,6 +5,7 @@ import { act, renderHook } from '@testing-library/react-hooks';
 import { mockAccounts, mockSettings } from '../__mocks__/mock-state';
 import { useNotifications } from './useNotifications';
 import { AuthState } from '../types';
+import { mockedUser } from '../__mocks__/mockedData';
 
 describe('hooks/useNotifications.ts', () => {
   beforeEach(() => {
@@ -135,6 +136,7 @@ describe('hooks/useNotifications.ts', () => {
         const accounts: AuthState = {
           ...mockAccounts,
           enterpriseAccounts: [],
+          user: mockedUser,
         };
 
         const notifications = [
diff --git a/src/hooks/useNotifications.ts b/src/hooks/useNotifications.ts
@@ -96,7 +96,12 @@ export const useNotifications = (): NotificationsState => {
                 ]
               : [...enterpriseNotifications];
 
-            triggerNativeNotifications(notifications, data, settings);
+            triggerNativeNotifications(
+              notifications,
+              data,
+              settings,
+              accounts.user
+            );
             setNotifications(data);
             setIsFetching(false);
           })
diff --git a/src/routes/LoginEnterprise.test.tsx b/src/routes/LoginEnterprise.test.tsx
@@ -18,6 +18,7 @@ describe('routes/LoginEnterprise.js', () => {
 
   const mockAccounts: AuthState = {
     enterpriseAccounts: [],
+    user: null,
   };
 
   beforeEach(function () {
@@ -93,6 +94,7 @@ describe('routes/LoginEnterprise.js', () => {
         value={{
           accounts: {
             enterpriseAccounts: mockedEnterpriseAccounts,
+            user: null,
           },
         }}
       >
diff --git a/src/routes/LoginWithToken.tsx b/src/routes/LoginWithToken.tsx
@@ -67,7 +67,11 @@ export const LoginWithToken: React.FC = () => {
               >
                 personal access tokens
               </a>{' '}
-              and create one with the <strong>notifications</strong> scope.
+              and create one with the{' '}
+              <span className="underline font-extrabold text-yellow-500">
+                {Constants.AUTH_SCOPE.join(', ')}{' '}
+              </span>
+              scopes.
             </>
           }
         />
@@ -81,7 +85,7 @@ export const LoginWithToken: React.FC = () => {
 
         {!isValidToken && (
           <div className="mt-4 text-red-500 text-sm font-medium">
-            This token could not get validated with {values.hostname}.
+            This token could not be validated with {values.hostname}.
           </div>
         )}
 
diff --git a/src/routes/__snapshots__/LoginWithToken.test.tsx.snap b/src/routes/__snapshots__/LoginWithToken.test.tsx.snap
@@ -79,11 +79,15 @@ exports[`routes/LoginWithToken.js renders correctly 1`] = `
             personal access tokens
           </a>
            
-          and create one with the 
-          <strong>
-            notifications
-          </strong>
-           scope.
+          and create one with the
+           
+          <span
+            className="underline font-extrabold text-yellow-500"
+          >
+            read:user, notifications
+             
+          </span>
+          scopes.
         </div>
       </div>
       <div
diff --git a/src/types.ts b/src/types.ts
@@ -1,8 +1,9 @@
-import { Notification } from './typesGithub';
+import { Notification, User } from './typesGithub';
 
 export interface AuthState {
   token?: string;
   enterpriseAccounts: EnterpriseAccount[];
+  user: User | null;
 }
 
 export interface SettingsState {
diff --git a/src/typesGithub.ts b/src/typesGithub.ts
@@ -33,6 +33,12 @@ export interface Notification {
   subscription_url: string;
 }
 
+export interface User {
+  login: string;
+  name: string;
+  id: number;
+}
+
 export interface Repository {
   id: number;
   node_id: string;
diff --git a/src/utils/auth.test.ts b/src/utils/auth.test.ts
@@ -35,7 +35,7 @@ describe('utils/auth.tsx', () => {
 
       expect(loadURLMock).toHaveBeenCalledTimes(1);
       expect(loadURLMock).toHaveBeenCalledWith(
-        'https://github.com/login/oauth/authorize?client_id=FAKE_CLIENT_ID_123&scope=user:email,notifications'
+        'https://github.com/login/oauth/authorize?client_id=FAKE_CLIENT_ID_123&scope=read:user,notifications'
       );
 
       expect(new BrowserWindow().destroy).toHaveBeenCalledTimes(1);
@@ -103,6 +103,7 @@ describe('utils/auth.tsx', () => {
     const accounts: AuthState = {
       token: null,
       enterpriseAccounts: [],
+      user: null,
     };
 
     it('should add a github.com accont', async () => {
diff --git a/src/utils/auth.ts b/src/utils/auth.ts
@@ -1,9 +1,10 @@
 const { remote } = require('electron');
 const BrowserWindow = remote.BrowserWindow;
 
-import { apiRequest } from '../utils/api-requests';
+import { apiRequest, apiRequestAuth } from '../utils/api-requests';
 import { AuthResponse, AuthState, AuthTokenResponse } from '../types';
 import { Constants } from '../utils/constants';
+import { User } from '../typesGithub';
 
 export const authGitHub = (
   authOptions = Constants.DEFAULT_AUTH_OPTIONS
@@ -67,6 +68,20 @@ export const authGitHub = (
   });
 };
 
+export const getUserData = async (token: string): Promise<User> => {
+  const response = await apiRequestAuth(
+    `https://api.${Constants.DEFAULT_AUTH_OPTIONS.hostname}/user`,
+    'GET',
+    token
+  );
+
+  return {
+    id: response.data.id,
+    login: response.data.login,
+    name: response.data.name,
+  };
+};
+
 export const getToken = async (
   authCode: string,
   authOptions = Constants.DEFAULT_AUTH_OPTIONS
@@ -85,11 +100,17 @@ export const getToken = async (
   };
 };
 
-export const addAccount = (accounts: AuthState, token, hostname): AuthState => {
+export const addAccount = (
+  accounts: AuthState,
+  token,
+  hostname,
+  user?: User
+): AuthState => {
   if (hostname === Constants.DEFAULT_AUTH_OPTIONS.hostname) {
     return {
       ...accounts,
       token,
+      user: user ?? null,
     };
   }
 
diff --git a/src/utils/constants.ts b/src/utils/constants.ts
@@ -1,6 +1,6 @@
 export const Constants = {
   // GitHub OAuth
-  AUTH_SCOPE: ['user:email', 'notifications'],
+  AUTH_SCOPE: ['read:user', 'notifications'],
 
   DEFAULT_AUTH_OPTIONS: {
     hostname: 'github.com',
diff --git a/src/utils/helpers.test.ts b/src/utils/helpers.test.ts
diff --git a/src/utils/helpers.ts b/src/utils/helpers.ts
diff --git a/src/utils/notifications.test.ts b/src/utils/notifications.test.ts
diff --git a/src/utils/notifications.ts b/src/utils/notifications.ts
diff --git a/src/utils/storage.test.ts b/src/utils/storage.test.ts
