Add telemetry for "open on github"

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<boolean> {
+	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');

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':

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<TItem extends IssueModel = IssueModel> 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':

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<boolean> {
+	if (kind === 'pullRequest') {
+		/* __GDPR__
+			"pr.openInGitHub" : {}
+		*/
+		telemetry.sendTelemetryEvent('pr.openInGitHub');
+	} else {
+		/* __GDPR__
+			"issue.openOnGitHub" : {}
+		*/
+		telemetry.sendTelemetryEvent('issue.openOnGitHub');
+	}
+	return openWithDefaultExternalOpener(uri);
+}

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<PullRequest> 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);
 			}
 		});
 

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<PullRequestMode
 			vscode.commands.registerCommand('review.approveOnDotComDescription', (ctx: ReviewCommentContext) => {
 				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);
 				}
 			}),
 		);

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;
 	}

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'));
+	});
+});

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<string, string>, _measurements?: Record<string, number>) { }
+	sendTelemetryErrorEvent(_eventName: string, _properties?: Record<string, string>, _measurements?: Record<string, number>) { }
 	dispose() {
 		return Promise.resolve();
 	}