add composite names#839
Open
AbdelHedhili wants to merge 9 commits into
Open
Conversation
Signed-off-by: HEDHILI Abdelsalem <hedhiliabd@gm0winl1147.bureau.si.interne>
This comment was marked as low quality.
This comment was marked as low quality.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as low quality.
This comment was marked as low quality.
Sorry, something went wrong.
Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
There was a problem hiding this comment.
Actionable comments posted: 1
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/modification/server/repositories/NetworkModificationRepository.java (1)
145-145: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winApply the requested
namein the single-composite fast path.This method now requires
name, but whenmodificationUuidscontains exactly one composite the early return above skips the new naming logic entirely. That means this create path preserves the source composite name instead of the caller-supplied one.Minimal fix
if (copyEntities.size() == 1 && copyEntities.getFirst() instanceof CompositeModificationEntity single) { + single.setName(name); return modificationRepository.save(single).getId(); }Also applies to: 162-165
🤖 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` at line 145, The single-composite early return in createNetworkCompositeModification is bypassing the caller-supplied name, so update that fast path to apply the provided name before returning. Use the existing createNetworkCompositeModification flow and the composite-copy logic around modificationUuids so the one-item case still creates a renamed composite instead of preserving the source name.
🧹 Nitpick comments (1)
src/test/java/org/gridsuite/modification/server/CompositeControllerTest.java (1)
333-346: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the new update-time rename path.
This test only exercises
modifications_uuids. The newnameparameter onPUT /{uuid}is never sent or asserted, so a broken rename implementation would still pass the suite. Please add at least one rename-only or rename+reorder assertion here.🤖 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/modification/server/CompositeControllerTest.java` around lines 333 - 346, The CompositeControllerTest update path only validates modifications_uuids and does not cover the new name parameter on the PUT flow. Extend this test around the existing composite modification update/get assertions to send a rename through CompositeController’s PUT /{uuid} handling (either rename-only or rename plus reorder), then assert the returned composite metadata reflects the updated name. Use the existing compositeModificationUuid, mockMvc, and updatedMap/updatedCompositeContent checks to verify the rename path is exercised.
🤖 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/CompositeController.java`:
- Around line 127-131: The `updateNetworkCompositeModification` endpoint is
incorrectly reading `modifications_uuids` from a request parameter, which shifts
a potentially large UUID list into the URL. Update
`CompositeController.updateNetworkCompositeModification` to keep the UUID list
in the request body instead of `@RequestParam`, and adjust the service
invocation so the controller still passes the same `modificationUuids` data from
the body-based contract.
---
Outside diff comments:
In
`@src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java`:
- Line 145: The single-composite early return in
createNetworkCompositeModification is bypassing the caller-supplied name, so
update that fast path to apply the provided name before returning. Use the
existing createNetworkCompositeModification flow and the composite-copy logic
around modificationUuids so the one-item case still creates a renamed composite
instead of preserving the source name.
---
Nitpick comments:
In
`@src/test/java/org/gridsuite/modification/server/CompositeControllerTest.java`:
- Around line 333-346: The CompositeControllerTest update path only validates
modifications_uuids and does not cover the new name parameter on the PUT flow.
Extend this test around the existing composite modification update/get
assertions to send a rename through CompositeController’s PUT /{uuid} handling
(either rename-only or rename plus reorder), then assert the returned composite
metadata reflects the updated name. Use the existing compositeModificationUuid,
mockMvc, and updatedMap/updatedCompositeContent checks to verify the rename path
is exercised.
🪄 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: 60568464-a571-4702-857a-350a752ba575
📒 Files selected for processing (4)
src/main/java/org/gridsuite/modification/server/CompositeController.javasrc/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.javasrc/main/java/org/gridsuite/modification/server/service/NetworkModificationService.javasrc/test/java/org/gridsuite/modification/server/CompositeControllerTest.java
Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
SlimaneAmar
reviewed
Jun 25, 2026
| return ResponseEntity.ok().body(networkModificationService.createNetworkCompositeModification(modificationUuids)); | ||
| public ResponseEntity<UUID> createNetworkCompositeModification(@Parameter(description = "Composite modifications name", required = true) @RequestParam("name") String name, | ||
| @RequestBody List<UUID> modificationUuids) { | ||
| return ResponseEntity.ok().body(networkModificationService.createNetworkCompositeModification(modificationUuids, name == null ? "" : name)); |
Contributor
There was a problem hiding this comment.
Actually it is already required so I don't see the point of the null (Abdel added it but he is absent). I remove it : 3cacc3c
Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
SlimaneAmar
reviewed
Jun 25, 2026
| } | ||
|
|
||
| public void updateCompositeModification(@NonNull UUID compositeUuid, String name) { | ||
| ModificationEntity modificationEntity = modificationRepository.findById(compositeUuid) |
Contributor
There was a problem hiding this comment.
Use CompositeModificationRepository
Contributor
There was a problem hiding this comment.
Done for update and replace oo. And removed the is instance test : 3cacc3c
Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/org/gridsuite/modification/server/CompositeController.java (1)
124-131: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDrop the JSON
consumesconstraint from rename-only updates.Line 124 now maps an endpoint with no
@RequestBody; clients sendingPUT /{uuid}?name=...withoutContent-Type: application/jsoncan hit415 Unsupported Media Typebefore Line 130 is reached.Suggested fix
- `@PutMapping`(value = "/{uuid}", consumes = MediaType.APPLICATION_JSON_VALUE) + `@PutMapping`(value = "/{uuid}")🤖 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/CompositeController.java` around lines 124 - 131, The update endpoint in CompositeController.updateNetworkCompositeModification is rename-only and does not accept a request body, so the JSON consumes constraint is causing unnecessary 415 responses for clients using PUT with only the name query parameter. Remove the MediaType.APPLICATION_JSON_VALUE consumes declaration from the `@PutMapping` on updateNetworkCompositeModification so the endpoint can be called without a Content-Type header, while still delegating to networkModificationService.updateCompositeModification as before.src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java (1)
158-160: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winApply the requested name in the single-composite shortcut.
When
modificationUuidscontains one composite, Line 159 returns the cloned composite without applying the newname, so the create endpoint can return a composite with the source name instead of the requested name.Suggested fix
if (copyEntities.size() == 1 && copyEntities.getFirst() instanceof CompositeModificationEntity single) { + single.setName(name); return modificationRepository.save(single).getId(); }🤖 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 158 - 160, The single-composite shortcut in NetworkModificationRepository is returning the cloned CompositeModificationEntity before the requested name is applied. Update the save path in this branch so the copied entity gets the new name from the create request before calling modificationRepository.save. Make sure the same name application logic used for the general copyEntities flow is also applied when copyEntities contains exactly one composite, so the returned id corresponds to a composite with the requested name.
🤖 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 172-182: The replace flow in NetworkModificationRepository
currently drops unresolved UUIDs by filtering nulls when building copyEntities,
which can let partial replacements succeed silently. Update the clone/reorder
logic around modificationRepository.findAllByIdIn and copyEntities so every
requested UUID must resolve, and if any UUID is missing, fail the replace
operation with an error instead of proceeding with a partial list.
---
Outside diff comments:
In `@src/main/java/org/gridsuite/modification/server/CompositeController.java`:
- Around line 124-131: The update endpoint in
CompositeController.updateNetworkCompositeModification is rename-only and does
not accept a request body, so the JSON consumes constraint is causing
unnecessary 415 responses for clients using PUT with only the name query
parameter. Remove the MediaType.APPLICATION_JSON_VALUE consumes declaration from
the `@PutMapping` on updateNetworkCompositeModification so the endpoint can be
called without a Content-Type header, while still delegating to
networkModificationService.updateCompositeModification as before.
In
`@src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java`:
- Around line 158-160: The single-composite shortcut in
NetworkModificationRepository is returning the cloned
CompositeModificationEntity before the requested name is applied. Update the
save path in this branch so the copied entity gets the new name from the create
request before calling modificationRepository.save. Make sure the same name
application logic used for the general copyEntities flow is also applied when
copyEntities contains exactly one composite, so the returned id corresponds to a
composite with the requested name.
🪄 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: 5e8c0756-f3d3-411c-8b22-09c7d57ff7a0
📒 Files selected for processing (4)
src/main/java/org/gridsuite/modification/server/CompositeController.javasrc/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.javasrc/main/java/org/gridsuite/modification/server/service/NetworkModificationService.javasrc/test/java/org/gridsuite/modification/server/CompositeControllerTest.java
|
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
3 participants
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.
composite modification will now always have a name.
an endpoint is added in order to update it.