Skip to content

Commit b272691

Browse files
alexr00alexr00
authored
Fix PR description update timing (#8349)
1 parent bc3e067 commit b272691

2 files changed

Lines changed: 59 additions & 36 deletions

File tree

‎src/github/activityBarViewProvider.ts‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { MergeArguments, PullRequest, ReviewType } from './views';
1515
import { IComment } from '../common/comment';
1616
import { emojify, ensureEmojis } from '../common/emoji';
1717
import { disposeAll } from '../common/lifecycle';
18+
import Logger from '../common/logger';
1819
import { CHECKOUT_DEFAULT_BRANCH, CHECKOUT_PULL_REQUEST_BASE_BRANCH, DELETE_BRANCH_AFTER_MERGE, POST_DONE, PR_SETTINGS_NAMESPACE } from '../common/settingKeys';
1920
import { ReviewEvent } from '../common/timelineEvent';
2021
import { formatError } from '../common/utils';
@@ -25,7 +26,7 @@ import { ReviewManager } from '../view/reviewManager';
2526
export class PullRequestViewProvider extends WebviewViewBase implements vscode.WebviewViewProvider {
2627
public override readonly viewType = 'github:activePullRequest';
2728
private _existingReviewers: ReviewState[] = [];
28-
private _isUpdating: boolean = false;
29+
private _updatingPromise: Promise<unknown> | undefined;
2930

3031
constructor(
3132
extensionUri: vscode.Uri,
@@ -151,7 +152,7 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W
151152
}
152153
this._prDisposables = [];
153154
this._prDisposables.push(pullRequestModel.onDidChange(e => {
154-
if ((e.state || e.comments || e.reviewers) && !this._isUpdating) {
155+
if ((e.state || e.comments || e.reviewers) && !this._updatingPromise) {
155156
this.updatePullRequest(pullRequestModel);
156157
}
157158
}));
@@ -160,10 +161,16 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W
160161

161162
private _updatePendingVisibility: vscode.Disposable | undefined = undefined;
162163
public async updatePullRequest(pullRequestModel: PullRequestModel): Promise<void> {
163-
if (this._isUpdating) {
164-
throw new Error('Already updating pull request view');
164+
const isSamePullRequest = pullRequestModel.equals(this._item);
165+
if (this._updatingPromise && isSamePullRequest) {
166+
Logger.error('Already updating pull request view', PullRequestViewProvider.name);
167+
return;
168+
} else if (this._updatingPromise && !isSamePullRequest) {
169+
this._item = pullRequestModel;
170+
await this._updatingPromise;
171+
} else {
172+
this._item = pullRequestModel;
165173
}
166-
this._isUpdating = true;
167174

168175
try {
169176
if (this._view && !this._view.visible) {
@@ -178,7 +185,7 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W
178185
this.registerPrSpecificListeners(pullRequestModel);
179186
}
180187
this._item = pullRequestModel;
181-
const [pullRequest, repositoryAccess, timelineEvents, requestedReviewers, branchInfo, defaultBranch, currentUser, viewerCanEdit, hasReviewDraft, coAuthors] = await Promise.all([
188+
const updatingPromise = Promise.all([
182189
this._folderRepositoryManager.resolvePullRequest(
183190
pullRequestModel.remote.owner,
184191
pullRequestModel.remote.repositoryName,
@@ -195,13 +202,19 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W
195202
pullRequestModel.getCoAuthors(),
196203
ensureEmojis(this._folderRepositoryManager.context)
197204
]);
205+
this._updatingPromise = updatingPromise;
206+
const [pullRequest, repositoryAccess, timelineEvents, requestedReviewers, branchInfo, defaultBranch, currentUser, viewerCanEdit, hasReviewDraft, coAuthors] = await updatingPromise;
198207

199208
if (!pullRequest) {
200209
throw new Error(
201210
`Fail to resolve Pull Request #${pullRequestModel.number} in ${pullRequestModel.remote.owner}/${pullRequestModel.remote.repositoryName}`,
202211
);
203212
}
204213

214+
if (!this._item.equals(pullRequestModel)) {
215+
return;
216+
}
217+
205218
this._item = pullRequest;
206219
if (!this._view) {
207220
// If the there is no PR webview, then there is nothing else to update.
@@ -292,8 +305,6 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W
292305

293306
} catch (e) {
294307
vscode.window.showErrorMessage(`Error updating active pull request view: ${formatError(e)}`);
295-
} finally {
296-
this._isUpdating = false;
297308
}
298309
}
299310

‎src/github/pullRequestOverview.ts‎

Lines changed: 40 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
5858
private _assignableUsers: { [key: string]: IAccount[] } = {};
5959

6060
private _prListeners: vscode.Disposable[] = [];
61-
private _isUpdating: boolean = false;
61+
private _updatingPromise: Promise<unknown> | undefined;
6262

6363
public static override async createOrShow(
6464
telemetry: ITelemetry,
@@ -173,7 +173,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
173173

174174
if (this._item) {
175175
this._prListeners.push(this._item.onDidChange(e => {
176-
if ((e.state || e.comments) && !this._isUpdating) {
176+
if ((e.state || e.comments) && !this._updatingPromise) {
177177
this.refreshPanel();
178178
}
179179
}));
@@ -236,32 +236,19 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
236236
}
237237

238238
protected override async updateItem(pullRequestModel: PullRequestModel): Promise<void> {
239-
this._item = pullRequestModel;
240-
241-
if (this._isUpdating) {
242-
throw new Error('Already updating pull request webview');
239+
const isSamePullRequest = pullRequestModel.equals(this._item);
240+
if (this._updatingPromise && isSamePullRequest) {
241+
Logger.error('Already updating pull request webview', PullRequestOverviewPanel.ID);
242+
return;
243+
} else if (this._updatingPromise && !isSamePullRequest) {
244+
this._item = pullRequestModel;
245+
await this._updatingPromise;
246+
} else {
247+
this._item = pullRequestModel;
243248
}
244-
this._isUpdating = true;
249+
245250
try {
246-
const [
247-
pullRequest,
248-
timelineEvents,
249-
defaultBranch,
250-
status,
251-
requestedReviewers,
252-
repositoryAccess,
253-
branchInfo,
254-
currentUser,
255-
viewerCanEdit,
256-
orgTeamsCount,
257-
mergeQueueMethod,
258-
isBranchUpToDateWithBase,
259-
mergeability,
260-
emailForCommit,
261-
coAuthors,
262-
hasReviewDraft,
263-
assignableUsers
264-
] = await Promise.all([
251+
const updatingPromise = Promise.all([
265252
this._folderRepositoryManager.resolvePullRequest(
266253
pullRequestModel.remote.owner,
267254
pullRequestModel.remote.repositoryName,
@@ -284,12 +271,39 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
284271
pullRequestModel.validateDraftMode(),
285272
this._folderRepositoryManager.getAssignableUsers()
286273
]);
274+
this._updatingPromise = updatingPromise;
275+
276+
const [
277+
pullRequest,
278+
timelineEvents,
279+
defaultBranch,
280+
status,
281+
requestedReviewers,
282+
repositoryAccess,
283+
branchInfo,
284+
currentUser,
285+
viewerCanEdit,
286+
orgTeamsCount,
287+
mergeQueueMethod,
288+
isBranchUpToDateWithBase,
289+
mergeability,
290+
emailForCommit,
291+
coAuthors,
292+
hasReviewDraft,
293+
assignableUsers
294+
] = await updatingPromise;
295+
this._updatingPromise = undefined;
287296
if (!pullRequest) {
288297
throw new Error(
289298
`Fail to resolve Pull Request #${pullRequestModel.number} in ${pullRequestModel.remote.owner}/${pullRequestModel.remote.repositoryName}`,
290299
);
291300
}
292301

302+
if (!this._item.equals(pullRequestModel)) {
303+
// Updated PR is no longer the current one
304+
return;
305+
}
306+
293307
this._item = pullRequest;
294308
this.registerPrListeners();
295309
this._repositoryDefaultBranch = defaultBranch!;
@@ -362,8 +376,6 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
362376
}
363377
} catch (e) {
364378
vscode.window.showErrorMessage(`Error updating pull request description: ${formatError(e)}`);
365-
} finally {
366-
this._isUpdating = false;
367379
}
368380
}
369381

0 commit comments

Comments
 (0)