Merge branch 'main' into shati-nora/add-remote-repositories

Author: shati-patel

extensions/ql-vscode/package.json

@@ -375,8 +375,7 @@
       },
       {
         "command": "codeQLDatabasesExperimental.setSelectedItem",
-        "title": "Select Item",
-        "icon": "$(circle-small-filled)"
+        "title": "✓"
       },
       {
         "command": "codeQLDatabases.chooseDatabaseFolder",

extensions/ql-vscode/src/databases/db-item.ts

@@ -11,6 +11,20 @@ export enum DbItemKind {
   RemoteRepo = "RemoteRepo",
 }
 
+export const remoteDbKinds = [
+  DbItemKind.RootRemote,
+  DbItemKind.RemoteSystemDefinedList,
+  DbItemKind.RemoteUserDefinedList,
+  DbItemKind.RemoteOwner,
+  DbItemKind.RemoteRepo,
+];
+
+export const localDbKinds = [
+  DbItemKind.RootLocal,
+  DbItemKind.LocalList,
+  DbItemKind.LocalDatabase,
+];
+
 export interface RootLocalDbItem {
   kind: DbItemKind.RootLocal;
   expanded: boolean;

extensions/ql-vscode/src/databases/db-manager.ts

@@ -2,7 +2,7 @@ import { App } from "../common/app";
 import { AppEvent, AppEventEmitter } from "../common/events";
 import { ValueResult } from "../common/value-result";
 import { DbConfigStore } from "./config/db-config-store";
-import { DbItem } from "./db-item";
+import { DbItem, DbItemKind, remoteDbKinds } from "./db-item";
 import { calculateNewExpandedState } from "./db-item-expansion";
 import {
   getSelectedDbItem,
@@ -83,16 +83,19 @@ export class DbManager {
     await this.dbConfigStore.addRemoteOwner(owner);
   }
 
-  public async addNewRemoteList(listName: string): Promise<void> {
-    if (listName === "") {
-      throw Error("List name cannot be empty");
+  public async addNewList(kind: DbItemKind, listName: string): Promise<void> {
+    if (remoteDbKinds.includes(kind)) {
+      if (listName === "") {
+        throw Error("List name cannot be empty");
+      }
+      if (this.dbConfigStore.doesRemoteListExist(listName)) {
+        throw Error(`A list with the name '${listName}' already exists`);
+      }
+
+      await this.dbConfigStore.addRemoteList(listName);
+    } else {
+      throw Error("Cannot add a local list");
     }
-
-    if (this.dbConfigStore.doesRemoteListExist(listName)) {
-      throw Error(`A list with the name '${listName}' already exists`);
-    }
-
-    await this.dbConfigStore.addRemoteList(listName);
   }
 
   public doesRemoteListExist(listName: string): boolean {

extensions/ql-vscode/src/databases/ui/db-panel.ts

@@ -1,5 +1,6 @@
 import {
   QuickPickItem,
+  TreeView,
   TreeViewExpansionEvent,
   window,
   workspace,
@@ -11,8 +12,8 @@ import {
   getOwnerFromGitHubUrl,
   isValidGitHubOwner,
 } from "../../common/github-url-identifier-helper";
-import { showAndLogErrorMessage } from "../../helpers";
 import { DisposableObject } from "../../pure/disposable-object";
+import { DbItem, DbItemKind } from "../db-item";
 import { DbManager } from "../db-manager";
 import { DbTreeDataProvider } from "./db-tree-data-provider";
 import { DbTreeViewItem } from "./db-tree-view-item";
@@ -23,29 +24,30 @@ interface RemoteDatabaseQuickPickItem extends QuickPickItem {
 
 export class DbPanel extends DisposableObject {
   private readonly dataProvider: DbTreeDataProvider;
+  private readonly treeView: TreeView<DbTreeViewItem>;
 
   public constructor(private readonly dbManager: DbManager) {
     super();
 
     this.dataProvider = new DbTreeDataProvider(dbManager);
 
-    const treeView = window.createTreeView("codeQLDatabasesExperimental", {
+    this.treeView = window.createTreeView("codeQLDatabasesExperimental", {
       treeDataProvider: this.dataProvider,
       canSelectMany: false,
     });
 
     this.push(
-      treeView.onDidCollapseElement(async (e) => {
+      this.treeView.onDidCollapseElement(async (e) => {
         await this.onDidCollapseElement(e);
       }),
     );
     this.push(
-      treeView.onDidExpandElement(async (e) => {
+      this.treeView.onDidExpandElement(async (e) => {
         await this.onDidExpandElement(e);
       }),
     );
 
-    this.push(treeView);
+    this.push(this.treeView);
   }
 
   public async initialize(): Promise<void> {
@@ -158,13 +160,15 @@ export class DbPanel extends DisposableObject {
       return;
     }
 
-    if (this.dbManager.doesRemoteListExist(listName)) {
-      void showAndLogErrorMessage(
-        `A list with the name '${listName}' already exists`,
-      );
-    } else {
-      await this.dbManager.addNewRemoteList(listName);
-    }
+    const highlightedItem = await this.getHighlightedDbItem();
+
+    // For now: we only support adding remote lists, so if no item is highlighted,
+    // we default to the "RootRemote" kind.
+    // In future: if the highlighted item is undefined, we'll show a quick pick where
+    // a user can select whether to add a remote or local list.
+    const listKind = highlightedItem?.kind || DbItemKind.RootRemote;
+
+    await this.dbManager.addNewList(listKind, listName);
   }
 
   private async setSelectedItem(treeViewItem: DbTreeViewItem): Promise<void> {
@@ -197,4 +201,16 @@ export class DbPanel extends DisposableObject {
 
     await this.dbManager.updateDbItemExpandedState(event.element.dbItem, true);
   }
+
+  /**
+   * Gets the currently highlighted database item in the tree view.
+   * The VS Code API calls this the "selection", but we already have a notion of selection
+   * (i.e. which item has a check mark next to it), so we call this "highlighted".
+   *
+   * @returns The highlighted database item, or `undefined` if no item is highlighted.
+   */
+  private async getHighlightedDbItem(): Promise<DbItem | undefined> {
+    // You can only select one item at a time, so selection[0] gives the selection
+    return this.treeView.selection[0]?.dbItem;
+  }
 }

extensions/ql-vscode/src/databases/ui/db-selection-decoration-provider.ts

@@ -13,7 +13,7 @@ export class DbSelectionDecorationProvider implements FileDecorationProvider {
   ): ProviderResult<FileDecoration> {
     if (uri?.query === "selected=true") {
       return {
-        badge: "●",
+        badge: "✓",
         tooltip: "Currently selected",
       };
     }

extensions/ql-vscode/src/vscode-tests/minimal-workspace/databases/db-panel.test.ts

@@ -443,52 +443,63 @@ describe("db panel", () => {
     }
   });
 
-  it("should add a new list to the remote db list", async () => {
-    const dbConfig: DbConfig = createDbConfig({
-      remoteLists: [
-        {
-          name: "my-list-1",
-          repositories: ["owner1/repo1", "owner1/repo2"],
+  describe("addNewList", () => {
+    it("should add a new remote list", async () => {
+      const dbConfig: DbConfig = createDbConfig({
+        remoteLists: [
+          {
+            name: "my-list-1",
+            repositories: ["owner1/repo1", "owner1/repo2"],
+          },
+        ],
+        selected: {
+          kind: SelectedDbItemKind.RemoteUserDefinedList,
+          listName: "my-list-1",
         },
-      ],
-      selected: {
-        kind: SelectedDbItemKind.RemoteUserDefinedList,
-        listName: "my-list-1",
-      },
-    });
+      });
 
-    await saveDbConfig(dbConfig);
+      await saveDbConfig(dbConfig);
 
-    const dbTreeItems = await dbTreeDataProvider.getChildren();
+      const dbTreeItems = await dbTreeDataProvider.getChildren();
 
-    expect(dbTreeItems).toBeTruthy();
-    const items = dbTreeItems!;
+      expect(dbTreeItems).toBeTruthy();
+      const items = dbTreeItems!;
 
-    const remoteRootNode = items[0];
-    const remoteUserDefinedLists = remoteRootNode.children.filter(
-      (c) => c.dbItem?.kind === DbItemKind.RemoteUserDefinedList,
-    );
-    const list1 = remoteRootNode.children.find(
-      (c) =>
-        c.dbItem?.kind === DbItemKind.RemoteUserDefinedList &&
-        c.dbItem?.listName === "my-list-1",
-    );
+      const remoteRootNode = items[0];
+      const remoteUserDefinedLists = remoteRootNode.children.filter(
+        (c) => c.dbItem?.kind === DbItemKind.RemoteUserDefinedList,
+      );
+      const list1 = remoteRootNode.children.find(
+        (c) =>
+          c.dbItem?.kind === DbItemKind.RemoteUserDefinedList &&
+          c.dbItem?.listName === "my-list-1",
+      );
 
-    expect(remoteUserDefinedLists.length).toBe(1);
-    expect(remoteUserDefinedLists[0]).toBe(list1);
+      expect(remoteUserDefinedLists.length).toBe(1);
+      expect(remoteUserDefinedLists[0]).toBe(list1);
 
-    await dbManager.addNewRemoteList("my-list-2");
+      await dbManager.addNewList(DbItemKind.RootRemote, "my-list-2");
 
-    // Read the workspace databases JSON file directly to check that the new list has been added.
-    // We can't use the dbConfigStore's `read` function here because it depends on the file watcher
-    // picking up changes, and we don't control the timing of that.
-    const dbConfigFileContents = await readJSON(dbConfigFilePath);
-    expect(dbConfigFileContents.databases.remote.repositoryLists.length).toBe(
-      2,
-    );
-    expect(dbConfigFileContents.databases.remote.repositoryLists[1]).toEqual({
-      name: "my-list-2",
-      repositories: [],
+      // Read the workspace databases JSON file directly to check that the new list has been added.
+      // We can't use the dbConfigStore's `read` function here because it depends on the file watcher
+      // picking up changes, and we don't control the timing of that.
+      const dbConfigFileContents = await readJSON(dbConfigFilePath);
+      expect(dbConfigFileContents.databases.remote.repositoryLists.length).toBe(
+        2,
+      );
+      expect(dbConfigFileContents.databases.remote.repositoryLists[1]).toEqual({
+        name: "my-list-2",
+        repositories: [],
+      });
+    });
+
+    it("should throw error when adding a new list to a local node", async () => {
+      const dbConfig: DbConfig = createDbConfig();
+      await saveDbConfig(dbConfig);
+
+      await expect(
+        dbManager.addNewList(DbItemKind.RootLocal, ""),
+      ).rejects.toThrow(new Error("Cannot add a local list"));
     });
   });
 
@@ -555,9 +566,9 @@ describe("db panel", () => {
 
       await saveDbConfig(dbConfig);
 
-      await expect(dbManager.addNewRemoteList("")).rejects.toThrow(
-        new Error("List name cannot be empty"),
-      );
+      await expect(
+        dbManager.addNewList(DbItemKind.RootRemote, ""),
+      ).rejects.toThrow(new Error("List name cannot be empty"));
     });
 
     it("should not allow adding a list with duplicate name", async () => {
@@ -572,7 +583,9 @@ describe("db panel", () => {
 
       await saveDbConfig(dbConfig);
 
-      await expect(dbManager.addNewRemoteList("my-list-1")).rejects.toThrow(
+      await expect(
+        dbManager.addNewList(DbItemKind.RootRemote, "my-list-1"),
+      ).rejects.toThrow(
         new Error("A list with the name 'my-list-1' already exists"),
       );
     });