limit reordering during assembling

[Mathieu-Deharbe]

corrects a reordering problem at the root level when deleting and assembling netmods

[coderabbitai]

📝 Walkthrough

Walkthrough

assembleNetworkModificationsIntoNewComposite now reuses a shared modification list when removing assembled sub-modifications and inserting the new composite, with a final conditional assignment to the target group.

Changes

Composite assembly

Layer / File(s)	Summary
Shared modification list handling src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java	Origin cleanup and target insertion now operate on a shared list, while the target group receives the updated list only when it is non-null.

Suggested reviewers: souissimai

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title is concise and matches the main change about limiting reordering during assembling.
Description check	✅ Passed	The description is related to the change and describes the reordering issue being corrected.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java (1)

1048-1051: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Dead-code branch: groupModifications is always non-null when targetGroup != null.

When targetGroup != null, the first modification has a group, so originGroup (found at lines 1010-1014) is always non-null and equal to targetGroup. This means groupModifications is already set at line 1017, making the if (groupModifications == null) check at line 1048 unreachable. Consider removing the dead branch or adding a comment explaining its defensive purpose.

🤖 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/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java`
around lines 1048 - 1051, Remove the unreachable null-check around
groupModifications in the targetGroup handling logic, since it is already
initialized when targetGroup is non-null; directly add newCompositeEntity at
targetIndex. If retaining the defensive fallback, document its purpose
explicitly.

🤖 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/modification/server/repositories/NetworkModificationRepository.java`:
- Around line 1060-1063: Guard the final update in the logic surrounding
targetGroup and groupModifications: do not call targetGroup.setModifications
when targetGroup is null. Handle mixed composite/group inputs explicitly by
updating the edited origin group when available or rejecting the combination,
and remove any dead groupModifications == null branch under a targetGroup !=
null condition.

---

Nitpick comments:
In
`@src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java`:
- Around line 1048-1051: Remove the unreachable null-check around
groupModifications in the targetGroup handling logic, since it is already
initialized when targetGroup is non-null; directly add newCompositeEntity at
targetIndex. If retaining the defensive fallback, document its purpose
explicitly.

🪄 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: 52701586-395d-4db4-a6ff-91ff879a923d

📥 Commits

Reviewing files that changed from the base of the PR and between 4563146 and 3c53cfb.

📒 Files selected for processing (1)

• src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java

[coderabbitai]

🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Search for validation in the service/controller layer that ensures
# all assembled modifications share the same container (group or composite).

# Check the service layer for any validation before calling the repository
rg -n -B5 -A10 'assembleNetworkModificationsIntoNewComposite' \
  src/main/java/org/gridsuite/modification/server/service/NetworkModificationService.java

# Check the controller for any validation
rg -n -B5 -A10 'assembleNetworkModificationsIntoNewComposite' \
  src/main/java/org/gridsuite/modification/server/CompositeController.java

# Search for any validation on assembledModificationsUuids (e.g., same group check)
rg -n -i 'same.*group\|same.*composite\|validate.*assembl\|check.*assembl' \
  src/main/java/org/gridsuite/modification/server/

Repository: gridsuite/network-modification-server

Length of output: 2523

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Call sites and surrounding service/controller code =="
sed -n '500,525p' src/main/java/org/gridsuite/modification/server/service/NetworkModificationService.java
echo
sed -n '70,95p' src/main/java/org/gridsuite/modification/server/CompositeController.java
echo

echo "== Repository method around the suspected lines =="
sed -n '990,1070p' src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java
echo

echo "== Search for any validation or invariant checks on mixed containers =="
rg -n -i 'same.*group|same.*composite|mixed.*group|mixed.*composite|validate.*assembl|check.*assembl|container.*check|group.*uuid|composite.*uuid' src/main/java/org/gridsuite/modification/server/

Repository: gridsuite/network-modification-server

Length of output: 50393

---

Guard targetGroup before the final setModifications call.
At NetworkModificationRepository.java:1060-1063, if the first UUID comes from a composite and another assembled UUID comes from a group, groupModifications is non-null while targetGroup is still null, so targetGroup.setModifications(...) throws a NullPointerException. The groupModifications == null branch under targetGroup != null is dead code; update the edited origin group or reject mixed inputs instead.

🤖 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/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java`
around lines 1060 - 1063, Guard the final update in the logic surrounding
targetGroup and groupModifications: do not call targetGroup.setModifications
when targetGroup is null. Handle mixed composite/group inputs explicitly by
updating the edited origin group when available or rejecting the combination,
and remove any dead groupModifications == null branch under a targetGroup !=
null condition.