Skip to content

Commit 2cd62f1

Browse files
committed
CCR
1 parent 18c836f commit 2cd62f1

9 files changed

Lines changed: 148 additions & 50 deletions

‎package.nls.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"displayName": "GitHub Pull Requests",
33
"description": "Pull Request and Issue Provider for GitHub",
44
"githubPullRequests.pullRequestDescription.description": "The description used when creating pull requests.",
5-
"githubPullRequests.experimental.stacks.description": "Enable experimental pull request stack features.",
5+
"githubPullRequests.experimental.stacks.description": "Enable experimental pull request stack features. Reload the window to apply changes to multi-selection in the Pull Requests view.",
66
"githubPullRequests.pullRequestDescription.template": "Use a pull request template and commit description, or just use the commit description if no templates were found.",
77
"githubPullRequests.pullRequestDescription.commit": "Use the latest commit message only.",
88
"githubPullRequests.pullRequestDescription.branchName": "Use the branch name as the pull request title",

‎src/common/settingsUtils.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,12 @@ export function areStacksEnabled(): boolean {
1212
return vscode.workspace.getConfiguration(PR_SETTINGS_NAMESPACE).get<boolean>(EXPERIMENTAL_STACKS, false);
1313
}
1414

15+
export function assertStacksEnabled(): void {
16+
if (!areStacksEnabled()) {
17+
throw new Error(vscode.l10n.t('Pull request stack features are disabled.'));
18+
}
19+
}
20+
1521
export function getReviewMode(): { merged: boolean, closed: boolean } {
1622
const desktopDefaults = { merged: false, closed: false };
1723
const config = vscode.workspace.getConfiguration(PR_SETTINGS_NAMESPACE)

‎src/github/createPRViewProvider.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ import {
3939
PUSH_BRANCH,
4040
SHOW_CREATE_PULL_REQUEST_CANCEL_CONFIRMATION
4141
} from '../common/settingKeys';
42-
import { areStacksEnabled } from '../common/settingsUtils';
42+
import { areStacksEnabled, assertStacksEnabled } from '../common/settingsUtils';
4343
import { ITelemetry } from '../common/telemetry';
4444
import { toOpenPullRequestWebviewUri } from '../common/uri';
4545
import { asPromise, compareIgnoreCase, formatError, promiseWithTimeout } from '../common/utils';
@@ -1562,9 +1562,7 @@ Don't forget to commit your template file to the repository so that it can be us
15621562
try {
15631563
let stackCandidate: StackCandidate | undefined;
15641564
if (message.args.addToStack) {
1565-
if (!areStacksEnabled()) {
1566-
throw new Error(vscode.l10n.t('Pull request stack features are disabled.'));
1567-
}
1565+
assertStacksEnabled();
15681566
if (message.args.autoMerge) {
15691567
throw new Error(vscode.l10n.t('Auto-merge is not available for stacked pull requests.'));
15701568
}
@@ -1685,9 +1683,7 @@ Don't forget to commit your template file to the repository so that it can be us
16851683
}
16861684
if (stackCandidate) {
16871685
try {
1688-
if (!areStacksEnabled()) {
1689-
throw new Error(vscode.l10n.t('Pull request stack features are disabled.'));
1690-
}
1686+
assertStacksEnabled();
16911687
await createdPR.githubRepository.addPullRequestToStack(stackCandidate, createdPR.number);
16921688
} catch (error) {
16931689
stackAdditionFailed = true;

‎src/github/pullRequestOverview.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ import { openWithDefaultExternalOpener } from '../common/externalUri';
3838
import { disposeAll } from '../common/lifecycle';
3939
import Logger from '../common/logger';
4040
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';
41-
import { areStacksEnabled } from '../common/settingsUtils';
41+
import { areStacksEnabled, assertStacksEnabled } from '../common/settingsUtils';
4242
import { ITelemetry } from '../common/telemetry';
4343
import { EventType, ReviewEvent, SessionLinkInfo, TimelineEvent } from '../common/timelineEvent';
4444
import { toOpenIssueWebviewUri, toOpenPullRequestWebviewUri } from '../common/uri';
@@ -1132,9 +1132,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
11321132

11331133
private async unstackAll(message: IRequestMessage<undefined>): Promise<void> {
11341134
try {
1135-
if (!areStacksEnabled()) {
1136-
throw new Error(vscode.l10n.t('Pull request stack features are disabled.'));
1137-
}
1135+
assertStacksEnabled();
11381136
const access = await this._folderRepositoryManager.getPullRequestRepositoryAccessAndMergeMethods(this._item);
11391137
if (!access.hasWritePermission) {
11401138
throw new Error(vscode.l10n.t('You do not have permission to unstack these pull requests.'));

‎src/github/pullRequestReviewCommon.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { PullRequestModel } from './pullRequestModel';
1212
import { ConvertToDraftReply, PullRequest, ReadyForReviewReply, ReviewType, StackMergeResult, SubmitReviewReply } from './views';
1313
import Logger from '../common/logger';
1414
import { DEFAULT_DELETION_METHOD, DELETE_BRANCH_AFTER_MERGE, PR_SETTINGS_NAMESPACE, SELECT_LOCAL_BRANCH, SELECT_REMOTE, SELECT_WORKTREE } from '../common/settingKeys';
15-
import { areStacksEnabled } from '../common/settingsUtils';
15+
import { assertStacksEnabled } from '../common/settingsUtils';
1616
import { ReviewEvent, TimelineEvent } from '../common/timelineEvent';
1717
import { Schemes } from '../common/uri';
1818
import { formatError } from '../common/utils';
@@ -38,9 +38,7 @@ export interface ReviewContext {
3838
export namespace PullRequestReviewCommon {
3939
export async function mergeStack(ctx: ReviewContext, message: IRequestMessage<{ method: MergeMethod }>): Promise<void> {
4040
try {
41-
if (!areStacksEnabled()) {
42-
throw new Error(vscode.l10n.t('Pull request stack features are disabled.'));
43-
}
41+
assertStacksEnabled();
4442
const { item, folderRepositoryManager } = ctx;
4543
const stack = await item.getStack();
4644
if (!stack) {

‎src/github/pullRequestStack.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import { GithubItemStateEnum } from './interface';
77
import { PullRequestModel } from './pullRequestModel';
88
import { StackCandidate } from '../../common/views';
9-
import { areStacksEnabled } from '../common/settingsUtils';
9+
import { assertStacksEnabled } from '../common/settingsUtils';
1010
import { compareIgnoreCase } from '../common/utils';
1111

1212
function sameRepository(first: PullRequestModel, second: PullRequestModel): boolean {
@@ -56,9 +56,7 @@ export function orderStackablePullRequests(pullRequests: readonly PullRequestMod
5656
}
5757

5858
export async function addPullRequestsToStack(pullRequests: readonly PullRequestModel[], confirmedCandidate: StackCandidate): Promise<number[]> {
59-
if (!areStacksEnabled()) {
60-
throw new Error('Pull request stack features are disabled.');
61-
}
59+
assertStacksEnabled();
6260

6361
const initial = orderStackablePullRequests(pullRequests);
6462
if (!initial) {
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
import { default as assert } from 'assert';
7+
import * as vscode from 'vscode';
8+
import { createSandbox, SinonSandbox } from 'sinon';
9+
import { areStacksEnabled, assertStacksEnabled } from '../../common/settingsUtils';
10+
import { mockStackSetting } from '../mocks/mockStackSetting';
11+
12+
describe('Stack settings', function () {
13+
let sinon: SinonSandbox;
14+
let setStacksEnabled: (enabled: boolean) => void;
15+
16+
beforeEach(function () {
17+
sinon = createSandbox();
18+
setStacksEnabled = mockStackSetting(sinon);
19+
});
20+
21+
afterEach(function () {
22+
sinon.restore();
23+
});
24+
25+
it('checks the current setting on every assertion', function () {
26+
assert.strictEqual(areStacksEnabled(), true);
27+
assert.doesNotThrow(() => assertStacksEnabled());
28+
29+
setStacksEnabled(false);
30+
assert.strictEqual(areStacksEnabled(), false);
31+
assert.throws(() => assertStacksEnabled(), /Pull request stack features are disabled/);
32+
33+
setStacksEnabled(true);
34+
assert.doesNotThrow(() => assertStacksEnabled());
35+
});
36+
37+
it('localizes the disabled-feature error', function () {
38+
setStacksEnabled(false);
39+
const localize = sinon.stub(vscode.l10n, 't').returns('Localized stacks-disabled message.');
40+
41+
assert.throws(() => assertStacksEnabled(), { message: 'Localized stacks-disabled message.' });
42+
assert(localize.calledOnce);
43+
assert.deepStrictEqual(localize.firstCall.args, ['Pull request stack features are disabled.']);
44+
});
45+
});

‎src/test/view/prsTree.test.ts‎

Lines changed: 74 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,19 @@ describe('GitHub Pull Requests view', function () {
106106
sinon.stub(folderManager, 'createGitHubRepository').resolves(githubRepository);
107107
}
108108

109+
function stackablePullRequest(repository: MockGitHubRepository, number: number, base: string, head: string): PullRequestModel {
110+
const remote = repository.remote;
111+
const rest = new PullRequestBuilder().number(number)
112+
.base(ref => ref.ref(base)).head(ref => ref.ref(head)).build();
113+
for (const ref of [rest.base, rest.head]) {
114+
ref.repo.owner.login = remote.owner;
115+
ref.repo.name = remote.repositoryName;
116+
ref.repo.clone_url = `https://github.com/${remote.owner}/${remote.repositoryName}.git`;
117+
}
118+
return new PullRequestModel(credentialStore, telemetry, repository, remote,
119+
convertRESTPullRequestToRawPullRequest(rest, repository));
120+
}
121+
109122
afterEach(function () {
110123
provider.dispose();
111124
discoveredRepository?.dispose();
@@ -136,7 +149,7 @@ describe('GitHub Pull Requests view', function () {
136149
assert.strictEqual(options.canSelectMany, true);
137150
});
138151

139-
it('does not enable multi-selection when stacks are disabled', function () {
152+
it('disables multi-selection when created with stacks disabled', function () {
140153
setStacksEnabled(false);
141154
provider.dispose();
142155
provider = new PullRequestsTreeDataProvider(prsTreeModel, telemetry, context, reposManager);
@@ -147,6 +160,64 @@ describe('GitHub Pull Requests view', function () {
147160
assert.strictEqual(options.canSelectMany, false);
148161
});
149162

163+
it('applies multi-selection setting changes only when recreating the tree', function () {
164+
const configurationChanged = new vscode.EventEmitter<vscode.ConfigurationChangeEvent>();
165+
context.subscriptions.push(configurationChanged);
166+
sinon.stub(vscode.workspace, 'onDidChangeConfiguration').callsFake(configurationChanged.event);
167+
provider.dispose();
168+
provider = new PullRequestsTreeDataProvider(prsTreeModel, telemetry, context, reposManager);
169+
const event = {
170+
affectsConfiguration: (section: string) => section === 'githubPullRequests.experimental.stacks',
171+
};
172+
173+
for (const enabled of [false, true, false]) {
174+
const view = provider.view;
175+
const treeCount = createTreeView.getCalls().filter(call => call.args[0] === 'pr:github').length;
176+
setStacksEnabled(enabled);
177+
configurationChanged.fire(event);
178+
assert.strictEqual(provider.view, view);
179+
assert.strictEqual(createTreeView.getCalls().filter(call => call.args[0] === 'pr:github').length, treeCount);
180+
181+
provider.dispose();
182+
provider = new PullRequestsTreeDataProvider(prsTreeModel, telemetry, context, reposManager);
183+
const tree = createTreeView.getCalls().filter(call => call.args[0] === 'pr:github').pop();
184+
assert(tree);
185+
const options = tree.args[1] as { canSelectMany?: boolean };
186+
assert.strictEqual(options.canSelectMany, enabled);
187+
}
188+
});
189+
190+
it('updates stack actions when the setting changes on an existing multi-select tree', function () {
191+
const configurationChanged = new vscode.EventEmitter<vscode.ConfigurationChangeEvent>();
192+
context.subscriptions.push(configurationChanged);
193+
sinon.stub(vscode.workspace, 'onDidChangeConfiguration').callsFake(configurationChanged.event);
194+
provider.dispose();
195+
provider = new PullRequestsTreeDataProvider(prsTreeModel, telemetry, context, reposManager);
196+
197+
const url = 'https://github.com/aaa/bbb';
198+
const remote = new GitHubRemote('origin', url, new Protocol(url), GitHubServerType.GitHubDotCom);
199+
discoveredRepository = new MockGitHubRepository(remote, credentialStore, telemetry, sinon);
200+
const selected = [
201+
stackablePullRequest(discoveredRepository, 1, 'main', 'D1'),
202+
stackablePullRequest(discoveredRepository, 2, 'D1', 'D2'),
203+
].map(model => Object.assign(Object.create(PRNode.prototype), { pullRequestModel: model }) as PRNode);
204+
sinon.stub(provider.view, 'selection').get(() => selected);
205+
const executeCommand = sinon.spy(vscode.commands, 'executeCommand');
206+
const treeCount = createTreeView.getCalls().filter(call => call.args[0] === 'pr:github').length;
207+
const event = {
208+
affectsConfiguration: (section: string) => section === 'githubPullRequests.experimental.stacks',
209+
};
210+
211+
for (const enabled of [true, false, true]) {
212+
setStacksEnabled(enabled);
213+
executeCommand.resetHistory();
214+
configurationChanged.fire(event);
215+
assert(executeCommand.calledOnceWithExactly('setContext', 'github:canAddToStack', enabled));
216+
}
217+
218+
assert.strictEqual(createTreeView.getCalls().filter(call => call.args[0] === 'pr:github').length, treeCount);
219+
});
220+
150221
it('does not offer or execute Add to Stack when stacks are disabled', async function () {
151222
setStacksEnabled(false);
152223
const showError = sinon.stub(vscode.window, 'showErrorMessage').resolves(undefined);
@@ -164,19 +235,8 @@ describe('GitHub Pull Requests view', function () {
164235
const remote = new GitHubRemote('origin', url, new Protocol(url), GitHubServerType.GitHubDotCom);
165236
const repository = new MockGitHubRepository(remote, credentialStore, telemetry, sinon);
166237
try {
167-
const makePR = (number: number, base: string, head: string) => {
168-
const rest = new PullRequestBuilder().number(number)
169-
.base(ref => ref.ref(base)).head(ref => ref.ref(head)).build();
170-
for (const ref of [rest.base, rest.head]) {
171-
ref.repo.owner.login = remote.owner;
172-
ref.repo.name = remote.repositoryName;
173-
ref.repo.clone_url = `${url}.git`;
174-
}
175-
return new PullRequestModel(credentialStore, telemetry, repository, remote,
176-
convertRESTPullRequestToRawPullRequest(rest, repository));
177-
};
178-
const bottom = makePR(1, 'main', 'D1');
179-
const top = makePR(2, 'D1', 'D2');
238+
const bottom = stackablePullRequest(repository, 1, 'main', 'D1');
239+
const top = stackablePullRequest(repository, 2, 'D1', 'D2');
180240
const node = (model: PullRequestModel) => Object.assign(Object.create(PRNode.prototype), { pullRequestModel: model }) as PRNode;
181241
const selected = [node(bottom), node(top)];
182242
const existing = {

‎src/view/prsTreeDataProvider.ts‎

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { Disposable } from '../common/lifecycle';
1515
import Logger from '../common/logger';
1616
import { Remote } from '../common/remote';
1717
import { EXPERIMENTAL_STACKS, FILE_LIST_LAYOUT, GITHUB_ENTERPRISE, PR_SETTINGS_NAMESPACE, QUERIES, REMOTES, URI, URIS } from '../common/settingKeys';
18-
import { areStacksEnabled } from '../common/settingsUtils';
18+
import { areStacksEnabled, assertStacksEnabled } from '../common/settingsUtils';
1919
import { ITelemetry } from '../common/telemetry';
2020
import { createPRNodeIdentifier } from '../common/uri';
2121
import { formatError } from '../common/utils';
@@ -243,22 +243,19 @@ export class PullRequestsTreeDataProvider extends Disposable implements vscode.T
243243
}
244244

245245
private async addSelectedPullRequestsToStack(clicked: PRNode, selected: TreeNode[] | undefined): Promise<void> {
246-
if (!areStacksEnabled()) {
247-
void vscode.window.showErrorMessage(vscode.l10n.t('Pull request stack features are disabled.'));
248-
return;
249-
}
250-
const selection = selected ?? this._view.selection;
251-
if (!(clicked instanceof PRNode) || !Array.isArray(selection) || selection.length < 2
252-
|| !selection.includes(clicked) || !selection.every(node => node instanceof PRNode)) {
253-
void vscode.window.showErrorMessage(vscode.l10n.t('Select at least two pull requests in the Pull Requests view to add them to a stack.'));
254-
return;
255-
}
256-
const ordered = orderStackablePullRequests(selection.map(node => (node as PRNode).pullRequestModel));
257-
if (!ordered) {
258-
void vscode.window.showErrorMessage(vscode.l10n.t('Selected pull requests must be open and have matching head and base branches in the same repository.'));
259-
return;
260-
}
261246
try {
247+
assertStacksEnabled();
248+
const selection = selected ?? this._view.selection;
249+
if (!(clicked instanceof PRNode) || !Array.isArray(selection) || selection.length < 2
250+
|| !selection.includes(clicked) || !selection.every(node => node instanceof PRNode)) {
251+
void vscode.window.showErrorMessage(vscode.l10n.t('Select at least two pull requests in the Pull Requests view to add them to a stack.'));
252+
return;
253+
}
254+
const ordered = orderStackablePullRequests(selection.map(node => (node as PRNode).pullRequestModel));
255+
if (!ordered) {
256+
void vscode.window.showErrorMessage(vscode.l10n.t('Selected pull requests must be open and have matching head and base branches in the same repository.'));
257+
return;
258+
}
262259
const bottom = ordered[0];
263260
const candidate = await bottom.githubRepository.getStackCandidate(bottom.head!.ref);
264261
if (!candidate || candidate.parentPullRequestNumber !== bottom.number) {

0 commit comments

Comments
 (0)