Skip to content

Commit ec53fb6

Browse files
author
Rachel Macfarlane
authored
Add a way to approve/request changes on a PR, fixes #109
1 parent 51d89d8 commit ec53fb6

File tree

9 files changed

+232
-79
lines changed

9 files changed

+232
-79
lines changed

preview-src/index.css

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,10 @@ body .comment-form textarea {
285285
resize: none;
286286
}
287287

288+
.reply-button {
289+
margin-left: auto;
290+
}
291+
288292
body .comment-form textarea:focus {
289293
outline: 1px solid var(--vscode-focusBorder);
290294
}
@@ -293,6 +297,7 @@ body .comment-form .form-actions {
293297
overflow: auto;
294298
padding-top: 20px;
295299
margin-bottom: 10px;
300+
display: flex;
296301
}
297302

298303
body button:disabled {

preview-src/index.ts

Lines changed: 54 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55
import './index.css';
6-
import { renderTimelineEvent, getStatus, renderComment, PullRequestStateEnum } from './pullRequestOverviewRenderer';
6+
import { renderTimelineEvent, getStatus, renderComment, PullRequestStateEnum, renderReview } from './pullRequestOverviewRenderer';
77
import md from './mdRenderer';
88

99
declare var acquireVsCodeApi: any;
@@ -14,6 +14,8 @@ const ElementIds = {
1414
CheckoutDefaultBranch: 'checkout-default-branch',
1515
Close: 'close',
1616
Reply: 'reply',
17+
Approve: 'approve',
18+
RequestChanges: 'request-changes',
1719
Status: 'status',
1820
CommentTextArea: 'comment-textarea',
1921
TimelineEvents:'timeline-events' // If updating this value, change id in pullRequestOverview.ts as well.
@@ -33,6 +35,16 @@ function handleMessage(event: any) {
3335
break;
3436
case 'pr.append-comment':
3537
appendComment(message.value);
38+
break;
39+
case 'pr.append-review':
40+
appendReview(message.value);
41+
break;
42+
case 'pr.enable-approve':
43+
(<HTMLButtonElement>document.getElementById(ElementIds.Approve)).disabled = false;
44+
break;
45+
case 'pr.enable-request-changes':
46+
(<HTMLButtonElement>document.getElementById(ElementIds.RequestChanges)).disabled = false;
47+
break;
3648
default:
3749
break;
3850
}
@@ -93,10 +105,13 @@ function addEventListeners(pr: any) {
93105
});
94106
});
95107

96-
// Enable 'Comment' button only when the user has entered text
108+
// Enable 'Comment' and 'RequestChanges' button only when the user has entered text
97109
document.getElementById(ElementIds.CommentTextArea)!.addEventListener('input', (e) => {
98-
(<HTMLButtonElement>document.getElementById(ElementIds.Reply)).disabled = !(<any>e.target).value;
99-
})
110+
const hasNoText = !(<any>e.target).value;
111+
(<HTMLButtonElement>document.getElementById(ElementIds.Reply)).disabled = hasNoText;
112+
(<HTMLButtonElement>document.getElementById(ElementIds.RequestChanges)).disabled = hasNoText;
113+
114+
});
100115

101116
document.getElementById(ElementIds.Reply)!.addEventListener('click', () => {
102117
submitComment();
@@ -111,6 +126,24 @@ function addEventListeners(pr: any) {
111126
});
112127
});
113128

129+
document.getElementById(ElementIds.Approve)!.addEventListener('click', () => {
130+
(<HTMLButtonElement>document.getElementById(ElementIds.Approve)).disabled = true;
131+
const inputBox = (<HTMLTextAreaElement>document.getElementById(ElementIds.CommentTextArea));
132+
vscode.postMessage({
133+
command: 'pr.approve',
134+
text: inputBox.value
135+
});
136+
});
137+
138+
document.getElementById(ElementIds.RequestChanges)!.addEventListener('click', () => {
139+
(<HTMLButtonElement>document.getElementById(ElementIds.RequestChanges)).disabled = true;
140+
const inputBox = (<HTMLTextAreaElement>document.getElementById(ElementIds.CommentTextArea));
141+
vscode.postMessage({
142+
command: 'pr.request-changes',
143+
text: inputBox.value
144+
});
145+
});
146+
114147
document.getElementById(ElementIds.CheckoutDefaultBranch)!.addEventListener('click', () => {
115148
(<HTMLButtonElement>document.getElementById(ElementIds.CheckoutDefaultBranch)).disabled = true;
116149
vscode.postMessage({
@@ -120,6 +153,12 @@ function addEventListeners(pr: any) {
120153
});
121154
}
122155

156+
function clearTextArea() {
157+
(<HTMLTextAreaElement>document.getElementById(ElementIds.CommentTextArea)!).value = '';
158+
(<HTMLButtonElement>document.getElementById(ElementIds.Reply)).disabled = true;
159+
(<HTMLButtonElement>document.getElementById(ElementIds.RequestChanges)).disabled = true;
160+
}
161+
123162
function submitComment() {
124163
(<HTMLButtonElement>document.getElementById(ElementIds.Reply)).disabled = true;
125164
vscode.postMessage({
@@ -129,10 +168,16 @@ function submitComment() {
129168

130169
}
131170

171+
function appendReview(review: any): void {
172+
const newReview = renderReview(review);
173+
document.getElementById(ElementIds.TimelineEvents)!.insertAdjacentHTML('beforeend', newReview);
174+
clearTextArea();
175+
}
176+
132177
function appendComment(comment: any) {
133178
let newComment = renderComment(comment);
134179
document.getElementById(ElementIds.TimelineEvents)!.insertAdjacentHTML('beforeend', newComment);
135-
(<HTMLTextAreaElement>document.getElementById(ElementIds.CommentTextArea)!).value = '';
180+
clearTextArea();
136181
}
137182

138183
function updateCheckoutButton(isCheckedOut: boolean) {
@@ -156,8 +201,10 @@ function updateCheckoutButton(isCheckedOut: boolean) {
156201
function setTextArea() {
157202
document.getElementById('comment-form')!.innerHTML = `<textarea id="${ElementIds.CommentTextArea}"></textarea>
158203
<div class="form-actions">
159-
<button class="reply-button" id="${ElementIds.Reply}" disabled="true"></button>
160-
<button class="close-button" id="${ElementIds.Close}"></button>
204+
<button id="${ElementIds.Close}">Close Pull Request</button>
205+
<button id="${ElementIds.RequestChanges}" disabled="true">Request Changes</button>
206+
<button id="${ElementIds.Approve}">Approve</button>
207+
<button class="reply-button" id="${ElementIds.Reply}" disabled="true">Comment</button>
161208
</div>`;
162209

163210
(<HTMLTextAreaElement>document.getElementById(ElementIds.CommentTextArea)!).placeholder = 'Leave a comment';
@@ -172,6 +219,4 @@ function setTextArea() {
172219
return;
173220
}
174221
});
175-
(<HTMLButtonElement>document.getElementById(ElementIds.Reply)!).textContent = 'Comment';
176-
(<HTMLButtonElement>document.getElementById(ElementIds.Close)!).textContent = 'Close Pull Request';
177222
}

preview-src/pullRequestOverviewRenderer.ts

Lines changed: 30 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ export function renderReview(timelineEvent: ReviewEvent): string {
290290
}
291291

292292
let reviewState = '';
293-
switch (timelineEvent.state) {
293+
switch (timelineEvent.state.toLowerCase()) {
294294
case 'approved':
295295
reviewState = `<span><a href="${timelineEvent.user.html_url}">${timelineEvent.user.login}</a> approved these changes</span>`
296296
break;
@@ -306,43 +306,45 @@ export function renderReview(timelineEvent: ReviewEvent): string {
306306

307307
let reviewBody = timelineEvent.body ? `${md.render(timelineEvent.body)}` : '';
308308

309-
let groups = groupBy(timelineEvent.comments, comment => comment.path + ':' + (comment.position !== null ? `pos:${comment.position}` : `ori:${comment.original_position}`));
310309
let body = '';
311-
let avatar = '';
312-
313-
for (let path in groups) {
314-
let comments = groups[path];
315-
let diffView = '';
316-
let diffLines = [];
317-
if (comments && comments.length) {
318-
avatar = `<div class="avatar-container">
319-
<a href="${timelineEvent.comments[0].user.html_url}"><img class="avatar" src="${timelineEvent.comments[0].user.avatar_url}"></a>
320-
</div>`;
321-
for (let i = 0; i < comments[0].diff_hunks.length; i++) {
322-
diffLines.push(comments[0].diff_hunks[i].diffLines.slice(-4).map(diffLine => `<div class="diffLine ${getDiffChangeClass(diffLine.type)}">
323-
<span class="lineNumber old">${diffLine.oldLineNumber > 0 ? diffLine.oldLineNumber : ' '}</span>
324-
<span class="lineNumber new">${diffLine.newLineNumber > 0 ? diffLine.newLineNumber : ' '}</span>
325-
<span class="lineContent">${(diffLine as any)._raw}</span>
326-
</div>`).join(''));
310+
if (timelineEvent.comments) {
311+
let groups = groupBy(timelineEvent.comments, comment => comment.path + ':' + (comment.position !== null ? `pos:${comment.position}` : `ori:${comment.original_position}`));
312+
313+
for (let path in groups) {
314+
let comments = groups[path];
315+
let diffView = '';
316+
let diffLines = [];
317+
if (comments && comments.length) {
318+
for (let i = 0; i < comments[0].diff_hunks.length; i++) {
319+
diffLines.push(comments[0].diff_hunks[i].diffLines.slice(-4).map(diffLine => `<div class="diffLine ${getDiffChangeClass(diffLine.type)}">
320+
<span class="lineNumber old">${diffLine.oldLineNumber > 0 ? diffLine.oldLineNumber : ' '}</span>
321+
<span class="lineNumber new">${diffLine.newLineNumber > 0 ? diffLine.newLineNumber : ' '}</span>
322+
<span class="lineContent">${(diffLine as any)._raw}</span>
323+
</div>`).join(''));
324+
}
325+
326+
diffView = `<div class="diff">
327+
<div class="diffHeader">${comments[0].path}</div>
328+
${diffLines.join('')}
329+
</div>`;
327330
}
328331

329-
diffView = `<div class="diff">
330-
<div class="diffHeader">${comments[0].path}</div>
331-
${diffLines.join('')}
332-
</div>`;
332+
body += `
333+
${diffView}
334+
<div>${ comments && comments.length ? comments.map(comment => renderComment(comment)).join('') : ''}</div>
335+
`;
333336
}
334-
335-
body += `
336-
${diffView}
337-
<div>${ comments && comments.length ? comments.map(comment => renderComment(comment)).join('') : ''}</div>
338-
`;
339337
}
338+
339+
340340
return `<div class="comment-container" data-type="review">
341341
342342
<div class="review-comment" role="treeitem">
343343
344344
<div class="review-comment-contents review">
345-
${avatar}
345+
<div class="avatar-container">
346+
<a href="${timelineEvent.user.html_url}"><img class="avatar" src="${timelineEvent.user.avatar_url}"></a>
347+
</div>
346348
<div class="review-comment-container">
347349
<div class="review-comment-header">
348350
${reviewState}

src/commands.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,14 @@ export function registerCommands(context: vscode.ExtensionContext, prManager: IP
102102
});
103103
}));
104104

105+
context.subscriptions.push(vscode.commands.registerCommand('pr.approve', async (pr: IPullRequestModel, message?: string) => {
106+
return await prManager.approvePullRequest(pr, message);
107+
}));
108+
109+
context.subscriptions.push(vscode.commands.registerCommand('pr.requestChanges', async (pr: IPullRequestModel, message?: string) => {
110+
return await prManager.requestChanges(pr, message);
111+
}));
112+
105113
context.subscriptions.push(vscode.commands.registerCommand('pr.openDescription', async (pr: IPullRequestModel) => {
106114
const pullRequest = ensurePR(prManager, pr);
107115
// Create and show a new webview

src/common/utils.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,13 @@ export function formatError(e: any): string {
9898
if (message) {
9999
errorMessage = message.message;
100100

101-
const furtherInfo = message.errors && message.errors.map(error => error.message).join(', ');
101+
const furtherInfo = message.errors && message.errors.map(error => {
102+
if (typeof error === "string") {
103+
return error;
104+
} else {
105+
return error.message;
106+
}
107+
}).join(', ');
102108
if (furtherInfo) {
103109
errorMessage = `${errorMessage}: ${furtherInfo}`;
104110
}

src/github/interface.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@ export enum PRType {
1313
LocalPullRequest = 5
1414
}
1515

16+
export enum ReviewEvent {
17+
Approve = "APPROVE",
18+
RequestChanges = "REQUEST_CHANGES",
19+
Comment = "COMMENT"
20+
}
21+
1622
export enum PullRequestStateEnum {
1723
Open,
1824
Merged,
@@ -140,6 +146,8 @@ export interface IPullRequestManager {
140146
createCommentReply(pullRequest: IPullRequestModel, body: string, reply_to: string): Promise<Comment>;
141147
createComment(pullRequest: IPullRequestModel, body: string, path: string, position: number): Promise<Comment>;
142148
closePullRequest(pullRequest: IPullRequestModel): Promise<any>;
149+
approvePullRequest(pullRequest: IPullRequestModel, message?: string): Promise<any>;
150+
requestChanges(pullRequest: IPullRequestModel, message?: string): Promise<any>;
143151
getPullRequestChangedFiles(pullRequest: IPullRequestModel): Promise<FileChange[]>;
144152
getPullRequestRepositoryDefaultBranch(pullRequest: IPullRequestModel): Promise<string>;
145153

src/github/pullRequestManager.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { Remote } from "../common/remote";
1010
import { Repository } from "../common/repository";
1111
import { TimelineEvent, EventType } from "../common/timelineEvent";
1212
import { GitHubRepository, PULL_REQUEST_PAGE_SIZE } from "./githubRepository";
13-
import { IPullRequestManager, IPullRequestModel, IPullRequestsPagingOptions, PRType, Commit, FileChange } from "./interface";
13+
import { IPullRequestManager, IPullRequestModel, IPullRequestsPagingOptions, PRType, Commit, FileChange, ReviewEvent } from "./interface";
1414
import { PullRequestGitHelper } from "./pullRequestGitHelper";
1515
import { PullRequestModel } from "./pullRequestModel";
1616
import { parserCommentDiffHunk } from "../common/diffHunk";
@@ -348,6 +348,28 @@ export class PullRequestManager implements IPullRequestManager {
348348
return ret.data;
349349
}
350350

351+
private async createReview(pullRequest: IPullRequestModel, event: ReviewEvent, message?: string): Promise<any> {
352+
const { octokit, remote } = await (pullRequest as PullRequestModel).githubRepository.ensure();
353+
354+
let ret = await octokit.pullRequests.createReview({
355+
owner: remote.owner,
356+
repo: remote.repositoryName,
357+
number: pullRequest.prNumber,
358+
event: event,
359+
body: message,
360+
});
361+
362+
return ret.data;
363+
}
364+
365+
async requestChanges(pullRequest: IPullRequestModel, message?: string): Promise<any> {
366+
return this.createReview(pullRequest, ReviewEvent.RequestChanges, message);
367+
}
368+
369+
async approvePullRequest(pullRequest: IPullRequestModel, message?: string): Promise<any> {
370+
return this.createReview(pullRequest, ReviewEvent.Approve, message);
371+
}
372+
351373
async getPullRequestChangedFiles(pullRequest: IPullRequestModel): Promise<FileChange[]> {
352374
const { octokit, remote } = await (pullRequest as PullRequestModel).githubRepository.ensure();
353375

0 commit comments

Comments
 (0)