Skip to content

Commit 80901bf

Browse files
authored
Mark merged PRs with meaningless updates (#6821)
1 parent 83bb3f0 commit 80901bf

5 files changed

Lines changed: 106 additions & 2 deletions

File tree

package.json

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,12 @@
471471
"markdownDescription": "%githubPullRequests.experimental.notificationsView.description%",
472472
"default": false
473473
},
474+
"githubPullRequests.experimental.notificationsMarkPullRequests": {
475+
"type": "string",
476+
"markdownDescription": "%githubPullRequests.experimental.notificationsMarkPullRequests.description%",
477+
"enum": ["markAsDone", "markAsRead", "none"],
478+
"default": "none"
479+
},
474480
"githubPullRequests.experimental.useQuickChat": {
475481
"type": "boolean",
476482
"markdownDescription": "%githubPullRequests.experimental.useQuickChat.description%",
@@ -1572,6 +1578,12 @@
15721578
"title": "%command.notifications.markAsDone.title%",
15731579
"category": "%command.notifications.category%",
15741580
"icon": "$(check-all)"
1581+
},
1582+
{
1583+
"command": "notifications.markMergedPullRequestsAsRead",
1584+
"title": "%command.notifications.markMergedPullRequestsAsRead.title%",
1585+
"category": "%command.notifications.category%",
1586+
"icon": "$(git-pull-request)"
15751587
}
15761588
],
15771589
"viewsWelcome": [
@@ -2260,6 +2272,10 @@
22602272
"command": "notification.chatSummarizeNotification",
22612273
"when": "false"
22622274
},
2275+
{
2276+
"command": "notifications.markMergedPullRequestsAsRead",
2277+
"when": "false"
2278+
},
22632279
{
22642280
"command": "review.copyPrLink",
22652281
"when": "github:inReviewMode"
@@ -2391,6 +2407,11 @@
23912407
"when": "gitHubOpenRepositoryCount != 0 && github:initialized && view == notifications:github",
23922408
"group": "sortNotifications@2"
23932409
},
2410+
{
2411+
"command": "notifications.markMergedPullRequestsAsRead",
2412+
"when": "gitHubOpenRepositoryCount != 0 && github:initialized && view == notifications:github && config.githubPullRequests.experimental.notificationsMarkPullRequests != none",
2413+
"group": "navigation@0"
2414+
},
23942415
{
23952416
"command": "notifications.refresh",
23962417
"when": "gitHubOpenRepositoryCount != 0 && github:initialized && view == notifications:github",

package.nls.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@
8888
"githubPullRequests.createDefaultBaseBranch.auto": "When the current repository is a fork, this will work like \"repositoryDefault\". Otherwise, it will work like \"createdFromBranch\".",
8989
"githubPullRequests.experimental.chat.description": "Enables the `@githubpr` Copilot chat participant in the chat view. `@githubpr` can help search for issues and pull requests, suggest fixes for issues, and summarize issues, pull requests, and notifications.",
9090
"githubPullRequests.experimental.notificationsView.description": "Enables the notifications view, which shows a list of your GitHub notifications. When combined with `#githubPullRequests.experimental.chat#`, you can have Copilot sort and summarize your notifications. View will not show in a Codespace accessed from the browser.",
91+
"githubPullRequests.experimental.notificationsMarkPullRequests.description": "Adds an action in the Notifications view to mark merged pull requests with no reviews, comments, or commits since you last viewed the pull request as read.",
9192
"githubPullRequests.experimental.useQuickChat.description": "Controls whether the Copilot \"Summarize\" commands in the Pull Requests, Issues, and Notifications views will use quick chat. Only has an effect if `#githubPullRequests.experimental.chat#` is enabled.",
9293
"githubIssues.ignoreMilestones.description": "An array of milestones titles to never show issues from.",
9394
"githubIssues.createIssueTriggers.description": "Strings that will cause the 'Create issue from comment' code action to show.",
@@ -298,6 +299,7 @@
298299
"command.notifications.openOnGitHub.title": "Open on GitHub",
299300
"command.notifications.markAsRead.title": "Mark as Read",
300301
"command.notifications.markAsDone.title": "Mark as Done",
302+
"command.notifications.markMergedPullRequestsAsRead.title": "Mark Merged Pull Requests as Read",
301303
"command.notification.chatSummarizeNotification.title": "Summarize With Copilot",
302304
"welcome.github.login.contents": {
303305
"message": "You have not yet signed in with GitHub\n[Sign in](command:pr.signin)",

src/common/settingKeys.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ export const EXPERIMENTAL_USE_QUICK_CHAT = 'experimental.useQuickChat';
5757
export const EXPERIMENTAL_NOTIFICATIONS = 'experimental.notificationsView';
5858
export const EXPERIMENTAL_NOTIFICATIONS_PAGE_SIZE = 'experimental.notificationsViewPageSize';
5959
export const EXPERIMENTAL_NOTIFICATIONS_SCORE = 'experimental.notificationsScore';
60+
export const EXPERIMENTAL_NOTIFICATIONS_MARK_PRS = 'experimental.notificationsMarkPullRequests';
6061

6162
// git
6263
export const GIT = 'git';

src/notifications/notificationsFeatureRegistar.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export class NotificationsFeatureRegister extends Disposable {
2727
const notificationsProvider = new NotificationsProvider(credentialStore, this._repositoriesManager);
2828
this._register(notificationsProvider);
2929

30-
const notificationsManager = new NotificationsManager(notificationsProvider);
30+
const notificationsManager = new NotificationsManager(notificationsProvider, credentialStore);
3131
this._register(notificationsManager);
3232

3333
// Decorations
@@ -121,6 +121,16 @@ export class NotificationsFeatureRegister extends Disposable {
121121
})
122122
);
123123

124+
this._register(
125+
vscode.commands.registerCommand('notifications.markMergedPullRequestsAsRead', () => {
126+
/* __GDPR__
127+
"notifications.markMergedPullRequestsAsRead" : {}
128+
*/
129+
this._telemetry.sendTelemetryEvent('notifications.markMergedPullRequestsAsRead');
130+
return notificationsManager.markMergedPullRequestAsRead();
131+
})
132+
);
133+
124134
// Events
125135
this._register(onceEvent(this._repositoriesManager.onDidLoadAnyRepositories)(() => {
126136
notificationsManager.refresh();

src/notifications/notificationsManager.ts

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@
55

66
import * as vscode from 'vscode';
77
import { Disposable } from '../common/lifecycle';
8+
import { EXPERIMENTAL_NOTIFICATIONS_MARK_PRS } from '../common/settingKeys';
9+
import { EventType, TimelineEvent } from '../common/timelineEvent';
810
import { toNotificationUri } from '../common/uri';
11+
import { CredentialStore } from '../github/credentials';
912
import { NotificationSubjectType } from '../github/interface';
1013
import { IssueModel } from '../github/issueModel';
1114
import { PullRequestModel } from '../github/pullRequestModel';
@@ -38,7 +41,7 @@ export class NotificationsManager extends Disposable implements vscode.TreeDataP
3841
private _sortingMethod: NotificationsSortMethod = NotificationsSortMethod.Timestamp;
3942
get sortingMethod(): NotificationsSortMethod { return this._sortingMethod; }
4043

41-
constructor(private readonly _notificationProvider: NotificationsProvider) {
44+
constructor(private readonly _notificationProvider: NotificationsProvider, private readonly _credentialStore: CredentialStore) {
4245
super();
4346
this._register(this._onDidChangeTreeData);
4447
this._register(this._onDidChangeNotifications);
@@ -233,6 +236,73 @@ export class NotificationsManager extends Disposable implements vscode.TreeDataP
233236
}
234237
}
235238

239+
private _getMeaningfulEventTime(event: TimelineEvent, currentUser: string, isCurrentUser: boolean): Date | undefined {
240+
const userCheck = (testUser?: string) => {
241+
if (isCurrentUser) {
242+
return testUser === currentUser;
243+
} else if (!isCurrentUser) {
244+
return testUser !== currentUser;
245+
}
246+
};
247+
248+
if (event.event === EventType.Committed) {
249+
if (userCheck(event.author.login)) {
250+
return new Date(event.authoredDate);
251+
}
252+
} else if (event.event === EventType.Commented) {
253+
if (userCheck(event.user?.login)) {
254+
return new Date(event.createdAt);
255+
}
256+
} else if (event.event === EventType.Reviewed) {
257+
// We only count empty reviews as meaningful if the user is the current user
258+
if (isCurrentUser || (event.comments.length > 0 || event.body.length > 0)) {
259+
if (userCheck(event.user?.login)) {
260+
return new Date(event.submittedAt);
261+
}
262+
}
263+
}
264+
}
265+
266+
public async markMergedPullRequestAsRead(): Promise<void> {
267+
const markAsDone = vscode.workspace.getConfiguration('githubPullRequests').get<'markAsRead' | 'markAsDone'>(EXPERIMENTAL_NOTIFICATIONS_MARK_PRS, 'markAsRead') === 'markAsDone';
268+
const filteredNotifications = Array.from(this._notifications.values()).filter(notification => notification.notification.subject.type === NotificationSubjectType.PullRequest && notification.model.isMerged);
269+
const timlines = await Promise.all(filteredNotifications.map(notification => (notification.model as PullRequestModel).getTimelineEvents()));
270+
271+
const markPromises: Promise<void>[] = [];
272+
273+
for (const [index, notification] of filteredNotifications.entries()) {
274+
const currentUser = await this._credentialStore.getCurrentUser(notification.model.remote.authProviderId);
275+
276+
// Check that there have been no comments, reviews, or commits, since last read
277+
const timeline = timlines[index];
278+
let userLastEvent: Date | undefined = undefined;
279+
let nonUserLastEvent: Date | undefined = undefined;
280+
for (let i = timeline.length - 1; i >= 0; i--) {
281+
const event = timeline[i];
282+
if (!userLastEvent) {
283+
userLastEvent = this._getMeaningfulEventTime(event, currentUser.login, true);
284+
}
285+
if (!nonUserLastEvent) {
286+
nonUserLastEvent = this._getMeaningfulEventTime(event, currentUser.login, false);
287+
}
288+
if (userLastEvent && nonUserLastEvent) {
289+
break;
290+
}
291+
}
292+
293+
if (!nonUserLastEvent || (userLastEvent && (userLastEvent.getTime() > nonUserLastEvent.getTime()))) {
294+
if (markAsDone) {
295+
markPromises.push(this._notificationProvider.markAsDone({ threadId: notification.notification.id, notificationKey: notification.notification.key }));
296+
} else {
297+
markPromises.push(this._notificationProvider.markAsRead({ threadId: notification.notification.id, notificationKey: notification.notification.key }));
298+
}
299+
this._notifications.delete(notification.notification.key);
300+
}
301+
}
302+
await Promise.all(markPromises);
303+
this.refresh();
304+
}
305+
236306
public sortNotifications(method: NotificationsSortMethod): void {
237307
if (this._sortingMethod === method) {
238308
return;

0 commit comments

Comments
 (0)