fix: clean validation on additional input list remove#208
fix: clean validation on additional input list remove#208
Conversation
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughDestructures Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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
🤖 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/mui/formik-inputs/additional-input/additional-input-list.js`:
- Line 70: The call to setFieldTouched should be moved inside removeFromUI and
only invoked after a successful removal to avoid clearing touched state on API
failure; update the invocation from setFieldTouched(name, []) to
setFieldTouched(name, false) and call it after the code path that confirms the
item was removed (i.e., inside removeFromUI or immediately after a successful
API response), and ensure no unconditional setFieldTouched call remains where
the API call could still fail.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 564e6f1a-94a3-4297-aeff-66ae486ef95e
📒 Files selected for processing (1)
src/components/mui/formik-inputs/additional-input/additional-input-list.js
| } | ||
| setFieldValue(name, newValues); | ||
| }; | ||
| setFieldTouched(name, [], false); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Formik setFieldTouched API signature and parameters - what is the second parameter type?
💡 Result:
The Formik setFieldTouched API signature is: setFieldTouched: (field: string, isTouched?: boolean, shouldValidate?: boolean) => Promise<void> The second parameter is isTouched, which is a boolean (optional, defaults to true). It sets whether the specified field is touched.
Citations:
- 1: https://formik.org/docs/api/formik
- 2: https://stackoverflow.com/questions/62739162/cant-set-touched-to-true
🏁 Script executed:
cat -n src/components/mui/formik-inputs/additional-input/additional-input-list.js | sed -n '25,80p'Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 2024
Move setFieldTouched call inside removeFromUI to prevent inconsistent state on API failure.
The setFieldTouched call at line 70 executes unconditionally after confirmation, before the conditional removal logic. If the API call (line 73) fails and is caught at line 75, the touched state is already cleared but the item remains in the list, leaving form state inconsistent.
Additionally, passing [] as the second parameter to setFieldTouched violates Formik's API—the parameter must be a boolean (isTouched?: boolean). The correct value is false to clear the touched state.
🐛 Proposed fix: move setFieldTouched inside removeFromUI
const removeFromUI = () => {
const newValues = metaFields.filter((_, idx) => idx !== index);
if (newValues.length === 0) {
newValues.push({ ...DEFAULT_META_FIELD, _key: `draft_${Date.now()}` });
}
setFieldValue(name, newValues);
+ setFieldTouched(name, false, false);
};
- setFieldTouched(name, [], false);
if (item.id && onDelete) {📝 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.
| setFieldTouched(name, [], false); | |
| const removeFromUI = () => { | |
| const newValues = metaFields.filter((_, idx) => idx !== index); | |
| if (newValues.length === 0) { | |
| newValues.push({ ...DEFAULT_META_FIELD, _key: `draft_${Date.now()}` }); | |
| } | |
| setFieldValue(name, newValues); | |
| setFieldTouched(name, false, false); | |
| }; | |
| if (item.id && onDelete) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/mui/formik-inputs/additional-input/additional-input-list.js`
at line 70, The call to setFieldTouched should be moved inside removeFromUI and
only invoked after a successful removal to avoid clearing touched state on API
failure; update the invocation from setFieldTouched(name, []) to
setFieldTouched(name, false) and call it after the code path that confirms the
item was removed (i.e., inside removeFromUI or immediately after a successful
API response), and ensure no unconditional setFieldTouched call remains where
the API call could still fail.
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
1 participant
ref: https://app.clickup.com/t/86b8t9k79
Signed-off-by: Tomás Castillo tcastilloboireau@gmail.com
Summary by CodeRabbit