Skip to content

merge some modifications into a new composite#987

Open
Mathieu-Deharbe wants to merge 10 commits into
mainfrom
merge-modifications-into-composite
Open

merge some modifications into a new composite#987
Mathieu-Deharbe wants to merge 10 commits into
mainfrom
merge-modifications-into-composite

Conversation

@Mathieu-Deharbe
Copy link
Copy Markdown
Contributor

PR Summary

merge some modifications into a new composite

Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
@Mathieu-Deharbe Mathieu-Deharbe self-assigned this Apr 30, 2026
@Mathieu-Deharbe Mathieu-Deharbe marked this pull request as draft April 30, 2026 15:16
@coderabbitai

This comment was marked as spam.

Mathieu-Deharbe and others added 6 commits May 5, 2026 15:24
Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
@Mathieu-Deharbe Mathieu-Deharbe marked this pull request as ready for review May 18, 2026 11:30
coderabbitai[bot]

This comment was marked as resolved.

Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

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/test/java/org/gridsuite/study/server/NetworkModificationTest.java`:
- Around line 2158-2163: The test currently only asserts the POST to
"/v1/network-composite-modifications/composite-modification" was called; update
the assertion to also verify the request body contains the expected modification
UUIDs produced by NetworkModificationService.mergeModificationsIntoComposite.
Build the same expectedBody map/JSON used in testInsertComposite (or mirror its
structure) and call the overload of WireMockUtilsCriteria.verifyPostRequest that
accepts an expected body (or use the equivalent body-verifying helper used for
verifyPutRequest) to assert the payload matches the expected modification UUID
list; ensure you reference the same endpoint and request-count arguments as
before.
🪄 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: c18d804c-0ba6-4ed7-87d2-423e997c4b08

📥 Commits

Reviewing files that changed from the base of the PR and between 2eb2141 and 086eb96.

📒 Files selected for processing (2)
  • src/main/java/org/gridsuite/study/server/service/RebuildNodeService.java
  • src/test/java/org/gridsuite/study/server/NetworkModificationTest.java

Comment on lines +2158 to +2163
WireMockUtilsCriteria.verifyPostRequest(
wireMockServer,
"/v1/network-composite-modifications/composite-modification",
Map.of(),
1
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Verify the request body sent to the composite-modification endpoint.

The test verifies that the WireMock POST endpoint was called, but does not verify that the correct modification UUIDs were sent in the request body. According to the service layer contract, NetworkModificationService.mergeModificationsIntoComposite should POST the modification UUIDs to the downstream API. Similar tests in this file (e.g., testInsertComposite at lines 2106-2116) construct an expected body and verify it was sent.

🧪 Suggested enhancement to verify request body
+        String expectedBody = mapper.writeValueAsString(modificationUuids);
+
         WireMockUtilsCriteria.verifyPostRequest(
                 wireMockServer,
                 "/v1/network-composite-modifications/composite-modification",
                 Map.of(),
+                expectedBody,
                 1
         );

Note: You may need to verify the actual signature of WireMockUtilsCriteria.verifyPostRequest to ensure it accepts a body parameter. If the current overload only accepts URL, query params, and count, you might need to use a different verification method that includes body matching, similar to how testInsertComposite uses WireMockUtilsCriteria.verifyPutRequest with an expectedBody parameter.

🤖 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/NetworkModificationTest.java` around
lines 2158 - 2163, The test currently only asserts the POST to
"/v1/network-composite-modifications/composite-modification" was called; update
the assertion to also verify the request body contains the expected modification
UUIDs produced by NetworkModificationService.mergeModificationsIntoComposite.
Build the same expectedBody map/JSON used in testInsertComposite (or mirror its
structure) and call the overload of WireMockUtilsCriteria.verifyPostRequest that
accepts an expected body (or use the equivalent body-verifying helper used for
verifyPutRequest) to assert the payload matches the expected modification UUID
list; ensure you reference the same endpoint and request-count arguments as
before.

Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
@sonarqubecloud
Copy link
Copy Markdown

@Mathieu-Deharbe Mathieu-Deharbe changed the title draft merge some modifications into a new composite merge some modifications into a new composite May 18, 2026
Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant