Skip to content

Commit d00e05c

Browse files
authored
Check if there are new PRs in the repo before doing PR search (#7799)
* Check if there are new PRs in the repo before doing PR search Fixes #7798 * Not sure why these errors didn't show in the watch task...
1 parent 8a825ab commit d00e05c

File tree

8 files changed

+153
-33
lines changed

8 files changed

+153
-33
lines changed

common/types.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
export interface RemoteInfo {
7+
owner: string;
8+
repositoryName: string;
9+
}

common/views.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,7 @@
66
import { ClosedEvent, CommentEvent } from '../src/common/timelineEvent';
77
import { GithubItemStateEnum, IAccount, ILabel, IMilestone, IProject, ITeam, MergeMethod, MergeMethodsAvailability } from '../src/github/interface';
88
import { DisplayLabel, PreReviewState } from '../src/github/views';
9-
10-
export interface RemoteInfo {
11-
owner: string;
12-
repositoryName: string;
13-
}
9+
import { RemoteInfo } from './types';
1410

1511
export interface CreateParams {
1612
availableBaseRemotes: RemoteInfo[];

src/github/createPRViewProvider.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import * as vscode from 'vscode';
7-
import { ChooseBaseRemoteAndBranchResult, ChooseCompareRemoteAndBranchResult, ChooseRemoteAndBranchArgs, CreateParamsNew, CreatePullRequestNew, RemoteInfo, TitleAndDescriptionArgs } from '../../common/views';
7+
import { RemoteInfo } from '../../common/types';
8+
import { ChooseBaseRemoteAndBranchResult, ChooseCompareRemoteAndBranchResult, ChooseRemoteAndBranchArgs, CreateParamsNew, CreatePullRequestNew, TitleAndDescriptionArgs } from '../../common/views';
89
import type { Branch, Ref } from '../api/api';
910
import { GitHubServerType } from '../common/authentication';
1011
import { emojify, ensureEmojis } from '../common/emoji';

src/github/githubRepository.ts

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -834,29 +834,37 @@ export class GitHubRepository extends Disposable {
834834
}
835835
}
836836

837-
async getMaxIssue(): Promise<number | undefined> {
837+
private async _getMaxItem(isIssue: boolean): Promise<number | undefined> {
838838
try {
839-
Logger.debug(`Fetch max issue - enter`, this.id);
839+
Logger.debug(`Fetch max ${isIssue ? 'issue' : 'pull request'} - enter`, this.id);
840840
const { query, remote, schema } = await this.ensure();
841841
const { data } = await query<MaxIssueResponse>({
842-
query: schema.MaxIssue,
842+
query: isIssue ? schema.MaxIssue : schema.MaxPullRequest,
843843
variables: {
844844
owner: remote.owner,
845845
name: remote.repositoryName,
846846
},
847847
});
848-
Logger.debug(`Fetch max issue - done`, this.id);
848+
Logger.debug(`Fetch max ${isIssue ? 'issue' : 'pull request'} - done`, this.id);
849849

850850
if (data?.repository && data.repository.issues.edges.length === 1) {
851851
return data.repository.issues.edges[0].node.number;
852852
}
853853
return;
854854
} catch (e) {
855-
Logger.error(`Unable to fetch issues with query: ${e}`, this.id);
855+
Logger.error(`Unable to fetch ${isIssue ? 'issues' : 'pull requests'} with query: ${e}`, this.id);
856856
return;
857857
}
858858
}
859859

860+
async getMaxIssue(): Promise<number | undefined> {
861+
return this._getMaxItem(true);
862+
}
863+
864+
async getMaxPullRequest(): Promise<number | undefined> {
865+
return this._getMaxItem(false);
866+
}
867+
860868
async getViewerPermission(): Promise<ViewerPermission> {
861869
try {
862870
Logger.debug(`Fetch viewer permission - enter`, this.id);

src/github/queriesShared.gql

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1247,6 +1247,23 @@ query MaxIssue($owner: String!, $name: String!) {
12471247
}
12481248
}
12491249

1250+
query MaxPullRequest($owner: String!, $name: String!) {
1251+
repository(owner: $owner, name: $name) {
1252+
issues: pullRequests(first: 1, orderBy: { direction: DESC, field: CREATED_AT }) {
1253+
edges {
1254+
node {
1255+
... on PullRequest {
1256+
number
1257+
}
1258+
}
1259+
}
1260+
}
1261+
}
1262+
rateLimit {
1263+
...RateLimit
1264+
}
1265+
}
1266+
12501267
query GetMilestones($owner: String!, $name: String!, $states: [MilestoneState!]!) {
12511268
repository(owner: $owner, name: $name) {
12521269
milestones(first: 100, orderBy: { direction: DESC, field: DUE_DATE }, states: $states) {

src/view/prsTreeModel.ts

Lines changed: 107 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import * as vscode from 'vscode';
7+
import { RemoteInfo } from '../../common/types';
78
import { Disposable, disposeAll } from '../common/lifecycle';
89
import { getReviewMode } from '../common/settingsUtils';
910
import { ITelemetry } from '../common/telemetry';
@@ -13,7 +14,7 @@ import { PullRequestChangeEvent } from '../github/githubRepository';
1314
import { CheckState, PRType, PullRequestChecks, PullRequestReviewRequirement } from '../github/interface';
1415
import { PullRequestModel } from '../github/pullRequestModel';
1516
import { RepositoriesManager } from '../github/repositoriesManager';
16-
import { UnsatisfiedChecks } from '../github/utils';
17+
import { UnsatisfiedChecks, variableSubstitution } from '../github/utils';
1718
import { CategoryTreeNode } from './treeNodes/categoryNode';
1819
import { TreeNode } from './treeNodes/treeNode';
1920

@@ -24,6 +25,12 @@ interface PRStatusChange {
2425
status: UnsatisfiedChecks;
2526
}
2627

28+
interface CachedPRs {
29+
clearRequested: boolean;
30+
maxKnownPR: number | undefined; // used to determine if there have been new PRs created since last query
31+
items: ItemsResponseResult<PullRequestModel>;
32+
}
33+
2734
export class PrsTreeModel extends Disposable {
2835
private _activePRDisposables: Map<FolderRepositoryManager, vscode.Disposable[]> = new Map();
2936
private readonly _onDidChangePrStatus: vscode.EventEmitter<string[]> = this._register(new vscode.EventEmitter<string[]>());
@@ -38,9 +45,10 @@ export class PrsTreeModel extends Disposable {
3845
// Key is identifier from createPRNodeUri
3946
private readonly _queriedPullRequests: Map<string, PRStatusChange> = new Map();
4047

41-
private _cachedPRs: Map<FolderRepositoryManager, Map<string | PRType.LocalPullRequest | PRType.All, ItemsResponseResult<PullRequestModel>>> = new Map();
48+
private _cachedPRs: Map<FolderRepositoryManager, Map<string | PRType.LocalPullRequest | PRType.All, CachedPRs>> = new Map();
4249
private readonly _repoEvents: Map<FolderRepositoryManager, vscode.Disposable[]> = new Map();
4350
private _getPullRequestsForQueryLock: Promise<void> = Promise.resolve();
51+
private _sentNoRepoTelemetry: boolean = false;
4452

4553
constructor(private _telemetry: ITelemetry, private readonly _reposManager: RepositoriesManager, private readonly _context: vscode.ExtensionContext) {
4654
super();
@@ -144,17 +152,21 @@ export class PrsTreeModel extends Disposable {
144152
if (this._cachedPRs.size === 0) {
145153
return;
146154
}
147-
this._cachedPRs.clear();
155+
156+
// Instead of clearing the entire cache, mark each cached query as requiring refresh.
157+
for (const queries of this._cachedPRs.values()) {
158+
for (const [, cachedPRs] of queries.entries()) {
159+
if (cachedPRs) {
160+
cachedPRs.clearRequested = true;
161+
}
162+
}
163+
}
164+
148165
if (!silent) {
149166
this._onDidChangeData.fire();
150167
}
151168
}
152169

153-
public clearRepo(folderRepoManager: FolderRepositoryManager) {
154-
this._cachedPRs.delete(folderRepoManager);
155-
this._onDidChangeData.fire(folderRepoManager);
156-
}
157-
158170
private async _getChecks(pullRequests: PullRequestModel[]) {
159171
// If there are too many pull requests then we could hit our internal rate limit
160172
// or even GitHub's secondary rate limit. If there are more than 100 PRs,
@@ -207,7 +219,7 @@ export class PrsTreeModel extends Disposable {
207219
this._onDidChangePrStatus.fire(changedStatuses);
208220
}
209221

210-
private getFolderCache(folderRepoManager: FolderRepositoryManager): Map<string | PRType.LocalPullRequest | PRType.All, ItemsResponseResult<PullRequestModel>> {
222+
private getFolderCache(folderRepoManager: FolderRepositoryManager): Map<string | PRType.LocalPullRequest | PRType.All, CachedPRs> {
211223
let cache = this._cachedPRs.get(folderRepoManager);
212224
if (!cache) {
213225
cache = new Map();
@@ -216,17 +228,22 @@ export class PrsTreeModel extends Disposable {
216228
return cache;
217229
}
218230

219-
async getLocalPullRequests(folderRepoManager: FolderRepositoryManager, update?: boolean) {
231+
async getLocalPullRequests(folderRepoManager: FolderRepositoryManager, update?: boolean): Promise<ItemsResponseResult<PullRequestModel>> {
220232
const cache = this.getFolderCache(folderRepoManager);
221233
if (!update && cache.has(PRType.LocalPullRequest)) {
222-
return cache.get(PRType.LocalPullRequest)!;
234+
return cache.get(PRType.LocalPullRequest)!.items;
223235
}
224236

225237
const useReviewConfiguration = getReviewMode();
226238

227239
const prs = (await folderRepoManager.getLocalPullRequests())
228240
.filter(pr => pr.isOpen || (pr.isClosed && useReviewConfiguration.closed) || (pr.isMerged && useReviewConfiguration.merged));
229-
cache.set(PRType.LocalPullRequest, { hasMorePages: false, hasUnsearchedRepositories: false, items: prs, totalCount: prs.length });
241+
const toCache: CachedPRs = {
242+
clearRequested: false,
243+
maxKnownPR: undefined,
244+
items: { hasMorePages: false, hasUnsearchedRepositories: false, items: prs, totalCount: prs.length }
245+
};
246+
cache.set(PRType.LocalPullRequest, toCache);
230247

231248
/* __GDPR__
232249
"pr.expand.local" : {}
@@ -238,6 +255,62 @@ export class PrsTreeModel extends Disposable {
238255
return { hasMorePages: false, hasUnsearchedRepositories: false, items: prs };
239256
}
240257

258+
private async _extractRepoFromQuery(folderManager: FolderRepositoryManager, query: string): Promise<RemoteInfo | undefined> {
259+
if (!query) {
260+
return undefined;
261+
}
262+
263+
const defaults = await folderManager.getPullRequestDefaults();
264+
const substituted = await variableSubstitution(query, undefined, defaults, (await folderManager.getCurrentUser()).login);
265+
266+
const repoRegex = /(?:^|\s)repo:(?:"?(?<owner>[A-Za-z0-9_.-]+)\/(?<repo>[A-Za-z0-9_.-]+)"?)/i;
267+
const repoMatch = repoRegex.exec(substituted);
268+
if (repoMatch && repoMatch.groups) {
269+
return { owner: repoMatch.groups.owner, repositoryName: repoMatch.groups.repo };
270+
}
271+
272+
return undefined;
273+
}
274+
275+
private async _testIfRefreshNeeded(cached: CachedPRs, query: string, folderManager: FolderRepositoryManager): Promise<boolean> {
276+
if (!cached.clearRequested) {
277+
return false;
278+
}
279+
280+
const repoInfo = await this._extractRepoFromQuery(folderManager, query);
281+
if (!repoInfo) {
282+
// Query doesn't specify a repo or org, so always refresh
283+
// Send telemetry once indicating we couldn't find a repo in the query.
284+
if (!this._sentNoRepoTelemetry) {
285+
/* __GDPR__
286+
"pr.expand.noRepo" : {}
287+
*/
288+
this._telemetry.sendTelemetryEvent('pr.expand.noRepo');
289+
this._sentNoRepoTelemetry = true;
290+
}
291+
return true;
292+
}
293+
294+
const currentMax = await this._getMaxKnownPR(repoInfo);
295+
if (currentMax !== cached.maxKnownPR) {
296+
cached.maxKnownPR = currentMax;
297+
return true;
298+
}
299+
return false;
300+
}
301+
302+
private async _getMaxKnownPR(repoInfo: RemoteInfo): Promise<number | undefined> {
303+
const manager = this._reposManager.getManagerForRepository(repoInfo.owner, repoInfo.repositoryName);
304+
if (!manager) {
305+
return;
306+
}
307+
const repo = manager.findExistingGitHubRepository({ owner: repoInfo.owner, repositoryName: repoInfo.repositoryName });
308+
if (!repo) {
309+
return;
310+
}
311+
return repo.getMaxPullRequest();
312+
}
313+
241314
async getPullRequestsForQuery(folderRepoManager: FolderRepositoryManager, fetchNextPage: boolean, query: string): Promise<ItemsResponseResult<PullRequestModel>> {
242315
let release: () => void;
243316
const lock = new Promise<void>(resolve => { release = resolve; });
@@ -246,17 +319,31 @@ export class PrsTreeModel extends Disposable {
246319
await prev;
247320

248321
try {
322+
let maxKnownPR: number | undefined;
249323
const cache = this.getFolderCache(folderRepoManager);
250324
if (!fetchNextPage && cache.has(query)) {
251-
return cache.get(query)!;
325+
const shouldRefresh = await this._testIfRefreshNeeded(cache.get(query)!, query, folderRepoManager);
326+
const cachedPRs = cache.get(query)!;
327+
maxKnownPR = cachedPRs.maxKnownPR;
328+
if (!shouldRefresh) {
329+
cachedPRs.clearRequested = false;
330+
return cachedPRs.items;
331+
}
332+
}
333+
334+
if (!maxKnownPR) {
335+
const repoInfo = await this._extractRepoFromQuery(folderRepoManager, query);
336+
if (repoInfo) {
337+
maxKnownPR = await this._getMaxKnownPR(repoInfo);
338+
}
252339
}
253340

254341
const prs = await folderRepoManager.getPullRequests(
255342
PRType.Query,
256343
{ fetchNextPage },
257344
query,
258345
);
259-
cache.set(query, prs);
346+
cache.set(query, { clearRequested: false, items: prs, maxKnownPR });
260347

261348
/* __GDPR__
262349
"pr.expand.query" : {}
@@ -274,14 +361,14 @@ export class PrsTreeModel extends Disposable {
274361
async getAllPullRequests(folderRepoManager: FolderRepositoryManager, fetchNextPage: boolean, update?: boolean): Promise<ItemsResponseResult<PullRequestModel>> {
275362
const cache = this.getFolderCache(folderRepoManager);
276363
if (!update && cache.has(PRType.All) && !fetchNextPage) {
277-
return cache.get(PRType.All)!;
364+
return cache.get(PRType.All)!.items;
278365
}
279366

280367
const prs = await folderRepoManager.getPullRequests(
281368
PRType.All,
282369
{ fetchNextPage }
283370
);
284-
cache.set(PRType.All, prs);
371+
cache.set(PRType.All, { clearRequested: false, items: prs, maxKnownPR: undefined });
285372

286373
/* __GDPR__
287374
"pr.expand.all" : {}
@@ -299,15 +386,15 @@ export class PrsTreeModel extends Disposable {
299386
return;
300387
}
301388
for (const [, queries] of this._cachedPRs.entries()) {
302-
for (const [queryKey, itemsResult] of queries.entries()) {
303-
if (!itemsResult || !itemsResult.items || itemsResult.items.length === 0) {
389+
for (const [queryKey, cachedPRs] of queries.entries()) {
390+
if (!cachedPRs || !cachedPRs.items.items || cachedPRs.items.items.length === 0) {
304391
continue;
305392
}
306393
const hasPR = withStateChange.some(prChange =>
307-
itemsResult.items.some(item => item === prChange.model)
394+
cachedPRs.items.items.some(item => item === prChange.model)
308395
);
309396
if (hasPR) {
310-
queries.delete(queryKey);
397+
queries.get(queryKey)!.clearRequested = true;
311398
}
312399
}
313400
}

webviews/common/createContextNew.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { createContext } from 'react';
7-
import { ChooseBaseRemoteAndBranchResult, ChooseCompareRemoteAndBranchResult, ChooseRemoteAndBranchArgs, CreateParamsNew, CreatePullRequestNew, RemoteInfo, ScrollPosition, TitleAndDescriptionArgs, TitleAndDescriptionResult } from '../../common/views';
7+
import { RemoteInfo } from '../../common/types';
8+
import { ChooseBaseRemoteAndBranchResult, ChooseCompareRemoteAndBranchResult, ChooseRemoteAndBranchArgs, CreateParamsNew, CreatePullRequestNew, ScrollPosition, TitleAndDescriptionArgs, TitleAndDescriptionResult } from '../../common/views';
89
import { compareIgnoreCase } from '../../src/common/utils';
910
import { PreReviewState } from '../../src/github/views';
1011
import { getMessageHandler, MessageHandler, vscode } from './message';

webviews/createPullRequestViewNew/app.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55

66
import React, { useCallback, useContext, useEffect, useRef, useState } from 'react';
77
import { render } from 'react-dom';
8-
import { CreateParamsNew, RemoteInfo } from '../../common/views';
8+
import { RemoteInfo } from '../../common/types';
9+
import { CreateParamsNew } from '../../common/views';
910
import { isITeam, MergeMethod } from '../../src/github/interface';
1011
import PullRequestContextNew from '../common/createContextNew';
1112
import { ErrorBoundary } from '../common/errorBoundary';

0 commit comments

Comments
 (0)