Skip to content

Commit b4eecd2

Browse files
committed
fix discussions-url in native notification
1 parent fce57b6 commit b4eecd2

File tree

3 files changed

+28
-16
lines changed

3 files changed

+28
-16
lines changed

src/hooks/useNotifications.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ export const useNotifications = (): NotificationsState => {
100100
notifications,
101101
data,
102102
settings,
103-
accounts.user
103+
accounts
104104
);
105105
setNotifications(data);
106106
setIsFetching(false);

src/utils/notifications.test.ts

+7-6
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
mockedSingleAccountNotifications,
88
mockedUser,
99
} from '../__mocks__/mockedData';
10+
import { mockAccounts } from '../__mocks__/mock-state';
1011
import * as comms from './comms';
1112
import * as notificationsHelpers from './notifications';
1213
import { SettingsState } from '../types';
@@ -27,7 +28,7 @@ describe('utils/notifications.ts', () => {
2728
[],
2829
mockedAccountNotifications,
2930
settings,
30-
mockedUser
31+
mockAccounts
3132
);
3233

3334
expect(notificationsHelpers.raiseNativeNotification).toHaveBeenCalledTimes(
@@ -52,7 +53,7 @@ describe('utils/notifications.ts', () => {
5253
[],
5354
mockedAccountNotifications,
5455
settings,
55-
mockedUser
56+
mockAccounts
5657
);
5758

5859
expect(notificationsHelpers.raiseNativeNotification).not.toHaveBeenCalled();
@@ -73,7 +74,7 @@ describe('utils/notifications.ts', () => {
7374
mockedSingleAccountNotifications,
7475
mockedSingleAccountNotifications,
7576
settings,
76-
mockedUser
77+
mockAccounts
7778
);
7879

7980
expect(notificationsHelpers.raiseNativeNotification).not.toHaveBeenCalled();
@@ -94,7 +95,7 @@ describe('utils/notifications.ts', () => {
9495
[],
9596
[],
9697
settings,
97-
mockedUser
98+
mockAccounts
9899
);
99100

100101
expect(notificationsHelpers.raiseNativeNotification).not.toHaveBeenCalled();
@@ -106,7 +107,7 @@ describe('utils/notifications.ts', () => {
106107

107108
const nativeNotification: Notification = notificationsHelpers.raiseNativeNotification(
108109
[mockedGithubNotifications[0]],
109-
mockedUser.id
110+
mockAccounts
110111
);
111112
nativeNotification.onclick(null);
112113

@@ -125,7 +126,7 @@ describe('utils/notifications.ts', () => {
125126

126127
const nativeNotification = notificationsHelpers.raiseNativeNotification(
127128
mockedGithubNotifications,
128-
mockedUser.id
129+
mockAccounts
129130
);
130131
nativeNotification.onclick(null);
131132

src/utils/notifications.ts

+20-9
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
const { remote } = require('electron');
1+
const { remote, shell } = require('electron');
22

3-
import { generateGitHubWebUrl } from './helpers';
3+
import { generateGitHubWebUrl, getDiscussionUrl } from './helpers';
44
import { reOpenWindow, openExternalLink, updateTrayIcon } from './comms';
5-
import { Notification, User } from '../typesGithub';
5+
import { Notification } from '../typesGithub';
66

7-
import { AccountNotifications, SettingsState } from '../types';
7+
import { AccountNotifications, SettingsState, AuthState } from '../types';
88

99
export const setTrayIconColor = (notifications: AccountNotifications[]) => {
1010
const allNotificationsCount = notifications.reduce(
@@ -19,7 +19,7 @@ export const triggerNativeNotifications = (
1919
previousNotifications: AccountNotifications[],
2020
newNotifications: AccountNotifications[],
2121
settings: SettingsState,
22-
user: User
22+
accounts: AuthState
2323
) => {
2424
const diffNotifications = newNotifications
2525
.map((account) => {
@@ -55,13 +55,13 @@ export const triggerNativeNotifications = (
5555
}
5656

5757
if (settings.showNotifications) {
58-
raiseNativeNotification(diffNotifications, user?.id);
58+
raiseNativeNotification(diffNotifications, accounts);
5959
}
6060
};
6161

6262
export const raiseNativeNotification = (
6363
notifications: Notification[],
64-
userId?: number
64+
accounts: AuthState
6565
) => {
6666
let title: string;
6767
let body: string;
@@ -87,11 +87,22 @@ export const raiseNativeNotification = (
8787
const appWindow = remote.getCurrentWindow();
8888
appWindow.hide();
8989

90+
const { subject, id, repository } = notifications[0];
91+
9092
// Some Notification types from GitHub are missing urls in their subjects.
9193
if (notificationUrl) {
92-
const { subject, id } = notifications[0];
93-
const url = generateGitHubWebUrl(subject.url, id, userId);
94+
const url = generateGitHubWebUrl(subject.url, id, accounts.user?.id);
9495
openExternalLink(url);
96+
} else if (notifications[0].subject.type === 'Discussion') {
97+
getDiscussionUrl(notifications[0], accounts.token).then(url =>
98+
shell.openExternal(
99+
generateGitHubWebUrl(
100+
url || `${repository.url}/discussions`,
101+
id,
102+
accounts.user?.id
103+
)
104+
)
105+
);
95106
}
96107
} else {
97108
reOpenWindow();

0 commit comments

Comments
 (0)