feat(webapp): add 'Set new total' tab to asset quantity adjustment dialog - #2756
Open
Pallavikumarimdb wants to merge 1 commit into
Open
feat(webapp): add 'Set new total' tab to asset quantity adjustment dialog#2756Pallavikumarimdb wants to merge 1 commit into
Pallavikumarimdb wants to merge 1 commit into
Conversation
Pallavikumarimdb
marked this pull request as ready for review
July 28, 2026 12:47
Contributor
WalkthroughChangesAsset quantity adjustment
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant AssetQuantityView
participant QuickAdjustDialog
participant Fetcher
AssetQuantityView->>QuickAdjustDialog: provides current and available quantities
QuickAdjustDialog->>QuickAdjustDialog: validates target total and computes delta
QuickAdjustDialog->>Fetcher: submits add or subtract inventory adjustment
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/webapp/test/components/assets/quick-adjust-dialog.test.tsx (2)
35-138: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRepeated hardcoded props across all 5 test cases.
totalQuantity={10}/availableQuantity={5}are duplicated literally in every test. Extracting a shared factory/default-props object would reduce duplication and ease future changes.As per coding guidelines, "Avoid hardcoding data within tests; use factories to keep tests clean and maintainable."
🤖 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 `@apps/webapp/test/components/assets/quick-adjust-dialog.test.tsx` around lines 35 - 138, Extract the repeated QuickAdjustDialog props from the five tests into a shared default-props object or factory, including totalQuantity and availableQuantity, and reuse it in each render while preserving any test-specific props such as open and onOpenChange.Source: Coding guidelines
21-139: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing test for the
hasTotal=falsefallback layout.All tests pass
totalQuantity={10}, so the single-input fallback path (rendered whentotalQuantityisnull/undefined) is untested even though it's part of this PR's conditional branch inquick-adjust-dialog.tsx.🤖 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 `@apps/webapp/test/components/assets/quick-adjust-dialog.test.tsx` around lines 21 - 139, Extend the QuickAdjustDialog test suite with coverage for the fallback layout when totalQuantity is omitted or undefined. Verify that the single amount input renders, the total-specific tabs and new-total controls are absent, and the fallback form remains usable for submission.
🤖 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 `@apps/webapp/app/components/assets/quick-adjust-dialog.tsx`:
- Around line 151-155: Update the quantity validation in both handleSubmit and
handleSetTotal to reject fractional values, while retaining the existing
non-numeric and non-positive/negative checks. Ensure only finite whole-unit
quantities can be submitted or used as totals, and preserve the current
error-handling flow.
- Around line 257-302: The “Set new total” tab does not move focus to its input
when selected. Update the total-tab flow around the `Tabs` and `newTotal`
`Input` to add equivalent focus management for the “New Total Quantity” field
when `activeTab` changes to `"total"`, while preserving the existing
`quantityInputRef` behavior for the adjust tab.
---
Nitpick comments:
In `@apps/webapp/test/components/assets/quick-adjust-dialog.test.tsx`:
- Around line 35-138: Extract the repeated QuickAdjustDialog props from the five
tests into a shared default-props object or factory, including totalQuantity and
availableQuantity, and reuse it in each render while preserving any
test-specific props such as open and onOpenChange.
- Around line 21-139: Extend the QuickAdjustDialog test suite with coverage for
the fallback layout when totalQuantity is omitted or undefined. Verify that the
single amount input renders, the total-specific tabs and new-total controls are
absent, and the fallback form remains usable for submission.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0da85ed1-3366-4036-828b-d81fb1904745
📒 Files selected for processing (4)
apps/webapp/app/components/assets/actions-dropdown.tsxapps/webapp/app/components/assets/quantity-overview-card.tsxapps/webapp/app/components/assets/quick-adjust-dialog.tsxapps/webapp/test/components/assets/quick-adjust-dialog.test.tsx
Comment on lines
+151
to
+155
| if (isNaN(qty) || qty <= 0) { | ||
| setQuantityError("Quantity must be a positive number."); | ||
| return; | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
No integer/whole-unit validation on quantity fields.
Both handleSubmit (qty <= 0) and handleSetTotal (newTotal < 0) only guard against non-numeric/negative input, not fractional values. Since asset quantities are discrete units, a user typing e.g. 1.5 passes validation and gets submitted as a fractional delta.
🛡️ Proposed fix
- if (isNaN(qty) || qty <= 0) {
- setQuantityError("Quantity must be a positive number.");
+ if (isNaN(qty) || qty <= 0 || !Number.isInteger(qty)) {
+ setQuantityError("Quantity must be a positive whole number.");
return;
}- if (isNaN(newTotal) || newTotal < 0) {
- setQuantityError("New total must be a non-negative number.");
+ if (isNaN(newTotal) || newTotal < 0 || !Number.isInteger(newTotal)) {
+ setQuantityError("New total must be a non-negative whole number.");
return;
}Also applies to: 193-196
🤖 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 `@apps/webapp/app/components/assets/quick-adjust-dialog.tsx` around lines 151 -
155, Update the quantity validation in both handleSubmit and handleSetTotal to
reject fractional values, while retaining the existing non-numeric and
non-positive/negative checks. Ensure only finite whole-unit quantities can be
submitted or used as totals, and preserve the current error-handling flow.
Comment on lines
+257
to
+302
| {hasTotal ? ( | ||
| <Tabs | ||
| value={activeTab} | ||
| onValueChange={(v) => setActiveTab(v as "adjust" | "total")} | ||
| className="w-full" | ||
| > | ||
| <TabsList className="mb-4 grid w-full grid-cols-2"> | ||
| <TabsTrigger value="adjust">Adjust by amount</TabsTrigger> | ||
| <TabsTrigger value="total">Set new total</TabsTrigger> | ||
| </TabsList> | ||
|
|
||
| <TabsContent value="adjust" className="flex flex-col gap-4"> | ||
| <Input | ||
| ref={quantityInputRef} | ||
| name="quantity" | ||
| type="number" | ||
| label={`Quantity (${unitLabel})`} | ||
| placeholder="Enter quantity" | ||
| min={1} | ||
| step={1} | ||
| required={activeTab === "adjust"} | ||
| error={quantityError || serverError || undefined} | ||
| onChange={() => setQuantityError(null)} | ||
| /> | ||
| </TabsContent> | ||
|
|
||
| <TabsContent value="total" className="flex flex-col gap-4"> | ||
| <div className="mb-1 text-sm text-gray-500"> | ||
| Current total:{" "} | ||
| <span className="font-semibold text-gray-900"> | ||
| {totalQuantity} {unitLabel} | ||
| </span> | ||
| </div> | ||
| <Input | ||
| name="newTotal" | ||
| type="number" | ||
| label={`New Total Quantity (${unitLabel})`} | ||
| placeholder="Enter new total quantity" | ||
| min={0} | ||
| step={1} | ||
| required={activeTab === "total"} | ||
| error={quantityError || serverError || undefined} | ||
| onChange={() => setQuantityError(null)} | ||
| /> | ||
| </TabsContent> | ||
| </Tabs> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
No focus management when switching to the "Set new total" tab.
The "adjust" input uses quantityInputRef + useAutoFocus to move focus on open, but the "New Total Quantity" input in the total TabsContent has no equivalent, so keyboard users switching tabs land with no input focused.
🤖 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 `@apps/webapp/app/components/assets/quick-adjust-dialog.tsx` around lines 257 -
302, The “Set new total” tab does not move focus to its input when selected.
Update the total-tab flow around the `Tabs` and `newTotal` `Input` to add
equivalent focus management for the “New Total Quantity” field when `activeTab`
changes to `"total"`, while preserving the existing `quantityInputRef` behavior
for the adjust tab.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Allows users to directly set a new absolute total quantity for
QUANTITY_TRACKEDassets from the adjustment dialog. It automatically calculates the difference (delta) on the client side and submits it to the existing API endpoint, which avoids database migrations or backend changes.Key Changes
RESTOCK(+delta) orLOSS(-delta) action.totalQuantityprop to the dialog inQuantityOverviewCardandActionsDropdown.Closes: #2716
Summary by CodeRabbit