Fix changing reviewers when one of the reviewers is a bot (#6628)

* Fix changing reviewers when one of the reviewers is a bot
Fixes #6443

* Fixe tests

src/common/timelineEvent.ts

@@ -47,6 +47,8 @@ export interface ReviewResolveInfo {
 	isResolved: boolean;
 }
 
+export type ReviewStateValue = 'COMMENTED' | 'APPROVED' | 'CHANGES_REQUESTED' | 'PENDING' | 'REQUESTED';
+
 export interface ReviewEvent {
 	id: number;
 	reviewThread?: ReviewResolveInfo
@@ -58,7 +60,7 @@ export interface ReviewEvent {
 	htmlUrl: string;
 	user: IAccount;
 	authorAssociation: string;
-	state?: 'COMMENTED' | 'APPROVED' | 'CHANGES_REQUESTED' | 'PENDING' | 'REQUESTED';
+	state?: ReviewStateValue;
 }
 
 export interface CommitEvent {

src/github/activityBarViewProvider.ts

@@ -12,7 +12,7 @@ import { formatError } from '../common/utils';
 import { getNonce, IRequestMessage, WebviewViewBase } from '../common/webview';
 import { ReviewManager } from '../view/reviewManager';
 import { FolderRepositoryManager } from './folderRepositoryManager';
-import { GithubItemStateEnum, IAccount, isTeam, PullRequestMergeability, reviewerId, ReviewEvent, ReviewState } from './interface';
+import { GithubItemStateEnum, IAccount, isTeam, ITeam, PullRequestMergeability, reviewerId, ReviewEvent, ReviewState } from './interface';
 import { PullRequestModel } from './pullRequestModel';
 import { getDefaultMergeMethod } from './pullRequestOverview';
 import { PullRequestView } from './pullRequestOverviewCommon';
@@ -153,12 +153,12 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W
 
 	private reRequestReview(message: IRequestMessage<string>): void {
 		let targetReviewer: ReviewState | undefined;
-		const userReviewers: string[] = [];
-		const teamReviewers: string[] = [];
+		const userReviewers: IAccount[] = [];
+		const teamReviewers: ITeam[] = [];
 
 		for (const reviewer of this._existingReviewers) {
 			let id: string | undefined;
-			let reviewerArray: string[] | undefined;
+			let reviewerArray: (IAccount | ITeam)[] | undefined;
 			if (reviewer && isTeam(reviewer.reviewer)) {
 				id = reviewer.reviewer.id;
 				reviewerArray = teamReviewers;
@@ -167,7 +167,7 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W
 				reviewerArray = userReviewers;
 			}
 			if (reviewerArray && id && ((reviewer.state === 'REQUESTED') || (id === message.args))) {
-				reviewerArray.push(id);
+				reviewerArray.push(reviewer.reviewer);
 				if (id === message.args) {
 					targetReviewer = reviewer;
 				}
@@ -288,7 +288,8 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W
 						avatarUrl: pullRequest.userAvatar,
 						url: pullRequest.author.url,
 						email: pullRequest.author.email,
-						id: pullRequest.author.id
+						id: pullRequest.author.id,
+						accountType: pullRequest.author.accountType,
 					},
 					state: pullRequest.state,
 					isCurrentlyCheckedOut: isCurrentlyCheckedOut,

src/github/createPRViewProvider.ts

@@ -306,13 +306,13 @@ export abstract class BaseCreatePullRequestViewProvider<T extends BasePullReques
 
 	private async setReviewers(pr: PullRequestModel, reviewers: (IAccount | ITeam)[]): Promise<void> {
 		if (reviewers.length) {
-			const users: string[] = [];
-			const teams: string[] = [];
+			const users: IAccount[] = [];
+			const teams: ITeam[] = [];
 			for (const reviewer of reviewers) {
 				if (isTeam(reviewer)) {
-					teams.push(reviewer.id);
+					teams.push(reviewer);
 				} else {
-					users.push(reviewer.id);
+					users.push(reviewer);
 				}
 			}
 			await pr.requestReview(users, teams);

src/github/githubRepository.ts

@@ -64,6 +64,7 @@ import {
 	getAvatarWithEnterpriseFallback,
 	getOverrideBranch,
 	isInCodespaces,
+	parseAccount,
 	parseGraphQLIssue,
 	parseGraphQLPullRequest,
 	parseGraphQLViewerPermission,
@@ -1177,14 +1178,7 @@ export class GitHubRepository extends Disposable {
 
 				ret.push(
 					...result.data.repository.mentionableUsers.nodes.map(node => {
-						return {
-							login: node.login,
-							avatarUrl: getAvatarWithEnterpriseFallback(node.avatarUrl, undefined, this.remote.isEnterprise),
-							name: node.name,
-							url: node.url,
-							email: node.email,
-							id: node.id
-						};
+						return parseAccount(node, this);
 					}),
 				);
 
@@ -1226,14 +1220,7 @@ export class GitHubRepository extends Disposable {
 
 				ret.push(
 					...result.data.repository.assignableUsers.nodes.map(node => {
-						return {
-							login: node.login,
-							avatarUrl: getAvatarWithEnterpriseFallback(node.avatarUrl, undefined, this.remote.isEnterprise),
-							name: node.name,
-							url: node.url,
-							email: node.email,
-							id: node.id
-						};
+						return parseAccount(node, this);
 					}),
 				);
 
@@ -1370,14 +1357,7 @@ export class GitHubRepository extends Disposable {
 
 			ret.push(
 				...result.data.repository.pullRequest.participants.nodes.map(node => {
-					return {
-						login: node.login,
-						avatarUrl: getAvatarWithEnterpriseFallback(node.avatarUrl, undefined, this.remote.isEnterprise),
-						name: node.name,
-						url: node.url,
-						email: node.email,
-						id: node.id
-					};
+					return parseAccount(node, this);
 				}),
 			);
 		} catch (e) {

src/github/graphql.ts

@@ -14,11 +14,7 @@ interface PageInfo {
 export interface MergedEvent {
 	__typename: string;
 	id: string;
-	actor: {
-		login: string;
-		avatarUrl: string;
-		url: string;
-	};
+	actor: Actor;
 	createdAt: string;
 	mergeRef: {
 		name: string;
@@ -33,23 +29,13 @@ export interface MergedEvent {
 export interface HeadRefDeletedEvent {
 	__typename: string;
 	id: string;
-	actor: {
-		login: string;
-		avatarUrl: string;
-		url: string;
-	};
+	actor: Actor;
 	createdAt: string;
 	headRefName: string;
 }
 
 export interface AbbreviatedIssueComment {
-	author: {
-		login: string;
-		avatarUrl: string;
-		url: string;
-		email?: string;
-		id: string;
-	};
+	author: Account;
 	body: string;
 	databaseId: number;
 	reactions: {
@@ -82,16 +68,28 @@ export interface ReactionGroup {
 	};
 }
 
-export interface Account {
+export interface Actor {
+	__typename: string;
+	id: string;
 	login: string;
 	avatarUrl: string;
-	name: string;
 	url: string;
+}
+
+export interface Account extends Actor {
+	name: string;
 	email: string;
-	id: string;
 }
 
-interface Team {
+export function isAccount(x: Actor | Team | undefined | null): x is Account {
+	return !!x && 'name' in x && 'email' in x;
+}
+
+export function isTeam(x: Actor | Team | undefined | null): x is Team {
+	return !!x && 'slug' in x;
+}
+
+export interface Team {
 	avatarUrl: string;
 	name: string;
 	url: string;
@@ -109,13 +107,7 @@ export interface ReviewComment {
 	id: string;
 	databaseId: number;
 	url: string;
-	author?: {
-		login: string;
-		avatarUrl: string;
-		url: string;
-		id: string;
-		name?: string;
-	};
+	author?: Actor | Account;
 	path: string;
 	originalPosition: number;
 	body: string;
@@ -146,12 +138,7 @@ export interface Commit {
 	id: string;
 	commit: {
 		author: {
-			user: {
-				login: string;
-				avatarUrl: string;
-				url: string;
-				id: string;
-			};
+			user: Account;
 		};
 		committer: {
 			avatarUrl: string;
@@ -168,21 +155,12 @@ export interface Commit {
 export interface AssignedEvent {
 	__typename: string;
 	id: number;
-	actor: {
-		login: string;
-		avatarUrl: string;
-		url: string;
-	};
-	user: {
-		login: string;
-		avatarUrl: string;
-		url: string;
-		id: string;
-	};
+	actor: Actor;
+	user: Account;
 }
 
 export interface MergeQueueEntry {
-	position: number,
+	position: number;
 	state: MergeQueueState;
 	mergeQueue: {
 		url: string;
@@ -195,12 +173,7 @@ export interface Review {
 	databaseId: number;
 	authorAssociation: string;
 	url: string;
-	author: {
-		login: string;
-		avatarUrl: string;
-		url: string;
-		id: string;
-	};
+	author: Actor | Account;
 	state: 'COMMENTED' | 'APPROVED' | 'CHANGES_REQUESTED' | 'PENDING';
 	body: string;
 	bodyHTML?: string;
@@ -271,18 +244,7 @@ export interface GetReviewRequestsResponse {
 		pullRequest: {
 			reviewRequests: {
 				nodes: {
-					requestedReviewer: {
-						// Shared properties between accounts and teams
-						avatarUrl: string;
-						url: string;
-						name: string;
-						// Account properties
-						login?: string;
-						email?: string;
-						// Team properties
-						slug?: string;
-						id: string;
-					} | null;
+					requestedReviewer: Actor | Account | Team | null;
 				}[];
 			};
 		};
@@ -554,13 +516,7 @@ export interface Ref {
 export interface SuggestedReviewerResponse {
 	isAuthor: boolean;
 	isCommenter: boolean;
-	reviewer: {
-		login: string;
-		avatarUrl: string;
-		name: string;
-		url: string;
-		id: string;
-	};
+	reviewer: Actor | Account;
 }
 
 export type MergeMethod = 'MERGE' | 'REBASE' | 'SQUASH';
@@ -577,20 +533,9 @@ export interface PullRequest {
 	title: string;
 	titleHTML: string;
 	assignees?: {
-		nodes: {
-			login: string;
-			url: string;
-			email: string;
-			avatarUrl: string;
-			id: string;
-		}[];
-	};
-	author: {
-		login: string;
-		url: string;
-		avatarUrl: string;
-		id: string;
+		nodes: Account[];
 	};
+	author: Account;
 	commits: {
 		nodes: {
 			commit: {
@@ -827,6 +772,7 @@ export interface UserResponse {
 		contributionsCollection: ContributionsCollection;
 		url: string;
 		id: string;
+		__typename: string;
 	};
 }
 

src/github/interface.ts

@@ -3,6 +3,8 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
+import { ReviewStateValue } from '../common/timelineEvent';
+
 export enum PRType {
 	Query,
 	All,
@@ -39,7 +41,7 @@ export enum MergeQueueState {
 
 export interface ReviewState {
 	reviewer: IAccount | ITeam;
-	state: string;
+	state: ReviewStateValue;
 }
 
 export interface ReadyForReview {
@@ -54,14 +56,35 @@ export interface IActor {
 	url: string;
 }
 
+export enum AccountType {
+	User = 'User',
+	Organization = 'Organization',
+	Mannequin = 'Mannequin',
+	Bot = 'Bot'
+}
+
+export function toAccountType(type: string): AccountType {
+	switch (type) {
+		case 'Organization':
+			return AccountType.Organization;
+		case 'Mannequin':
+			return AccountType.Mannequin;
+		case 'Bot':
+			return AccountType.Bot;
+		default:
+			return AccountType.User;
+	}
+}
+
 export interface IAccount extends IActor {
 	login: string;
 	id: string;
 	name?: string;
 	avatarUrl?: string;
 	url: string;
 	email?: string;
-	specialDisplayName?: string
+	specialDisplayName?: string;
+	accountType: AccountType;
 }
 
 export interface ITeam {