diff --git a/package.json b/package.json index 25c52d2a28..c456ed6e85 100644 --- a/package.json +++ b/package.json @@ -598,6 +598,12 @@ "markdownDescription": "%githubPullRequests.experimental.chat.description%", "default": true }, + "githubPullRequests.experimental.stacks": { + "type": "boolean", + "description": "%githubPullRequests.experimental.stacks.description%", + "default": false, + "included": false + }, "githubPullRequests.codingAgent.enabled": { "type": "boolean", "default": true, diff --git a/package.nls.json b/package.nls.json index 2004a852e7..63ef9494ab 100644 --- a/package.nls.json +++ b/package.nls.json @@ -2,6 +2,7 @@ "displayName": "GitHub Pull Requests", "description": "Pull Request and Issue Provider for GitHub", "githubPullRequests.pullRequestDescription.description": "The description used when creating pull requests.", + "githubPullRequests.experimental.stacks.description": "Enable experimental pull request stack features. Reload the window to apply changes to multi-selection in the Pull Requests view.", "githubPullRequests.pullRequestDescription.template": "Use a pull request template and commit description, or just use the commit description if no templates were found.", "githubPullRequests.pullRequestDescription.commit": "Use the latest commit message only.", "githubPullRequests.pullRequestDescription.branchName": "Use the branch name as the pull request title", diff --git a/src/common/settingKeys.ts b/src/common/settingKeys.ts index acfd160bda..bd268bcbe1 100644 --- a/src/common/settingKeys.ts +++ b/src/common/settingKeys.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ export const PR_SETTINGS_NAMESPACE = 'githubPullRequests'; +export const EXPERIMENTAL_STACKS = 'experimental.stacks'; export const TERMINAL_LINK_HANDLER = 'terminalLinksHandler'; export const OPEN_PULL_LINKS = 'openPullLinks'; export const BRANCH_PUBLISH = 'createOnPublishBranch'; diff --git a/src/common/settingsUtils.ts b/src/common/settingsUtils.ts index 3522ce3e98..80f0819046 100644 --- a/src/common/settingsUtils.ts +++ b/src/common/settingsUtils.ts @@ -6,7 +6,17 @@ 'use strict'; import * as vscode from 'vscode'; import { commands } from './executeCommands'; -import { CHAT_SETTINGS_NAMESPACE, DISABLE_AI_FEATURES, PR_SETTINGS_NAMESPACE, QUERIES, USE_REVIEW_MODE } from './settingKeys'; +import { CHAT_SETTINGS_NAMESPACE, DISABLE_AI_FEATURES, EXPERIMENTAL_STACKS, PR_SETTINGS_NAMESPACE, QUERIES, USE_REVIEW_MODE } from './settingKeys'; + +export function areStacksEnabled(): boolean { + return vscode.workspace.getConfiguration(PR_SETTINGS_NAMESPACE).get(EXPERIMENTAL_STACKS, false); +} + +export function assertStacksEnabled(): void { + if (!areStacksEnabled()) { + throw new Error(vscode.l10n.t('Pull request stack features are disabled.')); + } +} export function getReviewMode(): { merged: boolean, closed: boolean } { const desktopDefaults = { merged: false, closed: false }; diff --git a/src/github/activityBarViewProvider.ts b/src/github/activityBarViewProvider.ts index 9ca57f20aa..9ac08527bc 100644 --- a/src/github/activityBarViewProvider.ts +++ b/src/github/activityBarViewProvider.ts @@ -17,7 +17,8 @@ import { IComment } from '../common/comment'; import { emojify, ensureEmojis } from '../common/emoji'; import { disposeAll } from '../common/lifecycle'; import Logger from '../common/logger'; -import { CHECKOUT_DEFAULT_BRANCH, CHECKOUT_PULL_REQUEST_BASE_BRANCH, POST_DONE, PR_SETTINGS_NAMESPACE } from '../common/settingKeys'; +import { CHECKOUT_DEFAULT_BRANCH, CHECKOUT_PULL_REQUEST_BASE_BRANCH, EXPERIMENTAL_STACKS, POST_DONE, PR_SETTINGS_NAMESPACE } from '../common/settingKeys'; +import { areStacksEnabled } from '../common/settingsUtils'; import { ReviewEvent, TimelineEvent } from '../common/timelineEvent'; import { formatError } from '../common/utils'; import { generateUuid } from '../common/uuid'; @@ -36,6 +37,11 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W private _item: PullRequestModel, ) { super(extensionUri); + this._register(vscode.workspace.onDidChangeConfiguration(e => { + if (e.affectsConfiguration(`${PR_SETTINGS_NAMESPACE}.${EXPERIMENTAL_STACKS}`)) { + void this.updatePullRequest(this._item); + } + })); this._register(vscode.commands.registerCommand('pr.readyForReview', async () => { return this.readyForReviewCommand(); @@ -303,7 +309,7 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W defaultMergeMethod, mergeQueueMethod, stack: undefined, - stackLoaded: false, + stackLoaded: !areStacksEnabled(), stackLoadError: false, repositoryDefaultBranch: defaultBranch, doneCheckoutBranch, @@ -323,27 +329,29 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W command: 'pr.initialize', pullrequest: context, }); - void pullRequest.getStack().then(async stack => { - if (!this._item.equals(pullRequest)) { - return; - } - const stackQueueMethod = stack ? await this._folderRepositoryManager.mergeQueueMethodForBranch(stack.base, pullRequest.remote.owner, pullRequest.remote.repositoryName) : undefined; - if (this._item.equals(pullRequest)) { - this._postMessage({ - command: 'pr.update', - pullrequest: { - stack, - stackLoaded: true, - ...(stack ? { mergeQueueMethod: stackQueueMethod } : {}), - } satisfies Partial, - }); - } - }).catch(error => { - Logger.error(`Failed to load active pull request stack: ${formatError(error)}`, PullRequestViewProvider.name); - if (this._item.equals(pullRequest)) { - this._postMessage({ command: 'pr.update', pullrequest: { stackLoadError: true } satisfies Partial }); - } - }); + if (areStacksEnabled()) { + void pullRequest.getStack().then(async stack => { + if (!this._item.equals(pullRequest) || !areStacksEnabled()) { + return; + } + const stackQueueMethod = stack ? await this._folderRepositoryManager.mergeQueueMethodForBranch(stack.base, pullRequest.remote.owner, pullRequest.remote.repositoryName) : undefined; + if (this._item.equals(pullRequest) && areStacksEnabled()) { + this._postMessage({ + command: 'pr.update', + pullrequest: { + stack, + stackLoaded: true, + ...(stack ? { mergeQueueMethod: stackQueueMethod } : {}), + } satisfies Partial, + }); + } + }).catch(error => { + Logger.error(`Failed to load active pull request stack: ${formatError(error)}`, PullRequestViewProvider.name); + if (this._item.equals(pullRequest) && areStacksEnabled()) { + this._postMessage({ command: 'pr.update', pullrequest: { stackLoadError: true } satisfies Partial }); + } + }); + } } catch (e) { vscode.window.showErrorMessage(`Error updating active pull request view: ${formatError(e)}`); diff --git a/src/github/createPRViewProvider.ts b/src/github/createPRViewProvider.ts index 787fcd3fb8..fc09dc3463 100644 --- a/src/github/createPRViewProvider.ts +++ b/src/github/createPRViewProvider.ts @@ -32,12 +32,14 @@ import { ASSIGN_TO, CREATE_BASE_BRANCH, DEFAULT_CREATE_OPTION, + EXPERIMENTAL_STACKS, PR_SETTINGS_NAMESPACE, PULL_REQUEST_DESCRIPTION, PULL_REQUEST_LABELS, PUSH_BRANCH, SHOW_CREATE_PULL_REQUEST_CANCEL_CONFIRMATION } from '../common/settingKeys'; +import { areStacksEnabled, assertStacksEnabled } from '../common/settingsUtils'; import { ITelemetry } from '../common/telemetry'; import { toOpenPullRequestWebviewUri } from '../common/uri'; import { asPromise, compareIgnoreCase, formatError, promiseWithTimeout } from '../common/utils'; @@ -685,6 +687,21 @@ export class CreatePullRequestViewProvider extends BaseCreatePullRequestViewProv ) { super(telemetry, model, extensionUri, folderRepositoryManager, pullRequestDefaults, model.compareBranch); + this._register(vscode.workspace.onDidChangeConfiguration(e => { + if (!e.affectsConfiguration(`${PR_SETTINGS_NAMESPACE}.${EXPERIMENTAL_STACKS}`) || !this._view) { + return; + } + const sequence = ++this._stackCandidateSequence; + void this.getStackCandidateForView( + { owner: this.model.baseOwner, repositoryName: this.model.repositoryName }, this.model.baseBranch, + { owner: this.model.compareOwner, repositoryName: this.model.repositoryName }, this.model.compareBranch, + ).then(stackCandidate => { + if (sequence === this._stackCandidateSequence) { + return this._postMessage({ command: 'pr.initialize', params: { stackCandidate } }); + } + }); + })); + this._register(this.model.onDidChange(async (e) => { const stackCandidateSequence = ++this._stackCandidateSequence; let baseRemote: RemoteInfo | undefined; @@ -1111,8 +1128,14 @@ Don't forget to commit your template file to the repository so that it can be us } protected async getStackCandidateForView(baseRemote: RemoteInfo | undefined, baseBranch: string | undefined, compareRemote: RemoteInfo | undefined, compareBranch: string | undefined): Promise { + if (!areStacksEnabled()) { + return; + } try { const candidate = await this.getStackCandidate(baseRemote, baseBranch, compareRemote, compareBranch); + if (!areStacksEnabled()) { + return; + } if (!candidate || !baseRemote) { return candidate; } @@ -1539,6 +1562,7 @@ Don't forget to commit your template file to the repository so that it can be us try { let stackCandidate: StackCandidate | undefined; if (message.args.addToStack) { + assertStacksEnabled(); if (message.args.autoMerge) { throw new Error(vscode.l10n.t('Auto-merge is not available for stacked pull requests.')); } @@ -1659,6 +1683,7 @@ Don't forget to commit your template file to the repository so that it can be us } if (stackCandidate) { try { + assertStacksEnabled(); await createdPR.githubRepository.addPullRequestToStack(stackCandidate, createdPR.number); } catch (error) { stackAdditionFailed = true; diff --git a/src/github/pullRequestOverview.ts b/src/github/pullRequestOverview.ts index 916796f792..e2d9d5b3b1 100644 --- a/src/github/pullRequestOverview.ts +++ b/src/github/pullRequestOverview.ts @@ -37,7 +37,8 @@ import { commands, contexts } from '../common/executeCommands'; import { openWithDefaultExternalOpener } from '../common/externalUri'; import { disposeAll } from '../common/lifecycle'; import Logger from '../common/logger'; -import { CHECKOUT_DEFAULT_BRANCH, CHECKOUT_PULL_REQUEST_BASE_BRANCH, DEFAULT_MERGE_METHOD, DELETE_BRANCH_AFTER_MERGE, POST_DONE, PR_SETTINGS_NAMESPACE } from '../common/settingKeys'; +import { CHECKOUT_DEFAULT_BRANCH, CHECKOUT_PULL_REQUEST_BASE_BRANCH, DEFAULT_MERGE_METHOD, DELETE_BRANCH_AFTER_MERGE, EXPERIMENTAL_STACKS, POST_DONE, PR_SETTINGS_NAMESPACE } from '../common/settingKeys'; +import { areStacksEnabled, assertStacksEnabled } from '../common/settingsUtils'; import { ITelemetry } from '../common/telemetry'; import { EventType, ReviewEvent, SessionLinkInfo, TimelineEvent } from '../common/timelineEvent'; import { toOpenIssueWebviewUri, toOpenPullRequestWebviewUri } from '../common/uri'; @@ -255,6 +256,11 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel { + if (e.affectsConfiguration(`${PR_SETTINGS_NAMESPACE}.${EXPERIMENTAL_STACKS}`)) { + void this.refreshPanel(); + } + })); this.setVisibilityContext(); } @@ -276,15 +282,17 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel { - if (stack) { - return PullRequestOverviewPanel.refreshStackPanels(item.remote.owner, item.remote.repositoryName, - stack.pullRequests.filter(entry => entry.number !== item.number).map(entry => entry.number)); - } - }).catch(error => { - Logger.error(`Failed to refresh pull request stack after draft change: ${formatError(error)}`, PullRequestOverviewPanel.ID); - void vscode.window.showErrorMessage(vscode.l10n.t('Unable to refresh pull request stack: {0}', formatError(error))); - }); + if (areStacksEnabled()) { + void item.getStack().then(stack => { + if (stack) { + return PullRequestOverviewPanel.refreshStackPanels(item.remote.owner, item.remote.repositoryName, + stack.pullRequests.filter(entry => entry.number !== item.number).map(entry => entry.number)); + } + }).catch(error => { + Logger.error(`Failed to refresh pull request stack after draft change: ${formatError(error)}`, PullRequestOverviewPanel.ID); + void vscode.window.showErrorMessage(vscode.l10n.t('Unable to refresh pull request stack: {0}', formatError(error))); + }); + } } else if ((e.state || e.comments) && !this._refreshing && !this._updateItemPromise) { this.refreshPanel(); } @@ -519,7 +527,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel { Logger.error(`Failed to update deferred pull request data: ${formatError(error)}`, PullRequestOverviewPanel.ID); }); - void pullRequestModel.getStack().then(async stack => { - if (updateSequence !== this._updateSequence) { - return; - } - const stackQueueMethod = stack ? await this._folderRepositoryManager.mergeQueueMethodForBranch(stack.base, pullRequest.remote.owner, pullRequest.remote.repositoryName) : undefined; - const linkedStack = stack && { - ...stack, - pullRequests: await Promise.all(stack.pullRequests.map(async entry => ({ - ...entry, - url: (await toOpenPullRequestWebviewUri({ - owner: pullRequest.remote.owner, - repo: pullRequest.remote.repositoryName, - pullRequestNumber: entry.number, - })).toString(), - }))), - }; - if (updateSequence === this._updateSequence) { - stackLoaded = true; - await this._postMessage({ - command: 'pr.update', - pullrequest: { - stack: linkedStack, - stackLoaded: true, - ...(stack ? { mergeQueueMethod: stackQueueMethod } : {}), - } satisfies Partial, - }); - } - }).catch(error => { - Logger.error(`Failed to load pull request stack: ${formatError(error)}`, PullRequestOverviewPanel.ID); - if (updateSequence === this._updateSequence) { - void this._postMessage({ command: 'pr.update', pullrequest: { stackLoadError: true } satisfies Partial }); - } - }); + if (areStacksEnabled()) { + void this.loadStack(pullRequestModel, updateSequence, () => { stackLoaded = true; }); + } const timelineStart = performance.now(); void Promise.all([pullRequestModel.getTimelineEvents(), reviewRequestsPromise]).then(async ([latestTimelineEvents, requestedReviewers]) => { const events = latestTimelineEvents ?? []; @@ -680,6 +658,43 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel void): Promise { + try { + const stack = await pullRequestModel.getStack(); + if (updateSequence !== this._updateSequence || !areStacksEnabled()) { + return; + } + const stackQueueMethod = stack ? await this._folderRepositoryManager.mergeQueueMethodForBranch(stack.base, pullRequestModel.remote.owner, pullRequestModel.remote.repositoryName) : undefined; + const linkedStack = stack && { + ...stack, + pullRequests: await Promise.all(stack.pullRequests.map(async entry => ({ + ...entry, + url: (await toOpenPullRequestWebviewUri({ + owner: pullRequestModel.remote.owner, + repo: pullRequestModel.remote.repositoryName, + pullRequestNumber: entry.number, + })).toString(), + }))), + }; + if (updateSequence === this._updateSequence && areStacksEnabled()) { + onLoaded(); + await this._postMessage({ + command: 'pr.update', + pullrequest: { + stack: linkedStack, + stackLoaded: true, + ...(stack ? { mergeQueueMethod: stackQueueMethod } : {}), + } satisfies Partial, + }); + } + } catch (error) { + Logger.error(`Failed to load pull request stack: ${formatError(error)}`, PullRequestOverviewPanel.ID); + if (updateSequence === this._updateSequence && areStacksEnabled()) { + void this._postMessage({ command: 'pr.update', pullrequest: { stackLoadError: true } satisfies Partial }); + } + } + } + public override async refreshPanel(): Promise { if (!this._panel.visible || this._refreshing) { return; @@ -1117,6 +1132,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel): Promise { try { + assertStacksEnabled(); const access = await this._folderRepositoryManager.getPullRequestRepositoryAccessAndMergeMethods(this._item); if (!access.hasWritePermission) { throw new Error(vscode.l10n.t('You do not have permission to unstack these pull requests.')); diff --git a/src/github/pullRequestReviewCommon.ts b/src/github/pullRequestReviewCommon.ts index 571ad0dcec..3a9993eef4 100644 --- a/src/github/pullRequestReviewCommon.ts +++ b/src/github/pullRequestReviewCommon.ts @@ -12,6 +12,7 @@ import { PullRequestModel } from './pullRequestModel'; import { ConvertToDraftReply, PullRequest, ReadyForReviewReply, ReviewType, StackMergeResult, SubmitReviewReply } from './views'; import Logger from '../common/logger'; import { DEFAULT_DELETION_METHOD, DELETE_BRANCH_AFTER_MERGE, PR_SETTINGS_NAMESPACE, SELECT_LOCAL_BRANCH, SELECT_REMOTE, SELECT_WORKTREE } from '../common/settingKeys'; +import { assertStacksEnabled } from '../common/settingsUtils'; import { ReviewEvent, TimelineEvent } from '../common/timelineEvent'; import { Schemes } from '../common/uri'; import { formatError } from '../common/utils'; @@ -37,6 +38,7 @@ export interface ReviewContext { export namespace PullRequestReviewCommon { export async function mergeStack(ctx: ReviewContext, message: IRequestMessage<{ method: MergeMethod }>): Promise { try { + assertStacksEnabled(); const { item, folderRepositoryManager } = ctx; const stack = await item.getStack(); if (!stack) { diff --git a/src/github/pullRequestStack.ts b/src/github/pullRequestStack.ts index b6670374c4..3f5c3e2f5d 100644 --- a/src/github/pullRequestStack.ts +++ b/src/github/pullRequestStack.ts @@ -6,6 +6,7 @@ import { GithubItemStateEnum } from './interface'; import { PullRequestModel } from './pullRequestModel'; import { StackCandidate } from '../../common/views'; +import { assertStacksEnabled } from '../common/settingsUtils'; import { compareIgnoreCase } from '../common/utils'; function sameRepository(first: PullRequestModel, second: PullRequestModel): boolean { @@ -55,6 +56,8 @@ export function orderStackablePullRequests(pullRequests: readonly PullRequestMod } export async function addPullRequestsToStack(pullRequests: readonly PullRequestModel[], confirmedCandidate: StackCandidate): Promise { + assertStacksEnabled(); + const initial = orderStackablePullRequests(pullRequests); if (!initial) { throw new Error('Select two or more open pull requests whose head and base branches form a chain in the same repository.'); diff --git a/src/test/common/settingsUtils.test.ts b/src/test/common/settingsUtils.test.ts new file mode 100644 index 0000000000..ea3d19b001 --- /dev/null +++ b/src/test/common/settingsUtils.test.ts @@ -0,0 +1,45 @@ +/*--------------------------------------------------------------------------------------------- + * 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 * as vscode from 'vscode'; +import { createSandbox, SinonSandbox } from 'sinon'; +import { areStacksEnabled, assertStacksEnabled } from '../../common/settingsUtils'; +import { mockStackSetting } from '../mocks/mockStackSetting'; + +describe('Stack settings', function () { + let sinon: SinonSandbox; + let setStacksEnabled: (enabled: boolean) => void; + + beforeEach(function () { + sinon = createSandbox(); + setStacksEnabled = mockStackSetting(sinon); + }); + + afterEach(function () { + sinon.restore(); + }); + + it('checks the current setting on every assertion', function () { + assert.strictEqual(areStacksEnabled(), true); + assert.doesNotThrow(() => assertStacksEnabled()); + + setStacksEnabled(false); + assert.strictEqual(areStacksEnabled(), false); + assert.throws(() => assertStacksEnabled(), /Pull request stack features are disabled/); + + setStacksEnabled(true); + assert.doesNotThrow(() => assertStacksEnabled()); + }); + + it('localizes the disabled-feature error', function () { + setStacksEnabled(false); + const localize = sinon.stub(vscode.l10n, 't').returns('Localized stacks-disabled message.'); + + assert.throws(() => assertStacksEnabled(), { message: 'Localized stacks-disabled message.' }); + assert(localize.calledOnce); + assert.deepStrictEqual(localize.firstCall.args, ['Pull request stack features are disabled.']); + }); +}); diff --git a/src/test/github/createPRViewProvider.test.ts b/src/test/github/createPRViewProvider.test.ts index 2ee262e3e7..3f84542329 100644 --- a/src/test/github/createPRViewProvider.test.ts +++ b/src/test/github/createPRViewProvider.test.ts @@ -30,6 +30,7 @@ import { MockCommandRegistry } from '../mocks/mockCommandRegistry'; import { MockExtensionContext } from '../mocks/mockExtensionContext'; import { MockGitHubRepository } from '../mocks/mockGitHubRepository'; import { MockRepository } from '../mocks/mockRepository'; +import { mockStackSetting } from '../mocks/mockStackSetting'; import { MockTelemetry } from '../mocks/mockTelemetry'; import { MockThemeWatcher } from '../mocks/mockThemeWatcher'; @@ -81,10 +82,12 @@ describe('Create pull request stack', function () { let githubRepository: MockGitHubRepository; let provider: TestCreatePullRequestViewProvider; let model: CreatePullRequestDataModel; + let setStacksEnabled: (enabled: boolean) => void; beforeEach(async function () { sinon = createSandbox(); MockCommandRegistry.install(sinon); + setStacksEnabled = mockStackSetting(sinon); context = new MockExtensionContext(); const telemetry = new MockTelemetry(); credentials = new CredentialStore(telemetry, context); @@ -112,6 +115,42 @@ describe('Create pull request stack', function () { sinon.restore(); }); + it('does not look for stack candidates while stacks are disabled', async function () { + setStacksEnabled(false); + const lookup = sinon.stub(folderManager, 'createGitHubRepositoryFromOwnerName'); + + const candidate = await provider.getStackCandidateForTest( + { owner: 'github', repositoryName: 'test' }, 'D3', + { owner: 'github', repositoryName: 'test' }, 'D4', + ); + + assert.strictEqual(candidate, undefined); + assert(lookup.notCalled); + }); + + it('rejects a stale stack selection before creating a pull request when disabled', async function () { + setStacksEnabled(false); + const cancellation = new vscode.CancellationTokenSource(); + sinon.stub(vscode.window, 'withProgress').callsFake((_options, task) => task({ report: () => undefined }, cancellation.token)); + const create = sinon.stub(folderManager, 'createPullRequest'); + const throwError = sinon.stub(provider, '_throwError').resolves(); + sinon.stub(provider, '_replyMessage').resolves(); + + await provider.createForTest({ + command: 'pr.create', req: '1', + args: { + title: 'Fourth change', body: '', owner: 'github', repo: 'test', base: 'D3', + compareOwner: 'github', compareRepo: 'test', compareBranch: 'D4', + draft: false, autoMerge: false, labels: [], projects: [], assignees: [], reviewers: [], + addToStack: true, stackParentPullRequest: 795, stackNumber: 12, + }, + }); + + assert(create.notCalled); + assert.match(throwError.firstCall.args[1], /stack features are disabled/); + cancellation.dispose(); + }); + it('links to the parent PR webview from the stack option', async function () { sinon.stub(folderManager, 'createGitHubRepositoryFromOwnerName').resolves(githubRepository); sinon.stub(githubRepository, 'getStackCandidate').resolves({ diff --git a/src/test/github/pullRequestOverview.test.ts b/src/test/github/pullRequestOverview.test.ts index 8bd92facac..f047d9fb4a 100644 --- a/src/test/github/pullRequestOverview.test.ts +++ b/src/test/github/pullRequestOverview.test.ts @@ -27,6 +27,7 @@ import { CheckState, GithubItemStateEnum, IAccount, PullRequestMergeability, Pul import { CreatePullRequestHelper } from '../../view/createPullRequestHelper'; import { RepositoriesManager } from '../../github/repositoriesManager'; import { MockThemeWatcher } from '../mocks/mockThemeWatcher'; +import { mockStackSetting } from '../mocks/mockStackSetting'; import { TimelineEvent } from '../../common/timelineEvent'; import { PullRequestReviewCommon, ReviewContext } from '../../github/pullRequestReviewCommon'; import { COPILOT_REVIEWER_ACCOUNT } from '../../common/copilot'; @@ -44,10 +45,12 @@ describe('PullRequestOverview', function () { let telemetry: MockTelemetry; let credentialStore: CredentialStore; let mockThemeWatcher: MockThemeWatcher; + let setStacksEnabled: (enabled: boolean) => void; beforeEach(async function () { sinon = createSandbox(); MockCommandRegistry.install(sinon); + setStacksEnabled = mockStackSetting(sinon); context = new MockExtensionContext(); const repository = new MockRepository(); @@ -75,6 +78,25 @@ describe('PullRequestOverview', function () { }); describe('createOrShow', function () { + it('does not load stack membership when stacks are disabled', async function () { + setStacksEnabled(false); + const model = new PullRequestModel(credentialStore, telemetry, repo, remote, + convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).build(), repo)); + const getStack = sinon.stub(model, 'getStack'); + const postMessage = sinon.spy(PullRequestOverviewPanel.prototype as any, '_postMessage'); + sinon.stub(pullRequestManager, 'getCurrentUser').resolves(model.author); + + await PullRequestOverviewPanel.createOrShow(telemetry, EXTENSION_URI, pullRequestManager, + { owner: remote.owner, repo: remote.repositoryName, number: model.number }, model); + + assert(getStack.notCalled); + const initialize = postMessage.getCalls().find(call => call.args[0].command === 'pr.initialize' + && call.args[0].pullrequest?.stackLoaded !== undefined); + assert(initialize); + assert.strictEqual(initialize?.args[0].pullrequest.stackLoaded, true); + assert.strictEqual(initialize?.args[0].pullrequest.stack, undefined); + }); + it('creates a new panel', async function () { assert.strictEqual(PullRequestOverviewPanel.findPanel('aaa', 'bbb', 1000), undefined); const createWebviewPanel = sinon.spy(vscode.window, 'createWebviewPanel'); @@ -710,63 +732,6 @@ describe('PullRequestOverview', function () { }); }); - describe('mergeStack', function () { - const stack: PullRequestStack = { - position: 2, - size: 2, - base: 'production', - pullRequests: [ - { position: 1, number: 999, title: 'First', url: '', head: 'D1', state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Mergeable }, - { position: 2, number: 1000, title: 'Second', url: '', head: 'D2', state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Mergeable }, - ], - }; - - function createMergeContext() { - const item = new PullRequestModel(credentialStore, telemetry, repo, remote, - convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).build(), repo)); - sinon.stub(item, 'getStack').resolves(stack); - return { - item, - folderRepositoryManager: pullRequestManager, - existingReviewers: [], - postMessage: sinon.stub().resolves(), - replyMessage: sinon.spy(), - throwError: sinon.spy(), - getTimeline: sinon.stub().resolves([]), - } satisfies ReviewContext; - } - - it('uses the stack target branch merge queue and reports enqueue without marking the PR merged', async function () { - const ctx = createMergeContext(); - sinon.stub(repo, 'getPullRequest').resolves(ctx.item); - const queue = sinon.stub(pullRequestManager, 'mergeQueueMethodForBranch').resolves('squash'); - const merge = sinon.stub(ctx.item, 'mergeStack').resolves('enqueued'); - const information = sinon.stub(vscode.window, 'showInformationMessage').resolves(undefined); - const message = { req: '1', command: 'pr.merge-stack', args: { method: 'squash' as const } }; - - await PullRequestReviewCommon.mergeStack(ctx, message); - - assert(queue.calledOnceWithExactly('production', remote.owner, remote.repositoryName)); - assert(merge.calledOnceWithExactly(pullRequestManager.repository, stack, 'squash', 'merge_queue')); - sinon.assert.calledWithExactly(ctx.replyMessage, message, { status: 'enqueued', state: undefined }); - assert(information.calledOnce); - assert(ctx.throwError.notCalled); - }); - - it('reports a rejected merge rather than sending a successful response', async function () { - const ctx = createMergeContext(); - sinon.stub(ctx.item, 'mergeStack').rejects(new Error('Required checks failed')); - const showError = sinon.stub(vscode.window, 'showErrorMessage').resolves(undefined); - const message = { req: '2', command: 'pr.merge-stack', args: { method: 'merge' as const } }; - - await PullRequestReviewCommon.mergeStack(ctx, message); - - assert(showError.calledOnce); - assert(ctx.replyMessage.notCalled); - sinon.assert.calledWithExactly(ctx.throwError, message, 'Required checks failed'); - }); - }); - const prItem = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).build(), repo); const prModel = new PullRequestModel(credentialStore, telemetry, repo, remote, prItem); const identity = { owner: prModel.remote.owner, repo: prModel.remote.repositoryName, number: prModel.number }; @@ -796,28 +761,140 @@ describe('PullRequestOverview', function () { }); }); - describe('unstackAll', function () { - async function createPanel() { - const prItem = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).build(), repo); - const model = new PullRequestModel(credentialStore, telemetry, repo, remote, prItem); - const identity = { owner: remote.owner, repo: remote.repositoryName, number: model.number }; - await PullRequestOverviewPanel.createOrShow(telemetry, EXTENSION_URI, pullRequestManager, identity, model); - const panel = PullRequestOverviewPanel.findPanel(identity.owner, identity.repo, identity.number)!; - const stackQuery = sinon.stub(model, 'getStack').resolves({ - position: 2, size: 2, base: 'main', - pullRequests: [ - { position: 1, number: 999, title: 'First', url: '', head: 'D1', state: GithubItemStateEnum.Merged, isDraft: false, mergeable: PullRequestMergeability.Unknown }, - { position: 2, number: 1000, title: 'Second', url: '', head: 'D2', state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Mergeable }, - ], - }); - const access = sinon.stub(pullRequestManager, 'getPullRequestRepositoryAccessAndMergeMethods').resolves({ - hasWritePermission: true, - mergeMethodsAvailability: { merge: true, squash: true, rebase: true }, - viewerCanAutoMerge: false, - }); - return { panel, model, access, stackQuery }; + describe('mergeStack', function () { + const stack: PullRequestStack = { + position: 2, + size: 2, + base: 'production', + pullRequests: [ + { position: 1, number: 999, title: 'First', url: '', head: 'D1', state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Mergeable }, + { position: 2, number: 1000, title: 'Second', url: '', head: 'D2', state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Mergeable }, + ], + }; + + function createMergeContext() { + const item = new PullRequestModel(credentialStore, telemetry, repo, remote, + convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).build(), repo)); + sinon.stub(item, 'getStack').resolves(stack); + return { + item, + folderRepositoryManager: pullRequestManager, + existingReviewers: [], + postMessage: sinon.stub().resolves(), + replyMessage: sinon.spy(), + throwError: sinon.spy(), + getTimeline: sinon.stub().resolves([]), + } satisfies ReviewContext; } + it('uses the stack target branch merge queue and reports enqueue without marking the PR merged', async function () { + const ctx = createMergeContext(); + sinon.stub(repo, 'getPullRequest').resolves(ctx.item); + const queue = sinon.stub(pullRequestManager, 'mergeQueueMethodForBranch').resolves('squash'); + const merge = sinon.stub(ctx.item, 'mergeStack').resolves('enqueued'); + const information = sinon.stub(vscode.window, 'showInformationMessage').resolves(undefined); + const message = { req: '1', command: 'pr.merge-stack', args: { method: 'squash' as const } }; + + await PullRequestReviewCommon.mergeStack(ctx, message); + + assert(queue.calledOnceWithExactly('production', remote.owner, remote.repositoryName)); + assert(merge.calledOnceWithExactly(pullRequestManager.repository, stack, 'squash', 'merge_queue')); + sinon.assert.calledWithExactly(ctx.replyMessage, message, { status: 'enqueued', state: undefined }); + assert(information.calledOnce); + assert(ctx.throwError.notCalled); + }); + + it('reports a rejected merge rather than sending a successful response', async function () { + const ctx = createMergeContext(); + sinon.stub(ctx.item, 'mergeStack').rejects(new Error('Required checks failed')); + const showError = sinon.stub(vscode.window, 'showErrorMessage').resolves(undefined); + const message = { req: '2', command: 'pr.merge-stack', args: { method: 'merge' as const } }; + + await PullRequestReviewCommon.mergeStack(ctx, message); + + assert(showError.calledOnce); + assert(ctx.replyMessage.notCalled); + sinon.assert.calledWithExactly(ctx.throwError, message, 'Required checks failed'); + }); + + it('does not merge a stack when the feature is disabled', async function () { + setStacksEnabled(false); + const ctx = createMergeContext(); + const getStack = ctx.item.getStack as ReturnType; + const showError = sinon.stub(vscode.window, 'showErrorMessage').resolves(undefined); + const message = { req: '3', command: 'pr.merge-stack', args: { method: 'merge' as const } }; + + await PullRequestReviewCommon.mergeStack(ctx, message); + + assert(getStack.notCalled); + assert(ctx.replyMessage.notCalled); + sinon.assert.calledWithExactly(ctx.throwError, message, 'Pull request stack features are disabled.'); + assert(showError.calledOnce); + }); + }); + + async function createPanel() { + const prItem = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).build(), repo); + const model = new PullRequestModel(credentialStore, telemetry, repo, remote, prItem); + const identity = { owner: remote.owner, repo: remote.repositoryName, number: model.number }; + await PullRequestOverviewPanel.createOrShow(telemetry, EXTENSION_URI, pullRequestManager, identity, model); + const panel = PullRequestOverviewPanel.findPanel(identity.owner, identity.repo, identity.number)!; + const stackQuery = sinon.stub(model, 'getStack').resolves({ + position: 2, size: 2, base: 'main', + pullRequests: [ + { position: 1, number: 999, title: 'First', url: '', head: 'D1', state: GithubItemStateEnum.Merged, isDraft: false, mergeable: PullRequestMergeability.Unknown }, + { position: 2, number: 1000, title: 'Second', url: '', head: 'D2', state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Mergeable }, + ], + }); + const access = sinon.stub(pullRequestManager, 'getPullRequestRepositoryAccessAndMergeMethods').resolves({ + hasWritePermission: true, + mergeMethodsAvailability: { merge: true, squash: true, rebase: true }, + viewerCanAutoMerge: false, + }); + return { panel, model, access, stackQuery }; + } + + describe('loadStack', function () { + it('marks the stack loaded before posting linked stack details', async function () { + const externalUri = sinon.stub(vscode.env, 'asExternalUri').callsFake(async uri => uri.with({ scheme: 'test-external' })); + const { panel, model, stackQuery } = await createPanel(); + sinon.stub(pullRequestManager, 'mergeQueueMethodForBranch').resolves(undefined); + const onLoaded = sinon.spy(); + const postMessage = sinon.stub(panel as any, '_postMessage').callsFake(async (message: { pullrequest?: { stackLoaded?: boolean } }) => { + if (message.pullrequest?.stackLoaded) { + assert(onLoaded.calledOnce); + } + }); + + await (panel as any).loadStack(model, (panel as any)._updateSequence, onLoaded); + + assert(stackQuery.calledOnce); + const update = postMessage.getCalls().find(call => call.args[0].pullrequest?.stackLoaded); + assert(update); + assert.strictEqual(update.args[0].pullrequest.stack.pullRequests.length, 2); + assert(externalUri.calledTwice); + assert(update.args[0].pullrequest.stack.pullRequests.every(entry => entry.url.includes('/open-pull-request-webview'))); + assert(update.args[0].pullrequest.stack.pullRequests.every(entry => entry.url.startsWith('test-external:'))); + assert.deepStrictEqual(update.args[0].pullrequest.stack.pullRequests.map(entry => JSON.parse(vscode.Uri.parse(entry.url).query)), [ + { owner: remote.owner, repo: remote.repositoryName, pullRequestNumber: 999 }, + { owner: remote.owner, repo: remote.repositoryName, pullRequestNumber: 1000 }, + ]); + }); + + it('ignores results from a stale overview update', async function () { + const { panel, model } = await createPanel(); + const postMessage = sinon.stub(panel as any, '_postMessage').resolves(); + const onLoaded = sinon.spy(); + + await (panel as any).loadStack(model, (panel as any)._updateSequence - 1, onLoaded); + + assert(onLoaded.notCalled); + assert(postMessage.notCalled); + }); + }); + + describe('unstackAll', function () { + it('confirms unstacking all eligible PRs and reports remaining locked PRs', async function () { const { panel } = await createPanel(); const confirm = sinon.stub(vscode.window, 'showWarningMessage').resolves('Unstack all' as never); @@ -871,6 +948,21 @@ describe('PullRequestOverview', function () { assert.match(information.firstCall.args[0], /No pull requests were unstacked.*currently merging pull requests remain/); }); + it('does not unstack when the feature is disabled', async function () { + setStacksEnabled(false); + const { panel } = await createPanel(); + const unstack = sinon.stub(repo, 'unstackAll'); + const showError = sinon.stub(vscode.window, 'showErrorMessage').resolves(undefined); + const throwError = sinon.stub(panel as any, '_throwError').resolves(); + const message = { req: 'disabled', command: 'pr.unstack-all', args: undefined }; + + await (panel as any).unstackAll(message); + + assert(unstack.notCalled); + assert(showError.calledOnce); + assert.match(throwError.firstCall.args[1], /stack features are disabled/); + }); + it('refreshes other visible PR panels in the unstacked stack', async function () { const { panel } = await createPanel(); const siblingModel = new PullRequestModel(credentialStore, telemetry, repo, remote, diff --git a/src/test/github/pullRequestStack.test.ts b/src/test/github/pullRequestStack.test.ts index 4435213f33..14b53bc56b 100644 --- a/src/test/github/pullRequestStack.test.ts +++ b/src/test/github/pullRequestStack.test.ts @@ -20,6 +20,7 @@ import { MockCommandRegistry } from '../mocks/mockCommandRegistry'; import { MockExtensionContext } from '../mocks/mockExtensionContext'; import { MockGitHubRepository } from '../mocks/mockGitHubRepository'; import { MockTelemetry } from '../mocks/mockTelemetry'; +import { mockStackSetting } from '../mocks/mockStackSetting'; describe('Pull request stack selection', function () { let sinon: SinonSandbox; @@ -28,10 +29,12 @@ describe('Pull request stack selection', function () { let repository: MockGitHubRepository; let remote: GitHubRemote; let telemetry: MockTelemetry; + let setStacksEnabled: (enabled: boolean) => void; beforeEach(function () { sinon = createSandbox(); MockCommandRegistry.install(sinon); + setStacksEnabled = mockStackSetting(sinon); context = new MockExtensionContext(); telemetry = new MockTelemetry(); credentials = new CredentialStore(telemetry, context); @@ -194,6 +197,17 @@ describe('Pull request stack selection', function () { }); } + it('rejects stack creation when the feature is disabled', async function () { + setStacksEnabled(false); + const bottom = pullRequest(1, 'main', 'D1'); + const top = pullRequest(2, 'D1', 'D2'); + const candidate = { parentPullRequestNumber: bottom.number, size: 1, url: bottom.html_url }; + const add = sinon.stub(repository, 'addPullRequestsToStack'); + + await assert.rejects(addPullRequestsToStack([bottom, top], candidate), /stack features are disabled/); + assert(add.notCalled); + }); + it('rejects stale branch chains and PRs already in another stack before writing', async function () { const bottom = pullRequest(1, 'main', 'D1'); const top = pullRequest(2, 'D1', 'D2'); diff --git a/src/test/mocks/mockStackSetting.ts b/src/test/mocks/mockStackSetting.ts new file mode 100644 index 0000000000..33edbd38c6 --- /dev/null +++ b/src/test/mocks/mockStackSetting.ts @@ -0,0 +1,26 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as vscode from 'vscode'; +import { SinonSandbox } from 'sinon'; +import { EXPERIMENTAL_STACKS, PR_SETTINGS_NAMESPACE } from '../../common/settingKeys'; + +export function mockStackSetting(sandbox: SinonSandbox): (enabled: boolean) => void { + let enabled = true; + const getConfiguration = vscode.workspace.getConfiguration.bind(vscode.workspace); + sandbox.stub(vscode.workspace, 'getConfiguration').callsFake((section?: string, scope?: vscode.ConfigurationScope) => { + const configuration = getConfiguration(section, scope); + if (section !== PR_SETTINGS_NAMESPACE) { + return configuration; + } + const mock = Object.create(configuration) as vscode.WorkspaceConfiguration; + Object.defineProperty(mock, 'get', { + value: (key: string, defaultValue?: unknown) => + key === EXPERIMENTAL_STACKS ? enabled : configuration.get(key, defaultValue), + }); + return mock; + }); + return value => { enabled = value; }; +} diff --git a/src/test/mocks/mockTreeViewWorkbench.ts b/src/test/mocks/mockTreeViewWorkbench.ts index c08338ad6d..9053a67abf 100644 --- a/src/test/mocks/mockTreeViewWorkbench.ts +++ b/src/test/mocks/mockTreeViewWorkbench.ts @@ -7,13 +7,14 @@ import { default as assert } from 'assert'; import { SinonSandbox, SinonStub } from 'sinon'; import * as vscode from 'vscode'; -export function mockTreeViewWorkbench(sinon: SinonSandbox): SinonStub { - sinon.stub(vscode.commands, 'executeCommand').callsFake(async command => { +/** Reuse the returned stubs instead of wrapping the VS Code APIs again. */ +export function mockTreeViewWorkbench(sinon: SinonSandbox): { createTreeView: SinonStub; executeCommand: SinonStub } { + const executeCommand = sinon.stub(vscode.commands, 'executeCommand').callsFake(async command => { assert.strictEqual(command, 'setContext', 'Tree fixtures must not execute workbench commands'); return undefined; }); const noEvent: vscode.Event = () => new vscode.Disposable(() => { }); - return sinon.stub(vscode.window, 'createTreeView').callsFake((): vscode.TreeView => ({ + const createTreeView = sinon.stub(vscode.window, 'createTreeView').callsFake((): vscode.TreeView => ({ onDidExpandElement: noEvent, onDidCollapseElement: noEvent, onDidChangeSelection: noEvent, @@ -24,4 +25,5 @@ export function mockTreeViewWorkbench(sinon: SinonSandbox): SinonStub { reveal: async () => { }, dispose: () => { }, })); + return { createTreeView, executeCommand }; } diff --git a/src/test/view/prsTree.test.ts b/src/test/view/prsTree.test.ts index 6e4de4302a..a1e834bbe4 100644 --- a/src/test/view/prsTree.test.ts +++ b/src/test/view/prsTree.test.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as vscode from 'vscode'; -import { SinonSandbox, SinonSpy, createSandbox } from 'sinon'; +import { SinonSandbox, SinonStub, createSandbox } from 'sinon'; import { default as assert } from 'assert'; import { Octokit } from '@octokit/rest'; import { ApolloClient, ApolloLink, InMemoryCache } from 'apollo-boost'; @@ -39,6 +39,7 @@ import { GithubItemStateEnum, IAccount, ITeam, PullRequestMergeability } from '. import { asPromise } from '../../common/utils'; import { CreatePullRequestHelper } from '../../view/createPullRequestHelper'; import { MockThemeWatcher } from '../mocks/mockThemeWatcher'; +import { mockStackSetting } from '../mocks/mockStackSetting'; import { PrsTreeModel } from '../../view/prsTreeModel'; import { escapeMarkdownText } from '../../github/markdownUtils'; @@ -54,13 +55,16 @@ describe('GitHub Pull Requests view', function () { let mockNotificationsManager: MockNotificationManager; let prsTreeModel: PrsTreeModel; let discoveredRepository: MockGitHubRepository | undefined; - let createTreeView: SinonSpy; + let createTreeView: SinonStub; + let executeCommand: SinonStub; + let setStacksEnabled: (enabled: boolean) => void; beforeEach(function () { sinon = createSandbox(); discoveredRepository = undefined; MockCommandRegistry.install(sinon); - createTreeView = mockTreeViewWorkbench(sinon); + setStacksEnabled = mockStackSetting(sinon); + ({ createTreeView, executeCommand } = mockTreeViewWorkbench(sinon)); mockThemeWatcher = new MockThemeWatcher(); context = new MockExtensionContext(); @@ -103,6 +107,19 @@ describe('GitHub Pull Requests view', function () { sinon.stub(folderManager, 'createGitHubRepository').resolves(githubRepository); } + function stackablePullRequest(repository: MockGitHubRepository, number: number, base: string, head: string): PullRequestModel { + const remote = repository.remote; + const rest = new PullRequestBuilder().number(number) + .base(ref => ref.ref(base)).head(ref => ref.ref(head)).build(); + for (const ref of [rest.base, rest.head]) { + ref.repo.owner.login = remote.owner; + ref.repo.name = remote.repositoryName; + ref.repo.clone_url = `https://github.com/${remote.owner}/${remote.repositoryName}.git`; + } + return new PullRequestModel(credentialStore, telemetry, repository, remote, + convertRESTPullRequestToRawPullRequest(rest, repository)); + } + afterEach(function () { provider.dispose(); discoveredRepository?.dispose(); @@ -133,24 +150,92 @@ describe('GitHub Pull Requests view', function () { assert.strictEqual(options.canSelectMany, true); }); + it('disables multi-selection when created with stacks disabled', function () { + setStacksEnabled(false); + provider.dispose(); + provider = new PullRequestsTreeDataProvider(prsTreeModel, telemetry, context, reposManager); + + const tree = createTreeView.getCalls().filter(call => call.args[0] === 'pr:github').pop(); + assert(tree); + const options = tree.args[1] as { canSelectMany?: boolean }; + assert.strictEqual(options.canSelectMany, false); + }); + + it('applies multi-selection setting changes only when recreating the tree', function () { + const configurationChanged = new vscode.EventEmitter(); + context.subscriptions.push(configurationChanged); + sinon.stub(vscode.workspace, 'onDidChangeConfiguration').callsFake(configurationChanged.event); + provider.dispose(); + provider = new PullRequestsTreeDataProvider(prsTreeModel, telemetry, context, reposManager); + const event = { + affectsConfiguration: (section: string) => section === 'githubPullRequests.experimental.stacks', + }; + + for (const enabled of [false, true, false]) { + const view = provider.view; + const treeCount = createTreeView.getCalls().filter(call => call.args[0] === 'pr:github').length; + setStacksEnabled(enabled); + configurationChanged.fire(event); + assert.strictEqual(provider.view, view); + assert.strictEqual(createTreeView.getCalls().filter(call => call.args[0] === 'pr:github').length, treeCount); + + provider.dispose(); + provider = new PullRequestsTreeDataProvider(prsTreeModel, telemetry, context, reposManager); + const tree = createTreeView.getCalls().filter(call => call.args[0] === 'pr:github').pop(); + assert(tree); + const options = tree.args[1] as { canSelectMany?: boolean }; + assert.strictEqual(options.canSelectMany, enabled); + } + }); + + it('updates stack actions when the setting changes on an existing multi-select tree', function () { + const configurationChanged = new vscode.EventEmitter(); + context.subscriptions.push(configurationChanged); + sinon.stub(vscode.workspace, 'onDidChangeConfiguration').callsFake(configurationChanged.event); + provider.dispose(); + provider = new PullRequestsTreeDataProvider(prsTreeModel, telemetry, context, reposManager); + + const url = 'https://github.com/aaa/bbb'; + const remote = new GitHubRemote('origin', url, new Protocol(url), GitHubServerType.GitHubDotCom); + discoveredRepository = new MockGitHubRepository(remote, credentialStore, telemetry, sinon); + const selected = [ + stackablePullRequest(discoveredRepository, 1, 'main', 'D1'), + stackablePullRequest(discoveredRepository, 2, 'D1', 'D2'), + ].map(model => Object.assign(Object.create(PRNode.prototype), { pullRequestModel: model }) as PRNode); + sinon.stub(provider.view, 'selection').get(() => selected); + const treeCount = createTreeView.getCalls().filter(call => call.args[0] === 'pr:github').length; + const event = { + affectsConfiguration: (section: string) => section === 'githubPullRequests.experimental.stacks', + }; + + for (const enabled of [true, false, true]) { + setStacksEnabled(enabled); + executeCommand.resetHistory(); + configurationChanged.fire(event); + assert(executeCommand.calledOnceWithExactly('setContext', 'github:canAddToStack', enabled)); + } + + assert.strictEqual(createTreeView.getCalls().filter(call => call.args[0] === 'pr:github').length, treeCount); + }); + + it('does not offer or execute Add to Stack when stacks are disabled', async function () { + setStacksEnabled(false); + const showError = sinon.stub(vscode.window, 'showErrorMessage').resolves(undefined); + + (provider as any).updateCanAddToStack(); + await (provider as any).addSelectedPullRequestsToStack(undefined, undefined); + + assert(executeCommand.calledWith('setContext', 'github:canAddToStack', false)); + assert.match(showError.firstCall.args[0], /stack features are disabled/); + }); + it('refreshes selected and existing stack PR panels after adding from the tree', async function () { const url = 'https://github.com/aaa/bbb'; const remote = new GitHubRemote('origin', url, new Protocol(url), GitHubServerType.GitHubDotCom); const repository = new MockGitHubRepository(remote, credentialStore, telemetry, sinon); try { - const makePR = (number: number, base: string, head: string) => { - const rest = new PullRequestBuilder().number(number) - .base(ref => ref.ref(base)).head(ref => ref.ref(head)).build(); - for (const ref of [rest.base, rest.head]) { - ref.repo.owner.login = remote.owner; - ref.repo.name = remote.repositoryName; - ref.repo.clone_url = `${url}.git`; - } - return new PullRequestModel(credentialStore, telemetry, repository, remote, - convertRESTPullRequestToRawPullRequest(rest, repository)); - }; - const bottom = makePR(1, 'main', 'D1'); - const top = makePR(2, 'D1', 'D2'); + const bottom = stackablePullRequest(repository, 1, 'main', 'D1'); + const top = stackablePullRequest(repository, 2, 'D1', 'D2'); const node = (model: PullRequestModel) => Object.assign(Object.create(PRNode.prototype), { pullRequestModel: model }) as PRNode; const selected = [node(bottom), node(top)]; const existing = { diff --git a/src/view/prsTreeDataProvider.ts b/src/view/prsTreeDataProvider.ts index 9b3c737e8d..03ded633b6 100644 --- a/src/view/prsTreeDataProvider.ts +++ b/src/view/prsTreeDataProvider.ts @@ -14,7 +14,8 @@ import { commands, contexts } from '../common/executeCommands'; import { Disposable } from '../common/lifecycle'; import Logger from '../common/logger'; import { Remote } from '../common/remote'; -import { FILE_LIST_LAYOUT, GITHUB_ENTERPRISE, PR_SETTINGS_NAMESPACE, QUERIES, REMOTES, URI, URIS } from '../common/settingKeys'; +import { EXPERIMENTAL_STACKS, FILE_LIST_LAYOUT, GITHUB_ENTERPRISE, PR_SETTINGS_NAMESPACE, QUERIES, REMOTES, URI, URIS } from '../common/settingKeys'; +import { areStacksEnabled, assertStacksEnabled } from '../common/settingsUtils'; import { ITelemetry } from '../common/telemetry'; import { createPRNodeIdentifier } from '../common/uri'; import { formatError } from '../common/utils'; @@ -131,7 +132,7 @@ export class PullRequestsTreeDataProvider extends Disposable implements vscode.T this._view = this._register(vscode.window.createTreeView('pr:github', { treeDataProvider: this, showCollapseAll: true, - canSelectMany: true, + canSelectMany: areStacksEnabled(), manageCheckboxStateManually: true })); this._loginView = this._register(vscode.window.createTreeView('github:login', { @@ -146,11 +147,11 @@ export class PullRequestsTreeDataProvider extends Disposable implements vscode.T })); void commands.setContext(contexts.CAN_ADD_TO_STACK, false); - this._register(this._view.onDidChangeSelection(e => { - const selectedPRs = e.selection.filter((node): node is PRNode => node instanceof PRNode); - const stackable = selectedPRs.length === e.selection.length - && !!orderStackablePullRequests(selectedPRs.map(node => node.pullRequestModel)); - void commands.setContext(contexts.CAN_ADD_TO_STACK, stackable); + this._register(this._view.onDidChangeSelection(() => this.updateCanAddToStack())); + this._register(vscode.workspace.onDidChangeConfiguration(e => { + if (e.affectsConfiguration(`${PR_SETTINGS_NAMESPACE}.${EXPERIMENTAL_STACKS}`)) { + this.updateCanAddToStack(); + } })); this._register({ dispose: () => { void commands.setContext(contexts.CAN_ADD_TO_STACK, false); } }); this._register(vscode.commands.registerCommand('pr.addToStack', @@ -242,18 +243,19 @@ export class PullRequestsTreeDataProvider extends Disposable implements vscode.T } private async addSelectedPullRequestsToStack(clicked: PRNode, selected: TreeNode[] | undefined): Promise { - const selection = selected ?? this._view.selection; - if (!(clicked instanceof PRNode) || !Array.isArray(selection) || selection.length < 2 - || !selection.includes(clicked) || !selection.every(node => node instanceof PRNode)) { - void vscode.window.showErrorMessage(vscode.l10n.t('Select at least two pull requests in the Pull Requests view to add them to a stack.')); - return; - } - const ordered = orderStackablePullRequests(selection.map(node => (node as PRNode).pullRequestModel)); - if (!ordered) { - void vscode.window.showErrorMessage(vscode.l10n.t('Selected pull requests must be open and have matching head and base branches in the same repository.')); - return; - } try { + assertStacksEnabled(); + const selection = selected ?? this._view.selection; + if (!(clicked instanceof PRNode) || !Array.isArray(selection) || selection.length < 2 + || !selection.includes(clicked) || !selection.every(node => node instanceof PRNode)) { + void vscode.window.showErrorMessage(vscode.l10n.t('Select at least two pull requests in the Pull Requests view to add them to a stack.')); + return; + } + const ordered = orderStackablePullRequests(selection.map(node => (node as PRNode).pullRequestModel)); + if (!ordered) { + void vscode.window.showErrorMessage(vscode.l10n.t('Selected pull requests must be open and have matching head and base branches in the same repository.')); + return; + } const bottom = ordered[0]; const candidate = await bottom.githubRepository.getStackCandidate(bottom.head!.ref); if (!candidate || candidate.parentPullRequestNumber !== bottom.number) { @@ -281,6 +283,14 @@ export class PullRequestsTreeDataProvider extends Disposable implements vscode.T } } + private updateCanAddToStack(): void { + const selection = this._view.selection; + const selectedPRs = selection.filter((node): node is PRNode => node instanceof PRNode); + const stackable = areStacksEnabled() && selectedPRs.length === selection.length + && !!orderStackablePullRequests(selectedPRs.map(node => node.pullRequestModel)); + void commands.setContext(contexts.CAN_ADD_TO_STACK, stackable); + } + private filterNotificationsToKnown(notifications: PullRequestModel[]): PullRequestModel[] { return notifications.filter(notification => { if (!this.prsTreeModel.hasPullRequest(notification)) {