Skip to content

Commit c3683dc

Browse files
codebyterecodebytere
authored
feat: use notification_referrer_id for better UX (#519)
1 parent 20e80eb commit c3683dc

21 files changed

+272
-95
lines changed

‎src/__mocks__/mock-state.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { Appearance, AuthState, SettingsState } from '../types';
2+
import { mockedUser } from './mockedData';
23

34
export const mockAccounts: AuthState = {
45
token: 'token-123-456',
@@ -8,6 +9,7 @@ export const mockAccounts: AuthState = {
89
hostname: 'github.gitify.io',
910
},
1011
],
12+
user: mockedUser,
1113
};
1214

1315
export const mockSettings: SettingsState = {

‎src/__mocks__/mockedData.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { AccountNotifications, EnterpriseAccount } from '../types';
2-
import { Notification, Repository } from '../typesGithub';
2+
import { Notification, Repository, User } from '../typesGithub';
33

44
export const mockedEnterpriseAccounts: EnterpriseAccount[] = [
55
{
@@ -8,6 +8,12 @@ export const mockedEnterpriseAccounts: EnterpriseAccount[] = [
88
},
99
];
1010

11+
export const mockedUser: User = {
12+
login: 'octocat',
13+
name: 'Mona Lisa Octocat',
14+
id: 123456789,
15+
};
16+
1117
// prettier-ignore
1218
export const mockedSingleNotification: Notification = {
1319
id: '138661096',

‎src/components/NotificationRow.test.tsx

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ const { shell } = require('electron');
77
import { AppContext } from '../context/App';
88
import { mockedSingleNotification } from '../__mocks__/mockedData';
99
import { NotificationRow } from './NotificationRow';
10-
import { mockSettings } from '../__mocks__/mock-state';
10+
import { mockAccounts, mockSettings } from '../__mocks__/mock-state';
1111

1212
describe('components/Notification.js', () => {
1313
beforeEach(() => {
@@ -39,6 +39,7 @@ describe('components/Notification.js', () => {
3939
value={{
4040
settings: { ...mockSettings, markOnClick: true },
4141
markNotification,
42+
accounts: mockAccounts,
4243
}}
4344
>
4445
<NotificationRow {...props} />
@@ -62,6 +63,7 @@ describe('components/Notification.js', () => {
6263
value={{
6364
settings: { ...mockSettings, markOnClick: true },
6465
markNotification,
66+
accounts: mockAccounts,
6567
}}
6668
>
6769
<NotificationRow {...props} />
@@ -83,7 +85,10 @@ describe('components/Notification.js', () => {
8385

8486
const { getByTitle } = render(
8587
<AppContext.Provider
86-
value={{ settings: { ...mockSettings, markOnClick: false } }}
88+
value={{
89+
settings: { ...mockSettings, markOnClick: false },
90+
accounts: mockAccounts,
91+
}}
8792
>
8893
<AppContext.Provider value={{ markNotification }}>
8994
<NotificationRow {...props} />

‎src/components/NotificationRow.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ export const NotificationRow: React.FC<IProps> = ({
1818
notification,
1919
hostname,
2020
}) => {
21-
const { settings } = useContext(AppContext);
21+
const { settings, accounts } = useContext(AppContext);
2222
const { markNotification, unsubscribeNotification } = useContext(AppContext);
2323

2424
const pressTitle = useCallback(() => {
@@ -32,7 +32,11 @@ export const NotificationRow: React.FC<IProps> = ({
3232
const openBrowser = useCallback(() => {
3333
// Some Notification types from GitHub are missing urls in their subjects.
3434
if (notification.subject.url) {
35-
const url = generateGitHubWebUrl(notification.subject.url);
35+
const url = generateGitHubWebUrl(
36+
notification.subject.url,
37+
notification.id,
38+
accounts.user?.id
39+
);
3640
shell.openExternal(url);
3741
}
3842
}, [notification]);

‎src/context/App.test.tsx

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ describe('context/App.tsx', () => {
105105

106106
expect(markNotificationMock).toHaveBeenCalledTimes(1);
107107
expect(markNotificationMock).toHaveBeenCalledWith(
108-
{ enterpriseAccounts: [], token: null },
108+
{ enterpriseAccounts: [], token: null, user: null },
109109
'123-456',
110110
'github.com'
111111
);
@@ -132,7 +132,7 @@ describe('context/App.tsx', () => {
132132

133133
expect(unsubscribeNotificationMock).toHaveBeenCalledTimes(1);
134134
expect(unsubscribeNotificationMock).toHaveBeenCalledWith(
135-
{ enterpriseAccounts: [], token: null },
135+
{ enterpriseAccounts: [], token: null, user: null },
136136
'123-456',
137137
'github.com'
138138
);
@@ -161,7 +161,7 @@ describe('context/App.tsx', () => {
161161

162162
expect(markRepoNotificationsMock).toHaveBeenCalledTimes(1);
163163
expect(markRepoNotificationsMock).toHaveBeenCalledWith(
164-
{ enterpriseAccounts: [], token: null },
164+
{ enterpriseAccounts: [], token: null, user: null },
165165
'manosim/gitify',
166166
'github.com'
167167
);
@@ -192,12 +192,17 @@ describe('context/App.tsx', () => {
192192
expect(fetchNotificationsMock).toHaveBeenCalledTimes(2)
193193
);
194194

195-
expect(apiRequestAuthMock).toHaveBeenCalledTimes(1);
195+
expect(apiRequestAuthMock).toHaveBeenCalledTimes(2);
196196
expect(apiRequestAuthMock).toHaveBeenCalledWith(
197197
'https://api.github.com/notifications',
198198
'HEAD',
199199
'123-456'
200200
);
201+
expect(apiRequestAuthMock).toHaveBeenCalledWith(
202+
'https://api.github.com/user',
203+
'GET',
204+
'123-456'
205+
);
201206
});
202207
});
203208

@@ -240,7 +245,7 @@ describe('context/App.tsx', () => {
240245

241246
expect(saveStateMock).toHaveBeenCalled();
242247
expect(saveStateMock).toHaveBeenCalledWith(
243-
{ enterpriseAccounts: [], token: null },
248+
{ enterpriseAccounts: [], token: null, user: null },
244249
{
245250
appearance: 'SYSTEM',
246251
markOnClick: false,
@@ -276,7 +281,7 @@ describe('context/App.tsx', () => {
276281
expect(setAutoLaunchMock).toHaveBeenCalledWith(true);
277282
expect(saveStateMock).toHaveBeenCalled();
278283
expect(saveStateMock).toHaveBeenCalledWith(
279-
{ enterpriseAccounts: [], token: null },
284+
{ enterpriseAccounts: [], token: null, user: null },
280285
{
281286
appearance: 'SYSTEM',
282287
markOnClick: false,

‎src/context/App.tsx

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,18 @@ import {
1515
SettingsState,
1616
} from '../types';
1717
import { apiRequestAuth } from '../utils/api-requests';
18-
import { addAccount, authGitHub, getToken } from '../utils/auth';
18+
import { addAccount, authGitHub, getToken, getUserData } from '../utils/auth';
1919
import { clearState, loadState, saveState } from '../utils/storage';
2020
import { setAppearance } from '../utils/appearance';
2121
import { setAutoLaunch } from '../utils/comms';
2222
import { useInterval } from '../hooks/useInterval';
2323
import { useNotifications } from '../hooks/useNotifications';
24+
import Constants from '../utils/constants';
2425

2526
const defaultAccounts: AuthState = {
2627
token: null,
2728
enterpriseAccounts: [],
29+
user: null,
2830
};
2931

3032
export const defaultSettings: SettingsState = {
@@ -111,8 +113,11 @@ export const AppProvider = ({ children }: { children: React.ReactNode }) => {
111113
const login = useCallback(async () => {
112114
const { authCode } = await authGitHub();
113115
const { token } = await getToken(authCode);
114-
setAccounts({ ...accounts, token });
115-
saveState({ ...accounts, token }, settings);
116+
const user = await getUserData(token);
117+
const hostname = Constants.DEFAULT_AUTH_OPTIONS.hostname;
118+
const updatedAccounts = addAccount(accounts, token, hostname, user);
119+
setAccounts(updatedAccounts);
120+
saveState(updatedAccounts, settings);
116121
}, [accounts, settings]);
117122

118123
const loginEnterprise = useCallback(
@@ -133,7 +138,8 @@ export const AppProvider = ({ children }: { children: React.ReactNode }) => {
133138
'HEAD',
134139
token
135140
);
136-
const updatedAccounts = addAccount(accounts, token, hostname);
141+
const user = await getUserData(token);
142+
const updatedAccounts = addAccount(accounts, token, hostname, user);
137143
setAccounts(updatedAccounts);
138144
saveState(updatedAccounts, settings);
139145
},

‎src/hooks/useNotifications.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { act, renderHook } from '@testing-library/react-hooks';
55
import { mockAccounts, mockSettings } from '../__mocks__/mock-state';
66
import { useNotifications } from './useNotifications';
77
import { AuthState } from '../types';
8+
import { mockedUser } from '../__mocks__/mockedData';
89

910
describe('hooks/useNotifications.ts', () => {
1011
beforeEach(() => {
@@ -135,6 +136,7 @@ describe('hooks/useNotifications.ts', () => {
135136
const accounts: AuthState = {
136137
...mockAccounts,
137138
enterpriseAccounts: [],
139+
user: mockedUser,
138140
};
139141

140142
const notifications = [

‎src/hooks/useNotifications.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,12 @@ export const useNotifications = (): NotificationsState => {
9696
]
9797
: [...enterpriseNotifications];
9898

99-
triggerNativeNotifications(notifications, data, settings);
99+
triggerNativeNotifications(
100+
notifications,
101+
data,
102+
settings,
103+
accounts.user
104+
);
100105
setNotifications(data);
101106
setIsFetching(false);
102107
})

‎src/routes/LoginEnterprise.test.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ describe('routes/LoginEnterprise.js', () => {
1818

1919
const mockAccounts: AuthState = {
2020
enterpriseAccounts: [],
21+
user: null,
2122
};
2223

2324
beforeEach(function () {
@@ -93,6 +94,7 @@ describe('routes/LoginEnterprise.js', () => {
9394
value={{
9495
accounts: {
9596
enterpriseAccounts: mockedEnterpriseAccounts,
97+
user: null,
9698
},
9799
}}
98100
>

‎src/routes/LoginWithToken.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,11 @@ export const LoginWithToken: React.FC = () => {
6767
>
6868
personal access tokens
6969
</a>{' '}
70-
and create one with the <strong>notifications</strong> scope.
70+
and create one with the{' '}
71+
<span className="underline font-extrabold text-yellow-500">
72+
{Constants.AUTH_SCOPE.join(', ')}{' '}
73+
</span>
74+
scopes.
7175
</>
7276
}
7377
/>
@@ -81,7 +85,7 @@ export const LoginWithToken: React.FC = () => {
8185

8286
{!isValidToken && (
8387
<div className="mt-4 text-red-500 text-sm font-medium">
84-
This token could not get validated with {values.hostname}.
88+
This token could not be validated with {values.hostname}.
8589
</div>
8690
)}
8791

‎src/routes/__snapshots__/LoginWithToken.test.tsx.snap

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,15 @@ exports[`routes/LoginWithToken.js renders correctly 1`] = `
7979
personal access tokens
8080
</a>
8181
82-
and create one with the
83-
<strong>
84-
notifications
85-
</strong>
86-
scope.
82+
and create one with the
83+
84+
<span
85+
className="underline font-extrabold text-yellow-500"
86+
>
87+
read:user, notifications
88+
89+
</span>
90+
scopes.
8791
</div>
8892
</div>
8993
<div

‎src/types.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
import { Notification } from './typesGithub';
1+
import { Notification, User } from './typesGithub';
22

33
export interface AuthState {
44
token?: string;
55
enterpriseAccounts: EnterpriseAccount[];
6+
user: User | null;
67
}
78

89
export interface SettingsState {

‎src/typesGithub.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ export interface Notification {
3333
subscription_url: string;
3434
}
3535

36+
export interface User {
37+
login: string;
38+
name: string;
39+
id: number;
40+
}
41+
3642
export interface Repository {
3743
id: number;
3844
node_id: string;

‎src/utils/auth.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ describe('utils/auth.tsx', () => {
3535

3636
expect(loadURLMock).toHaveBeenCalledTimes(1);
3737
expect(loadURLMock).toHaveBeenCalledWith(
38-
'https://github.com/login/oauth/authorize?client_id=FAKE_CLIENT_ID_123&scope=user:email,notifications'
38+
'https://github.com/login/oauth/authorize?client_id=FAKE_CLIENT_ID_123&scope=read:user,notifications'
3939
);
4040

4141
expect(new BrowserWindow().destroy).toHaveBeenCalledTimes(1);
@@ -103,6 +103,7 @@ describe('utils/auth.tsx', () => {
103103
const accounts: AuthState = {
104104
token: null,
105105
enterpriseAccounts: [],
106+
user: null,
106107
};
107108

108109
it('should add a github.com accont', async () => {

‎src/utils/auth.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
const { remote } = require('electron');
22
const BrowserWindow = remote.BrowserWindow;
33

4-
import { apiRequest } from '../utils/api-requests';
4+
import { apiRequest, apiRequestAuth } from '../utils/api-requests';
55
import { AuthResponse, AuthState, AuthTokenResponse } from '../types';
66
import { Constants } from '../utils/constants';
7+
import { User } from '../typesGithub';
78

89
export const authGitHub = (
910
authOptions = Constants.DEFAULT_AUTH_OPTIONS
@@ -67,6 +68,20 @@ export const authGitHub = (
6768
});
6869
};
6970

71+
export const getUserData = async (token: string): Promise<User> => {
72+
const response = await apiRequestAuth(
73+
`https://api.${Constants.DEFAULT_AUTH_OPTIONS.hostname}/user`,
74+
'GET',
75+
token
76+
);
77+
78+
return {
79+
id: response.data.id,
80+
login: response.data.login,
81+
name: response.data.name,
82+
};
83+
};
84+
7085
export const getToken = async (
7186
authCode: string,
7287
authOptions = Constants.DEFAULT_AUTH_OPTIONS
@@ -85,11 +100,17 @@ export const getToken = async (
85100
};
86101
};
87102

88-
export const addAccount = (accounts: AuthState, token, hostname): AuthState => {
103+
export const addAccount = (
104+
accounts: AuthState,
105+
token,
106+
hostname,
107+
user?: User
108+
): AuthState => {
89109
if (hostname === Constants.DEFAULT_AUTH_OPTIONS.hostname) {
90110
return {
91111
...accounts,
92112
token,
113+
user: user ?? null,
93114
};
94115
}
95116

‎src/utils/constants.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
export const Constants = {
22
// GitHub OAuth
3-
AUTH_SCOPE: ['user:email', 'notifications'],
3+
AUTH_SCOPE: ['read:user', 'notifications'],
44

55
DEFAULT_AUTH_OPTIONS: {
66
hostname: 'github.com',

0 commit comments

Comments
 (0)