Skip to content

Conversation

@niko-exo
Copy link

@niko-exo niko-exo commented Jan 6, 2026

ref: https://app.clickup.com/t/86b7yv08q

86b7yv08q - chore: add validation rate inventory items

Changelog

  • Move yup validation functions to a utils file.
  • Avoid user of being able to type e and E on 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

    • Blocked exponent and sign characters ("e", "E", "+", "-") in numeric price inputs to prevent invalid entries.
  • Refactor

    • Centralized number and decimal validation into shared utilities with localized messages and two-decimal/non-negative rules.
    • Replaced inline validators across forms with the shared validators.
  • New Features

    • Switched price fields to a dedicated price input component for consistent behavior and validation.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 6, 2026

📝 Walkthrough

Walkthrough

Centralizes 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

Cohort / File(s) Summary
Price field input handling
src/components/mui/formik-inputs/mui-formik-pricefield.js
Added BLOCKED_KEYS (["e","E","+","-"]) and an onKeyDown handler that calls e.nativeEvent.preventDefault() and e.nativeEvent.stopImmediatePropagation() to block exponent/sign characters in the numeric price input.
Shared Yup validators
src/utils/yup.js
Added numberValidation() (Yup number with typeError message from i18n "validation.number") and decimalValidation() (typeError, non-negative check via "validation.number_positive", and max-2-decimals custom check with "validation.two_decimals").
Form validation refactor — sponsor item form
src/pages/sponsors/sponsor-form-item-list-page/components/item-form.js
Removed local numberValidation/decimalValidation definitions and now import numberValidation and decimalValidation from ../../../../utils/yup; validation schema usage remains the same.
Sponsor inventory popup changes
src/pages/sponsors_inventory/popup/sponsor-inventory-popup.js
Replaced three MuiFormikTextField price fields with MuiFormikPriceField, imported numberValidation/decimalValidation from ../../../utils/yup, and added a useEffect to call formik.validateForm on value changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I hop through code with careful paws,
I block the "e" and plus because—
Prices stay neat, decimals in line,
Shared validators make things fine.
A tiny twitch, a tidy cause.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title refers to adding validation for rate inventory items, which is a core part of the changeset. However, it doesn't clearly communicate the key implementation details: moving validation functions to utils and blocking exponent characters in price fields.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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() and stopImmediatePropagation() methods should be called on the synthetic event e rather than e.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 the onKeyDown handler to block 'e', 'E', '+', and '-' like the MuiFormikPriceField component 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:

  1. Add the same onKeyDown handler to these MuiFormikTextField components, or
  2. Create a reusable MuiFormikIntegerField component similar to MuiFormikPriceField that 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

📥 Commits

Reviewing files that changed from the base of the PR and between e32a4a4 and bd7d272.

📒 Files selected for processing (3)
  • src/components/mui/formik-inputs/mui-formik-pricefield.js
  • src/pages/sponsors/sponsor-form-item-list-page/components/item-form.js
  • src/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 numberValidation function 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 and numberValidation() with integer constraints for quantity fields. The validator usage is appropriate for each field type.

@fntechgit fntechgit deleted a comment from coderabbitai bot Jan 6, 2026
Copy link

@smarcet smarcet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@niko-exo please review comments

@niko-exo niko-exo requested a review from smarcet January 8, 2026 18:45
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 552548a and 98cebc3.

📒 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 in item-form.js.

Also applies to: 33-33, 38-38


73-84: LGTM!

The validation schema correctly uses the centralized decimalValidation() and numberValidation() utilities, consistent with the pattern in item-form.js.


311-335: No changes needed — MuiFormikPriceField correctly uses Formik context.

The MuiFormikPriceField components are correct without the formik={formik} prop. The underlying MuiFormikTextField uses the useField(name) hook, which automatically accesses the Formik context from FormikProvider. The formik prop passed to MuiFormikTextField in 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.

@fntechgit fntechgit deleted a comment from coderabbitai bot Jan 8, 2026
Copy link

@smarcet smarcet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@smarcet smarcet merged commit cdb4dbe into master Jan 8, 2026
8 of 9 checks passed
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/pages/sponsors_inventory/popup/sponsor-inventory-popup.js (1)

304-308: Add the formik prop for consistency.

While the FormikProvider makes the formik context available, all other MuiFormikTextField usages in this file explicitly pass formik={formik} (see lines 265-270, 276-281, 337-342, etc.). For consistency with the established pattern, add the prop to these three MuiFormikPriceField instances.

♻️ Proposed fix for consistency
 <MuiFormikPriceField
   variant="outlined"
   name="early_bird_rate"
+  formik={formik}
   fullWidth
 />

Apply similar changes to standard_rate (line 314) and onsite_rate (line 324).

Also applies to: 314-318, 324-328

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 98cebc3 and 5a93638.

📒 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 MuiFormikPriceField is 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 and numberValidation() for integer quantities. The use of centralized validators improves maintainability.


38-38: Import validation functions are properly implemented.

Both numberValidation and decimalValidation are correctly exported from src/utils/yup.js. The decimalValidation function properly validates monetary values with up to 2 decimal places using the regex pattern /^\d+(\.\d{1,2})?$/ and enforces non-negative values. The numberValidation function provides general number validation with appropriate type error handling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants