Skip to content

Commit ec5d189

Browse files
lszomorulszomoru
authored
Notifications - improve paging (#6438)
* Initial refactoring * Make sure that only the necessary data is fetched when sorting * Renames * Reset the dateTime when refreshing
1 parent 09520fb commit ec5d189

6 files changed

Lines changed: 156 additions & 207 deletions

‎src/notifications/notificationDecorationProvider.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@ import * as vscode from 'vscode';
77
import { EXPERIMENTAL_NOTIFICATIONS_SCORE, PR_SETTINGS_NAMESPACE } from '../common/settingKeys';
88
import { fromNotificationUri, toNotificationUri } from '../common/uri';
99
import { dispose } from '../common/utils';
10-
import { NotificationsSortMethod } from './notificationItem';
11-
import { NotificationsManager } from './notificationsManager';
10+
import { NotificationsManager, NotificationsSortMethod } from './notificationsManager';
1211

1312
export class NotificationsDecorationProvider implements vscode.FileDecorationProvider {
1413
private _readonlyOnDidChangeFileDecorations: vscode.EventEmitter<vscode.Uri[]> = new vscode.EventEmitter<vscode.Uri[]>();

‎src/notifications/notificationItem.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,6 @@
66
import { Notification } from '../github/interface';
77
import { IssueModel } from '../github/issueModel';
88

9-
export enum NotificationsSortMethod {
10-
Timestamp = 'Timestamp',
11-
Priority = 'Priority'
12-
}
13-
149
export type NotificationTreeDataItem = NotificationTreeItem | LoadMoreNotificationsTreeItem;
1510

1611
export interface LoadMoreNotificationsTreeItem {

‎src/notifications/notificationsFeatureRegistar.ts‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,9 @@ import { CredentialStore } from '../github/credentials';
1010
import { RepositoriesManager } from '../github/repositoriesManager';
1111
import { chatCommand } from '../lm/utils';
1212
import { NotificationsDecorationProvider } from './notificationDecorationProvider';
13-
import { isNotificationTreeItem, NotificationsSortMethod, NotificationTreeDataItem } from './notificationItem';
14-
import { NotificationsManager } from './notificationsManager';
13+
import { isNotificationTreeItem, NotificationTreeDataItem } from './notificationItem';
14+
import { NotificationsManager, NotificationsSortMethod } from './notificationsManager';
1515
import { NotificationsProvider } from './notificationsProvider';
16-
import { NotificationsTreeData } from './notificationsView';
1716

1817
export class NotificationsFeatureRegister implements vscode.Disposable {
1918

@@ -35,10 +34,8 @@ export class NotificationsFeatureRegister implements vscode.Disposable {
3534
this._disposables.push(vscode.window.registerFileDecorationProvider(decorationsProvider));
3635

3736
// View
38-
const dataProvider = new NotificationsTreeData(notificationsManager);
39-
this._disposables.push(dataProvider);
4037
const view = vscode.window.createTreeView<any>('notifications:github', {
41-
treeDataProvider: dataProvider
38+
treeDataProvider: notificationsManager
4239
});
4340
this._disposables.push(view);
4441

@@ -51,7 +48,7 @@ export class NotificationsFeatureRegister implements vscode.Disposable {
5148
"notifications.sortByTimestamp" : {}
5249
*/
5350
this._telemetry.sendTelemetryEvent('notifications.sortByTimestamp');
54-
notificationsManager.sortingMethod = NotificationsSortMethod.Timestamp;
51+
notificationsManager.sortNotifications(NotificationsSortMethod.Timestamp);
5552
},
5653
this,
5754
),
@@ -64,7 +61,7 @@ export class NotificationsFeatureRegister implements vscode.Disposable {
6461
"notifications.sortByTimestamp" : {}
6562
*/
6663
this._telemetry.sendTelemetryEvent('notifications.sortByTimestamp');
67-
notificationsManager.sortingMethod = NotificationsSortMethod.Priority;
64+
notificationsManager.sortNotifications(NotificationsSortMethod.Priority);
6865
},
6966
this,
7067
),
@@ -77,8 +74,7 @@ export class NotificationsFeatureRegister implements vscode.Disposable {
7774
"notifications.refresh" : {}
7875
*/
7976
this._telemetry.sendTelemetryEvent('notifications.refresh');
80-
notificationsManager.clear();
81-
dataProvider.refresh(true);
77+
notificationsManager.refresh();
8278
},
8379
this,
8480
),
@@ -89,7 +85,7 @@ export class NotificationsFeatureRegister implements vscode.Disposable {
8985
"notifications.loadMore" : {}
9086
*/
9187
this._telemetry.sendTelemetryEvent('notifications.loadMore');
92-
dataProvider.loadMore();
88+
notificationsManager.loadMore();
9389
})
9490
);
9591
this._disposables.push(
@@ -121,14 +117,13 @@ export class NotificationsFeatureRegister implements vscode.Disposable {
121117
"notification.markAsRead" : {}
122118
*/
123119
this._telemetry.sendTelemetryEvent('notification.markAsRead');
124-
dataProvider.markAsRead({ threadId, notificationKey });
120+
notificationsManager.markAsRead({ threadId, notificationKey });
125121
})
126122
);
127123

128124
// Events
129125
onceEvent(this._repositoriesManager.onDidLoadAnyRepositories)(() => {
130-
notificationsManager.clear();
131-
dataProvider.refresh(true);
126+
notificationsManager.refresh();
132127
}, this, this._disposables);
133128
}
134129

‎src/notifications/notificationsManager.ts‎

Lines changed: 143 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -4,114 +4,166 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import * as vscode from 'vscode';
7+
import { toNotificationUri } from '../common/uri';
78
import { dispose } from '../common/utils';
8-
import { NotificationsSortMethod, NotificationTreeItem } from './notificationItem';
9+
import { NotificationSubjectType } from '../github/interface';
10+
import { IssueModel } from '../github/issueModel';
11+
import { PullRequestModel } from '../github/pullRequestModel';
12+
import { isNotificationTreeItem, NotificationTreeDataItem, NotificationTreeItem } from './notificationItem';
913
import { NotificationsProvider } from './notificationsProvider';
1014

1115
export interface INotificationTreeItems {
1216
readonly notifications: NotificationTreeItem[];
1317
readonly hasNextPage: boolean
1418
}
1519

16-
export class NotificationsManager {
17-
private readonly _onDidChangeNotifications = new vscode.EventEmitter<NotificationTreeItem[]>();
18-
readonly onDidChangeNotifications = this._onDidChangeNotifications.event;
19-
20-
private _sortingMethod: NotificationsSortMethod = NotificationsSortMethod.Timestamp;
21-
public get sortingMethod(): NotificationsSortMethod { return this._sortingMethod; }
22-
public set sortingMethod(value: NotificationsSortMethod) {
23-
if (this._sortingMethod === value) {
24-
return;
25-
}
20+
export enum NotificationsSortMethod {
21+
Timestamp = 'Timestamp',
22+
Priority = 'Priority'
23+
}
2624

27-
this._sortingMethod = value;
28-
this._onDidChangeSortingMethod.fire();
29-
}
25+
export class NotificationsManager implements vscode.TreeDataProvider<NotificationTreeDataItem>, vscode.Disposable {
26+
private _onDidChangeTreeData: vscode.EventEmitter<NotificationTreeDataItem | undefined | void> = new vscode.EventEmitter<NotificationTreeDataItem | undefined | void>();
27+
readonly onDidChangeTreeData: vscode.Event<NotificationTreeDataItem | undefined | void> = this._onDidChangeTreeData.event;
3028

31-
private readonly _onDidChangeSortingMethod = new vscode.EventEmitter<void>();
32-
readonly onDidChangeSortingMethod = this._onDidChangeSortingMethod.event;
29+
private readonly _onDidChangeNotifications = new vscode.EventEmitter<NotificationTreeItem[]>();
30+
readonly onDidChangeNotifications = this._onDidChangeNotifications.event;
3331

32+
private _pageCount: number = 1;
3433
private _hasNextPage: boolean = false;
34+
private _dateTime: Date = new Date();
35+
private _fetchNotifications: boolean = false;
3536
private _notifications = new Map<string, NotificationTreeItem>();
3637

38+
private _sortingMethod: NotificationsSortMethod = NotificationsSortMethod.Timestamp;
39+
get sortingMethod(): NotificationsSortMethod { return this._sortingMethod; }
40+
3741
private readonly _disposable: vscode.Disposable[] = [];
3842

3943
constructor(private readonly _notificationProvider: NotificationsProvider) {
44+
this._disposable.push(this._onDidChangeTreeData);
4045
this._disposable.push(this._onDidChangeNotifications);
4146
}
4247

4348
dispose() {
4449
dispose(this._disposable);
4550
}
4651

47-
public clear() {
48-
if (this._notifications.size === 0) {
49-
return;
52+
//#region TreeDataProvider
53+
54+
async getChildren(element?: unknown): Promise<NotificationTreeDataItem[] | undefined> {
55+
if (element !== undefined) {
56+
return undefined;
5057
}
51-
const updates = Array.from(this._notifications.values());
52-
this._notifications.clear();
53-
this._onDidChangeNotifications.fire(updates);
54-
}
5558

56-
public async getNotifications(compute: boolean, pageCount: number): Promise<INotificationTreeItems | undefined> {
57-
if (!compute) {
58-
const notifications = Array.from(this._notifications.values());
59+
const notificationsData = await this.getNotifications();
60+
if (notificationsData === undefined) {
61+
return undefined;
62+
}
5963

60-
return {
61-
notifications: this._sortNotifications(notifications),
62-
hasNextPage: this._hasNextPage
63-
};
64+
if (notificationsData.hasNextPage) {
65+
return [...notificationsData.notifications, { kind: 'loadMoreNotifications' }];
6466
}
6567

66-
// Get raw notifications
67-
const notificationsData = await this._notificationProvider.computeNotifications(pageCount);
68-
if (!notificationsData) {
69-
return undefined;
68+
return notificationsData.notifications;
69+
}
70+
71+
async getTreeItem(element: NotificationTreeDataItem): Promise<vscode.TreeItem> {
72+
if (isNotificationTreeItem(element)) {
73+
return this._resolveNotificationTreeItem(element);
7074
}
75+
return this._resolveLoadMoreNotificationsTreeItem();
76+
}
77+
78+
private _resolveNotificationTreeItem(element: NotificationTreeItem): vscode.TreeItem {
79+
const label = element.notification.subject.title;
80+
const item = new vscode.TreeItem(label, vscode.TreeItemCollapsibleState.None);
81+
const notification = element.notification;
82+
const model = element.model;
83+
84+
if (notification.subject.type === NotificationSubjectType.Issue && model instanceof IssueModel) {
85+
item.iconPath = element.model.isOpen
86+
? new vscode.ThemeIcon('issues', new vscode.ThemeColor('issues.open'))
87+
: new vscode.ThemeIcon('issue-closed', new vscode.ThemeColor('issues.closed'));
88+
}
89+
if (notification.subject.type === NotificationSubjectType.PullRequest && model instanceof PullRequestModel) {
90+
item.iconPath = model.isOpen
91+
? new vscode.ThemeIcon('git-pull-request', new vscode.ThemeColor('pullRequests.open'))
92+
: new vscode.ThemeIcon('git-pull-request', new vscode.ThemeColor('pullRequests.merged'));
93+
}
94+
item.description = `${notification.owner}/${notification.name}`;
95+
item.contextValue = notification.subject.type;
96+
item.resourceUri = toNotificationUri({ key: element.notification.key });
97+
item.command = {
98+
command: 'notification.chatSummarizeNotification',
99+
title: 'Summarize Notification',
100+
arguments: [element]
101+
};
102+
return item;
103+
}
104+
105+
private _resolveLoadMoreNotificationsTreeItem(): vscode.TreeItem {
106+
const item = new vscode.TreeItem(vscode.l10n.t('Load More Notifications...'), vscode.TreeItemCollapsibleState.None);
107+
item.command = {
108+
title: 'Load More Notifications',
109+
command: 'notifications.loadMore'
110+
};
111+
item.contextValue = 'loadMoreNotifications';
112+
return item;
113+
}
71114

72-
// Resolve notifications
73-
const notificationItems = new Map<string, NotificationTreeItem>();
74-
await Promise.all(notificationsData.notifications.map(async notification => {
75-
const cachedNotification = this._notifications.get(notification.key);
76-
if (cachedNotification && cachedNotification.notification.updatedAd.getTime() === notification.updatedAd.getTime()) {
77-
notificationItems.set(notification.key, cachedNotification);
78-
return;
115+
//#endregion
116+
117+
public async getNotifications(): Promise<INotificationTreeItems | undefined> {
118+
if (this._fetchNotifications) {
119+
// Get raw notifications
120+
const notificationsData = await this._notificationProvider.getNotifications(this._dateTime.toISOString(), this._pageCount);
121+
if (!notificationsData) {
122+
return undefined;
79123
}
80124

81-
const model = await this._notificationProvider.getNotificationModel(notification);
82-
if (!model) {
83-
return;
125+
// Resolve notifications
126+
const notificationTreeItems = new Map<string, NotificationTreeItem>();
127+
await Promise.all(notificationsData.notifications.map(async notification => {
128+
const model = await this._notificationProvider.getNotificationModel(notification);
129+
if (!model) {
130+
return;
131+
}
132+
133+
notificationTreeItems.set(notification.key, {
134+
notification, model, kind: 'notification'
135+
});
136+
}));
137+
138+
for (const [key, value] of notificationTreeItems.entries()) {
139+
this._notifications.set(key, value);
84140
}
141+
this._hasNextPage = notificationsData.hasNextPage;
85142

86-
notificationItems.set(notification.key, {
87-
notification, model, kind: 'notification'
88-
});
89-
}));
143+
this._fetchNotifications = false;
144+
}
90145

91146
// Calculate notification priority
92-
if (this.sortingMethod === NotificationsSortMethod.Priority) {
93-
const notificationsWithoutPriority = Array.from(notificationItems.values())
147+
if (this._sortingMethod === NotificationsSortMethod.Priority) {
148+
const notificationsWithoutPriority = Array.from(this._notifications.values())
94149
.filter(notification => notification.priority === undefined);
95150

96151
const notificationPriorities = await this._notificationProvider
97152
.getNotificationsPriority(notificationsWithoutPriority);
98153

99154
for (const { key, priority, priorityReasoning } of notificationPriorities) {
100-
const notification = notificationItems.get(key);
155+
const notification = this._notifications.get(key);
101156
if (!notification) {
102157
continue;
103158
}
104159

105160
notification.priority = priority;
106161
notification.priorityReason = priorityReasoning;
107162

108-
notificationItems.set(key, notification);
163+
this._notifications.set(key, notification);
109164
}
110165
}
111166

112-
this._notifications = notificationItems;
113-
this._hasNextPage = notificationsData.hasNextPage;
114-
115167
const notifications = Array.from(this._notifications.values());
116168
this._onDidChangeNotifications.fire(notifications);
117169

@@ -129,16 +181,50 @@ export class NotificationsManager {
129181
return Array.from(this._notifications.values());
130182
}
131183

132-
public async markAsRead(notificationIdentifier: { threadId: string, notificationKey: string }): Promise<void> {
133-
await this._notificationProvider.markAsRead(notificationIdentifier);
184+
public refresh(): void {
185+
if (this._notifications.size !== 0) {
186+
const updates = Array.from(this._notifications.values());
187+
this._onDidChangeNotifications.fire(updates);
188+
}
189+
190+
this._pageCount = 1;
191+
this._dateTime = new Date();
192+
this._notifications.clear();
134193

194+
this._refresh(true);
195+
}
196+
197+
public loadMore(): void {
198+
this._pageCount++;
199+
this._refresh(true);
200+
}
201+
202+
public _refresh(fetch: boolean): void {
203+
this._fetchNotifications = fetch;
204+
this._onDidChangeTreeData.fire();
205+
}
206+
207+
public async markAsRead(notificationIdentifier: { threadId: string, notificationKey: string }): Promise<void> {
135208
const notification = this._notifications.get(notificationIdentifier.notificationKey);
136209
if (notification) {
210+
await this._notificationProvider.markAsRead(notificationIdentifier);
211+
137212
this._onDidChangeNotifications.fire([notification]);
138213
this._notifications.delete(notificationIdentifier.notificationKey);
214+
215+
this._refresh(false);
139216
}
140217
}
141218

219+
public sortNotifications(method: NotificationsSortMethod): void {
220+
if (this._sortingMethod === method) {
221+
return;
222+
}
223+
224+
this._sortingMethod = method;
225+
this._refresh(false);
226+
}
227+
142228
private _sortNotifications(notifications: NotificationTreeItem[]): NotificationTreeItem[] {
143229
if (this._sortingMethod === NotificationsSortMethod.Timestamp) {
144230
return notifications.sort((n1, n2) => n2.notification.updatedAd.getTime() - n1.notification.updatedAd.getTime());

0 commit comments

Comments
 (0)