Merge remote-tracking branch 'origin/master' into pr/JeffreyCA/879

Author: rebornix

package.json

@@ -40,7 +40,7 @@
         "githubPullRequests.hosts": {
           "type": "array",
           "default": [],
-          "description": "List of host credentials. For example, \"github.hosts\": [ { \"host\": \"https://github.com\", \"token\": \"GITHUB TOKEN\" } ]",
+          "description": "List of host credentials. For example, \"githubPullRequests.hosts\": [ { \"host\": \"https://github.com\", \"token\": \"GITHUB TOKEN\" } ]",
           "items": {
             "type": "object",
             "properties": {
@@ -434,6 +434,7 @@
   "devDependencies": {
     "@types/chai": "^4.1.4",
     "@types/debounce": "^3.0.0",
+    "@types/graphql": "^14.0.5",
     "@types/keytar": "^4.0.1",
     "@types/lodash": "^4.14.106",
     "@types/markdown-it": "^0.0.5",

preview-src/cache.ts

@@ -38,6 +38,14 @@ export function getState(): PullRequest {
 }
 
 export function setState(pullRequest: PullRequest): void {
+	let oldPullRequest = getState();
+
+	if (oldPullRequest.number && oldPullRequest.number === pullRequest.number) {
+		pullRequest = Object.assign(pullRequest, {
+			pendingCommentText: oldPullRequest.pendingCommentText
+		});
+	}
+
 	vscode.setState(pullRequest);
 }
 

preview-src/index.css

@@ -286,6 +286,7 @@ body button.checkedOut svg {
 	padding-top: 5px;
 	margin-left: auto;
 	display: flex;
+	align-self: start;
 }
 
 .title-container {

preview-src/index.ts

@@ -7,7 +7,7 @@ import * as debounce from 'debounce';
 import { dateFromNow } from '../src/common/utils';
 import { EventType, isReviewEvent } from '../src/common/timelineEvent';
 import { PullRequestStateEnum } from '../src/github/interface';
-import { renderTimelineEvent, getStatus, renderComment, renderReview, ActionsBar, renderStatusChecks, updatePullRequestState, ElementIds, EditAction } from './pullRequestOverviewRenderer';
+import { renderTimelineEvent, getStatus, renderComment, renderReview, ActionsBar, renderStatusChecks, updatePullRequestState, ElementIds } from './pullRequestOverviewRenderer';
 import md from './mdRenderer';
 const emoji = require('node-emoji');
 import { getMessageHandler } from './message';
@@ -110,7 +110,8 @@ function renderTitle(pr: PullRequest): HTMLElement {
 			title.textContent = text;
 		}
 
-		const editAction = new EditAction(
+		const actionsBar = new ActionsBar(
+			titleContainer,
 			{
 				body: pr.title,
 				id: pr.number.toString()
@@ -119,11 +120,21 @@ function renderTitle(pr: PullRequest): HTMLElement {
 			messageHandler,
 			updateTitle,
 			'pr.edit-title',
+			undefined,
+			undefined,
 			[prNumber]
-		);
+			);
+
+		const renderedActionsBar = actionsBar.render();
+		actionsBar.registerActionBarListeners();
+		titleHeader.appendChild(renderedActionsBar);
+
+		if (pr.pendingCommentDrafts && pr.pendingCommentDrafts[pr.number]) {
+			actionsBar.startEdit(pr.pendingCommentDrafts[pr.number]);
+		}
 
 		title.addEventListener('click', () => {
-			editAction.startEdit();
+			actionsBar.startEdit();
 		});
 	}
 
@@ -142,6 +153,7 @@ function renderDescription(pr: PullRequest): HTMLElement {
 	commentHeader.classList.add('description-header');
 
 	const commentBody = document.createElement('div');
+	commentBody.className = 'comment-body';
 	commentBody.innerHTML = pr.bodyHTML ?
 		pr.bodyHTML :
 		pr.body

preview-src/pullRequestOverviewRenderer.ts

@@ -469,7 +469,8 @@ export class ActionsBar {
 		private _updateHandler: (value: any) => void,
 		private _editCommand?: string,
 		private _deleteCommand?: string,
-		private _review?: ReviewNode) {
+		private _review?: ReviewNode,
+		private _elementsToHide?: HTMLElement[]) {
 
 	}
 
@@ -480,7 +481,7 @@ export class ActionsBar {
 		if (this._editCommand) {
 			const editButton = document.createElement('button');
 			editButton.innerHTML = editIcon;
-			this._editAction = new EditAction(this._data, this._renderedComment, this._messageHandler, this._updateHandler, this._editCommand, [this._actionsBar]);
+			this._editAction = new EditAction(this._data, this._renderedComment, this._messageHandler, this._updateHandler, this._editCommand, (this._elementsToHide || []).concat(this._actionsBar));
 			editButton.addEventListener('click', () => this._editAction.startEdit());
 			this._actionsBar.appendChild(editButton);
 		}

src/authentication/githubServer.ts

@@ -92,15 +92,22 @@ export class GitHubManager {
 
 	public static validateScopes(host: vscode.Uri, scopes: string): boolean {
 		if (!scopes) {
-			return false;
+			Logger.appendLine(`[SKIP] validateScopes(${host.toString()}): No scopes available.`);
+			return true;
 		}
 		const tokenScopes = scopes.split(', ');
-		return this.AppScopes.every(x =>
+		const scopesNotFound = this.AppScopes.filter(x => !(
 			tokenScopes.indexOf(x) >= 0 ||
 			tokenScopes.indexOf(this.getScopeSuperset(x)) >= 0 ||
 			// some scopes don't exist on older versions of GHE, treat them as optional
 			(this.isDotCom(host) || GHE_OPTIONAL_SCOPES[x])
-		);
+		));
+		if (scopesNotFound.length) {
+			Logger.appendLine(`[FAIL] validateScopes(${host.toString()}): ${scopesNotFound.length} scopes missing`);
+			scopesNotFound.forEach(scope => Logger.appendLine(`   - ${scope}`));
+			return false;
+		}
+		return true;
 	}
 
 	private static getScopeSuperset(scope: string): string {
@@ -186,19 +193,15 @@ export class GitHubServer {
 
 		return new Promise<IHostConfiguration>((resolve, _) => {
 			const get = https.request(options, res => {
-				let hostConfig: IHostConfiguration | undefined;
 				try {
 					if (res.statusCode === 200) {
 						const scopes = res.headers['x-oauth-scopes'] as string;
-						if (GitHubManager.validateScopes(this.hostUri, scopes)) {
-							this.hostConfiguration.token = token;
-							hostConfig = this.hostConfiguration;
-						}
+						GitHubManager.validateScopes(this.hostUri, scopes);
+						resolve(this.hostConfiguration);
 					}
 				} catch (e) {
 					Logger.appendLine(`validate() error ${e}`);
 				}
-				resolve(hostConfig);
 			});
 
 			get.end();

src/github/credentials.ts

@@ -4,7 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import * as Octokit from '@octokit/rest';
-import { ApolloClient, InMemoryCache, NormalizedCacheObject } from 'apollo-boost';
+import { ApolloClient, InMemoryCache, NormalizedCacheObject, gql } from 'apollo-boost';
 import { setContext } from 'apollo-link-context';
 import * as vscode from 'vscode';
 import { agent } from '../common/net';
@@ -25,7 +25,7 @@ const AUTH_INPUT_TOKEN_CMD = 'auth.inputTokenCallback';
 
 export interface GitHub {
 	octokit: Octokit;
-	graphql: ApolloClient<NormalizedCacheObject>;
+	graphql: ApolloClient<NormalizedCacheObject> | null;
 }
 
 export class CredentialStore {
@@ -74,7 +74,7 @@ export class CredentialStore {
 
 		if (token) {
 			if (await server.validate(token)) {
-				octokit = this.createHub({ host, token });
+				octokit = await this.createHub({ host, token });
 			} else {
 				Logger.debug(`Token is no longer valid for host ${host}.`, 'Authentication');
 			}
@@ -137,7 +137,7 @@ export class CredentialStore {
 				this.willStartLogin(authority);
 				const login = await server.login();
 				if (login && login.token) {
-					octokit = this.createHub(login);
+					octokit = await this.createHub(login);
 					await setToken(login.host, login.token, { emit: false });
 					vscode.window.showInformationMessage(`You are now signed in to ${authority}`);
 				}
@@ -174,7 +174,7 @@ export class CredentialStore {
 		return octokit && (octokit as any).currentUser && (octokit as any).currentUser.login === username;
 	}
 
-	private createHub(creds: IHostConfiguration): GitHub {
+	private async createHub(creds: IHostConfiguration): Promise<GitHub> {
 		const baseUrl = `${HostHelper.getApiHost(creds).toString().slice(0, -1)}${HostHelper.getApiPath(creds, '')}`;
 		let octokit = new Octokit({
 			agent,
@@ -187,17 +187,30 @@ export class CredentialStore {
 			token: creds.token || '',
 		});
 
-		return {
-			octokit,
-			graphql: new ApolloClient({
-				link: link(baseUrl, creds.token || ''),
-				cache: new InMemoryCache,
-				defaultOptions: {
-					query: {
-						fetchPolicy: 'no-cache'
-					}
+		const graphql = new ApolloClient({
+			link: link(baseUrl, creds.token || ''),
+			cache: new InMemoryCache,
+			defaultOptions: {
+				query: {
+					fetchPolicy: 'no-cache'
 				}
+			}
+		});
+
+		let supportsGraphQL = true;
+		await graphql.query({ query: gql `query { viewer { login } }` })
+			.then(result => {
+				Logger.appendLine(`${baseUrl}: GraphQL support detected`);
+				Logger.appendLine(JSON.stringify(result, null, 2));
 			})
+			.catch(err => {
+				Logger.appendLine(`${baseUrl}: GraphQL not supported (${err.message})`);
+				supportsGraphQL = false;
+			});
+
+		return {
+			octokit,
+			graphql: supportsGraphQL ? graphql : null,
 		};
 	}
 

src/github/githubRepository.ts

@@ -11,7 +11,7 @@ import { PRType, IGitHubRepository, IAccount } from './interface';
 import { PullRequestModel } from './pullRequestModel';
 import { CredentialStore, GitHub } from './credentials';
 import { AuthenticationError } from '../common/authentication';
-import { QueryOptions, MutationOptions, ApolloQueryResult, NetworkStatus } from 'apollo-boost';
+import { QueryOptions, MutationOptions, ApolloQueryResult, NetworkStatus, FetchResult } from 'apollo-boost';
 import { PRDocumentCommentProvider, PRDocumentCommentProviderGraphQL } from '../view/prDocumentCommentProvider';
 import { convertRESTPullRequestToRawPullRequest, parseGraphQLPullRequest } from './utils';
 import { PullRequestResponse, MentionableUsersResponse } from './graphql';
@@ -53,7 +53,7 @@ export class GitHubRepository implements IGitHubRepository, vscode.Disposable {
 			}
 
 			await this.ensure();
-			this.commentsProvider = this.supportsGraphQl() ? new PRDocumentCommentProviderGraphQL() : new PRDocumentCommentProvider();
+			this.commentsProvider = this.supportsGraphQl ? new PRDocumentCommentProviderGraphQL() : new PRDocumentCommentProvider();
 			this._toDispose.push(vscode.workspace.registerDocumentCommentProvider(this.commentsProvider));
 		} catch (e) {
 			console.log(e);
@@ -72,7 +72,7 @@ export class GitHubRepository implements IGitHubRepository, vscode.Disposable {
 	constructor(public remote: Remote, private readonly _credentialStore: CredentialStore) {
 	}
 
-	supportsGraphQl(): boolean {
+	get supportsGraphQl(): boolean {
 		return !!(this.hub && this.hub.graphql);
 	}
 
@@ -94,7 +94,7 @@ export class GitHubRepository implements IGitHubRepository, vscode.Disposable {
 		return rsp;
 	}
 
-	mutate = async <T>(mutation: MutationOptions): Promise<ApolloQueryResult<T>> => {
+	mutate = async <T>(mutation: MutationOptions): Promise<FetchResult<T>> => {
 		const gql = this.hub && this.hub.graphql;
 		if (!gql) {
 			Logger.debug(`Not available for query: ${mutation}`, GRAPHQL_COMPONENT_ID);
@@ -120,8 +120,8 @@ export class GitHubRepository implements IGitHubRepository, vscode.Disposable {
 		}
 		const { octokit, remote } = await this.ensure();
 		const result = await octokit.repos.get({
-				owner: remote.owner,
-				repo: remote.repositoryName
+			owner: remote.owner,
+			repo: remote.repositoryName
 		});
 		Logger.debug(`Fetch metadata ${remote.owner}/${remote.repositoryName} - done`, GitHubRepository.ID);
 		this._metadata = Object.assign(result.data, { currentUser: (octokit as any).currentUser });
@@ -130,7 +130,7 @@ export class GitHubRepository implements IGitHubRepository, vscode.Disposable {
 
 	async resolveRemote(): Promise<void> {
 		try {
-			const {clone_url} = await this.getMetadata();
+			const { clone_url } = await this.getMetadata();
 			this.remote = parseRemote(this.remote.remoteName, clone_url, this.remote.gitProtocol)!;
 		} catch (e) {
 			Logger.appendLine(`Unable to resolve remote: ${e}`);
@@ -279,20 +279,9 @@ export class GitHubRepository implements IGitHubRepository, vscode.Disposable {
 	async getPullRequest(id: number): Promise<PullRequestModel | undefined> {
 		try {
 			Logger.debug(`Fetch pull request ${id} - enter`, GitHubRepository.ID);
-			const { octokit, query, remote } = await this.ensure();
-			let prsResult = await octokit.pullRequests.get({
-				owner: remote.owner,
-				repo: remote.repositoryName,
-				number: id
-			});
-			Logger.debug(`Fetch pull request ${id} - done`, GitHubRepository.ID);
-
-			if (!prsResult.data.head.repo) {
-				Logger.appendLine('The remote branch for this PR was already deleted.', GitHubRepository.ID);
-				return;
-			}
+			const { octokit, query, remote, supportsGraphQl } = await this.ensure();
 
-			if (this.supportsGraphQl()) {
+			if (supportsGraphQl) {
 				const { data } = await query<PullRequestResponse>({
 					query: queries.PullRequest,
 					variables: {
@@ -327,16 +316,16 @@ export class GitHubRepository implements IGitHubRepository, vscode.Disposable {
 	}
 
 	async getMentionableUsers(): Promise<IAccount[]> {
-		try {
-			Logger.debug(`Fetch mentionable users - enter`, GitHubRepository.ID);
-			const { query, supportsGraphQl, remote } = await this.ensure();
+		Logger.debug(`Fetch mentionable users - enter`, GitHubRepository.ID);
+		const { query, supportsGraphQl, remote } = await this.ensure();
 
-			if (supportsGraphQl) {
-				let after = null;
-				let hasNextPage = false;
-				let ret: IAccount[] = [];
+		if (supportsGraphQl) {
+			let after = null;
+			let hasNextPage = false;
+			let ret: IAccount[] = [];
 
-				do {
+			do {
+				try {
 					const result: { data: MentionableUsersResponse } = await query<MentionableUsersResponse>({
 						query: queries.GetMentionableUsers,
 						variables: {
@@ -359,13 +348,13 @@ export class GitHubRepository implements IGitHubRepository, vscode.Disposable {
 
 					hasNextPage = result.data.repository.mentionableUsers.pageInfo.hasNextPage;
 					after = result.data.repository.mentionableUsers.pageInfo.endCursor;
-				} while (hasNextPage);
+				} catch (e) {
+					Logger.debug(`Unable to fetch mentionable users: ${e}`, GitHubRepository.ID);
+					return ret;
+				}
+			} while (hasNextPage);
 
-				return ret;
-			}
-		} catch (e) {
-			Logger.appendLine(`Unable to fetch mentionable users: ${e}`, GitHubRepository.ID);
-			return [];
+			return ret;
 		}
 
 		return [];