diff --git a/package.json b/package.json index 82addcb1f1..fe13f1f8ee 100644 --- a/package.json +++ b/package.json @@ -4288,8 +4288,9 @@ "compile:web": "webpack --mode development --config-name extension:webworker --config-name webviews", "lint": "eslint --fix --cache . --ext .ts,.tsx", "package": "npx vsce package", - "test": "npm run test:preprocess && npm run test:scripts && node ./out/src/test/runTests.js", + "test": "npm run test:preprocess && npm run test:scripts && npm run test:webviews && node ./out/src/test/runTests.js", "test:scripts": "mocha \"out/src/test/scripts/**/*.test.js\"", + "test:webviews": "node scripts/test-webviews.js", "test:preprocess": "npm run compile:test && npm run test:preprocess-gql && npm run test:preprocess-svg && npm run test:preprocess-fixtures", "browsertest:preprocess": "tsc ./src/test/browser/runTests.ts --outDir ./dist/browser/test --rootDir ./src/test/browser --target es6 --module commonjs", "browsertest": "npm run browsertest:preprocess && node ./dist/browser/test/runTests.js", diff --git a/scripts/test-webviews.js b/scripts/test-webviews.js new file mode 100644 index 0000000000..1b950f3525 --- /dev/null +++ b/scripts/test-webviews.js @@ -0,0 +1,81 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +const path = require('path'); +const glob = require('glob'); +const Mocha = require('mocha'); +const webpack = require('webpack'); +const installJsDomGlobal = require('jsdom-global'); + +async function main() { + const root = path.resolve(__dirname, '..'); + const tests = glob.sync('webviews/**/test/**/*.test.{ts,tsx}', { cwd: root, absolute: true }).sort(); + if (!tests.length) { + throw new Error('No webview test files found.'); + } + + const configs = await require('../webpack.config')({ esbuild: true }, { mode: 'development' }); + const config = configs.find(config => config.name === 'webviews'); + config.entry = [path.join(root, 'src', 'test', 'webviews', 'setup.ts'), ...tests]; + config.target = 'node'; + config.output = { path: path.join(root, 'out', 'webview-tests'), filename: 'index.js' }; + config.externals = [({ request }, callback) => { + if (!request.startsWith('.') && !path.isAbsolute(request)) { + callback(null, 'commonjs ' + request); + } else { + callback(); + } + }]; + + await new Promise((resolve, reject) => { + const compiler = webpack(config); + compiler.run((error, stats) => compiler.close(closeError => { + if (error || closeError) { + reject(error ?? closeError); + } else if (stats.hasErrors()) { + reject(new Error(stats.toString('errors-warnings'))); + } else { + if (stats.hasWarnings()) { + process.stderr.write(stats.toString('errors-warnings') + '\n'); + } + resolve(); + } + })); + }); + + const mocha = new Mocha({ ui: 'bdd', color: true, failZero: true }); + if (process.env.TEST_JUNIT_XML_PATH) { + const report = process.env.TEST_JUNIT_XML_PATH; + mocha.reporter('mocha-multi-reporters', { + reporterEnabled: 'mocha-junit-reporter, spec', + mochaJunitReporterReporterOptions: { + mochaFile: path.join(path.dirname(report), `webviews-${path.basename(report)}`), + suiteTitleSeparatedBy: ' / ', + outputs: true, + }, + }); + } + + const cleanup = installJsDomGlobal('', { pretendToBeVisual: true }); + // Match JSDOM's APIs rather than exposing Node's MessageChannel to React's scheduler. + const messageChannel = global.MessageChannel; + global.MessageChannel = window.MessageChannel; + try { + require('source-map-support').install(); + mocha.addFile(path.join(config.output.path, config.output.filename)); + const failures = await new Promise(resolve => mocha.run(resolve)); + process.exitCode = failures ? 1 : 0; + } finally { + mocha.dispose(); + window.close(); + cleanup(); + global.MessageChannel = messageChannel; + } +} + +main().catch(error => { + process.stderr.write(`${error.stack ?? error}\n`); + process.exitCode = 1; +}); diff --git a/src/commands.ts b/src/commands.ts index c9a4e82917..259f408fc7 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -565,14 +565,14 @@ export function registerCommands( })); - const resolvePr = async (context: BaseContext | undefined): Promise<{ folderManager: FolderRepositoryManager, pr: PullRequestModel } | undefined> => { + const resolvePr = async (context: BaseContext | undefined, loadMode: 'default' | 'overview' = 'default'): Promise<{ folderManager: FolderRepositoryManager, pr: PullRequestModel } | undefined> => { if (!context) { return undefined; } const folderManager = folderRepositoryManagerResolver.getManagerForRepository(context.owner, context.repo); - const pr = await folderManager.resolvePullRequest(context.owner, context.repo, context.number, true); + const pr = await folderManager.resolvePullRequest(context.owner, context.repo, context.number, true, loadMode); if (!pr) { return undefined; } @@ -1102,7 +1102,7 @@ export function registerCommands( repo: argument.pullRequestDetails.repository.name, number: argument.pullRequestDetails.number, preventDefaultContextMenuItems: true, - }))?.pr; + }, 'overview'))?.pr; } else if (PRChatContextItem.is(argument)) { issueModel = argument.pr; } else if (IssueChatContextItem.is(argument)) { diff --git a/src/github/externalUriOpener.ts b/src/github/externalUriOpener.ts index 41d920c44f..f582b506c3 100644 --- a/src/github/externalUriOpener.ts +++ b/src/github/externalUriOpener.ts @@ -9,8 +9,10 @@ import { IssueOverviewPanel } from './issueOverview'; import { PullRequestOverviewPanel } from './pullRequestOverview'; import { getGitHubIssueOrPullRequestUriOpenerPriority, openWithDefaultExternalOpener, parseGitHubIssueOrPullRequestUri } from '../common/externalUri'; import { Disposable } from '../common/lifecycle'; +import Logger from '../common/logger'; import { OPEN_PULL_LINKS, PR_SETTINGS_NAMESPACE } from '../common/settingKeys'; import { ITelemetry } from '../common/telemetry'; +import { formatError } from '../common/utils'; import { EXTENSION_ID } from '../constants'; class GitHubIssueOrPullRequestExternalUriOpener extends Disposable implements vscode.ExternalUriOpener { @@ -44,21 +46,31 @@ class GitHubIssueOrPullRequestExternalUriOpener extends Disposable implements vs const folderRepositoryManager = this._folderRepositoryManagerResolver.getManagerForRepository(identity.owner, identity.repo); if (identity.kind === 'pullRequest') { - const pullRequest = await folderRepositoryManager.resolvePullRequest(identity.owner, identity.repo, identity.number, true); - if (token.isCancellationRequested) { - return; - } - if (!pullRequest) { - await openWithDefaultExternalOpener(openContext.sourceUri); - return; + const pullRequest = folderRepositoryManager.resolvePullRequest(identity.owner, identity.repo, identity.number, true, 'overview').then(async (pullRequest) => { + if (token.isCancellationRequested) { + throw new vscode.CancellationError(); + } + if (!pullRequest) { + await openWithDefaultExternalOpener(openContext.sourceUri); + throw new vscode.CancellationError(); + } + return pullRequest; + }); + // Start the webview while the first repository and PR requests are in flight. + try { + await PullRequestOverviewPanel.createOrShow( + this._telemetry, + this._context.extensionUri, + folderRepositoryManager, + identity, + pullRequest, + ); + } catch (error) { + if (!(error instanceof vscode.CancellationError)) { + Logger.error(`Failed to open pull request: ${formatError(error)}`, 'GitHubIssueOrPullRequestExternalUriOpener'); + await vscode.window.showErrorMessage(formatError(error)); + } } - await PullRequestOverviewPanel.createOrShow( - this._telemetry, - this._context.extensionUri, - folderRepositoryManager, - identity, - pullRequest, - ); } else { const issue = await folderRepositoryManager.resolveIssue(identity.owner, identity.repo, identity.number, true, true); if (token.isCancellationRequested) { diff --git a/src/github/folderRepositoryManager.ts b/src/github/folderRepositoryManager.ts index 689dbdacd6..a447e2608c 100644 --- a/src/github/folderRepositoryManager.ts +++ b/src/github/folderRepositoryManager.ts @@ -15,6 +15,7 @@ import { CopilotWorkingStatus, GitHubRepository, isRateLimitError, ItemsData, PU import { PullRequestState } from './graphql'; import { IAccount, ILabel, IMilestone, IProject, IPullRequestsPagingOptions, Issue, ITeam, MergeMethod, PRType, PullRequestMergeability, RepoAccessAndMergeMethods, User } from './interface'; import { IssueModel } from './issueModel'; +import { getErrorCode } from './loggingOctokit'; import { PullRequestGitHelper, PullRequestMetadata } from './pullRequestGitHelper'; import { IResolvedPullRequestModel, PullRequestModel } from './pullRequestModel'; import { @@ -30,7 +31,7 @@ import { import type { Branch, Commit, Repository, UpstreamRef } from '../api/api'; import { GitApiImpl, GitErrorCodes } from '../api/api1'; import { GitHubManager } from '../authentication/githubServer'; -import { AuthProvider, GitHubServerType } from '../common/authentication'; +import { AuthProvider, GitHubServerType, isSamlError } from '../common/authentication'; import { commands, contexts } from '../common/executeCommands'; import { InMemFileChange, SlimFileChange } from '../common/file'; import { findLocalRepoRemoteFromGitHubRef } from '../common/githubRef'; @@ -2481,7 +2482,7 @@ export class FolderRepositoryManager extends Disposable { //#region Git related APIs - private async resolveItem(owner: string, repositoryName: string): Promise { + private async resolveItem(owner: string, repositoryName: string, resolveMetadata: boolean = true): Promise { let githubRepo = this._githubRepositories.find(repo => { const ret = repo.remote.owner.toLowerCase() === owner.toLowerCase() && @@ -2492,7 +2493,7 @@ export class FolderRepositoryManager extends Disposable { if (!githubRepo) { Logger.appendLine(`GitHubRepository not found: ${owner}/${repositoryName}`, this.id); // try to create the repository - githubRepo = await this.createGitHubRepositoryFromOwnerName(owner, repositoryName); + githubRepo = await this.createGitHubRepositoryFromOwnerName(owner, repositoryName, resolveMetadata); } return githubRepo; } @@ -2510,11 +2511,23 @@ export class FolderRepositoryManager extends Disposable { repositoryName: string, pullRequestNumber: number, useCache: boolean = false, + loadMode: 'default' | 'overview' = 'default', ): Promise { - const githubRepo = await this.resolveItem(owner, repositoryName); + const githubRepo = await this.resolveItem(owner, repositoryName, loadMode !== 'overview'); Logger.trace(`Found GitHub repo for pr #${pullRequestNumber}: ${githubRepo ? 'yes' : 'no'}`, this.id); if (githubRepo) { - const pr = await githubRepo.getPullRequest(pullRequestNumber, 'FolderRepositoryManager.resolvePullRequest', useCache); + const pullRequestPromise = githubRepo.getPullRequest(pullRequestNumber, 'FolderRepositoryManager.resolvePullRequest', useCache, false, loadMode); + let pr: PullRequestModel | undefined; + if (loadMode === 'overview') { + // Both are needed to render the overview, but neither depends on the other. + const [accessibleRepository, pullRequest] = await Promise.all([ + this.validateGitHubRepositoryAccess(githubRepo), + pullRequestPromise, + ]); + pr = accessibleRepository ? pullRequest : undefined; + } else { + pr = await pullRequestPromise; + } Logger.trace(`Found GitHub pr repo for pr #${pullRequestNumber}: ${pr ? 'yes' : 'no'}`, this.id); return pr; } @@ -3067,7 +3080,7 @@ export class FolderRepositoryManager extends Disposable { }); } - async createGitHubRepositoryFromOwnerName(owner: string, repositoryName: string): Promise { + async createGitHubRepositoryFromOwnerName(owner: string, repositoryName: string, resolveMetadata: boolean = true): Promise { const existing = this.findExistingGitHubRepository({ owner, repositoryName }); if (existing) { return existing; @@ -3080,25 +3093,37 @@ export class FolderRepositoryManager extends Disposable { const gitRemotes = await parseRepositoryRemotesAsync(this.repository); const gitRemote = gitRemotes.find(r => r.owner === owner && r.repositoryName === repositoryName); const uri = gitRemote?.url ?? `https://github.com/${owner}/${repositoryName}`; - const repo = await this.createAndAddGitHubRepository(new Remote(gitRemote?.remoteName ?? repositoryName, uri, new Protocol(uri)), this._credentialStore); + const repo = await this.createGitHubRepository(new Remote(gitRemote?.remoteName ?? repositoryName, uri, new Protocol(uri)), this._credentialStore, undefined, true); + return resolveMetadata ? this.validateGitHubRepositoryAccess(repo) : repo; + } + + private async validateGitHubRepositoryAccess(repo: GitHubRepository): Promise { + const { owner, repositoryName } = repo.remote; let reason: string; try { await repo.getMetadata(); return repo; } catch (e) { + // Only a definitive not-found response should prevent subsequent retries. + if (getErrorCode(e) !== '404' || isSamlError(e)) { + Logger.warn(`Failed to validate repository ${owner}/${repositoryName}: ${formatError(e)}`, this.id); + return undefined; + } reason = 'error'; Logger.appendLine(`Repository ${owner}/${repositoryName} is not accessible: ${e}`, this.id); } Logger.appendLine(`Repository ${owner}/${repositoryName} is not accessible.`, this.id); - this._inaccessibleRepos.add(repoKey); + this._inaccessibleRepos.add(`${owner.toLowerCase()}/${repositoryName.toLowerCase()}`); this.removeGitHubRepository(repo.remote); + const gitRemotes = await parseRepositoryRemotesAsync(this.repository); + const hasLocalRemote = gitRemotes.some(remote => remote.owner === owner && remote.repositoryName === repositoryName); /* __GDPR__ "repository.inaccessible" : { "hasLocalRemote" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" }, "reason" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" } } */ - this.telemetry.sendTelemetryEvent('repository.inaccessible', { hasLocalRemote: (!!gitRemote).toString(), reason }); + this.telemetry.sendTelemetryEvent('repository.inaccessible', { hasLocalRemote: hasLocalRemote.toString(), reason }); return undefined; } diff --git a/src/github/githubRepository.ts b/src/github/githubRepository.ts index 1f9a9ff3d6..b6b9e3f22c 100644 --- a/src/github/githubRepository.ts +++ b/src/github/githubRepository.ts @@ -67,6 +67,7 @@ import { convertRESTPullRequestToRawPullRequest, getAvatarWithEnterpriseFallback, getOverrideBranch, + GraphQLAccount, isInCodespaces, parseAccount, parseGraphQLIssue, @@ -77,6 +78,7 @@ import { parseMilestone, restPaginate, } from './utils'; +import { PullRequestPreview } from './views'; import { StackCandidate } from '../../common/views'; import { AuthenticationError, AuthProvider, GitHubServerType, isSamlError } from '../common/authentication'; @@ -1403,13 +1405,44 @@ export class GitHubRepository extends Disposable { } } - async getPullRequest(id: number, callerName: string, useCache: boolean = false, silent: boolean = false): Promise { + async getPullRequestPreview(number: number): Promise { + if (!Number.isSafeInteger(number) || number <= 0) { + throw new Error(`Invalid pull request number: ${number}`); + } + const { query, remote, schema } = await this.ensure(); + type PreviewData = Omit & { + author: GraphQLAccount | null; + baseRefName: string; + headRefName: string; + baseRepository: { owner: { login: string } }; + headRepository: { owner: { login: string } } | null; + }; + const { data } = await query<{ repository: { pullRequest: PreviewData | null } | null }>({ + query: schema.PullRequestPreview, + variables: { owner: remote.owner, name: remote.repositoryName, number }, + }); + if (!data.repository?.pullRequest) { + throw new Error(`Unable to load pull request preview for ${remote.owner}/${remote.repositoryName}#${number}`); + } + // A preview must never populate the shared cache of actionable PR models. + const { author, baseRefName, headRefName, baseRepository, headRepository, ...preview } = data.repository.pullRequest; + return { + ...preview, + author: parseAccount(author, this), + base: `${baseRepository.owner.login}/${remote.repositoryName}:${baseRefName}`, + head: headRepository ? `${headRepository.owner.login}/${remote.repositoryName}:${headRefName}` : '', + }; + } + + async getPullRequest(id: number, callerName: string, useCache: boolean = false, silent: boolean = false, loadMode: 'default' | 'overview' = 'default'): Promise { if (useCache && this._pullRequestModelsByNumber.has(id)) { Logger.debug(`Using cached pull request model for ${id}`, this.id); return this._pullRequestModelsByNumber.get(id)!.model; } - if (!(await this.isPlausibleItemNumber(id))) { + // Explicit overview requests already identify a PR; the max-number lookup is + // only useful for speculative references extracted from text. + if (!Number.isSafeInteger(id) || id <= 0 || (loadMode === 'default' && !(await this.isPlausibleItemNumber(id)))) { Logger.debug(`Skipping pull request fetch for implausible number ${id} (caller: ${callerName})`, this.id); return; } @@ -1433,7 +1466,9 @@ export class GitHubRepository extends Disposable { Logger.debug(`Fetch pull request ${id} - done`, this.id); const pr = this.createOrUpdatePullRequestModel(await parseGraphQLPullRequest(data.repository.pullRequest, this), silent); - await pr.getLastUpdateTime(new Date(pr.item.updatedAt)); + if (loadMode === 'default') { + await pr.getLastUpdateTime(new Date(pr.item.updatedAt)); + } let repoIds = GitHubRepository._succeededPullRequests.get(id); if (!repoIds) { repoIds = new Set(); diff --git a/src/github/issueOverview.ts b/src/github/issueOverview.ts index cc7b02de03..aa3beb0a71 100644 --- a/src/github/issueOverview.ts +++ b/src/github/issueOverview.ts @@ -11,6 +11,7 @@ import { decodeBase64, guessExtensionFromMime, pickFilesForUpload, placeholdersF import { FolderRepositoryManager } from './folderRepositoryManager'; import { GithubItemStateEnum, IAccount, IMilestone, IProject, IProjectItem, RepoAccessAndMergeMethods } from './interface'; import { IssueModel } from './issueModel'; +import { openIssueOrPullRequestOnGitHub } from './openOnGitHub'; import { getAssigneesQuickPickItems, getLabelOptions, getMilestoneFromQuickPick, getProjectFromQuickPick } from './quickPicks'; import { isInCodespaces, processPermalinks, vscodeDevPrLink } from './utils'; import { ChangeAssigneesReply, DisplayLabel, FileUploadCompletedMessage, Issue, ProjectItemsReply, SubmitReviewArgs, SubmitReviewReply, UnresolvedIdentity, UploadFilesReply, UploadPastedFilesArgs } from './views'; @@ -42,6 +43,7 @@ export class IssueOverviewPanel extends W protected _identity: UnresolvedIdentity; protected _folderRepositoryManager: FolderRepositoryManager; protected _scrollPosition = { x: 0, y: 0 }; + private _identityUpdateSequence = 0; protected static _getViewColumn(toTheSide: boolean, panel?: IssueOverviewPanel): number | undefined { const tabViewColumn = vscode.window.tabGroups.activeTabGroup.viewColumn; @@ -56,7 +58,7 @@ export class IssueOverviewPanel extends W extensionUri: vscode.Uri, folderRepositoryManager: FolderRepositoryManager, identity: UnresolvedIdentity, - issue?: IssueModel, + issue?: IssueModel | Promise, toTheSide: boolean = false, _preserveFocus: boolean = true, existingPanel?: vscode.WebviewPanel @@ -223,7 +225,7 @@ export class IssueOverviewPanel extends W protected onDidChangeViewState(e: vscode.WebviewPanelOnDidChangeViewStateEvent): void { if (e.webviewPanel.visible) { - this.pollForUpdates(!!this._item, true); + this.pollForUpdates(true, true); } } @@ -231,16 +233,31 @@ export class IssueOverviewPanel extends W private lastRefreshTime: Date; private pollForUpdates(isVisible: boolean, refreshImmediately: boolean = false): void { clearTimeout(this.timeout); + if (this.isDisposed) { + return; + } const refresh = async () => { - const previousRefreshTime = this.lastRefreshTime; - this.lastRefreshTime = await this._item.getLastUpdateTime(previousRefreshTime); - if (this.lastRefreshTime.getTime() > previousRefreshTime.getTime()) { - return this.refreshPanel(); + const item = this._item; + if (!item || this.isDisposed) { + return; + } + try { + const previousRefreshTime = this.lastRefreshTime; + const lastRefreshTime = await item.getLastUpdateTime(previousRefreshTime); + if (this.isDisposed || item !== this._item) { + return; + } + this.lastRefreshTime = lastRefreshTime; + if (lastRefreshTime.getTime() > previousRefreshTime.getTime()) { + await this.refreshPanel(); + } + } catch (error) { + Logger.error(`Failed to poll overview updates: ${formatError(error)}`, IssueOverviewPanel.ID); } }; if (refreshImmediately) { - refresh(); + void refresh(); } const webview = isVisible || vscode.window.tabGroups.all.find(group => group.activeTab?.input instanceof vscode.TabInputWebview && group.activeTab.input.viewType.endsWith(this.type)); const timeoutDuration = 1000 * (webview ? this.getRefreshInterval() : (5 * 60)); @@ -377,7 +394,8 @@ export class IssueOverviewPanel extends W * Update the panel with an unresolved identity and optional model. * If no model is provided, it will be resolved from the identity. */ - public async updateWithIdentity(foldersManager: FolderRepositoryManager, identity: UnresolvedIdentity, issueModel?: TItem, progressLocation?: string): Promise { + public async updateWithIdentity(foldersManager: FolderRepositoryManager, identity: UnresolvedIdentity, issueModel?: TItem | Promise, progressLocation?: string): Promise { + const updateSequence = ++this._identityUpdateSequence; this._identity = identity; this._folderRepositoryManager = foldersManager; @@ -393,6 +411,20 @@ export class IssueOverviewPanel extends W } } + if (issueModel instanceof Promise) { + try { + issueModel = await issueModel; + } catch (error) { + if (updateSequence === this._identityUpdateSequence && !this._item) { + this.dispose(); + } + throw error; + } + } + if (this.isDisposed || updateSequence !== this._identityUpdateSequence) { + return; + } + // If no model provided, resolve it from the identity if (!issueModel) { const resolvedModel = await this.resolveModel(identity); @@ -404,6 +436,10 @@ export class IssueOverviewPanel extends W issueModel = resolvedModel; } + if (this.isDisposed || updateSequence !== this._identityUpdateSequence) { + return; + } + if (progressLocation) { return vscode.window.withProgress({ location: { viewId: progressLocation } }, () => this.updateItem(issueModel!)); } else { @@ -462,6 +498,9 @@ export class IssueOverviewPanel extends W case 'pr.copy-vscodedevlink': return this.copyVscodeDevLink(); case 'pr.openOnGitHub': + if (!this._item && typeof message.args?.url === 'string') { + return openIssueOrPullRequestOnGitHub(vscode.Uri.parse(message.args.url), this.type === IssueOverviewPanel.viewType ? 'issue' : 'pullRequest', this._telemetry); + } return openItemOnGitHub(this._item, this._telemetry); case 'pr.open-local-file': return this.openLocalFile(message); diff --git a/src/github/overviewRestorer.ts b/src/github/overviewRestorer.ts index 95ce7416eb..9d31987486 100644 --- a/src/github/overviewRestorer.ts +++ b/src/github/overviewRestorer.ts @@ -43,7 +43,7 @@ export class OverviewRestorer extends Disposable implements vscode.WebviewPanelS } return IssueOverviewPanel.createOrShow(this._telemetry, this._context.extensionUri, folderManager, identity, issueModel, undefined, true, webviewPanel); } else { - const pullRequestModel = await folderManager.resolvePullRequest(state.owner, state.repo, state.number, true); + const pullRequestModel = await folderManager.resolvePullRequest(state.owner, state.repo, state.number, true, 'overview'); if (!pullRequestModel) { webviewPanel.dispose(); return; diff --git a/src/github/pullRequestOverview.ts b/src/github/pullRequestOverview.ts index ce1d39ea88..6b751e1677 100644 --- a/src/github/pullRequestOverview.ts +++ b/src/github/pullRequestOverview.ts @@ -71,6 +71,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel | undefined; private _updateSequence = 0; + private _previewSequence = 0; private _resolveCommentThreadQueue: Promise = Promise.resolve(); public static override async createOrShow( @@ -78,19 +79,12 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel, toTheSide: boolean = false, preserveFocus: boolean = true, existingPanel?: vscode.WebviewPanel ) { - /* __GDPR__ - "pr.openDescription" : { - "isCopilot" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" } - } - */ - telemetry.sendTelemetryEvent('pr.openDescription', { isCopilot: (issue?.author.login === COPILOT_SWE_AGENT) ? 'true' : 'false' }); - const key = panelKey(identity.owner, identity.repo, identity.number); let panel = this._panels.get(key); if (existingPanel && panel && panel._panel !== existingPanel) { @@ -116,6 +110,14 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel { const enterpriseUri = pullRequest.remote.isEnterprise ? pullRequest.githubRepository.hub.serverUri : undefined; @@ -458,7 +456,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel { + if (updateSequence !== this._updateSequence) { + return; + } + this._assignableUsers = assignableUsers; + const users = assignableUsers[pullRequestModel.remote.remoteName] ?? []; + const canAssignCopilot = users.some(user => COPILOT_ACCOUNTS[user.login]); + const reviewers = parseReviewers(requestedReviewers, [...(pullRequestModel.timelineEvents ?? timelineEvents)], pullRequest.author); + const isCopilotAlreadyReviewer = reviewers.some(reviewer => !isITeam(reviewer.reviewer) && reviewer.reviewer.login === COPILOT_REVIEWER); + await this._postMessage({ + command: 'pr.update', + pullrequest: { + canAssignCopilot, + canRequestCopilotReview: canAssignCopilot && !isCopilotAlreadyReviewer, + } satisfies Partial, + }); + Logger.debug(`Deferred assignable users loaded in ${Math.round(performance.now() - assignableUsersStart)}ms`, PullRequestOverviewPanel.ID); + }).catch(error => { + Logger.error(`Failed to update deferred assignable users: ${formatError(error)}`, PullRequestOverviewPanel.ID); + }); let stackLoaded = false; const deferredDataPromise = Promise.all([ measureDeferred('statusChecks', pullRequestModel.getStatusChecks()), @@ -549,8 +570,6 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel { const latestTimelineEvents = [...(pullRequestModel.timelineEvents ?? timelineEvents)]; const reviewers = parseReviewers(requestedReviewers!, latestTimelineEvents, pullRequest.author); - const copilotUser = users.find(user => COPILOT_ACCOUNTS[user.login]); - const isCopilotAlreadyReviewer = reviewers.some(reviewer => !isITeam(reviewer.reviewer) && reviewer.reviewer.login === COPILOT_REVIEWER); const isCopilotOnBehalf = await isCopilotOnMyBehalf(pullRequest, currentUser, coAuthors); if (updateSequence !== this._updateSequence) { return; @@ -570,7 +589,6 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel, }); const deferredTimingSummary = [...deferredTimings] @@ -670,7 +688,9 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel, progressLocation?: string ): Promise { - await super.updateWithIdentity(folderRepositoryManager, identity, pullRequestModel, progressLocation); + const previewSequence = ++this._previewSequence; + let loading = true; + const isLoading = () => loading && !this.isDisposed && previewSequence === this._previewSequence; + const update = super.updateWithIdentity(folderRepositoryManager, identity, pullRequestModel, progressLocation); + if (isLoading() && (!pullRequestModel || pullRequestModel instanceof Promise)) { + void (async () => { + try { + const start = Date.now(); + const repository = await folderRepositoryManager.createGitHubRepositoryFromOwnerName(identity.owner, identity.repo, false); + if (!repository || !isLoading()) { + return; + } + const preview = await repository.getPullRequestPreview(identity.number); + if (isLoading()) { + await this._postMessage({ command: 'pr.preview', pullrequest: preview }); + Logger.debug(`PR overview preview loaded in ${Date.now() - start}ms`, PullRequestOverviewPanel.ID); + } + } catch (error) { + Logger.error(`Unable to load PR overview preview: ${formatError(error)}`, PullRequestOverviewPanel.ID); + } + })(); + } + try { + await update; + } finally { + loading = false; + } // Notify that this PR overview is now active - PullRequestOverviewPanel._onVisible.fire(this._item); + if (!this.isDisposed && this._item) { + PullRequestOverviewPanel._onVisible.fire(this._item); + } } protected override async _onDidReceiveMessage(message: IRequestMessage) { @@ -1447,6 +1495,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel; + export interface PullRequest extends Issue { stack?: PullRequestStack; stackLoaded?: boolean; diff --git a/src/test/github/externalUriOpener.test.ts b/src/test/github/externalUriOpener.test.ts index 10a916c20a..c769541d56 100644 --- a/src/test/github/externalUriOpener.test.ts +++ b/src/test/github/externalUriOpener.test.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { default as assert } from 'assert'; -import { createSandbox, SinonSandbox } from 'sinon'; +import { createSandbox, SinonSandbox, SinonStub } from 'sinon'; import * as vscode from 'vscode'; import { RemoteOnlyRepository } from '../../api/remoteOnlyRepository'; import { CredentialStore } from '../../github/credentials'; @@ -12,6 +12,8 @@ import { registerGitHubIssueOrPullRequestExternalUriOpener } from '../../github/ import { FolderRepositoryManager } from '../../github/folderRepositoryManager'; import { FolderRepositoryManagerResolver } from '../../github/folderRepositoryManagerResolver'; import { RepositoriesManager } from '../../github/repositoriesManager'; +import { PullRequestModel } from '../../github/pullRequestModel'; +import { PullRequestOverviewPanel } from '../../github/pullRequestOverview'; import { MockExtensionContext } from '../mocks/mockExtensionContext'; import { MockTelemetry } from '../mocks/mockTelemetry'; @@ -113,4 +115,108 @@ describe('GitHubIssueOrPullRequestExternalUriOpener', () => { context.dispose(); } }); + + describe('opening pull requests', () => { + const uri = vscode.Uri.parse('https://github.com/aaa/bbb/pull/1000'); + let context: MockExtensionContext; + let opener: vscode.ExternalUriOpener; + let cancellation: vscode.CancellationTokenSource; + let resolvePullRequest: (pr: PullRequestModel | undefined) => void; + let rejectPullRequest: (error: Error) => void; + let resolvePullRequestStub: SinonStub, ReturnType>; + let openExternal: SinonStub, ReturnType>; + + beforeEach(() => { + context = new MockExtensionContext(); + const telemetry = new MockTelemetry(); + const credentialStore = new CredentialStore(telemetry, context); + const repositoriesManager = new RepositoriesManager(credentialStore, telemetry); + const resolver = new FolderRepositoryManagerResolver(context, repositoriesManager, telemetry); + cancellation = new vscode.CancellationTokenSource(); + context.subscriptions.push(credentialStore, repositoriesManager, resolver, cancellation); + sandbox.stub(vscode.window, 'registerExternalUriOpener').callsFake((_id, value) => { + opener = value; + return new vscode.Disposable(() => undefined); + }); + context.subscriptions.push(registerGitHubIssueOrPullRequestExternalUriOpener(context, resolver, telemetry)); + sandbox.stub(opener as any, 'isOpenPullLinksEnabled').returns(true); + const pendingPullRequest = new Promise((resolve, reject) => { + resolvePullRequest = resolve; + rejectPullRequest = reject; + }); + resolvePullRequestStub = sandbox.stub(FolderRepositoryManager.prototype, 'resolvePullRequest').returns(pendingPullRequest); + openExternal = sandbox.stub(vscode.env, 'openExternal').resolves(true); + }); + + afterEach(() => { + PullRequestOverviewPanel.findPanel('aaa', 'bbb', 1000)?.dispose(); + context.dispose(); + }); + + it('creates the first tab and loads its HTML before resolving the PR', async () => { + const createWebviewPanel = sandbox.spy(vscode.window, 'createWebviewPanel'); + const showError = sandbox.stub(vscode.window, 'showErrorMessage').resolves(undefined); + const opening = opener.openExternalUri(uri, { sourceUri: uri }, cancellation.token); + try { + assert.strictEqual(createWebviewPanel.callCount, 1); + assert.ok(createWebviewPanel.firstCall.returnValue.webview.html.includes('webview-pr-description.js')); + assert.ok(PullRequestOverviewPanel.findPanel('aaa', 'bbb', 1000)); + sandbox.assert.calledOnce(resolvePullRequestStub); + sandbox.assert.calledWithExactly(resolvePullRequestStub, 'aaa', 'bbb', 1000, true, 'overview'); + } finally { + resolvePullRequest(undefined); + await opening; + } + sandbox.assert.calledOnce(resolvePullRequestStub); + sandbox.assert.calledOnce(openExternal); + sandbox.assert.calledWithExactly(openExternal, uri, { allowContributedOpeners: 'default' }); + sandbox.assert.notCalled(showError); + assert.strictEqual(PullRequestOverviewPanel.findPanel('aaa', 'bbb', 1000), undefined); + }); + + it('does not create a tab for an already-cancelled request', async () => { + const createWebviewPanel = sandbox.spy(vscode.window, 'createWebviewPanel'); + cancellation.cancel(); + await opener.openExternalUri(uri, { sourceUri: uri }, cancellation.token); + + sandbox.assert.notCalled(createWebviewPanel); + sandbox.assert.notCalled(resolvePullRequestStub); + sandbox.assert.notCalled(openExternal); + }); + + it('closes the new tab without an error when cancelled during resolution', async () => { + const showError = sandbox.stub(vscode.window, 'showErrorMessage').resolves(undefined); + const opening = opener.openExternalUri(uri, { sourceUri: uri }, cancellation.token); + assert.ok(PullRequestOverviewPanel.findPanel('aaa', 'bbb', 1000)); + cancellation.cancel(); + resolvePullRequest(undefined); + await opening; + + assert.strictEqual(PullRequestOverviewPanel.findPanel('aaa', 'bbb', 1000), undefined); + sandbox.assert.notCalled(showError); + sandbox.assert.notCalled(openExternal); + }); + + it('reports a browser fallback failure and closes the new tab', async () => { + openExternal.rejects(new Error('Browser unavailable')); + const showError = sandbox.stub(vscode.window, 'showErrorMessage').resolves(undefined); + const opening = opener.openExternalUri(uri, { sourceUri: uri }, cancellation.token); + resolvePullRequest(undefined); + await opening; + + sandbox.assert.calledOnce(resolvePullRequestStub); + assert.strictEqual(showError.firstCall.args[0], 'Browser unavailable'); + assert.strictEqual(PullRequestOverviewPanel.findPanel('aaa', 'bbb', 1000), undefined); + }); + + it('reports resolution failures and closes the new tab', async () => { + const showError = sandbox.stub(vscode.window, 'showErrorMessage').resolves(undefined); + const opening = opener.openExternalUri(uri, { sourceUri: uri }, cancellation.token); + rejectPullRequest(new Error('PR lookup failed')); + await opening; + + assert.strictEqual(showError.firstCall.args[0], 'PR lookup failed'); + assert.strictEqual(PullRequestOverviewPanel.findPanel('aaa', 'bbb', 1000), undefined); + }); + }); }); diff --git a/src/test/github/folderRepositoryManager.test.ts b/src/test/github/folderRepositoryManager.test.ts index da8909a26f..1620912b49 100644 --- a/src/test/github/folderRepositoryManager.test.ts +++ b/src/test/github/folderRepositoryManager.test.ts @@ -29,6 +29,8 @@ import { PullRequestReviewCommon, ReviewContext } from '../../github/pullRequest import { IRequestMessage } from '../../common/webview'; import { PullRequestMergeability } from '../../github/interface'; import { PullRequest } from '../../github/views'; +import { RepositoryBuilder } from '../builders/rest/repoBuilder'; +import { UserBuilder } from '../builders/rest/userBuilder'; describe('PullRequestManager', function () { let sinon: SinonSandbox; @@ -54,6 +56,93 @@ describe('PullRequestManager', function () { sinon.restore(); }); + describe('overview resolution', function () { + const metadata = { ...new RepositoryBuilder().build(), currentUser: new UserBuilder().build() }; + + beforeEach(function () { + sinon.stub(GitHubRepository.prototype, 'ensure').callsFake(async function (this: GitHubRepository) { + return this; + }); + }); + + afterEach(function () { + for (const repo of manager.gitHubRepositories) { + repo.dispose(); + } + manager.dispose(); + if (manager.context instanceof MockExtensionContext) { + manager.context.dispose(); + } + }); + + it('fetches the PR concurrently with cold repository metadata', async function () { + let resolveMetadata: (value: typeof metadata) => void; + const pendingMetadata = new Promise(resolve => resolveMetadata = resolve); + const getMetadata = sinon.stub(GitHubRepository.prototype, 'getMetadata').returns(pendingMetadata); + const getPullRequest = sinon.stub(GitHubRepository.prototype, 'getPullRequest').callsFake(async function (this: GitHubRepository) { + const item = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1347).build(), this); + return new PullRequestModel(manager.credentialStore, telemetry, this, this.remote, item); + }); + const updates = sinon.stub(PullRequestModel.prototype, 'getLastUpdateTime').resolves(new Date()); + const opening = manager.resolvePullRequest('owner', 'repo', 1347, false, 'overview'); + try { + await new Promise(resolve => setImmediate(resolve)); + sinon.assert.calledOnce(getMetadata); + sinon.assert.calledOnce(getPullRequest); + sinon.assert.calledWithExactly(getPullRequest, 1347, 'FolderRepositoryManager.resolvePullRequest', false, false, 'overview'); + } finally { + resolveMetadata!(metadata); + } + const pr = await opening; + assert.strictEqual(pr?.number, 1347); + sinon.assert.notCalled(updates); + }); + + it('shares repository creation between concurrent preview and full loads', async function () { + const [previewRepository, fullRepository] = await Promise.all([ + manager.createGitHubRepositoryFromOwnerName('owner', 'repo', false), + manager.createGitHubRepositoryFromOwnerName('owner', 'repo', false), + ]); + + assert.ok(previewRepository); + assert.strictEqual(previewRepository, fullRepository); + assert.deepStrictEqual(manager.gitHubRepositories, [previewRepository]); + }); + + it('still rejects and remembers inaccessible repositories', async function () { + const getMetadata = sinon.stub(GitHubRepository.prototype, 'getMetadata').rejects(Object.assign(new Error('Not Found'), { status: 404 })); + sinon.stub(GitHubRepository.prototype, 'getPullRequest').resolves(undefined); + + assert.strictEqual(await manager.resolvePullRequest('owner', 'repo', 1347, false, 'overview'), undefined); + assert.strictEqual(manager.gitHubRepositories.length, 0); + assert.strictEqual(await manager.resolvePullRequest('owner', 'repo', 1347, false, 'overview'), undefined); + sinon.assert.calledOnce(getMetadata); + }); + + for (const [name, error] of [ + ['network timeout', new Error('Temporary network timeout')], + ['server error', Object.assign(new Error('Service unavailable'), { status: 503 })], + ['rate limit', Object.assign(new Error('Rate limited'), { status: 429 })], + ['SAML authorization', Object.assign(new Error('Resource protected by organization SAML enforcement.'), { status: 404 })], + ] as const) { + it(`retries metadata after a ${name} failure without removing the repository`, async function () { + const repo = await manager.createGitHubRepositoryFromOwnerName('owner', 'repo', false); + assert.ok(repo); + const item = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1347).build(), repo); + const pr = new PullRequestModel(manager.credentialStore, telemetry, repo, repo.remote, item); + sinon.stub(repo, 'getPullRequest').resolves(pr); + const getMetadata = sinon.stub(repo, 'getMetadata'); + getMetadata.onFirstCall().rejects(error); + getMetadata.onSecondCall().resolves(metadata); + + assert.strictEqual(await manager.resolvePullRequest('owner', 'repo', 1347, false, 'overview'), undefined); + assert.deepStrictEqual(manager.gitHubRepositories, [repo]); + assert.strictEqual(await manager.resolvePullRequest('owner', 'repo', 1347, false, 'overview'), pr); + sinon.assert.calledTwice(getMetadata); + }); + } + }); + describe('updateRepositories', function () { it('skips a repository after a 404 without affecting healthy repositories', async function () { const inaccessibleUrl = 'https://github.com/owner/missing'; diff --git a/src/test/github/githubRepository.test.ts b/src/test/github/githubRepository.test.ts index 080c1def1d..93df1050c4 100644 --- a/src/test/github/githubRepository.test.ts +++ b/src/test/github/githubRepository.test.ts @@ -5,7 +5,7 @@ import { default as assert } from 'assert'; import { NetworkStatus } from 'apollo-boost'; -import { SinonSandbox, createSandbox } from 'sinon'; +import { SinonSandbox, createSandbox, match } from 'sinon'; import { CredentialStore } from '../../github/credentials'; import { MockCommandRegistry } from '../mocks/mockCommandRegistry'; import { MockTelemetry } from '../mocks/mockTelemetry'; @@ -16,10 +16,14 @@ import { Uri } from 'vscode'; import { MockExtensionContext } from '../mocks/mockExtensionContext'; import { GitHubManager } from '../../authentication/githubServer'; import { GitHubServerType } from '../../common/authentication'; -import { CheckState, PullRequestCheckStatus } from '../../github/interface'; +import { CheckState, GithubItemStateEnum, PullRequestCheckStatus } from '../../github/interface'; import { PullRequestBuilder as GraphQLPullRequestBuilder } from '../builders/graphql/pullRequestBuilder'; import Logger from '../../common/logger'; import { LoggingApolloClient, LoggingOctokit } from '../../github/loggingOctokit'; +import { MockGitHubRepository } from '../mocks/mockGitHubRepository'; +import { PullRequestModel } from '../../github/pullRequestModel'; +import { visit } from 'graphql'; +import { parseAccount } from '../../github/utils'; describe('GitHubRepository', function () { let sinon: SinonSandbox; @@ -95,6 +99,127 @@ describe('GitHubRepository', function () { }); }); + describe('getPullRequest', function () { + let repo: MockGitHubRepository; + + beforeEach(function () { + const url = 'https://github.com/owner/repo'; + const remote = new GitHubRemote('origin', url, new Protocol(url), GitHubServerType.GitHubDotCom); + repo = new MockGitHubRepository(remote, credentialStore, telemetry, sinon); + }); + + afterEach(function () { + repo.dispose(); + }); + + it('loads a read-only preview without populating the PR model cache', async function () { + const preview = { + number: 1347, title: 'Preview', titleHTML: 'Preview', + body: 'Description', bodyHTML: '

Description

', url: 'https://github.com/owner/repo/pull/1347', + state: GithubItemStateEnum.Open, isDraft: true, createdAt: '2026-10-01T10:00:00Z', + author: { __typename: 'User', id: 'author', login: 'contributor', url: 'https://github.com/contributor', avatarUrl: '' }, + baseRefName: 'main', headRefName: 'feature', + baseRepository: { owner: { login: 'owner' } }, headRepository: { owner: { login: 'contributor' } }, + }; + const query = sinon.stub(repo, 'query').resolves({ + data: { repository: { pullRequest: preview } }, + loading: false, stale: false, networkStatus: NetworkStatus.ready, + }); + + const { author, baseRefName, headRefName, baseRepository, headRepository, ...content } = preview; + assert.deepStrictEqual(await repo.getPullRequestPreview(1347), { + ...content, + author: parseAccount(author, repo), + base: 'owner/repo:main', + head: 'contributor/repo:feature', + }); + assert.strictEqual(repo.getExistingPullRequestModel(1347), undefined); + sinon.assert.calledOnce(query); + assert.strictEqual(query.firstCall.args[0].query, repo.schema.PullRequestPreview); + assert.deepStrictEqual(query.firstCall.args[0].variables, { owner: 'owner', name: 'repo', number: 1347 }); + const fields: string[] = []; + visit(repo.schema.PullRequestPreview, { Field(node) { fields.push(node.name.value); } }); + assert.ok(fields.includes('titleHTML') && fields.includes('bodyHTML')); + for (const field of ['commits', 'suggestedReviewers', 'mergeable', 'mergeStateStatus', 'reactionGroups', 'reviewThreads']) { + assert.ok(!fields.includes(field), `Preview must not query ${field}`); + } + assert.ok(!fields.includes('email'), 'Preview must not require additional user scopes'); + + query.resolves({ + data: { repository: { pullRequest: { ...preview, author: null, headRepository: null } } }, + loading: false, stale: false, networkStatus: NetworkStatus.ready, + }); + const deletedAuthorPreview = await repo.getPullRequestPreview(1347); + assert.deepStrictEqual(deletedAuthorPreview.author, parseAccount(null, repo)); + assert.strictEqual(deletedAuthorPreview.head, ''); + }); + + it('rejects missing previews and invalid preview numbers', async function () { + const query = sinon.stub(repo, 'query').resolves({ + data: { repository: { pullRequest: null } }, + loading: false, stale: false, networkStatus: NetworkStatus.ready, + }); + for (const number of [0, -1, NaN, Infinity, 1.5]) { + await assert.rejects(repo.getPullRequestPreview(number), /Invalid pull request number/); + } + sinon.assert.notCalled(query); + await assert.rejects(repo.getPullRequestPreview(1347), /Unable to load pull request preview/); + assert.strictEqual(repo.getExistingPullRequestModel(1347), undefined); + }); + + it('loads an overview with only the PR query and reuses its cached model', async function () { + const data = new GraphQLPullRequestBuilder().build(); + const query = sinon.stub(repo, 'query').resolves({ data, loading: false, stale: false, networkStatus: NetworkStatus.ready }); + const updates = sinon.stub(PullRequestModel.prototype, 'getLastUpdateTime').resolves(new Date()); + + const pr = await repo.getPullRequest(1347, 'test', false, false, 'overview'); + + assert.ok(pr); + assert.strictEqual(pr.title, data.repository!.pullRequest.title); + assert.strictEqual(pr.bodyHTML, data.repository!.pullRequest.bodyHTML); + sinon.assert.calledOnce(query); + assert.strictEqual(query.firstCall.args[0].query, repo.schema.PullRequest); + sinon.assert.notCalled(updates); + assert.strictEqual(await repo.getPullRequest(1347, 'test', true, false, 'overview'), pr); + sinon.assert.calledOnce(query); + }); + + it('preserves number validation and update checks for default loads', async function () { + const query = sinon.stub(repo, 'query').resolves({ + data: new GraphQLPullRequestBuilder().build(), + loading: false, stale: false, networkStatus: NetworkStatus.ready, + }); + const maxItemResult = { + data: { repository: { issues: { edges: [{ node: { number: 1347 } }] } } }, + loading: false, stale: false, networkStatus: NetworkStatus.ready, + }; + query.withArgs(match.has('query', repo.schema.MaxIssue)).resolves(maxItemResult); + query.withArgs(match.has('query', repo.schema.MaxPullRequest)).resolves(maxItemResult); + const updates = sinon.stub(PullRequestModel.prototype, 'getLastUpdateTime').resolves(new Date()); + + assert.ok(await repo.getPullRequest(1347, 'test')); + + assert.strictEqual(query.callCount, 3); + sinon.assert.calledOnce(updates); + }); + + it('rejects invalid overview numbers without a network request', async function () { + const query = sinon.spy(repo, 'query'); + for (const number of [0, -1, NaN, Infinity, 1.5]) { + assert.strictEqual(await repo.getPullRequest(number, 'test', false, false, 'overview'), undefined); + } + sinon.assert.notCalled(query); + }); + + it('logs a failed overview fetch instead of returning a partial model', async function () { + sinon.stub(repo, 'query').rejects(new Error('PR unavailable')); + const logError = sinon.spy(Logger, 'error'); + + assert.strictEqual(await repo.getPullRequest(1347, 'test', false, false, 'overview'), undefined); + assert.strictEqual(logError.firstCall.args[0], 'Unable to fetch PR: Error: PR unavailable'); + }); + }); + describe('isGitHubDotCom', function () { it('detects when the remote is pointing to github.com', function () { const url = 'https://github.com/some/repo'; diff --git a/src/test/github/pullRequestOverview.test.ts b/src/test/github/pullRequestOverview.test.ts index 33c4d28903..7212301f7c 100644 --- a/src/test/github/pullRequestOverview.test.ts +++ b/src/test/github/pullRequestOverview.test.ts @@ -5,7 +5,7 @@ import { default as assert } from 'assert'; import * as vscode from 'vscode'; -import { SinonSandbox, createSandbox, match as sinonMatch } from 'sinon'; +import { SinonSandbox, SinonStub, createSandbox, match as sinonMatch } from 'sinon'; import { FolderRepositoryManager } from '../../github/folderRepositoryManager'; import { MockTelemetry } from '../mocks/mockTelemetry'; @@ -23,12 +23,15 @@ import { GitApiImpl } from '../../api/api1'; import { CredentialStore } from '../../github/credentials'; import { GitHubServerType } from '../../common/authentication'; import { GitHubRemote } from '../../common/remote'; -import { CheckState, GithubItemStateEnum, PullRequestMergeability, PullRequestStack } from '../../github/interface'; +import { CheckState, GithubItemStateEnum, IAccount, PullRequestMergeability, PullRequestStack } from '../../github/interface'; import { CreatePullRequestHelper } from '../../view/createPullRequestHelper'; import { RepositoriesManager } from '../../github/repositoriesManager'; import { MockThemeWatcher } from '../mocks/mockThemeWatcher'; import { TimelineEvent } from '../../common/timelineEvent'; import { PullRequestReviewCommon, ReviewContext } from '../../github/pullRequestReviewCommon'; +import { COPILOT_REVIEWER_ACCOUNT } from '../../common/copilot'; +import Logger from '../../common/logger'; +import { PullRequest, PullRequestPreview } from '../../github/views'; const EXTENSION_URI = vscode.Uri.joinPath(vscode.Uri.file(__dirname), '../../..'); @@ -316,6 +319,386 @@ describe('PullRequestOverview', function () { }); }); + describe('deferred assignable users', function () { + let prModel: PullRequestModel; + let webviewPanel: vscode.WebviewPanel; + let messages: { command: string; pullrequest?: Partial }[]; + let onDidReceiveMessage: vscode.EventEmitter<{ command: string; args?: { url: string } }>; + let onDidChangeViewState: vscode.EventEmitter; + let pollInterval: number; + let resolveUsers: (users: { [key: string]: IAccount[] }) => void; + let rejectUsers: (error: Error) => void; + let usersPromise: Promise<{ [key: string]: IAccount[] }>; + let getAssignableUsers: SinonStub, ReturnType>; + let getReviewRequests: SinonStub<[], ReturnType>; + let getPreview: SinonStub<[number], Promise>; + const preview: PullRequestPreview = { + number: 1000, title: 'Preview title', titleHTML: 'Preview title', + body: 'Preview description', bodyHTML: '

Preview description

', url: 'https://github.com/aaa/bbb/pull/1000', + author: COPILOT_REVIEWER_ACCOUNT, createdAt: '2026-10-01T10:00:00Z', + state: GithubItemStateEnum.Open, isDraft: false, base: 'aaa/bbb:main', head: 'aaa/bbb:feature', + }; + + beforeEach(function () { + const prItem = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).build(), repo); + prModel = new PullRequestModel(credentialStore, telemetry, repo, remote, prItem); + sinon.stub(pullRequestManager, 'createGitHubRepositoryFromOwnerName').resolves(repo); + getPreview = sinon.stub(repo, 'getPullRequestPreview').resolves(preview); + sinon.stub(pullRequestManager, 'getCurrentUser').resolves(prModel.author); + sinon.stub(prModel, 'canEdit').resolves(true); + getReviewRequests = sinon.stub(prModel, 'getReviewRequests').resolves([]); + sinon.stub(prModel, 'getTimelineEvents').resolves([]); + sinon.stub(prModel, 'validateDraftMode').resolves(false); + sinon.stub(prModel, 'getStatusChecks').resolves([{ state: CheckState.Success, statuses: [] }, null]); + sinon.stub(prModel, 'getMergeability').resolves({ mergeability: PullRequestMergeability.Mergeable }); + sinon.stub(pullRequestManager, 'getBranchNameForPullRequest').resolves(undefined); + sinon.stub(pullRequestManager, 'mergeQueueMethodForBranch').resolves(undefined); + sinon.stub(pullRequestManager, 'isHeadUpToDateWithBase').resolves(true); + sinon.stub(pullRequestManager, 'getPreferredEmail').resolves(undefined); + sinon.stub(pullRequestManager, 'checkBranchUpToDate').resolves(); + usersPromise = new Promise((resolve, reject) => { + resolveUsers = resolve; + rejectUsers = reject; + }); + getAssignableUsers = sinon.stub(pullRequestManager, 'getAssignableUsers').returns(usersPromise); + + messages = []; + webviewPanel = vscode.window.createWebviewPanel(PullRequestOverviewPanel.viewType, '#1000', vscode.ViewColumn.One, {}); + onDidReceiveMessage = new vscode.EventEmitter(); + onDidChangeViewState = new vscode.EventEmitter(); + pollInterval = 1000 * (vscode.workspace.getConfiguration().get('githubPullRequests.webviewRefreshInterval') || 60); + context.subscriptions.push(webviewPanel, onDidReceiveMessage, onDidChangeViewState); + sinon.stub(webviewPanel.webview, 'onDidReceiveMessage').callsFake(onDidReceiveMessage.event); + sinon.stub(webviewPanel, 'onDidChangeViewState').callsFake(onDidChangeViewState.event); + sinon.stub(webviewPanel.webview, 'postMessage').callsFake(async message => { + messages.push(message.res); + return true; + }); + }); + + async function openPanel(model: PullRequestModel | Promise = prModel): Promise { + const identity = { owner: remote.owner, repo: remote.repositoryName, number: prModel.number }; + const opening = PullRequestOverviewPanel.createOrShow(telemetry, EXTENSION_URI, pullRequestManager, identity, model, false, true, webviewPanel); + onDidReceiveMessage.fire({ command: 'ready' }); + await opening; + await new Promise(resolve => setImmediate(resolve)); + } + + afterEach(async function () { + resolveUsers({}); + await new Promise(resolve => setImmediate(resolve)); + }); + + it('shows the title and description while the full PR is still pending', async function () { + let resolveModel: (model: PullRequestModel) => void; + const opening = openPanel(new Promise(resolve => resolveModel = resolve)); + try { + await new Promise(resolve => setImmediate(resolve)); + assert.deepStrictEqual(messages.find(message => message.command === 'pr.preview')?.pullrequest, preview); + assert.strictEqual(messages.some(message => message.command === 'pr.initialize'), false); + sinon.assert.notCalled(getAssignableUsers); + } finally { + resolveModel!(prModel); + await opening; + } + assert.ok(messages.some(message => message.command === 'pr.initialize')); + }); + + it('does not fetch a preview for an already available PR model', async function () { + await openPanel(); + sinon.assert.notCalled(getPreview); + }); + + for (const previewHasStarted of [false, true]) { + it(`shows a preview during slow initialization when the model resolves ${previewHasStarted ? 'after' : 'before'} the preview query starts`, async function () { + let resolveModel: ((model: PullRequestModel) => void) | undefined; + let resolvePreview: (value: PullRequestPreview) => void; + let resolveDefaultBranch: (branch: string) => void; + const getDefaultBranch = sinon.stub(pullRequestManager, 'getPullRequestRepositoryDefaultBranch') + .returns(new Promise(resolve => resolveDefaultBranch = resolve)); + getPreview.returns(new Promise(resolve => resolvePreview = resolve)); + const model = previewHasStarted + ? new Promise(resolve => resolveModel = resolve) + : Promise.resolve(prModel); + const opening = openPanel(model); + try { + if (previewHasStarted) { + await new Promise(resolve => setImmediate(resolve)); + sinon.assert.calledOnce(getPreview); + resolveModel!(prModel); + } + await new Promise(resolve => setImmediate(resolve)); + sinon.assert.calledOnce(getDefaultBranch); + assert.strictEqual(messages.some(message => message.command === 'pr.initialize'), false); + + resolvePreview!(preview); + await new Promise(resolve => setImmediate(resolve)); + assert.deepStrictEqual(messages.find(message => message.command === 'pr.preview')?.pullrequest, preview); + assert.strictEqual(messages.some(message => message.command === 'pr.initialize'), false); + } finally { + resolveModel?.(prModel); + resolvePreview!(preview); + resolveDefaultBranch!('main'); + await opening; + } + assert.ok(messages.some(message => message.command === 'pr.initialize')); + }); + } + + it('skips polling a pending model and resumes once the PR is available', async function () { + const clock = sinon.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + const getLastUpdateTime = sinon.stub(prModel, 'getLastUpdateTime').resolves(new Date(0)); + let resolveModel: (model: PullRequestModel) => void; + const opening = openPanel(new Promise(resolve => resolveModel = resolve)); + try { + onDidChangeViewState.fire({ webviewPanel }); + clock.tick(pollInterval); + await new Promise(resolve => setImmediate(resolve)); + sinon.assert.notCalled(getLastUpdateTime); + assert.strictEqual(messages.some(message => message.command === 'pr.initialize'), false); + } finally { + resolveModel!(prModel); + await opening; + } + clock.tick(pollInterval); + await new Promise(resolve => setImmediate(resolve)); + sinon.assert.calledOnce(getLastUpdateTime); + }); + + it('logs poll failures and continues polling on the next interval', async function () { + const clock = sinon.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + const getLastUpdateTime = sinon.stub(prModel, 'getLastUpdateTime'); + getLastUpdateTime.onFirstCall().rejects(new Error('Temporary polling failure')); + getLastUpdateTime.onSecondCall().resolves(new Date(0)); + const logError = sinon.spy(Logger, 'error'); + await openPanel(); + + clock.tick(pollInterval); + await new Promise(resolve => setImmediate(resolve)); + assert.ok(logError.getCalls().some(call => call.args[0] === 'Failed to poll overview updates: Temporary polling failure')); + clock.tick(pollInterval); + await new Promise(resolve => setImmediate(resolve)); + sinon.assert.calledTwice(getLastUpdateTime); + }); + + it('does not refresh or restart polling after an in-flight poll is disposed', async function () { + const clock = sinon.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + let resolvePoll: (date: Date) => void; + const getLastUpdateTime = sinon.stub(prModel, 'getLastUpdateTime').returns(new Promise(resolve => resolvePoll = resolve)); + await openPanel(); + const panel = PullRequestOverviewPanel.findPanel(remote.owner, remote.repositoryName, prModel.number)!; + const refresh = sinon.stub(panel, 'refreshPanel').resolves(); + + clock.tick(pollInterval); + webviewPanel.dispose(); + resolvePoll!(new Date(Date.now() + 1000)); + await new Promise(resolve => setImmediate(resolve)); + clock.tick(pollInterval); + await new Promise(resolve => setImmediate(resolve)); + + sinon.assert.notCalled(refresh); + sinon.assert.calledOnce(getLastUpdateTime); + }); + + it('opens a preview link with the default browser before the full PR resolves', async function () { + const openExternal = sinon.stub(vscode.env, 'openExternal').resolves(true); + let resolveModel: (model: PullRequestModel) => void; + const opening = openPanel(new Promise(resolve => resolveModel = resolve)); + try { + await new Promise(resolve => setImmediate(resolve)); + onDidReceiveMessage.fire({ command: 'pr.openOnGitHub', args: { url: preview.url } }); + await new Promise(resolve => setImmediate(resolve)); + + sinon.assert.calledOnce(openExternal); + assert.strictEqual(openExternal.firstCall.args[0].toString(), preview.url); + assert.deepStrictEqual(openExternal.firstCall.args[1], { allowContributedOpeners: 'default' }); + assert.strictEqual(messages.some(message => message.command === 'pr.initialize'), false); + } finally { + resolveModel!(prModel); + await opening; + } + }); + + it('ignores a preview from an older lookup while a newer lookup is pending', async function () { + let resolvePreview: (value: PullRequestPreview) => void; + let resolveFirst: (value: PullRequestModel) => void; + let resolveSecond: (value: PullRequestModel) => void; + getPreview.onFirstCall().returns(new Promise(resolve => resolvePreview = resolve)); + const first = openPanel(new Promise(resolve => resolveFirst = resolve)); + await new Promise(resolve => setImmediate(resolve)); + const second = openPanel(new Promise(resolve => resolveSecond = resolve)); + try { + await new Promise(resolve => setImmediate(resolve)); + messages.length = 0; + resolvePreview!({ ...preview, title: 'Outdated preview' }); + await new Promise(resolve => setImmediate(resolve)); + assert.strictEqual(messages.some(message => message.command === 'pr.preview'), false); + } finally { + resolveFirst!(prModel); + resolveSecond!(prModel); + await Promise.all([first, second]); + } + }); + + it('does not let a late preview replace the complete PR', async function () { + let resolvePreview: (value: PullRequestPreview) => void; + let resolveModel: (value: PullRequestModel) => void; + getPreview.returns(new Promise(resolve => resolvePreview = resolve)); + const opening = openPanel(new Promise(resolve => resolveModel = resolve)); + await new Promise(resolve => setImmediate(resolve)); + resolveModel!(prModel); + await opening; + resolvePreview!(preview); + await new Promise(resolve => setImmediate(resolve)); + + assert.strictEqual(messages.some(message => message.command === 'pr.preview'), false); + assert.ok(messages.some(message => message.command === 'pr.initialize')); + }); + + it('ignores a preview that finishes after the panel is closed', async function () { + let resolvePreview: (value: PullRequestPreview) => void; + let resolveModel: (value: PullRequestModel) => void; + getPreview.returns(new Promise(resolve => resolvePreview = resolve)); + const opening = openPanel(new Promise(resolve => resolveModel = resolve)); + await new Promise(resolve => setImmediate(resolve)); + webviewPanel.dispose(); + resolvePreview!(preview); + resolveModel!(prModel); + await opening; + + assert.strictEqual(messages.some(message => message.command === 'pr.preview'), false); + }); + + it('logs a preview failure without preventing full PR initialization', async function () { + let resolveModel: (value: PullRequestModel) => void; + getPreview.rejects(new Error('Preview unavailable')); + const logError = sinon.spy(Logger, 'error'); + const opening = openPanel(new Promise(resolve => resolveModel = resolve)); + try { + await new Promise(resolve => setImmediate(resolve)); + assert.ok(logError.getCalls().some(call => typeof call.args[0] === 'string' && call.args[0].includes('Preview unavailable'))); + } finally { + resolveModel!(prModel); + await opening; + } + assert.ok(messages.some(message => message.command === 'pr.initialize')); + }); + + it('loads the webview before a supplied PR model resolves', async function () { + let resolveModel: (model: PullRequestModel) => void; + const pendingModel = new Promise(resolve => resolveModel = resolve); + const opening = openPanel(pendingModel); + try { + assert.ok(webviewPanel.webview.html.includes('webview-pr-description.js')); + assert.strictEqual(messages.some(message => message.command === 'pr.initialize'), false); + } finally { + resolveModel!(prModel); + await opening; + } + assert.strictEqual(messages.find(message => message.command === 'pr.initialize')?.pullrequest?.title, prModel.title); + }); + + it('does not initialize a closed panel when its PR model resolves', async function () { + let resolveModel: (model: PullRequestModel) => void; + const pendingModel = new Promise(resolve => resolveModel = resolve); + const opening = openPanel(pendingModel); + webviewPanel.dispose(); + resolveModel!(prModel); + await opening; + + assert.strictEqual(messages.some(message => message.command === 'pr.initialize'), false); + sinon.assert.notCalled(getAssignableUsers); + }); + + it('does not overwrite a newer model when an older lookup finishes', async function () { + let resolveModel: (model: PullRequestModel) => void; + const pendingModel = new Promise(resolve => resolveModel = resolve); + const opening = openPanel(pendingModel); + await openPanel(); + messages.length = 0; + resolveModel!(prModel); + await opening; + + assert.strictEqual(messages.some(message => message.command === 'pr.initialize'), false); + sinon.assert.calledOnce(getAssignableUsers); + }); + + it('initializes the PR and loads checks before cold assignable users finish', async function () { + const opening = openPanel(); + await new Promise(resolve => setImmediate(resolve)); + try { + const initial = messages.find(message => message.command === 'pr.initialize')?.pullrequest; + assert.ok(initial, 'PR initialization must not wait for assignable users'); + assert.strictEqual(initial.title, prModel.title); + assert.strictEqual(initial.body, prModel.body); + assert.strictEqual(initial.canAssignCopilot, false); + assert.strictEqual(initial.canRequestCopilotReview, false); + assert.ok(messages.some(message => message.command === 'pr.update' && message.pullrequest?.status?.state === CheckState.Success)); + } finally { + resolveUsers({ [remote.remoteName]: [COPILOT_REVIEWER_ACCOUNT] }); + await opening; + await new Promise(resolve => setImmediate(resolve)); + } + + assert.ok(messages.some(message => message.command === 'pr.update' + && message.pullrequest?.canAssignCopilot === true + && message.pullrequest.canRequestCopilotReview === true)); + }); + + it('keeps Copilot actions unavailable when no assignable Copilot account exists', async function () { + resolveUsers({ [remote.remoteName]: [prModel.author] }); + await openPanel(); + + assert.ok(messages.some(message => message.command === 'pr.update' + && message.pullrequest?.canAssignCopilot === false + && message.pullrequest.canRequestCopilotReview === false)); + }); + + it('does not offer another Copilot review when one is already requested', async function () { + getReviewRequests.resolves([COPILOT_REVIEWER_ACCOUNT]); + resolveUsers({ [remote.remoteName]: [COPILOT_REVIEWER_ACCOUNT] }); + await openPanel(); + + assert.ok(messages.some(message => message.command === 'pr.update' + && message.pullrequest?.canAssignCopilot === true + && message.pullrequest.canRequestCopilotReview === false)); + }); + + it('logs assignable-user failures without preventing PR initialization', async function () { + const logError = sinon.spy(Logger, 'error'); + await openPanel(); + rejectUsers(new Error('Assignable users unavailable')); + await new Promise(resolve => setImmediate(resolve)); + + assert.ok(messages.some(message => message.command === 'pr.initialize')); + sinon.assert.calledWith(logError, 'Failed to update deferred assignable users: Assignable users unavailable', PullRequestOverviewPanel.ID); + assert.strictEqual(messages.some(message => message.pullrequest?.canAssignCopilot === true), false); + }); + + it('ignores assignable users from an older update', async function () { + await openPanel(); + getAssignableUsers.resolves({}); + await openPanel(); + messages.length = 0; + + resolveUsers({ [remote.remoteName]: [COPILOT_REVIEWER_ACCOUNT] }); + await new Promise(resolve => setImmediate(resolve)); + + assert.deepStrictEqual(messages, []); + }); + + it('ignores assignable users after the panel is disposed', async function () { + await openPanel(); + webviewPanel.dispose(); + messages.length = 0; + + resolveUsers({ [remote.remoteName]: [COPILOT_REVIEWER_ACCOUNT] }); + await new Promise(resolve => setImmediate(resolve)); + + assert.deepStrictEqual(messages, []); + }); + }); + describe('mergePullRequest', function () { it('prompts to delete the local branch when GitHub deletes branches after merge', async function () { repo.buildMetadata(repository => repository.delete_branch_on_merge!(true)); diff --git a/src/test/mocks/mockWebviewEnvironment.ts b/src/test/mocks/mockWebviewEnvironment.ts index e3c19af551..14df7eef48 100644 --- a/src/test/mocks/mockWebviewEnvironment.ts +++ b/src/test/mocks/mockWebviewEnvironment.ts @@ -1,4 +1,8 @@ -import installJsDomGlobal from 'jsdom-global'; +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + import { Suite } from 'mocha'; interface WebviewEnvironmentSetters { @@ -59,6 +63,11 @@ class MockWebviewEnvironment { this._uninstall(); } + reset() { + this._persistedState = undefined; + this._messages.length = 0; + } + /** * Install before and after hooks to configure a Mocha test suite to use this Webview environment. * diff --git a/src/test/webviews/setup.ts b/src/test/webviews/setup.ts new file mode 100644 index 0000000000..4cddd3b1ee --- /dev/null +++ b/src/test/webviews/setup.ts @@ -0,0 +1,54 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { after, afterEach, beforeEach } from 'mocha'; +import { cleanup } from 'react-testing-library'; +import { SinonSpy, spy } from 'sinon'; +import { mockWebviewEnvironment } from '../mocks/mockWebviewEnvironment'; + +mockWebviewEnvironment.install(globalThis); +let listeners: SinonSpy; +beforeEach(() => { + mockWebviewEnvironment.reset(); + listeners = spy(window, 'addEventListener'); +}); +afterEach(() => { + cleanup(); + for (const call of listeners.getCalls()) { + window.removeEventListener(call.args[0], call.args[1], call.args[2]); + } + listeners.restore(); + window.onscroll = null; +}); +after(() => mockWebviewEnvironment.uninstall()); + +window.scrollTo = () => { }; +window.matchMedia = media => ({ + media, + matches: false, + onchange: null, + addListener() { }, + removeListener() { }, + addEventListener() { }, + removeEventListener() { }, + dispatchEvent() { return true; }, +}); + +// JSDOM has no layout engine; tests can replace these observers to simulate layout changes. +globalThis.ResizeObserver = class implements ResizeObserver { + observe() { } + unobserve() { } + disconnect() { } +}; + +globalThis.IntersectionObserver = class implements IntersectionObserver { + readonly root = null; + readonly rootMargin = '0px'; + readonly thresholds: readonly number[] = []; + observe() { } + unobserve() { } + disconnect() { } + takeRecords(): IntersectionObserverEntry[] { return []; } +}; diff --git a/tsconfig.webviews.json b/tsconfig.webviews.json index c806a45e06..68fbe07bdb 100644 --- a/tsconfig.webviews.json +++ b/tsconfig.webviews.json @@ -13,6 +13,7 @@ "src/@types", "src/common", "src/github", + "src/test/webviews", "webviews" ] } diff --git a/webviews/common/context.tsx b/webviews/common/context.tsx index 6b41bc635c..420a05da7a 100644 --- a/webviews/common/context.tsx +++ b/webviews/common/context.tsx @@ -11,7 +11,7 @@ import { CloseResult, DescriptionResult, OpenCommitChangesArgs, OpenLocalFileArg import { IComment } from '../../src/common/comment'; import { EventType, ReviewEvent, SessionLinkInfo, TimelineEvent } from '../../src/common/timelineEvent'; import { IProjectItem, MergeMethod, PullRequestCheckStatus, ReadyForReview } from '../../src/github/interface'; -import { CancelCodingAgentReply, ChangeAssigneesReply, ChangeBaseReply, ConvertToDraftReply, DeleteReviewResult, FileUploadCompletedMessage, MergeArguments, MergeResult, ProjectItemsReply, PullRequest, ReadyForReviewReply, StackMergeResult, SubmitReviewArgs, SubmitReviewReply, UploadFilesReply } from '../../src/github/views'; +import { CancelCodingAgentReply, ChangeAssigneesReply, ChangeBaseReply, ConvertToDraftReply, DeleteReviewResult, FileUploadCompletedMessage, MergeArguments, MergeResult, ProjectItemsReply, PullRequest, PullRequestPreview, ReadyForReviewReply, StackMergeResult, SubmitReviewArgs, SubmitReviewReply, UploadFilesReply } from '../../src/github/views'; /** * Encode a {@linkcode Uint8Array} as a base64 string. Uses fixed-size chunks to @@ -32,6 +32,9 @@ function bytesToBase64(bytes: Uint8Array): string { const MAX_UPLOAD_SIZE_BYTES = 25 * 1024 * 1024; export class PRContext { + public preview: PullRequestPreview | undefined; + public onPreviewChange: ((preview: PullRequestPreview | undefined) => void) | null = null; + constructor( public pr: PullRequest | undefined = getState(), public onchange: ((ctx: PullRequest | undefined) => void) | null = null, @@ -101,7 +104,10 @@ export class PRContext { public mergeStack = (method: MergeMethod): Promise => this.postMessage({ command: 'pr.merge-stack', args: { method } }); - public openOnGitHub = () => this.postMessage({ command: 'pr.openOnGitHub' }); + public openOnGitHub = () => this.postMessage({ + command: 'pr.openOnGitHub', + args: this.preview ? { url: this.preview.url } : undefined, + }); public deleteBranch = async () => { const result = await this.postMessage({ command: 'pr.deleteBranch' }); @@ -534,6 +540,8 @@ export class PRContext { }; setPR = (pr: PullRequest | undefined) => { + this.preview = undefined; + this.onPreviewChange?.(undefined); this.pr = pr; setState(this.pr); if (this.onchange) { @@ -557,6 +565,12 @@ export class PRContext { handleMessage = (message: any) => { switch (message.command) { + case 'pr.preview': + if (!this.pr) { + this.preview = message.pullrequest; + this.onPreviewChange?.(this.preview); + } + return; case 'pr.clear': this.setPR(undefined); return; diff --git a/webviews/components/comment.tsx b/webviews/components/comment.tsx index adde992549..30242de31d 100644 --- a/webviews/components/comment.tsx +++ b/webviews/components/comment.tsx @@ -12,7 +12,7 @@ import { AuthorLink, Avatar } from './user'; import { IComment } from '../../src/common/comment'; import { CommentEvent, EventType, ReviewEvent } from '../../src/common/timelineEvent'; import { GithubItemStateEnum } from '../../src/github/interface'; -import { PullRequest, ReviewCommentContext, ReviewType } from '../../src/github/views'; +import { PullRequest, PullRequestPreview, ReviewCommentContext, ReviewType } from '../../src/github/views'; import { ariaAnnouncementForReview } from '../common/aria'; import { COMMENT_TEXTAREA_ID } from '../common/constants'; import PullRequestContext from '../common/context'; @@ -182,8 +182,14 @@ export function CommentView(commentProps: Props) { ); } +export const CommentPreview = (preview: PullRequestPreview) => ( + + + +); + type CommentBoxProps = { - for: IComment | ReviewEvent | PullRequest | CommentEvent; + for: IComment | ReviewEvent | PullRequest | CommentEvent | PullRequestPreview; header?: React.ReactChild; onFocus?: React.FocusEventHandler; onMouseEnter?: React.MouseEventHandler; @@ -191,7 +197,7 @@ type CommentBoxProps = { children?: React.ReactNode; }; -function isReviewEvent(comment: IComment | ReviewEvent | PullRequest | CommentEvent): comment is ReviewEvent { +function isReviewEvent(comment: CommentBoxProps['for']): comment is ReviewEvent { return (comment as ReviewEvent).authorAssociation !== undefined; } diff --git a/webviews/components/header.tsx b/webviews/components/header.tsx index a29a308dbb..9c6cb89aee 100644 --- a/webviews/components/header.tsx +++ b/webviews/components/header.tsx @@ -11,7 +11,7 @@ import { AuthorLink, Avatar } from './user'; import { copilotEventToStatus, CopilotPRStatus, mostRecentCopilotEvent } from '../../src/common/copilot'; import { CopilotStartedEvent, TimelineEvent } from '../../src/common/timelineEvent'; import { GithubItemStateEnum, PullRequestStack, StateReason } from '../../src/github/interface'; -import { BaseContext, CodingAgentContext, OverviewContext, PullRequest } from '../../src/github/views'; +import { BaseContext, CodingAgentContext, OverviewContext, PullRequest, PullRequestPreview } from '../../src/github/views'; import { EDIT_TITLE_BUTTON_ID } from '../common/constants'; import PullRequestContext from '../common/context'; import { useStateProp } from '../common/hooks'; @@ -75,6 +75,44 @@ export function Header({ ); } +export function HeaderPreview(preview: PullRequestPreview) { + const { openOnGitHub } = useContext(PullRequestContext); + return <> +
+ +
+ +
+ + Loading... +
+ ; +} + +function TitleText({ titleHTML, number, url, context, onOpen }: Pick & { + context?: BaseContext; + onOpen?: () => void; +}) { + return

+ + {' '} + { + event.preventDefault(); + onOpen(); + } : undefined} + > + #{number} + +

; +} + interface TitleProps { title: string; titleHTML: string; @@ -130,21 +168,7 @@ function Title({ title, titleHTML, number, url, inEditMode, setEditMode, setCurr const displayTitle = (
-

- - {' '} - { - event.preventDefault(); - void openOnGitHub(); - }} - > - #{number} - -

+ {canEdit ?