fix: bugs #126
fix: bugs #126
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds a centralized, retryable network-switching hook and retry utility; applies network switching across payment flows; normalizes subscription payee addresses to checksummed form; unifies invoice mutation to async mutate and currentUser-aware navigation; updates InvoiceForm onSubmit signature and submit-button state; bumps a Radix dependency. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Payment Form
participant Hook as useSwitchNetwork
participant Wallet
participant API as Backend
rect rgb(245,250,255)
User->>UI: Submit payment (includes paymentCurrency)
UI->>Hook: switchToPaymentNetwork(paymentCurrency)
Hook->>Wallet: switchWithRetry(targetChain)
end
alt switch success
Wallet-->>UI: switched
UI->>API: proceed with payment request
API-->>UI: response
UI-->>User: toast/result
else switch failure
Wallet-->>UI: error
UI-->>User: error toast
end
sequenceDiagram
participant User
participant UI as InvoiceCreator
participant API as api.invoice.create (mutateAsync)
participant Cache as utils.invoice.getAll
User->>UI: Submit invoice form
UI->>API: await createInvoice(data)
alt success
UI->>Cache: invalidate
UI-->>User: success toast
UI->>UI: navigate to /dashboard (if applicable)
else failure
UI-->>User: error toast
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/components/subscription-plans/blocks/create-subscription-plan.tsx (1)
86-90: Avoid overwriting user input and guard against unexpected address normalization errorsIf the user edits the payee field and the wallet address changes, this effect will overwrite their manual input. Also, while unlikely, getAddress can throw if fed an unexpected value. Consider only setting when payee is empty and add a small safeguard.
Apply this diff:
- useEffect(() => { - if (address) { - form.setValue("payee", getAddress(address)); - } - }, [address, form]); + useEffect(() => { + if (address && !form.getValues("payee")) { + try { + form.setValue("payee", getAddress(address), { shouldValidate: true }); + } catch { + toast.error("Failed to normalize connected address"); + } + } + }, [address, form]);src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsx (1)
98-106: Confirm product intent: Recurring payments were previously Sepolia-onlyPer prior preference captured in learnings, recurring payments were deliberately limited to Sepolia. This PR makes the target chain dynamic based on invoiceCurrency. Please confirm whether this change is desired for recurring flows; otherwise, we should hardcode Sepolia or validate the selection accordingly.
src/components/invoice-creator.tsx (3)
75-86: Awaiting mutate() is a no-op; simplify onSubmit or switch to mutateAsync
createInvoicehere ismutate(void-returning).await createInvoice(data)won't wait for the server call, and thetry/catchwon’t catch server errors (they're handled byonError). Either:
- Minimal change: keep
mutateand dropasync/awaitand thetry/catch, or- If you need to await and handle errors here, switch to
mutateAsyncin the hook and keeptry/catch.Minimal-change diff (recommended):
- const onSubmit = async (data: InvoiceFormValues) => { - try { - await createInvoice(data); - } catch (error) { - toast.error("Failed to create invoice", { - description: - error instanceof Error - ? error.message - : "An unexpected error occurred", - }); - } - }; + const onSubmit = (data: InvoiceFormValues) => { + createInvoice(data); + };
46-53: Avoid surfacing raw error messages to end-users
error.messagecan be technical. Consider logging the raw error for observability and showing a stable, friendly message in the toast description to avoid leaking internals.- onError: (error) => { - toast.error("Failed to create invoice", { - description: - error instanceof Error - ? error.message - : "An unexpected error occurred", - }); - }, + onError: (error) => { + console.error("invoice.create failed:", error); + toast.error("Failed to create invoice", { + description: "Please try again. If the problem persists, contact support.", + }); + },
96-104: Avoid force-casting to User; pass a narrower type or adjust InvoiceForm’s propsThe cast to
User(including theunknown as Userfallback) is a code smell and can mask type mismatches. Prefer narrowing the prop type consumed byInvoiceFormto exactly what it needs (e.g.,Pick<User, "id" | "name" | "email">) and make it optional, so you don’t need unsafe casts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
package.json(1 hunks)src/components/batch-payout.tsx(4 hunks)src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsx(4 hunks)src/components/direct-payout.tsx(4 hunks)src/components/invoice-creator.tsx(1 hunks)src/components/subscription-plans/blocks/create-subscription-plan.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-06-04T10:08:40.123Z
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#64
File: src/components/batch-payout.tsx:100-106
Timestamp: 2025-06-04T10:08:40.123Z
Learning: In src/components/batch-payout.tsx, the user prefers to keep the simple 2-second timeout for AppKit initialization over more complex polling mechanisms when the current approach is working adequately. They favor simplicity over potentially more robust but complex solutions.
Applied to files:
src/components/batch-payout.tsx
📚 Learning: 2025-07-11T11:17:32.659Z
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#82
File: src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsx:131-131
Timestamp: 2025-07-11T11:17:32.659Z
Learning: In the RequestNetwork/easy-invoice codebase, the user aimensahnoun prefers to keep recurring payment functionality limited to Sepolia only. Hardcoding the chain to "sepolia" in recurring payment components is intentional and acceptable, rather than making it dynamic based on the connected network.
Applied to files:
src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsxsrc/components/direct-payout.tsx
📚 Learning: 2025-08-08T09:52:43.700Z
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#117
File: src/app/not-found.tsx:29-67
Timestamp: 2025-08-08T09:52:43.700Z
Learning: Repository: RequestNetwork/easy-invoice
Context: src/app/not-found.tsx (Next.js, TypeScript/React)
Learning: It is acceptable (by current product decision) for the NotFound page to display links to /invoices/create and /subscription-plans even though these routes are auth-gated. Rationale: prevent the 404 from feeling empty; most users are authenticated. No lock icons or conditional rendering required for now; can be revisited later as an enhancement.
Applied to files:
src/components/invoice-creator.tsx
🧬 Code Graph Analysis (4)
src/components/batch-payout.tsx (1)
src/lib/constants/chains.ts (2)
NETWORK_TO_ID(19-27)ID_TO_APPKIT_NETWORK(29-36)
src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsx (1)
src/lib/constants/chains.ts (2)
NETWORK_TO_ID(19-27)ID_TO_APPKIT_NETWORK(29-36)
src/components/invoice-creator.tsx (2)
src/trpc/react.tsx (1)
api(12-12)src/trpc/server.ts (1)
api(31-61)
src/components/direct-payout.tsx (1)
src/lib/constants/chains.ts (2)
NETWORK_TO_ID(19-27)ID_TO_APPKIT_NETWORK(29-36)
🔇 Additional comments (11)
package.json (1)
34-34: Patch bump to @radix-ui/react-tooltip looks safeMinor version bump within the same major range; low risk and no downstream changes in this PR depend on it.
src/components/subscription-plans/blocks/create-subscription-plan.tsx (1)
40-40: Using viem.getAddress to checksum the address is correctThis aligns with EIP-55 and the stated PR objective to pre-populate a checksummed address.
src/components/direct-payout.tsx (2)
7-7: Importing useAppKitNetwork is appropriate hereEnables safe pre-submit chain checks/switching. Usage downstream is consistent.
42-42: Chain-ID mappings import is correct and consistent with other componentsMatches the approach used elsewhere in this PR.
src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsx (2)
23-23: Importing chain mappings is fineConsistent with how other payout components derive and switch networks.
56-56: useAppKitNetwork usage LGTMCorrectly retrieving chainId and switchNetwork for pre-submit checks.
src/components/batch-payout.tsx (3)
7-7: Importing useAppKitNetwork for pre-submit checks is appropriatePrepares this component for chain-aware batch execution.
53-53: Chain mappings import is consistent with other componentsNo issues.
79-79: Hook usage LGTMCorrectly pulls chainId and switchNetwork for the submit path.
src/components/invoice-creator.tsx (2)
40-54: Consolidation to a single invoice.create mutation looks goodSwitching to a unified mutation and handling success (toast, invalidate, redirect) within the mutation callback is clean and reduces branching complexity. The invalidate-then-redirect order is sensible.
119-121: Confirmed:paymentDetailsIdis declared and optional in the form schemaThe Zod schema in
src/lib/schemas/invoice.tsdefinespaymentDetailsId: z.string().optional()so
InvoiceFormValuesincludespaymentDetailsId?: string.
form.watch("paymentDetailsId")correctly returnsstring | undefinedand matchesInvoicePreviewProps.paymentDetailsId. No further action needed.
src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsx
Outdated
Show resolved
Hide resolved
src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsx
Outdated
Show resolved
Hide resolved
src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
src/components/payment-section.tsx (1)
331-340: Await the network switch and handle failures to avoid race conditions and bad UX.
switchToChainIdreturns a Promise (via the retry wrapper). Not awaiting it means subsequent provider/signer operations may run on the wrong chain or race the switch. Also, errors during switching won’t be surfaced.Apply this diff:
- switchToChainId(targetChain); + try { + await switchToChainId(targetChain); + } catch (err) { + console.error("Network switch failed:", err); + toast("Network switch failed", { + description: + (err as Error)?.message ?? + "We couldn't switch your wallet to the required network.", + }); + setPaymentProgress("idle"); + return; + }Additional note:
- Consider guarding
selectedRoute: if it’s null (e.g., brief race before selection),targetChaincould beundefined, andtargetAppkitNetwork.nameaccess will throw. A simple early return with a toast would help.
🧹 Nitpick comments (3)
src/lib/utils.ts (2)
42-46: Clarify semantics of "retries" (it currently means max attempts).The loop below runs at most
retriesattempts total. The inline comment implies "number of retries," which often means "retries after the initial attempt." Please clarify to avoid misuse.Apply this diff to make the intent explicit:
export interface RetryConfig { - retries?: number; // number of retries (default: 3) + retries?: number; // max attempts including the first try (default: 3) delay?: number; // delay between retries in ms (default: 1000) backoff?: boolean; // exponential backoff (default: false) }
56-68: Guard against 0 (or negative) retries and add an optional non-retryable error predicate.
- If
retriesis 0, the loop never runs,lastErrorstays undefined, and you end upthrowingundefined.- For wallet UX, we typically should not retry on user-rejected requests (EIP-1193
code === 4001). AshouldRetrypredicate lets callers short-circuit on non-retryable errors.Apply this diff:
export async function retry<T>( fn: () => Promise<T>, options: RetryOptions<T> = {}, ): Promise<T> { const { retries = 3, delay = 1000, backoff = false, onError, onSuccess, onRetry, + // Optional predicate to decide if we should continue retrying + // Returning false will stop retrying immediately. + // Useful to avoid retrying on user-rejected wallet requests (e.g., code 4001). + // Not provided => always retry (until attempts are exhausted). + shouldRetry, } = options; + // Ensure at least one attempt + const maxAttempts = Math.max(1, retries); let lastError: unknown; - for (let attempt = 1; attempt <= retries; attempt++) { + for (let attempt = 1; attempt <= maxAttempts; attempt++) { try { const result = await fn(); if (onSuccess) await onSuccess(result, attempt); return result; } catch (err) { lastError = err; - if (attempt < retries) { + if (attempt < maxAttempts) { + if (shouldRetry) { + const retryable = await Promise.resolve(shouldRetry(err, attempt)); + if (!retryable) break; + } if (onRetry) await onRetry(attempt); const waitTime = backoff ? delay * 2 ** (attempt - 1) : delay; await new Promise((res) => setTimeout(res, waitTime)); } } } if (onError) await onError(lastError); throw lastError; }And extend the options type:
export interface RetryConfig { retries?: number; // max attempts including the first try (default: 3) delay?: number; // delay between retries in ms (default: 1000) backoff?: boolean; // exponential backoff (default: false) + shouldRetry?: (error: unknown, attempt: number) => boolean | Promise<boolean>; } export interface RetryHooks<T> { onError?: (error: unknown) => void | Promise<void>; onSuccess?: (result: T, attempt: number) => void | Promise<void>; onRetry?: (attempt: number) => void | Promise<void>; } export type RetryOptions<T> = RetryConfig & RetryHooks<T>;Also applies to: 71-85
src/components/dashboard/invoices-received.tsx (1)
76-76: Also grab chainId to avoid unnecessary switches.Without checking, you may prompt redundant wallet switches even when already on the correct network.
Apply this diff:
- const { switchToChainId } = useSwitchNetwork(); + const { switchToChainId, chainId } = useSwitchNetwork();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
src/components/batch-payout.tsx(3 hunks)src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsx(3 hunks)src/components/dashboard/invoices-received.tsx(3 hunks)src/components/direct-payout.tsx(3 hunks)src/components/payment-section.tsx(3 hunks)src/lib/hooks/use-switch-network.ts(1 hunks)src/lib/utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/direct-payout.tsx
- src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsx
- src/components/batch-payout.tsx
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/components/payment-section.tsx (1)
src/lib/hooks/use-switch-network.ts (1)
useSwitchNetwork(7-37)
src/lib/hooks/use-switch-network.ts (3)
src/lib/utils.ts (2)
RetryHooks(48-52)retry(56-90)src/lib/helpers/chain.ts (1)
getChainFromPaymentCurrency(3-10)src/lib/constants/chains.ts (1)
ID_TO_APPKIT_NETWORK(29-36)
src/components/dashboard/invoices-received.tsx (2)
src/lib/hooks/use-switch-network.ts (1)
useSwitchNetwork(7-37)src/lib/constants/chains.ts (1)
NETWORK_TO_ID(19-27)
🔇 Additional comments (3)
src/components/payment-section.tsx (2)
24-24: Good move to centralize network switching via a hook.Shifting to
useSwitchNetworkhelps standardize behavior and makes future improvements (e.g., non-retryable errors) easy to propagate.
109-109: Expose and use chainId via the centralized hook — LGTM.This keeps consumers in sync with the same source of truth used by switching logic.
src/components/dashboard/invoices-received.tsx (1)
19-19: Adopting centralized network switching hook — LGTM.This aligns batch payments with the same retry and error-handling strategy used elsewhere.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/components/payment-section.tsx (1)
328-340: Network switch can throw before try/catch and freeze UI; also route.chain may be undefined for direct RN routeTwo coupled issues here:
- switchToChainId is awaited outside any try/catch. If the wallet rejects or the hook exhausts retries, the exception escapes, leaving paymentProgress stuck at "getting-transactions" and the button disabled.
- targetChain derives from selectedRoute?.chain. For REQUEST_NETWORK_PAYMENT, route.chain can be undefined; indexing CHAIN_TO_ID with undefined yields undefined, which then breaks ID_TO_APPKIT_NETWORK[targetChain].name and the switch call.
Harden the logic by:
- Deriving the target chain id with a safe fallback to invoiceChain for direct RN route.
- Guarding for unmapped/unknown chains.
- Wrapping the network switch in try/catch and resetting paymentProgress on failure.
Apply this diff:
- const targetChain = - CHAIN_TO_ID[selectedRoute?.chain as keyof typeof CHAIN_TO_ID]; - - if (targetChain !== chainId) { - const targetAppkitNetwork = - ID_TO_APPKIT_NETWORK[targetChain as keyof typeof ID_TO_APPKIT_NETWORK]; - - toast("Switching to network", { - description: `Switching to ${targetAppkitNetwork.name} network`, - }); - - await switchToChainId(targetChain); - } + // Derive target network robustly: + // - For direct RN route, fall back to the invoice chain. + // - For crosschain routes, use the route's chain. + const targetNetworkKey = + selectedRoute?.id === "REQUEST_NETWORK_PAYMENT" + ? invoiceChain + : selectedRoute?.chain; + + const targetChainId = targetNetworkKey + ? CHAIN_TO_ID[ + targetNetworkKey.toLowerCase() as keyof typeof CHAIN_TO_ID + ] + : undefined; + + if (!targetChainId) { + toast("Unsupported network", { + description: + "Could not determine the target network for this payment route.", + }); + setPaymentProgress("idle"); + return; + } + + if (targetChainId !== chainId) { + const targetAppkitNetwork = + ID_TO_APPKIT_NETWORK[ + targetChainId as keyof typeof ID_TO_APPKIT_NETWORK + ]; + + toast("Switching to network", { + description: targetAppkitNetwork?.name + ? `Switching to ${targetAppkitNetwork.name} network` + : "Switching network in your wallet", + }); + + try { + await switchToChainId(targetChainId); + } catch (_err) { + toast("Network switch failed", { + description: "Please switch networks in your wallet and try again.", + }); + setPaymentProgress("idle"); + return; + } + }
🧹 Nitpick comments (1)
src/components/payment-section.tsx (1)
109-109: Optional: Prefer switchToPaymentNetwork for direct RN routeGiven the hook already maps payment currency → chain, you can simplify direct-request flows by calling switchToPaymentNetwork(invoice.paymentCurrency) when selectedRoute?.id === "REQUEST_NETWORK_PAYMENT". That removes the need to juggle CHAIN_TO_ID/ID_TO_APPKIT_NETWORK here and keeps all mapping logic in one place.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
src/components/batch-payout.tsx(3 hunks)src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsx(3 hunks)src/components/dashboard/invoices-received.tsx(3 hunks)src/components/direct-payout.tsx(3 hunks)src/components/payment-section.tsx(3 hunks)src/lib/hooks/use-switch-network.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/components/batch-payout.tsx
- src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsx
- src/components/direct-payout.tsx
- src/components/dashboard/invoices-received.tsx
- src/lib/hooks/use-switch-network.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/components/payment-section.tsx (1)
src/lib/hooks/use-switch-network.ts (1)
useSwitchNetwork(7-40)
🔇 Additional comments (1)
src/components/payment-section.tsx (1)
24-24: Good move to centralized network switchingSwitching to the shared useSwitchNetwork hook aligns this component with the new cross-app pattern and reduces duplicated switch logic. LGTM.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/payment-section.tsx (1)
332-345: Guard undefined target chain and handle switch failures to avoid a stuck UITwo issues:
- targetChain can be undefined (e.g., REQUEST_NETWORK_PAYMENT route), leading to an invalid switch call.
- switchToChainId errors are uncaught and occur outside the try/catch, leaving paymentProgress stuck on “getting-transactions”.
Add proper chain resolution + try/catch around the switch and early-return on failure.
Apply this diff:
- const targetChain = - CHAIN_TO_ID[selectedRoute?.chain as keyof typeof CHAIN_TO_ID]; - - if (targetChain !== chainId) { - const targetAppkitNetwork = - ID_TO_APPKIT_NETWORK[targetChain as keyof typeof ID_TO_APPKIT_NETWORK]; - - toast("Switching to network", { - description: `Switching to ${targetAppkitNetwork?.name} network`, - }); - - await switchToChainId(targetChain); - } + // Resolve target chain id for the selected route (direct route falls back to invoice chain) + const directRouteChain = + invoiceChain && + REQUEST_NETWORK_CHAIN_TO_PAYMENT_NETWORK[ + invoiceChain as keyof typeof REQUEST_NETWORK_CHAIN_TO_PAYMENT_NETWORK + ]; + const chainKeyRaw = + selectedRoute?.id === "REQUEST_NETWORK_PAYMENT" + ? directRouteChain + : selectedRoute?.chain; + const chainKey = chainKeyRaw?.toLowerCase(); + const targetChainId = chainKey + ? CHAIN_TO_ID[chainKey as keyof typeof CHAIN_TO_ID] + : undefined; + + if (targetChainId == null) { + toast("Unsupported network", { + description: + "Could not determine the target network for the selected route.", + }); + setPaymentProgress("idle"); + return; + } + + if (targetChainId !== chainId) { + const targetAppkitNetwork = + ID_TO_APPKIT_NETWORK[ + targetChainId as keyof typeof ID_TO_APPKIT_NETWORK + ]; + + toast("Switching to network", { + description: `Switching to ${targetAppkitNetwork?.name} network`, + }); + + try { + await switchToChainId(targetChainId); + } catch { + toast("Network switch failed", { + description: "Please switch in your wallet and try again.", + }); + setPaymentProgress("idle"); + return; + } + }
🧹 Nitpick comments (1)
src/components/payment-section.tsx (1)
109-109: Hook usage is fine; consider pushing “name” concerns into the hook (optional)To reduce coupling with ID_TO_APPKIT_NETWORK here, consider exposing a getNetworkName(targetChainId) from the hook or returning the resolved Chain to describe the toast. Keeps constants localized.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
package.json(1 hunks)src/components/payment-section.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/payment-section.tsx (1)
src/lib/hooks/use-switch-network.ts (1)
useSwitchNetwork(7-40)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (1)
src/components/payment-section.tsx (1)
24-24: Centralized network switching hook adoption — LGTMSwitching to the shared useSwitchNetwork hook aligns this component with the new centralized logic and retry behavior.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/app/i/[id]/page.tsx (1)
25-33: Guard against null before accessing invoiceMeLink.user; reorder checks to avoid runtime crash
invoiceMeLinkis dereferenced before thenotFound()guard. IfgetByIdreturns null,invoiceMeLink.user.idwill throw. Move the existence check immediately after the fetch, then fetch session and run the redirect check.Apply this diff:
- const currentUser = await api.auth.getSessionInfo.query(); - - if (currentUser.user?.id === invoiceMeLink.user.id) { - return redirect("/dashboard"); - } - - if (!invoiceMeLink) { - notFound(); - } + if (!invoiceMeLink) { + notFound(); + } + const currentUser = await api.auth.getSessionInfo.query(); + if (currentUser.user?.id && currentUser.user.id === invoiceMeLink.user.id) { + return redirect("/dashboard"); + }src/components/invoice-form.tsx (3)
56-70: Invalid cast: filterValidPaymentDetails returns wrong shape for LinkedPaymentDetailYou’re returning
(PaymentDetails & { paymentDetailsPayers: ... })[]and casting toLinkedPaymentDetail[], but the rest of the code expectsdetail.paymentDetails.*. This will beundefinedat runtime.Map to the expected shape instead of casting:
-const filterValidPaymentDetails = ( - paymentDetails: (PaymentDetails & { - paymentDetailsPayers: (User & PaymentDetailsPayers)[]; - })[], -): LinkedPaymentDetail[] => { - // Return all payment details without filtering by client email - const validPaymentDetails = paymentDetails.filter((detail) => { - // Check if any payer is compliant - return detail.paymentDetailsPayers.some( - (payer: User & PaymentDetailsPayers) => payer.isCompliant, - ); - }); - return validPaymentDetails as unknown as LinkedPaymentDetail[]; -}; +const filterValidPaymentDetails = ( + paymentDetails: (PaymentDetails & { + paymentDetailsPayers: (User & PaymentDetailsPayers)[]; + })[], +): LinkedPaymentDetail[] => { + return paymentDetails + .filter((detail) => + detail.paymentDetailsPayers.some((payer) => payer.isCompliant), + ) + .map(({ paymentDetailsPayers, ...rest }) => ({ + paymentDetails: rest as PaymentDetails, + paymentDetailsPayers, + })); +};
480-491: Effect won’t re-run on paymentCurrency/isCryptoToFiat changesUsing
form.watch()inside the effect but not in deps means the effect only runs on mount (and whenformidentity changes). The “disable crypto-to-fiat when currency ≠ fUSDC” rule won’t consistently apply.Switch to
useWatchand depend on those values:-useEffect(() => { - const paymentCurrency = form.watch("paymentCurrency"); - const isCryptoToFiatEnabled = form.watch("isCryptoToFiatAvailable"); - - if (paymentCurrency !== "fUSDC-sepolia" && isCryptoToFiatEnabled) { - form.setValue("isCryptoToFiatAvailable", false); - // Clear payment details when disabling crypto-to-fiat - form.setValue("paymentDetailsId", ""); - form.clearErrors("paymentDetailsId"); - } -}, [form]); +useEffect(() => { + if (paymentCurrency !== "fUSDC-sepolia" && isCryptoToFiatEnabled) { + form.setValue("isCryptoToFiatAvailable", false); + form.setValue("paymentDetailsId", ""); + form.clearErrors("paymentDetailsId"); + } +}, [paymentCurrency, isCryptoToFiatEnabled, form]);Add these outside the effect (top-level of component):
import { useFieldArray, useWatch } from "react-hook-form"; const paymentCurrency = useWatch({ control: form.control, name: "paymentCurrency" }); const isCryptoToFiatEnabled = useWatch({ control: form.control, name: "isCryptoToFiatAvailable", });
405-413: Missing early return causes invoice creation while approval is pendingIn the “no payer linked” branch you set
waitingForPaymentApprovaland link the payment method, but you then fall through toonSubmit, bypassing the approval gate. This can create invoices with unapproved payment methods and even trigger duplicate submissions (approval flow will also submit).Add an early return after linking:
} else { setShowPendingApprovalModal(true); setWaitingForPaymentApproval(true); await handleBankAccountSuccess({ success: true, message: "Payment method linked successfully", paymentDetails: selectedPaymentDetail.paymentDetails, }); + // Wait for approval before submitting + return; }
🧹 Nitpick comments (2)
src/components/invoice-form.tsx (2)
493-519: Keep button disabled during submitAfterApproval to avoid double submit
submitAfterApprovalsetsisSubmittingto false before awaitingonSubmit, momentarily enabling the button and creating a race with user clicks.Set
isSubmitting(true)before submit and clear in finally:- // Reset the submitting state and directly call onSubmit - setIsSubmitting(false); + // Ensure UI stays disabled during the async submit + setIsSubmitting(true); @@ - await onSubmit(formData); - setInvoiceCreated(true); - setWaitingForPaymentApproval(false); + await onSubmit(formData); + setInvoiceCreated(true); + setWaitingForPaymentApproval(false); @@ - setInvoiceCreated(false); - setWaitingForPaymentApproval(false); - setIsSubmitting(false); + setInvoiceCreated(false); + setWaitingForPaymentApproval(false); + setIsSubmitting(false);
103-116: Minor: normalize status label to Title CasePure nit. If API returns uppercase enums, the current logic renders “APPROVED”. For a cleaner label:
- {status ? status.charAt(0).toUpperCase() + status.slice(1) : "Unknown"} + {status ? status.toLowerCase().replace(/^\w/, (c) => c.toUpperCase()) : "Unknown"}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/app/i/[id]/page.tsx(3 hunks)src/components/invoice-creator.tsx(1 hunks)src/components/invoice-form.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/invoice-creator.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-05-19T13:00:48.790Z
Learnt from: rodrigopavezi
PR: RequestNetwork/easy-invoice#45
File: src/components/invoice-form.tsx:316-319
Timestamp: 2025-05-19T13:00:48.790Z
Learning: The handleFormSubmit function in src/components/invoice-form.tsx correctly uses data.clientEmail from the form submission data to find matching payers, which is the proper implementation.
Applied to files:
src/components/invoice-form.tsx
📚 Learning: 2025-08-08T09:52:43.700Z
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#117
File: src/app/not-found.tsx:29-67
Timestamp: 2025-08-08T09:52:43.700Z
Learning: Repository: RequestNetwork/easy-invoice
Context: src/app/not-found.tsx (Next.js, TypeScript/React)
Learning: It is acceptable (by current product decision) for the NotFound page to display links to /invoices/create and /subscription-plans even though these routes are auth-gated. Rationale: prevent the 404 from feeling empty; most users are authenticated. No lock icons or conditional rendering required for now; can be revisited later as an enhancement.
Applied to files:
src/app/i/[id]/page.tsx
🧬 Code graph analysis (1)
src/components/invoice-form.tsx (1)
src/lib/schemas/invoice.ts (1)
InvoiceFormValues(75-75)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (4)
src/app/i/[id]/page.tsx (2)
10-10: Importing redirect is correct for server componentsImport looks good and matches usage below.
62-70: Prop shape for currentUser looks sane; verify InvoiceCreator typingPassing an optional
{ id, name, email }aligns with the summary. EnsureInvoiceCreator’s prop type markscurrentUseroptional and handles empty string fallbacks as intended.Would you like a quick repo scan script to confirm all
InvoiceCreatorusages are updated?src/components/invoice-form.tsx (2)
93-94: Prop type update to async onSubmit is correct; ensure callers await itGood change. Confirm all
InvoiceFormcallers now provide an async handler and properly reflect the loading state back viaisLoading.If helpful, I can generate a quick search to flag non-async onSubmit usages.
1036-1041: Nice UX: disable submit after successDisabling the button when
invoiceCreatedis true prevents accidental duplicate creations. Looks good.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/app/i/[id]/page.tsx (4)
25-25: Fetch session and link in parallel to shave an RTT.
Cuts latency and keeps semantics identical.Apply:
- const invoiceMeLink = await api.invoiceMe.getById.query(params.id); - const currentUser = await api.auth.getSessionInfo.query(); + const [invoiceMeLink, currentUser] = await Promise.all([ + api.invoiceMe.getById.query(params.id), + api.auth.getSessionInfo.query(), + ]);
31-33: Authorization intent unclear; current guard blocks authenticated non-owners from using InvoiceMe links.
If InvoiceMe is meant for clients (non-owners), this will bounce legitimate users.Options:
- Allow anyone (public link): remove the guard.
- if (currentUser.user && currentUser.user.id !== invoiceMeLink.user.id) { - return redirect("/dashboard"); - } + // Public link: no auth-based redirect
- Owner-only: require exact owner match.
- if (currentUser.user && currentUser.user.id !== invoiceMeLink.user.id) { - return redirect("/dashboard"); - } + if (!currentUser.user || currentUser.user.id !== invoiceMeLink.user.id) { + redirect("/dashboard"); + }Also ensure server-side invoice creation pins recipient to
invoiceMeLink.user.id, not a client-provided value.
31-33: Nit: no need toreturnredirect in Next.js.
redirect()throws; thereturnis redundant.- return redirect("/dashboard"); + redirect("/dashboard");
62-70: ConfirmInvoiceCreatorprop types; consider avoiding empty-string fallbacks.
Ifname/InvoiceCreator, preferundefinedto distinguish “unknown” from “intentionally empty.”- ? { - id: currentUser.user.id, - name: currentUser.user.name ?? "", - email: currentUser.user.email ?? "", - } + ? { + id: currentUser.user.id, + name: currentUser.user.name ?? undefined, + email: currentUser.user.email ?? undefined, + }If the component requires non-empty strings for controlled inputs, keep as-is and ignore this nit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/app/i/[id]/page.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-08T09:52:43.700Z
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#117
File: src/app/not-found.tsx:29-67
Timestamp: 2025-08-08T09:52:43.700Z
Learning: Repository: RequestNetwork/easy-invoice
Context: src/app/not-found.tsx (Next.js, TypeScript/React)
Learning: It is acceptable (by current product decision) for the NotFound page to display links to /invoices/create and /subscription-plans even though these routes are auth-gated. Rationale: prevent the 404 from feeling empty; most users are authenticated. No lock icons or conditional rendering required for now; can be revisited later as an enhancement.
Applied to files:
src/app/i/[id]/page.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (1)
src/app/i/[id]/page.tsx (1)
10-10: Import of redirect alongside notFound looks correct.
No concerns here.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/app/i/[id]/page.tsx (1)
31-33: Owner-only redirect logic is correct; tiny cleanup: dropreturn.
Matches prior feedback and prevents locking out other users. Prefer callingredirect()directly since it throws.Apply this diff:
- if (currentUser.user && currentUser.user.id === invoiceMeLink.user.id) { - return redirect("/dashboard"); - } + if (currentUser.user?.id === invoiceMeLink.user.id) { + redirect("/dashboard"); + }
🧹 Nitpick comments (2)
src/app/i/[id]/page.tsx (2)
25-25: Parallelize session + link fetch to reduce latency.
These two queries are independent; fetch them concurrently to shave one RTT.Apply this diff:
- const invoiceMeLink = await api.invoiceMe.getById.query(params.id); - const currentUser = await api.auth.getSessionInfo.query(); + const [invoiceMeLink, currentUser] = await Promise.all([ + api.invoiceMe.getById.query(params.id), + api.auth.getSessionInfo.query(), + ]);
62-70: Prop wiring looks good; consider avoiding empty-string fallbacks.
IfInvoiceCreatorcan handle missing name/email, prefer making these optional to avoid sentinel empty strings propagating through UI/validation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/app/i/[id]/page.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-08T09:52:43.700Z
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#117
File: src/app/not-found.tsx:29-67
Timestamp: 2025-08-08T09:52:43.700Z
Learning: Repository: RequestNetwork/easy-invoice
Context: src/app/not-found.tsx (Next.js, TypeScript/React)
Learning: It is acceptable (by current product decision) for the NotFound page to display links to /invoices/create and /subscription-plans even though these routes are auth-gated. Rationale: prevent the 404 from feeling empty; most users are authenticated. No lock icons or conditional rendering required for now; can be revisited later as an enhancement.
Applied to files:
src/app/i/[id]/page.tsx
🔇 Additional comments (1)
src/app/i/[id]/page.tsx (1)
10-10: Importing redirect with notFound — OK.
Usage is consistent with Next.js App Router APIs.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/app/i/[id]/page.tsx (1)
31-33: Redirect only the owner of the InvoiceMe link — correct fix.This aligns with the earlier review feedback and prevents redirecting other authenticated users.
🧹 Nitpick comments (3)
src/app/i/[id]/page.tsx (3)
10-10: Importing redirect is correct; ensure route is dynamic to avoid session-dependent caching pitfalls.Since the redirect depends on the logged-in user, confirm this page is rendered dynamically (not statically cached). If not already inferred via session access in your tRPC call, explicitly force dynamic rendering.
Apply this addition at module scope:
+export const dynamic = "force-dynamic";
25-25: Fetch session and invoice link concurrently to reduce TTFB.Both queries are independent; run them in parallel.
- const invoiceMeLink = await api.invoiceMe.getById.query(params.id); - const currentUser = await api.auth.getSessionInfo.query(); + const [invoiceMeLink, currentUser] = await Promise.all([ + api.invoiceMe.getById.query(params.id), + api.auth.getSessionInfo.query(), + ]);
62-70: ConfirmedcurrentUserProp Typing – Empty-String Fallback Is RequiredI’ve verified that
InvoiceCreatorPropsdeclarescurrentUser?as an object whosenameandstringtypes (not optional). That means:
- Using
currentUser.user.name ?? ""andcurrentUser.user.email ?? ""is necessary to satisfy the prop’s type and ensure the form’s defaultValues never receiveundefined.- If you instead want to allow
name/undefined, you would need to update the interface to:and add checks downstream. Otherwise, the existing fallbacks are correct.interface InvoiceCreatorProps { currentUser?: { id: string; name?: string; email?: string; }; /* … */ }Optional readability tweak: destructure the mapped
currentUserbefore JSX:- <InvoiceCreator - currentUser={ - currentUser.user - ? { - id: currentUser.user.id, - name: currentUser.user.name ?? "", - email: currentUser.user.email ?? "", - } - : undefined - } - invoiceCount={invoiceCount} - /> + const u = currentUser.user; + + <InvoiceCreator + currentUser={ + u + ? { id: u.id, name: u.name ?? "", email: u.email ?? "" } + : undefined + } + invoiceCount={invoiceCount} + />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/app/i/[id]/page.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-08T09:52:43.700Z
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#117
File: src/app/not-found.tsx:29-67
Timestamp: 2025-08-08T09:52:43.700Z
Learning: Repository: RequestNetwork/easy-invoice
Context: src/app/not-found.tsx (Next.js, TypeScript/React)
Learning: It is acceptable (by current product decision) for the NotFound page to display links to /invoices/create and /subscription-plans even though these routes are auth-gated. Rationale: prevent the 404 from feeling empty; most users are authenticated. No lock icons or conditional rendering required for now; can be revisited later as an enhancement.
Applied to files:
src/app/i/[id]/page.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
|
Congratulations, your pull request has been merged! Thank you for your valuable contribution to Request Network. As a reminder, every merged PR is automatically entered into our Best PR Initiative, offering a quarterly prize of $500. Your work significantly supports our project's growth, and we encourage you to continue engaging with our community. Additionally, if you want to build or add crypto payments and invoicing features, explore how our API can reduce deployment time from months to hours while offering advanced features. Book a call with our expert to learn more and fast-track your development. |
3 participants
Fixes: #124, #123, #119, #121
Summary by CodeRabbit
New Features
Improvements
Chores