Skip to content

Fix tree cross-tab navigation : sync tree model globally #3831

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ayolab wants to merge 3 commits into
base: main
Choose a base branch
from
+107 −104
Open

Fix tree cross-tab navigation : sync tree model globally #3831

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d5b327e
Fix tree cross-tab navigation : sync tree model globally
ayolab Mar 24, 2026
072e381
review
ayolab Mar 24, 2026
0d7f33e
Merge branch 'main' of https://github.com/gridsuite/gridstudy-app int…
Mar 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/components/graph/network-modification-tree-model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,15 @@ export default class NetworkModificationTreeModel {
* @returns true if the order was changed
*/
reorderChildrenNodes(parentNodeId: string, orderedNodeIds: string[]) {
// Guard against incoherent notification before reordering
const currentChildren = new Set(this.getChildren(parentNodeId).map((c) => c.id));
if (
orderedNodeIds.length !== currentChildren.size ||
!orderedNodeIds.every((id) => currentChildren.has(id as UUID))
) {
console.warn('reorderChildrenNodes: orderedNodeIds does not match the current children set, skipping');
return false;
}
if (!this.needReorder(parentNodeId, orderedNodeIds)) {
return false;
}
Expand Down
71 changes: 70 additions & 1 deletion src/components/network-modification-tree-pane-event-handlers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,20 @@

import type { UUID } from 'node:crypto';
import type { NodeSelectionForCopy } from 'redux/reducer.type';
import type { NodeCreatedEventData, NodeMovedEventData } from 'types/notification-types';
import type {
NodeCreatedEventData,
NodeMovedEventData,
NodesColumnPositionsChangedEventData,
StudyUpdateEventData,
} from 'types/notification-types';
import { NotificationType } from 'types/notification-types';
import {
networkModificationHandleSubtree,
networkModificationTreeNodeAdded,
networkModificationTreeNodeMoved,
networkModificationTreeNodesRemoved,
networkModificationTreeNodesUpdated,
reorderNetworkModificationTreeNodes,
} from '../redux/actions';
import type { AppDispatch } from '../redux/store';
import {
Expand Down Expand Up @@ -97,3 +106,63 @@ export const fetchAndHandleSubtree = (
dispatch(networkModificationHandleSubtree(nodes, parentNode));
});
};

const fetchAndDispatchUpdatedNodes = (
dispatch: AppDispatch,
studyUuid: UUID,
rootNetworkUuid: UUID,
nodeIds: UUID[]
): void => {
Promise.allSettled(
nodeIds.map((nodeId) => fetchNetworkModificationTreeNode(studyUuid, nodeId, rootNetworkUuid))
).then((results) => {
const values = results.flatMap((result) => (result.status === 'fulfilled' ? [result.value] : []));
if (values.length > 0) {
dispatch(networkModificationTreeNodesUpdated(values));
}
});
};

export const handleTreeModelUpdate = (
dispatch: AppDispatch,
studyUuid: UUID,
rootNetworkUuid: UUID,
eventData: StudyUpdateEventData
): void => {
switch (eventData.headers.updateType) {
case NotificationType.NODE_BUILD_STATUS_UPDATED:
if (eventData.headers.rootNetworkUuid !== rootNetworkUuid) break;
fetchAndDispatchUpdatedNodes(dispatch, studyUuid, rootNetworkUuid, eventData.headers.nodes);
break;
case NotificationType.NODE_CREATED:
fetchAndDispatchAddedNode(dispatch, studyUuid, rootNetworkUuid, eventData as NodeCreatedEventData);
break;
case NotificationType.SUBTREE_CREATED:
fetchAndHandleSubtree(dispatch, studyUuid, eventData.headers.newNode, eventData.headers.parentNode);
break;
case NotificationType.NODE_MOVED:
fetchAndDispatchMovedNode(dispatch, studyUuid, rootNetworkUuid, eventData as NodeMovedEventData);
break;
case NotificationType.SUBTREE_MOVED:
fetchAndHandleSubtree(dispatch, studyUuid, eventData.headers.movedNode, eventData.headers.parentNode);
break;
case NotificationType.NODES_COLUMN_POSITION_CHANGED: {
const { headers, payload } = eventData as NodesColumnPositionsChangedEventData;
dispatch(reorderNetworkModificationTreeNodes(headers.parentNode, JSON.parse(payload)));
break;
}
Comment on lines +149 to +153
Copy link

@coderabbitai coderabbitai bot Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Wrap JSON.parse(payload) in try-catch to avoid unhandled exceptions.

If payload is undefined, empty, or malformed JSON, JSON.parse() throws a SyntaxError, which will propagate unhandled and potentially break subsequent event processing.

🛡️ Proposed defensive fix 
         case NotificationType.NODES_COLUMN_POSITION_CHANGED: {
             const { headers, payload } = eventData as NodesColumnPositionsChangedEventData;
-            dispatch(reorderNetworkModificationTreeNodes(headers.parentNode, JSON.parse(payload)));
+            try {
+                dispatch(reorderNetworkModificationTreeNodes(headers.parentNode, JSON.parse(payload)));
+            } catch (e) {
+                console.warn('NODES_COLUMN_POSITION_CHANGED: failed to parse payload', e);
+            }
             break;
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
case NotificationType.NODES_COLUMN_POSITION_CHANGED: {
const { headers, payload } = eventData as NodesColumnPositionsChangedEventData;
dispatch(reorderNetworkModificationTreeNodes(headers.parentNode, JSON.parse(payload)));
break;
}
case NotificationType.NODES_COLUMN_POSITION_CHANGED: {
const { headers, payload } = eventData as NodesColumnPositionsChangedEventData;
try {
dispatch(reorderNetworkModificationTreeNodes(headers.parentNode, JSON.parse(payload)));
} catch (e) {
console.warn('NODES_COLUMN_POSITION_CHANGED: failed to parse payload', e);
}
break;
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/components/network-modification-tree-pane-event-handlers.ts` around lines
149 - 153, The case handling NotificationType.NODES_COLUMN_POSITION_CHANGED
currently calls JSON.parse(payload) directly which can throw; wrap the parse in
a try-catch inside that case (the switch branch in
network-modification-tree-pane-event-handlers.ts) so that you catch
SyntaxError/other exceptions from JSON.parse(payload), log or warn the parsing
error and safely skip or provide a sane fallback (e.g. empty array) before
calling dispatch(reorderNetworkModificationTreeNodes(headers.parentNode, ...)),
ensuring dispatch is only called with a valid parsed value.

case NotificationType.NODES_DELETED:
dispatch(networkModificationTreeNodesRemoved(eventData.headers.nodes));
break;
case NotificationType.NODES_UPDATED:
fetchAndDispatchUpdatedNodes(dispatch, studyUuid, rootNetworkUuid, eventData.headers.nodes);
break;
case NotificationType.NODE_EDITED:
fetchNetworkModificationTreeNode(studyUuid, eventData.headers.node, rootNetworkUuid).then((node) =>
dispatch(networkModificationTreeNodesUpdated([node]))
);
break;
Comment on lines +132 to +164
Copy link

@coderabbitai coderabbitai bot Mar 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Filter root-scoped events before mutating the active tree.

The store only holds the tree for the current root network, but this centralized handler now runs for every study notification and only the build-status branch checks headers.rootNetworkUuid. A create/move/update/edit event from another root can now fetch or remove nodes against the wrong model.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/components/network-modification-tree-pane-event-handlers.ts` around lines
121 - 148, The switch handles notifications for all studies but only
NODE_BUILD_STATUS_UPDATED checks headers.rootNetworkUuid, so other branches
(NODE_CREATED, SUBTREE_CREATED, NODE_MOVED, SUBTREE_MOVED, NODES_DELETED,
NODES_UPDATED, NODE_EDITED) can act on the wrong tree; add a single guard using
eventData.headers.rootNetworkUuid and rootNetworkUuid at the top of the handler
(before the switch) or immediately inside the switch for every root-scoped case
to return/skip when they differ, ensuring functions like
fetchAndDispatchAddedNode, fetchAndHandleSubtree, fetchAndDispatchMovedNode,
fetchAndDispatchUpdatedNodes and fetchNetworkModificationTreeNode only run for
matching rootNetworkUuid.

Copy link
Contributor Author

@ayolab ayolab Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tree is shared across all root networks by design these events correctly apply globally. Only NODE_BUILD_STATUS_UPDATED is per-root-network-scoped, and it is already filtered

Copy link

@coderabbitai coderabbitai bot Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ayolab, thank you for the clarification! That makes sense — since the tree model is shared across all root networks, node creation, movement, deletion, and update events correctly apply globally without needing a rootNetworkUuid guard. The existing filter on NODE_BUILD_STATUS_UPDATED is the only root-scoped check needed. I was mistaken in flagging this.

✏️ Learnings added 
Learnt from: ayolab
Repo: gridsuite/gridstudy-app PR: 3831
File: src/components/network-modification-tree-pane-event-handlers.ts:121-148
Timestamp: 2026-03-24T11:06:27.690Z
Learning: In gridsuite/gridstudy-app, the network modification tree model (Redux store) is shared across all root networks by design. Tree structure events (NODE_CREATED, SUBTREE_CREATED, NODE_MOVED, SUBTREE_MOVED, NODES_DELETED, NODES_UPDATED, NODE_EDITED) apply globally and do not need to be filtered by rootNetworkUuid. Only NODE_BUILD_STATUS_UPDATED is per-root-network-scoped and must be filtered by rootNetworkUuid. This is relevant to `handleTreeModelUpdate` in `src/components/network-modification-tree-pane-event-handlers.ts`.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

default:
break;
}
};
94 changes: 13 additions & 81 deletions src/components/network-modification-tree-pane.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,8 @@
*/

import { useCallback, useEffect, useRef, useState } from 'react';
import {
networkModificationTreeNodesRemoved,
networkModificationTreeNodesUpdated,
removeNotificationByNode,
reorderNetworkModificationTreeNodes,
resetLogsFilter,
resetLogsPagination,
} from '../redux/actions';
import {
fetchAndDispatchAddedNode,
fetchAndDispatchMovedNode,
fetchAndHandleSubtree,
invalidateClipboardIfImpacted,
refreshStashedNodes,
} from './network-modification-tree-pane-event-handlers';
import { removeNotificationByNode, resetLogsFilter, resetLogsPagination } from '../redux/actions';
import { invalidateClipboardIfImpacted, refreshStashedNodes } from './network-modification-tree-pane-event-handlers';
import { useDispatch, useSelector } from 'react-redux';
import PropTypes from 'prop-types';
import NetworkModificationTree from './network-modification-tree';
Expand All @@ -40,7 +27,6 @@ import {
createTreeNode,
cutSubtree,
cutTreeNode,
fetchNetworkModificationTreeNode,
fetchStashedNodes,
stashSubtree,
stashTreeNode,
Expand All @@ -61,7 +47,7 @@ import { fetchNetworkModificationsToExport } from 'services/study/network-modifi

export const NetworkModificationTreePane = ({ studyUuid, currentRootNetworkUuid }) => {
const dispatch = useDispatch();
const { snackError, snackWarning } = useSnackMessage();
const { snackError } = useSnackMessage();
const [nodesToRestore, setNodesToRestore] = useState([]);

const { selectionForCopy, copyNode, cutNode, cleanClipboard } = useCopiedNodes();
Expand Down Expand Up @@ -95,52 +81,18 @@ export const NetworkModificationTreePane = ({ studyUuid, currentRootNetworkUuid

const { subscribeExport } = useExportSubscription();

const updateNodes = useCallback(
(updatedNodesIds) => {
Promise.all(
updatedNodesIds.map((nodeId) =>
fetchNetworkModificationTreeNode(studyUuid, nodeId, currentRootNetworkUuid)
)
).then((values) => {
dispatch(networkModificationTreeNodesUpdated(values));
});
},
[studyUuid, currentRootNetworkUuid, dispatch]
);

const resetNodeClipboard = useCallback(() => {
cleanClipboard();
}, [cleanClipboard]);

const reorderSubtree = useCallback(
(parentNodeId, orderedChildrenNodeIds) => {
// We check that the received node order from the notification is coherent with what we have locally.
const children = new Set(treeModelRef.current.getChildren(parentNodeId).map((c) => c.id));
let isListsEqual =
orderedChildrenNodeIds.length === children.size &&
orderedChildrenNodeIds.every((id) => children.has(id));
if (!isListsEqual) {
snackWarning({
messageId: 'ReorderSubtreeInvalidNotifInfo',
});
console.warn('Subtree order update cancelled : the ordered children list is incompatible');
return;
}

// dispatch reorder
dispatch(reorderNetworkModificationTreeNodes(parentNodeId, orderedChildrenNodeIds));
},
[dispatch, snackWarning]
);

const handleEvent = useCallback(
(event) => {
const eventData = parseEventData(event);
if (!eventData.headers) return;

switch (eventData.headers.updateType) {
case NotificationType.NODE_CREATED: {
fetchAndDispatchAddedNode(dispatch, studyUuid, currentRootNetworkUuid, eventData);
// Tree model update handled globally in study-container.jsx
invalidateClipboardIfImpacted(
[eventData.headers.parentNode],
nodeSelectionForCopyRef.current,
Expand All @@ -151,37 +103,23 @@ export const NetworkModificationTreePane = ({ studyUuid, currentRootNetworkUuid
}

case NotificationType.SUBTREE_CREATED: {
// Tree model update handled globally in study-container.jsx
invalidateClipboardIfImpacted(
[eventData.headers.parentNode],
nodeSelectionForCopyRef.current,
resetNodeClipboard
);
fetchAndHandleSubtree(dispatch, studyUuid, eventData.headers.newNode, eventData.headers.parentNode);
break;
}

case NotificationType.NODES_COLUMN_POSITION_CHANGED: {
reorderSubtree(eventData.headers.parentNode, JSON.parse(eventData.payload));
break;
}

case NotificationType.NODE_MOVED: {
fetchAndDispatchMovedNode(dispatch, studyUuid, currentRootNetworkUuid, eventData);
invalidateClipboardIfImpacted(
[eventData.headers.movedNode, eventData.headers.parentNode],
nodeSelectionForCopyRef.current,
resetNodeClipboard
);
// Tree model update handled globally in study-container.jsx
break;
}

case NotificationType.NODE_MOVED:
case NotificationType.SUBTREE_MOVED: {
fetchAndHandleSubtree(
dispatch,
studyUuid,
eventData.headers.movedNode,
eventData.headers.parentNode
);
// Tree model update handled globally in study-container.jsx
invalidateClipboardIfImpacted(
[eventData.headers.movedNode, eventData.headers.parentNode],
nodeSelectionForCopyRef.current,
Expand All @@ -191,20 +129,19 @@ export const NetworkModificationTreePane = ({ studyUuid, currentRootNetworkUuid
}

case NotificationType.NODES_DELETED: {
// Tree model update handled globally in study-container.jsx
invalidateClipboardIfImpacted(
eventData.headers.nodes,
nodeSelectionForCopyRef.current,
resetNodeClipboard
);
dispatch(networkModificationTreeNodesRemoved(eventData.headers.nodes));
refreshStashedNodes(studyUuid, setNodesToRestore);
break;
}

case NotificationType.NODES_UPDATED: {
updateNodes(eventData.headers.nodes);

if (eventData.headers.nodes.some((nodeId) => nodeId === currentNodeRef.current?.id)) {
// Tree model update handled globally in study-container.jsx
if (eventData.headers.nodes.includes(currentNodeRef.current?.id)) {
dispatch(removeNotificationByNode([currentNodeRef.current?.id]));
}
invalidateClipboardIfImpacted(
Expand All @@ -215,18 +152,13 @@ export const NetworkModificationTreePane = ({ studyUuid, currentRootNetworkUuid
break;
}

case NotificationType.NODE_EDITED: {
updateNodes([eventData.headers.node]);
break;
}

case NotificationType.NODE_BUILD_STATUS_UPDATED: {
if (eventData.headers.rootNetworkUuid !== currentRootNetworkUuidRef.current) break;

// Note: The actual node updates are now handled globally in study-container.jsx
// to ensure all workspaces open in other browser tabs (including those without tree panel) stay synchronized.
// Here we only handle tree-specific cleanup operations.
if (eventData.headers.nodes.some((nodeId) => nodeId === currentNodeRef.current?.id)) {
if (eventData.headers.nodes.includes(currentNodeRef.current?.id)) {
dispatch(removeNotificationByNode([currentNodeRef.current?.id]));
// when the current node is updated, we need to reset the logs filter
dispatch(resetLogsFilter());
Expand All @@ -247,7 +179,7 @@ export const NetworkModificationTreePane = ({ studyUuid, currentRootNetworkUuid
}
}
},
[studyUuid, updateNodes, reorderSubtree, dispatch, currentRootNetworkUuid, resetNodeClipboard]
[studyUuid, dispatch, resetNodeClipboard]
);

useNotificationsListener(NotificationsUrlKeys.STUDY, {
Expand Down
29 changes: 9 additions & 20 deletions src/components/study-container.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { PARAMS_LOADED } from '../utils/config-params';
import {
closeStudy,
loadNetworkModificationTreeSuccess,
networkModificationTreeNodesUpdated,
openStudy,
resetEquipmentsPostComputation,
setCurrentRootNetworkUuid,
Expand Down Expand Up @@ -41,7 +40,8 @@ import NetworkModificationTreeModel from './graph/network-modification-tree-mode
import { getFirstNodeOfType } from './graph/util/model-functions';
import { BUILD_STATUS } from './network/constants';
import { useAllComputingStatus } from './computing-status/use-all-computing-status';
import { fetchNetworkModificationTree, fetchNetworkModificationTreeNode } from '../services/study/tree-subtree';
import { fetchNetworkModificationTree } from '../services/study/tree-subtree';
import { handleTreeModelUpdate } from './network-modification-tree-pane-event-handlers';
import { fetchNetworkExistence, fetchRootNetworkIndexationStatus } from '../services/study/network';
import { fetchStudy, recreateStudyNetwork, reindexAllRootNetwork } from 'services/study/study';

Expand Down Expand Up @@ -265,29 +265,18 @@ export function StudyContainer() {
const handleStudyUpdate = useCallback(
(event) => {
const eventData = JSON.parse(event.data);
const updateTypeHeader = eventData.headers.updateType;
if (updateTypeHeader === NotificationType.STUDY_ALERT) {
if (eventData.headers.updateType === NotificationType.STUDY_ALERT) {
sendAlert(eventData);
return; // here, we do not want to update the redux state
}
displayErrorNotifications(eventData);

// Handle build status updates globally so all workspaces open in other browser tabs update currentTreeNode
// This fixes the issue where tabs without tree panel don't get updates
if (
updateTypeHeader === NotificationType.NODE_BUILD_STATUS_UPDATED &&
eventData.headers.rootNetworkUuid === currentRootNetworkUuidRef.current
) {
// Fetch updated nodes and dispatch to Redux to sync currentTreeNode
const updatedNodeIds = eventData.headers.nodes;
Promise.all(
updatedNodeIds.map((nodeId) =>
fetchNetworkModificationTreeNode(studyUuid, nodeId, currentRootNetworkUuidRef.current)
)
).then((values) => {
dispatch(networkModificationTreeNodesUpdated(values));
});
// Handle tree model updates globally so all workspaces (including those without a tree panel)
// stay synchronized. This ensures navigation sync works across browser tabs regardless of
// which panels are open in each tab's active workspace.
if (!currentRootNetworkUuidRef.current) {
return; // root networks not yet loaded, skip tree sync
}
handleTreeModelUpdate(dispatch, studyUuid, currentRootNetworkUuidRef.current, eventData);
},
// Note: dispatch doesn't change
[dispatch, displayErrorNotifications, sendAlert, studyUuid]
Expand Down
8 changes: 6 additions & 2 deletions src/hooks/use-study-navigation-sync.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { useStudyScopedNavigationKeys } from './use-study-scoped-navigation-keys
const useStudyNavigationSync = () => {
const syncEnabled = useSelector((state: AppState) => state.syncEnabled);
const currentRootNetworkUuid = useSelector((state: AppState) => state.currentRootNetworkUuid);
const rootNetworks = useSelector((state: AppState) => state.rootNetworks);
const currentTreeNode = useSelector((state: AppState) => state.currentTreeNode);
const treeModel = useSelector((state: AppState) => state.networkModificationTreeModel);
const dispatch = useDispatch();
Expand All @@ -29,10 +30,13 @@ const useStudyNavigationSync = () => {
const updateRootNetworkUuid = useCallback(
(uuid: UUID | null) => {
if (uuid !== null && uuid !== currentRootNetworkUuid) {
dispatch(setCurrentRootNetworkUuid(uuid));
const rootNetwork = rootNetworks.find((rn) => rn.rootNetworkUuid === uuid);
if (rootNetwork) {
dispatch(setCurrentRootNetworkUuid(uuid));
}
}
},
[dispatch, currentRootNetworkUuid]
[dispatch, currentRootNetworkUuid, rootNetworks]
);

const updateTreeNode = useCallback(
Expand Down
Loading