feat: use equipment ID as contingency name for busbar section based on app metadata#3834
feat: use equipment ID as contingency name for busbar section based on app metadata#3834
Conversation
Signed-off-by: Florent <florent@users.noreply.github.com>
📝 WalkthroughWalkthroughAdds a flag that controls whether busbar/equipment IDs are used as contingency names, reads that flag from app metadata in the UI, threads it into map/identifier creation services, updates affected function signatures, and removes an unused exported nominal-voltages helper. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(230,240,255,0.5)
participant User
end
rect rgba(255,245,230,0.5)
participant UIHook as "UI Hook\n(use-save-map)"
end
rect rgba(240,255,240,0.5)
participant Meta as "App Metadata\nService"
end
rect rgba(255,240,245,0.5)
participant MapSvc as "Map Service\n(createMapContingencyList)"
end
rect rgba(245,255,240,0.5)
participant IdSvc as "Identifier Service\n(createIdentifierContingencyList)"
end
User->>UIHook: open/save map panel
UIHook->>Meta: fetchAppsMetadata()
Meta-->>UIHook: app metadata (Study entry)
UIHook->>UIHook: set busbarIdAsContingencyName flag
User->>UIHook: trigger onSaveSelection
UIHook->>MapSvc: createMapContingencyList(..., busbarIdAsContingencyName)
MapSvc->>IdSvc: createIdentifierContingencyList(..., equipmentIdAsContingencyName)
IdSvc-->>MapSvc: identifiers with contingencyId based on flag
MapSvc-->>UIHook: contingency list
UIHook-->>User: save complete / export result
🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/services/study/contingency-list.ts (1)
28-49: Please add focused coverage for both identifier modes.This helper now owns two valid payload shapes (
eq.idandeq.name ?? eq.id), and both will still look reasonable in manual testing. A small unit test aroundcreateIdentifierContingencyListwould make regressions here much easier to catch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/study/contingency-list.ts` around lines 28 - 49, The code lacks unit tests for the two valid payload shapes produced by createIdentifiersList/createIdentifierContingencyList (when equipmentIdAsContingencyName is true vs false). Add focused unit tests for createIdentifierContingencyList: one test with equipmentIdAsContingencyName = true asserting each contingencyId equals eq.id and identifierList contains ID_BASED identifier eq.id, and another with equipmentIdAsContingencyName = false using an EquipmentInfos with name set to assert contingencyId equals eq.name (and falls back to eq.id when name is undefined); also assert the identifierList still contains identifier eq.id and the overall returned array length and type='LIST' for each entry.
🤖 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/network/selection-creation-panel/use-save-map.ts`:
- Around line 33-40: Change equipmentIdAsContingencyName to start as null and
add a metadataLoadError state; update the effect using fetchAppsMetadata to use
an AbortController, catch and set metadataLoadError on rejection, and only call
setEquipmentIdAsContingencyName when not aborted; in onSaveSelection (the code
path that calls createMapContingencyList) guard early and return (or skip
creating contingencies) if equipmentIdAsContingencyName is null or
metadataLoadError is true so createMapContingencyList never runs before metadata
is loaded or when the fetch failed.
---
Nitpick comments:
In `@src/services/study/contingency-list.ts`:
- Around line 28-49: The code lacks unit tests for the two valid payload shapes
produced by createIdentifiersList/createIdentifierContingencyList (when
equipmentIdAsContingencyName is true vs false). Add focused unit tests for
createIdentifierContingencyList: one test with equipmentIdAsContingencyName =
true asserting each contingencyId equals eq.id and identifierList contains
ID_BASED identifier eq.id, and another with equipmentIdAsContingencyName = false
using an EquipmentInfos with name set to assert contingencyId equals eq.name
(and falls back to eq.id when name is undefined); also assert the identifierList
still contains identifier eq.id and the overall returned array length and
type='LIST' for each entry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5f7a10c4-fdcd-44df-8c73-2af188d12fa0
📒 Files selected for processing (3)
src/components/network/selection-creation-panel/use-save-map.tssrc/services/study/contingency-list.tssrc/services/study/network-map.ts
src/components/network/selection-creation-panel/use-save-map.ts
Outdated
Show resolved
Hide resolved
Refactors variable and metadata property to reflect the updated naming convention from equipment ID to busbar ID for contingency-related logic. Updates all associated occurrences to maintain consistency across the codebase. Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/network/selection-creation-panel/use-save-map.ts (1)
33-40:⚠️ Potential issue | 🟠 MajorHandle metadata loading race condition and promise rejection.
This metadata fetch has no error handling and no cleanup. If
fetchAppsMetadata()rejects, the promise rejection is unhandled. Additionally,onSaveSelectioncan be invoked before the fetch completes, causingbusbarIdAsContingencyNameto incorrectly default tofalse.Consider initializing state as
nullto distinguish "not loaded" from "loaded as false", adding error handling, and guarding contingency creation until metadata is available.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/network/selection-creation-panel/use-save-map.ts` around lines 33 - 40, The metadata fetch is unguarded and defaults busbarIdAsContingencyName to false which can race with onSaveSelection; change the state initialization for busbarIdAsContingencyName to null (e.g., useState<boolean | null>(null), update setBusbarIdAsContingencyName accordingly), add error handling and a try/catch around fetchAppsMetadata() inside the useEffect and log or set a safe fallback, add an isMounted/abort flag to the useEffect to avoid setting state after unmount, and update onSaveSelection to check busbarIdAsContingencyName !== null (or wait for metadata) before creating contingencies so the code does not proceed with an incorrect false value.
🧹 Nitpick comments (1)
src/services/study/network-map.ts (1)
8-27: Minor formatting issues flagged by Prettier.The CI pipeline reports spacing inconsistencies in these import statements. Run your formatter (e.g.,
npm run formator Prettier) to auto-fix.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/study/network-map.ts` around lines 8 - 27, The import block has inconsistent spacing/line breaks (e.g., around getStudyUrlWithNodeUuidAndRootNetworkUuid, HvdcLccDeletionInfos, and the grouped imports like backendFetchJson/createFilter), so run the project's formatter (npm run format or Prettier) to normalize spacing and line breaks across imports; alternatively, manually adjust spacing to match the repo style (consistent single-line/grouped imports, no extra blank lines, proper spaces inside braces) for the imports referencing getStudyUrlWithNodeUuidAndRootNetworkUuid, getQueryParamsList, EQUIPMENT_INFOS_TYPES, backendFetchJson/backendFetchText/createFilter, fetchNetworkElementsInfos, createContingencyList, ContingencyList/createIdentifierContingencyList, and HvdcLccDeletionInfos.
🤖 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/services/study/network-map.ts`:
- Around line 270-274: Replace the TypeScript ignore with an ESLint-aware
fallthrough marker: in the switch branch for EquipmentType.BUSBAR_SECTION (where
equipmentIdAsContingencyName = busbarIdAsContingencyName) remove the "//
`@ts-ignore` fallthrough on purpose" and add the recognized fallthrough comment
(e.g. "// falls through") immediately after the assignment so ESLint's
no-fallthrough rule knows the fallthrough from the EquipmentType.BUSBAR_SECTION
case into the default case is intentional.
---
Duplicate comments:
In `@src/components/network/selection-creation-panel/use-save-map.ts`:
- Around line 33-40: The metadata fetch is unguarded and defaults
busbarIdAsContingencyName to false which can race with onSaveSelection; change
the state initialization for busbarIdAsContingencyName to null (e.g.,
useState<boolean | null>(null), update setBusbarIdAsContingencyName
accordingly), add error handling and a try/catch around fetchAppsMetadata()
inside the useEffect and log or set a safe fallback, add an isMounted/abort flag
to the useEffect to avoid setting state after unmount, and update
onSaveSelection to check busbarIdAsContingencyName !== null (or wait for
metadata) before creating contingencies so the code does not proceed with an
incorrect false value.
---
Nitpick comments:
In `@src/services/study/network-map.ts`:
- Around line 8-27: The import block has inconsistent spacing/line breaks (e.g.,
around getStudyUrlWithNodeUuidAndRootNetworkUuid, HvdcLccDeletionInfos, and the
grouped imports like backendFetchJson/createFilter), so run the project's
formatter (npm run format or Prettier) to normalize spacing and line breaks
across imports; alternatively, manually adjust spacing to match the repo style
(consistent single-line/grouped imports, no extra blank lines, proper spaces
inside braces) for the imports referencing
getStudyUrlWithNodeUuidAndRootNetworkUuid, getQueryParamsList,
EQUIPMENT_INFOS_TYPES, backendFetchJson/backendFetchText/createFilter,
fetchNetworkElementsInfos, createContingencyList,
ContingencyList/createIdentifierContingencyList, and HvdcLccDeletionInfos.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e8446b2e-6ad5-4a20-814e-e0783c5285de
📒 Files selected for processing (2)
src/components/network/selection-creation-panel/use-save-map.tssrc/services/study/network-map.ts
src/services/study/network-map.ts
Outdated
| // @ts-ignore fallthrough on purpose | ||
| case EquipmentType.BUSBAR_SECTION: | ||
| equipmentIdAsContingencyName = busbarIdAsContingencyName; | ||
|
|
There was a problem hiding this comment.
let equipmentIdAsContingencyName = equipmentType === EquipmentType.BUSBAR_SECTION ? busbarIdAsContingencyName : false;
There was a problem hiding this comment.
Good idea then no need TS ignore.
I don't really like the fact that we already have a switch for this, but it's okay.
Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/services/study/network-map.ts (1)
268-271:⚠️ Potential issue | 🔴 CriticalUse an ESLint-recognized fallthrough marker (current one can still break CI).
@ts-ignoredoes not satisfy ESLintno-fallthrough. Replace it with a recognized marker (or remove fallthrough via explicit initialization) to avoid lint failure.🐛 Minimal fix
- // `@ts-ignore` fallthrough on purpose case EquipmentType.BUSBAR_SECTION: equipmentIdAsContingencyName = busbarIdAsContingencyName; + // falls through default:#!/bin/bash # Verify fallthrough marker in this switch block (read-only). rg -n "case EquipmentType.BUSBAR_SECTION|@ts-ignore fallthrough on purpose|falls through|default:" src/services/study/network-map.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/study/network-map.ts` around lines 268 - 271, Replace the TypeScript ignore used to silence the fallthrough with an ESLint-recognized fallthrough marker: remove the line containing "@ts-ignore fallthrough on purpose" in the switch handling EquipmentType.BUSBAR_SECTION and add a recognized comment (e.g., /* falls through */) immediately before the next case so the fallthrough between EquipmentType.BUSBAR_SECTION and the assignment to equipmentIdAsContingencyName = busbarIdAsContingencyName is explicit to ESLint; alternatively, break the fallthrough by explicitly initializing equipmentIdAsContingencyName in the BUSBAR_SECTION case if you prefer no-fallthrough behavior.
🤖 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/services/study/network-map.ts`:
- Around line 268-271: Replace the TypeScript ignore used to silence the
fallthrough with an ESLint-recognized fallthrough marker: remove the line
containing "@ts-ignore fallthrough on purpose" in the switch handling
EquipmentType.BUSBAR_SECTION and add a recognized comment (e.g., /* falls
through */) immediately before the next case so the fallthrough between
EquipmentType.BUSBAR_SECTION and the assignment to equipmentIdAsContingencyName
= busbarIdAsContingencyName is explicit to ESLint; alternatively, break the
fallthrough by explicitly initializing equipmentIdAsContingencyName in the
BUSBAR_SECTION case if you prefer no-fallthrough behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c79928f2-e53c-498e-98f4-21ead68597f2
📒 Files selected for processing (1)
src/services/study/network-map.ts
…ipt and ESLint fallthrough issues. Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>
Summary
useBusbarIdAsContingencyNameForPolygonCreationflag from app metadata inuseSaveMaphookequipmentIdAsContingencyNamethroughcreateMapContingencyList→createIdentifierContingencyList→createIdentifiersListeq.idinstead ofeq.nameas the contingency ID