Skip to content

Commit f21a3b0

Browse files
IllusionMHRachel Macfarlane
IllusionMH
authored and
Rachel Macfarlane
committed
Show suggested reviewers and limit list to assignable users, fixes #1424 
1 parent 1be7acd commit f21a3b0

10 files changed

Lines changed: 283 additions & 38 deletions

File tree

‎src/github/enterprise.gql‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,16 @@ query PullRequest($owner: String!, $name: String!, $number: Int!) {
248248
mergeable
249249
id
250250
databaseId
251+
suggestedReviewers {
252+
isAuthor
253+
isCommenter
254+
reviewer {
255+
login
256+
avatarUrl
257+
name
258+
url
259+
}
260+
}
251261
}
252262
}
253263
rateLimit {
@@ -382,4 +392,27 @@ query GetMentionableUsers($owner: String!, $name: String!, $first: Int!, $after:
382392
remaining
383393
resetAt
384394
}
385-
}
395+
}
396+
397+
query GetAssignableUsers($owner: String!, $name: String!, $first: Int!, $after: String) {
398+
repository(owner: $owner, name: $name) {
399+
assignableUsers(first: $first, after: $after) {
400+
nodes {
401+
login
402+
avatarUrl
403+
name
404+
url
405+
}
406+
pageInfo {
407+
hasNextPage
408+
endCursor
409+
}
410+
}
411+
}
412+
rateLimit {
413+
limit
414+
cost
415+
remaining
416+
resetAt
417+
}
418+
}

‎src/github/githubRepository.ts‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { AuthenticationError } from '../common/authentication';
1414
import { QueryOptions, MutationOptions, ApolloQueryResult, NetworkStatus, FetchResult } from 'apollo-boost';
1515
import { PRCommentController } from '../view/prCommentController';
1616
import { convertRESTPullRequestToRawPullRequest, parseGraphQLPullRequest, parseMergeability } from './utils';
17-
import { PullRequestResponse, MentionableUsersResponse } from './graphql';
17+
import { PullRequestResponse, MentionableUsersResponse, AssignableUsersResponse } from './graphql';
1818

1919
export const PULL_REQUEST_PAGE_SIZE = 20;
2020

@@ -444,6 +444,50 @@ export class GitHubRepository implements vscode.Disposable {
444444
return [];
445445
}
446446

447+
async getAssignableUsers(): Promise<IAccount[]> {
448+
Logger.debug(`Fetch assignable users - enter`, GitHubRepository.ID);
449+
const { query, supportsGraphQl, remote, schema } = await this.ensure();
450+
451+
if (supportsGraphQl) {
452+
let after = null;
453+
let hasNextPage = false;
454+
const ret: IAccount[] = [];
455+
456+
do {
457+
try {
458+
const result: { data: AssignableUsersResponse } = await query<AssignableUsersResponse>({
459+
query: schema.GetAssignableUsers,
460+
variables: {
461+
owner: remote.owner,
462+
name: remote.repositoryName,
463+
first: 100,
464+
after: after
465+
}
466+
});
467+
468+
ret.push(...result.data.repository.assignableUsers.nodes.map(node => {
469+
return {
470+
login: node.login,
471+
avatarUrl: node.avatarUrl,
472+
name: node.name,
473+
url: node.url
474+
};
475+
}));
476+
477+
hasNextPage = result.data.repository.assignableUsers.pageInfo.hasNextPage;
478+
after = result.data.repository.assignableUsers.pageInfo.endCursor;
479+
} catch (e) {
480+
Logger.debug(`Unable to fetch assignable users: ${e}`, GitHubRepository.ID);
481+
return ret;
482+
}
483+
} while (hasNextPage);
484+
485+
return ret;
486+
}
487+
488+
return [];
489+
}
490+
447491
private getPRFetchQuery(repo: string, user: string, query: string) {
448492
const filter = query.replace('${user}', user);
449493
return `is:open ${filter} type:pr repo:${repo}`;

‎src/github/graphql.ts‎

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -205,14 +205,30 @@ export interface PullRequestCommentsResponse {
205205
export interface MentionableUsersResponse {
206206
repository: {
207207
mentionableUsers: {
208-
nodes: [
209-
{
210-
login: string;
211-
avatarUrl: string;
212-
name: string;
213-
url: string;
214-
}
215-
];
208+
nodes: {
209+
login: string;
210+
avatarUrl: string;
211+
name: string;
212+
url: string;
213+
}[];
214+
pageInfo: {
215+
hasNextPage: boolean;
216+
endCursor: string;
217+
};
218+
}
219+
};
220+
rateLimit: RateLimit;
221+
}
222+
223+
export interface AssignableUsersResponse {
224+
repository: {
225+
assignableUsers: {
226+
nodes: {
227+
login: string;
228+
avatarUrl: string;
229+
name: string;
230+
url: string;
231+
}[];
216232
pageInfo: {
217233
hasNextPage: boolean;
218234
endCursor: string;
@@ -300,6 +316,17 @@ export interface Ref {
300316
};
301317
}
302318

319+
export interface SuggestedReviewerResponse {
320+
isAuthor: boolean;
321+
isCommenter: boolean;
322+
reviewer: {
323+
login: string;
324+
avatarUrl: string;
325+
name: string;
326+
url: string;
327+
};
328+
}
329+
303330
export interface PullRequestResponse {
304331
repository: {
305332
pullRequest: {
@@ -323,11 +350,12 @@ export interface PullRequestResponse {
323350
labels: {
324351
nodes: {
325352
name: string;
326-
}[],
353+
}[];
327354
}
328355
merged: boolean;
329356
mergeable: 'MERGEABLE' | 'CONFLICTING' | 'UNKNOWN';
330357
isDraft?: boolean;
358+
suggestedReviewers: SuggestedReviewerResponse[];
331359
}
332360
};
333361
rateLimit: RateLimit;
@@ -341,4 +369,4 @@ export interface RateLimit {
341369
cost: number;
342370
remaining: number;
343371
resetAt: string;
344-
}
372+
}

‎src/github/interface.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,11 @@ export interface IAccount {
3939
url: string;
4040
}
4141

42+
export interface ISuggestedReviewer extends IAccount {
43+
isAuthor: boolean;
44+
isCommenter: boolean;
45+
}
46+
4247
export interface MergePullRequest {
4348
sha: string;
4449
merged: boolean;
@@ -80,6 +85,7 @@ export interface PullRequest {
8085
merged: boolean;
8186
mergeable: PullRequestMergeability;
8287
isDraft?: boolean;
88+
suggestedReviewers: ISuggestedReviewer[];
8389
}
8490

8591
export interface IRawFileChange {

‎src/github/pullRequestManager.ts‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import { Repository, RefType, UpstreamRef } from '../api/api';
2020
import Logger from '../common/logger';
2121
import { EXTENSION_ID } from '../constants';
2222
import { fromPRUri } from '../common/uri';
23-
import { convertRESTPullRequestToRawPullRequest, convertPullRequestsGetCommentsResponseItemToComment, convertIssuesCreateCommentResponseToComment, parseGraphQLTimelineEvents, convertRESTTimelineEvents, getRelatedUsersFromTimelineEvents, parseGraphQLComment, getReactionGroup, convertRESTUserToAccount, convertRESTReviewEvent, parseGraphQLReviewEvent } from './utils';
23+
import { convertRESTPullRequestToRawPullRequest, convertPullRequestsGetCommentsResponseItemToComment, convertIssuesCreateCommentResponseToComment, parseGraphQLTimelineEvents, convertRESTTimelineEvents, getRelatedUsersFromTimelineEvents, parseGraphQLComment, getReactionGroup, convertRESTUserToAccount, convertRESTReviewEvent, parseGraphQLReviewEvent, loginComparator } from './utils';
2424
import { PendingReviewIdResponse, TimelineEventsResponse, PullRequestCommentsResponse, AddCommentResponse, SubmitReviewResponse, DeleteReviewResponse, EditCommentResponse, DeleteReactionResponse, AddReactionResponse, MarkPullRequestReadyForReviewResponse, PullRequestState } from './graphql';
2525
import { ITelemetry } from '../common/telemetry';
2626
import { ApiImpl } from '../api/api1';
@@ -114,6 +114,8 @@ export class PullRequestManager implements vscode.Disposable {
114114
private _allGitHubRemotes: Remote[] = [];
115115
private _mentionableUsers?: { [key: string]: IAccount[] };
116116
private _fetchMentionableUsersPromise?: Promise<{ [key: string]: IAccount[] }>;
117+
private _assignableUsers?: { [key: string]: IAccount[] };
118+
private _fetchAssignableUsersPromise?: Promise<{ [key: string]: IAccount[] }>;
117119
private _gitBlameCache: { [key: string]: string } = {};
118120
private _githubManager: GitHubManager;
119121
private _repositoryPageInformation: Map<string, PageInformation> = new Map<string, PageInformation>();
@@ -459,6 +461,7 @@ export class PullRequestManager implements vscode.Disposable {
459461
|| !oldRepositories.every(oldRepo => this._githubRepositories.some(newRepo => newRepo.remote.equals(oldRepo.remote)));
460462

461463
this.getMentionableUsers(repositoriesChanged);
464+
this.getAssignableUsers(repositoriesChanged);
462465
this.state = hasAuthenticated || !activeRemotes.length ? PRManagerState.RepositoriesLoaded : PRManagerState.NeedsAuthentication;
463466
return Promise.resolve();
464467
});
@@ -493,6 +496,35 @@ export class PullRequestManager implements vscode.Disposable {
493496
return this._fetchMentionableUsersPromise;
494497
}
495498

499+
async getAssignableUsers(clearCache?: boolean): Promise<{ [key: string]: IAccount[] }> {
500+
if (clearCache) {
501+
delete this._assignableUsers;
502+
}
503+
504+
if (this._assignableUsers) {
505+
return this._assignableUsers;
506+
}
507+
508+
if (!this._fetchAssignableUsersPromise) {
509+
const cache: { [key: string]: IAccount[] } = {};
510+
return this._fetchAssignableUsersPromise = new Promise((resolve) => {
511+
const promises = this._githubRepositories.map(async githubRepository => {
512+
const data = await githubRepository.getAssignableUsers();
513+
cache[githubRepository.remote.remoteName] = data.sort(loginComparator);
514+
return;
515+
});
516+
517+
Promise.all(promises).then(() => {
518+
this._assignableUsers = cache;
519+
this._fetchAssignableUsersPromise = undefined;
520+
resolve(cache);
521+
});
522+
});
523+
}
524+
525+
return this._fetchAssignableUsersPromise;
526+
}
527+
496528
/**
497529
* Returns the remotes that are currently active, which is those that are important by convention (origin, upstream),
498530
* or the remotes configured by the setting githubPullRequests.remotes

‎src/github/pullRequestModel.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import * as vscode from 'vscode';
77
import { GitHubRef } from '../common/githubRef';
88
import { Remote } from '../common/remote';
99
import { GitHubRepository } from './githubRepository';
10-
import { IAccount, PullRequest, PullRequestStateEnum } from './interface';
10+
import { IAccount, PullRequest, PullRequestStateEnum, ISuggestedReviewer } from './interface';
1111

1212
interface IPullRequestModel {
1313
head: GitHubRef | null;
@@ -31,6 +31,7 @@ export class PullRequestModel implements IPullRequestModel {
3131
public localBranchName?: string;
3232
public mergeBase?: string;
3333
public isDraft?: boolean;
34+
public suggestedReviewers: ISuggestedReviewer[];
3435

3536
public get isOpen(): boolean {
3637
return this.state === PullRequestStateEnum.Open;
@@ -106,6 +107,7 @@ export class PullRequestModel implements IPullRequestModel {
106107
this.html_url = prItem.url;
107108
this.author = prItem.user;
108109
this.isDraft = prItem.isDraft;
110+
this.suggestedReviewers = prItem.suggestedReviewers;
109111

110112
if (prItem.state.toLowerCase() === 'open') {
111113
this.state = PullRequestStateEnum.Open;

‎src/github/pullRequestOverview.ts‎

Lines changed: 59 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import * as path from 'path';
88
import * as vscode from 'vscode';
99
import Octokit = require('@octokit/rest');
10-
import { PullRequestStateEnum, ReviewEvent, ReviewState, ILabel, IAccount, MergeMethodsAvailability, MergeMethod, PullRequestMergeability } from './interface';
10+
import { PullRequestStateEnum, ReviewEvent, ReviewState, ILabel, IAccount, MergeMethodsAvailability, MergeMethod, PullRequestMergeability, ISuggestedReviewer } from './interface';
1111
import { onDidUpdatePR } from '../commands';
1212
import { formatError } from '../common/utils';
1313
import { GitErrorCodes } from '../api/api';
@@ -350,30 +350,71 @@ export class PullRequestOverviewPanel {
350350
}
351351
}
352352

353+
private getReviewersQuickPickItems(assignableUsers: IAccount[], suggestedReviewers: ISuggestedReviewer[]): vscode.QuickPickItem[] {
354+
// used to track logins that shouldn't be added to pick list
355+
// e.g. author, existing and already added reviewers
356+
const skipList: Set<string> = new Set([
357+
this._pullRequest.author.login,
358+
...this._existingReviewers.map(reviewer => reviewer.reviewer.login)
359+
]);
360+
361+
const reviewers: vscode.QuickPickItem[] = [];
362+
for (const { login, name, isAuthor, isCommenter } of suggestedReviewers) {
363+
if (skipList.has(login)) {
364+
continue;
365+
}
366+
367+
const suggestionReason: string =
368+
isAuthor && isCommenter
369+
? 'Recently edited and reviewed changes to these files'
370+
: isAuthor
371+
? 'Recently edited these files'
372+
: isCommenter
373+
? 'Recently reviewed changes to these files'
374+
: 'Suggested reviewer';
375+
376+
reviewers.push({
377+
label: login,
378+
description: name,
379+
detail: suggestionReason
380+
});
381+
// this user shouldn't be added later from assignable users list
382+
skipList.add(login);
383+
}
384+
385+
for (const { login, name } of assignableUsers) {
386+
if (skipList.has(login)) {
387+
continue;
388+
}
389+
390+
reviewers.push({
391+
label: login,
392+
description: name
393+
});
394+
}
395+
396+
return reviewers;
397+
}
398+
353399
private async addReviewers(message: IRequestMessage<void>): Promise<void> {
354400
try {
355-
const allMentionableUsers = await this._pullRequestManager.getMentionableUsers();
356-
const mentionableUsers = allMentionableUsers[this._pullRequest.remote.remoteName];
357-
const newReviewers = mentionableUsers
358-
.filter(user =>
359-
!this._existingReviewers.some(reviewer => reviewer.reviewer.login === user.login)
360-
&& user.login !== this._pullRequest.author.login);
361-
362-
const reviewersToAdd = await vscode.window.showQuickPick(newReviewers.map(reviewer => {
363-
return {
364-
label: reviewer.login,
365-
description: reviewer.name
366-
};
367-
}), {
368-
canPickMany: true,
369-
matchOnDescription: true
370-
});
401+
const allAssignableUsers = await this._pullRequestManager.getAssignableUsers();
402+
const assignableUsers = allAssignableUsers[this._pullRequest.remote.remoteName];
403+
404+
const reviewersToAdd = await vscode.window.showQuickPick(
405+
this.getReviewersQuickPickItems(assignableUsers, this._pullRequest.suggestedReviewers),
406+
{
407+
canPickMany: true,
408+
matchOnDescription: true
409+
}
410+
);
371411

372412
if (reviewersToAdd) {
373413
await this._pullRequestManager.requestReview(this._pullRequest, reviewersToAdd.map(r => r.label));
374414
const addedReviewers: ReviewState[] = reviewersToAdd.map(reviewer => {
375415
return {
376-
reviewer: newReviewers.find(r => r.login === reviewer.label)!,
416+
// assumes that suggested reviewers will be a subset of assignable users
417+
reviewer: assignableUsers.find(r => r.login === reviewer.label)!,
377418
state: 'REQUESTED'
378419
};
379420
});

0 commit comments

Comments
 (0)