feat: add settings infrastructure, enhanced line items, i18n, and bug fixes#1082
Open
NickDabizaz wants to merge 2 commits intoal1abb:masterfrom
Open
feat: add settings infrastructure, enhanced line items, i18n, and bug fixes#1082NickDabizaz wants to merge 2 commits intoal1abb:masterfrom
NickDabizaz wants to merge 2 commits intoal1abb:masterfrom
Conversation
… fixes - feat(settings): add SettingsContext with localStorage persistence and dynamic Zod schema factory - feat(settings): add Settings panel (Sheet/Drawer) with gear icon in navbar - feat(line-items): add per-item discount, tax, and SKU/item code columns (settings-gated) - feat(payment): add cash payment mode with change field and conditional bank details - feat(currency): add auto currency symbol display in Invoice Details and line items - feat(import): add multi-format import support for CSV, XML, and XLS/XLSX - feat(i18n): register Bahasa Indonesia locale in LOCALES array - fix(navbar): comment out DevDebug panel to remove DEV debug overlay in navbar - chore(migration): add migrateInvoice() to handle legacy localStorage invoices with new fields
Contributor
|
@NickDabizaz is attempting to deploy a commit to the al1abb-team Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughA settings system and UI were added plus persisted settings driving runtime behavior: schema generation, form fields, PDF rendering, and UI visibility. Invoice items gained SKU/discount/tax fields and calculations. Import flow now accepts JSON/CSV/XML/XLS/XLSX. SettingsProvider, SettingsPanel, and form/schema integrations were introduced. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Navbar as BaseNavbar
participant Panel as SettingsPanel
participant SettingsHook as useSettings
participant Storage as localStorage
User->>Navbar: Click Settings icon
Navbar->>Panel: set isOpen=true
Panel->>SettingsHook: updateSettings(partial)
SettingsHook->>SettingsHook: merge settings
SettingsHook->>Storage: save JSON (localStorage)
SettingsHook->>Providers: triggers schema regeneration (resolver)
Note over Providers: Form resolver rebuilt using new settings
sequenceDiagram
participant User
participant ImportBtn as ImportInvoiceButton
participant FileInput as File Input
participant Parser as parseImportedFile
participant InvoiceCtx as InvoiceContext.importInvoice
participant Form as FormContext.reset
User->>ImportBtn: Click "Import invoice"
ImportBtn->>FileInput: open file dialog
User->>FileInput: select file (json/csv/xml/xls/xlsx)
FileInput->>Parser: parseImportedFile(file)
Parser->>Parser: format-specific parsing (JSON/CSV/XML/XLSX)
Parser-->>InvoiceCtx: return InvoiceType
InvoiceCtx->>InvoiceCtx: convert dates, clean items
InvoiceCtx->>Form: reset(importedData)
Form-->>User: form populated
ImportBtn->>User: close modal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
contexts/InvoiceContext.tsx (1)
52-52:⚠️ Potential issue | 🟡 MinorType signature mismatch:
importInvoiceis now async but the default context type does not reflect this.The actual implementation returns
Promise<void>, but the default shows a synchronous signature. This may cause TypeScript inference issues for consumers awaiting the function.🔧 Proposed fix
- importInvoice: (file: File) => {}, + importInvoice: async (file: File): Promise<void> => {},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contexts/InvoiceContext.tsx` at line 52, The default context type for importInvoice is declared as a synchronous function but the real implementation is async; update the type/signature where the InvoiceContext shape is defined so importInvoice: (file: File) => Promise<void>, and ensure any default no-op or provider stub (the default value passed into createContext or the InvoiceProvider) returns a Promise (e.g., an async no-op) to match the Promise<void> return type (referencing importInvoice and the InvoiceContext/InvoiceProvider symbols).
🧹 Nitpick comments (10)
app/components/invoice/form/sections/ImportJsonButton.tsx (1)
12-12: Consider renaming the component to reflect multi-format support.The component name
ImportJsonButtonis now misleading since it supports JSON, CSV, XML, XLS, and XLSX. A name likeImportInvoiceButtonwould be more accurate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/invoice/form/sections/ImportJsonButton.tsx` at line 12, Rename the React component ImportJsonButton to a more accurate name like ImportInvoiceButton across the module: update the component declaration (const ImportJsonButton -> const ImportInvoiceButton), its exported identifier, and any references/usages, and keep the same props signature (ImportButtonType and setOpen) so behavior is unchanged; also update any default or named export statements and import sites to use the new component name.app/components/layout/BaseNavbar.tsx (1)
17-17: Consider removing commented-out DevDebug code entirely.The PR objective states removing the DevDebug overlay. Rather than leaving commented imports and JSX (lines 17, 42), consider removing them completely to keep the codebase clean. If needed in the future, it can be restored from git history.
🧹 Cleanup suggestion
// Components -// import { DevDebug, LanguageSelector, ThemeSwitcher } from "@/app/components"; import { LanguageSelector, ThemeSwitcher } from "@/app/components";- {/* {devEnv && <DevDebug />} */} <div className="flex items-center gap-2">Also applies to: 42-42
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/layout/BaseNavbar.tsx` at line 17, Remove the leftover commented DevDebug import and any commented DevDebug JSX in BaseNavbar.tsx: delete the commented import line that references DevDebug (currently on the import block) and remove the commented DevDebug JSX/usage in the component render (the commented code around where DevDebug would appear), so only active imports/components (e.g., LanguageSelector, ThemeSwitcher if used) remain; rely on git history if you need to restore it later.app/components/invoice/actions/PdfViewer.tsx (1)
33-44: Consider extracting data cleaning logic for reuse.The item cleaning logic will likely need to be applied in multiple places (here, PDF generation, export). Consider extracting it into a shared utility function in
lib/helpers.ts.♻️ Example extraction
// In lib/helpers.ts export const cleanInvoiceItemsForSettings = ( items: InvoiceItem[], settings: { discountPerItem: { enabled: boolean }, taxPerItem: { enabled: boolean } } ) => { return items.map(item => { const cleanedItem = { ...item }; if (!settings.discountPerItem.enabled) { cleanedItem.discount = undefined; cleanedItem.discountType = undefined; } if (!settings.taxPerItem.enabled) { cleanedItem.tax = undefined; cleanedItem.taxType = undefined; } return cleanedItem; }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/invoice/actions/PdfViewer.tsx` around lines 33 - 44, Extract the inline item-cleaning logic from PdfViewer.tsx into a reusable helper (e.g., export cleanInvoiceItemsForSettings in lib/helpers.ts) that accepts items: InvoiceItem[] and the settings object ({ discountPerItem, taxPerItem }) and returns the cleaned items array; then replace the map in PdfViewer.actions/PdfViewer.tsx with an import and call to cleanInvoiceItemsForSettings(items, settings). Ensure the helper has correct types (InvoiceItem and the settings shape) and update other places that need the same logic (PDF generation, export) to reuse this new function.app/components/reusables/form-fields/FormSelect.tsx (2)
45-45: Consider adding proper typing instead ofas anycast.The
name as anytype escape bypasses type safety. If the form field paths are dynamic, consider using a generic type parameter or the properPath<T>type from react-hook-form.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/reusables/form-fields/FormSelect.tsx` at line 45, The component is using a type escape for the name prop ("name as any"); update the FormSelect component to be generic over the form data type (e.g., FormSelect<TFormValues>) and type the name prop using react-hook-form's Path or FieldPath (import Path or FieldPath from "react-hook-form") instead of casting to any, then remove the "as any" cast in the JSX; ensure callers supply the correct generic or adjust exported prop types so usages of FormSelect no longer require unsafe casts.
49-52: RemovedefaultValueprop from controlledSelectcomponent.Setting both
defaultValue(line 51) andvalue(line 52) on the sameSelectcreates a controlled/uncontrolled hybrid. Sincevalueis provided, the component is controlled anddefaultValueis ignored by React. This can cause confusion and React may emit a warning.♻️ Proposed fix
<Select onValueChange={field.onChange} - defaultValue={field.value || defaultValue} value={field.value || defaultValue || ""} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/reusables/form-fields/FormSelect.tsx` around lines 49 - 52, Remove the uncontrolled prop to avoid a hybrid component: delete the defaultValue prop passed to the Select and make the component fully controlled by ensuring the value prop covers the initial fallback (e.g., set value to field.value ?? defaultValue ?? ""), keep onValueChange={field.onChange} as-is; this touches the Select element and the symbols field.onChange, field.value, and defaultValue in FormSelect.tsx.app/components/reusables/form-fields/FormInput.tsx (1)
50-50: Inconsistent nullish operator usage between FormInput and FormSelect.
FormInputuses??(nullish coalescing) here, while the newFormSelectcomponent uses||(logical OR). This creates inconsistent behavior:FormInputpreserves empty strings whileFormSelecttreats them as falsy and falls back todefaultValue. Consider aligning both components to use??for consistent handling of empty strings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/reusables/form-fields/FormInput.tsx` at line 50, The value assignment in FormInput currently uses the nullish coalescing operator (value={field.value ?? defaultValue ?? ""}), which preserves empty strings, while FormSelect uses logical OR (||) causing inconsistent behavior; update FormSelect to use the same nullish coalescing strategy as FormInput (use ?? when falling back from field.value to defaultValue and then to ""), ensuring both components handle empty strings consistently—look for the value prop usage in FormSelect and replace the || fallbacks with ?? matching FormInput's value resolution.lib/helpers.ts (1)
210-223: Currency symbol extraction may strip trailing periods from symbols like "Rp."The regex
/[\d.,\s]/gremoves periods, which will strip the trailing dot from currency symbols like Indonesian Rupiah (Rp.), resulting inRpinstead ofRp.. Consider preserving trailing periods or using a more targeted regex.💡 Suggested fix
// Format 0 to get the symbol const formatted = formatter.format(0); // Extract symbol from formatted string (e.g., "$0" -> "$") - const symbol = formatted.replace(/[\d.,\s]/g, ""); + const symbol = formatted.replace(/[\d\s]/g, "").replace(/^[.,]+|[.,]+$/g, (match, offset) => offset === 0 ? "" : match);Alternatively, a simpler approach that only removes digits and surrounding whitespace:
- const symbol = formatted.replace(/[\d.,\s]/g, ""); + const symbol = formatted.replace(/\d/g, "").trim();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/helpers.ts` around lines 210 - 223, The getCurrencySymbol function currently strips periods by using formatted.replace(/[\d.,\s]/g, ""), which removes trailing dots like the one in "Rp."; instead, change the extraction to use Intl.NumberFormat.prototype.formatToParts(0) and return the part whose type === "currency" (falling back to currencyCode if missing) so you preserve punctuation such as trailing periods and avoid brittle regex-based removal.lib/schemas.ts (3)
236-281: Schema shape inconsistency may cause validation failures.When
cashPaymentMode.enabledisfalse(lines 276-281), the returned schema omitsisCashandchangefields entirely. If the form or imported data includes these fields, Zod's default behavior (strip unknown keys) will silently remove them during.parse(), but this inconsistency could cause confusion or issues with data flow.Consider always including these fields as optional for consistent schema shape:
♻️ Proposed fix for consistent schema shape
return z.object({ bankName: fieldValidators.stringMin1, accountName: fieldValidators.stringMin1, accountNumber: fieldValidators.stringMin1, + isCash: z.boolean().optional(), + change: fieldValidators.nonNegativeNumber.optional(), }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/schemas.ts` around lines 236 - 281, The returned schema from createPaymentInformationSchema is inconsistent: when settings.cashPaymentMode.enabled is false it omits isCash and change, which can lead to silent stripping of those keys; update the false-branch schema to include isCash: z.boolean().optional() and change: fieldValidators.nonNegativeNumber.optional() (matching the enabled-branch types) while keeping bankName/accountName/accountNumber as fieldValidators.stringMin1 so the overall shape stays consistent across all SettingsType paths.
283-357: Consider extracting common logic to reduce duplication.
createSenderSchemaandcreateReceiverSchemashare nearly identical structure, differing only in the settings keys (senderEmailvsreceiverEmail, etc.). A helper factory could reduce maintenance burden.♻️ Example refactor using a shared helper
type PartyType = 'sender' | 'receiver'; const createPartySchema = (settings: SettingsType, party: PartyType) => { const prefix = party; const getValidator = (field: string, baseValidator: z.ZodTypeAny) => { const key = `${prefix}${field.charAt(0).toUpperCase() + field.slice(1)}` as keyof typeof settings.fieldRequirements; return settings.fieldRequirements[key] === "required" ? baseValidator : makeOptional(baseValidator); }; return z.object({ name: fieldValidators.name, address: getValidator('address', fieldValidators.address), zipCode: getValidator('zipCode', fieldValidators.zipCode), city: getValidator('city', fieldValidators.city), country: getValidator('country', fieldValidators.country), email: getValidator('email', fieldValidators.email), phone: getValidator('phone', fieldValidators.phone), customInputs: z.array(CustomInputSchema).optional(), }); }; const createSenderSchema = (settings: SettingsType) => createPartySchema(settings, 'sender'); const createReceiverSchema = (settings: SettingsType) => createPartySchema(settings, 'receiver');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/schemas.ts` around lines 283 - 357, The two nearly identical factories createSenderSchema and createReceiverSchema should be collapsed into a single helper (e.g., createPartySchema) that takes (settings: SettingsType, party: 'sender'|'receiver') and uses a small getValidator helper to build the correct settings.fieldRequirements key (e.g., `${party}${CapitalizedField}`) and return either the base fieldValidators entry or makeOptional(base) as currently done; replace createSenderSchema and createReceiverSchema with thin wrappers that call createPartySchema, preserving use of fieldValidators, makeOptional and CustomInputSchema.
191-234: Type inference degrades with repeatedschema.extend()reassignments.TypeScript infers
schema's type from the initial assignment. Subsequent.extend()calls return a new Zod object with the merged shape, but the variable type remains the original. This may cause type errors when the schema is used downstream.Consider building the schema shape object first, then creating the schema once:
♻️ Proposed fix to preserve type inference
const createItemSchema = (settings: SettingsType) => { - let schema = z.object({ + const baseShape = { name: fieldValidators.stringMin1, description: fieldValidators.stringOptional, quantity: fieldValidators.quantity, unitPrice: fieldValidators.unitPrice, total: fieldValidators.stringToNumber, - }); + }; + + const dynamicShape: Record<string, z.ZodTypeAny> = {}; // Add SKU field if enabled if (settings.skuColumn.enabled) { const skuValidator = settings.skuColumn.required ? fieldValidators.stringMin1 : fieldValidators.stringOptional; - schema = schema.extend({ - sku: skuValidator, - }); + dynamicShape.sku = skuValidator; } // Add discount field if enabled if (settings.discountPerItem.enabled) { const discountValidator = settings.discountPerItem.required ? fieldValidators.nonNegativeNumber : fieldValidators.nonNegativeNumber.optional(); - schema = schema.extend({ - discount: discountValidator, - discountType: fieldValidators.stringOptional, - }); + dynamicShape.discount = discountValidator; + dynamicShape.discountType = fieldValidators.stringOptional; } // Add tax field if enabled if (settings.taxPerItem.enabled) { const taxValidator = settings.taxPerItem.required ? fieldValidators.nonNegativeNumber : fieldValidators.nonNegativeNumber.optional(); - schema = schema.extend({ - tax: taxValidator, - taxType: fieldValidators.stringOptional, - }); + dynamicShape.tax = taxValidator; + dynamicShape.taxType = fieldValidators.stringOptional; } - return schema; + return z.object({ ...baseShape, ...dynamicShape }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/schemas.ts` around lines 191 - 234, createItemSchema currently reassigns the Zod schema with repeated schema.extend() calls which preserves the original inferred type and degrades TypeScript type inference; instead build up a plain shape object (e.g. let shape: Record<string, ZodTypeAny> or a typed object) with the base fields (name, description, quantity, unitPrice, total), conditionally add sku/discount/discountType/tax/taxType entries using settings.skuColumn, settings.discountPerItem and settings.taxPerItem and the appropriate fieldValidators, and finally return z.object(shape) so the final schema type is inferred correctly from the single z.object(...) creation in createItemSchema.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/invoice/actions/PdfViewer.tsx`:
- Around line 28-54: Pdf preview vs exported PDF mismatch: the cleanedData
created in PdfViewer (used by LivePreview) is not passed into the PDF generation
flow, so FinalPdf/export still uses raw formValues; fix by applying the same
cleaning before generating the final PDF—either pass cleanedData into FinalPdf
and ensure FinalPdf calls generatePdf with that cleanedData, or move the
cleaning into InvoiceContext.generatePdf (create a helper like
cleanDataBasedOnSettings and call it at the start of generatePdf) so generatePdf
always posts the cleaned payload; update references to cleanedData, LivePreview,
FinalPdf and InvoiceContext.generatePdf accordingly.
In `@app/components/invoice/form/Charges.tsx`:
- Around line 68-100: The Charges switches (discountSwitch, taxSwitch) in
ChargesContext are not reset when per-item mode is enabled, causing hidden
invoice-level controls to persist; update ChargesContext (where discountSwitch,
taxSwitch, setValue are defined) to import and use the settings (via
useSettings) and add a useEffect that watches settings.discountPerItem.enabled
and settings.taxPerItem.enabled and when either becomes true call
setDiscountSwitch(false)/setTaxSwitch(false) and clear the related form values
via setValue("details.discountDetails.amount", 0) and
setValue("details.taxDetails.amount", 0) so invoice-level discounts/taxes are
removed when per-item mode is enabled.
In `@app/components/invoice/form/sections/ImportJsonButton.tsx`:
- Around line 26-31: Supported file types are checked with supportedFormats but
unsupported selections silently do nothing; update the ImportJsonButton file
(e.g., in the click/submit handler that uses supportedFormats, importInvoice,
and setOpen) to show user feedback when ext is not supported by calling the
app's toast/alert hook (or a fallback alert) with a clear message like
"Unsupported file format" and do not call importInvoice or setOpen in that case;
ensure you import/use the existing toast hook used elsewhere in the app and
reuse its success/error pattern for consistency.
In `@app/components/invoice/form/sections/Items.tsx`:
- Around line 55-59: The item initializer sets discountType and taxType to empty
strings which conflicts with the FormSelect in SingleItem.tsx that expects
"amount" or "percentage" (and uses "amount" as default) and with the calculation
logic in SingleItem (around the discount/tax calculation branches); update the
initializer in Items.tsx so that the object sets discountType: "amount" and
taxType: "amount" instead of "" to ensure the select pre-selects correctly and
matches the calculation assumptions.
In `@app/components/invoice/form/sections/PaymentInformation.tsx`:
- Around line 51-53: The bank-detail block is hidden using only !isCash which
causes fields to stay hidden if a draft has
details.paymentInformation.isCash=true but global cash mode is later disabled;
update the visibility condition in PaymentInformation.tsx to render bank fields
unless both the payment is cash AND the global cash-mode feature is enabled
(i.e., hide only when details.paymentInformation.isCash is true AND the global
cash-mode flag is true), referencing the existing isCash and
details.paymentInformation.isCash variables and the global cash-mode flag used
in this component so required fields remain visible when cash mode is globally
turned off.
In `@app/components/invoice/form/SingleItem.tsx`:
- Around line 100-126: The total is calculated incorrectly and hidden
discount/tax values can persist; update the useEffect (the calculation that
reads rate, quantity, discount, discountType, tax, taxType and calls
setValue(`${name}[${index}].total`)) to compute total = baseAmount -
discountValue + taxValue (not subtract tax), calculate discountValue only when
discount is valid and the discount feature is active, and calculate taxValue on
the post-discount base (if taxType === "percentage" then taxValue = (baseAmount
- discountValue) * (tax/100), else fixed taxValue = tax) — treat discount/tax as
0 when their features are off (e.g., check discountEnabled/taxEnabled or the
absence of discountType/taxType) and ensure values are parsed to numbers before
arithmetic and rounded with toFixed(2) before calling setValue.
In `@app/components/settings/SettingsPanel.tsx`:
- Line 22: The settings drawer in SettingsPanel uses hardcoded English strings
(start at the const t = useTranslations(); and throughout the SettingsPanel
component between lines ~77-375), so replace all literal UI text in
SettingsPanel (drawer title, labels, help text, buttons, placeholders, etc.)
with localized calls using the existing useTranslations() instance (e.g.,
t('settings.drawerTitle') or similar keys), and ensure you add corresponding
keys into the i18n locale resource files for each supported language; update any
subcomponents invoked by SettingsPanel that render static text to accept
translated strings or call t(...) directly.
- Around line 69-84: The settings drawer uses plain fixed <div>s and lacks
dialog semantics, focus trapping, Escape handling, and an accessible name for
the icon button; update the panel to act like a real modal by adding
role="dialog" and aria-modal="true" (and aria-labelledby pointing to the <h2>),
give the close Button an accessible name (aria-label="Close settings") and a ref
to receive initial focus, implement keyboard handling to call onClose on Escape
(e.g., attach a keydown listener in the SettingsPanel component), and add a
simple focus trap (or use a small utility/hook) to keep focus inside the panel
while open and restore focus on close so keyboard and screen-reader users cannot
tab into the page behind it.
In `@app/components/templates/invoice-pdf/InvoiceTemplate1.tsx`:
- Around line 237-247: In InvoiceTemplate1 (component rendering
details.paymentInformation) replace the non-cash branch's inline <span> wrapper
with a block container (e.g., a <div> or other block element) so the nested <p>
elements are valid HTML; update the JSX around the conditional that checks
details.paymentInformation?.isCash to render a block wrapper for the bank
details (Bank, Account name, Account no) while keeping the existing className
and text styling.
In `@app/components/templates/invoice-pdf/InvoiceTemplate2.tsx`:
- Around line 301-320: The current JSX in InvoiceTemplate2 nests <p> tags inside
a <span> for the non-cash branch (checking details.paymentInformation?.isCash),
which is invalid HTML; change the outer <span className="font-semibold text-md
text-gray-800"> to a block-level element (e.g., <div className="font-semibold
text-md text-gray-800">) or convert the inner <p> elements to inline elements to
ensure valid nesting and avoid hydration/rendering issues while keeping the same
classNames and content structure referencing
details.paymentInformation?.bankName, accountName, accountNumber.
In `@contexts/SettingsContext.tsx`:
- Around line 20-25: The effect reading LOCAL_STORAGE_SETTINGS_KEY must validate
and merge the parsed value instead of replacing state; when JSON.parse(stored)
is cast to SettingsType it can be partial and cause accesses like
settings.cashPaymentMode.enabled to crash. Update the useEffect logic that
currently calls setSettings(parsed) to first validate/normalize the parsed
object and deep-merge it with the app's default settings (or current settings)
so missing nested branches are preserved (e.g., use a utility deepMerge or
structured Object.assign/recursive merge) and only then call setSettings with
the merged result; keep the existing try/catch around JSON.parse and handle
invalid payloads by falling back to defaults.
In `@services/invoice/client/importInvoice.ts`:
- Around line 5-9: The docstring in the unflatten implementation is
inconsistent: it says "dot notation" but the code and example use
underscore-delimited keys (e.g., "sender_name"); update the comment to
accurately describe the behavior or make the function actually split on dots.
Specifically, in the Unflatten function / unflattenObject (the top comment block
in importInvoice.ts) either change the description and example to state
"underscore-separated keys (e.g., sender_name → sender.name)" or modify the
implementation to split on '.' instead of '_' so the docstring matches the code;
ensure the example and wording align with the chosen approach.
- Around line 74-94: The parseCSVLine function fails on RFC 4180 escaped quotes
because it simply toggles insideQuotes on every '"' and doesn't treat doubled
quotes as an escaped quote; update parseCSVLine to use lookahead when
encountering a '"'—if insideQuotes and the next character is also '"' consume
the second quote and append a single '"' to current (without toggling
insideQuotes), otherwise toggle insideQuotes as before; ensure you advance the
index to skip the consumed escaped quote so fields like `He said ""Hello""` are
parsed correctly.
- Around line 54-69: The parseCSV function can retain trailing CR characters
when input uses CRLF; normalize line endings before parsing by converting all
"\r\n" (and lone "\r") to "\n" or trim "\r" from header/data values: inside
parseCSV (and when calling parseCSVLine) replace CRs from csvText (or trim each
header/dataLine entry) so headers and result[header] values do not contain
trailing "\r". Ensure parseCSVLine is still used but fed normalized lines and
that result assignment uses cleaned values.
---
Outside diff comments:
In `@contexts/InvoiceContext.tsx`:
- Line 52: The default context type for importInvoice is declared as a
synchronous function but the real implementation is async; update the
type/signature where the InvoiceContext shape is defined so importInvoice:
(file: File) => Promise<void>, and ensure any default no-op or provider stub
(the default value passed into createContext or the InvoiceProvider) returns a
Promise (e.g., an async no-op) to match the Promise<void> return type
(referencing importInvoice and the InvoiceContext/InvoiceProvider symbols).
---
Nitpick comments:
In `@app/components/invoice/actions/PdfViewer.tsx`:
- Around line 33-44: Extract the inline item-cleaning logic from PdfViewer.tsx
into a reusable helper (e.g., export cleanInvoiceItemsForSettings in
lib/helpers.ts) that accepts items: InvoiceItem[] and the settings object ({
discountPerItem, taxPerItem }) and returns the cleaned items array; then replace
the map in PdfViewer.actions/PdfViewer.tsx with an import and call to
cleanInvoiceItemsForSettings(items, settings). Ensure the helper has correct
types (InvoiceItem and the settings shape) and update other places that need the
same logic (PDF generation, export) to reuse this new function.
In `@app/components/invoice/form/sections/ImportJsonButton.tsx`:
- Line 12: Rename the React component ImportJsonButton to a more accurate name
like ImportInvoiceButton across the module: update the component declaration
(const ImportJsonButton -> const ImportInvoiceButton), its exported identifier,
and any references/usages, and keep the same props signature (ImportButtonType
and setOpen) so behavior is unchanged; also update any default or named export
statements and import sites to use the new component name.
In `@app/components/layout/BaseNavbar.tsx`:
- Line 17: Remove the leftover commented DevDebug import and any commented
DevDebug JSX in BaseNavbar.tsx: delete the commented import line that references
DevDebug (currently on the import block) and remove the commented DevDebug
JSX/usage in the component render (the commented code around where DevDebug
would appear), so only active imports/components (e.g., LanguageSelector,
ThemeSwitcher if used) remain; rely on git history if you need to restore it
later.
In `@app/components/reusables/form-fields/FormInput.tsx`:
- Line 50: The value assignment in FormInput currently uses the nullish
coalescing operator (value={field.value ?? defaultValue ?? ""}), which preserves
empty strings, while FormSelect uses logical OR (||) causing inconsistent
behavior; update FormSelect to use the same nullish coalescing strategy as
FormInput (use ?? when falling back from field.value to defaultValue and then to
""), ensuring both components handle empty strings consistently—look for the
value prop usage in FormSelect and replace the || fallbacks with ?? matching
FormInput's value resolution.
In `@app/components/reusables/form-fields/FormSelect.tsx`:
- Line 45: The component is using a type escape for the name prop ("name as
any"); update the FormSelect component to be generic over the form data type
(e.g., FormSelect<TFormValues>) and type the name prop using react-hook-form's
Path or FieldPath (import Path or FieldPath from "react-hook-form") instead of
casting to any, then remove the "as any" cast in the JSX; ensure callers supply
the correct generic or adjust exported prop types so usages of FormSelect no
longer require unsafe casts.
- Around line 49-52: Remove the uncontrolled prop to avoid a hybrid component:
delete the defaultValue prop passed to the Select and make the component fully
controlled by ensuring the value prop covers the initial fallback (e.g., set
value to field.value ?? defaultValue ?? ""), keep onValueChange={field.onChange}
as-is; this touches the Select element and the symbols field.onChange,
field.value, and defaultValue in FormSelect.tsx.
In `@lib/helpers.ts`:
- Around line 210-223: The getCurrencySymbol function currently strips periods
by using formatted.replace(/[\d.,\s]/g, ""), which removes trailing dots like
the one in "Rp."; instead, change the extraction to use
Intl.NumberFormat.prototype.formatToParts(0) and return the part whose type ===
"currency" (falling back to currencyCode if missing) so you preserve punctuation
such as trailing periods and avoid brittle regex-based removal.
In `@lib/schemas.ts`:
- Around line 236-281: The returned schema from createPaymentInformationSchema
is inconsistent: when settings.cashPaymentMode.enabled is false it omits isCash
and change, which can lead to silent stripping of those keys; update the
false-branch schema to include isCash: z.boolean().optional() and change:
fieldValidators.nonNegativeNumber.optional() (matching the enabled-branch types)
while keeping bankName/accountName/accountNumber as fieldValidators.stringMin1
so the overall shape stays consistent across all SettingsType paths.
- Around line 283-357: The two nearly identical factories createSenderSchema and
createReceiverSchema should be collapsed into a single helper (e.g.,
createPartySchema) that takes (settings: SettingsType, party:
'sender'|'receiver') and uses a small getValidator helper to build the correct
settings.fieldRequirements key (e.g., `${party}${CapitalizedField}`) and return
either the base fieldValidators entry or makeOptional(base) as currently done;
replace createSenderSchema and createReceiverSchema with thin wrappers that call
createPartySchema, preserving use of fieldValidators, makeOptional and
CustomInputSchema.
- Around line 191-234: createItemSchema currently reassigns the Zod schema with
repeated schema.extend() calls which preserves the original inferred type and
degrades TypeScript type inference; instead build up a plain shape object (e.g.
let shape: Record<string, ZodTypeAny> or a typed object) with the base fields
(name, description, quantity, unitPrice, total), conditionally add
sku/discount/discountType/tax/taxType entries using settings.skuColumn,
settings.discountPerItem and settings.taxPerItem and the appropriate
fieldValidators, and finally return z.object(shape) so the final schema type is
inferred correctly from the single z.object(...) creation in createItemSchema.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b23ce219-dcc1-48c7-af3b-88abe5014046
📒 Files selected for processing (21)
app/components/index.tsapp/components/invoice/actions/PdfViewer.tsxapp/components/invoice/form/Charges.tsxapp/components/invoice/form/SingleItem.tsxapp/components/invoice/form/sections/ImportJsonButton.tsxapp/components/invoice/form/sections/Items.tsxapp/components/invoice/form/sections/PaymentInformation.tsxapp/components/layout/BaseNavbar.tsxapp/components/reusables/form-fields/FormInput.tsxapp/components/reusables/form-fields/FormSelect.tsxapp/components/settings/SettingsPanel.tsxapp/components/templates/invoice-pdf/InvoiceTemplate1.tsxapp/components/templates/invoice-pdf/InvoiceTemplate2.tsxcontexts/InvoiceContext.tsxcontexts/Providers.tsxcontexts/SettingsContext.tsxlib/helpers.tslib/schemas.tslib/variables.tsservices/invoice/client/importInvoice.tstypes.ts
…irement status, and add missing settings translation for id locale
Author
|
Fixed based on code review feedback. |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
app/components/layout/BaseNavbar.tsx (1)
42-49: Consider addingaria-labelfor better accessibility.The
titleattribute provides a tooltip butaria-labelis more reliable for screen readers.<Button variant="ghost" size="icon" onClick={() => setIsSettingsOpen(true)} title="Settings" + aria-label="Open settings" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/layout/BaseNavbar.tsx` around lines 42 - 49, Add an accessible name to the Settings button by adding an aria-label attribute to the Button component in the BaseNavbar component (the same element that uses onClick={() => setIsSettingsOpen(true)} and renders the <Settings /> icon); keep the existing title but add aria-label="Open settings" (or similar concise label) so screen readers get a reliable description for the Button element.contexts/ChargesContext.tsx (1)
159-169: MissingsetValuein dependency array.While
setValuefromreact-hook-formis typically stable, ESLint'sreact-hooks/exhaustive-depsrule will flag this. Consider adding it for completeness.}, [settings.discountPerItem.enabled, settings.taxPerItem.enabled]); + }, [settings.discountPerItem.enabled, settings.taxPerItem.enabled, setValue]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contexts/ChargesContext.tsx` around lines 159 - 169, The useEffect that resets switches uses setValue (and calls setDiscountSwitch/setTaxSwitch) but omits setValue from the dependency array; update the dependency array for the useEffect that references settings.discountPerItem.enabled and settings.taxPerItem.enabled to include setValue (and optionally include setDiscountSwitch and setTaxSwitch if lint requires) so the effect lists all referenced stable callbacks (i.e., include setValue in the dependencies for the useEffect that contains setDiscountSwitch, setValue("details.discountDetails.amount", 0), setTaxSwitch, and setValue("details.taxDetails.amount", 0)).lib/helpers.ts (1)
234-247: Add defensive null checks and consider stronger typing.The function assumes
settings.discountPerItemandsettings.taxPerItemexist. Ifsettingsis null/undefined or malformed, this will throw.const cleanInvoiceItemsForSettings = (items: any[], settings: any) => { + if (!settings) return items; return items.map((item) => { const cleanedItem = { ...item }; - if (!settings.discountPerItem.enabled) { + if (!settings.discountPerItem?.enabled) { cleanedItem.discount = undefined; cleanedItem.discountType = undefined; } - if (!settings.taxPerItem.enabled) { + if (!settings.taxPerItem?.enabled) { cleanedItem.tax = undefined; cleanedItem.taxType = undefined; } return cleanedItem; }); };Additionally, consider using
ItemType[]andSettingsTypeimports for stronger typing instead ofany.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/helpers.ts` around lines 234 - 247, cleanInvoiceItemsForSettings currently assumes settings and nested properties exist and uses any; add defensive null/undefined checks and stronger typing: change the signature to accept items: ItemType[] and settings: SettingsType (imported) and guard against missing settings or missing settings.discountPerItem/settings.taxPerItem by treating them as disabled when falsy (e.g., if (!settings?.discountPerItem?.enabled) { ... } and similarly for taxPerItem) before setting cleanedItem.discount/discountType and cleanedItem.tax/taxType to undefined; ensure cleanedItem is typed as Partial<ItemType> or ItemType to match imports.lib/schemas.ts (2)
118-139: Static schemas are now more permissive than settings-driven factories.
ItemSchemaandPaymentInformationSchemanow have all extended fields as optional, whilecreateItemSchemaandcreatePaymentInformationSchemaconditionally require them based on settings. Ensure callers use the appropriate schema: static for loose validation (e.g., imports), factories for form validation.Consider adding a clarifying comment above these schemas indicating their purpose:
+// Static schema with all fields optional - used for loose validation (imports, migrations) const ItemSchema = z.object({ name: fieldValidators.stringMin1,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/schemas.ts` around lines 118 - 139, ItemSchema and PaymentInformationSchema are intentionally permissive (many fields optional) while createItemSchema and createPaymentInformationSchema enforce settings-driven required fields; add a short clarifying comment above ItemSchema and PaymentInformationSchema stating that these are loose/static schemas for broad/import-level validation and that callers needing strict, settings-dependent validation must use createItemSchema and createPaymentInformationSchema instead so consumers know which schema to use.
276-295: Dynamic party schema handles missing field requirements gracefully.If a key doesn't exist in
settings.fieldRequirements, the comparison=== "required"returnsfalse, defaulting to optional. This is a safe fallback but consider adding type safety to catch typos at compile time.-const getValidator = (field: string, baseValidator: z.ZodTypeAny) => { - const key = `${party}${field.charAt(0).toUpperCase() + field.slice(1)}` as keyof typeof settings.fieldRequirements; +const getValidator = (field: "address" | "zipCode" | "city" | "country" | "email" | "phone", baseValidator: z.ZodTypeAny) => { + const key = `${party}${field.charAt(0).toUpperCase() + field.slice(1)}` as keyof typeof settings.fieldRequirements;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/schemas.ts` around lines 276 - 295, The createPartySchema/getValidator currently builds keys with string concatenation and indexes settings.fieldRequirements which can silently miss typos; to fix, make the key construction type-safe by defining a template literal union for the allowed keys (e.g., type PartyFieldKey = `${"sender" | "receiver"}${Capitalize<"name" | "address" | "zipCode" | "city" | "country" | "email" | "phone">}`) and use that as the index type (or cast the computed key to keyof SettingsType["fieldRequirements"] only after ensuring it matches the union), update SettingsType.fieldRequirements to use that union for its keys, and optionally add a runtime assert (e.g., if (!(key in settings.fieldRequirements)) throw) before accessing settings.fieldRequirements in getValidator; refer to createPartySchema and getValidator to locate where to apply these changes.contexts/InvoiceContext.tsx (1)
62-63: Move import statement to top of file with other imports.The
useSettingsimport is placed after the context definition. While this works due to import hoisting, it breaks the standard convention of grouping all imports at the file's top for better readability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contexts/InvoiceContext.tsx` around lines 62 - 63, Move the useSettings import to the top of the file with the other imports so all module imports are grouped consistently; locate the existing import statement "import { useSettings } from \"./SettingsContext\";" (currently below the InvoiceContext definition) and cut/paste it into the file's import block at the top next to the other imports to restore conventional ordering without changing the symbol name or its usages in the InvoiceContext component.contexts/SettingsContext.tsx (1)
70-75:updateSettingsperforms shallow merge—nested objects will be fully replaced.If a caller passes a partial nested object (e.g.,
updateSettings({ fieldRequirements: { senderEmail: "optional" } })), it will completely overwritefieldRequirements, losing all other keys. Consider deep-merging here as well, or document that callers must pass complete nested objects.♻️ Proposed fix to deep-merge nested objects
const updateSettings = (partial: Partial<SettingsType>) => { setSettings((prev) => ({ ...prev, ...partial, + fieldRequirements: { + ...prev.fieldRequirements, + ...(partial.fieldRequirements ?? {}), + }, + discountPerItem: { + ...prev.discountPerItem, + ...(partial.discountPerItem ?? {}), + }, + taxPerItem: { + ...prev.taxPerItem, + ...(partial.taxPerItem ?? {}), + }, + skuColumn: { + ...prev.skuColumn, + ...(partial.skuColumn ?? {}), + }, + cashPaymentMode: { + ...prev.cashPaymentMode, + ...(partial.cashPaymentMode ?? {}), + }, })); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contexts/SettingsContext.tsx` around lines 70 - 75, The updateSettings function currently does a shallow merge (setSettings in SettingsContext.tsx) so passing a partial nested object (e.g., updateSettings({ fieldRequirements: { senderEmail: "optional" } })) overwrites the entire nested object and loses other keys; change updateSettings to perform a deep merge of prev and partial—either by using a safe deep-merge utility (e.g., lodash.merge or a small mergeDeep helper) or implement a recursive merge that merges nested plain objects while replacing non-objects—ensure you reference updateSettings, setSettings, and SettingsType (and preserve existing nested keys like fieldRequirements) so callers can provide partial nested updates without losing other settings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/invoice/form/sections/ImportJsonButton.tsx`:
- Around line 32-34: Wrap the await importInvoice(file) call in a try-catch
inside ImportJsonButton so errors from parseImportedFile/importInvoice are
caught; move setOpen(false) into the try so the dialog only closes on success,
and in the catch show user-visible feedback (using the app's
toast/alert/notification or setError state) and log the error for debugging;
reference importInvoice, setOpen, supportedFormats and the ImportJsonButton
handler to locate where to add the try-catch.
In `@app/components/invoice/form/SingleItem.tsx`:
- Around line 116-124: SingleItem.tsx computes percentage tax on the
post-discount amount (amountAfterDiscount * (Number(tax) / 100)) which
mismatches InvoiceTemplate1.tsx/InvoiceTemplate2.tsx that compute tax on the
gross amount (unitPrice * quantity * tax / 100); update SingleItem.tsx so that
when settings.taxPerItem.enabled and taxType === "percentage" you compute
taxValue from the gross base amount (use baseAmount * (Number(tax) / 100) or
equivalent using unitPrice and quantity) instead of amountAfterDiscount, keeping
numeric validation (tax !== undefined and !isNaN(Number(tax))).
In `@contexts/InvoiceContext.tsx`:
- Around line 390-414: In importInvoice, remove the misleading cast "as unknown
as string" and ensure the parsed dates are validated before calling
reset(importedData): for importedData.details.invoiceDate and .dueDate, create
Date objects (new Date(...)), check validity (e.g., isNaN(date.getTime()) or
Date.prototype.toString() !== "Invalid Date"), and only assign the Date object
back when valid; if invalid or missing, set those fields to undefined/null so
the form's validators handle them correctly; then call reset(importedData) as
before. Use the importInvoice and parseImportedFile identifiers to locate the
code to change.
---
Nitpick comments:
In `@app/components/layout/BaseNavbar.tsx`:
- Around line 42-49: Add an accessible name to the Settings button by adding an
aria-label attribute to the Button component in the BaseNavbar component (the
same element that uses onClick={() => setIsSettingsOpen(true)} and renders the
<Settings /> icon); keep the existing title but add aria-label="Open settings"
(or similar concise label) so screen readers get a reliable description for the
Button element.
In `@contexts/ChargesContext.tsx`:
- Around line 159-169: The useEffect that resets switches uses setValue (and
calls setDiscountSwitch/setTaxSwitch) but omits setValue from the dependency
array; update the dependency array for the useEffect that references
settings.discountPerItem.enabled and settings.taxPerItem.enabled to include
setValue (and optionally include setDiscountSwitch and setTaxSwitch if lint
requires) so the effect lists all referenced stable callbacks (i.e., include
setValue in the dependencies for the useEffect that contains setDiscountSwitch,
setValue("details.discountDetails.amount", 0), setTaxSwitch, and
setValue("details.taxDetails.amount", 0)).
In `@contexts/InvoiceContext.tsx`:
- Around line 62-63: Move the useSettings import to the top of the file with the
other imports so all module imports are grouped consistently; locate the
existing import statement "import { useSettings } from \"./SettingsContext\";"
(currently below the InvoiceContext definition) and cut/paste it into the file's
import block at the top next to the other imports to restore conventional
ordering without changing the symbol name or its usages in the InvoiceContext
component.
In `@contexts/SettingsContext.tsx`:
- Around line 70-75: The updateSettings function currently does a shallow merge
(setSettings in SettingsContext.tsx) so passing a partial nested object (e.g.,
updateSettings({ fieldRequirements: { senderEmail: "optional" } })) overwrites
the entire nested object and loses other keys; change updateSettings to perform
a deep merge of prev and partial—either by using a safe deep-merge utility
(e.g., lodash.merge or a small mergeDeep helper) or implement a recursive merge
that merges nested plain objects while replacing non-objects—ensure you
reference updateSettings, setSettings, and SettingsType (and preserve existing
nested keys like fieldRequirements) so callers can provide partial nested
updates without losing other settings.
In `@lib/helpers.ts`:
- Around line 234-247: cleanInvoiceItemsForSettings currently assumes settings
and nested properties exist and uses any; add defensive null/undefined checks
and stronger typing: change the signature to accept items: ItemType[] and
settings: SettingsType (imported) and guard against missing settings or missing
settings.discountPerItem/settings.taxPerItem by treating them as disabled when
falsy (e.g., if (!settings?.discountPerItem?.enabled) { ... } and similarly for
taxPerItem) before setting cleanedItem.discount/discountType and
cleanedItem.tax/taxType to undefined; ensure cleanedItem is typed as
Partial<ItemType> or ItemType to match imports.
In `@lib/schemas.ts`:
- Around line 118-139: ItemSchema and PaymentInformationSchema are intentionally
permissive (many fields optional) while createItemSchema and
createPaymentInformationSchema enforce settings-driven required fields; add a
short clarifying comment above ItemSchema and PaymentInformationSchema stating
that these are loose/static schemas for broad/import-level validation and that
callers needing strict, settings-dependent validation must use createItemSchema
and createPaymentInformationSchema instead so consumers know which schema to
use.
- Around line 276-295: The createPartySchema/getValidator currently builds keys
with string concatenation and indexes settings.fieldRequirements which can
silently miss typos; to fix, make the key construction type-safe by defining a
template literal union for the allowed keys (e.g., type PartyFieldKey =
`${"sender" | "receiver"}${Capitalize<"name" | "address" | "zipCode" | "city" |
"country" | "email" | "phone">}`) and use that as the index type (or cast the
computed key to keyof SettingsType["fieldRequirements"] only after ensuring it
matches the union), update SettingsType.fieldRequirements to use that union for
its keys, and optionally add a runtime assert (e.g., if (!(key in
settings.fieldRequirements)) throw) before accessing settings.fieldRequirements
in getValidator; refer to createPartySchema and getValidator to locate where to
apply these changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0d223f39-6f76-488a-80ce-9611f3c037a3
📒 Files selected for processing (21)
app/components/index.tsapp/components/invoice/actions/PdfViewer.tsxapp/components/invoice/form/SingleItem.tsxapp/components/invoice/form/sections/ImportJsonButton.tsxapp/components/invoice/form/sections/Items.tsxapp/components/invoice/form/sections/PaymentInformation.tsxapp/components/layout/BaseNavbar.tsxapp/components/modals/invoice/InvoiceLoaderModal.tsxapp/components/reusables/form-fields/FormSelect.tsxapp/components/settings/SettingsPanel.tsxapp/components/templates/invoice-pdf/InvoiceTemplate1.tsxapp/components/templates/invoice-pdf/InvoiceTemplate2.tsxcontexts/ChargesContext.tsxcontexts/InvoiceContext.tsxcontexts/SettingsContext.tsxi18n/locales/en.jsoni18n/locales/id.jsonlib/helpers.tslib/schemas.tsservices/invoice/client/importInvoice.tstypes.ts
✅ Files skipped from review due to trivial changes (3)
- i18n/locales/en.json
- app/components/invoice/form/sections/Items.tsx
- i18n/locales/id.json
🚧 Files skipped from review as they are similar to previous changes (7)
- app/components/invoice/actions/PdfViewer.tsx
- app/components/invoice/form/sections/PaymentInformation.tsx
- app/components/settings/SettingsPanel.tsx
- app/components/reusables/form-fields/FormSelect.tsx
- types.ts
- app/components/templates/invoice-pdf/InvoiceTemplate2.tsx
- services/invoice/client/importInvoice.ts
Comment on lines
+32
to
+34
| if (supportedFormats.includes(ext)) { | ||
| await importInvoice(file); | ||
| setOpen(false); |
There was a problem hiding this comment.
Wrap importInvoice call in try-catch for error feedback.
Per the context snippets, parseImportedFile can throw various errors (malformed JSON, empty XLSX, etc.). If importInvoice propagates these errors, the user receives no feedback since there's no try-catch here.
if (supportedFormats.includes(ext)) {
- await importInvoice(file);
- setOpen(false);
+ try {
+ await importInvoice(file);
+ setOpen(false);
+ } catch (error) {
+ importInvoiceError();
+ }
} else {
importInvoiceError();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (supportedFormats.includes(ext)) { | |
| await importInvoice(file); | |
| setOpen(false); | |
| if (supportedFormats.includes(ext)) { | |
| try { | |
| await importInvoice(file); | |
| setOpen(false); | |
| } catch (error) { | |
| importInvoiceError(); | |
| } | |
| } else { | |
| importInvoiceError(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/invoice/form/sections/ImportJsonButton.tsx` around lines 32 -
34, Wrap the await importInvoice(file) call in a try-catch inside
ImportJsonButton so errors from parseImportedFile/importInvoice are caught; move
setOpen(false) into the try so the dialog only closes on success, and in the
catch show user-visible feedback (using the app's toast/alert/notification or
setError state) and log the error for debugging; reference importInvoice,
setOpen, supportedFormats and the ImportJsonButton handler to locate where to
add the try-catch.
Comment on lines
+116
to
+124
| // Calculate tax if enabled - applied on post-discount amount if percentage | ||
| if (settings.taxPerItem.enabled && tax !== undefined && !isNaN(Number(tax))) { | ||
| const amountAfterDiscount = baseAmount - discountValue; | ||
| if (taxType === "percentage") { | ||
| taxValue = amountAfterDiscount * (Number(tax) / 100); | ||
| } else { | ||
| taxValue = Number(tax); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the tax calculation in InvoiceTemplate1.tsx
rg -n -A2 'taxAmount.*taxType.*percentage' app/components/templates/invoice-pdf/Repository: al1abb/invoify
Length of output: 809
🏁 Script executed:
#!/bin/bash
# Find baseAmount definition and usage in SingleItem.tsx
rg -n 'baseAmount' app/components/invoice/form/SingleItem.tsx | head -20Repository: al1abb/invoify
Length of output: 372
🏁 Script executed:
#!/bin/bash
# Get more context around the baseAmount and tax calculation in SingleItem.tsx
sed -n '60,130p' app/components/invoice/form/SingleItem.tsxRepository: al1abb/invoify
Length of output: 2280
Tax calculation inconsistency between form and PDF templates.
SingleItem.tsx computes percentage tax on the post-discount amount (line 118–120: amountAfterDiscount * (Number(tax) / 100)), but both InvoiceTemplate1.tsx and InvoiceTemplate2.tsx compute it on the gross amount (unitPrice * quantity * tax / 100). This causes the line item total shown in the form to differ from what appears in the PDF.
Align the calculation approach across both form and templates—either both should apply tax on the gross amount or both on the post-discount amount, based on your business requirements.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/invoice/form/SingleItem.tsx` around lines 116 - 124,
SingleItem.tsx computes percentage tax on the post-discount amount
(amountAfterDiscount * (Number(tax) / 100)) which mismatches
InvoiceTemplate1.tsx/InvoiceTemplate2.tsx that compute tax on the gross amount
(unitPrice * quantity * tax / 100); update SingleItem.tsx so that when
settings.taxPerItem.enabled and taxType === "percentage" you compute taxValue
from the gross base amount (use baseAmount * (Number(tax) / 100) or equivalent
using unitPrice and quantity) instead of amountAfterDiscount, keeping numeric
validation (tax !== undefined and !isNaN(Number(tax))).
Comment on lines
+390
to
414
| const importInvoice = async (file: File) => { | ||
| try { | ||
| const importedData = await parseImportedFile(file); | ||
|
|
||
| // Parse the dates | ||
| if (importedData.details) { | ||
| if (importedData.details.invoiceDate) { | ||
| importedData.details.invoiceDate = new Date( | ||
| importedData.details.invoiceDate | ||
| ) as unknown as string; | ||
| } | ||
| if (importedData.details.dueDate) { | ||
| importedData.details.dueDate = new Date( | ||
| importedData.details.dueDate | ||
| ) as unknown as string; | ||
| } | ||
|
|
||
| // Reset form with imported data | ||
| reset(importedData); | ||
| } catch (error) { | ||
| console.error("Error parsing JSON file:", error); | ||
| importInvoiceError(); | ||
| } | ||
| }; | ||
| reader.readAsText(file); | ||
|
|
||
| // Reset form with imported data | ||
| reset(importedData); | ||
| } catch (error) { | ||
| console.error("Error importing file:", error); | ||
| importInvoiceError(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check the InvoiceType and details typing for date fields
# Search for InvoiceType definition
ast-grep --pattern 'type InvoiceType = $_'
# Search for invoiceDate field typing
rg -n -C3 'invoiceDate' --type=ts -g '!node_modules/**'Repository: al1abb/invoify
Length of output: 7113
🏁 Script executed:
#!/bin/bash
# Find fieldValidators definition and its date validator
rg -n 'fieldValidators\s*=' lib/ --type ts -A 20 | head -100
# Also search for date validator definition
rg -n 'date\s*:' lib/schemas.ts -B 2 -A 2Repository: al1abb/invoify
Length of output: 1378
🏁 Script executed:
#!/bin/bash
# Find the full InvoiceSchema definition to see date field types
rg -n 'const InvoiceSchema' lib/schemas.ts -A 50Repository: al1abb/invoify
Length of output: 2210
🏁 Script executed:
#!/bin/bash
# Check if fieldValidators.date is a Zod date validator
rg -n 'z\.date\(\)' lib/ --type tsRepository: al1abb/invoify
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Read the complete fieldValidators.date definition
sed -n '48,65p' lib/schemas.tsRepository: al1abb/invoify
Length of output: 630
🏁 Script executed:
#!/bin/bash
# Check how Invalid Date behaves with Zod's z.date()
python3 << 'EOF'
# Simulate Zod's z.date() behavior with Invalid Date
import datetime
# In JavaScript, new Date("invalid") returns Invalid Date
# Zod's z.date() checks if it's a Date instance and if getTime() returns NaN
print("Invalid Date behavior:")
print("new Date('invalid').getTime() returns: NaN")
print("Zod z.date() checks: isNaN(getTime()) and throws if true")
print("\nSo Invalid Date objects ARE rejected by Zod validation")
EOFRepository: al1abb/invoify
Length of output: 236
🏁 Script executed:
#!/bin/bash
# Search for date validation error handling or tests
rg -n 'Invalid.*[Dd]ate|date.*[Ii]nvalid' --type ts -g '!node_modules/**'Repository: al1abb/invoify
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Check how parseImportedFile handles dates - what format does it return?
rg -n 'parseImportedFile' contexts/InvoiceContext.tsx -B 5 -A 15Repository: al1abb/invoify
Length of output: 1358
🏁 Script executed:
#!/bin/bash
# Find the parseImportedFile function definition
rg -n 'export.*parseImportedFile|const parseImportedFile|function parseImportedFile' --type ts -A 30Repository: al1abb/invoify
Length of output: 2102
🏁 Script executed:
#!/bin/bash
# Also search for importInvoice service file
fd -n 'importInvoice' --type tsRepository: al1abb/invoify
Length of output: 288
🏁 Script executed:
#!/bin/bash
# Find the individual parse functions and what they return for dates
rg -n 'parseCSVFile|parseXMLFile|parseXLSXFile|parseJsonFile' services/invoice/client/importInvoice.ts -A 5 | head -80Repository: al1abb/invoify
Length of output: 1811
🏁 Script executed:
#!/bin/bash
# Check what unflattenObject does (mentioned in review as logic that creates invalid dates)
rg -n 'unflattenObject' services/invoice/client/importInvoice.ts -B 3 -A 20 | head -100Repository: al1abb/invoify
Length of output: 2605
🏁 Script executed:
#!/bin/bash
# Check how the imported data is used after importInvoice - does it go through reset() with validation?
rg -n 'reset\(' contexts/InvoiceContext.tsx -B 2 -A 2Repository: al1abb/invoify
Length of output: 341
🏁 Script executed:
#!/bin/bash
# Check what reset() does and if it validates through the schema
rg -n 'const.*reset.*=|function reset' contexts/InvoiceContext.tsx -B 2 -A 10 | head -60Repository: al1abb/invoify
Length of output: 522
🏁 Script executed:
#!/bin/bash
# Check how the form is set up with Zod resolver and validation mode
rg -n 'useForm\|zodResolver\|mode:' contexts/InvoiceContext.tsx -B 2 -A 5Repository: al1abb/invoify
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Look at the full importInvoice function to understand the complete flow
sed -n '390,420p' contexts/InvoiceContext.tsxRepository: al1abb/invoify
Length of output: 897
🏁 Script executed:
#!/bin/bash
# Check if there's a FormProvider setup that shows validation mode
rg -n 'FormProvider\|useFormContext' contexts/ --type ts -A 5 | head -80Repository: al1abb/invoify
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Search for the actual form setup in InvoiceContext to see if validation is triggered on reset
rg -n 'useForm\|resolver' contexts/InvoiceContext.tsx -B 3 -A 8Repository: al1abb/invoify
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Search more broadly for form setup and resolver
rg -n 'FormProvider' --type ts -B 3 -A 10Repository: al1abb/invoify
Length of output: 2681
🏁 Script executed:
#!/bin/bash
# Find where InvoiceContext is used and how the form is set up
rg -n 'useFormContext\|InvoiceSchema' --type ts -B 2 -A 5 | head -150Repository: al1abb/invoify
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Get the form initialization code in Providers.tsx
sed -n '1,80p' contexts/Providers.tsx | grep -A 30 'useForm'Repository: al1abb/invoify
Length of output: 1610
🏁 Script executed:
#!/bin/bash
# Also check what createInvoiceSchema does
rg -n 'createInvoiceSchema' lib/schemas.ts -A 20 | head -60Repository: al1abb/invoify
Length of output: 515
🏁 Script executed:
#!/bin/bash
# Check if there are validation mode settings or shouldValidate options in reset calls
rg -n 'reset\(' contexts/ --type ts -A 3Repository: al1abb/invoify
Length of output: 771
🏁 Script executed:
#!/bin/bash
# Verify what happens when Invalid Date is passed to Zod's z.date()
# Check if there's any test or documentation
rg -n 'z\.date\|Invalid.*[Dd]ate' lib/ --type ts -B 2 -A 2Repository: al1abb/invoify
Length of output: 40
Remove the incorrect type cast and add date validation before form reset.
The cast as unknown as string is misleading—the schema's fieldValidators.date expects a Date object as input (which it then transforms to a localized string). The code correctly creates a Date object from the imported data, but the type cast contradicts this.
Additionally, when imported dates are undefined or invalid strings (common with CSV/XML/XLSX imports), new Date() creates an Invalid Date object without throwing. While Zod will reject this on form submission, passing an Invalid Date through form state creates poor UX.
Suggested fix: validate date conversion before reset
// Parse the dates
if (importedData.details) {
if (importedData.details.invoiceDate) {
- importedData.details.invoiceDate = new Date(
- importedData.details.invoiceDate
- ) as unknown as string;
+ const invoiceDate = new Date(importedData.details.invoiceDate);
+ if (isNaN(invoiceDate.getTime())) {
+ throw new Error("Invalid invoice date format");
+ }
+ importedData.details.invoiceDate = invoiceDate;
}
if (importedData.details.dueDate) {
- importedData.details.dueDate = new Date(
- importedData.details.dueDate
- ) as unknown as string;
+ const dueDate = new Date(importedData.details.dueDate);
+ if (isNaN(dueDate.getTime())) {
+ throw new Error("Invalid due date format");
+ }
+ importedData.details.dueDate = dueDate;
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contexts/InvoiceContext.tsx` around lines 390 - 414, In importInvoice, remove
the misleading cast "as unknown as string" and ensure the parsed dates are
validated before calling reset(importedData): for
importedData.details.invoiceDate and .dueDate, create Date objects (new
Date(...)), check validity (e.g., isNaN(date.getTime()) or
Date.prototype.toString() !== "Invalid Date"), and only assign the Date object
back when valid; if invalid or missing, set those fields to undefined/null so
the form's validators handle them correctly; then call reset(importedData) as
before. Use the importInvoice and parseImportedFile identifiers to locate the
code to change.
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
SettingsContextwith localStorage persistence, dynamic Zod schema factory, and a Settings panel (Sheet/Drawer) triggered by a gear icon in the navbaridlocale in the LOCALES array so it appears in the language selectorDevDebugpanel that was showing a red-bordered debug overlay in developmentChanges
New Files
contexts/SettingsContext.tsx— settings state with localStorage persistenceapp/components/settings/SettingsPanel.tsx— Sheet/Drawer UI for all settingsapp/components/reusables/form-fields/FormSelect.tsx— reusable select field componentservices/invoice/client/importInvoice.ts— unified import dispatcher for JSON/CSV/XML/XLSModified Files
types.ts—SettingsType,FieldRequirementSetting, new item fields (sku, discount, tax, isCash, change)lib/variables.ts—DEFAULT_SETTINGS,LOCAL_STORAGE_SETTINGS_KEY, Bahasa Indonesia inLOCALESlib/schemas.ts—createInvoiceSchema(settings)dynamic factory functionlib/helpers.ts—getCurrencySymbol()helper usingIntl.NumberFormatcontexts/Providers.tsx— addedSettingsContextProviderto provider hierarchycontexts/InvoiceContext.tsx—migrateInvoice()for legacy localStorage data, updatedimportInvoice()app/components/layout/BaseNavbar.tsx— gear icon + settings trigger, DevDebug commented outapp/components/invoice/form/SingleItem.tsx— SKU, discount, tax columns (settings-gated)app/components/invoice/form/sections/Items.tsx— column headers for new fieldsapp/components/invoice/form/sections/PaymentInformation.tsx— cash payment mode UIapp/components/invoice/form/sections/ImportJsonButton.tsx— refactored to multi-format importapp/components/invoice/actions/PdfViewer.tsx— updated for new schemaapp/components/invoice/form/Charges.tsx— updated totals calculationapp/components/templates/invoice-pdf/InvoiceTemplate1.tsx— new columns, cash payment, currency symbolapp/components/templates/invoice-pdf/InvoiceTemplate2.tsx— new columns, cash payment, currency symbolapp/components/reusables/form-fields/FormInput.tsx— minor updatesapp/components/index.ts— export new componentsTest Plan
🤖 Generated with Claude Code
Summary by CodeRabbit