Save line segments infos, used to generate limits, from catalog#3829
Open
Save line segments infos, used to generate limits, from catalog#3829
Conversation
|
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:
📝 WalkthroughWalkthroughAdds per-segment fields (area, temperature, shape factor) and a new LineSegmentInfos type; propagates editable segment arrays through segment dialog/form and catalog selector; recalculates per-segment limits; and wires segment arrays into line creation via getValues/setValue and setSegments, with dialog onSubmit forwarding data and calling onSave. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Creation as LineCreationDialog
participant SegmentDlg as LineTypeSegmentDialog
participant SegmentForm as LineTypeSegmentForm
participant CatalogDlg as LineTypesCatalogSelectorDialog
participant API as getLineTypeWithLimits
User->>Creation: Open line creation dialog
User->>Creation: Click "Edit Segments"
Creation->>SegmentDlg: open(editData=getValues(LINE_SEGMENTS), setSegments)
SegmentDlg->>SegmentForm: render(editData)
SegmentForm->>SegmentForm: useEffect -> populate form from editData
User->>SegmentForm: Open catalog for a segment
SegmentForm->>CatalogDlg: open(preselectedParams: area,temperature,shapeFactor)
User->>CatalogDlg: Select catalog line
CatalogDlg->>SegmentForm: return selected line data
SegmentForm->>API: fetchLimits(id, area, temperature, shapeFactor)
API->>SegmentForm: return limits
SegmentForm->>SegmentForm: update SEGMENT_CURRENT_LIMITS, recalc values/totals
User->>SegmentForm: Submit segments
SegmentForm->>SegmentDlg: form values
SegmentDlg->>Creation: setSegments(segments)
Creation->>Creation: setValue(LINE_SEGMENTS, segments)
SegmentDlg->>Creation: onSave(data)
🚥 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: 4
🧹 Nitpick comments (1)
src/components/dialogs/line-types-catalog/line-type-segment-dialog.tsx (1)
74-92: Consider using field constants instead of string literals for maintainability.The implementation is functionally correct. However, the property names in the mapping (lines 80-84) use string literals that duplicate the field constant values. Using the imported constants would improve maintainability.
♻️ Proposed refactor to use field constants
First, add to imports:
import { AREA, FINAL_CURRENT_LIMITS, SEGMENT_DISTANCE_VALUE, SEGMENT_TYPE_ID, SEGMENTS, SHAPE_FACTOR, TEMPERATURE, TOTAL_REACTANCE, TOTAL_RESISTANCE, TOTAL_SUSCEPTANCE, } from '../../utils/field-constants';Then update the mapping:
const segments: LineSegmentInfos[] = segmentsData.map((segment) => { return { - segmentTypeId: segment?.segmentTypeId ?? '', - segmentDistanceValue: segment?.segmentDistanceValue ?? 0, - area: segment?.area ?? '', - temperature: segment?.temperature ?? '', - shapeFactor: segment?.shapeFactor ?? null, + [SEGMENT_TYPE_ID]: segment?.[SEGMENT_TYPE_ID] ?? '', + [SEGMENT_DISTANCE_VALUE]: segment?.[SEGMENT_DISTANCE_VALUE] ?? 0, + [AREA]: segment?.[AREA] ?? '', + [TEMPERATURE]: segment?.[TEMPERATURE] ?? '', + [SHAPE_FACTOR]: segment?.[SHAPE_FACTOR] ?? null, }; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dialogs/line-types-catalog/line-type-segment-dialog.tsx` around lines 74 - 92, The mapping in onSubmit (function onSubmit) currently uses string literals for segment property names when building LineSegmentInfos; replace those literals with the corresponding imported field constants (e.g., use SEGMENT_TYPE_ID, SEGMENT_DISTANCE_VALUE, AREA, TEMPERATURE, SHAPE_FACTOR) and ensure those constants are imported from the field-constants module so the segmentsData mapping uses the constants instead of hard-coded strings before calling setSegments.
🤖 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/line-types-catalog/line-type-segment-form.tsx`:
- Around line 128-135: The async fetches started in fetchLimits (which calls
getLineTypeWithLimits and updateSegmentLimitsValues) must be awaited before
recomputing FINAL_CURRENT_LIMITS because keepMostConstrainingLimits reads
SEGMENT_CURRENT_LIMITS; modify fetchLimits to return the promise from
getLineTypeWithLimits (or an explicit Promise) and change the caller (the loop
around lines 199-223) to collect all fetch promises and use Promise.all(...) to
wait for them to resolve before invoking keepMostConstrainingLimits() to
recompute FINAL_CURRENT_LIMITS; ensure updateSegmentLimitsValues is called
inside each fetch's then and that the recompute runs only after all promises
settle.
- Around line 128-135: fetchLimits currently calls getLineTypeWithLimits without
catching rejections which causes unhandled promise errors during edit hydration;
wrap the promise call in a try/catch (or add .catch) around
getLineTypeWithLimits inside fetchLimits, log/report the error (e.g.,
console.error or use existing logger/notification), and ensure you call
updateSegmentLimitsValues(index, ...) with a safe fallback (empty array or
previously-known limits) when the backend fails so the segment limits are not
left unset; reference fetchLimits, getLineTypeWithLimits, and
updateSegmentLimitsValues when locating the change.
- Around line 249-256: The code sets AREA and TEMPERATURE but omits persisting
SHAPE_FACTOR, causing later reads (e.g., getPreselectedRowData) to drift; update
the same selection block that calls setValue for
`${SEGMENTS}.${openCatalogDialogIndex}.${AREA}` and
`${SEGMENTS}.${openCatalogDialogIndex}.${TEMPERATURE}` to also call setValue for
`${SEGMENTS}.${openCatalogDialogIndex}.${SHAPE_FACTOR}` using
selectedAreaAndTemperature2LineTypeData?.shapeFactor (ensure you reference the
SHAPE_FACTOR constant and the existing selectedAreaAndTemperature2LineTypeData
and openCatalogDialogIndex symbols so the form stores the chosen shapeFactor
along with area and temperature).
In
`@src/components/dialogs/line-types-catalog/line-types-catalog-selector-form.tsx`:
- Around line 79-86: The reset() call inside the preselectedRowData branch is
replacing the entire form state and wiping SELECTED_CATEGORIES_TAB; instead,
update only the specific fields (AERIAL_AREAS, AERIAL_TEMPERATURES,
UNDERGROUND_AREAS, UNDERGROUND_SHAPE_FACTORS) using setValue() so the previously
set SELECTED_CATEGORIES_TAB is preserved, or include SELECTED_CATEGORIES_TAB in
the reset payload if you must use reset(); also ensure values assigned to the
AutocompleteInput-backed fields (preselectedRowData.area and
preselectedRowData.temperature) are converted into the expected Option objects
(with id and label) before calling setValue() or reset().
---
Nitpick comments:
In `@src/components/dialogs/line-types-catalog/line-type-segment-dialog.tsx`:
- Around line 74-92: The mapping in onSubmit (function onSubmit) currently uses
string literals for segment property names when building LineSegmentInfos;
replace those literals with the corresponding imported field constants (e.g.,
use SEGMENT_TYPE_ID, SEGMENT_DISTANCE_VALUE, AREA, TEMPERATURE, SHAPE_FACTOR)
and ensure those constants are imported from the field-constants module so the
segmentsData mapping uses the constants instead of hard-coded strings before
calling setSegments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c5acdae8-e515-4cf5-824d-321804d61169
📒 Files selected for processing (9)
src/components/dialogs/line-types-catalog/line-type-segment-dialog.tsxsrc/components/dialogs/line-types-catalog/line-type-segment-form.tsxsrc/components/dialogs/line-types-catalog/line-types-catalog-selector-dialog.tsxsrc/components/dialogs/line-types-catalog/line-types-catalog-selector-form.tsxsrc/components/dialogs/line-types-catalog/segment-utils.tssrc/components/dialogs/network-modifications/line/creation/line-creation-dialog.tsxsrc/components/dialogs/network-modifications/line/creation/line-creation-type.tssrc/components/utils/field-constants.tssrc/services/network-modification-types.ts
src/components/dialogs/line-types-catalog/line-type-segment-form.tsx
Outdated
Show resolved
Hide resolved
src/components/dialogs/line-types-catalog/line-types-catalog-selector-form.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/dialogs/line-types-catalog/line-type-segment-form.tsx (1)
128-135:⚠️ Potential issue | 🔴 CriticalAwait and handle the edit-mode limit preload before recomputing final limits.
updateLimits()still fire-and-forgetsgetLineTypeWithLimits(), while the hydration effect immediately callskeepMostConstrainingLimits(). That leavesFINAL_CURRENT_LIMITSderived from empty/staleSEGMENT_CURRENT_LIMITS, and a failed preload turns into an unhandled rejection. Make the helper awaitable, catch/report failures there, and recompute once after all segment fetches settle.Suggested fix
const updateLimits = useCallback( - (index: number, id: string, area: string | null, temperature: string | null, shapeFactor: number | null) => { - getLineTypeWithLimits(id, area, temperature, shapeFactor).then((lineTypeWithLimits) => { - updateSegmentLimitsValues(index, lineTypeWithLimits.limitsForLineType); - }); - }, - [updateSegmentLimitsValues] + async ( + index: number, + id: string, + area: string | null, + temperature: string | null, + shapeFactor: number | null + ) => { + try { + const lineTypeWithLimits = await getLineTypeWithLimits(id, area, temperature, shapeFactor); + updateSegmentLimitsValues(index, lineTypeWithLimits.limitsForLineType); + } catch (error) { + updateSegmentLimitsValues(index, []); + snackWithFallback(snackError, error, { + headerId: 'LineTypesCatalogFetchingError', + }); + } + }, + [snackError, updateSegmentLimitsValues] ); useEffect(() => { if (!editData) { return; } - for (let index = 0; index < editData?.length; index++) { - const segmentCatalogId = editData[index][SEGMENT_TYPE_ID]; - setValue(`${SEGMENTS}.${index}.${SEGMENT_TYPE_ID}`, segmentCatalogId); - const lineTypeInfo: LineTypeInfo | undefined = lineTypesCatalog.find( - (segment) => segment.id === segmentCatalogId - ); - setValue(`${SEGMENTS}.${index}.${SEGMENT_TYPE_VALUE}`, lineTypeInfo?.type ?? ''); - setValue(`${SEGMENTS}.${index}.${SEGMENT_DISTANCE_VALUE}`, Number(editData[index][SEGMENT_DISTANCE_VALUE])); - setValue(`${SEGMENTS}.${index}.${AREA}`, editData[index][AREA]); - setValue(`${SEGMENTS}.${index}.${TEMPERATURE}`, editData[index][TEMPERATURE]); - setValue(`${SEGMENTS}.${index}.${SHAPE_FACTOR}`, editData[index][SHAPE_FACTOR]); - updateLimits( - index, - segmentCatalogId, - editData[index][AREA], - editData[index][TEMPERATURE], - editData[index][SHAPE_FACTOR] - ); - updateSegmentValues(index); - updateTotals(); - keepMostConstrainingLimits(); - } + const hydrateSegments = async () => { + const pendingLimits: Promise<void>[] = []; + + for (let index = 0; index < editData.length; index++) { + const segmentCatalogId = editData[index][SEGMENT_TYPE_ID]; + setValue(`${SEGMENTS}.${index}.${SEGMENT_TYPE_ID}`, segmentCatalogId); + const lineTypeInfo: LineTypeInfo | undefined = lineTypesCatalog.find( + (segment) => segment.id === segmentCatalogId + ); + setValue(`${SEGMENTS}.${index}.${SEGMENT_TYPE_VALUE}`, lineTypeInfo?.type ?? ''); + setValue(`${SEGMENTS}.${index}.${SEGMENT_DISTANCE_VALUE}`, Number(editData[index][SEGMENT_DISTANCE_VALUE])); + setValue(`${SEGMENTS}.${index}.${AREA}`, editData[index][AREA]); + setValue(`${SEGMENTS}.${index}.${TEMPERATURE}`, editData[index][TEMPERATURE]); + setValue(`${SEGMENTS}.${index}.${SHAPE_FACTOR}`, editData[index][SHAPE_FACTOR]); + pendingLimits.push( + updateLimits( + index, + segmentCatalogId, + editData[index][AREA], + editData[index][TEMPERATURE], + editData[index][SHAPE_FACTOR] + ) + ); + updateSegmentValues(index); + } + + await Promise.allSettled(pendingLimits); + updateTotals(); + keepMostConstrainingLimits(); + }; + + void hydrateSegments(); }, [Also applies to: 199-223
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dialogs/line-types-catalog/line-type-segment-form.tsx` around lines 128 - 135, The updateLimits helper currently fire-and-forgets getLineTypeWithLimits causing race conditions and unhandled rejections; make updateLimits return a Promise (i.e., async) that awaits getLineTypeWithLimits, catches and logs/report errors, and only calls updateSegmentLimitsValues after the awaited result; then update the hydration logic that computes FINAL_CURRENT_LIMITS from SEGMENT_CURRENT_LIMITS to await Promise.allSettled([...segments].map(s => updateLimits(...))) (also apply same change to the other helper around lines 199-223) and finally invoke keepMostConstrainingLimits() once after all fetches settle so recomputation uses hydrated limits and failures are handled.
🤖 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/line-types-catalog/line-type-segment-form.tsx`:
- Around line 128-135: The updateLimits helper currently fire-and-forgets
getLineTypeWithLimits causing race conditions and unhandled rejections; make
updateLimits return a Promise (i.e., async) that awaits getLineTypeWithLimits,
catches and logs/report errors, and only calls updateSegmentLimitsValues after
the awaited result; then update the hydration logic that computes
FINAL_CURRENT_LIMITS from SEGMENT_CURRENT_LIMITS to await
Promise.allSettled([...segments].map(s => updateLimits(...))) (also apply same
change to the other helper around lines 199-223) and finally invoke
keepMostConstrainingLimits() once after all fetches settle so recomputation uses
hydrated limits and failures are handled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ba08dc12-ce71-49a7-9c03-67db1c39d99b
📒 Files selected for processing (2)
src/components/dialogs/line-types-catalog/line-type-segment-form.tsxsrc/components/dialogs/line-types-catalog/line-types-catalog-selector-form.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/dialogs/line-types-catalog/line-types-catalog-selector-form.tsx
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/dialogs/line-types-catalog/line-types-catalog-selector-form.tsx (1)
73-85:⚠️ Potential issue | 🟠 MajorSync tab UI state when applying preselection.
At Line 77, only form state is updated. The rendered tab uses
tabIndex, so the UI can stay on the wrong tab for underground preselection.🐛 Proposed fix
setValue(SELECTED_CATEGORIES_TAB, newTabIndex); + setTabIndex(newTabIndex); if (preselectedRowData) { setValue(AERIAL_AREAS, preselectedRowData.area); setValue(AERIAL_TEMPERATURES, preselectedRowData.temperature); setValue(UNDERGROUND_AREAS, preselectedRowData.area); setValue(UNDERGROUND_SHAPE_FACTORS, preselectedRowData.shapeFactor); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dialogs/line-types-catalog/line-types-catalog-selector-form.tsx` around lines 73 - 85, The effect that applies preselection sets form values (SELECTED_CATEGORIES_TAB and various area/temperature/shape fields) but doesn't update the component's visible tab state, so the UI can remain on the wrong tab; after computing newTabIndex (based on preselectedRow and CATEGORIES_TABS) also call the tab state updater (e.g., setTabIndex or whatever state setter controls the rendered tabIndex) to keep the rendered tab in sync with the form value; update the effect that references preselectedRow, preselectedRowData and setValue to also call the tab state setter with newTabIndex whenever you set SELECTED_CATEGORIES_TAB.
♻️ Duplicate comments (1)
src/components/dialogs/line-types-catalog/line-types-catalog-selector-form.tsx (1)
27-31:⚠️ Potential issue | 🟠 MajorNormalize preselected values to match Autocomplete field value shape.
AutocompleteInputfields (Line 79-83) useOption[]andareIdsEqual; setting raw primitives can fail to display a selected value. Also,shapeFactoris typed asnumberhere (Line 30), whilesrc/components/dialogs/line-types-catalog/line-type-segment-form.tsx(Line 318-325) sources it fromgetValues(...), which is often string-typed in form state.♻️ Suggested direction
export interface LineCatalogParams { area: string; temperature: string; - shapeFactor: number; + shapeFactor: string | number; } @@ if (preselectedRowData) { - setValue(AERIAL_AREAS, preselectedRowData.area); - setValue(AERIAL_TEMPERATURES, preselectedRowData.temperature); - setValue(UNDERGROUND_AREAS, preselectedRowData.area); - setValue(UNDERGROUND_SHAPE_FACTORS, preselectedRowData.shapeFactor); + const areaOption = areasOptions.find((o) => String(o.id) === String(preselectedRowData.area)); + const tempOption = aerialTemperatures.find((o) => String(o.id) === String(preselectedRowData.temperature)); + const shapeOption = undergroundShapeFactor.find( + (o) => String(o.id) === String(preselectedRowData.shapeFactor) + ); + + setValue(AERIAL_AREAS, areaOption ?? null); + setValue(AERIAL_TEMPERATURES, tempOption ?? null); + setValue(UNDERGROUND_AREAS, areaOption ?? null); + setValue(UNDERGROUND_SHAPE_FACTORS, shapeOption ?? null); }#!/bin/bash set -euo pipefail # Verify Option shape and AutocompleteInput usage contracts inside the repo rg -n -C3 'export (type|interface) Option|type Option|interface Option|AutocompleteInput|areIdsEqual' # Verify how these form fields are assigned and consumed rg -n -C3 'AERIAL_AREAS|AERIAL_TEMPERATURES|UNDERGROUND_AREAS|UNDERGROUND_SHAPE_FACTORS|setValue\(' # Verify source type of preselected shapeFactor rg -n -C3 'getPreselectedRowData|shapeFactor:\s*getValues|SEGMENTS\..*SHAPE_FACTOR'Also applies to: 79-83
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dialogs/line-types-catalog/line-types-catalog-selector-form.tsx` around lines 27 - 31, LineCatalogParams uses raw primitives which don't match the Option[] shape expected by the AutocompleteInput (see AutocompleteInput usage lines 79-83 and areIdsEqual), and shapeFactor is typed as number but is sourced as a string via getValues in line-type-segment-form.tsx; update LineCatalogParams so area and temperature are Option (or Option | null) to match the Autocomplete value shape and change shapeFactor to string | number (or normalize it on read) so values from getValues display correctly, then ensure where preselected data is assigned (e.g., setValue or preselectedRow handling) you convert primitives into the Option shape using the same id/label structure used by AERIAL_AREAS/AERIAL_TEMPERATURES/UNDERGROUND_SHAPE_FACTORS and coerce shapeFactor to the expected numeric/string type before passing to the form.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@src/components/dialogs/line-types-catalog/line-types-catalog-selector-form.tsx`:
- Around line 73-85: The effect that applies preselection sets form values
(SELECTED_CATEGORIES_TAB and various area/temperature/shape fields) but doesn't
update the component's visible tab state, so the UI can remain on the wrong tab;
after computing newTabIndex (based on preselectedRow and CATEGORIES_TABS) also
call the tab state updater (e.g., setTabIndex or whatever state setter controls
the rendered tabIndex) to keep the rendered tab in sync with the form value;
update the effect that references preselectedRow, preselectedRowData and
setValue to also call the tab state setter with newTabIndex whenever you set
SELECTED_CATEGORIES_TAB.
---
Duplicate comments:
In
`@src/components/dialogs/line-types-catalog/line-types-catalog-selector-form.tsx`:
- Around line 27-31: LineCatalogParams uses raw primitives which don't match the
Option[] shape expected by the AutocompleteInput (see AutocompleteInput usage
lines 79-83 and areIdsEqual), and shapeFactor is typed as number but is sourced
as a string via getValues in line-type-segment-form.tsx; update
LineCatalogParams so area and temperature are Option (or Option | null) to match
the Autocomplete value shape and change shapeFactor to string | number (or
normalize it on read) so values from getValues display correctly, then ensure
where preselected data is assigned (e.g., setValue or preselectedRow handling)
you convert primitives into the Option shape using the same id/label structure
used by AERIAL_AREAS/AERIAL_TEMPERATURES/UNDERGROUND_SHAPE_FACTORS and coerce
shapeFactor to the expected numeric/string type before passing to the form.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: da10ad62-53ea-4d43-b767-143009873b33
📒 Files selected for processing (4)
src/components/dialogs/line-types-catalog/line-types-catalog-selector-form.tsxsrc/components/dialogs/network-modifications/line/creation/line-creation-dialog.tsxsrc/components/dialogs/network-modifications/line/creation/line-creation-type.tssrc/services/network-modification-types.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/dialogs/network-modifications/line/creation/line-creation-type.ts
- src/services/network-modification-types.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/dialogs/line-types-catalog/line-types-catalog-selector-form.tsx (1)
79-85: Consider guarding one-time prefill to avoid clobbering user edits.Because
preselectedRowDatais in the dependency list, any identity change can re-run Line 79–Line 84 and overwrite in-progress edits.♻️ Optional refactor (hydrate once per dialog open / preselected key)
+ const [hasHydratedPreselection, setHasHydratedPreselection] = useState(false); + useEffect(() => { if (preselectedRowId && rowData) { const preselectedRow = rowData?.find((entry) => entry.id === preselectedRowId); const newTabIndex = preselectedRow?.category === CATEGORIES_TABS.UNDERGROUND.name ? CATEGORIES_TABS.UNDERGROUND.id : CATEGORIES_TABS.AERIAL.id; setValue(SELECTED_CATEGORIES_TAB, newTabIndex); } - if (preselectedRowData) { + if (preselectedRowData && !hasHydratedPreselection) { setValue(AERIAL_AREAS, preselectedRowData.area); setValue(AERIAL_TEMPERATURES, preselectedRowData.temperature); setValue(UNDERGROUND_AREAS, preselectedRowData.area); setValue(UNDERGROUND_SHAPE_FACTORS, preselectedRowData.shapeFactor); + setHasHydratedPreselection(true); } - }, [rowData, preselectedRowId, setValue, preselectedRowData]); + }, [rowData, preselectedRowId, setValue, preselectedRowData, hasHydratedPreselection]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dialogs/line-types-catalog/line-types-catalog-selector-form.tsx` around lines 79 - 85, The effect that sets form fields (setValue calls to AERIAL_AREAS, AERIAL_TEMPERATURES, UNDERGROUND_AREAS, UNDERGROUND_SHAPE_FACTORS) runs whenever preselectedRowData identity changes and can clobber user edits; change the logic to only hydrate once per dialog open or per preselected key (e.g., track a local ref/flag like hasHydrated or store prevPreselectedRowId) and run the prefill only when the dialog opens or when preselectedRowId changes and hasHydrated is false, then set hasHydrated = true after calling setValue (or reset it when the dialog closes) so subsequent preselectedRowData identity changes won’t overwrite in-progress user edits.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/components/dialogs/line-types-catalog/line-types-catalog-selector-form.tsx`:
- Around line 79-85: The effect that sets form fields (setValue calls to
AERIAL_AREAS, AERIAL_TEMPERATURES, UNDERGROUND_AREAS, UNDERGROUND_SHAPE_FACTORS)
runs whenever preselectedRowData identity changes and can clobber user edits;
change the logic to only hydrate once per dialog open or per preselected key
(e.g., track a local ref/flag like hasHydrated or store prevPreselectedRowId)
and run the prefill only when the dialog opens or when preselectedRowId changes
and hasHydrated is false, then set hasHydrated = true after calling setValue (or
reset it when the dialog closes) so subsequent preselectedRowData identity
changes won’t overwrite in-progress user edits.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7f1a4709-1b21-4e4d-a1ea-b69de900efd9
📒 Files selected for processing (1)
src/components/dialogs/line-types-catalog/line-types-catalog-selector-form.tsx
…e catalog Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
basseche
force-pushed
the
save_lineSegments
branch
from
75cfa56 to
eb18831
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/dialogs/line-types-catalog/line-type-segment-form.tsx (1)
177-199:⚠️ Potential issue | 🟠 MajorCatch catalog fetch failures in
updateLimits().This helper is now part of edit-data hydration, but unlike the manual selection path it still lets
getLineTypeWithLimits()reject silently. A failed preload leaves the form partially hydrated and turns the backend error into an unhandled promise instead of a user-visible catalog error.Suggested fix
const updateLimits = useCallback( - (index: number, id: string, area: string | null, temperature: string | null, shapeFactor: number | null, distance: number) => { - getLineTypeWithLimits(id, area, temperature, shapeFactor).then((lineTypeWithLimits) => { + async ( + index: number, + id: string, + area: string | null, + temperature: string | null, + shapeFactor: number | null, + distance: number + ) => { + try { + const lineTypeWithLimits = await getLineTypeWithLimits(id, area, temperature, shapeFactor); const newResistance = roundToDefaultPrecision( calculateResistance(distance, lineTypeWithLimits?.linearResistance ?? 0) ); const newReactance = roundToDefaultPrecision( calculateReactance(distance, lineTypeWithLimits?.linearReactance ?? 0) @@ setValue(`${SEGMENTS}.${index}.${SEGMENT_SUSCEPTANCE}`, newSusceptance); updateTotals(); updateSegmentLimitsValues(index, lineTypeWithLimits.limitsForLineType); keepMostConstrainingLimits(); - }); + } catch (error) { + snackWithFallback(snackError, error, { + headerId: 'LineTypesCatalogFetchingError', + }); + } }, - [keepMostConstrainingLimits, setValue, updateSegmentLimitsValues, updateTotals] + [keepMostConstrainingLimits, setValue, snackError, updateSegmentLimitsValues, updateTotals] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dialogs/line-types-catalog/line-type-segment-form.tsx` around lines 177 - 199, The updateLimits callback calls getLineTypeWithLimits(...) but does not handle rejections; wrap the promise call in a try/catch (or use .catch) so fetch failures are caught, surface a user-visible error (e.g., call the existing error/UI-toast handler) and restore consistent form state (set default resist/react/susceptance to 0 or previous values and still call updateTotals(), updateSegmentLimitsValues(index, []) or a safe fallback, and keepMostConstrainingLimits()) to avoid leaving the form partially hydrated; update the code in updateLimits to handle both success and failure paths and reference getLineTypeWithLimits, setValue, updateTotals, updateSegmentLimitsValues, and keepMostConstrainingLimits.
🤖 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/line-types-catalog/line-type-segment-form.tsx`:
- Around line 177-199: updateLimits currently fetches linear characteristics
from getLineTypeWithLimits but then lets the older recalculation path
(updateSegmentValues) continue to recompute R/X/B from the base catalog row,
causing divergent results; modify updateLimits (and the similar block at lines
~247-277) so that after receiving lineTypeWithLimits you do not call
updateSegmentValues but instead apply the parameterized recalculation using
lineTypeWithLimits' linearResistance/linearReactance/linearCapacity (via
setValue for SEGMENT_RESISTANCE/SEGMENT_REACTANCE/SEGMENT_SUSCEPTANCE or by
calling a recalculation helper that accepts those parameters), then call
updateSegmentLimitsValues(index, lineTypeWithLimits.limitsForLineType),
updateTotals(), and keepMostConstrainingLimits() — ensure updateSegmentValues is
not invoked for catalog-selection flows so both saved and reopened segments use
the same lineTypeWithLimits-based calculation path.
---
Duplicate comments:
In `@src/components/dialogs/line-types-catalog/line-type-segment-form.tsx`:
- Around line 177-199: The updateLimits callback calls
getLineTypeWithLimits(...) but does not handle rejections; wrap the promise call
in a try/catch (or use .catch) so fetch failures are caught, surface a
user-visible error (e.g., call the existing error/UI-toast handler) and restore
consistent form state (set default resist/react/susceptance to 0 or previous
values and still call updateTotals(), updateSegmentLimitsValues(index, []) or a
safe fallback, and keepMostConstrainingLimits()) to avoid leaving the form
partially hydrated; update the code in updateLimits to handle both success and
failure paths and reference getLineTypeWithLimits, setValue, updateTotals,
updateSegmentLimitsValues, and keepMostConstrainingLimits.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7181095e-b59a-4c44-81e0-39fe90091b92
📒 Files selected for processing (9)
src/components/dialogs/line-types-catalog/line-type-segment-dialog.tsxsrc/components/dialogs/line-types-catalog/line-type-segment-form.tsxsrc/components/dialogs/line-types-catalog/line-types-catalog-selector-dialog.tsxsrc/components/dialogs/line-types-catalog/line-types-catalog-selector-form.tsxsrc/components/dialogs/line-types-catalog/segment-utils.tssrc/components/dialogs/network-modifications/line/creation/line-creation-dialog.tsxsrc/components/dialogs/network-modifications/line/creation/line-creation-type.tssrc/components/utils/field-constants.tssrc/services/network-modification-types.ts
✅ Files skipped from review due to trivial changes (2)
- src/components/dialogs/network-modifications/line/creation/line-creation-type.ts
- src/components/utils/field-constants.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/components/dialogs/line-types-catalog/line-types-catalog-selector-dialog.tsx
- src/components/dialogs/line-types-catalog/segment-utils.ts
- src/components/dialogs/line-types-catalog/line-types-catalog-selector-form.tsx
- src/components/dialogs/line-types-catalog/line-type-segment-dialog.tsx
src/components/dialogs/line-types-catalog/line-type-segment-form.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
|
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
basseche
force-pushed
the
save_lineSegments
branch
from
16e12fc to
dad8396
Compare
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
1 participant
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
needs gridsuite/commons-ui#1074 to work
the purpose is to save segments data from and then upload them