chore: use equipment deletion form from commons UI#3794
Open
chore: use equipment deletion form from commons UI#3794
Conversation
Signed-off-by: David BRAQUART <david.braquart@rte-france.com>
Signed-off-by: David BRAQUART <david.braquart@rte-france.com>
Signed-off-by: David BRAQUART <david.braquart@rte-france.com>
Signed-off-by: David BRAQUART <david.braquart@rte-france.com>
dbraquart
force-pushed
the
dbraquart/use-eqpt-deletion-form-from-commons-ui
branch
from
0411568 to
c10c38a
Compare
| currentRootNetworkUuid, | ||
| editData, | ||
| isUpdate, | ||
| defaultIdValue, // Used to pre-select an equipmentId when calling this dialog from the SLD/map |
…letion-form-from-commons-ui # Conflicts: # src/components/dialogs/network-modifications/equipment-deletion/equipment-deletion-form.tsx # src/services/study/network-modifications.ts # src/translations/en.json # src/translations/fr.json
Signed-off-by: David BRAQUART <david.braquart@rte-france.com>
Signed-off-by: David BRAQUART <david.braquart@rte-france.com>
Signed-off-by: David BRAQUART <david.braquart@rte-france.com>
Signed-off-by: David BRAQUART <david.braquart@rte-france.com>
📝 WalkthroughWalkthroughConsolidates equipment-deletion logic and shared UI into Changes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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
🧹 Nitpick comments (1)
src/hooks/use-equipment-dialogs.tsx (1)
102-108: Please verify the new deletion DTO path still accepts non-UUID equipment ids.This hook still works with plain
stringequipment ids, but the new commons-ui deletion helpers are only being satisfied viaas UUIDcasts here. That hides the mismatch rather than fixing it; ifnewEquipmentDeletionDtoor the shared deletion schema actually enforce UUID-shaped ids, direct deletes and the LCC preset path will stop working for regular network identifiers. The same cast was added inNetworkMapPanel, so it’s worth confirming both entrypoints together.Also applies to: 228-228
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/use-equipment-dialogs.tsx` around lines 102 - 108, The code is casting equipmentId to UUID when calling newEquipmentDeletionDto inside the deleteEquipment path (in use-equipment-dialogs hook) which masks a potential type mismatch: verify whether newEquipmentDeletionDto and the shared deletion schema (used by deleteEquipment and NetworkMapPanel) truly require UUID-shaped ids; if they must accept plain string network identifiers then remove the unsafe "as UUID" casts and update the DTO/type definitions or overloads for newEquipmentDeletionDto/deleteEquipment to accept string | UUID, or alternatively enforce UUID validation at the callsite before casting so direct deletes and the LCC preset path continue to work consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/components/dialogs/network-modifications/equipment-deletion/equipment-deletion-dialog.tsx`:
- Around line 108-119: The effect currently always prefers truthy editData which
can be stale; update the useEffect logic (the block that calls
fromEditDataToFormValues, presetTypeAndId, presetType) to only apply
fromEditDataToFormValues when editData actually corresponds to the dialog
instance (e.g. editData?.id === defaultIdValue or editData?.type ===
equipmentType or another identifying field), otherwise fall through to creation
presets; specifically modify the first branch to guard editData with a match
check and keep presetTypeAndId/presetType branches when
equipmentType/defaultIdValue indicate creation mode so stale editData won't
overwrite presets.
---
Nitpick comments:
In `@src/hooks/use-equipment-dialogs.tsx`:
- Around line 102-108: The code is casting equipmentId to UUID when calling
newEquipmentDeletionDto inside the deleteEquipment path (in
use-equipment-dialogs hook) which masks a potential type mismatch: verify
whether newEquipmentDeletionDto and the shared deletion schema (used by
deleteEquipment and NetworkMapPanel) truly require UUID-shaped ids; if they must
accept plain string network identifiers then remove the unsafe "as UUID" casts
and update the DTO/type definitions or overloads for
newEquipmentDeletionDto/deleteEquipment to accept string | UUID, or
alternatively enforce UUID validation at the callsite before casting so direct
deletes and the LCC preset path continue to work consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fda49a69-bd05-4701-8873-34b53d1105f2
📒 Files selected for processing (21)
src/components/app-wrapper.jsxsrc/components/dialogs/line-types-catalog/line-type-segment-creation.tsxsrc/components/dialogs/line-types-catalog/line-type-segment-form.tsxsrc/components/dialogs/network-modifications/by-filter/by-assignment/modification-by-assignment-form.tsxsrc/components/dialogs/network-modifications/by-filter/by-filter-deletion/by-filter-deletion-form.tsxsrc/components/dialogs/network-modifications/equipment-deletion/equipement-deletion-dialog.type.tssrc/components/dialogs/network-modifications/equipment-deletion/equipment-deletion-dialog.tsxsrc/components/dialogs/network-modifications/equipment-deletion/equipment-deletion-form.tsxsrc/components/dialogs/network-modifications/equipment-deletion/hvdc-lcc-deletion/hvdc-lcc-deletion-specific-form.tsxsrc/components/dialogs/network-modifications/equipment-deletion/hvdc-lcc-deletion/hvdc-lcc-deletion-utils.tssrc/components/dialogs/network-modifications/equipment-deletion/hvdc-lcc-deletion/use-hvdc-lcc-deletion.tssrc/components/graph/menus/network-modifications/network-modification-node-editor.tsxsrc/components/network/network-map-panel.tsxsrc/components/utils/rhf-inputs/read-only/button-read-only-input.tsxsrc/components/utils/rhf-inputs/read-only/read-only-input.tsxsrc/hooks/use-equipment-dialogs.tsxsrc/hooks/use-get-label-equipment-types.tssrc/services/study/network-map.tssrc/services/study/network-modifications.tssrc/translations/en.jsonsrc/translations/fr.json
💤 Files with no reviewable changes (10)
- src/hooks/use-get-label-equipment-types.ts
- src/components/dialogs/network-modifications/equipment-deletion/equipement-deletion-dialog.type.ts
- src/components/dialogs/network-modifications/equipment-deletion/hvdc-lcc-deletion/hvdc-lcc-deletion-utils.ts
- src/components/dialogs/network-modifications/equipment-deletion/hvdc-lcc-deletion/hvdc-lcc-deletion-specific-form.tsx
- src/components/utils/rhf-inputs/read-only/button-read-only-input.tsx
- src/components/dialogs/network-modifications/equipment-deletion/equipment-deletion-form.tsx
- src/components/utils/rhf-inputs/read-only/read-only-input.tsx
- src/translations/fr.json
- src/components/dialogs/network-modifications/equipment-deletion/hvdc-lcc-deletion/use-hvdc-lcc-deletion.ts
- src/translations/en.json
Comment on lines
108
to
+119
| useEffect(() => { | ||
| if (editData) { | ||
| // edition mode | ||
| fromEditDataToFormValues(editData); | ||
| } else if (defaultIdValue) { | ||
| fromMenuDataValues(defaultIdValue); | ||
| } else if (equipmentType && defaultIdValue) { | ||
| // creation mode with both Id and Type (ex: from diagram) | ||
| presetTypeAndId(equipmentType, defaultIdValue); | ||
| } else if (equipmentType) { | ||
| resetFormWithEquipmentType(equipmentType); | ||
| // creation mode with only Type (ex: from node modifications menu) | ||
| presetType(equipmentType); | ||
| } | ||
| }, [ | ||
| fromEditDataToFormValues, | ||
| editData, | ||
| fromMenuDataValues, | ||
| defaultIdValue, | ||
| equipmentType, | ||
| resetFormWithEquipmentType, | ||
| ]); | ||
| }, [defaultIdValue, editData, equipmentType, fromEditDataToFormValues, presetType, presetTypeAndId]); |
There was a problem hiding this comment.
Guard the new creation presets from stale edit data.
A late edit-data response can still land after a previous edit dialog was closed. With the new equipmentType / defaultIdValue branches, this effect will still prefer any truthy editData, so reopening the dialog in creation mode can reset the form with stale deletion values instead of the requested preset.
Suggested fix
- if (editData) {
+ if (isUpdate && editData) {
// edition mode
fromEditDataToFormValues(editData);
} else if (equipmentType && defaultIdValue) {
// creation mode with both Id and Type (ex: from diagram)
presetTypeAndId(equipmentType, defaultIdValue);
} else if (equipmentType) {
// creation mode with only Type (ex: from node modifications menu)
presetType(equipmentType);
}
- }, [defaultIdValue, editData, equipmentType, fromEditDataToFormValues, presetType, presetTypeAndId]);
+ }, [defaultIdValue, editData, equipmentType, fromEditDataToFormValues, isUpdate, presetType, presetTypeAndId]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/components/dialogs/network-modifications/equipment-deletion/equipment-deletion-dialog.tsx`
around lines 108 - 119, The effect currently always prefers truthy editData
which can be stale; update the useEffect logic (the block that calls
fromEditDataToFormValues, presetTypeAndId, presetType) to only apply
fromEditDataToFormValues when editData actually corresponds to the dialog
instance (e.g. editData?.id === defaultIdValue or editData?.type ===
equipmentType or another identifying field), otherwise fall through to creation
presets; specifically modify the first branch to guard editData with a match
check and keep presetTypeAndId/presetType branches when
equipmentType/defaultIdValue indicate creation mode so stale editData won't
overwrite presets.
Signed-off-by: David BRAQUART <david.braquart@rte-france.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/dialogs/network-modifications/equipment-deletion/equipment-deletion-dialog.tsx (1)
112-123:⚠️ Potential issue | 🟠 MajorGuard edit mapping with update mode to avoid stale creation presets.
Line 113 should not prioritize
editDatain creation mode; stale edit payloads can overwriteequipmentType/defaultIdValuepresets.Proposed fix
- useEffect(() => { - if (editData) { + useEffect(() => { + if (isUpdate && editData) { // edition mode fromEditDataToFormValues(editData); } else if (equipmentType && defaultIdValue) { // creation mode with both Id and Type (ex: from diagram) presetTypeAndId(equipmentType, defaultIdValue); } else if (equipmentType) { // creation mode with only Type (ex: from node modifications menu) presetType(equipmentType); } - }, [defaultIdValue, editData, equipmentType, fromEditDataToFormValues, presetType, presetTypeAndId]); + }, [defaultIdValue, editData, equipmentType, fromEditDataToFormValues, isUpdate, presetType, presetTypeAndId]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dialogs/network-modifications/equipment-deletion/equipment-deletion-dialog.tsx` around lines 112 - 123, The effect currently applies editData unconditionally, overwriting creation presets; change the condition to only call fromEditDataToFormValues(editData) when the dialog is in update mode (e.g., guard with an isUpdateMode/isEditing flag) so that creation flows use presetTypeAndId(presetType) when equipmentType/defaultIdValue exist; update the useEffect dependency list to include that mode flag and ensure the branches check (isUpdateMode && editData) first, otherwise preserve the existing equipmentType/defaultIdValue handling using presetType and presetTypeAndId.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@src/components/dialogs/network-modifications/equipment-deletion/equipment-deletion-dialog.tsx`:
- Around line 112-123: The effect currently applies editData unconditionally,
overwriting creation presets; change the condition to only call
fromEditDataToFormValues(editData) when the dialog is in update mode (e.g.,
guard with an isUpdateMode/isEditing flag) so that creation flows use
presetTypeAndId(presetType) when equipmentType/defaultIdValue exist; update the
useEffect dependency list to include that mode flag and ensure the branches
check (isUpdateMode && editData) first, otherwise preserve the existing
equipmentType/defaultIdValue handling using presetType and presetTypeAndId.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d1935305-dc5b-44a6-a271-6e8fc610a83d
📒 Files selected for processing (1)
src/components/dialogs/network-modifications/equipment-deletion/equipment-deletion-dialog.tsx
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
2 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.
PR Summary
All files copied to commons-ui in gridsuite/commons-ui#1049 have been removed from grid-study in this PR.