feat(frontend): add product catalog import and invoice autofill#157
Open
Atharva0506 wants to merge 8 commits intoStabilityNexus:mainfrom
Open
feat(frontend): add product catalog import and invoice autofill#157Atharva0506 wants to merge 8 commits intoStabilityNexus:mainfrom
Atharva0506 wants to merge 8 commits intoStabilityNexus:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds an IndexedDB-backed product catalog with CSV/JSON/URL import and persistence, a debounced accessible product autocomplete with pagination and keyboard support, invoice-item helpers for fixed-point math and product application, and integrates catalog import/autocomplete into invoice creation and batch pages. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CatalogUI as ProductCatalogImport
participant Hook as useProductCatalog
participant IDB as IndexedDB
participant Search as ProductAutocompleteInput
participant Invoice as InvoicePage
User->>CatalogUI: Upload file or provide URL
CatalogUI->>Hook: importFromFile()/importFromURL(persist?)
Hook->>Hook: Parse & normalize rows (CSV/JSON), map fields
Hook->>IDB: Save catalog metadata / saved URL (when persisted)
Hook-->>CatalogUI: Return metadata & status
CatalogUI-->>User: Show toast / status
User->>Search: Type query (debounced)
Search->>Hook: Read catalogMetadata.data
Hook-->>Search: Return filtered + paged results
Search-->>User: Render suggestions
User->>Search: Select product
Search->>Invoice: onSelectProduct(product)
Invoice->>Invoice: applyProductToInvoiceItem()
Invoice-->>User: Update invoice line UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ 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 |
Atharva0506
force-pushed
the
feature/product-catalog-import
branch
2 times, most recently
from
a1b5c65 to
1d7fd07
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/ProductAutocompleteInput.jsx`:
- Around line 115-121: The Enter key currently submits the surrounding form when
the suggestions dropdown is open and no item is active (activeIndex === -1);
update the key handler so that when e.key === 'Enter' and showSuggestions is
true you call e.preventDefault() first to stop the form submit, and only then
call handleSelect(suggestions[activeIndex]) if activeIndex is within range (>=0
&& < suggestions.length); modify the Enter branch in the keydown handler in
ProductAutocompleteInput.jsx to always preventDefault when showSuggestions is
true, and keep the existing selection behavior for valid activeIndex values.
In `@frontend/src/components/ProductCatalogImport.jsx`:
- Around line 64-69: The component currently reads/writes
LAST_CATALOG_URL_INPUT_KEY regardless of the persistUrl setting, causing
presigned/tokenized URLs to be stored unintentionally; update the mount
useEffect that currently calls
window.localStorage.getItem(LAST_CATALOG_URL_INPUT_KEY) to only restore when
persistUrl is true, change the input change handler and the successful-load/save
path (the handlers that call window.localStorage.setItem for
LAST_CATALOG_URL_INPUT_KEY) to guard with persistUrl before writing, and update
the persist toggle handler to remove LAST_CATALOG_URL_INPUT_KEY from
localStorage when persistUrl is turned off so stored URLs are cleared; refer to
the useEffect that calls setUrlInput, the input onChange/setUrlInput flow, the
successful-load/save logic, and the persist toggle handler to make these
changes.
In `@frontend/src/hooks/useProductCatalog.js`:
- Around line 53-59: The current validator (in useProductCatalog.js, around
hasRequired/normalizedData) only ensures presence of name/price but allows
formatted values like "$10", "5%", or "1,000" which later break
ethers.parseUnits; add a numeric-format validation: implement a helper (e.g.,
isDecimal) that checks a string matches a pure decimal pattern (/^\d+(\.\d+)?$/)
and update the normalizedData validation to reject any row where price, qty,
tax, or discount are present but do not pass isDecimal; throw a clear Error
listing the offending rows or a message like "Invalid numeric format in
price/qty/tax/discount" so imports with currency symbols/commas/percent signs
are rejected early.
In `@frontend/src/page/CreateInvoice.jsx`:
- Around line 130-137: The code currently focuses both itemRefsMobile and
itemRefsDesktop causing the hidden desktop input to steal focus on small
screens; change the setTimeout callback to only focus the visible/input for the
active layout by checking visibility (e.g., test offsetParent or
getBoundingClientRect().height) or an existing layout flag before calling focus:
inspect itemRefsMobile.current[index + 1] and only call .focus() if that element
exists and is visible, otherwise focus itemRefsDesktop.current[index + 1]; keep
the same index logic and timeout but ensure only one .focus() call runs.
In `@frontend/src/page/CreateInvoicesBatch.jsx`:
- Line 39: The code imports react-hot-toast but still calls react-toastify APIs
(e.g. toast.info), which will throw; locate the toast usages in
CreateInvoicesBatch.jsx (the progress/creation handlers around the batch
creation logic and the lines noted: ~39, 320, 356-362, 482, 503) and replace
react-toastify calls with react-hot-toast equivalents (use toast(message) or
toast.loading(message) for in-progress notifications, and
toast.success(message)/toast.error(message) for completion/error updates, and
use toast.dismiss(id) if dismissing a loading toast); ensure any toast IDs
returned from toast.loading are preserved and passed to
toast.success/error/dismiss to update the same notification.
In `@frontend/src/utils/productCatalogInvoiceHelpers.js`:
- Around line 41-45: The qty field currently uses nullish coalescing
(product.qty ?? item.qty ?? '1') which treats empty string as a defined value
and ends up blank; update the mapping to treat empty string as missing by using
a falsy check so the default "1" wins (e.g. use product.qty || item.qty || '1',
wrapped with String(...) if needed), and apply the same change to the other qty
mapping block referenced (the similar expressions at the later block around
lines 48-53) so selecting a product without a qty defaults to "1".
🪄 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: 2e550daa-638e-4773-a7ea-ac4500f92211
📒 Files selected for processing (7)
frontend/package.jsonfrontend/src/components/ProductAutocompleteInput.jsxfrontend/src/components/ProductCatalogImport.jsxfrontend/src/hooks/useProductCatalog.jsfrontend/src/page/CreateInvoice.jsxfrontend/src/page/CreateInvoicesBatch.jsxfrontend/src/utils/productCatalogInvoiceHelpers.js
Atharva0506
force-pushed
the
feature/product-catalog-import
branch
from
1d7fd07 to
bee58c7
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/page/CreateInvoicesBatch.jsx (1)
132-132: 🧹 Nitpick | 🔵 TrivialDependency array with stringified itemData may cause unnecessary recalculations.
The useEffect dependency
invoiceRows.map((r) => JSON.stringify(r.itemData)).join(",")serializes all item data on every render to detect changes. While functional, this creates a new string each render causing the comparison to always run.Consider using a ref to track previous state or memoizing the serialization, though the current approach is acceptable for moderate batch sizes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/page/CreateInvoicesBatch.jsx` at line 132, The dependency for the useEffect using invoiceRows.map((r) => JSON.stringify(r.itemData)).join(",") creates a new string every render causing unnecessary effect runs; update the effect to use a stable dependency instead—either memoize the serialized value with useMemo (e.g., const serializedItems = useMemo(() => invoiceRows.map(r => JSON.stringify(r.itemData)).join(","), [invoiceRows]) and depend on serializedItems) or maintain the previous serialized state in a useRef and compare before running logic; locate the useEffect that references invoiceRows and itemData and replace the inline serialization in the dependency array with one of these stable approaches (useMemo or useRef-based comparison).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/ProductAutocompleteInput.jsx`:
- Around line 82-88: The scrollActiveIntoView function is being invoked from
inside setActiveIndex's updater where the DOM may not reflect the new
activeIndex; move scrolling out of the state updater and trigger it after React
commits by adding a useEffect that runs when activeIndex changes and calls
scrollActiveIntoView(activeIndex). Ensure scrollActiveIntoView's useCallback
dependencies include listRef (or adjust to a stable function) so the effect can
safely depend on scrollActiveIntoView and activeIndex; also remove any direct
calls to scrollActiveIntoView from inside setActiveIndex updater.
In `@frontend/src/hooks/useProductCatalog.js`:
- Around line 277-278: The current fetch check in useProductCatalog (the
requestUrl/requestUrl fetch and response.ok check) throws a generic
Error('Failed to fetch data from URL') which hides CORS failure clues; update
the error handling and associated UI/guide: when response.ok is false or the
fetch throws, surface a clearer message suggesting CORS as a potential cause
(mention "CORS: ensure the external server sets Access-Control-Allow-Origin"),
and add a short UI/help text or link in the URL input/fetch section that
explains CORS requirements and how to enable it on the data source; reference
the fetch call around requestUrl and the error path where response is checked so
the UI/help text and enhanced error are shown to users when fetching fails.
---
Outside diff comments:
In `@frontend/src/page/CreateInvoicesBatch.jsx`:
- Line 132: The dependency for the useEffect using invoiceRows.map((r) =>
JSON.stringify(r.itemData)).join(",") creates a new string every render causing
unnecessary effect runs; update the effect to use a stable dependency
instead—either memoize the serialized value with useMemo (e.g., const
serializedItems = useMemo(() => invoiceRows.map(r =>
JSON.stringify(r.itemData)).join(","), [invoiceRows]) and depend on
serializedItems) or maintain the previous serialized state in a useRef and
compare before running logic; locate the useEffect that references invoiceRows
and itemData and replace the inline serialization in the dependency array with
one of these stable approaches (useMemo or useRef-based comparison).
🪄 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: 6725625f-a684-4cd6-9a29-b7a4e93fe16c
📒 Files selected for processing (7)
frontend/package.jsonfrontend/src/components/ProductAutocompleteInput.jsxfrontend/src/components/ProductCatalogImport.jsxfrontend/src/hooks/useProductCatalog.jsfrontend/src/page/CreateInvoice.jsxfrontend/src/page/CreateInvoicesBatch.jsxfrontend/src/utils/productCatalogInvoiceHelpers.js
Atharva0506
force-pushed
the
feature/product-catalog-import
branch
from
bee58c7 to
91e52fe
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/ProductAutocompleteInput.jsx`:
- Around line 82-92: The effect calls scrollActiveIntoView before the function
is declared, causing a temporal dead zone error; move the scrollActiveIntoView
const (the useCallback that accesses listRef and queries '[role="option"]') so
it is declared above the useEffect that uses it (keep the useCallback dependency
array as appropriate, [] is fine because listRef is stable) and then leave the
useEffect dependency array as [activeIndex, scrollActiveIntoView] so the effect
can safely call the function.
In `@frontend/src/components/ProductCatalogImport.jsx`:
- Around line 64-72: The effect that hydrates the URL input should prefer the
persisted IndexedDB value `savedUrl` before falling back to localStorage; update
the `useEffect` (the block using `persistUrl`, `LAST_CATALOG_URL_INPUT_KEY`, and
`setUrlInput`) to: if `savedUrl` exists use it to call `setUrlInput(savedUrl)`,
otherwise read `window.localStorage.getItem(LAST_CATALOG_URL_INPUT_KEY)` and
call `setUrlInput` with that fallback; keep the existing `persistUrl` guard and
only access `window` when defined.
In `@frontend/src/hooks/useProductCatalog.js`:
- Around line 128-130: The current branch that handles paths with
'/spreadsheets/d/e/' replaces '/pubhtml' with '/pub' and then always returns
`${url.origin}${path.replace('/pubhtml', '/pub')}?output=csv`, which discards
any existing query params like gid/single; update this branch to preserve and
merge the original query string: parse or read the existing url.search (the
query on the original URL), ensure `output=csv` is present (add or override it)
and then return `${url.origin}${path.replace('/pubhtml',
'/pub')}${mergedSearch}`, so gid/single and other params remain included; look
for the code around path.includes('/spreadsheets/d/e/') and the url/path
variables to implement this merge.
- Around line 328-338: The callback disableURLPersistence currently writes
downgraded metadata back to persistent storage (via saveCatalogData), causing
'url-temp' to survive reloads; change it to avoid persisting the
updatedMetadata—do not call saveCatalogData with the downgraded object that
writes to CATALOG_KEY, instead update in-memory/session state by calling
broadcastCatalogUpdate(updatedMetadata) (or an equivalent non-persistent
updater) and keep setSavedUrl(null); also add broadcastCatalogUpdate to the
disableURLPersistence dependency array so the callback is stable.
In `@frontend/src/page/CreateInvoicesBatch.jsx`:
- Around line 1104-1124: The suggestion list is clipped because the surrounding
items container still has overflow-hidden; locate the parent container that
wraps these rows (the container that renders the elements containing
ProductAutocompleteInput/handleItemData/handleProductSelect) and change its
overflow rule to allow visible overflow (e.g., remove the overflow-hidden
Tailwind class or replace it with overflow-visible or set style={{ overflow:
'visible' }}), keeping the existing relative positioning and zIndex logic so the
absolute-positioned autocomplete menu can render outside the card.
In `@frontend/src/utils/productCatalogInvoiceHelpers.js`:
- Around line 19-28: computeLineAmount currently adds the tax value as a flat
amount; change it to treat tax as a percent: after computing lineTotal (qtyBN *
priceBN / ONE) compute taxAmount = (lineTotal * taxBN) / (ONE * 100) and use
finalAmount = lineTotal - discountBN + taxAmount, keeping
formatUnits(finalAmount, PRECISION); update any corresponding invoice total
calculations that also assume tax is a flat amount to use the same
percent-to-amount conversion (look for functions/variables like lineTotal,
taxBN, discountBN, ONE, PRECISION and any invoice totals aggregation code in
this file).
🪄 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: d1c9c878-c04e-441b-870c-df79222f4f01
📒 Files selected for processing (7)
frontend/package.jsonfrontend/src/components/ProductAutocompleteInput.jsxfrontend/src/components/ProductCatalogImport.jsxfrontend/src/hooks/useProductCatalog.jsfrontend/src/page/CreateInvoice.jsxfrontend/src/page/CreateInvoicesBatch.jsxfrontend/src/utils/productCatalogInvoiceHelpers.js
Atharva0506
force-pushed
the
feature/product-catalog-import
branch
from
91e52fe to
66e5b4d
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/ProductCatalogImport.jsx`:
- Around line 59-61: persistUrl is currently true by default and code writes the
draft URL while typing/fetching and resets the toggle back on in handleClear,
causing sensitive presigned/tokenized URLs to be stored; change the default of
persistUrl to false, stop writing urlInput to any persistent draft storage
unless persistUrl is true (check persistUrl before saving during onChange/fetch
handlers), update handleClear to reset setPersistUrl(false) instead of true, and
audit other occurrences that read/write the draft URL (the onChange handler for
urlInput, any fetch-success handlers, and handleClear) to ensure they respect
persistUrl before persisting.
- Around line 286-303: The URL input currently lets Enter bubble up and submit
the parent form; add an onKeyDown handler to the Input (the component bound to
urlInput) that detects Enter (e.key === 'Enter'), prevents default and stops
propagation, and invokes handleUrlLoad() (only when not isProcessing and
urlInput.trim() is non-empty) so pressing Enter triggers the fetch instead of
submitting the parent form.
In `@frontend/src/hooks/useProductCatalog.js`:
- Around line 38-43: The loop currently treats falsy values (e.g. unit_price: 0)
as absent because it checks normRow[alias]; change the condition to check
property presence and only treat canonical as missing when its value is
undefined (e.g. use Object.prototype.hasOwnProperty.call(normRow, alias) &&
normRow[canonical] === undefined) so values like 0 are preserved; update the
block that assigns normRow[canonical] = normRow[alias] and deletes
normRow[alias] accordingly to use those checks and reference FIELD_ALIAS_MAP,
normRow, alias, and canonical.
- Around line 143-150: The current code in useProductCatalog.js detects docId
from path but only reads gid from url.searchParams, which misses sheet selection
when it's in the URL hash (`#gid`=...), causing wrong-tab imports; update the gid
resolution to first try url.searchParams.get('gid'), then fallback to parsing
url.hash (e.g., use new URLSearchParams(url.hash.slice(1)).get('gid') or
url.hash.match(/gid=(\d+)/)) and finally fallback to '0', keeping the rest of
the logic (docId, export URL) unchanged.
In `@frontend/src/page/CreateInvoicesBatch.jsx`:
- Around line 1104-1105: The zIndex calculation using "50 - itemIndex" (in
CreateInvoicesBatch.jsx) and the similar "50 - index" in CreateInvoice.jsx can
become zero or negative for long lists; change these to clamp the result to a
minimum of 1 (or compute from the current item count so the top-most item stays
highest) so popups stay above other rows: locate the inline style objects where
style={{ zIndex: 50 - itemIndex }} (and style={{ zIndex: 50 - index }}) are set
and replace the expression with a guarded/clamped value (e.g., Math.max(1, 50 -
itemIndex) or derive from items.length) to ensure zIndex never drops below 1.
🪄 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: ebd7cd6d-6b59-4103-8a32-8ae92f08dd5e
📒 Files selected for processing (7)
frontend/package.jsonfrontend/src/components/ProductAutocompleteInput.jsxfrontend/src/components/ProductCatalogImport.jsxfrontend/src/hooks/useProductCatalog.jsfrontend/src/page/CreateInvoice.jsxfrontend/src/page/CreateInvoicesBatch.jsxfrontend/src/utils/productCatalogInvoiceHelpers.js
- Prevent storing tokenized URLs implicitly by setting persistUrl to false by default - Fix ENTER submitting the form instead of fetching URL by grouping into form - Preserve valid falsy properties like unit_price: 0 in FIELD_ALIAS_MAP - Read gid from hash in Google Sheets exports to preserve sheet selection - Clamp overlapping zIndex calculation to prevent negative stacking contexts - Add missing papaparse and idb-keyval packages to dependencies - Fix mixed toast library usage in batch creation page
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/ProductCatalogImport.jsx`:
- Around line 59-76: The useEffect in ProductCatalogImport.jsx should
synchronize the persistUrl checkbox with an already-hydrated savedUrl: when
savedUrl is truthy, call setPersistUrl(true) and setUrlInput(savedUrl) so the UI
reflects that a URL is persisted; keep the early-return when persistUrl is
already true to avoid flipping user intent, and ensure the effect depends on
[persistUrl, savedUrl] (as shown) so hydration from IndexedDB will update the
checkbox and input consistently.
- Around line 286-320: Replace the outer <form> wrapper in
ProductCatalogImport.jsx with a non-form container (e.g., a <div>) to avoid
nested form elements; remove the onSubmit handler that preventsDefault and calls
handleUrlLoad and instead wire the Fetch action to the Fetch URL button's
onClick to call handleUrlLoad (ensure its type is not "submit"), and add an
onKeyDown handler on the URL input (the Input that uses urlInput/setUrlInput)
that detects Enter (and that !isProcessing and urlInput.trim()) and calls
handleUrlLoad; keep existing localStorage logic (persistUrl,
LAST_CATALOG_URL_INPUT_KEY) and preserve disabled/aria state for isProcessing,
and leave handleRefresh, hasSavedUrlSource and isLocalCatalogActive logic
unchanged.
In `@frontend/src/hooks/useProductCatalog.js`:
- Around line 131-154: The path parsing misses multi-account Google Sheets URLs
that include "/u/<n>/" (e.g. "/spreadsheets/u/0/d/..."); update the logic in
useProductCatalog (the block that checks path.includes('/spreadsheets/d/e/') and
the regex /\/spreadsheets\/d\/([^/]+)/) to normalize or accept an optional
"/u/<n>" segment before matching: either strip any "/u/<number>" segment from
path/url.pathname up front or change the regex to something like
/\/spreadsheets(?:\/u\/\d+)?\/d\/([^/]+)/ so docId extraction (docId = match[1])
and subsequent gid resolution (url.searchParams/get('gid') and url.hash parsing)
work for both single-account and multi-account links and the export-to-CSV URL
is produced correctly.
In `@frontend/src/page/CreateInvoice.jsx`:
- Around line 1114-1115: The rows are keyed by array index causing React to
reuse ProductAutocompleteInput instances and misalign refs when items are
spliced; update createEmptyInvoiceItem to generate a stable id (e.g.,
crypto.randomUUID()) and ensure each new item object includes id, change the row
key in the itemData.map from index to item.id, and replace
itemRefsMobile/itemRefsDesktop arrays with objects keyed by item.id so refs are
set/get by id; also update any places that splice or manipulate itemData to
preserve item.id on existing items and use id-based lookups for refs and focus
handling.
In `@frontend/src/page/CreateInvoicesBatch.jsx`:
- Around line 1102-1106: The mapped batch line uses array index as key which
breaks ProductAutocompleteInput state on deletes; modify
createEmptyInvoiceItem() to add a stable unique identifier (e.g., id generated
via UUID or incrementing counter/timestamp) on each item object, update any code
paths that create new items to include that id, and change the mapping in
CreateInvoicesBatch.jsx (the row.itemData.map callback) to use item.id as the
React key instead of itemIndex so each item retains its identity across
insert/delete operations.
🪄 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: 241a1650-8c2e-47e7-8920-50e5abe8a847
📒 Files selected for processing (4)
frontend/src/components/ProductCatalogImport.jsxfrontend/src/hooks/useProductCatalog.jsfrontend/src/page/CreateInvoice.jsxfrontend/src/page/CreateInvoicesBatch.jsx
- Sync savedUrl hydrate state to UI checkbox - Refactor nested form structure into explicit Enter key handler - Regex match multi-account URLs in Google Sheets importer - Use stable item.id identifiers to prevent row recycling bugs in invoice lines
✅ Actions performedReview triggered.
|
1 similar comment
✅ Actions performedReview triggered.
|
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/page/CreateInvoice.jsx (1)
1202-1207: 🧹 Nitpick | 🔵 TrivialMinor: Display amount calculation differs from stored
item.amount.The disabled amount inputs use inline
parseFloatarithmetic whileapplyProductToInvoiceItemuses ethers-based fixed-point math for the actual storeditem.amount. For display purposes in disabled fields this is acceptable, but could show slight floating-point discrepancies on edge cases. Consider usingitemData[index]?.amountdirectly if it's already computed.Optional: Use pre-computed amount
value={String( - (parseFloat(itemData[index]?.qty) || 0) * - (parseFloat(itemData[index]?.unitPrice) || 0) - - (parseFloat(itemData[index]?.discount) || 0) + - (parseFloat(itemData[index]?.tax) || 0) + itemData[index]?.amount || '0' )}Also applies to: 1301-1306
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/page/CreateInvoice.jsx` around lines 1202 - 1207, The disabled amount input is computing display value with inline parseFloat arithmetic which can differ from the stored fixed-point value computed by applyProductToInvoiceItem; change the value expression to use the precomputed itemData[index]?.amount (or fallback to the existing calc if amount is missing) so the UI shows the same amount as stored (refer to itemData, item.amount, and applyProductToInvoiceItem when making the change); ensure similar updates are applied to the other disabled amount input occurrences like the block around lines 1301-1306.frontend/src/page/CreateInvoicesBatch.jsx (1)
993-1004: 🧹 Nitpick | 🔵 TrivialConsider adding client address validation similar to CreateInvoice.jsx.
The batch page accepts client wallet addresses without the same validation present in
CreateInvoice.jsx(checkingethers.isAddress, preventing self-invoicing). Invalid addresses will fail at transaction time, but earlier validation would improve UX.Add inline validation
Consider reusing or adapting the
validateClientAddresslogic fromCreateInvoice.jsxto provide immediate feedback when users enter invalid addresses in batch invoices.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/page/CreateInvoicesBatch.jsx` around lines 993 - 1004, Add the same client wallet validation used in CreateInvoice.jsx to the batch row input: wire up a validateClientAddress check when updating row.clientAddress in the Input component handling (where updateInvoiceRow is called) and show inline feedback/error state for that specific row (e.g., mark the invalid row and prevent form submission). Reuse or adapt the validateClientAddress function from CreateInvoice.jsx to call onChange (or onBlur) for the Input tied to row.clientAddress, ensure it uses ethers.isAddress and checks against the current user's address to prevent self-invoicing, and surface the validation result in the row state so the batch submit logic can block or highlight invalid rows.
♻️ Duplicate comments (1)
frontend/src/hooks/useProductCatalog.js (1)
335-353:⚠️ Potential issue | 🟡 MinorAdd
broadcastCatalogUpdateto the dependency array.The
disableURLPersistencecallback usesbroadcastCatalogUpdatebut it's missing from the dependency array, which could cause stale closure issues.Suggested fix
- }, [catalogMetadata]); + }, [catalogMetadata, broadcastCatalogUpdate]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/hooks/useProductCatalog.js` around lines 335 - 353, The disableURLPersistence callback closes over broadcastCatalogUpdate but doesn't include it in its dependency array, risking stale closures; update the hook dependencies for useCallback on disableURLPersistence to include broadcastCatalogUpdate (alongside catalogMetadata) so the latest function reference is used, i.e., add broadcastCatalogUpdate to the dependency array for disableURLPersistence where it's declared.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/hooks/useProductCatalog.js`:
- Around line 347-349: Remove developer "thinking" comments left next to the
broadcast call; specifically delete the two inline comments around the if
(typeof broadcastCatalogUpdate === 'function')
broadcastCatalogUpdate(updatedMetadata); line so only the functional check and
call remain (leave the broadcastCatalogUpdate and updatedMetadata identifiers
unchanged). Ensure no other stray notes remain in the useProductCatalog hook.
In `@frontend/src/page/CreateInvoicesBatch.jsx`:
- Around line 245-262: The handleProductSelect function appends a new empty
invoice item but doesn't move focus to the new input like CreateInvoice.jsx
does; update handleProductSelect to manage refs and focus: when you detect
itemIndex === row.itemData.length - 1 and push createEmptyInvoiceItem(), also
create/maintain a refs structure (e.g., itemQuantityRefs per row and item) and
after setInvoiceRows completes, call focus on the newly appended item's input
(e.g., itemQuantityRefs[rowIndex][newItemIndex].current.focus()); ensure refs
are created when rows/items are created and cleaned/updated when items shift so
applyProductToInvoiceItem, createEmptyInvoiceItem and setInvoiceRows work with
the new refs.
---
Outside diff comments:
In `@frontend/src/page/CreateInvoice.jsx`:
- Around line 1202-1207: The disabled amount input is computing display value
with inline parseFloat arithmetic which can differ from the stored fixed-point
value computed by applyProductToInvoiceItem; change the value expression to use
the precomputed itemData[index]?.amount (or fallback to the existing calc if
amount is missing) so the UI shows the same amount as stored (refer to itemData,
item.amount, and applyProductToInvoiceItem when making the change); ensure
similar updates are applied to the other disabled amount input occurrences like
the block around lines 1301-1306.
In `@frontend/src/page/CreateInvoicesBatch.jsx`:
- Around line 993-1004: Add the same client wallet validation used in
CreateInvoice.jsx to the batch row input: wire up a validateClientAddress check
when updating row.clientAddress in the Input component handling (where
updateInvoiceRow is called) and show inline feedback/error state for that
specific row (e.g., mark the invalid row and prevent form submission). Reuse or
adapt the validateClientAddress function from CreateInvoice.jsx to call onChange
(or onBlur) for the Input tied to row.clientAddress, ensure it uses
ethers.isAddress and checks against the current user's address to prevent
self-invoicing, and surface the validation result in the row state so the batch
submit logic can block or highlight invalid rows.
---
Duplicate comments:
In `@frontend/src/hooks/useProductCatalog.js`:
- Around line 335-353: The disableURLPersistence callback closes over
broadcastCatalogUpdate but doesn't include it in its dependency array, risking
stale closures; update the hook dependencies for useCallback on
disableURLPersistence to include broadcastCatalogUpdate (alongside
catalogMetadata) so the latest function reference is used, i.e., add
broadcastCatalogUpdate to the dependency array for disableURLPersistence where
it's declared.
🪄 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: 0c9aac6b-ddac-4547-9ee0-655b62793b87
📒 Files selected for processing (5)
frontend/src/components/ProductCatalogImport.jsxfrontend/src/hooks/useProductCatalog.jsfrontend/src/page/CreateInvoice.jsxfrontend/src/page/CreateInvoicesBatch.jsxfrontend/src/utils/productCatalogInvoiceHelpers.js
Contributor
Author
|
@coderabbitai I have reviewed and resolved all the issues |
|
✅ Actions performedReview triggered.
|
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
- Align UX with CreateInvoice.jsx by auto-focusing the appended input field horizontally after autocomplete selections in CreateInvoicesBatch.jsx.
…tharva0506/Chainvoice into feature/product-catalog-import
… disableURLPersistence
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/hooks/useProductCatalog.js`:
- Around line 335-355: The disableURLPersistence callback captures
broadcastCatalogUpdate but does not list it in its dependency array, risking a
stale closure; update the dependency array for the disableURLPersistence
function to include broadcastCatalogUpdate (alongside catalogMetadata) so the
hook updates when broadcastCatalogUpdate changes, and ensure you only modify the
dependency list in the useCallback declaration for disableURLPersistence. Also,
replace the hard-coded user-visible error strings passed to toast.error
throughout this file (calls around the toast.error usages) with i18n resource
lookups so all messages (previously hard-coded at the noted lines) are
externalized for localization.
- Around line 84-103: The parseAndNormalizeCSV function currently only rejects
when Papa.parse returns errors AND zero data; change the logic to reject on any
parsing errors: inside the Papa.parse complete callback in parseAndNormalizeCSV,
check results.errors.length > 0 and immediately reject with a descriptive Error
(include results.errors info) instead of allowing normalizeRows(results.data) to
run; keep normalizeRows and the existing try/catch for successful parses so only
fully error-free parse results proceed.
🪄 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: 9c000cae-d1a1-4790-86f6-5c5bca0911e6
📒 Files selected for processing (1)
frontend/src/hooks/useProductCatalog.js
Contributor
Author
|
@coderabbitai review fixed all suggestions |
|
Regarding the approval — to approve the PR, please post ✅ Actions performedReview triggered.
|
|
✅ Actions performedReview triggered.
|
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.
Addressed Issues:
Fixes #155
Screenshots/Recordings:
Demo video attached (Product Catalog Import flow):
prdocut.catelog.1.mp4
Additional Notes:
Before creating this PR, I manually tested edge cases locally, including:
Package Updates
This PR introduces two new library dependencies to
package.jsonand here is why they were needed to support the feature robustly:papaparse: Used for parsing and converting uploaded CSV files or CSV text fetched from URLs into structured JavaScript objects. It's the industry standard for browser-side CSV parsing, seamlessly handling edge cases like malformed rows, tricky delimiters, and large datasets efficiently.idb-keyval: A super-lightweight promise-based wrapper for IndexedDB. It's used to asynchronously store and retrieve the parsed product catalog locally. This safely bypasses the strict 5MB limit oflocalStorage(allowing support for catalogs with thousands of products) and prevents main-thread blocking during read/write operations.AI Usage Disclosure:
I have used the following AI models and tools: N/A
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Chores