Fix set label command on history items

extensions/ql-vscode/CHANGELOG.md

@@ -11,6 +11,7 @@
 - Fix a bug where it is not possible to download some database archives. This fix specifically addresses large archives and archives whose central directories do not align with file headers. [#700](https://github.com/github/vscode-codeql/pull/700)
 - Avoid error dialogs when QL test discovery or database cleanup encounters a missing directory. [#706](https://github.com/github/vscode-codeql/pull/706)
 - Add descriptive text and a link in the results view. [#711](https://github.com/github/vscode-codeql/pull/711)
+- Fix the _Set Label_ command in the query history view. [#710](https://github.com/github/vscode-codeql/pull/710)
 
 ## 1.3.7 - 24 November 2020
 

extensions/ql-vscode/src/extension.ts

@@ -452,7 +452,7 @@ async function activateWithInstalledDistribution(
       // The call to showResults potentially creates SARIF file;
       // Update the tree item context value to allow viewing that
       // SARIF file from context menu.
-      await qhm.updateTreeItemContextValue(item);
+      await qhm.refreshTreeView(item);
     }
   }
 

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

@@ -52,14 +52,10 @@ const SHOW_QUERY_TEXT_QUICK_EVAL_MSG = `\
  */
 const FAILED_QUERY_HISTORY_ITEM_ICON = 'media/red-x.svg';
 
-interface QueryHistoryDataProvider extends vscode.TreeDataProvider<CompletedQuery> {
-  updateTreeItemContextValue(element: CompletedQuery): Promise<void>;
-}
-
 /**
  * Tree data provider for the query history view.
  */
-class HistoryTreeDataProvider extends DisposableObject implements QueryHistoryDataProvider {
+export class HistoryTreeDataProvider extends DisposableObject {
   private _onDidChangeTreeData = super.push(new vscode.EventEmitter<CompletedQuery | undefined>());
 
   readonly onDidChangeTreeData: vscode.Event<CompletedQuery | undefined> = this
@@ -82,37 +78,28 @@ class HistoryTreeDataProvider extends DisposableObject implements QueryHistoryDa
     );
   }
 
-  async updateTreeItemContextValue(element: CompletedQuery): Promise<void> {
-    // Mark this query history item according to whether it has a
-    // SARIF file so that we can make context menu items conditionally
-    // available.
-    const hasResults = await element.query.hasInterpretedResults();
-    element.treeItem!.contextValue = hasResults
-      ? 'interpretedResultsItem'
-      : 'rawResultsItem';
-    this.refresh();
-  }
-
   async getTreeItem(element: CompletedQuery): Promise<vscode.TreeItem> {
-    if (element.treeItem !== undefined)
-      return element.treeItem;
-
-    const it = new vscode.TreeItem(element.toString());
+    const treeItem = new vscode.TreeItem(element.toString());
 
-    it.command = {
+    treeItem.command = {
       title: 'Query History Item',
       command: 'codeQLQueryHistory.itemClicked',
       arguments: [element],
     };
 
-    element.treeItem = it;
-    this.updateTreeItemContextValue(element);
+    // Mark this query history item according to whether it has a
+    // SARIF file so that we can make context menu items conditionally
+    // available.
+    const hasResults = await element.query.hasInterpretedResults();
+    treeItem.contextValue = hasResults
+      ? 'interpretedResultsItem'
+      : 'rawResultsItem';
 
     if (!element.didRunSuccessfully) {
-      it.iconPath = this.failedIconPath;
+      treeItem.iconPath = this.failedIconPath;
     }
 
-    return it;
+    return treeItem;
   }
 
   getChildren(
@@ -157,8 +144,8 @@ class HistoryTreeDataProvider extends DisposableObject implements QueryHistoryDa
     return this.history;
   }
 
-  refresh() {
-    this._onDidChangeTreeData.fire(undefined);
+  refresh(completedQuery?: CompletedQuery) {
+    this._onDidChangeTreeData.fire(completedQuery);
   }
 
   find(queryId: number): CompletedQuery | undefined {
@@ -367,11 +354,9 @@ export class QueryHistoryManager extends DisposableObject {
     });
     // undefined response means the user cancelled the dialog; don't change anything
     if (response !== undefined) {
-      if (response === '')
-        // Interpret empty string response as 'go back to using default'
-        singleItem.options.label = undefined;
-      else singleItem.options.label = response;
-      this.treeDataProvider.refresh();
+      // Interpret empty string response as 'go back to using default'
+      singleItem.options.label = response === '' ? undefined : response;
+      this.treeDataProvider.refresh(singleItem);
     }
   }
 
@@ -695,7 +680,7 @@ the file in the file explorer and dragging it into the workspace.`
     };
   }
 
-  async updateTreeItemContextValue(element: CompletedQuery): Promise<void> {
-    this.treeDataProvider.updateTreeItemContextValue(element);
+  async refreshTreeView(completedQuery: CompletedQuery): Promise<void> {
+    this.treeDataProvider.refresh(completedQuery);
   }
 }

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

@@ -1,4 +1,4 @@
-import { env, TreeItem } from 'vscode';
+import { env } from 'vscode';
 
 import { QueryWithResults, tmpDir, QueryInfo } from './run-queries';
 import * as messages from './pure/messages';
@@ -17,7 +17,6 @@ export class CompletedQuery implements QueryWithResults {
   readonly database: DatabaseInfo;
   readonly logFileLocation?: string;
   options: QueryHistoryItemOptions;
-  treeItem?: TreeItem;
   dispose: () => void;
 
   /**

extensions/ql-vscode/src/vscode-tests/no-workspace/query-history.test.ts

@@ -5,7 +5,9 @@ import * as vscode from 'vscode';
 import * as sinon from 'sinon';
 import * as chaiAsPromised from 'chai-as-promised';
 import { logger } from '../../logging';
-import { QueryHistoryManager } from '../../query-history';
+import { QueryHistoryManager, HistoryTreeDataProvider } from '../../query-history';
+import { CompletedQuery } from '../../query-results';
+import { QueryInfo } from '../../run-queries';
 
 chai.use(chaiAsPromised);
 const expect = chai.expect;
@@ -43,7 +45,6 @@ describe('query-history', () => {
   });
 
   describe('tryOpenExternalFile', () => {
-
     it('should open an external file', async () => {
       await tryOpenExternalFile('xxx');
       expect(showTextDocumentSpy).to.have.been.calledOnceWith(
@@ -204,6 +205,67 @@ describe('query-history', () => {
       expect(queryHistory.compareWithItem).to.be.eq('a');
     });
   });
+
+  describe('HistoryTreeDataProvider', () => {
+    let historyTreeDataProvider: HistoryTreeDataProvider;
+    beforeEach(() => {
+      historyTreeDataProvider = new HistoryTreeDataProvider(vscode.Uri.file('/a/b/c').fsPath);
+    });
+
+    it('should get a tree item with raw results', async () => {
+      const mockQuery = {
+        query: {
+          hasInterpretedResults: () => Promise.resolve(false)
+        } as QueryInfo,
+        didRunSuccessfully: true,
+        toString: () => 'mock label'
+      } as CompletedQuery;
+      const treeItem = await historyTreeDataProvider.getTreeItem(mockQuery);
+      expect(treeItem.command).to.deep.eq({
+        title: 'Query History Item',
+        command: 'codeQLQueryHistory.itemClicked',
+        arguments: [mockQuery],
+      });
+      expect(treeItem.label).to.eq('mock label');
+      expect(treeItem.contextValue).to.eq('rawResultsItem');
+      expect(treeItem.iconPath).to.be.undefined;
+    });
+
+    it('should get a tree item with interpreted results', async () => {
+      const mockQuery = {
+        query: {
+          // as above, except for this line
+          hasInterpretedResults: () => Promise.resolve(true)
+        } as QueryInfo,
+        didRunSuccessfully: true,
+        toString: () => 'mock label'
+      } as CompletedQuery;
+      const treeItem = await historyTreeDataProvider.getTreeItem(mockQuery);
+      expect(treeItem.contextValue).to.eq('interpretedResultsItem');
+    });
+
+    it('should get a tree item that did not complete successfully', async () => {
+      const mockQuery = {
+        query: {
+          hasInterpretedResults: () => Promise.resolve(true)
+        } as QueryInfo,
+        // as above, except for this line
+        didRunSuccessfully: false,
+        toString: () => 'mock label'
+      } as CompletedQuery;
+      const treeItem = await historyTreeDataProvider.getTreeItem(mockQuery);
+      expect(treeItem.iconPath).to.eq(vscode.Uri.file('/a/b/c/media/red-x.svg').fsPath);
+    });
+
+    it('should get children', () => {
+      const mockQuery = {
+        databaseName: 'abc'
+      } as CompletedQuery;
+      historyTreeDataProvider.allHistory.push(mockQuery);
+      expect(historyTreeDataProvider.getChildren()).to.deep.eq([mockQuery]);
+      expect(historyTreeDataProvider.getChildren(mockQuery)).to.deep.eq([]);
+    });
+  });
 });
 
 function createMockQueryHistory(allHistory: {}[]) {