Pull Request Tree GitHub Enterprise Avatars

package.json

@@ -138,7 +138,7 @@
         {
           "id": "github-pull-requests",
           "title": "GitHub Pull Requests",
-          "icon": "resources/icons/github.svg"
+          "icon": "resources/icons/light/github.svg"
         }
       ]
     },

src/common/resources.ts

@@ -23,6 +23,7 @@ export class Resource {
 				Ignored: context.asAbsolutePath(path.join('resources', 'icons', 'light', 'status-ignored.svg')),
 				Conflict: context.asAbsolutePath(path.join('resources', 'icons', 'light', 'status-conflict.svg')),
 				Comment: context.asAbsolutePath(path.join('resources', 'icons', 'comment.svg')),
+				Avatar: context.asAbsolutePath(path.join('resources', 'icons', 'light', 'github.svg')),
 				Fold: context.asAbsolutePath(path.join('resources', 'icons', 'light', 'fold.svg')),
 				Description: context.asAbsolutePath(path.join('resources', 'icons', 'light', 'git-pull-request.svg'))
 			},
@@ -36,6 +37,7 @@ export class Resource {
 				Ignored: context.asAbsolutePath(path.join('resources', 'icons', 'dark', 'status-ignored.svg')),
 				Conflict: context.asAbsolutePath(path.join('resources', 'icons', 'dark', 'status-conflict.svg')),
 				Comment: context.asAbsolutePath(path.join('resources', 'icons', 'comment.svg')),
+				Avatar: context.asAbsolutePath(path.join('resources', 'icons', 'dark', 'github.svg')),
 				Fold: context.asAbsolutePath(path.join('resources', 'icons', 'dark', 'fold.svg')),
 				Description: context.asAbsolutePath(path.join('resources', 'icons', 'dark', 'git-pull-request.svg'))
 			},

src/github/githubRepository.ts

@@ -16,6 +16,7 @@ import { PRDocumentCommentProvider } from '../view/prDocumentCommentProvider';
 import { convertRESTPullRequestToRawPullRequest, parseGraphQLPullRequest } from './utils';
 import { PullRequestResponse, MentionableUsersResponse } from './graphql';
 const queries = require('./queries.gql');
+import axois, { AxiosResponse } from 'axios';
 
 export const PULL_REQUEST_PAGE_SIZE = 20;
 
@@ -30,6 +31,7 @@ export class GitHubRepository implements IGitHubRepository, vscode.Disposable {
 	static ID = 'GitHubRepository';
 	private _hub: GitHub | undefined;
 	private _initialized: boolean;
+	private _repositoryReturnsAvatar: boolean|null;
 	private _metadata: any;
 	private _toDispose: vscode.Disposable[] = [];
 	public commentsController?: vscode.CommentController;
@@ -72,6 +74,7 @@ export class GitHubRepository implements IGitHubRepository, vscode.Disposable {
 	}
 
 	constructor(public remote: Remote, private readonly _credentialStore: CredentialStore) {
+		this._repositoryReturnsAvatar = remote.host.toLowerCase() === 'github.com' ? true : null;
 	}
 
 	get supportsGraphQl(): boolean {
@@ -206,7 +209,29 @@ export class GitHubRepository implements IGitHubRepository, vscode.Disposable {
 	}
 
 	async getPullRequests(prType: PRType, page?: number): Promise<PullRequestData | undefined> {
-		return prType === PRType.All ? this.getAllPullRequests(page) : this.getPullRequestsForCategory(prType, page);
+		return await (prType === PRType.All ? this.getAllPullRequests(page) : this.getPullRequestsForCategory(prType, page));
+	}
+
+	public async ensureRepositoryReturnsAvatar(testAvatarUrl: string): Promise<boolean> {
+		if(this._repositoryReturnsAvatar === null) {
+			let response: AxiosResponse | null = null;
+
+			try {
+				response  = await axois({method: 'get', url: testAvatarUrl, maxRedirects: 0});
+			} catch (err) {
+				if(err && err instanceof Error) {
+					response = (<any> err).response as AxiosResponse;
+				}
+			}
+
+			if(response && response.status === 200) {
+				this._repositoryReturnsAvatar = true;
+			}
+
+			this._repositoryReturnsAvatar = false;
+		}
+
+		return this._repositoryReturnsAvatar;
 	}
 
 	private async getAllPullRequests(page?: number): Promise<PullRequestData | undefined> {
@@ -221,6 +246,12 @@ export class GitHubRepository implements IGitHubRepository, vscode.Disposable {
 			});
 
 			const hasMorePages = !!result.headers.link && result.headers.link.indexOf('rel="next"') > -1;
+
+			let repoReturnsAvatar: boolean = true;
+			if(result && result.data.length > 0) {
+				repoReturnsAvatar = await this.ensureRepositoryReturnsAvatar(result.data[0].user.avatar_url);
+			}
+
 			const pullRequests = result.data
 				.map(
 					pullRequest => {
@@ -232,7 +263,8 @@ export class GitHubRepository implements IGitHubRepository, vscode.Disposable {
 						}
 
 						const item = convertRESTPullRequestToRawPullRequest(pullRequest);
-						return new PullRequestModel(this, this.remote, item);
+
+						return new PullRequestModel(this, this.remote, item, repoReturnsAvatar);
 					}
 				)
 				.filter(item => item !== null) as PullRequestModel[];
@@ -278,15 +310,23 @@ export class GitHubRepository implements IGitHubRepository, vscode.Disposable {
 			});
 
 			const hasMorePages = !!headers.link && headers.link.indexOf('rel="next"') > -1;
-			const pullRequests = await Promise.all(promises).then(values => {
-				return values.map(item => {
-					if (!item.data.head.repo) {
-						Logger.appendLine('GitHubRepository> The remote branch for this PR was already deleted.');
-						return null;
-					}
-					return new PullRequestModel(this, this.remote, convertRESTPullRequestToRawPullRequest(item.data));
-				}).filter(item => item !== null) as PullRequestModel[];
-			});
+			const pullRequestResponses = await Promise.all(promises);
+
+			let repoReturnsAvatar = true;
+			if(pullRequestResponses && pullRequestResponses.length > 0) {
+				repoReturnsAvatar = await this.ensureRepositoryReturnsAvatar(pullRequestResponses[0].data.user.avatar_url);
+			}
+
+			const pullRequests = pullRequestResponses.map(response => {
+				if (!response.data.head.repo) {
+					Logger.appendLine('GitHubRepository> The remote branch for this PR was already deleted.');
+					return null;
+				}
+
+				const item = convertRESTPullRequestToRawPullRequest(response.data,);
+				return new PullRequestModel(this, this.remote, item, repoReturnsAvatar);
+			}).filter(item => item !== null) as PullRequestModel[];
+
 			Logger.debug(`Fetch pull request catogory ${PRType[prType]} - done`, GitHubRepository.ID);
 
 			return {
@@ -318,9 +358,11 @@ export class GitHubRepository implements IGitHubRepository, vscode.Disposable {
 						number: id
 					}
 				});
-
 				Logger.debug(`Fetch pull request ${id} - done`, GitHubRepository.ID);
-				return new PullRequestModel(this, remote, parseGraphQLPullRequest(data));
+
+				const repoReturnsAvatar = await this.ensureRepositoryReturnsAvatar(data.repository.pullRequest.author.avatarUrl);
+
+				return new PullRequestModel(this, remote, parseGraphQLPullRequest(data), repoReturnsAvatar);
 			} else {
 				let { data } = await octokit.pullRequests.get({
 					owner: remote.owner,
@@ -329,13 +371,15 @@ export class GitHubRepository implements IGitHubRepository, vscode.Disposable {
 				});
 				Logger.debug(`Fetch pull request ${id} - done`, GitHubRepository.ID);
 
+				const repoReturnsAvatar = await this.ensureRepositoryReturnsAvatar(data.user.avatar_url);
+
 				if (!data.head.repo) {
 					Logger.appendLine('The remote branch for this PR was already deleted.', GitHubRepository.ID);
 					return;
 				}
 
 				let item = convertRESTPullRequestToRawPullRequest(data);
-				return new PullRequestModel(this, remote, item);
+				return new PullRequestModel(this, remote, item, repoReturnsAvatar);
 			}
 		} catch (e) {
 			Logger.appendLine(`GithubRepository> Unable to fetch PR: ${e}`);

src/github/pullRequestManager.ts

@@ -1050,7 +1050,8 @@ export class PullRequestManager {
 			// Create PR
 			let { data } = await repo.octokit.pullRequests.create(params);
 			const item = convertRESTPullRequestToRawPullRequest(data);
-			const pullRequestModel = new PullRequestModel(repo, repo.remote, item);
+			const repoReturnsAvatar = await repo.ensureRepositoryReturnsAvatar(item.user.avatarUrl);
+			const pullRequestModel = new PullRequestModel(repo, repo.remote, item, repoReturnsAvatar);
 
 			const branchNameSeparatorIndex = params.head.indexOf(':');
 			const branchName = params.head.slice(branchNameSeparatorIndex + 1);

src/github/pullRequestModel.ts

@@ -28,25 +28,27 @@ export class PullRequestModel {
 		return this.state === PullRequestStateEnum.Merged;
 	}
 
-	public get userAvatar(): string {
-		if (this.prItem) {
+	public get userAvatar(): string | undefined {
+		if (this.prItem && this._repositoryReturnsAvatar) {
 			return this.prItem.user.avatarUrl;
 		}
 
-		return '';
+		return undefined;
 	}
 	public get userAvatarUri(): vscode.Uri | undefined {
 		if (this.prItem) {
 			let key = this.userAvatar;
-			let gravatar = vscode.Uri.parse(`${key}&s=${64}`);
+			if (key) {
+				let uri = vscode.Uri.parse(`${key}&s=${64}`);
 
-			// hack, to ensure queries are not wrongly encoded.
-			const originalToStringFn = gravatar.toString;
-			gravatar.toString = function (skipEncoding?: boolean | undefined) {
-				return originalToStringFn.call(gravatar, true);
-			};
+				// hack, to ensure queries are not wrongly encoded.
+				const originalToStringFn = uri.toString;
+				uri.toString = function (skipEncoding?: boolean | undefined) {
+					return originalToStringFn.call(uri, true);
+				};
 
-			return gravatar;
+				return uri;
+			}
 		}
 
 		return undefined;
@@ -80,7 +82,7 @@ export class PullRequestModel {
 	public head: GitHubRef;
 	public base: GitHubRef;
 
-	constructor(public readonly githubRepository: GitHubRepository, public readonly remote: Remote, public prItem: PullRequest) {
+	constructor(public readonly githubRepository: GitHubRepository, public readonly remote: Remote, public prItem: PullRequest, private _repositoryReturnsAvatar: boolean) {
 		this.update(prItem);
 	}
 

src/github/utils.ts

@@ -35,7 +35,7 @@ export function convertToVSCodeComment(comment: Comment, command: vscode.Command
 		body: new vscode.MarkdownString(comment.body),
 		selectCommand: command,
 		userName: comment.user!.login,
-		userIconPath: vscode.Uri.parse(comment.user!.avatarUrl),
+		userIconPath: comment.user && comment.user.avatarUrl ? vscode.Uri.parse(comment.user.avatarUrl) : undefined,
 		label: !!comment.isDraft ? 'Pending' : undefined,
 		commentReactions: comment.reactions ? comment.reactions.map(reaction => {
 			return { label: reaction.label, hasReacted: reaction.viewerHasReacted, count: reaction.count, iconPath: reaction.icon };

src/test/github/pullRequestModel.test.ts

@@ -171,19 +171,19 @@ const pr: Octokit.PullRequestsGetResponse | Octokit.PullRequestsGetAllResponseIt
 
 describe('PullRequestModel', () => {
 	it('should return `state` properly as `open`', () => {
-		const open = new PullRequestModel(repo, remote, convertRESTPullRequestToRawPullRequest(pr));
+		const open = new PullRequestModel(repo, remote, convertRESTPullRequestToRawPullRequest(pr), true);
 
 		assert.equal(open.state, PullRequestStateEnum.Open);
 	});
 
 	it('should return `state` properly as `closed`', () => {
-		const open = new PullRequestModel(repo, remote, convertRESTPullRequestToRawPullRequest({ ...pr, state: 'closed' }));
+		const open = new PullRequestModel(repo, remote, convertRESTPullRequestToRawPullRequest({ ...pr, state: 'closed' }), true);
 
 		assert.equal(open.state, PullRequestStateEnum.Closed);
 	});
 
 	it('should return `state` properly as `merged`', () => {
-		const open = new PullRequestModel(repo, remote, convertRESTPullRequestToRawPullRequest({ ...pr, merged: true, state: 'closed' }));
+		const open = new PullRequestModel(repo, remote, convertRESTPullRequestToRawPullRequest({ ...pr, merged: true, state: 'closed' }), true);
 
 		assert.equal(open.state, PullRequestStateEnum.Merged);
 	});

src/view/treeNodes/pullRequestNode.ts

@@ -407,6 +407,8 @@ export class PRNode extends TreeNode implements CommentHandler, vscode.Commentin
 			collapsibleState: 1,
 			contextValue: 'pullrequest' + (this._isLocal ? ':local' : '') + (currentBranchIsForThisPR ? ':active' : ':nonactive'),
 			iconPath: this.pullRequestModel.userAvatarUri
+				? this.pullRequestModel.userAvatarUri
+				: { light: Resource.icons.light.Avatar, dark: Resource.icons.dark.Avatar }
 		};
 	}