Set node column position when creation#995
Open
SlimaneAmar wants to merge 14 commits into
Open
Conversation
added 2 commits
May 20, 2026 15:39
…into set_node_column_position_when_creation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds column-position tracking: repository queries expose column positions and child UUIDs; service returns node-info and computes/assigns columnPosition for CHILD/BEFORE/AFTER insertions, reworks stash/delete flows; tests updated and added to assert columnPosition across insertion, cut/paste, duplication, stash, and delete flows. ChangesColumn Position Tracking for Network Modification Nodes
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/org/gridsuite/study/server/service/NetworkModificationTreeService.java`:
- Around line 151-159: getNextColumnPosition currently computes next sibling
columnPosition in-memory using
networkModificationNodeInfoRepository.findColumnPositionsByUuidIn(nodesRepository.findChildrenUuids(parentNodeId))
and returns max+1, which is race-prone; change it to perform the max calculation
inside a single transactional DB operation (e.g. a repository method that runs
SELECT MAX(column_position) FROM ... WHERE parent_id = :parentNodeId FOR UPDATE
or an equivalent DB-side atomic increment) or add a unique constraint on
(parent_id, column_position) and implement a retry-on-constraint-violation
strategy; ensure the new repository method is used in getNextColumnPosition and
that the surrounding service call is annotated/handled with a transaction to
prevent concurrent duplicates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 243a4303-d277-4f19-a49a-beb510f44a97
📒 Files selected for processing (6)
src/main/java/org/gridsuite/study/server/repository/networkmodificationtree/NetworkModificationNodeInfoRepository.javasrc/main/java/org/gridsuite/study/server/repository/networkmodificationtree/NodeRepository.javasrc/main/java/org/gridsuite/study/server/service/NetworkModificationTreeService.javasrc/test/java/org/gridsuite/study/server/NetworkModificationTreeTest.javasrc/test/java/org/gridsuite/study/server/studycontroller/NodeControllerTest.javasrc/test/java/org/gridsuite/study/server/studycontroller/StudyTestBase.java
added 3 commits
June 1, 2026 14:31
…into set_node_column_position_when_creation
…into set_node_column_position_when_creation
…into set_node_column_position_when_creation
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/org/gridsuite/study/server/service/NetworkModificationTreeService.java (1)
133-149:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCut/paste still leaves moved nodes with stale
columnPosition.This method now defines the
CHILD/BEFORE/AFTERposition rules for newly created nodes, butmoveNode(...)at Lines 250-274 still only rewires parents. After a cut/paste, the moved node keeps the position from its old parent,BEFOREnever copies the anchor position or resets the anchor to0, andAFTERnever puts the moved node at0under the new parent.
🧹 Nitpick comments (1)
src/test/java/org/gridsuite/study/server/NodePositionTest.java (1)
86-88: ⚡ Quick winFail fast when the expected node is missing
Returning
nullfromgetNodecauses downstream NPEs (Line 74, Lines 80-83) and hides the real test failure. Throw an assertion-friendly exception instead so failures point to the missing node name.Suggested change
private AbstractNode getNode(String name, List<AbstractNode> nodes) { - return nodes.stream().filter(node -> node.getName().equals(name)).findFirst().orElse(null); + return nodes.stream() + .filter(node -> name.equals(node.getName())) + .findFirst() + .orElseThrow(() -> new AssertionError("Node not found: " + name)); }Based on learnings: duplicate sibling
columnPositionvalues are intentionally accepted in this repository, so that concern is intentionally excluded.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/org/gridsuite/study/server/NodePositionTest.java` around lines 86 - 88, The helper getNode currently returns null when a node is not found which leads to downstream NPEs; change getNode(String name, List<AbstractNode> nodes) to fail fast by throwing an assertion-friendly exception (e.g., throw new AssertionError or IllegalStateException) including the missing node name and available node names so test failures clearly indicate the missing node (referencing getNode, AbstractNode and node.getName() to locate the method).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/org/gridsuite/study/server/service/NetworkModificationTreeService.java`:
- Around line 326-339: When deleting nodes with deleteChildren=false inside
deleteNodes(...), reuse the existing insertNodesToParent(...) promotion logic
instead of directly reparenting descendants: collect the list of promoted
children (the descendants to move up), and call
insertNodesToParent(parentOfDeletedNode, deletedNode.getColumnPosition(),
promotedChildren). This ensures each promoted NodeEntity has parent set and
NetworkModificationNodeInfoEntity positions are shifted/assigned consistently
(the same repositories used in insertNodesToParent: nodesRepository and
networkModificationNodeInfoRepository). Replace the ad-hoc reparenting branch in
deleteNodes(...) with this call so promoted children occupy the deleted node's
slot with correct columnPosition.
- Around line 306-335: The code unboxes and compares nullable column positions
which can NPE for legacy rows; in stashNode grab the column position as an
Integer (e.g. Integer columnPosition = nodeToStashInfo.getColumnPosition()),
handle null by deciding a safe default or by calling insertNodesToParent with a
nullable/optional position; update insertNodesToParent to accept Integer
position (or check for null) and when shifting siblings only operate on non-null
positions (e.g. filter info.getColumnPosition() != null &&
info.getColumnPosition() >= position) and compute the increment using
newChildrenInfos.size() safely; ensure any arithmetic avoids auto-unboxing of
nulls and that parent/position logic treats null columnPosition as a defined
insertion policy (end-of-list or zero) consistently across stashNode and
insertNodesToParent.
---
Nitpick comments:
In `@src/test/java/org/gridsuite/study/server/NodePositionTest.java`:
- Around line 86-88: The helper getNode currently returns null when a node is
not found which leads to downstream NPEs; change getNode(String name,
List<AbstractNode> nodes) to fail fast by throwing an assertion-friendly
exception (e.g., throw new AssertionError or IllegalStateException) including
the missing node name and available node names so test failures clearly indicate
the missing node (referencing getNode, AbstractNode and node.getName() to locate
the method).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cce12663-6530-4206-8863-586335624cb4
📒 Files selected for processing (4)
src/main/java/org/gridsuite/study/server/controller/StudyController.javasrc/main/java/org/gridsuite/study/server/repository/networkmodificationtree/NetworkModificationNodeInfoRepository.javasrc/main/java/org/gridsuite/study/server/service/NetworkModificationTreeService.javasrc/test/java/org/gridsuite/study/server/NodePositionTest.java
SlimaneAmar
force-pushed
the
set_node_column_position_when_creation
branch
from
81bd2f6 to
81fc850
Compare
added 6 commits
June 9, 2026 10:36
…into set_node_column_position_when_creation
…into set_node_column_position_when_creation
…into set_node_column_position_when_creation
|
SlimaneAmar
force-pushed
the
set_node_column_position_when_creation
branch
from
e1bb301 to
51a38d7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Summary