Fix resource tree node selection bug (#170)

* Fix bug with resource tree selection for graphs

* Reformat and type fixes

src/Explorer/Tree/ResourceTreeAdapter.test.ts

@@ -0,0 +1,112 @@
+import Explorer from "../Explorer";
+import * as ko from "knockout";
+import { ResourceTreeAdapter } from "./ResourceTreeAdapter";
+import * as ViewModels from "../../Contracts/ViewModels";
+import TabsBase from "../Tabs/TabsBase";
+
+describe("ResourceTreeAdapter", () => {
+  const mockContainer = (): Explorer =>
+    (({
+      selectedNode: ko.observable<ViewModels.TreeNode>({
+        nodeKind: "nodeKind",
+        rid: "rid",
+        id: ko.observable<string>("id")
+      }),
+      tabsManager: {
+        activeTab: ko.observable<TabsBase>({
+          tabKind: ViewModels.CollectionTabKind.Documents
+        } as TabsBase)
+      },
+      isNotebookEnabled: ko.observable<boolean>(true),
+      nonSystemDatabases: ko.observable<ViewModels.Database[]>([])
+    } as unknown) as Explorer);
+
+  // TODO isDataNodeSelected needs a better design and refactor, but for now, we protect some of the code paths
+  describe("isDataNodeSelected", () => {
+    it("it should not select if no selected node", () => {
+      const explorer = mockContainer();
+      explorer.selectedNode(undefined);
+      const resourceTreeAdapter = new ResourceTreeAdapter(explorer);
+      const isDataNodeSelected = resourceTreeAdapter.isDataNodeSelected("foo", "bar", undefined);
+      expect(isDataNodeSelected).toBeFalsy();
+    });
+
+    it("it should not select incorrect subnodekinds", () => {
+      const resourceTreeAdapter = new ResourceTreeAdapter(mockContainer());
+      const isDataNodeSelected = resourceTreeAdapter.isDataNodeSelected("foo", "bar", undefined);
+      expect(isDataNodeSelected).toBeFalsy();
+    });
+
+    it("it should not select if no active tab", () => {
+      const explorer = mockContainer();
+      explorer.tabsManager.activeTab(undefined);
+      const resourceTreeAdapter = new ResourceTreeAdapter(explorer);
+      const isDataNodeSelected = resourceTreeAdapter.isDataNodeSelected("foo", "bar", undefined);
+      expect(isDataNodeSelected).toBeFalsy();
+    });
+
+    it("should select if correct database node regardless of subnodekinds", () => {
+      const subNodeKind = ViewModels.CollectionTabKind.Documents;
+      const explorer = mockContainer();
+      explorer.selectedNode(({
+        nodeKind: "Database",
+        rid: "dbrid",
+        id: ko.observable<string>("id"),
+        selectedSubnodeKind: ko.observable<ViewModels.CollectionTabKind>(subNodeKind)
+      } as unknown) as ViewModels.TreeNode);
+      const resourceTreeAdapter = new ResourceTreeAdapter(explorer);
+      const isDataNodeSelected = resourceTreeAdapter.isDataNodeSelected("dbrid", "Database", [
+        ViewModels.CollectionTabKind.Documents
+      ]);
+      expect(isDataNodeSelected).toBeTruthy();
+    });
+
+    it("should select correct collection node (documents or graph node)", () => {
+      let subNodeKind = ViewModels.CollectionTabKind.Documents;
+      const explorer = mockContainer();
+      explorer.tabsManager.activeTab({
+        tabKind: subNodeKind
+      } as TabsBase);
+      explorer.selectedNode(({
+        nodeKind: "Collection",
+        rid: "collrid",
+        id: ko.observable<string>("id"),
+        selectedSubnodeKind: ko.observable<ViewModels.CollectionTabKind>(subNodeKind)
+      } as unknown) as ViewModels.TreeNode);
+      const resourceTreeAdapter = new ResourceTreeAdapter(explorer);
+      let isDataNodeSelected = resourceTreeAdapter.isDataNodeSelected("collrid", "Collection", [subNodeKind]);
+      expect(isDataNodeSelected).toBeTruthy();
+
+      subNodeKind = ViewModels.CollectionTabKind.Graph;
+      explorer.tabsManager.activeTab({
+        tabKind: subNodeKind
+      } as TabsBase);
+      explorer.selectedNode(({
+        nodeKind: "Collection",
+        rid: "collrid",
+        id: ko.observable<string>("id"),
+        selectedSubnodeKind: ko.observable<ViewModels.CollectionTabKind>(subNodeKind)
+      } as unknown) as ViewModels.TreeNode);
+      isDataNodeSelected = resourceTreeAdapter.isDataNodeSelected("collrid", "Collection", [subNodeKind]);
+      expect(isDataNodeSelected).toBeTruthy();
+    });
+
+    it("should not select incorrect collection node (e.g. Settings)", () => {
+      const explorer = mockContainer();
+      explorer.selectedNode(({
+        nodeKind: "Collection",
+        rid: "collrid",
+        id: ko.observable<string>("id"),
+        selectedSubnodeKind: ko.observable<ViewModels.CollectionTabKind>(ViewModels.CollectionTabKind.Documents)
+      } as unknown) as ViewModels.TreeNode);
+      explorer.tabsManager.activeTab({
+        tabKind: ViewModels.CollectionTabKind.Documents
+      } as TabsBase);
+      const resourceTreeAdapter = new ResourceTreeAdapter(explorer);
+      const isDataNodeSelected = resourceTreeAdapter.isDataNodeSelected("collrid", "Collection", [
+        ViewModels.CollectionTabKind.Settings
+      ]);
+      expect(isDataNodeSelected).toBeFalsy();
+    });
+  });
+});

src/Explorer/Tree/ResourceTreeAdapter.tsx

@@ -191,7 +191,7 @@ export class ResourceTreeAdapter implements ReactAdapter {
         databaseNode.children.push({
           label: "Scale",
           isSelected: () =>
-            this.isDataNodeSelected(database.rid, "Database", ViewModels.CollectionTabKind.DatabaseSettings),
+            this.isDataNodeSelected(database.rid, "Database", [ViewModels.CollectionTabKind.DatabaseSettings]),
           onClick: database.onSettingsClick.bind(database)
         });
       }
@@ -235,14 +235,18 @@ export class ResourceTreeAdapter implements ReactAdapter {
           data: collection.rid
         });
       },
-      isSelected: () => this.isDataNodeSelected(collection.rid, "Collection", ViewModels.CollectionTabKind.Documents),
+      isSelected: () =>
+        this.isDataNodeSelected(collection.rid, "Collection", [
+          ViewModels.CollectionTabKind.Documents,
+          ViewModels.CollectionTabKind.Graph
+        ]),
       contextMenu: ResourceTreeContextMenuButtonFactory.createCollectionContextMenuButton(this.container, collection)
     });
 
     children.push({
       label: database.isDatabaseShared() || this.container.isServerlessEnabled() ? "Settings" : "Scale & Settings",
       onClick: collection.onSettingsClick.bind(collection),
-      isSelected: () => this.isDataNodeSelected(collection.rid, "Collection", ViewModels.CollectionTabKind.Settings)
+      isSelected: () => this.isDataNodeSelected(collection.rid, "Collection", [ViewModels.CollectionTabKind.Settings])
     });
 
     if (ResourceTreeAdapter.showScriptNodes(this.container)) {
@@ -264,7 +268,8 @@ export class ResourceTreeAdapter implements ReactAdapter {
       children.push({
         label: "Conflicts",
         onClick: collection.onConflictsClick.bind(collection),
-        isSelected: () => this.isDataNodeSelected(collection.rid, "Collection", ViewModels.CollectionTabKind.Conflicts)
+        isSelected: () =>
+          this.isDataNodeSelected(collection.rid, "Collection", [ViewModels.CollectionTabKind.Conflicts])
       });
     }
 
@@ -302,7 +307,7 @@ export class ResourceTreeAdapter implements ReactAdapter {
         label: sp.id(),
         onClick: sp.open.bind(sp),
         isSelected: () =>
-          this.isDataNodeSelected(collection.rid, "Collection", ViewModels.CollectionTabKind.StoredProcedures),
+          this.isDataNodeSelected(collection.rid, "Collection", [ViewModels.CollectionTabKind.StoredProcedures]),
         contextMenu: ResourceTreeContextMenuButtonFactory.createStoreProcedureContextMenuItems(this.container, sp)
       })),
       onClick: () => {
@@ -321,7 +326,7 @@ export class ResourceTreeAdapter implements ReactAdapter {
         label: udf.id(),
         onClick: udf.open.bind(udf),
         isSelected: () =>
-          this.isDataNodeSelected(collection.rid, "Collection", ViewModels.CollectionTabKind.UserDefinedFunctions),
+          this.isDataNodeSelected(collection.rid, "Collection", [ViewModels.CollectionTabKind.UserDefinedFunctions]),
         contextMenu: ResourceTreeContextMenuButtonFactory.createUserDefinedFunctionContextMenuItems(this.container, udf)
       })),
       onClick: () => {
@@ -339,7 +344,8 @@ export class ResourceTreeAdapter implements ReactAdapter {
       children: collection.triggers().map((trigger: Trigger) => ({
         label: trigger.id(),
         onClick: trigger.open.bind(trigger),
-        isSelected: () => this.isDataNodeSelected(collection.rid, "Collection", ViewModels.CollectionTabKind.Triggers),
+        isSelected: () =>
+          this.isDataNodeSelected(collection.rid, "Collection", [ViewModels.CollectionTabKind.Triggers]),
         contextMenu: ResourceTreeContextMenuButtonFactory.createTriggerContextMenuItems(this.container, trigger)
       })),
       onClick: () => {
@@ -697,13 +703,19 @@ export class ResourceTreeAdapter implements ReactAdapter {
     window.requestAnimationFrame(() => this.parameters(Date.now()));
   }
 
-  private isDataNodeSelected(rid: string, nodeKind: string, subnodeKind: ViewModels.CollectionTabKind): boolean {
+  /**
+   * public for testing purposes
+   * @param rid
+   * @param nodeKind
+   * @param subnodeKinds
+   */
+  public isDataNodeSelected(rid: string, nodeKind: string, subnodeKinds: ViewModels.CollectionTabKind[]): boolean {
     if (!this.container.selectedNode || !this.container.selectedNode()) {
       return false;
     }
     const selectedNode = this.container.selectedNode();
 
-    if (subnodeKind === undefined) {
+    if (subnodeKinds === undefined || !Array.isArray(subnodeKinds)) {
       return selectedNode.rid === rid && selectedNode.nodeKind === nodeKind;
     } else {
       const activeTab = this.container.tabsManager.activeTab();
@@ -716,10 +728,10 @@ export class ResourceTreeAdapter implements ReactAdapter {
 
       return (
         activeTab &&
-        activeTab.tabKind === subnodeKind &&
+        subnodeKinds.includes(activeTab.tabKind) &&
         selectedNode.rid === rid &&
         selectedSubnodeKind !== undefined &&
-        selectedSubnodeKind === subnodeKind
+        subnodeKinds.includes(selectedSubnodeKind)
       );
     }
   }