diff --git a/src/common/githubRef.ts b/src/common/githubRef.ts index 89ad60b069..eb6847443b 100644 --- a/src/common/githubRef.ts +++ b/src/common/githubRef.ts @@ -8,7 +8,7 @@ import { Protocol } from './protocol'; export class GitHubRef { public repositoryCloneUrl: Protocol; constructor(public ref: string, public label: string, public sha: string, repositoryCloneUrl: string, - public readonly owner: string, public readonly name: string) { + public readonly owner: string, public readonly name: string, public readonly isInOrganization: boolean) { this.repositoryCloneUrl = new Protocol(repositoryCloneUrl); } } diff --git a/src/env/browser/ssh.ts b/src/env/browser/ssh.ts index 9754e4d0b1..511b33ac45 100644 --- a/src/env/browser/ssh.ts +++ b/src/env/browser/ssh.ts @@ -52,7 +52,7 @@ export const sshParse = (url: string): Config | undefined => { * @param {ConfigResolver?} resolveConfig ssh config resolver (default: from ~/.ssh/config) * @returns {Config} */ - export const resolve = (url: string, resolveConfig = Resolvers.current) => { +export const resolve = (url: string, resolveConfig = Resolvers.current) => { const config = sshParse(url); return config && resolveConfig(config); }; diff --git a/src/github/activityBarViewProvider.ts b/src/github/activityBarViewProvider.ts index 6e41f0d7c6..f178c4b515 100644 --- a/src/github/activityBarViewProvider.ts +++ b/src/github/activityBarViewProvider.ts @@ -11,7 +11,7 @@ import { dispose, formatError } from '../common/utils'; import { getNonce, IRequestMessage, WebviewViewBase } from '../common/webview'; import { ReviewManager } from '../view/reviewManager'; import { FolderRepositoryManager } from './folderRepositoryManager'; -import { GithubItemStateEnum, ReviewEvent, ReviewState } from './interface'; +import { GithubItemStateEnum, isTeam, reviewerId, ReviewEvent, ReviewState } from './interface'; import { PullRequestModel } from './pullRequestModel'; import { getDefaultMergeMethod } from './pullRequestOverview'; import { PullRequestView } from './pullRequestOverviewCommon'; @@ -116,8 +116,15 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W } private reRequestReview(message: IRequestMessage): void { - this._item.requestReview([message.args]).then(() => { - const reviewer = this._existingReviewers.find(reviewer => reviewer.reviewer.login === message.args); + const reviewer = this._existingReviewers.find(reviewer => reviewerId(reviewer.reviewer) === message.args); + const userReviewers: string[] = []; + const teamReviewers: string[] = []; + if (reviewer && isTeam(reviewer.reviewer)) { + teamReviewers.push(reviewer.reviewer.id); + } else if (reviewer && !isTeam(reviewer.reviewer)) { + userReviewers.push(reviewer.reviewer.login); + } + this._item.requestReview(userReviewers, teamReviewers).then(() => { if (reviewer) { reviewer.state = 'REQUESTED'; } @@ -266,7 +273,7 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W private updateReviewers(review?: CommonReviewEvent): void { if (review) { const existingReviewer = this._existingReviewers.find( - reviewer => review.user.login === reviewer.reviewer.login, + reviewer => review.user.login === reviewerId(reviewer.reviewer), ); if (existingReviewer) { existingReviewer.state = review.state; diff --git a/src/github/credentials.ts b/src/github/credentials.ts index 43b7cc933d..23ecd1d376 100644 --- a/src/github/credentials.ts +++ b/src/github/credentials.ts @@ -28,7 +28,11 @@ const PROMPT_FOR_SIGN_IN_STORAGE_KEY = 'login'; // If the scopes are changed, make sure to notify all interested parties to make sure this won't cause problems. const SCOPES_OLD = ['read:user', 'user:email', 'repo']; -export const SCOPES = ['read:user', 'user:email', 'repo', 'workflow']; +const SCOPES = ['read:user', 'user:email', 'repo', 'workflow']; +const SCOPES_WITH_ADDITIONAL = ['read:user', 'user:email', 'repo', 'workflow', 'read:org']; + +const LAST_USED_SCOPES_GITHUB_KEY = 'githubPullRequest.lastUsedScopes'; +const LAST_USED_SCOPES_ENTERPRISE_KEY = 'githubPullRequest.lastUsedScopesEnterprise'; export interface GitHub { octokit: LoggingOctokit; @@ -44,11 +48,15 @@ export class CredentialStore implements vscode.Disposable { private _disposables: vscode.Disposable[]; private _onDidInitialize: vscode.EventEmitter = new vscode.EventEmitter(); public readonly onDidInitialize: vscode.Event = this._onDidInitialize.event; + private _scopes: string[]; + private _scopesEnterprise: string[]; private _onDidGetSession: vscode.EventEmitter = new vscode.EventEmitter(); public readonly onDidGetSession = this._onDidGetSession.event; - constructor(private readonly _telemetry: ITelemetry, private readonly _context: vscode.ExtensionContext) { + constructor(private readonly _telemetry: ITelemetry, private readonly context: vscode.ExtensionContext) { + this.setScopesFromState(); + this._disposables = []; this._disposables.push( vscode.authentication.onDidChangeSessions(async () => { @@ -69,7 +77,17 @@ export class CredentialStore implements vscode.Disposable { ); } - private async initialize(authProviderId: AuthProvider, getAuthSessionOptions: vscode.AuthenticationGetSessionOptions = {}): Promise { + private setScopesFromState() { + this._scopes = this.context.globalState.get(LAST_USED_SCOPES_GITHUB_KEY, SCOPES); + this._scopesEnterprise = this.context.globalState.get(LAST_USED_SCOPES_ENTERPRISE_KEY, SCOPES); + } + + private async saveScopesInState() { + await this.context.globalState.update(LAST_USED_SCOPES_GITHUB_KEY, this._scopes); + await this.context.globalState.update(LAST_USED_SCOPES_ENTERPRISE_KEY, this._scopesEnterprise); + } + + private async initialize(authProviderId: AuthProvider, getAuthSessionOptions: vscode.AuthenticationGetSessionOptions = {}, scopes: string[] = authProviderId === AuthProvider.github ? this._scopes : this._scopesEnterprise): Promise { Logger.debug(`Initializing GitHub${getGitHubSuffix(authProviderId)} authentication provider.`, 'Authentication'); if (authProviderId === AuthProvider['github-enterprise']) { if (!hasEnterpriseUri()) { @@ -84,8 +102,16 @@ export class CredentialStore implements vscode.Disposable { let session: vscode.AuthenticationSession | undefined = undefined; let isNew: boolean = false; + let usedScopes: string[] | undefined = SCOPES; try { - const result = await this.getSession(authProviderId, getAuthSessionOptions); + // Set scopes before getting the session to prevent new session events from using the old scopes. + if (authProviderId === AuthProvider.github) { + this._scopes = scopes; + } else { + this._scopesEnterprise = scopes; + } + const result = await this.getSession(authProviderId, getAuthSessionOptions, scopes); + usedScopes = result.scopes; session = result.session; isNew = result.isNew; } catch (e) { @@ -115,9 +141,12 @@ export class CredentialStore implements vscode.Disposable { } if (authProviderId === AuthProvider.github) { this._githubAPI = github; + this._scopes = usedScopes; } else { this._githubEnterpriseAPI = github; + this._scopesEnterprise = usedScopes; } + await this.saveScopesInState(); if (!(getAuthSessionOptions.createIfNone || getAuthSessionOptions.forceNewSession) || isNew) { this._onDidInitialize.fire(); @@ -127,15 +156,15 @@ export class CredentialStore implements vscode.Disposable { } } - private async doCreate(options: vscode.AuthenticationGetSessionOptions) { - await this.initialize(AuthProvider.github, options); + private async doCreate(options: vscode.AuthenticationGetSessionOptions, additionalScopes: boolean = false) { + await this.initialize(AuthProvider.github, options, additionalScopes ? SCOPES_WITH_ADDITIONAL : undefined); if (hasEnterpriseUri()) { - await this.initialize(AuthProvider['github-enterprise'], options); + await this.initialize(AuthProvider['github-enterprise'], options, additionalScopes ? SCOPES_WITH_ADDITIONAL : undefined); } } - public async create(options: vscode.AuthenticationGetSessionOptions = {}) { - return this.doCreate(options); + public async create(options: vscode.AuthenticationGetSessionOptions = {}, additionalScopes: boolean = false) { + return this.doCreate(options, additionalScopes); } public async recreate(reason?: string) { @@ -159,6 +188,13 @@ export class CredentialStore implements vscode.Disposable { return !!this._githubEnterpriseAPI; } + public isAuthenticatedWithAdditionalScopes(authProviderId: AuthProvider): boolean { + if (authProviderId === AuthProvider.github) { + return !!this._githubAPI && this._scopes.length == SCOPES_WITH_ADDITIONAL.length; + } + return !!this._githubEnterpriseAPI && this._scopes.length == SCOPES_WITH_ADDITIONAL.length; + } + public getHub(authProviderId: AuthProvider): GitHub | undefined { if (authProviderId === AuthProvider.github) { return this._githubAPI; @@ -166,6 +202,12 @@ export class CredentialStore implements vscode.Disposable { return this._githubEnterpriseAPI; } + public async getHubEnsureAdditionalScopes(authProviderId: AuthProvider): Promise { + const hasScopesAlready = this.isAuthenticatedWithAdditionalScopes(authProviderId); + await this.initialize(authProviderId, { createIfNone: !hasScopesAlready }, SCOPES_WITH_ADDITIONAL); + return this.getHub(authProviderId); + } + public async getHubOrLogin(authProviderId: AuthProvider): Promise { if (authProviderId === AuthProvider.github) { return this._githubAPI ?? (await this.login(authProviderId)); @@ -265,39 +307,35 @@ export class CredentialStore implements vscode.Disposable { }); } - private async getSession(authProviderId: AuthProvider, getAuthSessionOptions: vscode.AuthenticationGetSessionOptions): Promise<{ session: vscode.AuthenticationSession | undefined, isNew: boolean }> { - let session: vscode.AuthenticationSession | undefined = getAuthSessionOptions.forceNewSession ? undefined : await vscode.authentication.getSession(authProviderId, SCOPES, { silent: true }); + private async getSession(authProviderId: AuthProvider, getAuthSessionOptions: vscode.AuthenticationGetSessionOptions, scopes: string[]): Promise<{ session: vscode.AuthenticationSession | undefined, isNew: boolean, scopes: string[] }> { + let session: vscode.AuthenticationSession | undefined = getAuthSessionOptions.forceNewSession ? undefined : await vscode.authentication.getSession(authProviderId, scopes, { silent: true }); if (session) { - return { session, isNew: false }; + return { session, isNew: false, scopes }; } + let usedScopes: string[]; + if (getAuthSessionOptions.createIfNone && !getAuthSessionOptions.forceNewSession) { const silent = getAuthSessionOptions.silent; getAuthSessionOptions.createIfNone = false; getAuthSessionOptions.silent = true; session = await vscode.authentication.getSession(authProviderId, SCOPES_OLD, getAuthSessionOptions); + usedScopes = SCOPES_OLD; if (!session) { getAuthSessionOptions.createIfNone = true; getAuthSessionOptions.silent = silent; - session = await vscode.authentication.getSession(authProviderId, SCOPES, getAuthSessionOptions); + session = await vscode.authentication.getSession(authProviderId, scopes, getAuthSessionOptions); + usedScopes = scopes; } } else if (getAuthSessionOptions.forceNewSession) { - session = await vscode.authentication.getSession(authProviderId, SCOPES, getAuthSessionOptions); + session = await vscode.authentication.getSession(authProviderId, scopes, getAuthSessionOptions); + usedScopes = scopes; } else { session = await vscode.authentication.getSession(authProviderId, SCOPES_OLD, getAuthSessionOptions); + usedScopes = SCOPES_OLD; } - return { session, isNew: !!session }; - } - - private async getSessionOrLogin(authProviderId: AuthProvider): Promise { - const session = (await this.getSession(authProviderId, { createIfNone: true })).session!; - if (authProviderId === AuthProvider.github) { - this._sessionId = session.id; - } else { - this._enterpriseSessionId = session.id; - } - return session.accessToken; + return { session, isNew: !!session, scopes: usedScopes }; } private async createHub(token: string, authProviderId: AuthProvider): Promise { diff --git a/src/github/folderRepositoryManager.ts b/src/github/folderRepositoryManager.ts index 521a57d1f4..a1615de398 100644 --- a/src/github/folderRepositoryManager.ts +++ b/src/github/folderRepositoryManager.ts @@ -25,9 +25,9 @@ import { git } from '../gitProviders/gitCommands'; import { UserCompletion, userMarkdown } from '../issues/util'; import { OctokitCommon } from './common'; import { CredentialStore } from './credentials'; -import { GitHubRepository, ItemsData, PullRequestData, ViewerPermission } from './githubRepository'; +import { GitHubRepository, ItemsData, PullRequestData, TeamReviewerRefreshKind, ViewerPermission } from './githubRepository'; import { PullRequestState, UserResponse } from './graphql'; -import { IAccount, ILabel, IMilestone, IPullRequestsPagingOptions, PRType, RepoAccessAndMergeMethods, User } from './interface'; +import { IAccount, ILabel, IMilestone, IPullRequestsPagingOptions, ITeam, PRType, RepoAccessAndMergeMethods, User } from './interface'; import { IssueModel } from './issueModel'; import { MilestoneModel } from './milestoneModel'; import { PullRequestGitHelper, PullRequestMetadata } from './pullRequestGitHelper'; @@ -39,6 +39,7 @@ import { getRelatedUsersFromTimelineEvents, loginComparator, parseGraphQLUser, + teamComparator, variableSubstitution, } from './utils'; @@ -125,7 +126,9 @@ export class FolderRepositoryManager implements vscode.Disposable { private _mentionableUsers?: { [key: string]: IAccount[] }; private _fetchMentionableUsersPromise?: Promise<{ [key: string]: IAccount[] }>; private _assignableUsers?: { [key: string]: IAccount[] }; + private _teamReviewers?: { [key: string]: ITeam[] }; private _fetchAssignableUsersPromise?: Promise<{ [key: string]: IAccount[] }>; + private _fetchTeamReviewersPromise?: Promise<{ [key: string]: ITeam[] }>; private _gitBlameCache: { [key: string]: string } = {}; private _githubManager: GitHubManager; private _repositoryPageInformation: Map = new Map(); @@ -734,7 +737,7 @@ export class FolderRepositoryManager implements vscode.Disposable { // file doesn't exist or json is unexpectedly invalid } if (repoSpecificCache) { - cache[repo.remote.repositoryName] = repoSpecificCache; + cache[repo.remote.remoteName] = repoSpecificCache; return true; } }))).every(value => value); @@ -747,6 +750,44 @@ export class FolderRepositoryManager implements vscode.Disposable { return undefined; } + private async getTeamReviewersFromGlobalState(): Promise<{ [key: string]: ITeam[] } | undefined> { + Logger.appendLine('Trying to use globalState for team reviewers.'); + + const teamReviewersCacheLocation = vscode.Uri.joinPath(this.context.globalStorageUri, 'teamReviewers'); + let teamReviewersCacheExists; + try { + teamReviewersCacheExists = await vscode.workspace.fs.stat(teamReviewersCacheLocation); + } catch (e) { + // file doesn't exit + } + if (!teamReviewersCacheExists) { + return undefined; + } + + const cache: { [key: string]: ITeam[] } = {}; + const hasAllRepos = (await Promise.all(this._githubRepositories.map(async (repo) => { + const key = `${repo.remote.owner}/${repo.remote.repositoryName}.json`; + const repoSpecificFile = vscode.Uri.joinPath(teamReviewersCacheLocation, key); + let repoSpecificCache; + try { + repoSpecificCache = await vscode.workspace.fs.readFile(repoSpecificFile); + } catch (e) { + // file doesn't exist + } + if (repoSpecificCache && repoSpecificCache.toString()) { + cache[repo.remote.remoteName] = JSON.parse(repoSpecificCache.toString()) ?? []; + return true; + } + }))).every(value => value); + if (hasAllRepos) { + Logger.appendLine(`Using globalState team reviewers for ${Object.keys(cache).length}.`); + return cache; + } + + Logger.appendLine(`No globalState for team reviewers.`); + return undefined; + } + private createFetchMentionableUsersPromise(): Promise<{ [key: string]: IAccount[] }> { const cache: { [key: string]: IAccount[] } = {}; return new Promise<{ [key: string]: IAccount[] }>(resolve => { @@ -821,6 +862,52 @@ export class FolderRepositoryManager implements vscode.Disposable { return this._fetchAssignableUsersPromise; } + async getTeamReviewers(refreshKind: TeamReviewerRefreshKind): Promise<{ [key: string]: ITeam[] }> { + if (refreshKind === TeamReviewerRefreshKind.Force) { + delete this._teamReviewers; + } + + if (this._teamReviewers) { + return this._teamReviewers; + } + + const globalStateTeamReviewers = (refreshKind === TeamReviewerRefreshKind.Force) ? undefined : await this.getTeamReviewersFromGlobalState(); + if (globalStateTeamReviewers) { + return globalStateTeamReviewers || {}; + } + + if (!this._fetchTeamReviewersPromise) { + const cache: { [key: string]: ITeam[] } = {}; + return (this._fetchTeamReviewersPromise = new Promise(async (resolve) => { + // 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 + } + } + + this._teamReviewers = cache; + this._fetchTeamReviewersPromise = undefined; + const teamReviewersCacheLocation = vscode.Uri.joinPath(this.context.globalStorageUri, 'teamReviewers'); + Promise.all(this._githubRepositories.map(async (repo) => { + const key = `${repo.remote.owner}/${repo.remote.repositoryName}.json`; + const repoSpecificFile = vscode.Uri.joinPath(teamReviewersCacheLocation, key); + await vscode.workspace.fs.writeFile(repoSpecificFile, new TextEncoder().encode(JSON.stringify(cache[repo.remote.remoteName]))); + })); + resolve(cache); + })); + } + + return this._fetchTeamReviewersPromise; + } + + async getOrgTeamsCount(repository: GitHubRepository): Promise { + return repository.getOrgTeamsCount(); + } + async getPullRequestParticipants(githubRepository: GitHubRepository, pullRequestNumber: number): Promise<{ participants: IAccount[], viewer: IAccount }> { return { participants: await githubRepository.getPullRequestParticipants(pullRequestNumber), diff --git a/src/github/githubRepository.ts b/src/github/githubRepository.ts index 2fdd8bb41f..57fe038525 100644 --- a/src/github/githubRepository.ts +++ b/src/github/githubRepository.ts @@ -26,6 +26,8 @@ import { MaxIssueResponse, MentionableUsersResponse, MilestoneIssuesResponse, + OrganizationTeamsCountResponse, + OrganizationTeamsResponse, PullRequestParticipantsResponse, PullRequestResponse, PullRequestsResponse, @@ -36,6 +38,7 @@ import { IAccount, IMilestone, Issue, + ITeam, PullRequest, PullRequestChecks, PullRequestReviewRequirement, @@ -88,6 +91,12 @@ export enum ViewerPermission { Write = 'WRITE', } +export enum TeamReviewerRefreshKind { + None, + Try, + Force +} + export interface ForkDetails { isFork: boolean; parent: { @@ -289,6 +298,23 @@ export class GitHubRepository implements vscode.Disposable { return this; } + async ensureAdditionalScopes(): Promise { + this._initialized = true; + + if (!this._credentialStore.isAuthenticated(this.remote.authProviderId)) { + // We need auth now. (ex., a PR is already checked out) + // We can no longer wait until later for login to be done + await this._credentialStore.create(undefined, true); + if (!this._credentialStore.isAuthenticated(this.remote.authProviderId)) { + this._hub = await this._credentialStore.showSignInNotification(this.remote.authProviderId); + } + } else { + this._hub = await this._credentialStore.getHubEnsureAdditionalScopes(this.remote.authProviderId); + } + + return this; + } + async getDefaultBranch(): Promise { const overrideSetting = getOverrideBranch(); if (overrideSetting) { @@ -760,7 +786,7 @@ export class GitHubRepository implements vscode.Disposable { if (model) { model.update(pullRequest); } else { - model = new PullRequestModel(this._telemetry, this, this.remote, pullRequest); + model = new PullRequestModel(this._credentialStore, this._telemetry, this, this.remote, pullRequest); model.onDidInvalidate(() => this.getPullRequest(pullRequest.number)); this._pullRequestModels.set(pullRequest.number, model); this._onDidAddPullRequest.fire(model); @@ -994,6 +1020,95 @@ export class GitHubRepository implements vscode.Disposable { return ret; } + async getOrgTeamsCount(): Promise { + Logger.debug(`Fetch Teams Count - enter`, GitHubRepository.ID); + if (!this._credentialStore.isAuthenticatedWithAdditionalScopes(this.remote.authProviderId)) { + return 0; + } + + const { query, remote, schema } = await this.ensureAdditionalScopes(); + + try { + const result: { data: OrganizationTeamsCountResponse } = await query({ + query: schema.GetOrganizationTeamsCount, + variables: { + login: remote.owner + }, + }); + return result.data.organization.teams.totalCount; + } catch (e) { + Logger.debug(`Unable to fetch teams Count: ${e}`, GitHubRepository.ID); + if ( + e.graphQLErrors && + e.graphQLErrors.length > 0 && + e.graphQLErrors[0].type === 'INSUFFICIENT_SCOPES' + ) { + vscode.window.showWarningMessage( + `GitHub teams features will not work. ${e.graphQLErrors[0].message}`, + ); + } + return 0; + } + } + + async getTeams(refreshKind: TeamReviewerRefreshKind): Promise { + 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); + return []; + } + + const { query, remote, schema } = await this.ensureAdditionalScopes(); + + let after: string | null = null; + let hasNextPage = false; + const ret: ITeam[] = []; + + do { + try { + const result: { data: OrganizationTeamsResponse } = await query({ + query: schema.GetOrganizationTeams, + variables: { + login: remote.owner, + after: after, + repoName: remote.repositoryName, + }, + }); + + 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 + }); + } + }); + + hasNextPage = result.data.organization.teams.pageInfo.hasNextPage; + after = result.data.organization.teams.pageInfo.endCursor; + } catch (e) { + Logger.debug(`Unable to fetch teams: ${e}`, GitHubRepository.ID); + if ( + e.graphQLErrors && + e.graphQLErrors.length > 0 && + e.graphQLErrors[0].type === 'INSUFFICIENT_SCOPES' + ) { + vscode.window.showWarningMessage( + `GitHub teams features will not work. ${e.graphQLErrors[0].message}`, + ); + } + return ret; + } + } while (hasNextPage); + + Logger.debug(`Fetch Teams - exit`, GitHubRepository.ID); + return ret; + } + async getPullRequestParticipants(pullRequestNumber: number): Promise { Logger.debug(`Fetch participants from a Pull Request`, GitHubRepository.ID); const { query, remote, schema } = await this.ensure(); diff --git a/src/github/graphql.ts b/src/github/graphql.ts index 21a6b09a12..47b8e0e33d 100644 --- a/src/github/graphql.ts +++ b/src/github/graphql.ts @@ -77,6 +77,19 @@ export interface Account { email: string; } +interface Team { + avatarUrl: string; + name: string; + url: string; + repositories: { + nodes: { + name: string + }[]; + }; + slug: string; + id: string; +} + export interface ReviewComment { __typename: string; id: string; @@ -226,6 +239,29 @@ export interface PendingReviewIdResponse { rateLimit: RateLimit; } +export interface GetReviewRequestsResponse { + repository: { + 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; + }[]; + }; + }; + }; +}; + export interface PullRequestState { repository: { pullRequest: { @@ -272,6 +308,28 @@ export interface AssignableUsersResponse { rateLimit: RateLimit; } +export interface OrganizationTeamsCountResponse { + organization: { + teams: { + totalCount: number; + }; + }; +} + +export interface OrganizationTeamsResponse { + organization: { + teams: { + nodes: Team[]; + totalCount: number; + pageInfo: { + hasNextPage: boolean; + endCursor: string; + }; + }; + }; + rateLimit: RateLimit; +} + export interface PullRequestParticipantsResponse { repository: { pullRequest: { @@ -399,6 +457,7 @@ export interface ListBranchesResponse { } export interface RefRepository { + isInOrganization: boolean; owner: { login: string; }; diff --git a/src/github/interface.ts b/src/github/interface.ts index 5080bba59a..90633d9fb8 100644 --- a/src/github/interface.ts +++ b/src/github/interface.ts @@ -30,7 +30,7 @@ export enum PullRequestMergeability { } export interface ReviewState { - reviewer: IAccount; + reviewer: IAccount | ITeam; state: string; } @@ -42,6 +42,27 @@ export interface IAccount { email?: string; } +export interface ITeam { + name: string; + avatarUrl?: string; + url: string; + slug: string; + org: string; + id: string; +} + +export function reviewerId(reviewer: ITeam | IAccount): string { + return isTeam(reviewer) ? reviewer.id : reviewer.login; +} + +export function reviewerLabel(reviewer: ITeam | IAccount): string { + return isTeam(reviewer) ? reviewer.name : reviewer.login; +} + +export function isTeam(reviewer: ITeam | IAccount): reviewer is ITeam { + return 'id' in reviewer; +} + export interface ISuggestedReviewer extends IAccount { isAuthor: boolean; isCommenter: boolean; @@ -63,6 +84,7 @@ export interface MergePullRequest { export interface IRepository { cloneUrl: string; + isInOrganization: boolean; owner: string; name: string; } diff --git a/src/github/pullRequestModel.ts b/src/github/pullRequestModel.ts index 1ca7b47674..173bf72709 100644 --- a/src/github/pullRequestModel.ts +++ b/src/github/pullRequestModel.ts @@ -19,6 +19,7 @@ import { ReviewEvent as CommonReviewEvent, EventType, TimelineEvent } from '../c import { resolvePath, toPRUri, toReviewUri } from '../common/uri'; import { formatError } from '../common/utils'; import { OctokitCommon } from './common'; +import { CredentialStore } from './credentials'; import { FolderRepositoryManager } from './folderRepositoryManager'; import { GitHubRepository } from './githubRepository'; import { @@ -28,6 +29,7 @@ import { DeleteReactionResponse, DeleteReviewResponse, EditCommentResponse, + GetReviewRequestsResponse, LatestReviewCommitResponse, MarkPullRequestReadyForReviewResponse, PendingReviewIdResponse, @@ -47,6 +49,7 @@ import { IAccount, IRawFileChange, ISuggestedReviewer, + ITeam, MergeMethod, PullRequest, PullRequestChecks, @@ -58,7 +61,6 @@ import { IssueModel } from './issueModel'; import { convertRESTPullRequestToRawPullRequest, convertRESTReviewEvent, - convertRESTUserToAccount, getReactionGroup, insertNewCommitsSinceReview, parseGraphQLComment, @@ -136,6 +138,7 @@ export class PullRequestModel extends IssueModel implements IPullRe _telemetry: ITelemetry; constructor( + private readonly credentialStore: CredentialStore, telemetry: ITelemetry, githubRepository: GitHubRepository, remote: Remote, @@ -230,14 +233,14 @@ export class PullRequestModel extends IssueModel implements IPullRe this.isRemoteHeadDeleted = item.isRemoteHeadDeleted; } if (item.head) { - this.head = new GitHubRef(item.head.ref, item.head.label, item.head.sha, item.head.repo.cloneUrl, item.head.repo.owner, item.head.repo.name); + this.head = new GitHubRef(item.head.ref, item.head.label, item.head.sha, item.head.repo.cloneUrl, item.head.repo.owner, item.head.repo.name, item.head.repo.isInOrganization); } if (item.isRemoteBaseDeleted != null) { this.isRemoteBaseDeleted = item.isRemoteBaseDeleted; } if (item.base) { - this.base = new GitHubRef(item.base.ref, item.base!.label, item.base!.sha, item.base!.repo.cloneUrl, item.base.repo.owner, item.base.repo.name); + this.base = new GitHubRef(item.base.ref, item.base!.label, item.base!.sha, item.base!.repo.cloneUrl, item.base.repo.owner, item.base.repo.name, item.base.repo.isInOrganization); } } @@ -761,29 +764,60 @@ export class PullRequestModel extends IssueModel implements IPullRe /** * Get existing requests to review. */ - async getReviewRequests(): Promise { + async getReviewRequests(): Promise<(IAccount | ITeam)[]> { const githubRepository = this.githubRepository; - const { remote, octokit } = await githubRepository.ensure(); - const result = await octokit.call(octokit.api.pulls.listRequestedReviewers, { - owner: remote.owner, - repo: remote.repositoryName, - pull_number: this.number, + const { remote, query, schema } = await githubRepository.ensure(); + + const { data } = await query({ + query: this.credentialStore.isAuthenticatedWithAdditionalScopes(githubRepository.remote.authProviderId) ? schema.GetReviewRequestsAdditionalScopes : schema.GetReviewRequests, + variables: { + number: this.number, + owner: remote.owner, + name: remote.repositoryName + }, }); - return result.data.users.map((user: any) => convertRESTUserToAccount(user, githubRepository)); + const reviewers: (IAccount | ITeam)[] = []; + for (const reviewer of data.repository.pullRequest.reviewRequests.nodes) { + if (reviewer.requestedReviewer?.login) { + const account: IAccount = { + login: reviewer.requestedReviewer.login, + url: reviewer.requestedReviewer.url, + avatarUrl: reviewer.requestedReviewer.avatarUrl, + email: reviewer.requestedReviewer.email, + name: reviewer.requestedReviewer.name + }; + reviewers.push(account); + } else if (reviewer.requestedReviewer) { + const team: ITeam = { + name: reviewer.requestedReviewer.name, + url: reviewer.requestedReviewer.url, + avatarUrl: reviewer.requestedReviewer.avatarUrl, + id: reviewer.requestedReviewer.id!, + org: remote.owner, + slug: reviewer.requestedReviewer.slug! + }; + reviewers.push(team); + } + } + return reviewers; } /** * Add reviewers to a pull request * @param reviewers A list of GitHub logins */ - async requestReview(reviewers: string[]): Promise { - const { octokit, remote } = await this.githubRepository.ensure(); - await octokit.call(octokit.api.pulls.requestReviewers, { - owner: remote.owner, - repo: remote.repositoryName, - pull_number: this.number, - reviewers, + async requestReview(reviewers: string[], teamReviewers: string[]): Promise { + const { mutate, schema } = await this.githubRepository.ensure(); + await mutate({ + mutation: schema.AddReviewers, + variables: { + input: { + pullRequestId: this.graphNodeId, + teamIds: teamReviewers, + userIds: reviewers + }, + }, }); } @@ -791,13 +825,14 @@ export class PullRequestModel extends IssueModel implements IPullRe * Remove a review request that has not yet been completed * @param reviewer A GitHub Login */ - async deleteReviewRequest(reviewers: string[]): Promise { + async deleteReviewRequest(reviewers: string[], teamReviewers: string[]): Promise { const { octokit, remote } = await this.githubRepository.ensure(); await octokit.call(octokit.api.pulls.removeRequestedReviewers, { owner: remote.owner, repo: remote.repositoryName, pull_number: this.number, reviewers, + team_reviewers: teamReviewers }); } diff --git a/src/github/pullRequestOverview.ts b/src/github/pullRequestOverview.ts index 046e82f511..4054c7daad 100644 --- a/src/github/pullRequestOverview.ts +++ b/src/github/pullRequestOverview.ts @@ -12,13 +12,17 @@ import { ReviewEvent as CommonReviewEvent } from '../common/timelineEvent'; import { asPromise, dispose, formatError } from '../common/utils'; import { IRequestMessage, PULL_REQUEST_OVERVIEW_VIEW_TYPE } from '../common/webview'; import { FolderRepositoryManager } from './folderRepositoryManager'; +import { TeamReviewerRefreshKind } from './githubRepository'; import { GithubItemStateEnum, IAccount, IMilestone, + isTeam, ISuggestedReviewer, + ITeam, MergeMethod, MergeMethodsAvailability, + reviewerId, ReviewEvent, ReviewState, } from './interface'; @@ -42,6 +46,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel r.reviewer.login === currentUser.login); + const review = reviewers.find(r => reviewerId(r.reviewer) === currentUser.login); // There will always be a review. If not then the PR shouldn't have been or fetched/shown for the current user return review?.state; } @@ -172,7 +177,8 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel { const [ @@ -184,7 +190,8 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel { + suggestedReviewers: ISuggestedReviewer[] | undefined, refreshKind: TeamReviewerRefreshKind, + ): Promise<(vscode.QuickPickItem & { reviewer?: IAccount | ITeam })[]> { if (!suggestedReviewers) { return []; } const allAssignableUsers = await this._folderRepositoryManager.getAssignableUsers(); - const assignableUsers = allAssignableUsers[this._item.remote.remoteName] ?? []; + const teamReviewers = this._item.base.isInOrganization ? await this._folderRepositoryManager.getTeamReviewers(refreshKind) : []; + const assignableUsers: (IAccount | ITeam)[] = teamReviewers[this._item.remote.remoteName] ?? []; + assignableUsers.push(...allAssignableUsers[this._item.remote.remoteName]); + // used to track logins that shouldn't be added to pick list // e.g. author, existing and already added reviewers const skipList: Set = new Set([ this._item.author.login, - ...this._existingReviewers.map(reviewer => reviewer.reviewer.login), + ...this._existingReviewers.map(reviewer => reviewerId(reviewer.reviewer)), ]); - const reviewers: (vscode.QuickPickItem & { reviewer?: IAccount })[] = []; + 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.login, + label: (reviewer.reviewer as IAccount).login ?? `${(reviewer.reviewer as ITeam).org}/${(reviewer.reviewer as ITeam).slug}`, description: reviewer.reviewer.name, reviewer: reviewer.reviewer, picked: true @@ -404,12 +415,12 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel): Promise { - const quickPick = vscode.window.createQuickPick(); + const quickPick = vscode.window.createQuickPick(); + // The quick-max is used to show the "update reviewers" button. If the number of teams is less than the quick-max, then they'll be automatically updated when the quick pick is opened. + const quickMaxTeamReviewers = 100; try { quickPick.busy = true; quickPick.canSelectMany = true; quickPick.matchOnDescription = true; quickPick.show(); - quickPick.items = await this.getReviewersQuickPickItems(this._item.suggestedReviewers); - quickPick.selectedItems = quickPick.items.filter(item => item.picked); + const updateItems = async (refreshKind: TeamReviewerRefreshKind) => { + quickPick.items = await this.getReviewersQuickPickItems(this._item.suggestedReviewers, refreshKind); + quickPick.selectedItems = quickPick.items.filter(item => item.picked); + }; + + await updateItems((this._teamsCount !== 0 && this._teamsCount <= quickMaxTeamReviewers) ? TeamReviewerRefreshKind.Try : TeamReviewerRefreshKind.None); + if (this._item.base.isInOrganization) { + quickPick.buttons = [{ iconPath: new vscode.ThemeIcon('organization'), tooltip: vscode.l10n.t('Show or refresh team reviewers') }]; + } + quickPick.onDidTriggerButton(() => { + quickPick.busy = true; + quickPick.ignoreFocusOut = true; + updateItems(TeamReviewerRefreshKind.Force).then(() => { + quickPick.ignoreFocusOut = false; + quickPick.busy = false; + }); + }); quickPick.busy = false; const acceptPromise = asPromise(quickPick.onDidAccept).then(() => { - return quickPick.selectedItems.filter(item => item.reviewer) as (vscode.QuickPickItem & { reviewer: IAccount })[] | undefined; + return quickPick.selectedItems.filter(item => item.reviewer) as (vscode.QuickPickItem & { reviewer: IAccount | ITeam })[] | undefined; }); const hidePromise = asPromise(quickPick.onDidHide); - const allReviewers = await Promise.race<(vscode.QuickPickItem & { reviewer: IAccount })[] | void>([acceptPromise, hidePromise]); + const allReviewers = await Promise.race<(vscode.QuickPickItem & { reviewer: IAccount | ITeam })[] | void>([acceptPromise, hidePromise]); quickPick.busy = true; + if (allReviewers) { - const newReviewers = allReviewers.map(r => r.label); - const removedReviewers = this._existingReviewers.filter(existing => !newReviewers.find(newReviewer => newReviewer === existing.reviewer.login)); - await this._item.requestReview(newReviewers); - await this._item.deleteReviewRequest(removedReviewers.map(reviewer => reviewer.reviewer.login)); + const newUserReviewers: string[] = []; + const newTeamReviewers: string[] = []; + allReviewers.forEach(reviewer => { + const newReviewers = isTeam(reviewer.reviewer) ? newTeamReviewers : newUserReviewers; + newReviewers.push(reviewerId(reviewer.reviewer)); + }); + + const removedUserReviewers: string[] = []; + const removedTeamReviewers: string[] = []; + this._existingReviewers.forEach(existing => { + let newReviewers: string[] = isTeam(existing.reviewer) ? newTeamReviewers : newUserReviewers; + let removedReviewers: string[] = isTeam(existing.reviewer) ? removedTeamReviewers : removedUserReviewers; + if (!newReviewers.find(newTeamReviewer => newTeamReviewer === reviewerId(existing.reviewer))) { + removedReviewers.push(reviewerId(existing.reviewer)); + } + }); + + await this._item.requestReview(newUserReviewers, newTeamReviewers); + await this._item.deleteReviewRequest(removedUserReviewers, removedTeamReviewers); const addedReviewers: ReviewState[] = allReviewers.map(selected => { return { reviewer: selected.reviewer, @@ -828,7 +872,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel review.user.login === reviewer.reviewer.login, + reviewer => review.user.login === (reviewer.reviewer as IAccount).login, ); if (existingReviewer) { existingReviewer.state = review.state; @@ -900,8 +944,15 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel): void { - this._item.requestReview([message.args]).then(() => { - const reviewer = this._existingReviewers.find(reviewer => reviewer.reviewer.login === message.args); + const reviewer = this._existingReviewers.find(reviewer => reviewerId(reviewer.reviewer) === message.args); + const userReviewers: string[] = []; + const teamReviewers: string[] = []; + if (reviewer && isTeam(reviewer.reviewer)) { + teamReviewers.push(reviewer.reviewer.id); + } else if (reviewer && !isTeam(reviewer.reviewer)) { + userReviewers.push(reviewer.reviewer.login); + } + this._item.requestReview(userReviewers, teamReviewers).then(() => { if (reviewer) { reviewer.state = 'REQUESTED'; } diff --git a/src/github/queries.gql b/src/github/queries.gql index 862b302aa2..49ff7a7109 100644 --- a/src/github/queries.gql +++ b/src/github/queries.gql @@ -187,6 +187,7 @@ fragment PullRequestFragment on PullRequest { headRefName headRefOid headRepository { + isInOrganization owner { login } @@ -198,6 +199,7 @@ fragment PullRequestFragment on PullRequest { baseRefName baseRefOid baseRepository { + isInOrganization owner { login } @@ -309,6 +311,109 @@ query LatestReviewCommit($owner: String!, $name: String!, $number: Int!) { } } +query LatestReviews($owner: String!, $name: String!, $number: Int!) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + latestReviews (first: 10) { + nodes { + state + } + } + } + } + rateLimit { + limit + cost + remaining + resetAt + } +} + +query GetOrganizationTeamsCount($login: String!) { + organization(login: $login) { + teams(first: 0, privacy: VISIBLE) { + totalCount + } + } +} + +query GetOrganizationTeams($login: String!, $after: String, $repoName: String!) { + organization(login: $login) { + teams(first: 100, after: $after, privacy: VISIBLE) { + nodes { + name + avatarUrl + url + repositories(first: 5, query: $repoName) { + nodes { + name + } + } + slug + id + } + totalCount + pageInfo { + hasNextPage + endCursor + } + } + } + rateLimit { + limit + cost + remaining + resetAt + } +} + +query GetReviewRequestsAdditionalScopes($owner: String!, $name: String!, $number: Int!) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + reviewRequests(first: 100) { + nodes { + requestedReviewer { + ... on User { + login + avatarUrl + url + email + name + } + ... on Team { + name + avatarUrl + url + slug + id + } + } + } + } + } + } +} + +query GetReviewRequests($owner: String!, $name: String!, $number: Int!) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + reviewRequests(first: 100) { + nodes { + requestedReviewer { + ... on User { + login + avatarUrl + url + email + name + } + } + } + } + } + } +} + fragment ReviewComment on PullRequestReviewComment { id databaseId @@ -687,6 +792,14 @@ mutation AddReviewThread($input: AddPullRequestReviewThreadInput!) { } } +mutation AddReviewers($input: RequestReviewsInput!) { + requestReviews(input: $input) { + pullRequest { + id + } + } +} + mutation EditComment($input: UpdatePullRequestReviewCommentInput!) { updatePullRequestReviewComment(input: $input) { pullRequestReviewComment { diff --git a/src/github/utils.ts b/src/github/utils.ts index b593ca95e1..dc455ede30 100644 --- a/src/github/utils.ts +++ b/src/github/utils.ts @@ -30,9 +30,12 @@ import { IMilestone, Issue, ISuggestedReviewer, + ITeam, MergeMethod, PullRequest, PullRequestMergeability, + reviewerId, + reviewerLabel, ReviewState, User, } from './interface'; @@ -244,13 +247,14 @@ export function convertRESTUserToAccount( }; } -export function convertRESTHeadToIGitHubRef(head: OctokitCommon.PullsListResponseItemHead) { +export function convertRESTHeadToIGitHubRef(head: OctokitCommon.PullsListResponseItemHead): IGitHubRef { return { label: head.label, ref: head.ref, sha: head.sha, repo: { cloneUrl: head.repo.clone_url, + isInOrganization: !!head.repo.organization, owner: head.repo.owner!.login, name: head.repo.name }, @@ -541,6 +545,7 @@ function parseRef(refName: string, oid: string, repository?: GraphQL.RefReposito sha: oid, repo: { cloneUrl: repository.url, + isInOrganization: repository.isInOrganization, owner: repository.owner.login, name: refName }, @@ -716,6 +721,13 @@ export function loginComparator(a: IAccount, b: IAccount) { // sensitivity: 'accent' allows case insensitive comparison return a.login.localeCompare(b.login, 'en', { sensitivity: 'accent' }); } +/** + * Used for case insensitive sort by team name + */ +export function teamComparator(a: ITeam, b: ITeam) { + // sensitivity: 'accent' allows case insensitive comparison + return a.name.localeCompare(b.name, 'en', { sensitivity: 'accent' }); +} export function parseGraphQLReviewEvent( review: GraphQL.SubmittedReview, @@ -1016,7 +1028,7 @@ export function getRepositoryForFile(gitAPI: GitApiImpl, file: vscode.Uri): Repo * @param author The author of the pull request */ export function parseReviewers( - requestedReviewers: IAccount[], + requestedReviewers: (IAccount | ITeam)[], timelineEvents: Common.TimelineEvent[], author: IAccount, ): ReviewState[] { @@ -1039,13 +1051,13 @@ export function parseReviewers( } requestedReviewers.forEach(request => { - if (!seen.get(request.login)) { + if (!seen.get(reviewerId(request))) { reviewers.push({ reviewer: request, state: 'REQUESTED', }); } else { - const reviewer = reviewers.find(r => r.reviewer.login === request.login); + const reviewer = reviewers.find(r => reviewerId(r.reviewer) === reviewerId(request)); reviewer!.state = 'REQUESTED'; } }); @@ -1060,7 +1072,7 @@ export function parseReviewers( return -1; } - return a.reviewer.login.toLowerCase() < b.reviewer.login.toLowerCase() ? -1 : 1; + return reviewerLabel(a.reviewer).toLowerCase() < reviewerLabel(b.reviewer).toLowerCase() ? -1 : 1; }); return reviewers; diff --git a/src/test/builders/graphql/pullRequestBuilder.ts b/src/test/builders/graphql/pullRequestBuilder.ts index 46830523dc..49522f9d63 100644 --- a/src/test/builders/graphql/pullRequestBuilder.ts +++ b/src/test/builders/graphql/pullRequestBuilder.ts @@ -9,6 +9,7 @@ import { PullRequestResponse, Ref, RefRepository } from '../../../github/graphql import { RateLimitBuilder } from './rateLimitBuilder'; const RefRepositoryBuilder = createBuilderClass()({ + isInOrganization: { default: false }, owner: createLink()({ login: { default: 'me' }, }), diff --git a/src/test/github/folderRepositoryManager.test.ts b/src/test/github/folderRepositoryManager.test.ts index 5a4e4e007d..244489e72e 100644 --- a/src/test/github/folderRepositoryManager.test.ts +++ b/src/test/github/folderRepositoryManager.test.ts @@ -55,7 +55,7 @@ describe('PullRequestManager', function () { const rootUri = Uri.file('C:\\users\\test\\repo'); const repository = new GitHubRepository(remote, rootUri, manager.credentialStore, telemetry); const prItem = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().build(), repository); - const pr = new PullRequestModel(telemetry, repository, remote, prItem); + const pr = new PullRequestModel(manager.credentialStore, telemetry, repository, remote, prItem); manager.activePullRequest = pr; assert(changeFired.called); diff --git a/src/test/github/pullRequestGitHelper.test.ts b/src/test/github/pullRequestGitHelper.test.ts index 193da32737..8779e56c9e 100644 --- a/src/test/github/pullRequestGitHelper.test.ts +++ b/src/test/github/pullRequestGitHelper.test.ts @@ -68,7 +68,7 @@ describe('PullRequestGitHelper', function () { repository.expectFetch('you', 'my-branch:pr/me/100', 1); repository.expectPull(true); - const pullRequest = new PullRequestModel(telemetry, gitHubRepository, remote, prItem); + const pullRequest = new PullRequestModel(credentialStore, telemetry, gitHubRepository, remote, prItem); if (!pullRequest.isResolved()) { assert(false, 'pull request head not resolved successfully'); diff --git a/src/test/github/pullRequestModel.test.ts b/src/test/github/pullRequestModel.test.ts index b5cea79c9d..5b93ac74a4 100644 --- a/src/test/github/pullRequestModel.test.ts +++ b/src/test/github/pullRequestModel.test.ts @@ -77,21 +77,21 @@ describe('PullRequestModel', function () { it('should return `state` properly as `open`', function () { const pr = new PullRequestBuilder().state('open').build(); - const open = new PullRequestModel(telemetry, repo, remote, convertRESTPullRequestToRawPullRequest(pr, repo)); + const open = new PullRequestModel(credentials, telemetry, repo, remote, convertRESTPullRequestToRawPullRequest(pr, repo)); assert.strictEqual(open.state, GithubItemStateEnum.Open); }); it('should return `state` properly as `closed`', function () { const pr = new PullRequestBuilder().state('closed').build(); - const open = new PullRequestModel(telemetry, repo, remote, convertRESTPullRequestToRawPullRequest(pr, repo)); + const open = new PullRequestModel(credentials, telemetry, repo, remote, convertRESTPullRequestToRawPullRequest(pr, repo)); assert.strictEqual(open.state, GithubItemStateEnum.Closed); }); it('should return `state` properly as `merged`', function () { const pr = new PullRequestBuilder().merged(true).state('closed').build(); - const open = new PullRequestModel(telemetry, repo, remote, convertRESTPullRequestToRawPullRequest(pr, repo)); + const open = new PullRequestModel(credentials, telemetry, repo, remote, convertRESTPullRequestToRawPullRequest(pr, repo)); assert.strictEqual(open.state, GithubItemStateEnum.Merged); }); @@ -100,6 +100,7 @@ describe('PullRequestModel', function () { it('should update the cache when then cache is initialized', async function () { const pr = new PullRequestBuilder().build(); const model = new PullRequestModel( + credentials, telemetry, repo, remote, diff --git a/src/test/github/pullRequestOverview.test.ts b/src/test/github/pullRequestOverview.test.ts index b4dbb6f487..bfc401eb8c 100644 --- a/src/test/github/pullRequestOverview.test.ts +++ b/src/test/github/pullRequestOverview.test.ts @@ -33,6 +33,7 @@ describe('PullRequestOverview', function () { let remote: GitHubRemote; let repo: MockGitHubRepository; let telemetry: MockTelemetry; + let credentialStore: CredentialStore; beforeEach(async function () { sinon = createSandbox(); @@ -41,7 +42,7 @@ describe('PullRequestOverview', function () { const repository = new MockRepository(); telemetry = new MockTelemetry(); - const credentialStore = new CredentialStore(telemetry, context); + credentialStore = new CredentialStore(telemetry, context); pullRequestManager = new FolderRepositoryManager(context, repository, telemetry, new GitApiImpl(), credentialStore); const url = 'https://github.com/aaa/bbb'; @@ -73,7 +74,7 @@ describe('PullRequestOverview', function () { }); const prItem = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).build(), repo); - const prModel = new PullRequestModel(telemetry, repo, remote, prItem); + const prModel = new PullRequestModel(credentialStore, telemetry, repo, remote, prItem); await PullRequestOverviewPanel.createOrShow(EXTENSION_URI, pullRequestManager, prModel); @@ -106,7 +107,7 @@ describe('PullRequestOverview', function () { }); const prItem0 = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).build(), repo); - const prModel0 = new PullRequestModel(telemetry, repo, remote, prItem0); + const prModel0 = new PullRequestModel(credentialStore, telemetry, repo, remote, prItem0); const resolveStub = sinon.stub(pullRequestManager, 'resolvePullRequest').resolves(prModel0); sinon.stub(prModel0, 'getReviewRequests').resolves([]); sinon.stub(prModel0, 'getTimelineEvents').resolves([]); @@ -119,7 +120,7 @@ describe('PullRequestOverview', function () { assert.strictEqual(panel0!.getCurrentTitle(), 'Pull Request #1000'); const prItem1 = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(2000).build(), repo); - const prModel1 = new PullRequestModel(telemetry, repo, remote, prItem1); + const prModel1 = new PullRequestModel(credentialStore, telemetry, repo, remote, prItem1); resolveStub.resolves(prModel1); sinon.stub(prModel1, 'getReviewRequests').resolves([]); sinon.stub(prModel1, 'getTimelineEvents').resolves([]); diff --git a/src/test/view/prsTree.test.ts b/src/test/view/prsTree.test.ts index 8f1f124db3..898fe1611a 100644 --- a/src/test/view/prsTree.test.ts +++ b/src/test/view/prsTree.test.ts @@ -148,7 +148,7 @@ describe('GitHub Pull Requests view', function () { }); }).pullRequest; const prItem0 = parseGraphQLPullRequest(pr0.repository.pullRequest, gitHubRepository); - const pullRequest0 = new PullRequestModel(telemetry, gitHubRepository, remote, prItem0); + const pullRequest0 = new PullRequestModel(credentialStore, telemetry, gitHubRepository, remote, prItem0); const pr1 = gitHubRepository.addGraphQLPullRequest(builder => { builder.pullRequest(pr => { @@ -164,7 +164,7 @@ describe('GitHub Pull Requests view', function () { }); }).pullRequest; const prItem1 = parseGraphQLPullRequest(pr1.repository.pullRequest, gitHubRepository); - const pullRequest1 = new PullRequestModel(telemetry, gitHubRepository, remote, prItem1); + const pullRequest1 = new PullRequestModel(credentialStore, telemetry, gitHubRepository, remote, prItem1); const repository = new MockRepository(); await repository.addRemote(remote.remoteName, remote.url); diff --git a/src/test/view/reviewCommentController.test.ts b/src/test/view/reviewCommentController.test.ts index f30896d02c..9d6f6297d3 100644 --- a/src/test/view/reviewCommentController.test.ts +++ b/src/test/view/reviewCommentController.test.ts @@ -84,6 +84,7 @@ describe('ReviewCommentController', function () { const pr = new PullRequestBuilder().build(); githubRepo = new MockGitHubRepository(remote, credentialStore, telemetry, sinon); activePullRequest = new PullRequestModel( + credentialStore, telemetry, githubRepo, remote, diff --git a/webviews/components/merge.tsx b/webviews/components/merge.tsx index 60c169d228..27cbfa4fc8 100644 --- a/webviews/components/merge.tsx +++ b/webviews/components/merge.tsx @@ -20,6 +20,7 @@ import { PullRequestCheckStatus, PullRequestMergeability, PullRequestReviewRequirement, + reviewerId, } from '../../src/github/interface'; import { PullRequest } from '../common/cache'; import PullRequestContext from '../common/context'; @@ -109,7 +110,7 @@ const InlineReviewers = ({ pr, isSimple }: { pr: PullRequest; isSimple: boolean
{' '} {pr.reviewers.map(state => ( - + ))}
) : null diff --git a/webviews/components/reviewer.tsx b/webviews/components/reviewer.tsx index ce8d1df75f..6f5b460425 100644 --- a/webviews/components/reviewer.tsx +++ b/webviews/components/reviewer.tsx @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ import React, { cloneElement, useContext } from 'react'; -import { ReviewState } from '../../src/github/interface'; +import { reviewerId, ReviewState } from '../../src/github/interface'; import PullRequestContext from '../common/context'; import { checkIcon, commentIcon, pendingIcon, requestChanges, syncIcon } from './icon'; import { AuthorLink, Avatar } from './user'; @@ -21,7 +21,7 @@ export function Reviewer(reviewState: ReviewState) {
{ state !== 'REQUESTED' ? - () : null } diff --git a/webviews/components/sidebar.tsx b/webviews/components/sidebar.tsx index 7ce2bcc5ae..9f05c0c5c8 100644 --- a/webviews/components/sidebar.tsx +++ b/webviews/components/sidebar.tsx @@ -5,7 +5,7 @@ import React, { useContext } from 'react'; import { gitHubLabelColor } from '../../src/common/utils'; -import { IMilestone } from '../../src/github/interface'; +import { IMilestone, reviewerId } from '../../src/github/interface'; import { PullRequest } from '../common/cache'; import PullRequestContext from '../common/context'; import { Label } from '../common/label'; @@ -43,7 +43,7 @@ export default function Sidebar({ reviewers, labels, hasWritePermission, isIssue
{reviewers && reviewers.length ? ( reviewers.map(state => ( - + )) ) : (
None yet
diff --git a/webviews/components/user.tsx b/webviews/components/user.tsx index 1ac5a516f5..98b6ab7156 100644 --- a/webviews/components/user.tsx +++ b/webviews/components/user.tsx @@ -4,10 +4,10 @@ *--------------------------------------------------------------------------------------------*/ import * as React from 'react'; -import { PullRequest } from '../common/cache'; +import { IAccount, ITeam, reviewerLabel } from '../../src/github/interface'; import { Icon } from './icon'; -export const Avatar = ({ for: author }: { for: Partial }) => ( +export const Avatar = ({ for: author }: { for: Partial }) => ( {author.avatarUrl ? ( @@ -17,7 +17,7 @@ export const Avatar = ({ for: author }: { for: Partial }) ); -export const AuthorLink = ({ for: author, text = author.login }: { for: PullRequest['author']; text?: string }) => ( +export const AuthorLink = ({ for: author, text = reviewerLabel(author) }: { for: IAccount | ITeam; text?: string }) => ( {text} diff --git a/webviews/editorWebview/test/builder/pullRequest.ts b/webviews/editorWebview/test/builder/pullRequest.ts index fbe2f4a42c..620c68727a 100644 --- a/webviews/editorWebview/test/builder/pullRequest.ts +++ b/webviews/editorWebview/test/builder/pullRequest.ts @@ -35,7 +35,7 @@ export const PullRequestBuilder = createBuilderClass()({ pendingCommentText: { default: undefined }, pendingCommentDrafts: { default: undefined }, status: { linked: CombinedStatusBuilder }, - reviewRequirement: { default: null}, + reviewRequirement: { default: null }, mergeable: { default: PullRequestMergeability.Mergeable }, defaultMergeMethod: { default: 'merge' }, mergeMethodsAvailability: { default: { merge: true, squash: true, rebase: true } },