Skip to content

Commit 525e502

Browse files
authored
Completion notification/update is delayed (#7198)
* Completion notification/update is delayed Fixes #7149 * Fix tests * Actually fix test
1 parent ea064fa commit 525e502

11 files changed

+165
-141
lines changed

src/github/activityBarViewProvider.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W
7979
} while (attemptsRemaining > 0 && mergability === PullRequestMergeability.Unknown);
8080

8181
const result: Partial<PullRequest> = {
82-
events: await this._item.getTimelineEvents(),
82+
events: await this._item.githubRepository.getTimelineEvents(this._item),
8383
mergeable: mergability,
8484
};
8585
await this.refresh();
@@ -213,7 +213,7 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W
213213
pullRequestModel.number,
214214
),
215215
this._folderRepositoryManager.getPullRequestRepositoryAccessAndMergeMethods(pullRequestModel),
216-
pullRequestModel.getTimelineEvents(),
216+
pullRequestModel.githubRepository.getTimelineEvents(pullRequestModel),
217217
pullRequestModel.getReviewRequests(),
218218
this._folderRepositoryManager.getBranchNameForPullRequest(pullRequestModel),
219219
this._folderRepositoryManager.getPullRequestRepositoryDefaultBranch(pullRequestModel),

src/github/copilotPrWatcher.ts

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -89,11 +89,10 @@ export class CopilotPRWatcher extends Disposable {
8989

9090
constructor(private readonly _reposManager: RepositoriesManager, private readonly _model: CopilotStateModel) {
9191
super();
92-
const query = this._queriesIncludeCopilot();
93-
if (query) {
94-
this._getStateChanges(query);
95-
}
92+
93+
this._getStateChanges();
9694
this._pollForChanges();
95+
this._register(this._reposManager.onDidChangeAnyPullRequests(() => this._getStateChanges()));
9796

9897
this._register(vscode.workspace.onDidChangeConfiguration(e => {
9998
if (e.affectsConfiguration(`${PR_SETTINGS_NAMESPACE}.${QUERIES}`)) {
@@ -110,16 +109,13 @@ export class CopilotPRWatcher extends Disposable {
110109

111110
private _timeout: NodeJS.Timeout | undefined;
112111
private async _pollForChanges(): Promise<void> {
113-
const query = this._queriesIncludeCopilot();
114-
if (!query) {
115-
return;
116-
}
112+
const shouldContinue = await this._getStateChanges();
117113

118-
await this._getStateChanges(query);
119-
120-
this._timeout = setTimeout(() => {
121-
this._pollForChanges();
122-
}, 60 * 1000); // Poll every minute
114+
if (shouldContinue) {
115+
this._timeout = setTimeout(() => {
116+
this._pollForChanges();
117+
}, 60 * 1000); // Poll every minute
118+
}
123119
}
124120

125121
private _currentUser: string | undefined;
@@ -130,7 +126,11 @@ export class CopilotPRWatcher extends Disposable {
130126
return this._currentUser;
131127
}
132128

133-
private async _getStateChanges(query: string) {
129+
private async _getStateChanges(): Promise<boolean> {
130+
const query = this._queriesIncludeCopilot();
131+
if (!query) {
132+
return false;
133+
}
134134
const stateChanges: { owner: string; repo: string; prNumber: number; status: CopilotPRStatus }[] = [];
135135
const unseenKeys: Set<string> = new Set(this._model.keys());
136136
let initialized = 0;
@@ -170,9 +170,9 @@ export class CopilotPRWatcher extends Disposable {
170170
if ((initialized === this._reposManager.folderManagers.length) && (this._reposManager.folderManagers.length > 0)) {
171171
this._model.setInitialized();
172172
}
173-
return [];
173+
return true;
174174
} else {
175-
return stateChanges;
175+
return true;
176176
}
177177
}
178178
}

src/github/githubRepository.ts

Lines changed: 116 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import * as buffer from 'buffer';
77
import { ApolloQueryResult, DocumentNode, FetchResult, MutationOptions, NetworkStatus, QueryOptions } from 'apollo-boost';
88
import * as vscode from 'vscode';
99
import { AuthenticationError, AuthProvider, GitHubServerType, isSamlError } from '../common/authentication';
10-
import { COPILOT_ACCOUNTS } from '../common/comment';
10+
import { COPILOT_ACCOUNTS, IComment, IReviewThread } from '../common/comment';
1111
import { Disposable } from '../common/lifecycle';
1212
import Logger from '../common/logger';
1313
import { GitHubRemote, parseRemote } from '../common/remote';
@@ -69,6 +69,7 @@ import {
6969
convertRESTPullRequestToRawPullRequest,
7070
getAvatarWithEnterpriseFallback,
7171
getOverrideBranch,
72+
insertNewCommitsSinceReview,
7273
isInCodespaces,
7374
parseAccount,
7475
parseCombinedTimelineEvents,
@@ -1537,10 +1538,13 @@ export class GitHubRepository extends Disposable {
15371538
crossRefs.delete(model.html_url);
15381539
}
15391540
}
1541+
const oldEvents = issueModel.timelineEvents;
1542+
issueModel.timelineEvents = events;
15401543
if (crossRefs.size > 0) {
15411544
this._onDidChangePullRequests.fire();
1545+
} else if (oldEvents.length !== events.length) {
1546+
this._onDidChangePullRequests.fire();
15421547
}
1543-
15441548
return events;
15451549
} catch (e) {
15461550
console.log(e);
@@ -1563,6 +1567,116 @@ export class GitHubRepository extends Disposable {
15631567
return CopilotWorkingStatus.NotCopilotIssue;
15641568
}
15651569

1570+
/**
1571+
* Get the timeline events of a pull request, including comments, reviews, commits, merges, deletes, and assigns.
1572+
*/
1573+
async getTimelineEvents(pullRequestModel: PullRequestModel): Promise<Common.TimelineEvent[]> {
1574+
const getTimelineEvents = async () => {
1575+
Logger.debug(`Fetch timeline events of PR #${pullRequestModel.number} - enter`, PullRequestModel.ID);
1576+
const { query, remote, schema } = await this.ensure();
1577+
try {
1578+
const { data } = await query<TimelineEventsResponse>({
1579+
query: schema.TimelineEvents,
1580+
variables: {
1581+
owner: remote.owner,
1582+
name: remote.repositoryName,
1583+
number: pullRequestModel.number,
1584+
},
1585+
});
1586+
1587+
if (data.repository === null) {
1588+
Logger.error('Unexpected null repository when fetching timeline', PullRequestModel.ID);
1589+
}
1590+
return data;
1591+
} catch (e) {
1592+
Logger.error(`Failed to get pull request timeline events: ${e}`, PullRequestModel.ID);
1593+
console.log(e);
1594+
return undefined;
1595+
}
1596+
};
1597+
1598+
const [data, latestReviewCommitInfo, currentUser, reviewThreads] = await Promise.all([
1599+
getTimelineEvents(),
1600+
pullRequestModel.getViewerLatestReviewCommit(),
1601+
(await this.getAuthenticatedUser()).login,
1602+
pullRequestModel.getReviewThreads()
1603+
]);
1604+
1605+
1606+
const ret = data?.repository?.pullRequest.timelineItems.nodes ?? [];
1607+
const events = await parseCombinedTimelineEvents(ret, await this.getCopilotTimelineEvents(pullRequestModel), this);
1608+
1609+
this.addReviewTimelineEventComments(events, reviewThreads);
1610+
insertNewCommitsSinceReview(events, latestReviewCommitInfo?.sha, currentUser, pullRequestModel.head);
1611+
Logger.debug(`Fetch timeline events of PR #${pullRequestModel.number} - done`, PullRequestModel.ID);
1612+
const oldEvents = pullRequestModel.timelineEvents;
1613+
pullRequestModel.timelineEvents = events;
1614+
if (oldEvents.length !== events.length) {
1615+
this._onDidChangePullRequests.fire();
1616+
}
1617+
return events;
1618+
}
1619+
1620+
private addReviewTimelineEventComments(events: Common.TimelineEvent[], reviewThreads: IReviewThread[]): void {
1621+
interface CommentNode extends IComment {
1622+
childComments?: CommentNode[];
1623+
}
1624+
1625+
const reviewEvents = events.filter((e): e is Common.ReviewEvent => e.event === Common.EventType.Reviewed);
1626+
const reviewComments = reviewThreads.reduce((previous, current) => (previous as IComment[]).concat(current.comments), []);
1627+
1628+
const reviewEventsById = reviewEvents.reduce((index, evt) => {
1629+
index[evt.id] = evt;
1630+
evt.comments = [];
1631+
return index;
1632+
}, {} as { [key: number]: Common.ReviewEvent });
1633+
1634+
const commentsById = reviewComments.reduce((index, evt) => {
1635+
index[evt.id] = evt;
1636+
return index;
1637+
}, {} as { [key: number]: CommentNode });
1638+
1639+
const roots: CommentNode[] = [];
1640+
let i = reviewComments.length;
1641+
while (i-- > 0) {
1642+
const c: CommentNode = reviewComments[i];
1643+
if (!c.inReplyToId) {
1644+
roots.unshift(c);
1645+
continue;
1646+
}
1647+
const parent = commentsById[c.inReplyToId];
1648+
parent.childComments = parent.childComments || [];
1649+
parent.childComments = [c, ...(c.childComments || []), ...parent.childComments];
1650+
}
1651+
1652+
roots.forEach(c => {
1653+
const review = reviewEventsById[c.pullRequestReviewId!];
1654+
if (review) {
1655+
review.comments = review.comments.concat(c).concat(c.childComments || []);
1656+
}
1657+
});
1658+
1659+
reviewThreads.forEach(thread => {
1660+
if (!thread.prReviewDatabaseId || !reviewEventsById[thread.prReviewDatabaseId]) {
1661+
return;
1662+
}
1663+
const prReviewThreadEvent = reviewEventsById[thread.prReviewDatabaseId];
1664+
prReviewThreadEvent.reviewThread = {
1665+
threadId: thread.id,
1666+
canResolve: thread.viewerCanResolve,
1667+
canUnresolve: thread.viewerCanUnresolve,
1668+
isResolved: thread.isResolved
1669+
};
1670+
1671+
});
1672+
1673+
const pendingReview = reviewEvents.filter(r => r.state?.toLowerCase() === 'pending')[0];
1674+
if (pendingReview) {
1675+
// Ensures that pending comments made in reply to other reviews are included for the pending review
1676+
pendingReview.comments = reviewComments.filter(c => c.isDraft);
1677+
}
1678+
}
1679+
15661680
/**
15671681
* Get the status checks of the pull request, those for the last commit.
15681682
*

src/github/issueModel.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import * as vscode from 'vscode';
77
import { IComment } from '../common/comment';
88
import Logger from '../common/logger';
99
import { Remote } from '../common/remote';
10-
import { ClosedEvent, EventType } from '../common/timelineEvent';
10+
import { ClosedEvent, EventType, TimelineEvent } from '../common/timelineEvent';
1111
import { formatError } from '../common/utils';
1212
import { GitHubRepository } from './githubRepository';
1313
import {
@@ -41,6 +41,8 @@ export class IssueModel<TItem extends Issue = Issue> {
4141
public item: TItem;
4242
public bodyHTML?: string;
4343

44+
private _timelineEvents: readonly TimelineEvent[] | undefined;
45+
4446
private _onDidInvalidate = new vscode.EventEmitter<void>();
4547
public onDidInvalidate = this._onDidInvalidate.event;
4648

@@ -54,6 +56,14 @@ export class IssueModel<TItem extends Issue = Issue> {
5456
}
5557
}
5658

59+
get timelineEvents(): readonly TimelineEvent[] {
60+
return this._timelineEvents ?? [];
61+
}
62+
63+
set timelineEvents(timelineEvents: readonly TimelineEvent[]) {
64+
this._timelineEvents = timelineEvents;
65+
}
66+
5767
public invalidate() {
5868
// Something about the PR data is stale
5969
this._onDidInvalidate.fire();

0 commit comments

Comments
 (0)