Remove historyItemId for variant analyses (#1651)

Author: charisk

extensions/ql-vscode/src/query-history-info.ts

@@ -20,14 +20,21 @@ export function getRawQueryName(item: QueryHistoryInfo): string {
   }
 }
 
-export function getQueryHistoryItemId(item: QueryHistoryInfo): string {
+/**
+ * Gets an identifier for the query history item which could be
+ * a local/remote query or a variant analysis. This id isn't guaranteed
+ * to be unique for each item in the query history.
+ * @param item the history item.
+ * @returns the id of the query or variant analysis.
+ */
+export function getQueryId(item: QueryHistoryInfo): string {
   switch (item.t) {
     case 'local':
       return item.initialInfo.id;
     case 'remote':
       return item.queryId;
     case 'variant-analysis':
-      return item.historyItemId;
+      return item.variantAnalysis.id.toString();
     default:
       assertNever(item);
   }
@@ -48,18 +55,18 @@ export function getQueryText(item: QueryHistoryInfo): string {
 
 export function buildRepoLabel(item: RemoteQueryHistoryItem | VariantAnalysisHistoryItem): string {
   if (item.t === 'remote') {
-      // Return the number of repositories queried if available. Otherwise, use the controller repository name.
-      const repositoryCount = item.remoteQuery.repositoryCount;
-  
-      if (repositoryCount) {
-        return pluralize(repositoryCount, 'repository', 'repositories');
-      }
-      return `${item.remoteQuery.controllerRepository.owner}/${item.remoteQuery.controllerRepository.name}`;
-    } else if (item.t === 'variant-analysis') {
-      const totalScannedRepositoryCount = item.variantAnalysis.scannedRepos?.length ?? 0;
-      const completedRepositoryCount = item.variantAnalysis.scannedRepos?.filter(repo => hasRepoScanCompleted(repo)).length ?? 0;
+    // Return the number of repositories queried if available. Otherwise, use the controller repository name.
+    const repositoryCount = item.remoteQuery.repositoryCount;
 
-      return `${completedRepositoryCount}/${pluralize(totalScannedRepositoryCount, 'repository', 'repositories')}`; // e.g. "2/3 repositories"
+    if (repositoryCount) {
+      return pluralize(repositoryCount, 'repository', 'repositories');
+    }
+    return `${item.remoteQuery.controllerRepository.owner}/${item.remoteQuery.controllerRepository.name}`;
+  } else if (item.t === 'variant-analysis') {
+    const totalScannedRepositoryCount = item.variantAnalysis.scannedRepos?.length ?? 0;
+    const completedRepositoryCount = item.variantAnalysis.scannedRepos?.filter(repo => hasRepoScanCompleted(repo)).length ?? 0;
+
+    return `${completedRepositoryCount}/${pluralize(totalScannedRepositoryCount, 'repository', 'repositories')}`; // e.g. "2/3 repositories"
   } else {
     assertNever(item);
   }

extensions/ql-vscode/src/query-history.ts

@@ -31,7 +31,7 @@ import { commandRunner } from './commandRunner';
 import { ONE_HOUR_IN_MS, TWO_HOURS_IN_MS } from './pure/time';
 import { assertNever, getErrorMessage, getErrorStack } from './pure/helpers-pure';
 import { CompletedLocalQueryInfo, LocalQueryInfo } from './query-results';
-import { getQueryHistoryItemId, getQueryText, QueryHistoryInfo } from './query-history-info';
+import { getQueryId, getQueryText, QueryHistoryInfo } from './query-history-info';
 import { DatabaseManager } from './databases';
 import { registerQueryHistoryScrubber } from './query-history-scrubber';
 import { QueryStatus, variantAnalysisStatusToQueryStatus } from './query-status';
@@ -51,7 +51,6 @@ import { EvalLogData, parseViewerData } from './pure/log-summary-parser';
 import { QueryWithResults } from './run-queries-shared';
 import { QueryRunner } from './queryRunner';
 import { VariantAnalysisManager } from './remote-queries/variant-analysis-manager';
-import { nanoid } from 'nanoid';
 import { VariantAnalysisHistoryItem } from './remote-queries/variant-analysis-history-item';
 import { getTotalResultCount } from './remote-queries/shared/variant-analysis';
 
@@ -606,7 +605,6 @@ export class QueryHistoryManager extends DisposableObject {
         t: 'variant-analysis',
         status: QueryStatus.InProgress,
         completed: false,
-        historyItemId: nanoid(),
         variantAnalysis,
       });
 
@@ -1109,7 +1107,7 @@ export class QueryHistoryManager extends DisposableObject {
       queryText: encodeURIComponent(getQueryText(finalSingleItem)),
     });
 
-    const queryId = getQueryHistoryItemId(finalSingleItem);
+    const queryId = getQueryId(finalSingleItem);
 
     const uri = Uri.parse(
       `codeql:${queryId}.ql?${params.toString()}`, true

extensions/ql-vscode/src/remote-queries/variant-analysis-history-item.ts

@@ -10,7 +10,6 @@ export interface VariantAnalysisHistoryItem {
   resultCount?: number;
   status: QueryStatus;
   completed: boolean;
-  readonly historyItemId: string,
   variantAnalysis: VariantAnalysis;
   userSpecifiedLabel?: string;
 }

extensions/ql-vscode/test/pure-tests/query-history-info.test.ts

@@ -1,7 +1,7 @@
 import { expect } from 'chai';
 
 import { QueryStatus } from '../../src/query-status';
-import { buildRepoLabel, getQueryHistoryItemId, getQueryText, getRawQueryName } from '../../src/query-history-info';
+import { buildRepoLabel, getQueryId, getQueryText, getRawQueryName } from '../../src/query-history-info';
 import { VariantAnalysisHistoryItem } from '../../src/remote-queries/variant-analysis-history-item';
 import { createMockVariantAnalysis } from '../../src/vscode-tests/factories/remote-queries/shared/variant-analysis';
 import { createMockScannedRepos } from '../../src/vscode-tests/factories/remote-queries/shared/scanned-repositories';
@@ -19,7 +19,6 @@ describe('Query history info', () => {
     t: 'variant-analysis',
     status: QueryStatus.InProgress,
     completed: false,
-    historyItemId: 'abc123',
     variantAnalysis: createMockVariantAnalysis(
       VariantAnalysisStatus.InProgress,
       createMockScannedRepos([
@@ -51,23 +50,23 @@ describe('Query history info', () => {
     });
   });
 
-  describe('getQueryHistoryItemId', () => {
+  describe('getQueryId', () => {
     it('should get the ID for local history items', () => {
-      const historyItemId = getQueryHistoryItemId(localQueryHistoryItem);
+      const historyItemId = getQueryId(localQueryHistoryItem);
 
       expect(historyItemId).to.equal(localQueryHistoryItem.initialInfo.id);
     });
 
     it('should get the ID for remote query history items', () => {
-      const historyItemId = getQueryHistoryItemId(remoteQueryHistoryItem);
+      const historyItemId = getQueryId(remoteQueryHistoryItem);
 
       expect(historyItemId).to.equal(remoteQueryHistoryItem.queryId);
     });
 
     it('should get the ID for variant analysis history items', () => {
-      const historyItemId = getQueryHistoryItemId(variantAnalysisHistoryItem);
+      const historyItemId = getQueryId(variantAnalysisHistoryItem);
 
-      expect(historyItemId).to.equal(variantAnalysisHistoryItem.historyItemId);
+      expect(historyItemId).to.equal(variantAnalysisHistoryItem.variantAnalysis.id.toString());
     });
   });
 
@@ -100,10 +99,10 @@ describe('Query history info', () => {
         expect(repoLabel).to.equal(expectedRepoLabel);
       });
       it('should return number of repositories when `repositoryCount` is non-zero', () => {
-        const remoteQueryHistoryItem2 = createMockRemoteQueryHistoryItem({repositoryCount: 3});
+        const remoteQueryHistoryItem2 = createMockRemoteQueryHistoryItem({ repositoryCount: 3 });
         const repoLabel2 = buildRepoLabel(remoteQueryHistoryItem2);
         const expectedRepoLabel2 = '3 repositories';
-  
+
         expect(repoLabel2).to.equal(expectedRepoLabel2);
       });
     });
@@ -113,7 +112,6 @@ describe('Query history info', () => {
           t: 'variant-analysis',
           status: QueryStatus.InProgress,
           completed: false,
-          historyItemId: 'abc123',
           variantAnalysis: createMockVariantAnalysis(
             VariantAnalysisStatus.InProgress,
             createMockScannedRepos([])
@@ -128,7 +126,6 @@ describe('Query history info', () => {
           t: 'variant-analysis',
           status: QueryStatus.InProgress,
           completed: false,
-          historyItemId: 'abc123',
           variantAnalysis: createMockVariantAnalysis(
             VariantAnalysisStatus.InProgress,
             createMockScannedRepos([
@@ -142,9 +139,9 @@ describe('Query history info', () => {
       });
       it('should return label when `totalScannedRepositoryCount` is greater than 1', () => {
         const repoLabel = buildRepoLabel(variantAnalysisHistoryItem);
-        
+
         expect(repoLabel).to.equal('2/4 repositories');
       });
     });
-  });  
+  });
 });