Skip to content

Commit 66a2b23

Browse files
Copilotalexr00
Copilot
and
alexr00
authored
Improve code reuse in PullRequestOverviewPanel and PullRequestViewProvider (#8145)
* Initial plan * Extract common PR review methods into shared utility functions Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com> * Clean up * More cleanup --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>
1 parent 7a03573 commit 66a2b23

File tree

4 files changed

+507
-520
lines changed

4 files changed

+507
-520
lines changed

‎src/github/activityBarViewProvider.ts‎

Lines changed: 52 additions & 173 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@
66
import * as vscode from 'vscode';
77
import { openPullRequestOnGitHub } from '../commands';
88
import { FolderRepositoryManager } from './folderRepositoryManager';
9-
import { GithubItemStateEnum, IAccount, isITeam, ITeam, MergeMethod, PullRequestMergeability, reviewerId, ReviewEventEnum, ReviewState } from './interface';
9+
import { GithubItemStateEnum, IAccount, MergeMethod, ReviewEventEnum, ReviewState } from './interface';
1010
import { isCopilotOnMyBehalf, PullRequestModel } from './pullRequestModel';
1111
import { getDefaultMergeMethod } from './pullRequestOverview';
12-
import { PullRequestView } from './pullRequestOverviewCommon';
12+
import { PullRequestReviewCommon, ReviewContext } from './pullRequestReviewCommon';
1313
import { isInCodespaces, parseReviewers } from './utils';
14-
import { MergeArguments, PullRequest, ReadyForReviewReply, ReviewType, SubmitReviewReply } from './views';
14+
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';
@@ -63,29 +63,11 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W
6363
}
6464

6565
private async updateBranch(message: IRequestMessage<string>): Promise<void> {
66-
if (this._folderRepositoryManager.repository.state.workingTreeChanges.length > 0 || this._folderRepositoryManager.repository.state.indexChanges.length > 0) {
67-
await vscode.window.showErrorMessage(vscode.l10n.t('The pull request branch cannot be updated when the there changed files in the working tree or index. Stash or commit all change and then try again.'), { modal: true });
68-
return this._replyMessage(message, {});
69-
}
70-
const mergeSucceeded = await this._folderRepositoryManager.tryMergeBaseIntoHead(this._item, true);
71-
if (!mergeSucceeded) {
72-
this._replyMessage(message, {});
73-
}
74-
// The mergability of the PR doesn't update immediately. Poll.
75-
let mergability = PullRequestMergeability.Unknown;
76-
let attemptsRemaining = 5;
77-
do {
78-
mergability = (await this._item.getMergeability()).mergeability;
79-
attemptsRemaining--;
80-
await new Promise(c => setTimeout(c, 1000));
81-
} while (attemptsRemaining > 0 && mergability === PullRequestMergeability.Unknown);
82-
83-
const result: Partial<PullRequest> = {
84-
events: await this._item.getTimelineEvents(),
85-
mergeable: mergability,
86-
};
87-
await this.refresh();
88-
this._replyMessage(message, result);
66+
return PullRequestReviewCommon.updateBranch(
67+
this.getReviewContext(),
68+
message,
69+
() => this.refresh()
70+
);
8971
}
9072

9173
protected override async _onDidReceiveMessage(message: IRequestMessage<any>) {
@@ -128,47 +110,11 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W
128110
}
129111

130112
private async checkoutDefaultBranch(message: IRequestMessage<string>): Promise<void> {
131-
try {
132-
const defaultBranch = await this._folderRepositoryManager.getPullRequestRepositoryDefaultBranch(this._item);
133-
const prBranch = this._folderRepositoryManager.repository.state.HEAD?.name;
134-
await this._folderRepositoryManager.checkoutDefaultBranch(defaultBranch);
135-
if (prBranch) {
136-
await this._folderRepositoryManager.cleanupAfterPullRequest(prBranch, this._item);
137-
}
138-
} finally {
139-
// Complete webview promise so that button becomes enabled again
140-
this._replyMessage(message, {});
141-
}
113+
return PullRequestReviewCommon.checkoutDefaultBranch(this.getReviewContext(), message);
142114
}
143115

144116
private reRequestReview(message: IRequestMessage<string>): void {
145-
let targetReviewer: ReviewState | undefined;
146-
const userReviewers: IAccount[] = [];
147-
const teamReviewers: ITeam[] = [];
148-
149-
for (const reviewer of this._existingReviewers) {
150-
let id = reviewer.reviewer.id;
151-
if (id && ((reviewer.state === 'REQUESTED') || (id === message.args))) {
152-
if (id === message.args) {
153-
targetReviewer = reviewer;
154-
}
155-
}
156-
}
157-
158-
if (targetReviewer && isITeam(targetReviewer.reviewer)) {
159-
teamReviewers.push(targetReviewer.reviewer);
160-
} else if (targetReviewer && !isITeam(targetReviewer.reviewer)) {
161-
userReviewers.push(targetReviewer.reviewer);
162-
}
163-
164-
this._item.requestReview(userReviewers, teamReviewers, true).then(() => {
165-
if (targetReviewer) {
166-
targetReviewer.state = 'REQUESTED';
167-
}
168-
this._replyMessage(message, {
169-
reviewers: this._existingReviewers,
170-
});
171-
});
117+
return PullRequestReviewCommon.reRequestReview(this.getReviewContext(), message);
172118
}
173119

174120
public async refresh(): Promise<void> {
@@ -179,9 +125,22 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W
179125
}
180126

181127
private getCurrentUserReviewState(reviewers: ReviewState[], currentUser: IAccount): string | undefined {
182-
const review = reviewers.find(r => reviewerId(r.reviewer) === currentUser.login);
183-
// There will always be a review. If not then the PR shouldn't have been or fetched/shown for the current user
184-
return review?.state;
128+
return PullRequestReviewCommon.getCurrentUserReviewState(reviewers, currentUser);
129+
}
130+
131+
/**
132+
* Get the review context for helper functions
133+
*/
134+
private getReviewContext(): ReviewContext {
135+
return {
136+
item: this._item,
137+
folderRepositoryManager: this._folderRepositoryManager,
138+
existingReviewers: this._existingReviewers,
139+
postMessage: (message: any) => this._postMessage(message),
140+
replyMessage: (message: IRequestMessage<any>, response: any) => this._replyMessage(message, response),
141+
throwError: (message: IRequestMessage<any> | undefined, error: string) => this._throwError(message, error),
142+
getTimeline: () => this._item.getTimelineEvents()
143+
};
185144
}
186145

187146
private _prDisposables: vscode.Disposable[] | undefined = undefined;
@@ -353,83 +312,48 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W
353312
});
354313
}
355314

356-
private updateReviewers(review?: ReviewEvent): void {
357-
if (review && review.state) {
358-
const existingReviewer = this._existingReviewers.find(
359-
reviewer => review.user.login === reviewerId(reviewer.reviewer),
360-
);
361-
if (existingReviewer) {
362-
existingReviewer.state = review.state;
363-
} else {
364-
this._existingReviewers.push({
365-
reviewer: review.user,
366-
state: review.state,
367-
});
368-
}
369-
}
370-
}
371315

372316
private async doReviewCommand(context: { body: string }, reviewType: ReviewType, action: (body: string) => Promise<ReviewEvent>) {
373-
const submittingMessage = {
374-
command: 'pr.submitting-review',
375-
lastReviewType: reviewType
376-
};
377-
this._postMessage(submittingMessage);
378-
try {
379-
const review = await action(context.body);
380-
this.updateReviewers(review);
381-
const reviewMessage: SubmitReviewReply & { command: string } = {
382-
command: 'pr.append-review',
383-
events: [],
384-
reviewers: this._existingReviewers,
385-
reviewedEvent: review,
386-
};
387-
await this._postMessage(reviewMessage);
388-
} catch (e) {
389-
vscode.window.showErrorMessage(vscode.l10n.t('Submitting review failed. {0}', formatError(e)));
390-
this._throwError(undefined, `${formatError(e)}`);
391-
this._postMessage({ command: 'pr.append-review' });
392-
}
317+
return PullRequestReviewCommon.doReviewCommand(
318+
this.getReviewContext(),
319+
context,
320+
reviewType,
321+
false,
322+
action
323+
);
393324
}
394325

395326
private async doReviewMessage(message: IRequestMessage<string>, action: (body) => Promise<ReviewEvent>) {
396-
try {
397-
const review = await action(message.args);
398-
this.updateReviewers(review);
399-
const reviewMessage: SubmitReviewReply = {
400-
events: [],
401-
reviewedEvent: review,
402-
reviewers: this._existingReviewers,
403-
};
404-
this._replyMessage(message, reviewMessage);
405-
} catch (e) {
406-
vscode.window.showErrorMessage(vscode.l10n.t('Submitting review failed. {0}', formatError(e)));
407-
this._throwError(message, `${formatError(e)}`);
408-
}
327+
return PullRequestReviewCommon.doReviewMessage(
328+
this.getReviewContext(),
329+
message,
330+
false,
331+
action
332+
);
409333
}
410334

411335
private approvePullRequest(body: string): Promise<ReviewEvent> {
412336
return this._item.approve(this._folderRepositoryManager.repository, body);
413337
}
414338

415-
private approvePullRequestMessage(message: IRequestMessage<string>): Promise<void> {
416-
return this.doReviewMessage(message, (body) => this.approvePullRequest(body));
339+
private async approvePullRequestMessage(message: IRequestMessage<string>): Promise<void> {
340+
await this.doReviewMessage(message, (body) => this.approvePullRequest(body));
417341
}
418342

419-
private approvePullRequestCommand(context: { body: string }): Promise<void> {
420-
return this.doReviewCommand(context, ReviewType.Approve, (body) => this.approvePullRequest(body));
343+
private async approvePullRequestCommand(context: { body: string }): Promise<void> {
344+
await this.doReviewCommand(context, ReviewType.Approve, (body) => this.approvePullRequest(body));
421345
}
422346

423347
private requestChanges(body: string): Promise<ReviewEvent> {
424348
return this._item.requestChanges(body);
425349
}
426350

427-
private requestChangesCommand(context: { body: string }): Promise<void> {
428-
return this.doReviewCommand(context, ReviewType.RequestChanges, (body) => this.requestChanges(body));
351+
private async requestChangesCommand(context: { body: string }): Promise<void> {
352+
await this.doReviewCommand(context, ReviewType.RequestChanges, (body) => this.requestChanges(body));
429353
}
430354

431-
private requestChangesMessage(message: IRequestMessage<string>): Promise<void> {
432-
return this.doReviewMessage(message, (body) => this.requestChanges(body));
355+
private async requestChangesMessage(message: IRequestMessage<string>): Promise<void> {
356+
await this.doReviewMessage(message, (body) => this.requestChanges(body));
433357
}
434358

435359
private submitReview(body: string): Promise<ReviewEvent> {
@@ -445,69 +369,24 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W
445369
}
446370

447371
private async deleteBranch(message: IRequestMessage<any>) {
448-
const result = await PullRequestView.deleteBranch(this._folderRepositoryManager, this._item);
372+
const result = await PullRequestReviewCommon.deleteBranch(this._folderRepositoryManager, this._item);
449373
if (result.isReply) {
450374
this._replyMessage(message, result.message);
451375
} else {
452376
this._postMessage(result.message);
453377
}
454378
}
455379

456-
private setReadyForReview(message: IRequestMessage<Record<string, unknown>>): void {
457-
this._item
458-
.setReadyForReview()
459-
.then(result => {
460-
this._replyMessage(message, result);
461-
})
462-
.catch(e => {
463-
vscode.window.showErrorMessage(vscode.l10n.t('Unable to set pull request ready for review. {0}', formatError(e)));
464-
this._throwError(message, '');
465-
});
380+
private async setReadyForReview(message: IRequestMessage<Record<string, unknown>>): Promise<void> {
381+
return PullRequestReviewCommon.setReadyForReview(this.getReviewContext(), message);
466382
}
467383

468384
private async readyForReviewCommand(): Promise<void> {
469-
this._postMessage({
470-
command: 'pr.readying-for-review'
471-
});
472-
try {
473-
const result = await this._item.setReadyForReview();
474-
475-
const readiedResult: ReadyForReviewReply = {
476-
isDraft: result.isDraft
477-
};
478-
await this._postMessage({
479-
command: 'pr.readied-for-review',
480-
result: readiedResult
481-
});
482-
} catch (e) {
483-
vscode.window.showErrorMessage(`Unable to set pull request ready for review. ${formatError(e)}`);
484-
this._throwError(undefined, e.message);
485-
}
385+
return PullRequestReviewCommon.readyForReviewCommand(this.getReviewContext());
486386
}
487387

488388
private async readyForReviewAndMergeCommand(context: { mergeMethod: MergeMethod }): Promise<void> {
489-
this._postMessage({
490-
command: 'pr.readying-for-review'
491-
});
492-
try {
493-
const [readyResult, approveResult] = await Promise.all([this._item.setReadyForReview(), this._item.approve(this._folderRepositoryManager.repository)]);
494-
await this._item.enableAutoMerge(context.mergeMethod);
495-
this.updateReviewers(approveResult);
496-
497-
const readiedResult: ReadyForReviewReply = {
498-
isDraft: readyResult.isDraft,
499-
autoMerge: true,
500-
reviewEvent: approveResult,
501-
reviewers: this._existingReviewers
502-
};
503-
await this._postMessage({
504-
command: 'pr.readied-for-review',
505-
result: readiedResult
506-
});
507-
} catch (e) {
508-
vscode.window.showErrorMessage(`Unable to set pull request ready for review. ${formatError(e)}`);
509-
this._throwError(undefined, e.message);
510-
}
389+
return PullRequestReviewCommon.readyForReviewAndMergeCommand(this.getReviewContext(), context);
511390
}
512391

513392
private async mergePullRequest(

0 commit comments

Comments
 (0)