Skip to content

Commit

Permalink
feat: use notification_referrer_id for better UX
Browse files Browse the repository at this point in the history
  • Loading branch information
codebytere committed Oct 4, 2021
1 parent 75eb1c8 commit a960842
Show file tree
Hide file tree
Showing 20 changed files with 254 additions and 88 deletions.
2 changes: 2 additions & 0 deletions src/__mocks__/mock-state.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Appearance, AuthState, SettingsState } from '../types';
import { mockedUser } from './mockedData';

export const mockAccounts: AuthState = {
token: 'token-123-456',
Expand All @@ -8,6 +9,7 @@ export const mockAccounts: AuthState = {
hostname: 'github.gitify.io',
},
],
user: mockedUser,
};

export const mockSettings: SettingsState = {
Expand Down
8 changes: 7 additions & 1 deletion src/__mocks__/mockedData.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { AccountNotifications, EnterpriseAccount } from '../types';
import { Notification, Repository } from '../typesGithub';
import { Notification, Repository, User } from '../typesGithub';

export const mockedEnterpriseAccounts: EnterpriseAccount[] = [
{
Expand All @@ -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',
Expand Down
9 changes: 7 additions & 2 deletions src/components/NotificationRow.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -39,6 +39,7 @@ describe('components/Notification.js', () => {
value={{
settings: { ...mockSettings, markOnClick: true },
markNotification,
accounts: mockAccounts,
}}
>
<NotificationRow {...props} />
Expand All @@ -62,6 +63,7 @@ describe('components/Notification.js', () => {
value={{
settings: { ...mockSettings, markOnClick: true },
markNotification,
accounts: mockAccounts,
}}
>
<NotificationRow {...props} />
Expand All @@ -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} />
Expand Down
8 changes: 6 additions & 2 deletions src/components/NotificationRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand All @@ -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]);
Expand Down
17 changes: 11 additions & 6 deletions src/context/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'
);
Expand All @@ -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'
);
Expand Down Expand Up @@ -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'
);
Expand Down Expand Up @@ -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'
);
});
});

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
14 changes: 10 additions & 4 deletions src/context/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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(
Expand All @@ -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);
},
Expand Down
2 changes: 2 additions & 0 deletions src/hooks/useNotifications.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -135,6 +136,7 @@ describe('hooks/useNotifications.ts', () => {
const accounts: AuthState = {
...mockAccounts,
enterpriseAccounts: [],
user: mockedUser,
};

const notifications = [
Expand Down
7 changes: 6 additions & 1 deletion src/hooks/useNotifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,12 @@ export const useNotifications = (): NotificationsState => {
]
: [...enterpriseNotifications];

triggerNativeNotifications(notifications, data, settings);
triggerNativeNotifications(
notifications,
data,
settings,
accounts.user
);
setNotifications(data);
setIsFetching(false);
})
Expand Down
2 changes: 2 additions & 0 deletions src/routes/LoginEnterprise.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ describe('routes/LoginEnterprise.js', () => {

const mockAccounts: AuthState = {
enterpriseAccounts: [],
user: null,
};

beforeEach(function () {
Expand Down Expand Up @@ -93,6 +94,7 @@ describe('routes/LoginEnterprise.js', () => {
value={{
accounts: {
enterpriseAccounts: mockedEnterpriseAccounts,
user: null,
},
}}
>
Expand Down
2 changes: 1 addition & 1 deletion src/routes/LoginWithToken.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,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>
)}

Expand Down
3 changes: 2 additions & 1 deletion src/types.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { Notification } from './typesGithub';
import { Notification, User } from './typesGithub';

export interface AuthState {
token?: string;
enterpriseAccounts: EnterpriseAccount[];
user: User;
}

export interface SettingsState {
Expand Down
6 changes: 6 additions & 0 deletions src/typesGithub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion src/utils/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -103,6 +103,7 @@ describe('utils/auth.tsx', () => {
const accounts: AuthState = {
token: null,
enterpriseAccounts: [],
user: null,
};

it('should add a github.com accont', async () => {
Expand Down
25 changes: 23 additions & 2 deletions src/utils/auth.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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,
};
}

Expand Down
2 changes: 1 addition & 1 deletion src/utils/constants.ts
Original file line number Diff line number Diff line change
@@ -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',
Expand Down
Loading

0 comments on commit a960842

Please sign in to comment.