Add an event for when PR model changes (#7310)

Part 1: add the event and dispose better
Still to be done:
- delete _onDidInvalidate
- move timeline back into model

Author: alexr00

src/commands.ts

@@ -1347,9 +1347,7 @@ ${contents}
 			if (prQuery) {
 				for (const githubRepos of (manager?.gitHubRepositories ?? [])) {
 					const prNumber = Number(prQuery.prNumber);
-					if (githubRepos.pullRequestModels.has(prNumber)) {
-						return githubRepos.pullRequestModels.get(prNumber);
-					}
+					return githubRepos.getExistingPullRequestModel(prNumber);
 				}
 			}
 		} else {
@@ -1732,8 +1730,8 @@ ${contents}
 		for (const folderManager of reposManager.folderManagers) {
 			for (const githubRepository of folderManager.gitHubRepositories) {
 				for (const pullRequest of githubRepository.pullRequestModels) {
-					if (pullRequest[1].isResolved() && pullRequest[1].reviewThreadsCacheReady) {
-						pullRequest[1].initializeReviewThreadCache();
+					if (pullRequest.isResolved() && pullRequest.reviewThreadsCacheReady) {
+						pullRequest.initializeReviewThreadCache();
 					}
 				}
 			}

src/github/createPRViewProvider.ts

@@ -278,8 +278,10 @@ export abstract class BaseCreatePullRequestViewProvider<T extends BasePullReques
 			return;
 		}
 		try {
-			// TODO: We need to resolve the user and then use the replace function.
-			await pr.addAssignees([resolved]);
+			const user = await pr.githubRepository.resolveUser(resolved);
+			if (user) {
+				await pr.replaceAssignees([user]);
+			}
 		} catch (e) {
 			Logger.error(`Unable to assign pull request to user ${resolved}.`, BaseCreatePullRequestViewProvider.ID);
 		}

src/github/folderRepositoryManager.ts

@@ -43,7 +43,7 @@ import { ConflictResolutionCoordinator } from './conflictResolutionCoordinator';
 import { Conflict, ConflictResolutionModel } from './conflictResolutionModel';
 import { CredentialStore } from './credentials';
 import { CopilotWorkingStatus, GitHubRepository, GraphQLError, GraphQLErrorType, IMetadata, ItemsData, PULL_REQUEST_PAGE_SIZE, PullRequestData, TeamReviewerRefreshKind, ViewerPermission } from './githubRepository';
-import { MergeMethod as GraphQLMergeMethod, MergePullRequestInput, MergePullRequestResponse, PullRequestResponse, PullRequestState, UserResponse } from './graphql';
+import { MergeMethod as GraphQLMergeMethod, MergePullRequestInput, MergePullRequestResponse, PullRequestResponse, PullRequestState } from './graphql';
 import { IAccount, ILabel, IMilestone, IProject, IPullRequestsPagingOptions, Issue, ITeam, MergeMethod, PRType, PullRequestMergeability, RepoAccessAndMergeMethods, User } from './interface';
 import { IssueModel } from './issueModel';
 import { PullRequestGitHelper, PullRequestMetadata } from './pullRequestGitHelper';
@@ -56,7 +56,6 @@ import {
 	loginComparator,
 	parseCombinedTimelineEvents,
 	parseGraphQLPullRequest,
-	parseGraphQLUser,
 	teamComparator,
 	variableSubstitution,
 } from './utils';
@@ -2221,23 +2220,7 @@ export class FolderRepositoryManager extends Disposable {
 	async resolveUser(owner: string, repositoryName: string, login: string): Promise<User | undefined> {
 		Logger.debug(`Fetch user ${login}`, this.id);
 		const githubRepository = await this.createGitHubRepositoryFromOwnerName(owner, repositoryName);
-		const { query, schema } = await githubRepository.ensure();
-
-		try {
-			const { data } = await query<UserResponse>({
-				query: schema.GetUser,
-				variables: {
-					login,
-				},
-			});
-			return parseGraphQLUser(data, githubRepository);
-		} catch (e) {
-			// Ignore cases where the user doesn't exist
-			if (!(e.message as (string | undefined))?.startsWith('GraphQL error: Could not resolve to a User with the login of')) {
-				Logger.warn(e.message);
-			}
-		}
-		return undefined;
+		return githubRepository.resolveUser(login);
 	}
 
 	async getMatchingPullRequestMetadataForBranch() {

src/github/githubRepository.ts

@@ -5,10 +5,11 @@
 
 import * as buffer from 'buffer';
 import { ApolloQueryResult, DocumentNode, FetchResult, MutationOptions, NetworkStatus, QueryOptions } from 'apollo-boost';
+import LRUCache from 'lru-cache';
 import * as vscode from 'vscode';
 import { AuthenticationError, AuthProvider, GitHubServerType, isSamlError } from '../common/authentication';
 import { COPILOT_ACCOUNTS, IComment, IReviewThread } from '../common/comment';
-import { Disposable } from '../common/lifecycle';
+import { Disposable, disposeAll } from '../common/lifecycle';
 import Logger from '../common/logger';
 import { GitHubRemote, parseRemote } from '../common/remote';
 import { ITelemetry } from '../common/telemetry';
@@ -43,6 +44,7 @@ import {
 	RevertPullRequestResponse,
 	SuggestedActorsResponse,
 	TimelineEventsResponse,
+	UserResponse,
 	ViewerPermissionResponse,
 } from './graphql';
 import {
@@ -57,6 +59,7 @@ import {
 	PullRequestChecks,
 	PullRequestReviewRequirement,
 	RepoAccessAndMergeMethods,
+	User,
 } from './interface';
 import { IssueModel } from './issueModel';
 import { LoggingOctokit } from './loggingOctokit';
@@ -76,6 +79,7 @@ import {
 	parseCombinedTimelineEvents,
 	parseGraphQLIssue,
 	parseGraphQLPullRequest,
+	parseGraphQLUser,
 	parseGraphQLViewerPermission,
 	parseMergeMethod,
 	parseMilestone,
@@ -162,7 +166,13 @@ export class GitHubRepository extends Disposable {
 	protected _metadata: Promise<IMetadata> | undefined;
 	public commentsController?: vscode.CommentController;
 	public commentsHandler?: PRCommentControllerRegistry;
-	private _pullRequestModels = new Map<number, PullRequestModel>();
+	private _pullRequestModelsByNumber: LRUCache<number, { model: PullRequestModel, disposables: vscode.Disposable[] }> = new LRUCache({
+		maxAge: 1000 * 60 * 60 * 4 /* 4 hours */, stale: true, updateAgeOnGet: true,
+		dispose: (_key, value) => {
+			disposeAll(value.disposables);
+			value.model.dispose();
+		}
+	});
 	private _queriesSchema: any;
 	private _areQueriesLimited: boolean = false;
 
@@ -186,8 +196,12 @@ export class GitHubRepository extends Disposable {
 		return this.remote.equals(repo.remote);
 	}
 
-	get pullRequestModels(): Map<number, PullRequestModel> {
-		return this._pullRequestModels;
+	getExistingPullRequestModel(prNumber: number): PullRequestModel | undefined {
+		return this._pullRequestModelsByNumber.get(prNumber)?.model;
+	}
+
+	get pullRequestModels(): PullRequestModel[] {
+		return Array.from(this._pullRequestModelsByNumber.values().map(value => value.model));
 	}
 
 	public async ensureCommentsController(): Promise<void> {
@@ -940,19 +954,26 @@ export class GitHubRepository extends Disposable {
 	}
 
 	createOrUpdatePullRequestModel(pullRequest: PullRequest): PullRequestModel {
-		let model = this._pullRequestModels.get(pullRequest.number);
+		let model = this._pullRequestModelsByNumber.get(pullRequest.number)?.model;
 		if (model) {
 			model.update(pullRequest);
 		} else {
 			model = new PullRequestModel(this._credentialStore, this._telemetry, this, this.remote, pullRequest);
+			const prModel = model;
 			model.onDidInvalidate(() => this.getPullRequest(pullRequest.number));
-			this._pullRequestModels.set(pullRequest.number, model);
+			const disposables: vscode.Disposable[] = [];
+			disposables.push(model.onDidChange(() => this._onPullRequestModelChanged(prModel)));
+			this._pullRequestModelsByNumber.set(pullRequest.number, { model, disposables });
 			this._onDidAddPullRequest.fire(model);
 		}
 
 		return model;
 	}
 
+	private _onPullRequestModelChanged(model: PullRequestModel): void {
+		this._onDidChangePullRequests.fire([model]);
+	}
+
 	async createPullRequest(params: OctokitCommon.PullsCreateParams): Promise<PullRequestModel> {
 		try {
 			Logger.debug(`Create pull request - enter`, this.id);
@@ -1233,6 +1254,27 @@ export class GitHubRepository extends Disposable {
 		return ret;
 	}
 
+	async resolveUser(login: string): Promise<User | undefined> {
+		Logger.debug(`Fetch user ${login}`, this.id);
+		const { query, schema } = await this.ensure();
+
+		try {
+			const { data } = await query<UserResponse>({
+				query: schema.GetUser,
+				variables: {
+					login,
+				},
+			});
+			return parseGraphQLUser(data, this);
+		} catch (e) {
+			// Ignore cases where the user doesn't exist
+			if (!(e.message as (string | undefined))?.startsWith('GraphQL error: Could not resolve to a User with the login of')) {
+				Logger.warn(e.message);
+			}
+		}
+		return undefined;
+	}
+
 	async getAssignableUsers(): Promise<IAccount[]> {
 		Logger.debug(`Fetch assignable users - enter`, this.id);
 		const { query, remote, schema } = await this.ensure();
@@ -1554,9 +1596,9 @@ export class GitHubRepository extends Disposable {
 					return [event.source.url, event];
 				}));
 
-			for (const model of this._pullRequestModels.values()) {
-				if (crossRefs.has(model.html_url)) {
-					crossRefs.delete(model.html_url);
+			for (const pr of this._pullRequestModelsByNumber.values()) {
+				if (crossRefs.has(pr.model.html_url)) {
+					crossRefs.delete(pr.model.html_url);
 				}
 			}
 			const oldEvents = issueModel.timelineEvents;

src/github/graphql.ts

@@ -558,6 +558,13 @@ export interface UpdateIssueResponse {
 			bodyHTML: string;
 			title: string;
 			titleHTML: string;
+			milestone?: {
+				title: string;
+				dueOn?: string;
+				id: string;
+				createdAt: string;
+				number: number;
+			};
 		};
 	};
 }