86b7yv08q - chore: add validation rate inventory items#746
Conversation
📝 WalkthroughWalkthroughCentralizes numeric validators into shared Yup utilities, replaces text price inputs with a price-specific input, adds a keydown handler to block "e", "E", "+", "-" in the price field, and updates the sponsor-inventory popup to use the price field and trigger form validation on value changes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @src/utils/yup.js:
- Around line 44-52: The test "max-decimals" in decimalValidation can mis-handle
very large numbers because value.toString() may produce scientific notation
(e.g., "1e+21") and fail the regex; update the test callback in
decimalValidation to first ensure the number is finite and below a safe upper
bound (e.g., Math.abs(value) < 1e21) and return false for values at/above that
threshold (or handle scientific notation explicitly), then run the existing
regex on value.toString() for the normal range—this keeps the two-decimal check
intact while avoiding false negatives for huge numbers.
🧹 Nitpick comments (2)
src/components/mui/formik-inputs/mui-formik-pricefield.js (1)
16-21: Simplify event handling and consider blocking additional characters.The
preventDefault()andstopImmediatePropagation()methods should be called on the synthetic eventerather thane.nativeEvent. React's synthetic event system handles cross-browser compatibility.Additionally, consider blocking other problematic characters like
+and-that can also be entered in number inputs, which may not be appropriate for price fields.🔎 Proposed refactor
onKeyDown={(e) => { - if (e.key === "e" || e.key === "E") { - e.nativeEvent.preventDefault(); - e.nativeEvent.stopImmediatePropagation(); + if (["e", "E", "+", "-"].includes(e.key)) { + e.preventDefault(); + e.stopPropagation(); } }}src/pages/sponsors/sponsor-form-item-list-page/components/item-form.js (1)
130-161: Consider applying the same key blocking to quantity fields for consistency.The quantity fields use
type="number"but don't include theonKeyDownhandler to block 'e', 'E', '+', and '-' like theMuiFormikPriceFieldcomponent does. While validation will catch invalid input, preventing these characters from being typed would provide a more consistent user experience across all numeric inputs.🔎 Optional enhancement
You could either:
- Add the same
onKeyDownhandler to theseMuiFormikTextFieldcomponents, or- Create a reusable
MuiFormikIntegerFieldcomponent similar toMuiFormikPriceFieldthat includes this behavior by default.Example for option 1:
<Grid2 size={4}> <MuiFormikTextField name="quantity_limit_per_show" label={T.translate( "sponsor_form_item_list.edit_item.quantity_limit_per_show" )} fullWidth type="number" inputProps={{ min: 0 }} + onKeyDown={(e) => { + if (["e", "E", "+", "-"].includes(e.key)) { + e.preventDefault(); + } + }} /> </Grid2>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/mui/formik-inputs/mui-formik-pricefield.jssrc/pages/sponsors/sponsor-form-item-list-page/components/item-form.jssrc/utils/yup.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (4)
src/utils/yup.js (2)
2-2: LGTM!The import of the translation utility is necessary for the validation error messages below.
41-42: LGTM!The
numberValidationfunction provides a clean, reusable validator with consistent error messaging.src/pages/sponsors/sponsor-form-item-list-page/components/item-form.js (2)
16-20: LGTM!The refactoring to import validation functions from a shared utility module improves code reusability and maintainability.
48-60: LGTM!The validation schema correctly applies
decimalValidation()for rate fields andnumberValidation()with integer constraints for quantity fields. The validator usage is appropriate for each field type.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/pages/sponsors_inventory/popup/sponsor-inventory-popup.js:
- Around line 153-158: Remove the debug log and meaningless cleanup in the
useEffect: delete the console.log("ERRORES", formik.errors) and the empty return
() => {}; if you require immediate validation on value changes, await
formik.validateForm() inside the effect and then read the resolved errors (e.g.,
const errors = await formik.validateForm()) instead of accessing formik.errors
synchronously; otherwise drop the entire useEffect since Formik already
validates on change via validateOnChange.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/sponsors_inventory/popup/sponsor-inventory-popup.js
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/sponsors_inventory/popup/sponsor-inventory-popup.js (2)
src/pages/sponsors/sponsor-form-item-list-page/components/item-form.js (1)
formik(36-63)src/components/mui/formik-inputs/mui-formik-pricefield.js (1)
MuiFormikPriceField(8-32)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (3)
src/pages/sponsors_inventory/popup/sponsor-inventory-popup.js (3)
1-1: LGTM!The imports are well-organized. The validation utilities are correctly centralized in
utils/yup, aligning with the pattern initem-form.js.Also applies to: 33-33, 38-38
73-84: LGTM!The validation schema correctly uses the centralized
decimalValidation()andnumberValidation()utilities, consistent with the pattern initem-form.js.
311-335: No changes needed —MuiFormikPriceFieldcorrectly uses Formik context.The
MuiFormikPriceFieldcomponents are correct without theformik={formik}prop. The underlyingMuiFormikTextFielduses theuseField(name)hook, which automatically accesses the Formik context fromFormikProvider. Theformikprop passed toMuiFormikTextFieldin other usages (lines 272, 283, 344) is unnecessary and unused by the component — it's simply spread into props without being referenced.Both approaches work identically. No action required.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/pages/sponsors_inventory/popup/sponsor-inventory-popup.js (1)
304-308: Add theformikprop for consistency.While the
FormikProvidermakes the formik context available, all otherMuiFormikTextFieldusages in this file explicitly passformik={formik}(see lines 265-270, 276-281, 337-342, etc.). For consistency with the established pattern, add the prop to these threeMuiFormikPriceFieldinstances.♻️ Proposed fix for consistency
<MuiFormikPriceField variant="outlined" name="early_bird_rate" + formik={formik} fullWidth />Apply similar changes to
standard_rate(line 314) andonsite_rate(line 324).Also applies to: 314-318, 324-328
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/sponsors_inventory/popup/sponsor-inventory-popup.js
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/sponsors_inventory/popup/sponsor-inventory-popup.js (1)
src/components/mui/formik-inputs/mui-formik-pricefield.js (1)
MuiFormikPriceField(8-32)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (3)
src/pages/sponsors_inventory/popup/sponsor-inventory-popup.js (3)
33-33: LGTM!The import of
MuiFormikPriceFieldis correctly structured and will enable specialized price input handling with key blocking and proper formatting.
73-76: LGTM!The validation schema correctly applies
decimalValidation()for monetary rate fields andnumberValidation()for integer quantities. The use of centralized validators improves maintainability.
38-38: Import validation functions are properly implemented.Both
numberValidationanddecimalValidationare correctly exported fromsrc/utils/yup.js. ThedecimalValidationfunction properly validates monetary values with up to 2 decimal places using the regex pattern/^\d+(\.\d{1,2})?$/and enforces non-negative values. ThenumberValidationfunction provides general number validation with appropriate type error handling.
ref: https://app.clickup.com/t/86b7yv08q
86b7yv08q - chore: add validation rate inventory items
Changelog
eandEon price inputs.Links
86b7yv08q - Add numeric validation for rate on inventory items ( edit / new )
Evidence
2026-01-06_14-18-23.mp4
Summary by CodeRabbit
Bug Fixes
Refactor
New Features
✏️ Tip: You can customize this high-level summary in your review settings.