[OGUI-689] Tree refresh should keep state of opened objects (#3235)

hehoon · web-flow · commit 0a6b618df5cb · 2026-01-06T17:00:04.000+01:00
* The tree on the object tree page now persists its expanded node state in local storage.

Note:
* The estimated storage size (in bytes) is approximately: `A * (N + 7) * 2`
  * `A` is the number of expanded (opened) nodes
  * `N` is the average character length of expanded (opened) node names
* The storing of tree node states will break if two identical names are stored directly under the same parent. This limitation already existed in the previous implementation and is unchanged by the current persistence mechanism.
* A temporary workaround was added to handle local storage resets in tests, since some tests are not fully isolated and depend on shared state. Ideally, all tests should be isolated, with local storage and page state fully reset between each test.
diff --git a/QualityControl/public/common/enums/storageKeys.enum.js b/QualityControl/public/common/enums/storageKeys.enum.js
@@ -20,4 +20,5 @@
 export const StorageKeysEnum = Object.freeze({
   OBJECT_VIEW_LEFT_PANEL_WIDTH: 'object-view-left-panel-width',
   OBJECT_VIEW_INFO_VISIBILITY_SETTING: 'object-view-info-visibility-setting',
+  OBJECT_TREE_OPEN_NODES: 'object-tree-open-nodes',
 });
diff --git a/QualityControl/public/object/ObjectTree.class.js b/QualityControl/public/object/ObjectTree.class.js
@@ -12,7 +12,8 @@
  * or submit itself to any jurisdiction.
  */
 
-import { Observable } from '/js/src/index.js';
+import { BrowserStorage, Observable, sessionService } from '/js/src/index.js';
+import { StorageKeysEnum } from '../common/enums/storageKeys.enum.js';
 
 /**
  * This class allows to transforms objects names (A/B/C) into a tree that can have
@@ -27,6 +28,7 @@ export default class ObjectTree extends Observable {
    */
   constructor(name, parent) {
     super();
+    this.storage = new BrowserStorage(StorageKeysEnum.OBJECT_TREE_OPEN_NODES);
     this.initTree(name, parent);
   }
 
@@ -46,12 +48,116 @@ export default class ObjectTree extends Observable {
     this.pathString = ''; // 'A/B'
   }
 
+  /**
+   * Load the expanded/collapsed state for this node and its children from localStorage.
+   * Updates the `open` property for the current node and recursively for all children.
+   */
+  loadExpandedNodes() {
+    if (!this.parent) {
+      // The main node may not be collapsable or expandable.
+      // Because of this we also have to load the expanded state of their direct children.
+      this.children.forEach((child) => child.loadExpandedNodes());
+    }
+
+    const session = sessionService.get();
+    const key = session.personid.toString();
+
+    // We traverse the path to reach the parent object of this node
+    let parentNode = this.storage.getLocalItem(key) ?? {};
+    for (let i = 0; i < this.path.length - 1; i++) {
+      parentNode = parentNode[this.path[i]];
+      if (!parentNode) {
+        // Cannot expand marked node because parent path does not exist
+        return;
+      }
+    }
+
+    this._applyExpandedNodesRecursive(parentNode, this);
+  }
+
+  /**
+   * Recursively traverse the stored data and update the tree nodes
+   * @param {object} data - The current level of the hierarchical expanded nodes object
+   * @param {ObjectTree} treeNode - The tree node to update
+   */
+  _applyExpandedNodesRecursive(data, treeNode) {
+    if (data[treeNode.name]) {
+      treeNode.open = true;
+      Object.keys(data[treeNode.name]).forEach((childName) => {
+        const child = treeNode.children.find((child) => child.name === childName);
+        if (child) {
+          this._applyExpandedNodesRecursive(data[treeNode.name], child);
+        }
+      });
+    }
+  };
+
+  /**
+   * Persist the current node's expanded/collapsed state in localStorage.
+   */
+  storeExpandedNodes() {
+    if (!this.parent) {
+      // The main node may not be collapsable or expandable.
+      // Because of this we have to store the expanded state of their direct children.
+      this.children.forEach((child) => child.storeExpandedNodes());
+    }
+
+    const session = sessionService.get();
+    const key = session.personid.toString();
+    const data = this.storage.getLocalItem(key) ?? {};
+
+    // We traverse the path to reach the parent object of this node
+    let parentNode = data;
+    for (let i = 0; i < this.path.length - 1; i++) {
+      const pathKey = this.path[i];
+      if (!parentNode[pathKey]) {
+        if (!this.open) {
+          // Cannot remove marked node because parent path does not exist
+          // Due to this the marked node also does not exist (so there is nothing to remove)
+          return;
+        }
+
+        // Parent path does not exist, we create it here so we can mark a deeper node
+        parentNode[pathKey] = {};
+      }
+
+      parentNode = parentNode[pathKey];
+    }
+
+    if (this.open) {
+      this._markExpandedNodesRecursive(parentNode, this);
+      this.storage.setLocalItem(key, data);
+    } else if (parentNode[this.name]) {
+      // Deleting from `parentNode` directly updates the `data` object
+      delete parentNode[this.name];
+      this.storage.setLocalItem(key, data);
+    }
+  }
+
+  /**
+   * Recursively mark a node and all open children in the hierarchical "expanded nodes" object.
+   * This method updates `data` to reflect the current node's expanded state:
+   * - If the node has any open children, it creates an object branch and recursively marks those children.
+   * - If the node has no open children (or is a leaf), it stores a marker value `{}`.
+   * @param {object} data - The current level in the hierarchical data object where nodes are stored.
+   * @param {ObjectTree} treeNode - The tree node whose expanded state should be stored.
+   */
+  _markExpandedNodesRecursive(data, treeNode) {
+    if (!data[treeNode.name]) {
+      data[treeNode.name] = {};
+    }
+    treeNode.children
+      .filter((child) => child.open)
+      .forEach((child) => this._markExpandedNodesRecursive(data[treeNode.name], child));
+  };
+
   /**
    * Toggle this node (open/close)
    * @returns {undefined}
    */
   toggle() {
     this.open = !this.open;
+    this.storeExpandedNodes();
     this.notify();
   }
 
@@ -60,6 +166,7 @@ export default class ObjectTree extends Observable {
    */
   closeAll() {
     this._closeAllRecursive();
+    this.storeExpandedNodes();
     this.notify();
   }
 
@@ -85,15 +192,14 @@ export default class ObjectTree extends Observable {
    *  addChild(o, [], ['A', 'B']) // end inserting, affecting B
    * @returns {undefined}
    */
-  addChild(object, path, pathParent) {
+  _addChild(object, path = undefined, pathParent = []) {
     // Fill the path argument through recursive call
     if (!path) {
       if (!object.name) {
         throw new Error('Object name must exist');
       }
       path = object.name.split('/');
-      this.addChild(object, path, []);
-      this.notify();
+      this._addChild(object, path);
       return;
     }
 
@@ -122,15 +228,26 @@ export default class ObjectTree extends Observable {
     }
 
     // Pass to child
-    subtree.addChild(object, path, fullPath);
+    subtree._addChild(object, path, fullPath);
   }
 
   /**
-   * Add a list of objects by calling `addChild`
+   * Add a single object as a child node
+   * @param {object} object - child to be added
+   */
+  addOneChild(object) {
+    this._addChild(object);
+    this.loadExpandedNodes();
+    this.notify();
+  }
+
+  /**
+   * Add a list of objects as child nodes
    * @param {Array<object>} objects - children to be added
-   * @returns {undefined}
    */
   addChildren(objects) {
-    objects.forEach((object) => this.addChild(object));
+    objects.forEach((object) => this._addChild(object));
+    this.loadExpandedNodes();
+    this.notify();
   }
 }
diff --git a/QualityControl/public/object/objectTreeHeader.js b/QualityControl/public/object/objectTreeHeader.js
@@ -58,6 +58,7 @@ export default function objectTreeHeader(qcObject, filterModel) {
       ' ',
       h('button.btn', {
         title: 'Close whole tree',
+        id: 'collapse-tree-button',
         onclick: () => qcObject.tree.closeAll(),
         disabled: Boolean(qcObject.searchInput),
       }, iconCollapseUp()),
diff --git a/QualityControl/test/public/features/filterTest.test.js b/QualityControl/test/public/features/filterTest.test.js
@@ -32,8 +32,8 @@ export const filterTests = async (url, page, timeout = 5000, testParent) => {
     strictEqual(location.href.includes('RunNumber=0'), true, 'URL should contain RunNumber=0');
 
     //Naviagte to object view
-    await extendTree(3, 5);
-    await page.locator('tr:last-of-type td').click();
+    await extendTree(3, 4);
+    await page.locator('tbody tr:nth-child(4) td').click();
     await page.waitForSelector('#fullscreen-button');
     await page.locator('#fullscreen-button').click();
     await page.waitForSelector('#runNumberFilter', { visible: true });
@@ -240,7 +240,6 @@ export const filterTests = async (url, page, timeout = 5000, testParent) => {
       { waitUntil: 'networkidle0' },
     );
 
-    await extendTree(3, 5);
     let rowCount = await page.evaluate(() => document.querySelectorAll('tr').length);
     strictEqual(rowCount, 7);
 
diff --git a/QualityControl/test/public/pages/object-tree.test.js b/QualityControl/test/public/pages/object-tree.test.js
@@ -11,9 +11,9 @@
  * or submit itself to any jurisdiction.
  */
 
-import { strictEqual, ok, deepStrictEqual } from 'node:assert';
+import { strictEqual, ok, deepStrictEqual, notDeepStrictEqual } from 'node:assert';
 import { delay } from '../../testUtils/delay.js';
-import { getLocalStorage } from '../../testUtils/localStorage.js';
+import { getLocalStorage, getLocalStorageAsJson } from '../../testUtils/localStorage.js';
 import { StorageKeysEnum } from '../../../public/common/enums/storageKeys.enum.js';
 
 const OBJECT_TREE_PAGE_PARAM = '?page=objectTree';
@@ -43,15 +43,22 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent)
     ok(rowsCount > 1); // more than 1 object in the tree
   });
 
-  await testParent.test('should not preserve state if refreshed not in run mode', { timeout }, async () => {
-    const tbodyPath = 'section > div > div > div > table > tbody';
-    await page.locator(`${tbodyPath} > tr:nth-child(2)`).click();
+  await testParent.test('should preserve state if refreshed', { timeout }, async () => {
+    const selector = 'section > div > div > div > table > tbody > tr:nth-child(2)';
+    await page.locator(selector).click();
     await page.reload({ waitUntil: 'networkidle0' });
 
-    const rowCount = await page.evaluate(() =>
+    const rowCountExpanded = await page.evaluate(() =>
       document.querySelectorAll('section > div > div > div > table > tbody > tr').length);
 
-    strictEqual(rowCount, 2);
+    await page.locator(selector).click();
+    await page.reload({ waitUntil: 'networkidle0' });
+
+    const rowCountCollapsed = await page.evaluate(() =>
+      document.querySelectorAll('section > div > div > div > table > tbody > tr').length);
+
+    strictEqual(rowCountExpanded, 3);
+    strictEqual(rowCountCollapsed, 2);
   });
 
   await testParent.test('should have a button to sort by (default "Name" ASC)', async () => {
@@ -136,10 +143,6 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent)
     async () => {
       const dragAmount = 35;
       await page.reload({ waitUntil: 'networkidle0' });
-      await page.evaluate(() => document.querySelector('tr.object-selectable:nth-child(2)').click());
-      await delay(500);
-      await page.evaluate(() => document.querySelector('tr.object-selectable:nth-child(3)').click());
-      await delay(500);
       await page.evaluate(() => document.querySelector('tr.object-selectable:nth-child(4)').click());
       await delay(1000);
       const panelWidth = await page.evaluate(() =>
@@ -228,6 +231,24 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent)
     }
   );
 
+  await testParent.test('should update local storage when tree node is clicked', { timeout }, async () => {
+    const selector = 'section > div > div > div > table > tbody > tr:nth-child(2)';
+    const personid = await page.evaluate(() => window.model.session.personid);
+    const storageKey = `${StorageKeysEnum.OBJECT_TREE_OPEN_NODES}-${personid}`;
+
+    await page.locator(selector).click();
+    const localStorageBefore = await getLocalStorageAsJson(page, storageKey);
+
+    await page.locator(selector).click();
+    const localStorageAfter = await getLocalStorageAsJson(page, storageKey);
+
+    notDeepStrictEqual(
+      localStorageBefore,
+      localStorageAfter,
+      'local storage should have changed after clicking a tree node',
+    );
+  });
+
   await testParent.test('should sort list of histograms by name in descending order', async () => {
     await page.locator('#sortTreeButton').click();
     const sortingByNameOptionPath = '#sortTreeButton > div > a:nth-child(2)';
@@ -257,6 +278,25 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent)
     strictEqual(sorted.list[0].name, 'qc/test/object/1');
   });
 
+  await testParent.test(
+    'should close all branches when clicking the collapse all button',
+    { timeout },
+    async () => {
+      const selector = '#collapse-tree-button';
+      const personid = await page.evaluate(() => window.model.session.personid);
+      const storageKey = `${StorageKeysEnum.OBJECT_TREE_OPEN_NODES}-${personid}`;
+
+      await page.locator(selector).click();
+      await delay(100);
+      const storedNodes = await getLocalStorageAsJson(page, storageKey);
+      const tableRowCount = await page.evaluate(() =>
+        document.querySelectorAll('section > div > div > div > table > tbody > tr').length);
+
+      deepStrictEqual(storedNodes, {}, 'Stores nodes should be empty');
+      strictEqual(tableRowCount, 1, 'Tree should be fully collapsed');
+    },
+  );
+
   await testParent.test('should have filtered results on input search', async () => {
     await page.type('#searchObjectTree', 'qc/test/object/1');
     const rowsDisplayed = await page.evaluate(() => {
