Skip to content

Commit 6b35751

Browse files
authored
Closing a pull request doesn't remove it from the copilot on my behalf section (#7401)
Fixes #7364
1 parent 1a00f21 commit 6b35751

10 files changed

+110
-53
lines changed

src/github/folderRepositoryManager.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ import { ConflictModel } from './conflictGuide';
4242
import { ConflictResolutionCoordinator } from './conflictResolutionCoordinator';
4343
import { Conflict, ConflictResolutionModel } from './conflictResolutionModel';
4444
import { CredentialStore } from './credentials';
45-
import { CopilotWorkingStatus, GitHubRepository, GraphQLError, GraphQLErrorType, IMetadata, ItemsData, PULL_REQUEST_PAGE_SIZE, PullRequestData, TeamReviewerRefreshKind, ViewerPermission } from './githubRepository';
45+
import { CopilotWorkingStatus, GitHubRepository, GraphQLError, GraphQLErrorType, IMetadata, ItemsData, PULL_REQUEST_PAGE_SIZE, PullRequestChangeEvent, PullRequestData, TeamReviewerRefreshKind, ViewerPermission } from './githubRepository';
4646
import { MergeMethod as GraphQLMergeMethod, MergePullRequestInput, MergePullRequestResponse, PullRequestResponse, PullRequestState } from './graphql';
4747
import { IAccount, ILabel, IMilestone, IProject, IPullRequestsPagingOptions, Issue, ITeam, MergeMethod, PRType, PullRequestMergeability, RepoAccessAndMergeMethods, User } from './interface';
4848
import { IssueModel } from './issueModel';
@@ -214,8 +214,8 @@ export class FolderRepositoryManager extends Disposable {
214214
readonly onDidChangeGithubRepositories: vscode.Event<GitHubRepository[]> = this._onDidChangeGithubRepositories.event;
215215

216216
private _onDidChangePullRequestsEvents: vscode.Disposable[] = [];
217-
private readonly _onDidChangeAnyPullRequests = this._register(new vscode.EventEmitter<IssueModel[]>());
218-
readonly onDidChangeAnyPullRequests: vscode.Event<IssueModel[]> = this._onDidChangeAnyPullRequests.event;
217+
private readonly _onDidChangeAnyPullRequests = this._register(new vscode.EventEmitter<PullRequestChangeEvent[]>());
218+
readonly onDidChangeAnyPullRequests: vscode.Event<PullRequestChangeEvent[]> = this._onDidChangeAnyPullRequests.event;
219219
private readonly _onDidAddPullRequest = this._register(new vscode.EventEmitter<IssueModel>());
220220
readonly onDidAddPullRequest: vscode.Event<IssueModel> = this._onDidAddPullRequest.event;
221221

src/github/githubRepository.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ import {
5858
RepoAccessAndMergeMethods,
5959
User,
6060
} from './interface';
61-
import { IssueModel } from './issueModel';
61+
import { IssueChangeEvent, IssueModel } from './issueModel';
6262
import { LoggingOctokit } from './loggingOctokit';
6363
import { PullRequestModel } from './pullRequestModel';
6464
import defaultSchema from './queries.gql';
@@ -152,6 +152,11 @@ export enum CopilotWorkingStatus {
152152
Done = 'Done',
153153
}
154154

155+
export interface PullRequestChangeEvent {
156+
model: IssueModel;
157+
event: IssueChangeEvent;
158+
}
159+
155160
export class GitHubRepository extends Disposable {
156161
static ID = 'GitHubRepository';
157162
protected _initialized: boolean = false;
@@ -171,8 +176,8 @@ export class GitHubRepository extends Disposable {
171176

172177
private _onDidAddPullRequest: vscode.EventEmitter<PullRequestModel> = this._register(new vscode.EventEmitter());
173178
public readonly onDidAddPullRequest: vscode.Event<PullRequestModel> = this._onDidAddPullRequest.event;
174-
private _onDidChangePullRequests: vscode.EventEmitter<IssueModel[]> = this._register(new vscode.EventEmitter());
175-
public readonly onDidChangePullRequests: vscode.Event<IssueModel[]> = this._onDidChangePullRequests.event;
179+
private _onDidChangePullRequests: vscode.EventEmitter<PullRequestChangeEvent[]> = this._register(new vscode.EventEmitter());
180+
public readonly onDidChangePullRequests: vscode.Event<PullRequestChangeEvent[]> = this._onDidChangePullRequests.event;
176181

177182
public get hub(): GitHub {
178183
if (!this._hub) {
@@ -977,7 +982,7 @@ export class GitHubRepository extends Disposable {
977982
model = new PullRequestModel(this._credentialStore, this.telemetry, this, this.remote, pullRequest);
978983
const prModel = model;
979984
const disposables: vscode.Disposable[] = [];
980-
disposables.push(model.onDidChange(() => this._onPullRequestModelChanged(prModel)));
985+
disposables.push(model.onDidChange(e => this._onPullRequestModelChanged(prModel, e)));
981986
this._pullRequestModelsByNumber.set(pullRequest.number, { model, disposables });
982987
if (!silent) {
983988
this._onDidAddPullRequest.fire(model);
@@ -987,8 +992,8 @@ export class GitHubRepository extends Disposable {
987992
return model;
988993
}
989994

990-
private _onPullRequestModelChanged(model: PullRequestModel): void {
991-
this._onDidChangePullRequests.fire([model]);
995+
private _onPullRequestModelChanged(model: PullRequestModel, change: IssueChangeEvent): void {
996+
this._onDidChangePullRequests.fire([{ model, event: change }]);
992997
}
993998

994999
async createPullRequest(params: OctokitCommon.PullsCreateParams): Promise<PullRequestModel> {

src/github/issueModel.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,12 @@ export interface IssueChangeEvent {
2929
title?: true;
3030
body?: true;
3131
milestone?: true;
32-
updatedAt?: true;
32+
// updatedAt?: true;
3333
state?: true;
3434
labels?: true;
3535
assignees?: true;
3636
projects?: true;
37+
comments?: true;
3738

3839
timeline?: true;
3940

@@ -164,7 +165,6 @@ export class IssueModel<TItem extends Issue = Issue> extends Disposable {
164165
this.milestone = issue.milestone;
165166
}
166167
if (this.updatedAt !== issue.updatedAt) {
167-
changes.updatedAt = true;
168168
this.updatedAt = issue.updatedAt;
169169
}
170170
const newState = this.stateToStateEnum(issue.state);
@@ -528,11 +528,7 @@ export class IssueModel<TItem extends Issue = Issue> extends Disposable {
528528
const newEvents = timelineEvents.filter(event => (eventTime(event) ?? 0) > oldEventTime);
529529
allEvents = [...issueModel.timelineEvents, ...newEvents];
530530
}
531-
const oldTimeline = issueModel.timelineEvents;
532531
issueModel.timelineEvents = allEvents;
533-
if (oldLastEvent && (allEvents.length !== oldTimeline.length)) {
534-
this._onDidChange.fire({ timeline: true });
535-
}
536532
}
537533
return timelineEvents;
538534
} catch (e) {

src/github/pullRequestModel.ts

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,6 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
146146

147147
private _hasComments: boolean;
148148
private _comments: readonly IComment[] | undefined;
149-
private _onDidChangeComments: vscode.EventEmitter<void> = this._register(new vscode.EventEmitter<void>());
150-
public readonly onDidChangeComments: vscode.Event<void> = this._onDidChangeComments.event;
151149

152150
// Whether the pull request is currently checked out locally
153151
private _isActive: boolean;
@@ -226,7 +224,7 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
226224

227225
set comments(comments: readonly IComment[]) {
228226
this._comments = comments;
229-
this._onDidChangeComments.fire();
227+
this._onDidChange.fire({ comments: true });
230228
}
231229

232230
get fileChangeViewedState(): FileViewedState {
@@ -356,7 +354,7 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
356354
"pr.approve" : {}
357355
*/
358356
this._telemetry.sendTelemetryEvent('pr.approve');
359-
this._onDidChangeComments.fire();
357+
this._onDidChange.fire({ comments: true, timeline: true });
360358
return x;
361359
});
362360
}
@@ -374,8 +372,7 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
374372
"pr.requestChanges" : {}
375373
*/
376374
this._telemetry.sendTelemetryEvent('pr.requestChanges');
377-
this._onDidChangeComments.fire();
378-
this._onDidChange.fire({ timeline: true });
375+
this._onDidChange.fire({ timeline: true, comments: true });
379376
return action;
380377
}
381378

@@ -470,8 +467,7 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
470467
threadWithComment.viewerCanResolve = true;
471468
this._onDidChangeReviewThreads.fire({ added: [], changed: [threadWithComment], removed: [] });
472469
}
473-
this._onDidChangeComments.fire();
474-
this._onDidChange.fire({ timeline: true });
470+
this._onDidChange.fire({ timeline: true, comments: true });
475471
return reviewEvent;
476472
} else {
477473
throw new Error(`Submitting review failed, no pending review for current pull request: ${this.number}.`);
@@ -696,8 +692,7 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
696692
this._onDidChangeReviewThreads.fire({ added: [], changed: [threadWithComment], removed: [] });
697693
}
698694

699-
this._onDidChangeComments.fire();
700-
this._onDidChange.fire({ timeline: true });
695+
this._onDidChange.fire({ timeline: true, comments: true });
701696
return newComment;
702697
}
703698

src/github/pullRequestOverview.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,8 +160,8 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
160160
}));
161161

162162
if (this._item) {
163-
this._prListeners.push(this._item.onDidChangeComments(() => {
164-
if (!this._isUpdating) {
163+
this._prListeners.push(this._item.onDidChange(e => {
164+
if (e.comments && !this._isUpdating) {
165165
this.refreshPanel();
166166
}
167167
}));

src/github/repositoriesManager.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { fromPRUri, fromRepoUri, Schemes } from '../common/uri';
1515
import { compareIgnoreCase, isDescendant } from '../common/utils';
1616
import { CredentialStore } from './credentials';
1717
import { FolderRepositoryManager, ReposManagerState, ReposManagerStateContext } from './folderRepositoryManager';
18+
import { PullRequestChangeEvent } from './githubRepository';
1819
import { IssueModel } from './issueModel';
1920
import { findDotComAndEnterpriseRemotes, getEnterpriseUri, hasEnterpriseUri, setEnterpriseUri } from './utils';
2021

@@ -45,7 +46,7 @@ export class RepositoriesManager extends Disposable {
4546
private _onDidLoadAnyRepositories = new vscode.EventEmitter<void>();
4647
readonly onDidLoadAnyRepositories = this._onDidLoadAnyRepositories.event;
4748

48-
private _onDidChangeAnyPullRequests = new vscode.EventEmitter<IssueModel[]>();
49+
private _onDidChangeAnyPullRequests = new vscode.EventEmitter<PullRequestChangeEvent[]>();
4950
readonly onDidChangeAnyPullRequests = this._onDidChangeAnyPullRequests.event;
5051

5152
private _onDidAddPullRequest = new vscode.EventEmitter<IssueModel>();

src/view/prsTreeDataProvider.ts

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ import { EXTENSION_ID } from '../constants';
1515
import { CopilotRemoteAgentManager } from '../github/copilotRemoteAgent';
1616
import { CredentialStore } from '../github/credentials';
1717
import { FolderRepositoryManager, ReposManagerState } from '../github/folderRepositoryManager';
18+
import { PullRequestChangeEvent } from '../github/githubRepository';
1819
import { PRType } from '../github/interface';
19-
import { IssueModel } from '../github/issueModel';
2020
import { issueMarkdown } from '../github/markdownUtils';
2121
import { NotificationProvider } from '../github/notifications';
2222
import { PullRequestModel } from '../github/pullRequestModel';
@@ -60,7 +60,7 @@ export class PullRequestsTreeDataProvider extends Disposable implements vscode.T
6060
this._register(this.prsTreeModel.onDidChangeData(e => {
6161
if (e instanceof FolderRepositoryManager) {
6262
this.refreshRepo(e);
63-
} else if (Array.isArray(e) && e[0] instanceof IssueModel) {
63+
} else if (Array.isArray(e)) {
6464
this.refreshPullRequests(e);
6565
} else {
6666
this.refresh(undefined, true);
@@ -322,18 +322,52 @@ export class PullRequestsTreeDataProvider extends Disposable implements vscode.T
322322
}
323323
}
324324

325-
private refreshPullRequests(pullRequests: IssueModel[]): void {
325+
private refreshPullRequests(pullRequests: PullRequestChangeEvent[]): void {
326326
if (!this._children?.length || !pullRequests?.length) {
327327
return;
328328
}
329-
const toRefresh: TreeNode[] = [];
329+
const prNodesToRefresh: TreeNode[] = [];
330+
const prsWithStateChange = new Set();
331+
const prNumbers = new Set();
332+
333+
for (const prChange of pullRequests) {
334+
prNumbers.add(prChange.model.number);
335+
if (prChange.event.state) {
336+
prsWithStateChange.add(prChange.model.number);
337+
}
338+
}
339+
340+
const hasPRNode = (node: TreeNode) => {
341+
const prNodes = node.children ?? [];
342+
for (const prNode of prNodes) {
343+
if (prNode instanceof PRNode && prsWithStateChange.has(prNode.pullRequestModel.number)) {
344+
return true;
345+
}
346+
}
347+
return false;
348+
};
349+
350+
const categoriesToRefresh: Set<CategoryTreeNode> = new Set();
351+
// First find the categories to refresh, since if we refresh a category we don't need to specifically refresh its children
352+
for (const child of this._children) {
353+
if (child instanceof WorkspaceFolderNode) {
354+
const categories = child.children ?? [];
355+
for (const category of categories) {
356+
if (category instanceof CategoryTreeNode && !categoriesToRefresh.has(category) && hasPRNode(category)) {
357+
categoriesToRefresh.add(category);
358+
}
359+
}
360+
} else if (child instanceof CategoryTreeNode && !categoriesToRefresh.has(child) && hasPRNode(child)) {
361+
categoriesToRefresh.add(child);
362+
}
363+
}
364+
330365
// Yes, multiple PRs can exist in different repos with the same number, but at worst we'll refresh all the duplicate numbers, which shouldn't be many.
331-
const prNumbers = new Set(pullRequests.map(pr => pr.number));
332366
const collectPRNodes = (node: TreeNode) => {
333367
const prNodes = node.children ?? [];
334368
for (const prNode of prNodes) {
335369
if (prNode instanceof PRNode && prNumbers.has(prNode.pullRequestModel.number)) {
336-
toRefresh.push(prNode);
370+
prNodesToRefresh.push(prNode);
337371
}
338372
}
339373
};
@@ -342,16 +376,16 @@ export class PullRequestsTreeDataProvider extends Disposable implements vscode.T
342376
if (child instanceof WorkspaceFolderNode) {
343377
const categories = child.children ?? [];
344378
for (const category of categories) {
345-
if (category instanceof CategoryTreeNode) {
379+
if (category instanceof CategoryTreeNode && !categoriesToRefresh.has(category)) {
346380
collectPRNodes(category);
347381
}
348382
}
349-
} else if (child instanceof CategoryTreeNode) {
383+
} else if (child instanceof CategoryTreeNode && !categoriesToRefresh.has(child)) {
350384
collectPRNodes(child);
351385
}
352386
}
353-
if (toRefresh.length) {
354-
this._onDidChangeTreeData.fire(toRefresh);
387+
if (prNodesToRefresh.length || categoriesToRefresh.size > 0) {
388+
this._onDidChangeTreeData.fire([...Array.from(categoriesToRefresh), ...prNodesToRefresh]);
355389
}
356390
}
357391

src/view/prsTreeModel.ts

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ import { getReviewMode } from '../common/settingsUtils';
99
import { ITelemetry } from '../common/telemetry';
1010
import { createPRNodeIdentifier } from '../common/uri';
1111
import { FolderRepositoryManager, ItemsResponseResult } from '../github/folderRepositoryManager';
12+
import { PullRequestChangeEvent } from '../github/githubRepository';
1213
import { CheckState, PRType, PullRequestChecks, PullRequestReviewRequirement } from '../github/interface';
13-
import { IssueModel } from '../github/issueModel';
1414
import { PullRequestModel } from '../github/pullRequestModel';
1515
import { RepositoriesManager } from '../github/repositoriesManager';
1616
import { UnsatisfiedChecks } from '../github/utils';
@@ -28,7 +28,7 @@ export class PrsTreeModel extends Disposable {
2828
private _activePRDisposables: Map<FolderRepositoryManager, vscode.Disposable[]> = new Map();
2929
private readonly _onDidChangePrStatus: vscode.EventEmitter<string[]> = this._register(new vscode.EventEmitter<string[]>());
3030
public readonly onDidChangePrStatus = this._onDidChangePrStatus.event;
31-
private readonly _onDidChangeData: vscode.EventEmitter<IssueModel[] | FolderRepositoryManager | void> = this._register(new vscode.EventEmitter<IssueModel[] | FolderRepositoryManager | void>());
31+
private readonly _onDidChangeData: vscode.EventEmitter<PullRequestChangeEvent[] | FolderRepositoryManager | void> = this._register(new vscode.EventEmitter<PullRequestChangeEvent[] | FolderRepositoryManager | void>());
3232
public readonly onDidChangeData = this._onDidChangeData.event;
3333
private _expandedQueries: Set<string> | undefined;
3434
private _hasLoaded: boolean = false;
@@ -51,12 +51,12 @@ export class PrsTreeModel extends Disposable {
5151
}
5252

5353
this._repoEvents.get(manager)!.push(manager.onDidChangeActivePullRequest(e => {
54-
const prs: IssueModel[] = [];
54+
const prs: PullRequestChangeEvent[] = [];
5555
if (e.old) {
56-
prs.push(e.old);
56+
prs.push({ model: e.old, event: {} });
5757
}
5858
if (e.new) {
59-
prs.push(e.new);
59+
prs.push({ model: e.new, event: {} });
6060
}
6161
this._onDidChangeData.fire(prs);
6262

@@ -66,9 +66,9 @@ export class PrsTreeModel extends Disposable {
6666
}
6767
if (manager.activePullRequest) {
6868
this._activePRDisposables.set(manager, [
69-
manager.activePullRequest.onDidChangeComments(() => {
70-
if (manager.activePullRequest) {
71-
this._onDidChangeData.fire([manager.activePullRequest]);
69+
manager.activePullRequest.onDidChange(e => {
70+
if (e.comments && manager.activePullRequest) {
71+
this._onDidChangeData.fire([{ model: manager.activePullRequest, event: e }]);
7272
}
7373
})]);
7474
}
@@ -81,7 +81,9 @@ export class PrsTreeModel extends Disposable {
8181
}
8282

8383
this._register(this._reposManager.onDidChangeAnyPullRequests((prs) => {
84-
this._onDidChangeData.fire(prs);
84+
const needsRefresh = prs.filter(pr => pr.event.state || pr.event.title || pr.event.body || pr.event.comments || pr.event.draft || pr.event.timeline);
85+
this.clearQueriesContainingPullRequests(needsRefresh);
86+
this._onDidChangeData.fire(needsRefresh);
8587
}));
8688

8789
this._register(this._reposManager.onDidAddPullRequest(() => {
@@ -280,6 +282,26 @@ export class PrsTreeModel extends Disposable {
280282
return prs;
281283
}
282284

285+
private clearQueriesContainingPullRequests(pullRequests: PullRequestChangeEvent[]): void {
286+
const withStateChange = pullRequests.filter(prChange => prChange.event.state);
287+
if (!withStateChange || withStateChange.length === 0) {
288+
return;
289+
}
290+
for (const [, queries] of this._cachedPRs.entries()) {
291+
for (const [queryKey, itemsResult] of queries.entries()) {
292+
if (!itemsResult || !itemsResult.items || itemsResult.items.length === 0) {
293+
continue;
294+
}
295+
const hasPR = withStateChange.some(prChange =>
296+
itemsResult.items.some(item => item === prChange.model)
297+
);
298+
if (hasPR) {
299+
queries.delete(queryKey);
300+
}
301+
}
302+
}
303+
}
304+
283305
override dispose() {
284306
super.dispose();
285307
disposeAll(Array.from(this._activePRDisposables.values()).flat());

src/view/treeNodes/commitsCategoryNode.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,11 @@ export class CommitsNode extends TreeNode implements vscode.TreeItem {
3131
Logger.appendLine(`Review threads have changed, refreshing Commits node`, PR_TREE);
3232
this.refresh(this);
3333
}));
34-
this.childrenDisposables.push(this._pr.onDidChangeComments(() => {
35-
Logger.appendLine(`Comments have changed, refreshing Commits node`, PR_TREE);
36-
this.refresh(this);
34+
this.childrenDisposables.push(this._pr.onDidChange(e => {
35+
if (e.comments) {
36+
Logger.appendLine(`Comments have changed, refreshing Commits node`, PR_TREE);
37+
this.refresh(this);
38+
}
3739
}));
3840
}
3941

0 commit comments

Comments
 (0)