Skip to content

Commit

Permalink
Notifications - improve paging (#6438)
Browse files Browse the repository at this point in the history
* Initial refactoring

* Make sure that only the necessary data is fetched when sorting

* Renames

* Reset the dateTime when refreshing
  • Loading branch information
lszomoru authored Oct 29, 2024
1 parent 09520fb commit ec5d189
Show file tree
Hide file tree
Showing 6 changed files with 156 additions and 207 deletions.
3 changes: 1 addition & 2 deletions src/notifications/notificationDecorationProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ import * as vscode from 'vscode';
import { EXPERIMENTAL_NOTIFICATIONS_SCORE, PR_SETTINGS_NAMESPACE } from '../common/settingKeys';
import { fromNotificationUri, toNotificationUri } from '../common/uri';
import { dispose } from '../common/utils';
import { NotificationsSortMethod } from './notificationItem';
import { NotificationsManager } from './notificationsManager';
import { NotificationsManager, NotificationsSortMethod } from './notificationsManager';

export class NotificationsDecorationProvider implements vscode.FileDecorationProvider {
private _readonlyOnDidChangeFileDecorations: vscode.EventEmitter<vscode.Uri[]> = new vscode.EventEmitter<vscode.Uri[]>();
Expand Down
5 changes: 0 additions & 5 deletions src/notifications/notificationItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,6 @@
import { Notification } from '../github/interface';
import { IssueModel } from '../github/issueModel';

export enum NotificationsSortMethod {
Timestamp = 'Timestamp',
Priority = 'Priority'
}

export type NotificationTreeDataItem = NotificationTreeItem | LoadMoreNotificationsTreeItem;

export interface LoadMoreNotificationsTreeItem {
Expand Down
23 changes: 9 additions & 14 deletions src/notifications/notificationsFeatureRegistar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,9 @@ import { CredentialStore } from '../github/credentials';
import { RepositoriesManager } from '../github/repositoriesManager';
import { chatCommand } from '../lm/utils';
import { NotificationsDecorationProvider } from './notificationDecorationProvider';
import { isNotificationTreeItem, NotificationsSortMethod, NotificationTreeDataItem } from './notificationItem';
import { NotificationsManager } from './notificationsManager';
import { isNotificationTreeItem, NotificationTreeDataItem } from './notificationItem';
import { NotificationsManager, NotificationsSortMethod } from './notificationsManager';
import { NotificationsProvider } from './notificationsProvider';
import { NotificationsTreeData } from './notificationsView';

export class NotificationsFeatureRegister implements vscode.Disposable {

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

// View
const dataProvider = new NotificationsTreeData(notificationsManager);
this._disposables.push(dataProvider);
const view = vscode.window.createTreeView<any>('notifications:github', {
treeDataProvider: dataProvider
treeDataProvider: notificationsManager
});
this._disposables.push(view);

Expand All @@ -51,7 +48,7 @@ export class NotificationsFeatureRegister implements vscode.Disposable {
"notifications.sortByTimestamp" : {}
*/
this._telemetry.sendTelemetryEvent('notifications.sortByTimestamp');
notificationsManager.sortingMethod = NotificationsSortMethod.Timestamp;
notificationsManager.sortNotifications(NotificationsSortMethod.Timestamp);
},
this,
),
Expand All @@ -64,7 +61,7 @@ export class NotificationsFeatureRegister implements vscode.Disposable {
"notifications.sortByTimestamp" : {}
*/
this._telemetry.sendTelemetryEvent('notifications.sortByTimestamp');
notificationsManager.sortingMethod = NotificationsSortMethod.Priority;
notificationsManager.sortNotifications(NotificationsSortMethod.Priority);
},
this,
),
Expand All @@ -77,8 +74,7 @@ export class NotificationsFeatureRegister implements vscode.Disposable {
"notifications.refresh" : {}
*/
this._telemetry.sendTelemetryEvent('notifications.refresh');
notificationsManager.clear();
dataProvider.refresh(true);
notificationsManager.refresh();
},
this,
),
Expand All @@ -89,7 +85,7 @@ export class NotificationsFeatureRegister implements vscode.Disposable {
"notifications.loadMore" : {}
*/
this._telemetry.sendTelemetryEvent('notifications.loadMore');
dataProvider.loadMore();
notificationsManager.loadMore();
})
);
this._disposables.push(
Expand Down Expand Up @@ -121,14 +117,13 @@ export class NotificationsFeatureRegister implements vscode.Disposable {
"notification.markAsRead" : {}
*/
this._telemetry.sendTelemetryEvent('notification.markAsRead');
dataProvider.markAsRead({ threadId, notificationKey });
notificationsManager.markAsRead({ threadId, notificationKey });
})
);

// Events
onceEvent(this._repositoriesManager.onDidLoadAnyRepositories)(() => {
notificationsManager.clear();
dataProvider.refresh(true);
notificationsManager.refresh();
}, this, this._disposables);
}

Expand Down
200 changes: 143 additions & 57 deletions src/notifications/notificationsManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,114 +4,166 @@
*--------------------------------------------------------------------------------------------*/

import * as vscode from 'vscode';
import { toNotificationUri } from '../common/uri';
import { dispose } from '../common/utils';
import { NotificationsSortMethod, NotificationTreeItem } from './notificationItem';
import { NotificationSubjectType } from '../github/interface';
import { IssueModel } from '../github/issueModel';
import { PullRequestModel } from '../github/pullRequestModel';
import { isNotificationTreeItem, NotificationTreeDataItem, NotificationTreeItem } from './notificationItem';
import { NotificationsProvider } from './notificationsProvider';

export interface INotificationTreeItems {
readonly notifications: NotificationTreeItem[];
readonly hasNextPage: boolean
}

export class NotificationsManager {
private readonly _onDidChangeNotifications = new vscode.EventEmitter<NotificationTreeItem[]>();
readonly onDidChangeNotifications = this._onDidChangeNotifications.event;

private _sortingMethod: NotificationsSortMethod = NotificationsSortMethod.Timestamp;
public get sortingMethod(): NotificationsSortMethod { return this._sortingMethod; }
public set sortingMethod(value: NotificationsSortMethod) {
if (this._sortingMethod === value) {
return;
}
export enum NotificationsSortMethod {
Timestamp = 'Timestamp',
Priority = 'Priority'
}

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

private readonly _onDidChangeSortingMethod = new vscode.EventEmitter<void>();
readonly onDidChangeSortingMethod = this._onDidChangeSortingMethod.event;
private readonly _onDidChangeNotifications = new vscode.EventEmitter<NotificationTreeItem[]>();
readonly onDidChangeNotifications = this._onDidChangeNotifications.event;

private _pageCount: number = 1;
private _hasNextPage: boolean = false;
private _dateTime: Date = new Date();
private _fetchNotifications: boolean = false;
private _notifications = new Map<string, NotificationTreeItem>();

private _sortingMethod: NotificationsSortMethod = NotificationsSortMethod.Timestamp;
get sortingMethod(): NotificationsSortMethod { return this._sortingMethod; }

private readonly _disposable: vscode.Disposable[] = [];

constructor(private readonly _notificationProvider: NotificationsProvider) {
this._disposable.push(this._onDidChangeTreeData);
this._disposable.push(this._onDidChangeNotifications);
}

dispose() {
dispose(this._disposable);
}

public clear() {
if (this._notifications.size === 0) {
return;
//#region TreeDataProvider

async getChildren(element?: unknown): Promise<NotificationTreeDataItem[] | undefined> {
if (element !== undefined) {
return undefined;
}
const updates = Array.from(this._notifications.values());
this._notifications.clear();
this._onDidChangeNotifications.fire(updates);
}

public async getNotifications(compute: boolean, pageCount: number): Promise<INotificationTreeItems | undefined> {
if (!compute) {
const notifications = Array.from(this._notifications.values());
const notificationsData = await this.getNotifications();
if (notificationsData === undefined) {
return undefined;
}

return {
notifications: this._sortNotifications(notifications),
hasNextPage: this._hasNextPage
};
if (notificationsData.hasNextPage) {
return [...notificationsData.notifications, { kind: 'loadMoreNotifications' }];
}

// Get raw notifications
const notificationsData = await this._notificationProvider.computeNotifications(pageCount);
if (!notificationsData) {
return undefined;
return notificationsData.notifications;
}

async getTreeItem(element: NotificationTreeDataItem): Promise<vscode.TreeItem> {
if (isNotificationTreeItem(element)) {
return this._resolveNotificationTreeItem(element);
}
return this._resolveLoadMoreNotificationsTreeItem();
}

private _resolveNotificationTreeItem(element: NotificationTreeItem): vscode.TreeItem {
const label = element.notification.subject.title;
const item = new vscode.TreeItem(label, vscode.TreeItemCollapsibleState.None);
const notification = element.notification;
const model = element.model;

if (notification.subject.type === NotificationSubjectType.Issue && model instanceof IssueModel) {
item.iconPath = element.model.isOpen
? new vscode.ThemeIcon('issues', new vscode.ThemeColor('issues.open'))
: new vscode.ThemeIcon('issue-closed', new vscode.ThemeColor('issues.closed'));
}
if (notification.subject.type === NotificationSubjectType.PullRequest && model instanceof PullRequestModel) {
item.iconPath = model.isOpen
? new vscode.ThemeIcon('git-pull-request', new vscode.ThemeColor('pullRequests.open'))
: new vscode.ThemeIcon('git-pull-request', new vscode.ThemeColor('pullRequests.merged'));
}
item.description = `${notification.owner}/${notification.name}`;
item.contextValue = notification.subject.type;
item.resourceUri = toNotificationUri({ key: element.notification.key });
item.command = {
command: 'notification.chatSummarizeNotification',
title: 'Summarize Notification',
arguments: [element]
};
return item;
}

private _resolveLoadMoreNotificationsTreeItem(): vscode.TreeItem {
const item = new vscode.TreeItem(vscode.l10n.t('Load More Notifications...'), vscode.TreeItemCollapsibleState.None);
item.command = {
title: 'Load More Notifications',
command: 'notifications.loadMore'
};
item.contextValue = 'loadMoreNotifications';
return item;
}

// Resolve notifications
const notificationItems = new Map<string, NotificationTreeItem>();
await Promise.all(notificationsData.notifications.map(async notification => {
const cachedNotification = this._notifications.get(notification.key);
if (cachedNotification && cachedNotification.notification.updatedAd.getTime() === notification.updatedAd.getTime()) {
notificationItems.set(notification.key, cachedNotification);
return;
//#endregion

public async getNotifications(): Promise<INotificationTreeItems | undefined> {
if (this._fetchNotifications) {
// Get raw notifications
const notificationsData = await this._notificationProvider.getNotifications(this._dateTime.toISOString(), this._pageCount);
if (!notificationsData) {
return undefined;
}

const model = await this._notificationProvider.getNotificationModel(notification);
if (!model) {
return;
// Resolve notifications
const notificationTreeItems = new Map<string, NotificationTreeItem>();
await Promise.all(notificationsData.notifications.map(async notification => {
const model = await this._notificationProvider.getNotificationModel(notification);
if (!model) {
return;
}

notificationTreeItems.set(notification.key, {
notification, model, kind: 'notification'
});
}));

for (const [key, value] of notificationTreeItems.entries()) {
this._notifications.set(key, value);
}
this._hasNextPage = notificationsData.hasNextPage;

notificationItems.set(notification.key, {
notification, model, kind: 'notification'
});
}));
this._fetchNotifications = false;
}

// Calculate notification priority
if (this.sortingMethod === NotificationsSortMethod.Priority) {
const notificationsWithoutPriority = Array.from(notificationItems.values())
if (this._sortingMethod === NotificationsSortMethod.Priority) {
const notificationsWithoutPriority = Array.from(this._notifications.values())
.filter(notification => notification.priority === undefined);

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

for (const { key, priority, priorityReasoning } of notificationPriorities) {
const notification = notificationItems.get(key);
const notification = this._notifications.get(key);
if (!notification) {
continue;
}

notification.priority = priority;
notification.priorityReason = priorityReasoning;

notificationItems.set(key, notification);
this._notifications.set(key, notification);
}
}

this._notifications = notificationItems;
this._hasNextPage = notificationsData.hasNextPage;

const notifications = Array.from(this._notifications.values());
this._onDidChangeNotifications.fire(notifications);

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

public async markAsRead(notificationIdentifier: { threadId: string, notificationKey: string }): Promise<void> {
await this._notificationProvider.markAsRead(notificationIdentifier);
public refresh(): void {
if (this._notifications.size !== 0) {
const updates = Array.from(this._notifications.values());
this._onDidChangeNotifications.fire(updates);
}

this._pageCount = 1;
this._dateTime = new Date();
this._notifications.clear();

this._refresh(true);
}

public loadMore(): void {
this._pageCount++;
this._refresh(true);
}

public _refresh(fetch: boolean): void {
this._fetchNotifications = fetch;
this._onDidChangeTreeData.fire();
}

public async markAsRead(notificationIdentifier: { threadId: string, notificationKey: string }): Promise<void> {
const notification = this._notifications.get(notificationIdentifier.notificationKey);
if (notification) {
await this._notificationProvider.markAsRead(notificationIdentifier);

this._onDidChangeNotifications.fire([notification]);
this._notifications.delete(notificationIdentifier.notificationKey);

this._refresh(false);
}
}

public sortNotifications(method: NotificationsSortMethod): void {
if (this._sortingMethod === method) {
return;
}

this._sortingMethod = method;
this._refresh(false);
}

private _sortNotifications(notifications: NotificationTreeItem[]): NotificationTreeItem[] {
if (this._sortingMethod === NotificationsSortMethod.Timestamp) {
return notifications.sort((n1, n2) => n2.notification.updatedAd.getTime() - n1.notification.updatedAd.getTime());
Expand Down
Loading

0 comments on commit ec5d189

Please sign in to comment.