Fix: global quantity bug#256
Conversation
|
Warning Review limit reached
More reviews will be available in 48 minutes and 54 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughNormalize number inputs by writing cleaned values to the DOM and Formik, change max-clamp to only apply when max is truthy (so zero is treated as unlimited), update tests for leading-zero normalization and unlimited behavior, and bump package version to 5.0.27. ChangesQuantity input normalization and release
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
santipalenque
force-pushed
the
fix/global-quantity-bug
branch
from
4c4751b to
5ca51d9
Compare
React skips syncing controlled number inputs during typing, so leading zeros persist unless e.target.value is set directly before reconciliation. Applied to both GlobalQuantityField and MuiFormikQuantityField, with tests covering the leading zero case. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![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
🧹 Nitpick comments (2)
src/components/mui/FormItemTable/components/GlobalQuantityField.js (1)
43-43: ⚡ Quick winConsider multi-line formatting for better readability.
The if-statement on line 43 contains multiple statements on a single line, which can make debugging and readability more difficult.
♻️ Suggested formatting
- if (isNaN(val)) { e.target.value = 0; helpers.setValue(0); return; } + if (isNaN(val)) { + e.target.value = 0; + helpers.setValue(0); + return; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/mui/FormItemTable/components/GlobalQuantityField.js` at line 43, The single-line if in GlobalQuantityField.js currently packs multiple statements into one line (if (isNaN(val)) { e.target.value = 0; helpers.setValue(0); return; }), hurting readability; refactor it into a multi-line if block with braces and each operation on its own line (check isNaN(val), set e.target.value = 0, call helpers.setValue(0), then return) so the flow is clearer and easier to debug.src/components/mui/formik-inputs/mui-formik-quantity-field.js (1)
31-31: ⚡ Quick winConsider multi-line formatting for better readability.
The if-statement on line 31 contains multiple statements on a single line, which can make debugging and readability more difficult.
♻️ Suggested formatting
- if (isNaN(val)) { e.target.value = effectiveMin; helpers.setValue(effectiveMin); return; } + if (isNaN(val)) { + e.target.value = effectiveMin; + helpers.setValue(effectiveMin); + return; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/mui/formik-inputs/mui-formik-quantity-field.js` at line 31, The single-line if in the mui-formik-quantity-field component (the if (isNaN(val)) { e.target.value = effectiveMin; helpers.setValue(effectiveMin); return; }) should be expanded into a multi-line block for readability: open an if block checking isNaN(val), set e.target.value = effectiveMin on its own line, call helpers.setValue(effectiveMin) on its own line, then return on its own line; keep the same logic and placement in the event handler so behavior is unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/mui/FormItemTable/components/GlobalQuantityField.js`:
- Line 45: The bug is inconsistent handling of a zero max between
GlobalQuantityField and MuiFormikQuantityField: GlobalQuantityField treats falsy
0 as "no limit" while MuiFormikQuantityField treats 0 as a real limit. Normalize
the boundary by choosing "null/undefined = unlimited" and update the code paths
that compute clamped values (the clamped expression in GlobalQuantityField and
the max handling in MuiFormikQuantityField) to treat 0 as unlimited (e.g.,
convert quantity_limit_per_sponsor or max to null when === 0, or use a nullish
check max != null) and add an inline comment by the normalization explaining "0
is treated as unlimited; use null/undefined for no limit" so both components and
tests align.
---
Nitpick comments:
In `@src/components/mui/formik-inputs/mui-formik-quantity-field.js`:
- Line 31: The single-line if in the mui-formik-quantity-field component (the if
(isNaN(val)) { e.target.value = effectiveMin; helpers.setValue(effectiveMin);
return; }) should be expanded into a multi-line block for readability: open an
if block checking isNaN(val), set e.target.value = effectiveMin on its own line,
call helpers.setValue(effectiveMin) on its own line, then return on its own
line; keep the same logic and placement in the event handler so behavior is
unchanged.
In `@src/components/mui/FormItemTable/components/GlobalQuantityField.js`:
- Line 43: The single-line if in GlobalQuantityField.js currently packs multiple
statements into one line (if (isNaN(val)) { e.target.value = 0;
helpers.setValue(0); return; }), hurting readability; refactor it into a
multi-line if block with braces and each operation on its own line (check
isNaN(val), set e.target.value = 0, call helpers.setValue(0), then return) so
the flow is clearer and easier to debug.
🪄 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: 83703ff4-1247-481b-800a-7f5dc6a13030
📒 Files selected for processing (5)
package.jsonsrc/components/mui/FormItemTable/__tests__/GlobalQuantityField.test.jssrc/components/mui/FormItemTable/components/GlobalQuantityField.jssrc/components/mui/__tests__/mui-formik-quantity-field.test.jssrc/components/mui/formik-inputs/mui-formik-quantity-field.js
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- src/components/mui/FormItemTable/tests/GlobalQuantityField.test.js
2 participants
https://app.clickup.com/t/86ba4k6eq
Summary by CodeRabbit
Bug Fixes
Tests
Chores