Polish team reviewers (#4742)

src/github/folderRepositoryManager.ts

@@ -880,14 +880,20 @@ export class FolderRepositoryManager implements vscode.Disposable {
 		if (!this._fetchTeamReviewersPromise) {
 			const cache: { [key: string]: ITeam[] } = {};
 			return (this._fetchTeamReviewersPromise = new Promise(async (resolve) => {
+				// Keep track of the org teams we have already gotten so we don't make duplicate calls
+				const orgTeams: Map<string, (ITeam & { repositoryNames: string[] })[]> = new Map();
 				// Go through one github repo at a time so that we don't make overlapping auth calls
 				for (const githubRepository of this._githubRepositories) {
-					try {
-						const data = await githubRepository.getTeams(refreshKind);
-						cache[githubRepository.remote.remoteName] = data.sort(teamComparator);
-					} catch (e) {
-						// ignore errors from getTeams
+					if (!orgTeams.has(githubRepository.remote.owner)) {
+						try {
+							const data = await githubRepository.getOrgTeams(refreshKind);
+							orgTeams.set(githubRepository.remote.owner, data);
+						} catch (e) {
+							// ignore errors from getTeams
+						}
 					}
+					const allTeamsForOrg = orgTeams.get(githubRepository.remote.owner) ?? [];
+					cache[githubRepository.remote.remoteName] = allTeamsForOrg.filter(team => team.repositoryNames.includes(githubRepository.remote.repositoryName)).sort(teamComparator);
 				}
 
 				this._teamReviewers = cache;

src/github/githubRepository.ts

@@ -1028,7 +1028,7 @@ export class GitHubRepository implements vscode.Disposable {
 		}
 	}
 
-	async getTeams(refreshKind: TeamReviewerRefreshKind): Promise<ITeam[]> {
+	async getOrgTeams(refreshKind: TeamReviewerRefreshKind): Promise<(ITeam & { repositoryNames: string[] })[]> {
 		Logger.debug(`Fetch Teams - enter`, GitHubRepository.ID);
 		if ((refreshKind === TeamReviewerRefreshKind.None) || (refreshKind === TeamReviewerRefreshKind.Try && !this._credentialStore.isAuthenticatedWithAdditionalScopes(this.remote.authProviderId))) {
 			Logger.debug(`Fetch Teams - exit without fetching teams`, GitHubRepository.ID);
@@ -1039,7 +1039,7 @@ export class GitHubRepository implements vscode.Disposable {
 
 		let after: string | null = null;
 		let hasNextPage = false;
-		const ret: ITeam[] = [];
+		const orgTeams: (ITeam & { repositoryNames: string[] })[] = [];
 
 		do {
 			try {
@@ -1053,16 +1053,15 @@ export class GitHubRepository implements vscode.Disposable {
 				});
 
 				result.data.organization.teams.nodes.forEach(node => {
-					if (node.repositories.nodes.find(repo => repo.name === remote.repositoryName)) {
-						ret.push({
-							avatarUrl: getAvatarWithEnterpriseFallback(node.avatarUrl, undefined, this.remote.authProviderId),
-							name: node.name,
-							url: node.url,
-							slug: node.slug,
-							id: node.id,
-							org: remote.owner
-						});
-					}
+					const team: ITeam = {
+						avatarUrl: getAvatarWithEnterpriseFallback(node.avatarUrl, undefined, this.remote.authProviderId),
+						name: node.name,
+						url: node.url,
+						slug: node.slug,
+						id: node.id,
+						org: remote.owner
+					};
+					orgTeams.push({ ...team, repositoryNames: node.repositories.nodes.map(repo => repo.name) });
 				});
 
 				hasNextPage = result.data.organization.teams.pageInfo.hasNextPage;
@@ -1078,12 +1077,12 @@ export class GitHubRepository implements vscode.Disposable {
 						`GitHub teams features will not work. ${e.graphQLErrors[0].message}`,
 					);
 				}
-				return ret;
+				return orgTeams;
 			}
 		} while (hasNextPage);
 
 		Logger.debug(`Fetch Teams - exit`, GitHubRepository.ID);
-		return ret;
+		return orgTeams;
 	}
 
 	async getPullRequestParticipants(pullRequestNumber: number): Promise<IAccount[]> {

src/github/pullRequestOverview.ts

@@ -365,24 +365,29 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
 		}
 
 		const allAssignableUsers = await this._folderRepositoryManager.getAssignableUsers();
-		const teamReviewers = this._item.base.isInOrganization ? await this._folderRepositoryManager.getTeamReviewers(refreshKind) : [];
-		const assignableUsers: (IAccount | ITeam)[] = teamReviewers[this._item.remote.remoteName] ?? [];
+		const allTeamReviewers = this._item.base.isInOrganization ? await this._folderRepositoryManager.getTeamReviewers(refreshKind) : [];
+		const teamReviewers: ITeam[] = allTeamReviewers[this._item.remote.remoteName] ?? [];
+		const assignableUsers: (IAccount | ITeam)[] = [...teamReviewers];
 		assignableUsers.push(...allAssignableUsers[this._item.remote.remoteName]);
-
+		let hasTeams = teamReviewers.length > 0;
 
 		// used to track logins that shouldn't be added to pick list
 		// e.g. author, existing and already added reviewers
 		const skipList: Set<string> = new Set([
 			this._item.author.login,
-			...this._existingReviewers.map(reviewer => reviewerId(reviewer.reviewer)),
+			...this._existingReviewers.map(reviewer => {
+				if (isTeam(reviewer.reviewer)) {
+					hasTeams = true;
+				}
+				return reviewerId(reviewer.reviewer);
+			}),
 		]);
 
 		const reviewers: (vscode.QuickPickItem & { reviewer?: IAccount | ITeam })[] = [];
-
 		// Start will all existing reviewers so they show at the top
 		for (const reviewer of this._existingReviewers) {
 			reviewers.push({
-				label: (reviewer.reviewer as IAccount).login ?? `${(reviewer.reviewer as ITeam).org}/${(reviewer.reviewer as ITeam).slug}`,
+				label: isTeam(reviewer.reviewer) ? `$(organization) ${reviewer.reviewer.org}/${reviewer.reviewer.slug}` : `${hasTeams ? `$(account) ` : ''}${reviewer.reviewer.login}`,
 				description: reviewer.reviewer.name,
 				reviewer: reviewer.reviewer,
 				picked: true
@@ -405,7 +410,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
 							: vscode.l10n.t('Suggested reviewer');
 
 			reviewers.push({
-				label: login,
+				label: `${hasTeams ? `$(account) ` : ''}${login}`,
 				description: name,
 				detail: suggestionReason,
 				reviewer: user,
@@ -420,7 +425,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
 			}
 
 			reviewers.push({
-				label: (user as IAccount).login ?? `${(user as ITeam).org}/${(user as ITeam).slug}`,
+				label: isTeam(user) ? `$(organization) ${user.org}/${user.slug}` : `${hasTeams ? `$(account) ` : ''}${user.login}`,
 				description: user.name,
 				reviewer: user,
 			});
@@ -528,8 +533,13 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
 			quickPick.matchOnDescription = true;
 			quickPick.show();
 			const updateItems = async (refreshKind: TeamReviewerRefreshKind) => {
+				const slowWarning = setTimeout(() => {
+					quickPick.placeholder = vscode.l10n.t('Getting team reviewers can take several minutes. Results will be cached.');
+				}, 3000);
 				quickPick.items = await this.getReviewersQuickPickItems(this._item.suggestedReviewers, refreshKind);
+				clearTimeout(slowWarning);
 				quickPick.selectedItems = quickPick.items.filter(item => item.picked);
+				quickPick.placeholder = undefined;
 			};
 
 			await updateItems((this._teamsCount !== 0 && this._teamsCount <= quickMaxTeamReviewers) ? TeamReviewerRefreshKind.Try : TeamReviewerRefreshKind.None);