fix: comprehensive invoice validation and UI enhancements#178
Conversation
|
Warning Review limit reached
Next review available in: 9 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR adds shared invoice amount and validation helpers, wires them into single and batch invoice creation flows, introduces amount-type toggles, resolves token decimals dynamically, hardens token selection interactions, and updates the copy action to work from the keyboard. ChangesInvoice calculation and validation rework
Estimated code review effort: 4 (Complex) | ~75 minutes TokenPicker and CopyButton UX fixes
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CreateInvoice
participant invoiceValidation
participant resolveTokenDecimals
participant Contract
User->>CreateInvoice: edit item / submit invoice
CreateInvoice->>invoiceValidation: validateSingleInvoiceData(...)
invoiceValidation-->>CreateInvoice: fieldErrors, isValid
CreateInvoice->>resolveTokenDecimals: fetch token decimals
resolveTokenDecimals-->>CreateInvoice: decimals or null
CreateInvoice->>Contract: createInvoice(normalizedData, amountDue)
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 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}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/ui/copyButton.jsx (1)
20-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer a native
<button type="button">over span+role reimplementation.The manual
role="button"+tabIndex={0}+onKeyDownpattern re-implements keyboard activation and focus semantics that a native<button>already provides for free. If the original motivation was to avoid accidental form submission (sinceCopyButtonis used inside<form onSubmit=...>inCreateInvoice.jsx/CreateInvoicesBatch.jsx), addingtype="button"to a real<button>element solves that without the extra custom keydown handling.♻️ Simpler alternative using a native button
return ( - <span - role="button" - tabIndex={0} + <button + type="button" onClick={handleCopy} - onKeyDown={handleKeyDown} className={`inline-flex cursor-pointer items-center gap-1 px-2 py-1 text-xs rounded hover:bg-gray-100 transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-gray-400 ${className}`} title="Copy address" > {copied ? ( <> <Check className="w-3 h-3 text-green-600" /> <span className="text-green-600">Copied!</span> </> ) : ( <> <Copy className="w-3 h-3 text-gray-500" /> <span className="text-gray-500">Copy</span> </> )} - </span> + </button> );(
handleKeyDowncan then be removed entirely.)As per path instructions, "The code adheres to best practices associated with React."
🤖 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 `@frontend/src/components/ui/copyButton.jsx` around lines 20 - 46, The CopyButton component is reimplementing button behavior with a span, role, tabIndex, and custom key handling. Update CopyButton to use a native button element with type="button" so it works correctly inside forms used by CreateInvoice and CreateInvoicesBatch, and remove the manual handleKeyDown logic since the browser will handle keyboard activation and focus semantics.
🤖 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 `@frontend/src/components/AmountTypeToggle.jsx`:
- Around line 6-31: The AmountTypeToggle component exposes the current “Flat” /
“%” state only through styling, so update its buttons to communicate state to
assistive tech. Add aria-pressed to the two state buttons in AmountTypeToggle
and give the icon-only toggle button an explicit accessible label via aria-label
(while keeping the existing onClick behavior and ArrowRightLeft control). Use
the component’s value/onChange logic to reflect the active state consistently
for screen readers.
In `@frontend/src/components/TokenPicker.jsx`:
- Around line 214-235: The open modal still allows interaction when disabled
state changes or the chain switches to a testnet because handlers like
handleSelect and handleCustomTokenClick only guard on isSelecting. Centralize an
interaction-disabled check in TokenPicker (using the same disabled/testnet
gating as the trigger) and apply it to handleSelect, token row clicks, the
query-clear button, and the custom-token action so the modal cannot bypass the
parent gating.
In `@frontend/src/page/CreateInvoice.jsx`:
- Around line 1279-1289: The AmountTypeToggle handlers in CreateInvoice are
updating every entry in setItemData instead of only the edited line item, which
can change unrelated rows’ discount/tax behavior and totals. Update the onChange
logic for the per-row toggles (both the discount and tax controls) so it targets
only the current item by index or a stable item identifier, and recomputes
amount for that single line using getSafeLineAmountDisplay while leaving other
items unchanged.
- Around line 407-413: The call to validateSingleInvoiceData in CreateInvoice is
missing required identity fields, causing required-field validation failures.
Update the validation payload to pass userFname, userEmail, clientFname, and
clientEmail from the current form data alongside clientAddress, itemData,
totalAmountDue, paymentToken, and ownerAddress, and verify the CreateInvoice
submit flow uses the same field names expected by validateSingleInvoiceData.
In `@frontend/src/page/CreateInvoicesBatch.jsx`:
- Around line 467-480: The batch invoice flow is still relying on a hardcoded
18-decimal token selection path, which can mis-scale amounts for non-18 tokens.
Update the token-picking and submission flow in CreateInvoicesBatch.jsx so
selectedToken preserves the validated decimals from the token list/provider, or
reuse the same decimals resolution logic used by the single-invoice path before
validateInvoicesBeforeSubmit runs. Make sure the token picker does not overwrite
decimals with 18 and that the invoice amount parsing uses the resolved token
decimals consistently in both affected sections.
- Around line 1288-1308: The per-item mobile toggle handler in
CreateInvoicesBatch is updating every entry in rowIndex’s itemData array instead
of only the edited item, so changing one line’s discount/tax type mutates
siblings and recalculates all amounts. Update the onChange logic for
AmountTypeToggle to target only the specific item being edited (using the item’s
own index or identifier) while leaving the rest of r.itemData unchanged, and
apply the same fix to the matching tax-type toggle block referenced in the other
section.
- Around line 169-177: The row-removal logic in CreateInvoicesBatch only
reindexes clientAddressErrors, so rowTotalErrors, rowItemErrors, and row-scoped
fieldErrors can remain tied to the deleted index and appear on the wrong
invoice. Update removeInvoiceRow to reindex every row-based error map using a
shared helper like reindexRowErrors, and apply it consistently to all affected
state setters so all remaining rows shift their errors down together.
In `@frontend/src/utils/invoiceValidation.js`:
- Around line 132-138: The line-item validation loop in invoiceValidation should
skip auto-appended untouched blank rows instead of passing them into
getLineItemError, since those empty items are triggering required-field errors
and blocking submission. Update the validation logic around the itemData
iteration so only populated line items are checked, while preserving existing
behavior for real item rows and keeping itemErrorsObj/hasItemErrors in sync.
- Around line 58-70: The submit-time validation in invoiceValidation.js only
checks for negative discountWei/taxRateWei and misses percentage upper bounds
for imported or manipulated state. Update the validator logic around the
existing discount/tax checks to also reject discountType or taxType values set
to "percentage" when their computed wei values exceed 100%, and add
corresponding errors so the submit path matches the direct-input limits. Use the
existing validation flow in the invoice validation function and the
errors.discount/errors.tax assignments to locate and extend the checks.
---
Outside diff comments:
In `@frontend/src/components/ui/copyButton.jsx`:
- Around line 20-46: The CopyButton component is reimplementing button behavior
with a span, role, tabIndex, and custom key handling. Update CopyButton to use a
native button element with type="button" so it works correctly inside forms used
by CreateInvoice and CreateInvoicesBatch, and remove the manual handleKeyDown
logic since the browser will handle keyboard activation and focus semantics.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c67c0664-dbed-404a-9dc4-d945565369bc
📒 Files selected for processing (8)
frontend/src/components/AmountTypeToggle.jsxfrontend/src/components/TokenPicker.jsxfrontend/src/components/ui/copyButton.jsxfrontend/src/page/CreateInvoice.jsxfrontend/src/page/CreateInvoicesBatch.jsxfrontend/src/utils/invoiceCalculations.jsfrontend/src/utils/invoiceValidation.jsfrontend/src/utils/productCatalogInvoiceHelpers.js
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@frontend/src/page/CreateInvoicesBatch.jsx`:
- Around line 211-235: `fieldErrors` is being reindexed with
`reindexSimpleErrors`, but that helper only works for plain numeric keys, so
row-scoped entries like `0_clientEmail` or nonnumeric field keys are lost when a
row is removed. Update the row-removal logic in `CreateInvoicesBatch` to use a
field-error-specific reindexer (near `reindexSimpleErrors`/`reindexItemErrors`)
that preserves non-row keys and shifts row-prefixed keys down by one, then pass
that helper to `setFieldErrors` instead of the simple numeric version.
In `@frontend/src/utils/invoiceValidation.js`:
- Around line 58-67: The percentage cap checks in validateInvoice values are
comparing the raw string via Number(item.discount || 0) and Number(item.tax ||
0), which can miss values slightly above 100 due to rounding. Update the
discount and tax validation branches in invoiceValidation.js to compare
discountWei and taxRateWei against a parsed 100 constant in wei form instead, so
the existing discountType and taxType percentage checks reject any amount over
the cap precisely.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ecfc8c1f-598a-4f0d-acba-8b6b16815c18
📒 Files selected for processing (5)
frontend/src/components/AmountTypeToggle.jsxfrontend/src/components/TokenPicker.jsxfrontend/src/page/CreateInvoice.jsxfrontend/src/page/CreateInvoicesBatch.jsxfrontend/src/utils/invoiceValidation.js
2 participants
Addressed Issues:
Fixes #148
Screenshots/Recordings:
freecompress-first.mp4
Summary of Changes:
Quantity,Unit Price,Discount, andTaxfields across both Single and Batch invoice pages.0and100.handleItemData,handleFieldChange) inCreateInvoice.jsxandCreateInvoicesBatch.jsxso that form validation errors immediately clear the moment a user types valid input, eliminating confusing lingering errors.handleInvoiceChangefunction inCreateInvoicesBatch.jsx, fixing a bug that completely broke input handling for Client Information fields in batch mode.AlertCircleicon to batch line item error states to perfectly match the design language of the single invoice page.Additional Notes:
AI Usage Disclosure:
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact. AI slop is strongly discouraged and may lead to banning and blocking. Do not spam our repos with AI slop.
Check one of the checkboxes below:
I have used the following AI models and tools: model-Gemini 3.1High, Used for testing
Checklist
Summary by CodeRabbit