Resole merge base when we fullfill missing information for a pull request.

Author: rebornix

src/github/interface.ts

@@ -110,6 +110,7 @@ export interface IPullRequestModel {
 	isMerged: boolean;
 	head?: GitHubRef;
 	base?: GitHubRef;
+	mergeBase?: string;
 	localBranchName?: string;
 	userAvatar: string;
 	userAvatarUri: vscode.Uri;
@@ -131,7 +132,6 @@ export interface IPullRequestManager {
 	mayHaveMorePages(): boolean;
 	getPullRequestComments(pullRequest: IPullRequestModel): Promise<Comment[]>;
 	getPullRequestCommits(pullRequest: IPullRequestModel): Promise<Commit[]>;
-	getPullRequestMergeBase(pullRequest: IPullRequestModel): Promise<string>;
 	getCommitChangedFiles(pullRequest: IPullRequestModel, commit: Commit): Promise<FileChange[]>;
 	getReviewComments(pullRequest: IPullRequestModel, reviewId: string): Promise<Comment[]>;
 	getTimelineEvents(pullRequest: IPullRequestModel): Promise<TimelineEvent[]>;
@@ -143,7 +143,14 @@ export interface IPullRequestManager {
 	getPullRequestChangedFiles(pullRequest: IPullRequestModel): Promise<FileChange[]>;
 	getPullRequestRepositoryDefaultBranch(pullRequest: IPullRequestModel): Promise<string>;
 
-	fullfillPullRequestCommitInfo(pullRequest: IPullRequestModel): Promise<void>;
+	/**
+	 * Fullfill information for a pull request which we can't fetch with one single api call.
+	 * 1. base. This property might not exist in search results
+	 * 2. head. This property might not exist in search results
+	 * 3. merge base. This is necessary as base might not be the commit that files in Pull Request are being compared to.
+	 * @param pullRequest
+	 */
+	fullfillPullRequestMissingInfo(pullRequest: IPullRequestModel): Promise<void>;
 	updateRepositories(): Promise<void>;
 
 	/**

src/github/pullRequestManager.ts

@@ -236,16 +236,6 @@ export class PullRequestManager implements IPullRequestManager {
 		}
 	}
 
-	async getPullRequestMergeBase(pullRequest: IPullRequestModel): Promise<string> {
-		try {
-			const { remote } = await (pullRequest as PullRequestModel).githubRepository.ensure();
-			return PullRequestGitHelper.getPullRequestMergeBase(this._repository, remote, pullRequest);
-		} catch (e) {
-			vscode.window.showErrorMessage(`Fetching Pull Request merge base failed: ${formatError(e)}`);
-			return null;
-		}
-	}
-
 	async getCommitChangedFiles(pullRequest: IPullRequestModel, commit: Commit): Promise<FileChange[]> {
 		try {
 			const { octokit, remote } = await (pullRequest as PullRequestModel).githubRepository.ensure();
@@ -375,17 +365,22 @@ export class PullRequestManager implements IPullRequestManager {
 		return branch;
 	}
 
-	async fullfillPullRequestCommitInfo(pullRequest: IPullRequestModel): Promise<void> {
-		if (!pullRequest.base) {
-			// this one is from search results, which is not complete.
+	async fullfillPullRequestMissingInfo(pullRequest: IPullRequestModel): Promise<void> {
+		try {
 			const { octokit, remote } = await (pullRequest as PullRequestModel).githubRepository.ensure();
 
-			const { data } = await octokit.pullRequests.get({
-				owner: remote.owner,
-				repo: remote.repositoryName,
-				number: pullRequest.prNumber
-			});
-			pullRequest.update(data);
+			if (!pullRequest.base) {
+				const { data } = await octokit.pullRequests.get({
+					owner: remote.owner,
+					repo: remote.repositoryName,
+					number: pullRequest.prNumber
+				});
+				pullRequest.update(data);
+			}
+
+			pullRequest.mergeBase = await PullRequestGitHelper.getPullRequestMergeBase(this._repository, remote, pullRequest);
+		} catch (e) {
+			vscode.window.showErrorMessage(`Fetching Pull Request merge base failed: ${formatError(e)}`);
 		}
 	}
 

src/view/reviewManager.ts

@@ -392,9 +392,9 @@ export class ReviewManager implements vscode.DecorationProvider {
 			let outdatedComments = this._comments.filter(comment => !comment.position);
 
 			const data = await this._prManager.getPullRequestChangedFiles(pr);
-			await this._prManager.fullfillPullRequestCommitInfo(pr);
+			await this._prManager.fullfillPullRequestMissingInfo(pr);
 			let headSha = pr.head.sha;
-			let mergeBase = await this._prManager.getPullRequestMergeBase(pr);
+			let mergeBase = pr.mergeBase;
 
 			const contentChanges = await parseDiff(data, this._repository, mergeBase);
 			this._localFileChanges = contentChanges.map(change => {

src/view/treeNodes/pullRequestNode.ts

@@ -136,8 +136,8 @@ export class PRNode extends TreeNode {
 
 			const comments = await this._prManager.getPullRequestComments(this.pullRequestModel);
 			const data = await this._prManager.getPullRequestChangedFiles(this.pullRequestModel);
-			await this._prManager.fullfillPullRequestCommitInfo(this.pullRequestModel);
-			let mergeBase = await this._prManager.getPullRequestMergeBase(this.pullRequestModel);
+			await this._prManager.fullfillPullRequestMissingInfo(this.pullRequestModel);
+			let mergeBase = this.pullRequestModel.mergeBase;
 			const rawChanges = await parseDiff(data, this.repository, mergeBase);
 			this._contentChanges = rawChanges.map(change => {
 				if (change instanceof SlimFileChange) {