copy cut paste reference modification and add ref with target infos - #1058
copy cut paste reference modification and add ref with target infos#1058souissimai wants to merge 6 commits into
Conversation
Signed-off-by: SOUISSI Maissa (Externe) <souissimai@gm0winl878.bureau.si.interne>
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesNetwork duplication
Mergeability Score: ⚪ Minimal · up to The change presents no actionable merge-blocking risk based on the available evidence and is ready for normal checks and review. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
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
🧹 Nitpick comments (1)
src/main/java/org/gridsuite/study/server/service/StudyService.java (1)
2167-2175: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd regression coverage for reference registration.
src/test/java/org/gridsuite/study/server/rootnetworks/ModificationToExcludeTest.javaverifies exclusion mappings but not the new directory-server side effect. Add a test that stubsnetworkModificationService.getReferences(...)and verifiesdirectoryService.createsReferencesToSharedComposites(...)receives the shared element UUID andtargetNodeUuid. Cover multiple references and an empty result.🤖 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/study/server/service/StudyService.java` around lines 2167 - 2175, Add regression tests in ModificationToExcludeTest for the reference-registration flow around createReferencesToSharedComposites: stub networkModificationService.getReferences(...) and verify directoryService.createsReferencesToSharedComposites(...) is called with each shared element UUID and targetNodeUuid, covering multiple references and an empty result with no calls.
🤖 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/StudyService.java`:
- Around line 2161-2165: The workflow around
networkModificationService.duplicateModifications must not depend on the local
transaction for remote directory-server writes. Make
createReferencesToSharedComposites idempotent and retryable, or track each
completed reference and compensate both those references and the duplicated
modifications on failure before propagating the error.
---
Nitpick comments:
In `@src/main/java/org/gridsuite/study/server/service/StudyService.java`:
- Around line 2167-2175: Add regression tests in ModificationToExcludeTest for
the reference-registration flow around createReferencesToSharedComposites: stub
networkModificationService.getReferences(...) and verify
directoryService.createsReferencesToSharedComposites(...) is called with each
shared element UUID and targetNodeUuid, covering multiple references and an
empty result with no calls.
🪄 Autofix
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 Plus
Run ID: b24090fe-464b-40e8-9a61-63b234061fd4
📒 Files selected for processing (1)
src/main/java/org/gridsuite/study/server/service/StudyService.java
| createReferencesToSharedComposites(networkModificationService.getReferences(modificationsUuids), userId, targetNodeUuid); | ||
| return networkModificationResults; | ||
| }, | ||
| userId); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not rely on the local transaction to coordinate remote writes.
After networkModificationService.duplicateModifications(...) returns, this code issues one or more remote directory-server POST requests. The local @Transactional boundary cannot roll back a completed remote request. A failure after one request can leave copied modifications with incomplete directory references. Use an idempotent and retryable workflow, or compensate every completed reference and the duplicated modifications before returning an error.
🤖 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/study/server/service/StudyService.java` around
lines 2161 - 2165, The workflow around
networkModificationService.duplicateModifications must not depend on the local
transaction for remote directory-server writes. Make
createReferencesToSharedComposites idempotent and retryable, or track each
completed reference and compensate both those references and the duplicated
modifications on failure before propagating the error.
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/test/java/org/gridsuite/study/server/NetworkModificationTest.java`:
- Around line 2346-2350: Update the reference lookup stubs and verifications in
the affected duplication tests to match the expected original and copied UUID
query values explicitly, rather than accepting any uuids parameter. Verify each
duplication operation performs exactly one lookup with its correct UUIDs,
including the cases currently using aggregate counts, while preserving the
existing no-reference response behavior.
🪄 Autofix
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 Plus
Run ID: 172843bd-b608-4798-9bac-0691ad90f60c
📒 Files selected for processing (1)
src/test/java/org/gridsuite/study/server/NetworkModificationTest.java
| // no shared-composite reference among the duplicated modifications | ||
| wireMockServer.stubFor(WireMock.get(WireMock.urlPathEqualTo("/v1/references")) | ||
| .willReturn(WireMock.ok() | ||
| .withBody(mapper.writeValueAsString(Map.of())) | ||
| .withHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE))); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Assert the UUIDs used in each reference lookup.
The /v1/references stubs match any query, and the verifications accept any uuids value. These tests can pass when the service omits an ID, sends the wrong original or copied ID, or sends an empty value. At Line 2405, the aggregate count across two duplications also does not prove that each operation performs one lookup. Match the expected UUIDs for each request and verify the count per duplication operation.
Also applies to: 2405-2405, 2452-2457, 2475-2475, 2512-2515, 2538-2538
🤖 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 2346 - 2350, Update the reference lookup stubs and verifications in the
affected duplication tests to match the expected original and copied UUID query
values explicitly, rather than accepting any uuids parameter. Verify each
duplication operation performs exactly one lookup with its correct UUIDs,
including the cases currently using aggregate counts, while preserving the
existing no-reference response behavior.
|
1 participant
PR Summary