From f05d5d97660091515b58829fa6ae462ff664b2cc Mon Sep 17 00:00:00 2001 From: Robert Date: Thu, 27 Jul 2023 12:22:19 +0100 Subject: [PATCH 1/5] Split codeQL.copyVariantAnalysisRepoList into two commands --- extensions/ql-vscode/src/common/commands.ts | 6 +++++- .../ql-vscode/src/query-history/query-history-manager.ts | 2 +- .../src/variant-analysis/variant-analysis-manager.ts | 4 +++- .../ql-vscode/src/variant-analysis/variant-analysis-view.ts | 2 +- .../query-history/query-history-manager.test.ts | 2 +- 5 files changed, 11 insertions(+), 5 deletions(-) diff --git a/extensions/ql-vscode/src/common/commands.ts b/extensions/ql-vscode/src/common/commands.ts index 54204571ab2..5f0a56ecf76 100644 --- a/extensions/ql-vscode/src/common/commands.ts +++ b/extensions/ql-vscode/src/common/commands.ts @@ -244,7 +244,11 @@ export type VariantAnalysisCommands = { scannedRepo: VariantAnalysisScannedRepository, variantAnalysisSummary: VariantAnalysis, ) => Promise; - "codeQL.copyVariantAnalysisRepoList": ( + "codeQL.copyVariantAnalysisRepoListQueryHistory": ( + variantAnalysisId: number, + filterSort?: RepositoriesFilterSortStateWithIds, + ) => Promise; + "codeQL.copyVariantAnalysisRepoListView": ( variantAnalysisId: number, filterSort?: RepositoriesFilterSortStateWithIds, ) => Promise; diff --git a/extensions/ql-vscode/src/query-history/query-history-manager.ts b/extensions/ql-vscode/src/query-history/query-history-manager.ts index e17ef1e4f66..607e9cbaa8e 100644 --- a/extensions/ql-vscode/src/query-history/query-history-manager.ts +++ b/extensions/ql-vscode/src/query-history/query-history-manager.ts @@ -953,7 +953,7 @@ export class QueryHistoryManager extends DisposableObject { } await this.app.commands.execute( - "codeQL.copyVariantAnalysisRepoList", + "codeQL.copyVariantAnalysisRepoListQueryHistory", item.variantAnalysis.id, ); } diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts index f47541aad89..a939e2e761c 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts @@ -148,7 +148,9 @@ export class VariantAnalysisManager return { "codeQL.autoDownloadVariantAnalysisResult": this.enqueueDownload.bind(this), - "codeQL.copyVariantAnalysisRepoList": + "codeQL.copyVariantAnalysisRepoListQueryHistory": + this.copyRepoListToClipboard.bind(this), + "codeQL.copyVariantAnalysisRepoListView": this.copyRepoListToClipboard.bind(this), "codeQL.loadVariantAnalysisRepoResults": this.loadResults.bind(this), "codeQL.monitorNewVariantAnalysis": diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts index d2cd2d5df4b..cd030efba0a 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts @@ -140,7 +140,7 @@ export class VariantAnalysisView break; case "copyRepositoryList": void this.app.commands.execute( - "codeQL.copyVariantAnalysisRepoList", + "codeQL.copyVariantAnalysisRepoListView", this.variantAnalysisId, msg.filterSort, ); diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/query-history-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/query-history-manager.test.ts index 42caebf3bf0..9992ba855f6 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/query-history-manager.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/query-history-manager.test.ts @@ -714,7 +714,7 @@ describe("QueryHistoryManager", () => { const item = variantAnalysisHistory[1]; await queryHistoryManager.handleCopyRepoList(item); expect(executeCommand).toBeCalledWith( - "codeQL.copyVariantAnalysisRepoList", + "codeQL.copyVariantAnalysisRepoListQueryHistory", item.variantAnalysis.id, ); }); From 3b8cea8df4e6c02b1ed343aee03be6def2abbd77 Mon Sep 17 00:00:00 2001 From: Robert Date: Fri, 28 Jul 2023 10:00:38 +0100 Subject: [PATCH 2/5] Remove codeQL.copyVariantAnalysisRepoListQueryHistory command and instead call copyRepoListToClipboard directly --- extensions/ql-vscode/src/common/commands.ts | 4 ---- .../ql-vscode/src/query-history/query-history-manager.ts | 3 +-- .../src/variant-analysis/variant-analysis-manager.ts | 2 -- .../no-workspace/query-history/query-history-manager.test.ts | 5 +++-- 4 files changed, 4 insertions(+), 10 deletions(-) diff --git a/extensions/ql-vscode/src/common/commands.ts b/extensions/ql-vscode/src/common/commands.ts index 5f0a56ecf76..4fac4ec1790 100644 --- a/extensions/ql-vscode/src/common/commands.ts +++ b/extensions/ql-vscode/src/common/commands.ts @@ -244,10 +244,6 @@ export type VariantAnalysisCommands = { scannedRepo: VariantAnalysisScannedRepository, variantAnalysisSummary: VariantAnalysis, ) => Promise; - "codeQL.copyVariantAnalysisRepoListQueryHistory": ( - variantAnalysisId: number, - filterSort?: RepositoriesFilterSortStateWithIds, - ) => Promise; "codeQL.copyVariantAnalysisRepoListView": ( variantAnalysisId: number, filterSort?: RepositoriesFilterSortStateWithIds, diff --git a/extensions/ql-vscode/src/query-history/query-history-manager.ts b/extensions/ql-vscode/src/query-history/query-history-manager.ts index 607e9cbaa8e..f5670591bf9 100644 --- a/extensions/ql-vscode/src/query-history/query-history-manager.ts +++ b/extensions/ql-vscode/src/query-history/query-history-manager.ts @@ -952,8 +952,7 @@ export class QueryHistoryManager extends DisposableObject { return; } - await this.app.commands.execute( - "codeQL.copyVariantAnalysisRepoListQueryHistory", + await this.variantAnalysisManager.copyRepoListToClipboard( item.variantAnalysis.id, ); } diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts index a939e2e761c..8e5efe142e7 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts @@ -148,8 +148,6 @@ export class VariantAnalysisManager return { "codeQL.autoDownloadVariantAnalysisResult": this.enqueueDownload.bind(this), - "codeQL.copyVariantAnalysisRepoListQueryHistory": - this.copyRepoListToClipboard.bind(this), "codeQL.copyVariantAnalysisRepoListView": this.copyRepoListToClipboard.bind(this), "codeQL.loadVariantAnalysisRepoResults": this.loadResults.bind(this), diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/query-history-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/query-history-manager.test.ts index 9992ba855f6..9c7974f63c3 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/query-history-manager.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/query-history-manager.test.ts @@ -710,11 +710,12 @@ describe("QueryHistoryManager", () => { it("should copy repo list for a single variant analysis", async () => { queryHistoryManager = await createMockQueryHistory(allHistory); + queryHistoryManager.handleCopyRepoList = jest.fn(); const item = variantAnalysisHistory[1]; await queryHistoryManager.handleCopyRepoList(item); - expect(executeCommand).toBeCalledWith( - "codeQL.copyVariantAnalysisRepoListQueryHistory", + + expect(queryHistoryManager.handleCopyRepoList).toBeCalledWith( item.variantAnalysis.id, ); }); From 3d9b2da51445e1b053835171bf8101e399f6c32f Mon Sep 17 00:00:00 2001 From: Robert Date: Fri, 28 Jul 2023 10:01:05 +0100 Subject: [PATCH 3/5] Remove codeQL.copyVariantAnalysisRepoListView command and instead call copyRepoListToClipboard directly --- extensions/ql-vscode/src/common/commands.ts | 5 ----- .../src/variant-analysis/variant-analysis-manager.ts | 2 -- .../src/variant-analysis/variant-analysis-view-manager.ts | 4 ++++ .../ql-vscode/src/variant-analysis/variant-analysis-view.ts | 3 +-- 4 files changed, 5 insertions(+), 9 deletions(-) diff --git a/extensions/ql-vscode/src/common/commands.ts b/extensions/ql-vscode/src/common/commands.ts index 4fac4ec1790..d7fd8589964 100644 --- a/extensions/ql-vscode/src/common/commands.ts +++ b/extensions/ql-vscode/src/common/commands.ts @@ -4,7 +4,6 @@ import type { AstItem } from "../language-support"; import type { DbTreeViewItem } from "../databases/ui/db-tree-view-item"; import type { DatabaseItem } from "../databases/local-databases"; import type { QueryHistoryInfo } from "../query-history/query-history-info"; -import type { RepositoriesFilterSortStateWithIds } from "../variant-analysis/shared/variant-analysis-filter-sort"; import type { TestTreeNode } from "../query-testing/test-tree-node"; import type { VariantAnalysis, @@ -244,10 +243,6 @@ export type VariantAnalysisCommands = { scannedRepo: VariantAnalysisScannedRepository, variantAnalysisSummary: VariantAnalysis, ) => Promise; - "codeQL.copyVariantAnalysisRepoListView": ( - variantAnalysisId: number, - filterSort?: RepositoriesFilterSortStateWithIds, - ) => Promise; "codeQL.loadVariantAnalysisRepoResults": ( variantAnalysisId: number, repositoryFullName: string, diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts index 8e5efe142e7..1b0a1e3c805 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts @@ -148,8 +148,6 @@ export class VariantAnalysisManager return { "codeQL.autoDownloadVariantAnalysisResult": this.enqueueDownload.bind(this), - "codeQL.copyVariantAnalysisRepoListView": - this.copyRepoListToClipboard.bind(this), "codeQL.loadVariantAnalysisRepoResults": this.loadResults.bind(this), "codeQL.monitorNewVariantAnalysis": this.monitorVariantAnalysis.bind(this), diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-view-manager.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-view-manager.ts index 5e44960ed2c..9cb29d677b4 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-view-manager.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-view-manager.ts @@ -32,4 +32,8 @@ export interface VariantAnalysisViewManager< variantAnalysisId: number, filterSort?: RepositoriesFilterSortStateWithIds, ): Promise; + copyRepoListToClipboard( + variantAnalysisId: number, + filterSort?: RepositoriesFilterSortStateWithIds, + ): Promise; } diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts index cd030efba0a..8b4ff80ac11 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts @@ -139,8 +139,7 @@ export class VariantAnalysisView await this.manager.openQueryText(this.variantAnalysisId); break; case "copyRepositoryList": - void this.app.commands.execute( - "codeQL.copyVariantAnalysisRepoListView", + await this.manager.copyRepoListToClipboard( this.variantAnalysisId, msg.filterSort, ); From 59958a5b32d7cdd51a5d5a966553fa90fbe1cf03 Mon Sep 17 00:00:00 2001 From: Robert Date: Fri, 28 Jul 2023 10:06:01 +0100 Subject: [PATCH 4/5] Log a ui-interaction telemetry event when copying repository lists --- .../ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx b/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx index d52f5a3c6c6..3f692d1a8cb 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx +++ b/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx @@ -131,6 +131,7 @@ export function VariantAnalysis({ repositoryIds: selectedRepositoryIds, }, }); + sendTelemetry("variant-analysis-copy-repository-list"); }, [filterSortState, selectedRepositoryIds]); const exportResults = useCallback(() => { From e8afa54584b9633c8f67957e0e30d2b3b5338757 Mon Sep 17 00:00:00 2001 From: Robert Date: Fri, 28 Jul 2023 10:26:52 +0100 Subject: [PATCH 5/5] Fix test by mocking the correct function --- .../no-workspace/query-history/query-history-manager.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/query-history-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/query-history-manager.test.ts index 9c7974f63c3..57068a774a2 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/query-history-manager.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/query-history-manager.test.ts @@ -709,13 +709,13 @@ describe("QueryHistoryManager", () => { }); it("should copy repo list for a single variant analysis", async () => { + variantAnalysisManagerStub.copyRepoListToClipboard = jest.fn(); queryHistoryManager = await createMockQueryHistory(allHistory); - queryHistoryManager.handleCopyRepoList = jest.fn(); const item = variantAnalysisHistory[1]; await queryHistoryManager.handleCopyRepoList(item); - expect(queryHistoryManager.handleCopyRepoList).toBeCalledWith( + expect(variantAnalysisManagerStub.copyRepoListToClipboard).toBeCalledWith( item.variantAnalysis.id, ); });