Hierarchy builder: API cleanup and tests (#323)

* Remove unnecessary interface * Use hierarchy definition's node parser * Don't expose processing params with returned hierarchy nodes * Unify nodes' pre and post processor APIs * fixup post-processor change * Fix merging by label not working when merge candidates come from parent and child hierarchy levels * Add unit tests for `QueryScheduler` * Cover `TreeQueryResultsReader` with unit tests * Cover `HierarchyProvider` with unit tests * Fixup `defaultNodesParser` * Cover common utils with unit tests * Set `NODE_ENV: development` in terminal & CI to ensure assertions have effect * Ensure `HierarchyNode.children` always contains nodes with determined children * Cleanup HierarchyNode related APIs and fix issues found by that * Make `ClassBasedInstanceLabelSelectClauseFactoryProps.defaultClauseFactory` public. Fix `check-internal` script to detect `@internal` APIs when the tag is indented. * Don't allow nodes' post-processor to omit nodes * fixup test app * fixup tests after rebase * Even more type safety related to different kinds of nodes * Use node key to get exhaustive type checks * Force 100% unit test coverage * empty changeset * prettier * cleanup * Apply suggestions from code review Co-authored-by: JonasDov <[email protected]> * Add tests for sorting nodes in grouping operator --------- Co-authored-by: JonasDov <[email protected]>
iTwin · Nov 8, 2023 · 2fadf33 · 2fadf33
1 parent 901f2fa
commit 2fadf33
Show file tree

Hide file tree

Showing 53 changed files with 4,041 additions and 1,682 deletions.
diff --git a/.changeset/eight-trainers-hang.md b/.changeset/eight-trainers-hang.md
@@ -0,0 +1,2 @@
+---
+---
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -61,4 +61,6 @@ jobs:
         run: pnpm prettier
 
       - name: Build, Lint, Test
+        env:
+          NODE_ENV: "development"
         run: pnpm lage build lint cover
diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -7,6 +7,13 @@
   "editor.codeActionsOnSave": {
     "source.fixAll": true
   },
+
+  "eslint.experimental.useFlatConfig": true,
+  "eslint.workingDirectories": [
+    {
+      "mode": "auto"
+    }
+  ],
 
   "files.associations": {
     ".nycrc": "json",
@@ -52,10 +59,14 @@
       "orderLevel": 30
     }
   ],
-  "eslint.experimental.useFlatConfig": true,
-  "eslint.workingDirectories": [
-    {
-      "mode": "auto"
-    }
-  ]
+
+  "terminal.integrated.env.linux": {
+    "NODE_ENV": "development"
+  },
+  "terminal.integrated.env.osx": {
+    "NODE_ENV": "development"
+  },
+  "terminal.integrated.env.windows": {
+    "NODE_ENV": "development"
+  }
 }
diff --git a/apps/test-app/frontend/src/components/tree-widget/TreeWidget.tsx b/apps/test-app/frontend/src/components/tree-widget/TreeWidget.tsx
@@ -151,13 +151,12 @@ function parseTreeNodeItem(node: HierarchyNode): DelayLoadedTreeNodeItem {
   if (node.children === undefined) {
     throw new Error("Invalid node: children not determined");
   }
-  const hasChildren = typeof node.children === "boolean" ? node.children : Array.isArray(node.children) ? node.children.length > 0 : undefined;
   return {
     __internal: node,
     id: JSON.stringify(node.key),
     label: PropertyRecord.fromString(node.label, "Label"),
     icon: node.extendedData?.imageId,
-    hasChildren,
+    hasChildren: !!node.children,
     autoExpand: node.autoExpand,
   } as DelayLoadedTreeNodeItem;
 }
diff --git a/packages/full-stack-tests/src/hierarchy-builder/CustomNodes.test.ts b/packages/full-stack-tests/src/hierarchy-builder/CustomNodes.test.ts
@@ -106,7 +106,7 @@ describe("Stateless hierarchy builder", () => {
         key: "hidden child",
         label: "hc",
         children: undefined,
-        params: {
+        processingParams: {
           hideInHierarchy: true,
         },
       };
@@ -149,7 +149,7 @@ describe("Stateless hierarchy builder", () => {
         key: "hidden child",
         label: "hc",
         children: undefined,
-        params: {
+        processingParams: {
           hideIfNoChildren: true,
         },
       };

diff --git a/packages/full-stack-tests/src/hierarchy-builder/Filtering.test.ts b/packages/full-stack-tests/src/hierarchy-builder/Filtering.test.ts
@@ -246,7 +246,7 @@ describe("Stateless hierarchy builder", () => {
                     key: "custom",
                     label: "custom",
                     children: undefined,
-                    params: {
+                    processingParams: {
                       hideInHierarchy: true,
                     },
                     extendedData: {
@@ -332,7 +332,7 @@ describe("Stateless hierarchy builder", () => {
                     key: "custom",
                     label: "custom",
                     children: undefined,
-                    params: {
+                    processingParams: {
                       hideInHierarchy: true,
                     },
                     extendedData: {

diff --git a/packages/full-stack-tests/src/hierarchy-builder/HierarchyValidation.ts b/packages/full-stack-tests/src/hierarchy-builder/HierarchyValidation.ts
@@ -3,10 +3,13 @@
  * See LICENSE.md in the project root for license terms and full copyright notice.
  *--------------------------------------------------------------------------------------------*/
 
+import { Logger } from "@itwin/core-bentley";
 import { InstanceKey } from "@itwin/presentation-common";
 import { HierarchyNode, HierarchyProvider } from "@itwin/presentation-hierarchy-builder";
 import { hasChildren } from "@itwin/presentation-hierarchy-builder/lib/cjs/hierarchy-builder/internal/Common";
 
+const loggingNamespace = `Presentation.HierarchyBuilder.HierarchyValidation`;
+
 export interface HierarchyDef<TNode> {
   node: TNode;
   children?: Array<HierarchyDef<TNode>> | boolean;
@@ -159,14 +162,12 @@ export namespace NodeValidators {
 }
 
 export async function validateHierarchy(props: { provider: HierarchyProvider; parentNode?: HierarchyNode; expect: ExpectedHierarchyDef[] }) {
+  const parentIdentifier = props.parentNode ? props.parentNode.label : "<root>";
   const nodes = await props.provider.getNodes(props.parentNode);
+  Logger.logInfo(loggingNamespace, `Received ${nodes.length} child nodes for ${parentIdentifier}`);
 
   if (nodes.length !== props.expect.length) {
-    throw new Error(
-      `[${props.parentNode ? props.parentNode.label : "<root>"}] Expected ${props.expect.length} ${props.parentNode ? "child" : "root"} nodes, got ${
-        nodes.length
-      }`,
-    );
+    throw new Error(`[${parentIdentifier}] Expected ${props.expect.length} ${props.parentNode ? "child" : "root"} nodes, got ${nodes.length}`);
   }
 
   const resultHierarchy = new Array<HierarchyDef<HierarchyNode>>();

diff --git a/packages/full-stack-tests/src/hierarchy-builder/grouping/LabelGrouping.test.ts b/packages/full-stack-tests/src/hierarchy-builder/grouping/LabelGrouping.test.ts
@@ -5,27 +5,27 @@
 
 import { Subject } from "@itwin/core-backend";
 import { IModel } from "@itwin/core-common";
-import { IHierarchyLevelDefinitionsFactory, NodeSelectClauseFactory } from "@itwin/presentation-hierarchy-builder";
+import { HierarchyNode, IHierarchyLevelDefinitionsFactory, NodeSelectClauseFactory } from "@itwin/presentation-hierarchy-builder";
 import { buildIModel, insertSubject } from "../../IModelUtils";
 import { initialize, terminate } from "../../IntegrationTests";
 import { NodeValidators, validateHierarchy } from "../HierarchyValidation";
 import { createProvider } from "../Utils";
 
 describe("Stateless hierarchy builder", () => {
-  describe("Label grouping", () => {
-    let selectClauseFactory: NodeSelectClauseFactory;
-    let subjectClassName: string;
+  let selectClauseFactory: NodeSelectClauseFactory;
+  let subjectClassName: string;
 
-    before(async function () {
-      await initialize();
-      subjectClassName = Subject.classFullName.replace(":", ".");
-      selectClauseFactory = new NodeSelectClauseFactory();
-    });
+  before(async function () {
+    await initialize();
+    subjectClassName = Subject.classFullName.replace(":", ".");
+    selectClauseFactory = new NodeSelectClauseFactory();
+  });
 
-    after(async () => {
-      await terminate();
-    });
+  after(async () => {
+    await terminate();
+  });
 
+  describe("Label grouping", () => {
     const basicHierarchy: IHierarchyLevelDefinitionsFactory = {
       async defineHierarchyLevel(parentNode) {
         if (!parentNode) {
@@ -97,4 +97,118 @@ describe("Stateless hierarchy builder", () => {
       });
     });
   });
+
+  describe("Label merging", () => {
+    it("merges instance nodes with same merge id", async function () {
+      const { imodel, ...keys } = await buildIModel(this, async (builder) => {
+        const rootSubject = { className: subjectClassName, id: IModel.rootSubjectId };
+        const childSubject1 = insertSubject({ builder, codeValue: "1", parentId: rootSubject.id });
+        const childSubject2 = insertSubject({ builder, codeValue: "2", parentId: rootSubject.id });
+        return { rootSubject, childSubject1, childSubject2 };
+      });
+
+      const hierarchy: IHierarchyLevelDefinitionsFactory = {
+        async defineHierarchyLevel(parentNode) {
+          if (!parentNode) {
+            return [
+              {
+                fullClassName: subjectClassName,
+                query: {
+                  ecsql: `
+                    SELECT ${await selectClauseFactory.createSelectClause({
+                      ecClassId: { selector: `this.ECClassId` },
+                      ecInstanceId: { selector: `this.ECInstanceId` },
+                      nodeLabel: "merge this",
+                      mergeByLabelId: "merge",
+                    })}
+                    FROM ${subjectClassName} AS this
+                    WHERE this.Parent.Id = (${IModel.rootSubjectId})
+                  `,
+                },
+              },
+            ];
+          }
+          return [];
+        },
+      };
+
+      await validateHierarchy({
+        provider: createProvider({ imodel, hierarchy }),
+        expect: [
+          NodeValidators.createForInstanceNode({
+            instanceKeys: [keys.childSubject1, keys.childSubject2],
+            label: "merge this",
+            children: false,
+          }),
+        ],
+      });
+    });
+
+    it("merges instance nodes from different hidden parent hierarchy levels ", async function () {
+      const { imodel, ...keys } = await buildIModel(this, async (builder) => {
+        const rootSubject = { className: subjectClassName, id: IModel.rootSubjectId };
+        const visibleSubject1 = insertSubject({ builder, codeValue: "merged", parentId: rootSubject.id });
+        const hiddenSubject = insertSubject({ builder, codeValue: "hide", parentId: rootSubject.id });
+        const visibleSubject2 = insertSubject({ builder, codeValue: "merged", parentId: hiddenSubject.id });
+        return { rootSubject, visibleSubject1, visibleSubject2 };
+      });
+
+      const hierarchy: IHierarchyLevelDefinitionsFactory = {
+        async defineHierarchyLevel(parentNode) {
+          if (!parentNode) {
+            return [
+              {
+                fullClassName: subjectClassName,
+                query: {
+                  ecsql: `
+                    SELECT ${await selectClauseFactory.createSelectClause({
+                      ecClassId: { selector: `this.ECClassId` },
+                      ecInstanceId: { selector: `this.ECInstanceId` },
+                      nodeLabel: { selector: `this.CodeValue` },
+                      mergeByLabelId: "merge",
+                      hideNodeInHierarchy: { selector: `IIF(this.CodeValue = 'hide', 1, 0)` },
+                    })}
+                    FROM ${subjectClassName} AS this
+                    WHERE this.Parent.Id = (${IModel.rootSubjectId})
+                  `,
+                },
+              },
+            ];
+          }
+          if (HierarchyNode.isInstancesNode(parentNode) && parentNode.label === "hide") {
+            return [
+              {
+                fullClassName: subjectClassName,
+                query: {
+                  ecsql: `
+                    SELECT ${await selectClauseFactory.createSelectClause({
+                      ecClassId: { selector: `this.ECClassId` },
+                      ecInstanceId: { selector: `this.ECInstanceId` },
+                      nodeLabel: { selector: `this.CodeValue` },
+                      mergeByLabelId: "merge",
+                    })}
+                    FROM ${subjectClassName} AS this
+                    WHERE this.Parent.Id = ?
+                  `,
+                  bindings: parentNode.key.instanceKeys.map((k) => ({ type: "id", value: k.id })),
+                },
+              },
+            ];
+          }
+          return [];
+        },
+      };
+
+      await validateHierarchy({
+        provider: createProvider({ imodel, hierarchy }),
+        expect: [
+          NodeValidators.createForInstanceNode({
+            instanceKeys: [keys.visibleSubject1, keys.visibleSubject2],
+            label: "merged",
+            children: false,
+          }),
+        ],
+      });
+    });
+  });
 });