diff --git a/src/github/externalUriOpener.ts b/src/github/externalUriOpener.ts index 8fa78bb09c..ac83493958 100644 --- a/src/github/externalUriOpener.ts +++ b/src/github/externalUriOpener.ts @@ -45,7 +45,6 @@ class GitHubIssueOrPullRequestExternalUriOpener extends Disposable implements vs return; } - const folderRepositoryManager = this._folderRepositoryManagerResolver.getManagerForRepository(identity.owner, identity.repo); const requireModel = async (model: T | undefined): Promise => { if (token.isCancellationRequested) { throw new vscode.CancellationError(); @@ -58,6 +57,7 @@ class GitHubIssueOrPullRequestExternalUriOpener extends Disposable implements vs }; try { + const folderRepositoryManager = this._folderRepositoryManagerResolver.getManagerForRepository(identity.owner, identity.repo); if (identity.kind === 'pullRequest') { const pullRequest = folderRepositoryManager.resolvePullRequest(identity.owner, identity.repo, identity.number, true, 'overview').then(requireModel); await PullRequestOverviewPanel.createOrShow( diff --git a/src/github/folderRepositoryManagerResolver.ts b/src/github/folderRepositoryManagerResolver.ts index a9cf80e313..09867f827c 100644 --- a/src/github/folderRepositoryManagerResolver.ts +++ b/src/github/folderRepositoryManagerResolver.ts @@ -30,6 +30,10 @@ export class FolderRepositoryManagerResolver extends Disposable { if (existingManager) { return existingManager; } + return this.getRemoteOnlyManager(); + } + + getRemoteOnlyManager(): FolderRepositoryManager { if (this._remoteFolderRepositoryManager) { return this._remoteFolderRepositoryManager; } diff --git a/src/github/issueOverview.ts b/src/github/issueOverview.ts index cb8c25f209..c89f11e143 100644 --- a/src/github/issueOverview.ts +++ b/src/github/issueOverview.ts @@ -6,6 +6,7 @@ import * as vscode from 'vscode'; import { CloseResult, OpenLocalFileArgs } from '../../common/views'; +import { RemoteOnlyRepository } from '../api/remoteOnlyRepository'; import { openItemOnGitHub } from '../commands'; import { decodeBase64, guessExtensionFromMime, pickFilesForUpload, placeholdersForNames, runFileUploads, runPendingUploads } from './fileUpload'; import { FolderRepositoryManager } from './folderRepositoryManager'; @@ -14,6 +15,7 @@ import { GithubItemStateEnum, IAccount, IMilestone, IProject, IProjectItem, Repo import { IssueModel } from './issueModel'; import { openIssueOrPullRequestOnGitHub } from './openOnGitHub'; import { getAssigneesQuickPickItems, getLabelOptions, getMilestoneFromQuickPick, getProjectFromQuickPick } from './quickPicks'; +import type { RepositoriesManager } from './repositoriesManager'; import { isInCodespaces, processPermalinks, vscodeDevPrLink } from './utils'; import { ChangeAssigneesReply, DisplayLabel, FileUploadCompletedMessage, Issue, IssuePreview, OverviewItemPreview, ProjectItemsReply, SubmitReviewArgs, SubmitReviewReply, UnresolvedIdentity, UploadFilesReply, UploadPastedFilesArgs } from './views'; import { COPILOT_ACCOUNTS, IComment } from '../common/comment'; @@ -36,17 +38,49 @@ export class IssueOverviewPanel extends W * All open panels, keyed by "owner/repo#number". */ protected static _panels: Map = new Map(); + protected static _repositoriesManager: RepositoriesManager | undefined; public static readonly viewType: string = 'IssueOverview'; protected readonly _panel: vscode.WebviewPanel; protected _item: TItem; protected _identity: UnresolvedIdentity; - protected _folderRepositoryManager: FolderRepositoryManager; + protected readonly _initialFolderRepositoryManager: FolderRepositoryManager; + protected _localFolderRepositoryManager: FolderRepositoryManager | undefined; protected _scrollPosition = { x: 0, y: 0 }; private _identityUpdateSequence = 0; protected readonly previewLog = { label: 'Issue', id: IssueOverviewPanel.ID }; + protected get _folderRepositoryManager(): FolderRepositoryManager { + if (this._localFolderRepositoryManager) { + return this._localFolderRepositoryManager; + } + const manager = this._initialFolderRepositoryManager; + if (!this.isDisposed && manager.repository instanceof RemoteOnlyRepository && this._identity) { + const localManager = IssueOverviewPanel._repositoriesManager?.getManagerForRepository(this._identity.owner, this._identity.repo); + if (localManager && !(localManager.repository instanceof RemoteOnlyRepository)) { + this._localFolderRepositoryManager = localManager; + this.registerPrListeners(); + Logger.debug(`Upgraded remote-only manager for ${this._identity.owner}/${this._identity.repo} to ${localManager.repository.rootUri.toString()}`, this.previewLog.id); + return localManager; + } + } + return manager; + } + + protected static registerRepositoriesManager(context: vscode.ExtensionContext, repositoriesManager: RepositoriesManager): void { + IssueOverviewPanel._repositoriesManager = repositoriesManager; + context.subscriptions.push( + { + dispose: () => { + if (IssueOverviewPanel._repositoriesManager === repositoriesManager) { + IssueOverviewPanel._repositoriesManager = undefined; + } + } + }, + ); + } + protected static _getViewColumn(toTheSide: boolean, panel?: IssueOverviewPanel): number | undefined { const tabViewColumn = vscode.window.tabGroups.activeTabGroup.viewColumn; const activeColumn = toTheSide @@ -90,7 +124,7 @@ export class IssueOverviewPanel extends W this._panels.set(key, panel); } - await panel.updateWithIdentity(folderRepositoryManager, identity, issue); + await panel.updateWithIdentity(identity, issue); } public static refresh(owner: string, repo: string, number: number): void { @@ -167,7 +201,7 @@ export class IssueOverviewPanel extends W } ) { super(); - this._folderRepositoryManager = folderRepositoryManager; + this._initialFolderRepositoryManager = folderRepositoryManager; // Create and show a new webview panel this._panel = existingPanel ?? this._register(vscode.window.createWebviewPanel(type, title, column, { @@ -192,17 +226,6 @@ export class IssueOverviewPanel extends W // This happens when the user closes the panel or when the panel is closed programmatically this._register(this._panel.onDidDispose(() => this.dispose())); - this._register(this._folderRepositoryManager.onDidChangeActiveIssue( - _ => { - if (this._folderRepositoryManager && this._item) { - const isCurrentlyCheckedOut = this._item.equals(this._folderRepositoryManager.activeIssue); - this._postMessage({ - command: 'pr.update-checkout-status', - isCurrentlyCheckedOut: isCurrentlyCheckedOut, - }); - } - })); - this._register(folderRepositoryManager.credentialStore.onDidUpgradeSession(() => { this.updateItem(this._item); })); @@ -418,13 +441,13 @@ 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. + * Repository manager selection is handled by the panel, not by updates. */ - public async updateWithIdentity(foldersManager: FolderRepositoryManager, identity: UnresolvedIdentity, issueModel?: TItem | Promise, progressLocation?: string): Promise { + public async updateWithIdentity(identity: UnresolvedIdentity, issueModel?: TItem | Promise, progressLocation?: string): Promise { const updateSequence = ++this._identityUpdateSequence; let loading = true; const isLoading = () => loading && !this.isDisposed && updateSequence === this._identityUpdateSequence; this._identity = identity; - this._folderRepositoryManager = foldersManager; this._postMessage({ command: 'set-scroll', @@ -442,7 +465,7 @@ export class IssueOverviewPanel extends W void (async () => { try { const start = Date.now(); - const repository = await foldersManager.createGitHubRepositoryFromOwnerName(identity.owner, identity.repo, false); + const repository = await this._folderRepositoryManager.createGitHubRepositoryFromOwnerName(identity.owner, identity.repo, false); if (!isLoading()) { return; } diff --git a/src/github/overviewRestorer.ts b/src/github/overviewRestorer.ts index 9d31987486..59daeacc47 100644 --- a/src/github/overviewRestorer.ts +++ b/src/github/overviewRestorer.ts @@ -33,7 +33,7 @@ export class OverviewRestorer extends Disposable implements vscode.WebviewPanelS await this.waitForAuth(); - const folderManager = this._folderRepositoryManagerResolver.getManagerForRepository(state.owner, state.repo); + const folderManager = this._folderRepositoryManagerResolver.getRemoteOnlyManager(); const identity = { owner: state.owner, repo: state.repo, number: state.number }; if (state.isIssue) { const issueModel = await folderManager.resolveIssue(state.owner, state.repo, state.number, true, true); diff --git a/src/github/pullRequestOverview.ts b/src/github/pullRequestOverview.ts index 008b618360..3053ef070d 100644 --- a/src/github/pullRequestOverview.ts +++ b/src/github/pullRequestOverview.ts @@ -63,7 +63,6 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel = new Map(); - private static _repositoriesManager: RepositoriesManager | undefined; private static readonly _updatingStacks = new Set(); /** @@ -129,10 +128,9 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel { for (const panel of this._panels.values()) { - if (panel._folderRepositoryManager !== manager) { + if ((panel._localFolderRepositoryManager ?? panel._initialFolderRepositoryManager) !== manager) { panel.postCheckoutStatus(); } } }), - { dispose: () => { this._repositoriesManager = undefined; } }, vscode.commands.registerCommand('pr.readyForReviewDescription', async (ctx: ReadyForReviewContext) => { const panel = PullRequestOverviewPanel.findPanel(ctx.owner, ctx.repo, ctx.number); if (panel) { @@ -343,7 +340,9 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel this.postCheckoutStatus())); + // Avoid re-entering the upgrading accessor while rebinding its listeners. + const manager = this._localFolderRepositoryManager ?? this._initialFolderRepositoryManager; + this._prListeners.push(manager.onDidChangeActivePullRequest(() => this.postCheckoutStatus())); if (this._item) { const repository = this._item.githubRepository; @@ -594,7 +593,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel, progressLocation?: string ): Promise { - await super.updateWithIdentity(folderRepositoryManager, identity, pullRequestModel, progressLocation); + await super.updateWithIdentity(identity, pullRequestModel, progressLocation); // Notify that this PR overview is now active if (!this.isDisposed && this._item) { diff --git a/src/test/github/folderRepositoryManagerResolver.test.ts b/src/test/github/folderRepositoryManagerResolver.test.ts new file mode 100644 index 0000000000..f74b95325d --- /dev/null +++ b/src/test/github/folderRepositoryManagerResolver.test.ts @@ -0,0 +1,97 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { default as assert } from 'assert'; +import { createSandbox, SinonSandbox } from 'sinon'; +import { Uri } from 'vscode'; +import { GitHubServerType } from '../../common/authentication'; +import { Protocol } from '../../common/protocol'; +import { GitHubRemote } from '../../common/remote'; +import { GitApiImpl } from '../../api/api1'; +import { RemoteOnlyRepository } from '../../api/remoteOnlyRepository'; +import { CredentialStore } from '../../github/credentials'; +import { FolderRepositoryManager } from '../../github/folderRepositoryManager'; +import { FolderRepositoryManagerResolver } from '../../github/folderRepositoryManagerResolver'; +import { RepositoriesManager } from '../../github/repositoriesManager'; +import { CreatePullRequestHelper } from '../../view/createPullRequestHelper'; +import { MockExtensionContext } from '../mocks/mockExtensionContext'; +import { MockRepository } from '../mocks/mockRepository'; +import { MockGitHubRepository } from '../mocks/mockGitHubRepository'; +import { MockTelemetry } from '../mocks/mockTelemetry'; +import { MockThemeWatcher } from '../mocks/mockThemeWatcher'; + +describe('FolderRepositoryManagerResolver', function () { + let context: MockExtensionContext; + let telemetry: MockTelemetry; + let credentialStore: CredentialStore; + let repositoriesManager: RepositoriesManager; + let sandbox: SinonSandbox; + + beforeEach(function () { + sandbox = createSandbox(); + context = new MockExtensionContext(); + telemetry = new MockTelemetry(); + credentialStore = new CredentialStore(telemetry, context); + repositoriesManager = new RepositoriesManager(credentialStore, telemetry); + }); + + afterEach(function () { + repositoriesManager.dispose(); + credentialStore.dispose(); + context.dispose(); + sandbox.restore(); + }); + + function createResolver(): FolderRepositoryManagerResolver { + const resolver = new FolderRepositoryManagerResolver(context, repositoriesManager, telemetry); + context.subscriptions.push(resolver); + return resolver; + } + + async function addLocalManager(url: string): Promise { + const repository = new MockRepository(); + repository.rootUri = Uri.file('/workspace'); + await repository.addRemote('origin', url); + const git = new GitApiImpl(repositoriesManager); + const helper = new CreatePullRequestHelper(); + context.subscriptions.push(git, helper); + const manager = new FolderRepositoryManager(0, context, repository, telemetry, git, credentialStore, helper, new MockThemeWatcher()); + const remote = new GitHubRemote('origin', url, new Protocol(url), GitHubServerType.GitHubDotCom); + const repo = new MockGitHubRepository(remote, credentialStore, telemetry, sandbox); + context.subscriptions.push(repo); + sandbox.stub(manager, 'gitHubRepositories').get(() => [repo]); + repositoriesManager.insertFolderManager(manager); + return manager; + } + + for (const url of ['https://github.com/Owner/Repo.git', 'git@github.com:Owner/Repo.git']) { + it(`uses the discovered local manager for ${url}`, async function () { + const manager = await addLocalManager(url); + const resolver = createResolver(); + + assert.strictEqual(resolver.getManagerForRepository('owner', 'repo'), manager); + }); + } + + it('reuses the remote-only manager only when no local repository matches', async function () { + const resolver = createResolver(); + const remoteManager = resolver.getManagerForRepository('owner', 'repo'); + assert.ok(remoteManager.repository instanceof RemoteOnlyRepository); + assert.strictEqual(resolver.getManagerForRepository('other', 'repo'), remoteManager); + const localManager = await addLocalManager('https://github.com/owner/repo.git'); + + assert.strictEqual(resolver.getManagerForRepository('owner', 'repo'), localManager); + assert.strictEqual(resolver.getManagerForRepository('other', 'repo'), remoteManager); + }); + + it('can supply a temporary remote-only manager even when a local repository is already discovered', async function () { + const localManager = await addLocalManager('https://github.com/owner/repo.git'); + const resolver = createResolver(); + + assert.strictEqual(resolver.getManagerForRepository('owner', 'repo'), localManager); + assert.ok(resolver.getRemoteOnlyManager().repository instanceof RemoteOnlyRepository); + assert.strictEqual(resolver.getRemoteOnlyManager(), resolver.getRemoteOnlyManager()); + }); +}); diff --git a/src/test/github/issueOverview.test.ts b/src/test/github/issueOverview.test.ts new file mode 100644 index 0000000000..2fcacfe0c0 --- /dev/null +++ b/src/test/github/issueOverview.test.ts @@ -0,0 +1,171 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { default as assert } from 'assert'; +import { createSandbox, SinonSandbox, SinonStub } from 'sinon'; +import * as vscode from 'vscode'; +import { GitApiImpl } from '../../api/api1'; +import { GitHubServerType } from '../../common/authentication'; +import { Protocol } from '../../common/protocol'; +import { GitHubRemote } from '../../common/remote'; +import { EXTENSION_ID } from '../../constants'; +import { CredentialStore } from '../../github/credentials'; +import { FolderRepositoryManager } from '../../github/folderRepositoryManager'; +import { FolderRepositoryManagerResolver } from '../../github/folderRepositoryManagerResolver'; +import { IssueModel } from '../../github/issueModel'; +import { IssueOverviewPanel, panelKey } from '../../github/issueOverview'; +import { RepositoriesManager } from '../../github/repositoriesManager'; +import { convertRESTPullRequestToRawPullRequest } from '../../github/utils'; +import { UnresolvedIdentity } from '../../github/views'; +import { CreatePullRequestHelper } from '../../view/createPullRequestHelper'; +import { PullRequestBuilder } from '../builders/rest/pullRequestBuilder'; +import { MockExtensionContext } from '../mocks/mockExtensionContext'; +import { MockGitHubRepository } from '../mocks/mockGitHubRepository'; +import { MockRepository } from '../mocks/mockRepository'; +import { MockTelemetry } from '../mocks/mockTelemetry'; +import { MockThemeWatcher } from '../mocks/mockThemeWatcher'; + +class TestIssueOverviewPanel extends IssueOverviewPanel { + public static override registerRepositoriesManager(context: vscode.ExtensionContext, repositoriesManager: RepositoriesManager): void { + super.registerRepositoriesManager(context, repositoriesManager); + } + + constructor(telemetry: MockTelemetry, manager: FolderRepositoryManager) { + super(telemetry, manager.context.extensionUri, vscode.ViewColumn.One, '#1000', manager); + } + + public setItem(item: IssueModel): void { + this._item = item; + this._identity = { owner: item.remote.owner, repo: item.remote.repositoryName, number: item.number }; + TestIssueOverviewPanel._panels.set(panelKey(item.remote.owner, item.remote.repositoryName, item.number), this); + } + + public get folderRepositoryManager(): FolderRepositoryManager { + return this._folderRepositoryManager; + } + + public override resolveModel(identity: UnresolvedIdentity): Promise { + return super.resolveModel(identity); + } + + public override _postMessage(message: { command: string; isCurrentlyCheckedOut?: boolean }): Promise { + return super._postMessage(message); + } +} + +describe('IssueOverview remote-only manager upgrades', function () { + let sandbox: SinonSandbox; + let context: MockExtensionContext; + let repositoriesManager: RepositoriesManager; + let localManager: FolderRepositoryManager; + let temporaryManager: FolderRepositoryManager; + let panel: TestIssueOverviewPanel; + let issue: IssueModel; + let discovered: boolean; + let postMessage: SinonStub; + + beforeEach(function () { + sandbox = createSandbox(); + context = new MockExtensionContext(); + context.extensionUri = vscode.extensions.getExtension(EXTENSION_ID)!.extensionUri; + const telemetry = new MockTelemetry(); + const credentialStore = new CredentialStore(telemetry, context); + repositoriesManager = new RepositoriesManager(credentialStore, telemetry); + const git = new GitApiImpl(repositoriesManager); + const helper = new CreatePullRequestHelper(); + localManager = new FolderRepositoryManager(0, context, new MockRepository(), telemetry, git, + credentialStore, helper, new MockThemeWatcher()); + const url = 'https://github.com/aaa/bbb'; + const remote = new GitHubRemote('origin', url, new Protocol(url), GitHubServerType.GitHubDotCom); + const repo = new MockGitHubRepository(remote, credentialStore, telemetry, sandbox); + discovered = false; + sandbox.stub(localManager, 'gitHubRepositories').get(() => discovered ? [repo] : []); + repositoriesManager.insertFolderManager(localManager); + TestIssueOverviewPanel.registerRepositoriesManager(context, repositoriesManager); + const resolver = new FolderRepositoryManagerResolver(context, repositoriesManager, telemetry); + temporaryManager = resolver.getRemoteOnlyManager(); + issue = new IssueModel(telemetry, repo, remote, { + ...convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).build(), repo), + url: 'https://github.com/aaa/bbb/issues/1000', + }); + panel = new TestIssueOverviewPanel(telemetry, temporaryManager); + panel.setItem(issue); + postMessage = sandbox.stub(panel, '_postMessage').resolves(); + context.subscriptions.push(credentialStore, repositoriesManager, git, helper, repo, resolver, issue, panel); + }); + + afterEach(function () { + IssueOverviewPanel.clearAll(); + context.dispose(); + sandbox.restore(); + }); + + it('uses the temporary manager while the repository is undiscovered', async function () { + const resolveIssue = sandbox.stub(temporaryManager, 'resolveIssue').resolves(issue); + + assert.strictEqual(await panel.resolveModel({ owner: 'aaa', repo: 'bbb', number: 1000 }), issue); + + assert.strictEqual(panel.folderRepositoryManager, temporaryManager); + sandbox.assert.calledWithExactly(resolveIssue, 'aaa', 'bbb', 1000); + }); + + it('upgrades during issue resolution without needing an active-issue event', async function () { + const localResolve = sandbox.stub(localManager, 'resolveIssue').resolves(issue); + const temporaryResolve = sandbox.stub(temporaryManager, 'resolveIssue').resolves(issue); + discovered = true; + + assert.strictEqual(await panel.resolveModel({ owner: 'aaa', repo: 'bbb', number: 1000 }), issue); + + assert.strictEqual(panel.folderRepositoryManager, localManager); + sandbox.assert.calledWithExactly(localResolve, 'aaa', 'bbb', 1000); + sandbox.assert.notCalled(temporaryResolve); + }); + + it('upgrades without subscribing to active-issue changes on the local manager', function () { + const subscribe = sandbox.spy(localManager, 'onDidChangeActiveIssue'); + discovered = true; + + assert.strictEqual(panel.folderRepositoryManager, localManager); + localManager.activeIssue = issue; + + sandbox.assert.notCalled(subscribe); + sandbox.assert.notCalled(postMessage); + }); + + it('does not send checkout-status messages when either manager changes its active issue', function () { + temporaryManager.activeIssue = issue; + localManager.activeIssue = issue; + sandbox.assert.notCalled(postMessage); + discovered = true; + assert.strictEqual(panel.folderRepositoryManager, localManager); + + temporaryManager.activeIssue = undefined; + localManager.activeIssue = undefined; + + sandbox.assert.notCalled(postMessage); + }); + + it('retains its local manager without looking it up again', function () { + discovered = true; + assert.strictEqual(panel.folderRepositoryManager, localManager); + const lookup = sandbox.spy(repositoriesManager, 'getManagerForRepository'); + discovered = false; + + assert.strictEqual(panel.folderRepositoryManager, localManager); + sandbox.assert.notCalled(lookup); + }); + + it('does not upgrade or attach listeners after disposal', function () { + panel.dispose(); + discovered = true; + const lookup = sandbox.spy(repositoriesManager, 'getManagerForRepository'); + + assert.strictEqual(panel.folderRepositoryManager, temporaryManager); + localManager.activeIssue = issue; + + sandbox.assert.notCalled(lookup); + sandbox.assert.notCalled(postMessage); + }); +}); diff --git a/src/test/github/overviewRestorer.test.ts b/src/test/github/overviewRestorer.test.ts index 4835d2d9a3..c15b18344e 100644 --- a/src/test/github/overviewRestorer.test.ts +++ b/src/test/github/overviewRestorer.test.ts @@ -48,7 +48,8 @@ describe('OverviewRestorer', function () { }); it('restores a pull request with a remote-only manager', async function () { - const folderManager = folderRepositoryManagerResolver.getManagerForRepository('microsoft', 'vscode-pull-request-github'); + const folderManager = folderRepositoryManagerResolver.getRemoteOnlyManager(); + const lookup = sandbox.spy(folderRepositoryManagerResolver, 'getManagerForRepository'); const pullRequest = {} as PullRequestModel; sandbox.stub(folderManager, 'resolvePullRequest').resolves(pullRequest); const createOrShow = sandbox.stub(PullRequestOverviewPanel, 'createOrShow').resolves(); @@ -66,5 +67,6 @@ describe('OverviewRestorer', function () { assert.strictEqual(createOrShow.firstCall.args[2], folderManager); assert.strictEqual(createOrShow.firstCall.args[4], pullRequest); assert.strictEqual(createOrShow.firstCall.args[7], webviewPanel); + sandbox.assert.notCalled(lookup); }); }); diff --git a/src/test/github/pullRequestOverview.test.ts b/src/test/github/pullRequestOverview.test.ts index a54b438377..dbf51ee61c 100644 --- a/src/test/github/pullRequestOverview.test.ts +++ b/src/test/github/pullRequestOverview.test.ts @@ -22,6 +22,8 @@ import { MockExtensionContext } from '../mocks/mockExtensionContext'; import { MockGitHubRepository } from '../mocks/mockGitHubRepository'; import { Repository } from '../../api/api'; import { GitApiImpl } from '../../api/api1'; +import { RemoteOnlyRepository } from '../../api/remoteOnlyRepository'; +import { FolderRepositoryManagerResolver } from '../../github/folderRepositoryManagerResolver'; import { openDescription } from '../../commands'; import { EXTENSION_ID } from '../../constants'; import { CredentialStore } from '../../github/credentials'; @@ -39,6 +41,7 @@ import { COPILOT_REVIEWER_ACCOUNT } from '../../common/copilot'; import * as emoji from '../../common/emoji'; import Logger from '../../common/logger'; import { Issue, IssuePreview, OverviewItemPreview, PullRequest, PullRequestPreview } from '../../github/views'; +import { IRequestMessage } from '../../common/webview'; const EXTENSION_URI = vscode.extensions.getExtension(EXTENSION_ID)!.extensionUri; @@ -54,6 +57,25 @@ class TestPullRequestOverviewPanel extends PullRequestOverviewPanel { public override _postMessage(message: { command: string; isCurrentlyCheckedOut?: boolean; pullrequest?: Partial }): Promise { return super._postMessage(message); } + + public setItem(item: PullRequestModel): void { + this._item = item; + this._identity = { owner: item.remote.owner, repo: item.remote.repositoryName, number: item.number }; + TestPullRequestOverviewPanel._panels.set(panelKey(item.remote.owner, item.remote.repositoryName, item.number), this); + this.registerPrListeners(); + } + + public get folderRepositoryManager(): FolderRepositoryManager { + return this._folderRepositoryManager; + } + + public override _onDidReceiveMessage(message: IRequestMessage) { + return super._onDidReceiveMessage(message); + } + + public override _replyMessage(message: IRequestMessage, response: unknown): Promise { + return super._replyMessage(message, response); + } } describe('PullRequestOverview', function () { @@ -344,6 +366,201 @@ describe('PullRequestOverview', function () { } describe('checkout status', function () { + describe('remote-only manager upgrades', function () { + let panel: TestPullRequestOverviewPanel; + let model: PullRequestModel; + let temporaryManager: FolderRepositoryManager; + let discovered: boolean; + let postMessage: SinonStub; + + beforeEach(function () { + setStacksEnabled(false); + discovered = false; + sinon.stub(pullRequestManager, 'gitHubRepositories').get(() => discovered ? [repo] : []); + repositoriesManager.insertFolderManager(pullRequestManager); + PullRequestOverviewPanel.registerGlobalCommands(context, telemetry, repositoriesManager); + const resolver = new FolderRepositoryManagerResolver(context, repositoriesManager, telemetry); + context.subscriptions.push(resolver); + temporaryManager = resolver.getRemoteOnlyManager(); + model = new PullRequestModel(credentialStore, telemetry, repo, remote, + convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).build(), repo)); + panel = new TestPullRequestOverviewPanel(telemetry, temporaryManager); + panel.setItem(model); + context.subscriptions.push(panel); + postMessage = sinon.stub(panel, '_postMessage').resolves(); + }); + + it('keeps the temporary manager when the repository has not been discovered', async function () { + const openChanges = sinon.stub(PullRequestModel, 'openChanges').resolves(); + + await panel._onDidReceiveMessage({ req: 'changes', command: 'pr.open-changes', args: undefined }); + + assert.strictEqual(panel.folderRepositoryManager, temporaryManager); + assert.ok(panel.folderRepositoryManager.repository instanceof RemoteOnlyRepository); + sinon.assert.calledWithExactly(openChanges, temporaryManager, model, false); + }); + + it('upgrades on an operation after discovery without needing a checkout event', async function () { + const openChanges = sinon.stub(PullRequestModel, 'openChanges').resolves(); + discovered = true; + + await panel._onDidReceiveMessage({ req: 'changes', command: 'pr.open-changes', args: { openToTheSide: true } }); + + assert.strictEqual(panel.folderRepositoryManager, pullRequestManager); + sinon.assert.calledWithExactly(openChanges, pullRequestManager, model, true); + }); + + it('checks out main and cleans up through the upgraded local manager', async function () { + await pullRequestManager.repository.createBranch('pr-branch', true); + await pullRequestManager.repository.createBranch('main', false); + await pullRequestManager.repository.setBranchUpstream('main', 'refs/remotes/origin/main'); + pullRequestManager.activePullRequest = model; + assert.strictEqual(panel.folderRepositoryManager, temporaryManager); + discovered = true; + const checkout = sinon.spy(pullRequestManager, 'checkoutDefaultBranch'); + const cleanup = sinon.stub(pullRequestManager, 'cleanupAfterPullRequest').resolves(); + const reply = sinon.stub(panel, '_replyMessage').resolves(); + const message = { req: 'exit', command: 'pr.checkout-default-branch', args: 'main' }; + + await panel._onDidReceiveMessage(message); + + assert.strictEqual(pullRequestManager.repository.state.HEAD?.name, 'main'); + assert.strictEqual(panel.folderRepositoryManager, pullRequestManager); + sinon.assert.calledWithExactly(checkout, 'main', model); + sinon.assert.calledWithExactly(cleanup, 'pr-branch', model); + sinon.assert.calledWithExactly(reply, message, {}); + }); + + it('updates checkout status and moves listeners when the local repository becomes active', function () { + discovered = true; + pullRequestManager.activePullRequest = model; + + assert.strictEqual(panel.folderRepositoryManager, pullRequestManager); + sinon.assert.calledOnce(postMessage); + sinon.assert.calledWithMatch(postMessage, { command: 'pr.update-checkout-status', isCurrentlyCheckedOut: true }); + postMessage.resetHistory(); + temporaryManager.activePullRequest = model; + sinon.assert.notCalled(postMessage); + pullRequestManager.activePullRequest = undefined; + sinon.assert.calledOnce(postMessage); + sinon.assert.calledWithMatch(postMessage, { command: 'pr.update-checkout-status', isCurrentlyCheckedOut: false }); + }); + + it('retains the upgraded local manager without looking it up again', function () { + discovered = true; + assert.strictEqual(panel.folderRepositoryManager, pullRequestManager); + const lookup = sinon.spy(repositoriesManager, 'getManagerForRepository'); + discovered = false; + + assert.strictEqual(panel.folderRepositoryManager, pullRequestManager); + sinon.assert.notCalled(lookup); + }); + + it('upgrades safely on the first access after the panel identity is set', function () { + panel.dispose(); + discovered = true; + panel = new TestPullRequestOverviewPanel(telemetry, temporaryManager); + context.subscriptions.push(panel); + assert.strictEqual(panel.folderRepositoryManager, temporaryManager); + panel.setItem(model); + postMessage = sinon.stub(panel, '_postMessage').resolves(); + + assert.strictEqual(panel.folderRepositoryManager, pullRequestManager); + pullRequestManager.activePullRequest = model; + sinon.assert.calledOnce(postMessage); + }); + }); + + describe('repository ownership', function () { + let panel: TestPullRequestOverviewPanel; + let model: PullRequestModel; + let postMessage: SinonStub; + + beforeEach(async function () { + setStacksEnabled(false); + await pullRequestManager.repository.addRemote('origin', remote.url); + repositoriesManager.insertFolderManager(pullRequestManager); + sinon.stub(pullRequestManager, 'gitHubRepositories').get(() => [repo]); + const resolver = new FolderRepositoryManagerResolver(context, repositoriesManager, telemetry); + context.subscriptions.push(resolver); + const manager = resolver.getManagerForRepository(remote.owner, remote.repositoryName); + assert.strictEqual(manager, pullRequestManager); + PullRequestOverviewPanel.registerGlobalCommands(context, telemetry, repositoriesManager); + model = new PullRequestModel(credentialStore, telemetry, repo, remote, + convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).build(), repo)); + panel = new TestPullRequestOverviewPanel(telemetry, manager); + panel.setItem(model); + context.subscriptions.push(panel); + postMessage = sinon.stub(panel, '_postMessage').resolves(); + }); + + it('checks out the default branch through the selected local manager', async function () { + await pullRequestManager.repository.createBranch('pr-branch', true); + await pullRequestManager.repository.createBranch('main', false); + await pullRequestManager.repository.setBranchUpstream('main', 'refs/remotes/origin/main'); + pullRequestManager.activePullRequest = model; + const checkout = sinon.spy(pullRequestManager, 'checkoutDefaultBranch'); + const cleanup = sinon.stub(pullRequestManager, 'cleanupAfterPullRequest').resolves(); + const reply = sinon.stub(panel, '_replyMessage').resolves(); + const message = { req: 'exit', command: 'pr.checkout-default-branch', args: 'main' }; + + await panel._onDidReceiveMessage(message); + + assert.strictEqual(pullRequestManager.repository.state.HEAD?.name, 'main'); + sinon.assert.calledWithExactly(checkout, 'main', model); + sinon.assert.calledWithExactly(cleanup, 'pr-branch', model); + sinon.assert.calledWithExactly(reply, message, {}); + }); + + it('does not change ownership when the same PR becomes active in another local repository', function () { + const otherRepository = new MockRepository(); + otherRepository.rootUri = vscode.Uri.file('/other'); + const otherManager = new FolderRepositoryManager(2, context, otherRepository, telemetry, + new GitApiImpl(repositoriesManager), credentialStore, new CreatePullRequestHelper(), mockThemeWatcher); + repositoriesManager.insertFolderManager(otherManager); + + otherManager.activePullRequest = model; + + assert.strictEqual(panel.folderRepositoryManager, pullRequestManager); + sinon.assert.calledWithMatch(postMessage, { command: 'pr.update-checkout-status', isCurrentlyCheckedOut: false }); + postMessage.resetHistory(); + pullRequestManager.activePullRequest = model; + sinon.assert.calledOnce(postMessage); + sinon.assert.calledWithMatch(postMessage, { command: 'pr.update-checkout-status', isCurrentlyCheckedOut: true }); + }); + + it('reports checkout in the local repository when the checkout command completes', async function () { + const executeCommand = sinon.stub(vscode.commands, 'executeCommand').callThrough(); + executeCommand.withArgs('pr.pick', model).callsFake(async () => { + pullRequestManager.activePullRequest = model; + }); + const reply = sinon.stub(panel, '_replyMessage').resolves(); + const message = { req: 'checkout', command: 'pr.checkout', args: undefined }; + + await panel._onDidReceiveMessage(message); + await new Promise(resolve => setImmediate(resolve)); + + sinon.assert.calledWithExactly(reply, message, { isCurrentlyCheckedOut: true }); + assert.strictEqual(panel.folderRepositoryManager, pullRequestManager); + }); + + it('retains its original manager when an existing panel is reopened with a different manager', async function () { + panel.dispose(); + const opened = await createPanel(); + const remoteRepository = new RemoteOnlyRepository(); + const otherManager = new FolderRepositoryManager(1, context, remoteRepository, telemetry, + new GitApiImpl(repositoriesManager), credentialStore, new CreatePullRequestHelper(), mockThemeWatcher); + context.subscriptions.push(remoteRepository, otherManager); + opened.currentUser.resetHistory(); + const identity = { owner: remote.owner, repo: remote.repositoryName, number: opened.model.number }; + + await PullRequestOverviewPanel.createOrShow(telemetry, EXTENSION_URI, otherManager, identity, opened.model); + + assert.strictEqual(PullRequestOverviewPanel.findPanel(identity.owner, identity.repo, identity.number), opened.panel); + sinon.assert.calledOnce(opened.currentUser); + }); + }); + for (const initiallyCheckedOut of [false, true]) { it(`initializes with the latest checkout state when ${initiallyCheckedOut ? 'leaving' : 'entering'} review mode during loading`, async function () { setStacksEnabled(false); @@ -368,7 +585,7 @@ describe('PullRequestOverview', function () { return blockedBody; }); - const opening = panel.updateWithIdentity(pullRequestManager, identity, model); + const opening = panel.updateWithIdentity(identity, model); await bodyStarted; pullRequestManager.activePullRequest = initiallyCheckedOut ? undefined : model; releaseBody!(model.bodyHTML); @@ -1183,7 +1400,7 @@ describe('PullRequestOverview', function () { viewerCanAutoMerge: false, }); sinon.stub(pullRequestManager, 'getPullRequestRepositoryDefaultBranch').resolves('main'); - sinon.stub(pullRequestManager, 'getCurrentUser').callsFake(async () => { + const currentUser = sinon.stub(pullRequestManager, 'getCurrentUser').callsFake(async () => { const model = models.values().next().value; assert(model); return model.author; @@ -1198,7 +1415,7 @@ describe('PullRequestOverview', function () { const externalUri = sinon.stub(vscode.env, 'asExternalUri').callsFake(async uri => uri); const showError = sinon.stub(vscode.window, 'showErrorMessage').resolves(undefined); const query = sinon.spy(repo, 'query'); - return { models, access, branch, queueMethod, externalUri, showError, query }; + return { models, access, currentUser, branch, queueMethod, externalUri, showError, query }; } async function createPanel(number = 1000) {