diff --git a/src/commands.ts b/src/commands.ts index e67fe8a32b..00021f7fdc 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -10,7 +10,6 @@ import { Repository } from './api/api'; import { GitErrorCodes } from './api/api1'; import { CommentReply, findActiveHandler, resolveCommentHandler } from './commentHandlerResolver'; import { commands } from './common/executeCommands'; -import { openWithDefaultExternalOpener } from './common/externalUri'; import Logger from './common/logger'; import { FILE_LIST_LAYOUT, HIDE_VIEWED_FILES, PR_SETTINGS_NAMESPACE } from './common/settingKeys'; import { editQuery } from './common/settingsUtils'; @@ -28,6 +27,7 @@ import { GitHubRepository } from './github/githubRepository'; import { Issue } from './github/interface'; import { IssueModel } from './github/issueModel'; import { IssueOverviewPanel } from './github/issueOverview'; +import { openIssueOrPullRequestOnGitHub } from './github/openOnGitHub'; import { GHPRComment, GHPRCommentThread, TemporaryComment } from './github/prComment'; import { PullRequestModel } from './github/pullRequestModel'; import { PullRequestOverviewPanel } from './github/pullRequestOverview'; @@ -103,26 +103,21 @@ export async function openDescription( } } -export async function openPullRequestOnGitHub(e: PRNode | RepositoryChangesNode | IssueModel | NotificationTreeItem, telemetry: ITelemetry) { - let url: string; +export function openItemOnGitHub(e: PRNode | RepositoryChangesNode | IssueModel | NotificationTreeItem, telemetry: ITelemetry): Thenable { + let item: IssueModel; if (e instanceof PRNode || e instanceof RepositoryChangesNode) { - url = e.pullRequestModel.html_url; + item = e.pullRequestModel; } else if (isNotificationTreeItem(e)) { - url = e.model.html_url; + item = e.model; } else { - url = e.html_url; + item = e; } - openPullRequestUrlOnGitHub(vscode.Uri.parse(url), telemetry); -} - -function openPullRequestUrlOnGitHub(url: vscode.Uri, telemetry: ITelemetry): void { - openWithDefaultExternalOpener(url); - - /** __GDPR__ - "pr.openInGitHub" : {} - */ - telemetry.sendTelemetryEvent('pr.openInGitHub'); + return openIssueOrPullRequestOnGitHub( + vscode.Uri.parse(item.html_url), + item instanceof PullRequestModel ? 'pullRequest' : 'issue', + telemetry, + ); } export async function closeAllPrAndReviewEditors() { @@ -157,7 +152,7 @@ export async function openPullRequestOnGitHubCommand( if (!e || e instanceof vscode.Uri) { const currentPullRequestUrl = PullRequestOverviewPanel.getCurrentPullRequestUrl(); if (currentPullRequestUrl) { - openPullRequestUrlOnGitHub(currentPullRequestUrl, telemetry); + openIssueOrPullRequestOnGitHub(currentPullRequestUrl, 'pullRequest', telemetry); return; } @@ -171,11 +166,11 @@ export async function openPullRequestOnGitHubCommand( itemValue => ({ label: itemValue.html_url }), ); if (result) { - openPullRequestOnGitHub(result, telemetry); + openItemOnGitHub(result, telemetry); } } } else { - openPullRequestOnGitHub(e, telemetry); + openItemOnGitHub(e, telemetry); } } @@ -209,7 +204,7 @@ export function registerCommands( 'notification.openOnGitHub', async (e: NotificationTreeItem | undefined) => { if (e) { - openPullRequestOnGitHub(e, telemetry); + openItemOnGitHub(e, telemetry); } }, ), @@ -1043,7 +1038,7 @@ export function registerCommands( const showMergeOnGitHub = isCrossRepository && isInCodespaces(); if (showMergeOnGitHub) { - return openPullRequestOnGitHub(pullRequest, telemetry); + return openItemOnGitHub(pullRequest, telemetry); } const yes = vscode.l10n.t('Yes'); diff --git a/src/github/activityBarViewProvider.ts b/src/github/activityBarViewProvider.ts index 7595d5111e..ae559e5462 100644 --- a/src/github/activityBarViewProvider.ts +++ b/src/github/activityBarViewProvider.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as vscode from 'vscode'; -import { openPullRequestOnGitHub } from '../commands'; +import { openItemOnGitHub } from '../commands'; import { addAttestationCommit, isAttestationCommitsEnabled } from './attestationCommit'; import { FolderRepositoryManager } from './folderRepositoryManager'; import { GithubItemStateEnum, IAccount, MergeMethod, ReviewEventEnum, ReviewState } from './interface'; @@ -47,10 +47,10 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W this._register(vscode.commands.registerCommand('review.comment', (e: { body: string }) => this.submitReviewCommand(e))); this._register(vscode.commands.registerCommand('review.requestChanges', (e: { body: string }) => this.requestChangesCommand(e))); this._register(vscode.commands.registerCommand('review.approveOnDotCom', () => { - return openPullRequestOnGitHub(this._item, this._folderRepositoryManager.telemetry); + return openItemOnGitHub(this._item, this._folderRepositoryManager.telemetry); })); this._register(vscode.commands.registerCommand('review.requestChangesOnDotCom', () => { - return openPullRequestOnGitHub(this._item, this._folderRepositoryManager.telemetry); + return openItemOnGitHub(this._item, this._folderRepositoryManager.telemetry); })); } @@ -102,7 +102,7 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W case 'pr.submit': return this.submitReviewMessage(message); case 'pr.openOnGitHub': - return openPullRequestOnGitHub(this._item, this._folderRepositoryManager.telemetry); + return openItemOnGitHub(this._item, this._folderRepositoryManager.telemetry); case 'pr.checkout-default-branch': return this.checkoutDefaultBranch(message); case 'pr.update-branch': diff --git a/src/github/issueOverview.ts b/src/github/issueOverview.ts index 69c702bb17..4a7ebf8182 100644 --- a/src/github/issueOverview.ts +++ b/src/github/issueOverview.ts @@ -6,7 +6,7 @@ import * as vscode from 'vscode'; import { CloseResult, OpenLocalFileArgs } from '../../common/views'; -import { openPullRequestOnGitHub } from '../commands'; +import { openItemOnGitHub } from '../commands'; import { decodeBase64, guessExtensionFromMime, pickFilesForUpload, placeholdersForNames, runFileUploads, runPendingUploads } from './fileUpload'; import { FolderRepositoryManager } from './folderRepositoryManager'; import { GithubItemStateEnum, IAccount, IMilestone, IProject, IProjectItem, RepoAccessAndMergeMethods } from './interface'; @@ -452,7 +452,7 @@ export class IssueOverviewPanel extends W case 'pr.copy-vscodedevlink': return this.copyVscodeDevLink(); case 'pr.openOnGitHub': - return openPullRequestOnGitHub(this._item, this._telemetry); + return openItemOnGitHub(this._item, this._telemetry); case 'pr.open-local-file': return this.openLocalFile(message); case 'pr.debug': diff --git a/src/github/openOnGitHub.ts b/src/github/openOnGitHub.ts new file mode 100644 index 0000000000..8e1433dd17 --- /dev/null +++ b/src/github/openOnGitHub.ts @@ -0,0 +1,29 @@ +/*--------------------------------------------------------------------------------------------- + * 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 { openWithDefaultExternalOpener } from '../common/externalUri'; +import { ITelemetry } from '../common/telemetry'; + +export type GitHubItemKind = 'issue' | 'pullRequest'; + +export function openIssueOrPullRequestOnGitHub( + uri: vscode.Uri, + kind: GitHubItemKind, + telemetry: ITelemetry, +): Thenable { + if (kind === 'pullRequest') { + /* __GDPR__ + "pr.openInGitHub" : {} + */ + telemetry.sendTelemetryEvent('pr.openInGitHub'); + } else { + /* __GDPR__ + "issue.openOnGitHub" : {} + */ + telemetry.sendTelemetryEvent('issue.openOnGitHub'); + } + return openWithDefaultExternalOpener(uri); +} diff --git a/src/github/pullRequestModel.ts b/src/github/pullRequestModel.ts index 0fc748ba29..9d13bc4adf 100644 --- a/src/github/pullRequestModel.ts +++ b/src/github/pullRequestModel.ts @@ -68,6 +68,7 @@ import { } from './interface'; import { IssueChangeEvent, IssueModel } from './issueModel'; import { compareCommits, getErrorCode, GraphQLError, GraphQLErrorType } from './loggingOctokit'; +import { openIssueOrPullRequestOnGitHub } from './openOnGitHub'; import { convertRESTPullRequestToRawPullRequest, convertRESTReviewEvent, @@ -89,7 +90,6 @@ import { Repository } from '../api/api'; import { COPILOT_ACCOUNTS, DiffSide, IComment, IReviewThread, SubjectType, ViewedState } from '../common/comment'; import { getGitChangeType, getModifiedContentFromDiffHunk, parseDiff } from '../common/diffHunk'; import { commands } from '../common/executeCommands'; -import { openWithDefaultExternalOpener } from '../common/externalUri'; import { GitChangeType, InMemFileChange, SlimFileChange } from '../common/file'; import { GitHubRef } from '../common/githubRef'; import Logger from '../common/logger'; @@ -319,7 +319,7 @@ export class PullRequestModel extends IssueModel implements IPullRe const openString = vscode.l10n.t('Open on GitHub'); vscode.window.showWarningMessage(message, openString).then(action => { if (action && action === openString) { - openWithDefaultExternalOpener(vscode.Uri.parse(this.html_url)); + openIssueOrPullRequestOnGitHub(vscode.Uri.parse(this.html_url), 'pullRequest', this._telemetry); } }); diff --git a/src/github/pullRequestOverview.ts b/src/github/pullRequestOverview.ts index 5c12e7ab8f..d961f1cd30 100644 --- a/src/github/pullRequestOverview.ts +++ b/src/github/pullRequestOverview.ts @@ -7,7 +7,7 @@ import * as crypto from 'crypto'; import * as vscode from 'vscode'; import { OpenCommitChangesArgs, OpenLocalFileArgs } from '../../common/views'; -import { openPullRequestOnGitHub } from '../commands'; +import { openItemOnGitHub } from '../commands'; import { addAttestationCommit, isAttestationCommitsEnabled } from './attestationCommit'; import { getCopilotApi } from './copilotApi'; import { SessionIdForPr } from './copilotRemoteAgent'; @@ -216,13 +216,13 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel { const panel = PullRequestOverviewPanel.findPanel(ctx.owner, ctx.repo, ctx.number); if (panel) { - return openPullRequestOnGitHub(panel._item, telemetry); + return openItemOnGitHub(panel._item, telemetry); } }), vscode.commands.registerCommand('review.requestChangesOnDotComDescription', (ctx: ReviewCommentContext) => { const panel = PullRequestOverviewPanel.findPanel(ctx.owner, ctx.repo, ctx.number); if (panel) { - return openPullRequestOnGitHub(panel._item, telemetry); + return openItemOnGitHub(panel._item, telemetry); } }), ); diff --git a/src/issues/issueFeatureRegistrar.ts b/src/issues/issueFeatureRegistrar.ts index c0edf0e1e7..4f17c809f3 100644 --- a/src/issues/issueFeatureRegistrar.ts +++ b/src/issues/issueFeatureRegistrar.ts @@ -65,6 +65,8 @@ import { FolderRepositoryManager, PullRequestDefaults } from '../github/folderRe import { IProject } from '../github/interface'; import { IssueModel } from '../github/issueModel'; import { IssueOverviewPanel } from '../github/issueOverview'; +import { openIssueOrPullRequestOnGitHub } from '../github/openOnGitHub'; +import { PullRequestModel } from '../github/pullRequestModel'; import { RepositoriesManager } from '../github/repositoriesManager'; import { ISSUE_OR_URL_EXPRESSION, parseIssueExpressionOutput } from '../github/utils'; import { ReviewManager } from '../view/reviewManager'; @@ -332,12 +334,7 @@ export class IssueFeatureRegistrar extends Disposable { return; } - vscode.commands.executeCommand('vscode.open', vscode.Uri.parse(issue.html_url)); - - /* __GDPR__ - "issue.openOnGitHub" : {} - */ - this.telemetry.sendTelemetryEvent('issue.openOnGitHub'); + return this.openIssue(issue); }), ); this._register( @@ -828,7 +825,11 @@ export class IssueFeatureRegistrar extends Disposable { openIssue(issueModel: any) { if (issueModel instanceof IssueModel) { - return vscode.env.openExternal(vscode.Uri.parse(issueModel.html_url)); + return openIssueOrPullRequestOnGitHub( + vscode.Uri.parse(issueModel.html_url), + issueModel instanceof PullRequestModel ? 'pullRequest' : 'issue', + this.telemetry, + ); } return undefined; } diff --git a/src/test/github/openOnGitHub.test.ts b/src/test/github/openOnGitHub.test.ts new file mode 100644 index 0000000000..6f6a36eaf1 --- /dev/null +++ b/src/test/github/openOnGitHub.test.ts @@ -0,0 +1,88 @@ +/*--------------------------------------------------------------------------------------------- + * 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 { openItemOnGitHub } from '../../commands'; +import { IssueModel } from '../../github/issueModel'; +import { openIssueOrPullRequestOnGitHub } from '../../github/openOnGitHub'; +import { PullRequestModel } from '../../github/pullRequestModel'; +import { NotificationTreeItem } from '../../notifications/notificationItem'; +import { MockTelemetry } from '../mocks/mockTelemetry'; + +describe('openIssueOrPullRequestOnGitHub', () => { + let openExternal: SinonStub; + let sandbox: SinonSandbox; + let telemetry: MockTelemetry; + + beforeEach(() => { + sandbox = createSandbox(); + telemetry = new MockTelemetry(); + openExternal = sandbox.stub(vscode.env, 'openExternal').resolves(true); + }); + + afterEach(() => { + sandbox.restore(); + }); + + it('sends pull request telemetry', async () => { + const sendTelemetryEvent = sandbox.spy(telemetry, 'sendTelemetryEvent'); + const uri = vscode.Uri.parse('https://github.com/microsoft/vscode/pull/1'); + + await openIssueOrPullRequestOnGitHub(uri, 'pullRequest', telemetry); + + assert.ok(sendTelemetryEvent.calledOnceWith('pr.openInGitHub')); + assert.ok(openExternal.calledOnceWith(uri, { allowContributedOpeners: 'default' })); + }); + + it('sends issue telemetry', async () => { + const sendTelemetryEvent = sandbox.spy(telemetry, 'sendTelemetryEvent'); + const uri = vscode.Uri.parse('https://github.com/microsoft/vscode/issues/1'); + + await openIssueOrPullRequestOnGitHub(uri, 'issue', telemetry); + + assert.ok(sendTelemetryEvent.calledOnceWith('issue.openOnGitHub')); + assert.ok(openExternal.calledOnceWith(uri, { allowContributedOpeners: 'default' })); + }); + + it('classifies pull request models', async () => { + const sendTelemetryEvent = sandbox.spy(telemetry, 'sendTelemetryEvent'); + const pullRequest: PullRequestModel = Object.assign(Object.create(PullRequestModel.prototype), { + html_url: 'https://github.com/microsoft/vscode/pull/1', + }); + + await openItemOnGitHub(pullRequest, telemetry); + + assert.ok(sendTelemetryEvent.calledOnceWith('pr.openInGitHub')); + }); + + it('classifies issue models', async () => { + const sendTelemetryEvent = sandbox.spy(telemetry, 'sendTelemetryEvent'); + const issue: IssueModel = Object.assign(Object.create(IssueModel.prototype), { + html_url: 'https://github.com/microsoft/vscode/issues/1', + }); + + await openItemOnGitHub(issue, telemetry); + + assert.ok(sendTelemetryEvent.calledOnceWith('issue.openOnGitHub')); + }); + + it('classifies pull request notifications', async () => { + const sendTelemetryEvent = sandbox.spy(telemetry, 'sendTelemetryEvent'); + const pullRequest: PullRequestModel = Object.assign(Object.create(PullRequestModel.prototype), { + html_url: 'https://github.com/microsoft/vscode/pull/1', + }); + const notification: NotificationTreeItem = { + kind: 'notification', + model: pullRequest, + notification: Object.create(null), + }; + + await openItemOnGitHub(notification, telemetry); + + assert.ok(sendTelemetryEvent.calledOnceWith('pr.openInGitHub')); + }); +}); diff --git a/src/test/mocks/mockTelemetry.ts b/src/test/mocks/mockTelemetry.ts index ad426ff8a3..e6844916a7 100644 --- a/src/test/mocks/mockTelemetry.ts +++ b/src/test/mocks/mockTelemetry.ts @@ -1,8 +1,13 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + import { ITelemetry } from '../../common/telemetry'; export class MockTelemetry implements ITelemetry { - sendTelemetryEvent() {} - sendTelemetryErrorEvent() {} + sendTelemetryEvent(_eventName: string, _properties?: Record, _measurements?: Record) { } + sendTelemetryErrorEvent(_eventName: string, _properties?: Record, _measurements?: Record) { } dispose() { return Promise.resolve(); }