Fix temporary limits update when add new limits set#3816
Conversation
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
basseche
force-pushed
the
fix_LimitSetsUpdate
branch
from
1b5f9d0 to
276e9c9
Compare
flomillot
left a comment
flomillot
left a comment
There was a problem hiding this comment.
I would have fixed the merge as discussed but OK if it fix the bug.
📝 WalkthroughWalkthroughRefactored operational limits form state management by eliminating prop drilling. Removed the merge step combining Changes
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 Tip You can enable review details to help with troubleshooting, context usage and more.Enable the |
|
![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
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/limits/limits-pane.tsx (1)
206-229:⚠️ Potential issue | 🟠 MajorSwitch
LimitsSidePanerendering to use watched group values for lookups.Line 121's comment explicitly documents that
namecan change withoutuseFieldArrayoperations. The map on lines 206-229 still usesoperationalLimitsGroups(the field-array snapshot) to resolvegetCurrentLimits*, meaning after inline edits the displayed pane shows stale identifiers. UsewatchedOperationalLimitsGroupsfor the loop and property access; keepoperationalLimitsGroups[index]only for the React key to maintain stability.Suggested change
- operationalLimitsGroups.map( + (watchedOperationalLimitsGroups ?? []).map( (operationalLimitsGroup: OperationalLimitsGroupFormSchema, index: number) => index === indexSelectedLimitSet && ( <LimitsSidePane - key={operationalLimitsGroup.id} + key={operationalLimitsGroups[index].id} name={`${id}.${OPERATIONAL_LIMITS_GROUPS}[${index}]`} clearableFields={clearableFields} permanentCurrentLimitPreviousValue={🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dialogs/limits/limits-pane.tsx` around lines 206 - 229, Replace the map over operationalLimitsGroups with watchedOperationalLimitsGroups so the pane uses up-to-date group values for lookups: change the iteration to watchedOperationalLimitsGroups.map(...) and inside use the iterated watchedOperationalLimitsGroup (instead of operationalLimitsGroup) when calling getCurrentLimits, getCurrentLimitsIgnoreApplicability and accessing .id/.name; retain operationalLimitsGroups[index] only for the React key passed to LimitsSidePane (keep the key using operationalLimitsGroups[index].id) and leave other props (name, clearableFields, permanentCurrentLimitPreviousValue, temporaryLimitsPreviousValues, currentNode, disabled) wired as before (still referencing index and indexSelectedLimitSet and olgEditable).
🤖 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/limits/operational-limits-groups-tabs.tsx`:
- Around line 60-63: The watched value operationalLimitsGroups (obtained via
useWatch with name `${parentFormName}.${OPERATIONAL_LIMITS_GROUPS}`) can be
undefined if the form defaultValues don't initialize that field; update the
useWatch call to provide a local defaultValue: [] so operationalLimitsGroups is
always an array before you call .length and .map on it, keeping the rest of the
component unchanged and referencing the same parentFormName and
OPERATIONAL_LIMITS_GROUPS identifiers.
---
Outside diff comments:
In `@src/components/dialogs/limits/limits-pane.tsx`:
- Around line 206-229: Replace the map over operationalLimitsGroups with
watchedOperationalLimitsGroups so the pane uses up-to-date group values for
lookups: change the iteration to watchedOperationalLimitsGroups.map(...) and
inside use the iterated watchedOperationalLimitsGroup (instead of
operationalLimitsGroup) when calling getCurrentLimits,
getCurrentLimitsIgnoreApplicability and accessing .id/.name; retain
operationalLimitsGroups[index] only for the React key passed to LimitsSidePane
(keep the key using operationalLimitsGroups[index].id) and leave other props
(name, clearableFields, permanentCurrentLimitPreviousValue,
temporaryLimitsPreviousValues, currentNode, disabled) wired as before (still
referencing index and indexSelectedLimitSet and olgEditable).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0e6a5b3e-f7e3-4219-8f66-21d6dc5337d7
📒 Files selected for processing (2)
src/components/dialogs/limits/limits-pane.tsxsrc/components/dialogs/limits/operational-limits-groups-tabs.tsx
| const operationalLimitsGroups: OperationalLimitsGroupFormSchema[] = useWatch({ | ||
| name: `${parentFormName}.${OPERATIONAL_LIMITS_GROUPS}`, | ||
| }); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In react-hook-form, what does useWatch return when the watched field has neither a local defaultValue nor a useForm defaultValues entry?
💡 Result:
In react-hook-form, useWatch returns undefined when the watched field has neither a local defaultValue (in useWatch options) nor a defaultValues entry (in useForm).
Citations:
🏁 Script executed:
# First, verify the file exists and get its line count
wc -l src/components/dialogs/limits/operational-limits-groups-tabs.tsx
# Read the relevant sections to see actual usage
echo "=== Lines 55-70 (around useWatch) ==="
sed -n '55,70p' src/components/dialogs/limits/operational-limits-groups-tabs.tsx
echo "=== Lines 85-95 (around Line 91 usage) ==="
sed -n '85,95p' src/components/dialogs/limits/operational-limits-groups-tabs.tsx
echo "=== Lines 108-118 (around Line 112 usage) ==="
sed -n '108,118p' src/components/dialogs/limits/operational-limits-groups-tabs.tsxRepository: gridsuite/gridstudy-app
Length of output: 1794
🏁 Script executed:
# Search for how this component is used and if form defaultValues are initialized
rg -B 5 -A 5 "operationalLimitsGroups" src/components/dialogs/limits/operational-limits-groups-tabs.tsxRepository: gridsuite/gridstudy-app
Length of output: 2108
🏁 Script executed:
# Check if there are any null checks or optional chaining on operationalLimitsGroups
rg "operationalLimitsGroups\?" src/components/dialogs/limits/operational-limits-groups-tabs.tsxRepository: gridsuite/gridstudy-app
Length of output: 49
🏁 Script executed:
# Find parent component or form initialization context
rg -l "operational-limits-groups-tabs" --type tsx --type ts | head -5Repository: gridsuite/gridstudy-app
Length of output: 94
🏁 Script executed:
# Check how this component is imported and used in parent components
rg "operational-limits-groups-tabs" --type-add 'web:*.{tsx,ts}' -l
# Find imports of this component
rg "import.*operational-limits-groups-tabs" --type-add 'web:*.{tsx,ts}'Repository: gridsuite/gridstudy-app
Length of output: 238
🏁 Script executed:
# Check if there's any form initialization context we should understand
# Look for useForm setup in parent components
rg -A 10 "operational-limits-groups-tabs" src/components/dialogs/limits/ --type-add 'web:*.{tsx,ts}' | head -40Repository: gridsuite/gridstudy-app
Length of output: 1315
🏁 Script executed:
# Verify the exact type definition to confirm it shouldn't be OperationalLimitsGroupFormSchema[] | undefined
rg "OperationalLimitsGroupFormSchema" --type-add 'web:*.{tsx,ts}' | head -10Repository: gridsuite/gridstudy-app
Length of output: 1310
🏁 Script executed:
# Check how limits-pane.tsx initializes the form and whether defaultValues are set
rg -B 5 -A 10 "useForm|defaultValues" src/components/dialogs/limits/limits-pane.tsx | head -80Repository: gridsuite/gridstudy-app
Length of output: 1632
🏁 Script executed:
# Look for where OperationalLimitsGroupsTabs is rendered and what parent form field it depends on
sed -n '1,50p' src/components/dialogs/limits/limits-pane.tsx && echo "..." && rg -B 5 -A 10 "OperationalLimitsGroupsTabs" src/components/dialogs/limits/limits-pane.tsxRepository: gridsuite/gridstudy-app
Length of output: 4273
🏁 Script executed:
# Check the full form structure and where operationalLimitsGroups is initialized
rg "operationalLimitsGroups\|OPERATIONAL_LIMITS_GROUPS" src/components/dialogs/limits/limits-pane.tsx | head -20Repository: gridsuite/gridstudy-app
Length of output: 49
🏁 Script executed:
# Check react-hook-form documentation on useFieldArray behavior with empty initial values
rg "useFieldArray" src/components/dialogs/limits/limits-pane.tsx -A 5 -B 2Repository: gridsuite/gridstudy-app
Length of output: 1261
🌐 Web query:
react-hook-form useFieldArray what is default initial value if field not in defaultValues
💡 Result:
If the field array name is not present in the defaultValues of useForm (or explicitly set to undefined/null there), the fields array returned by useFieldArray is empty by default. No fields are rendered until the user calls append, prepend, or insert (or replace with an array), or until reset/setValue populates the array. This is the standard behavior for dynamic field arrays, where fields start empty and are added on demand. Examples in the official docs show useFieldArray({ name: "test" }) with fields.map(...) but no initial population unless defaultValues: { test: [...] } is provided to useForm. To initialize with fields, provide an array under that name in defaultValues, e.g.: const { control } = useForm({ defaultValues: { test: [{ firstName: '' }] } }); const { fields } = useFieldArray({ control, name: 'test' }); For empty start: omit from defaultValues or set test: [].
Citations:
- 1: https://react-hook-form.com/docs/usefieldarray
- 2: getValues does not return default values on field array react-hook-form/react-hook-form#4634
- 3: useFieldArray, fields array can't have default data. react-hook-form/react-hook-form#1870
- 4: V7 useFieldArray if value null application crushes react-hook-form/react-hook-form#4826
- 5: [useFieldArray] The value will be the default value when removed all fields react-hook-form/react-hook-form#2382
- 6: https://stackoverflow.com/questions/73717993/cant-set-default-empty-array-to-react-hook-form-fieldarray
Default the watched array locally.
Line 91 calls .length and Line 112 calls .map() directly on operationalLimitsGroups. If the form field ${parentFormName}.${OPERATIONAL_LIMITS_GROUPS} is not initialized in the form's defaultValues, useWatch returns undefined, causing a runtime crash. Add a local defaultValue: [] to make the component self-contained.
Suggested change
const operationalLimitsGroups: OperationalLimitsGroupFormSchema[] = useWatch({
name: `${parentFormName}.${OPERATIONAL_LIMITS_GROUPS}`,
+ defaultValue: [],
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const operationalLimitsGroups: OperationalLimitsGroupFormSchema[] = useWatch({ | |
| name: `${parentFormName}.${OPERATIONAL_LIMITS_GROUPS}`, | |
| }); | |
| const operationalLimitsGroups: OperationalLimitsGroupFormSchema[] = useWatch({ | |
| name: `${parentFormName}.${OPERATIONAL_LIMITS_GROUPS}`, | |
| defaultValue: [], | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/dialogs/limits/operational-limits-groups-tabs.tsx` around
lines 60 - 63, The watched value operationalLimitsGroups (obtained via useWatch
with name `${parentFormName}.${OPERATIONAL_LIMITS_GROUPS}`) can be undefined if
the form defaultValues don't initialize that field; update the useWatch call to
provide a local defaultValue: [] so operationalLimitsGroups is always an array
before you call .length and .map on it, keeping the rest of the component
unchanged and referencing the same parentFormName and OPERATIONAL_LIMITS_GROUPS
identifiers.
2 participants
PR Summary