Feat: upgrade trpc and react query, add checkout widget playground #168
Conversation
…sion differences due to wagmi and trpc query
WalkthroughAdds a complete Payment Widget (types, context, providers, Web3 integration, hooks, UI components, utils, and receipt/PDF generation), a Widget Playground page, many new UI primitives, TRPC client/server refactors (.query() → direct caller), moves utilities from Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as PaymentWidget UI
participant Prov as PaymentWidgetProvider
participant Web3 as Web3Provider/Wagmi
participant Modal as PaymentModal
participant API as Request API (RN_API_URL)
participant Chain as Blockchain
User->>UI: Click "Pay"
UI->>Prov: validate config, open modal
Prov->>Web3: ensure wallet connection
alt wallet not connected
UI->>Web3: open WalletConnectModal
Web3-->>UI: connected
end
Modal->>Modal: select currency & enter buyer info
Modal->>API: POST v2/payouts (createPayout)
API-->>Modal: returns tx instructions
Modal->>Web3: send transactions (sendTransaction)
Web3->>Chain: execute tx(s)
Chain-->>Web3: confirmations
Web3-->>Modal: tx receipts
Modal->>API: poll/check payment status
API-->>Modal: payment success
Modal->>UI: render PaymentSuccess & receipt download
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Focus areas:
Possibly related issues
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
🚀 Pull Request Review Report📋 SummaryThis PR introduces a comprehensive Payment Widget System and performs a major architectural refactoring of the Easy Invoice application. The changes include moving to a route group structure, adding cryptocurrency payment capabilities, and enhancing the overall user experience. ✅ Build Status: PASSED
🔒 Security: EXCELLENT
🎯 Key Features Added🆕 Payment Widget System
🏗️ Architecture Improvements
🎨 UI/UX Enhancements
📊 Performance
🔧 Code Quality: VERY GOOD
📝 Change Summary
🎉 Recommendation: APPROVED ✅This is a high-quality, well-executed PR that significantly enhances the application with:
Ready for merge and deployment 🚀 Reviewed with comprehensive security, code quality, and functionality analysis |
…at/167-checkout-widget-playground
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/app/(dashboard)/crypto-to-fiat/_components/compliance-form.tsx (3)
218-227: Duplicate success/error toasts (mutation onSuccess/onError + onSubmit).Users will see two toasts on both success and error. Handle notifications in one place only (prefer mutation callbacks) and keep onSubmit minimal.
Apply:
- async function onSubmit(values: ComplianceFormValues) { - try { - await submitComplianceMutation.mutateAsync(values); - toast.success("Compliance information submitted successfully!"); - } catch (error) { - toast.error( - `Failed to submit compliance information${error instanceof Error ? `. Error: ${error.message}` : ". Please try again."}`, - ); - } - } + async function onSubmit(values: ComplianceFormValues) { + // Toasts are handled in mutation onSuccess/onError + submitComplianceMutation.mutate(values); + }
295-301: Harden window.open against reverse‑tabnabbing.Add noopener/noreferrer (or nullify opener) when opening external URLs.
Apply:
- window.open(complianceData.kycUrl, "_blank"); + const w = window.open( + complianceData.kycUrl, + "_blank", + "noopener,noreferrer" + ); + if (w) w.opener = null;
206-208: Reset iframe loading overlay when reloading src.Without toggling isIframeLoading back to true, users may see a flash while the refreshed URL loads.
Apply:
- if (iframeRef.current && complianceData?.agreementUrl) { - iframeRef.current.src = complianceData.agreementUrl; - } + if (iframeRef.current && complianceData?.agreementUrl) { + setIsIframeLoading(true); + iframeRef.current.src = complianceData.agreementUrl; + }src/server/routers/compliance.ts (2)
305-311: Authorization gap: anyone can “allow” any payment details by ID; remove userId input and enforce ownership.Input
userIdisn’t used for auth; code never checks the current user ownspaymentDetailsId. A logged‑in user could attach a victim’s details to a payer. EnforcepaymentDetailsData.userId === ctx.user.idand dropuserIdfrom the API to prevent parameter tampering. Also avoid sending internal fields (id,userId) to the external API.allowPaymentDetails: protectedProcedure .input( z.object({ - userId: z.string(), paymentDetailsId: z.string(), payerEmail: z.string(), }), ) .mutation(async ({ ctx, input }) => { try { // Extract payer_email from paymentDetails if it exists - const { payerEmail, paymentDetailsId } = input; + const { payerEmail, paymentDetailsId } = input; const payerUser = await ctx.db.query.userTable.findFirst({ where: eq(userTable.email, payerEmail), }); @@ const paymentDetailsData = await ctx.db.query.paymentDetailsTable.findFirst({ where: eq(paymentDetailsTable.id, paymentDetailsId), }); if (!paymentDetailsData) { throw new TRPCError({ code: "NOT_FOUND", message: "Payment details not found", }); } + // Authorization: only the owner can allow their payment details + if (!ctx.user || paymentDetailsData.userId !== ctx.user.id) { + throw new TRPCError({ + code: "FORBIDDEN", + message: "You are not authorized to allow these payment details", + }); + } + // Remove undefined and null values to avoid API validation errors - const cleanedPaymentDetails = filterDefinedValues(paymentDetailsData); + const cleanedPaymentDetails = filterDefinedValues(paymentDetailsData); + // Omit internal fields (avoid leaking DB identifiers) + const { id: _omitId, userId: _omitUserId, ...payload } = + cleanedPaymentDetails as Record<string, unknown>; let response: AxiosResponse<PaymentDetailApiResponse>; try { response = await apiClient.post<PaymentDetailApiResponse>( `/v2/payer/${encodeURIComponent(payerEmail)}/payment-details`, - cleanedPaymentDetails, + payload, ); } catch (error: unknown) {Also applies to: 335-346, 347-355, 372-379
436-463: Limit joined user columns to avoid PII leakage in responses.Current join pulls all
userTablecolumns and spreads them into the response. Restrict to explicit fields (id, email, name, isCompliant).- const results = await ctx.db - .select() - .from(paymentDetailsPayersTable) - .leftJoin( - userTable, - eq(paymentDetailsPayersTable.payerId, userTable.id), - ) + const results = await ctx.db + .select({ + payment_details_payers: { + id: paymentDetailsPayersTable.id, + paymentDetailsId: paymentDetailsPayersTable.paymentDetailsId, + payerId: paymentDetailsPayersTable.payerId, + status: paymentDetailsPayersTable.status, + externalPaymentDetailId: + paymentDetailsPayersTable.externalPaymentDetailId, + }, + user: { + id: userTable.id, + email: userTable.email, + name: userTable.name, + isCompliant: userTable.isCompliant, + }, + }) + .from(paymentDetailsPayersTable) + .leftJoin( + userTable, + eq(paymentDetailsPayersTable.payerId, userTable.id), + ) .where( inArray( paymentDetailsPayersTable.paymentDetailsId, paymentDetailIds, ), ); @@ - if (entry && row.payment_details_payers) { - entry.paymentDetailsPayers.push({ - ...row.payment_details_payers, - ...(row.user ?? {}), - }); - } + if (entry && row.payment_details_payers) { + entry.paymentDetailsPayers.push({ + ...row.payment_details_payers, + ...(row.user ?? {}), + }); + }Also applies to: 466-470
🧹 Nitpick comments (43)
src/app/(dashboard)/crypto-to-fiat/_components/compliance-form.tsx (5)
83-99: v5 query migration looks good; consider removing duplicated state via select/cache.Current flow sets local complianceData from query data. Prefer deriving from query (select mapping) and updating via queryClient.setQueryData on mutation to avoid drift and extra renders. This keeps a single source of truth.
If you want, I can draft a select + setQueryData patch.
330-337: Consider iframe sandboxing and referrer policy.If the provider supports it, add sandbox and a strict referrerPolicy.
Apply (verify required allow‑list with the vendor):
<iframe ref={iframeRef} src={complianceData?.agreementUrl ?? ""} className="w-full h-full border-0" title="Compliance Agreement" width="100%" height="100%" + sandbox="allow-scripts allow-forms allow-popups allow-same-origin" + referrerPolicy="strict-origin-when-cross-origin" onLoad={() => setIsIframeLoading(false)} />
69-77: Trim trusted origins from env to avoid whitespace mismatches.Whitespace can break origin checks.
Apply:
- const origins = process.env.NEXT_PUBLIC_CRYPTO_TO_FIAT_TRUSTED_ORIGINS - ? process.env.NEXT_PUBLIC_CRYPTO_TO_FIAT_TRUSTED_ORIGINS.split(",") + const origins = process.env.NEXT_PUBLIC_CRYPTO_TO_FIAT_TRUSTED_ORIGINS + ? process.env.NEXT_PUBLIC_CRYPTO_TO_FIAT_TRUSTED_ORIGINS.split(",").map((o) => o.trim()).filter(Boolean) : ["https://request.network", "https://core-api-staging.pay.so"];
247-259: Skeleton branch likely never renders; use isFetching for background refetch.Top-level already gates on isLoadingStatus; the nested isLoadingStatus check is redundant. Consider isFetching to show lightweight skeletons on background refetches.
Apply:
- {isLoadingStatus && !complianceData ? ( + {/* Show skeleton during background refetch if no data yet */} + {isFetchingStatus && !complianceData ? ( <div className="w-full"> <Skeleton className="w-full h-40" /> </div> ) : ...and destructure from the query:
- isLoading: isLoadingStatus, + isLoading: isLoadingStatus, + isFetching: isFetchingStatus,
687-692: Pending state swap looks correct; consider broader disable conditions.Good use of isPending. Optionally also disable on initial query load or while refetching to prevent duplicate submissions.
Example:
- disabled={submitComplianceMutation.isPending} + disabled={ + submitComplianceMutation.isPending || isLoadingStatus + }src/components/payment-widget/components/disconnect-wallet.tsx (1)
1-28: LGTM! Clean wallet disconnection component.The component provides a clear UI for disconnecting the wallet with appropriate visual feedback. The address masking and disconnect functionality are implemented correctly.
Optional enhancement: Consider adding a conditional render or fallback for the edge case where
addressmight be undefined:<span className="text-sm font-mono text-muted-foreground"> - {`${address?.slice(0, 6)}...${address?.slice(-4)}`} + {address ? `${address.slice(0, 6)}...${address.slice(-4)}` : 'No address'} </span>This is defensive programming for an unlikely scenario, as the component name implies a wallet is already connected.
src/app/(dashboard)/invoices/me/_components/invoice-me-links.tsx (1)
41-50: Harden creation flow (trim, double‑submit guard, error surfacing)
- Trim label to avoid whitespace-only names.
- Prevent double submit on Enter while pending.
- Add try/catch to surface mutation errors and keep local UI consistent.
Apply this diff:
@@ - const handleCreateInvoiceMeLink = async () => { - if (newLinkName.length === 0) { + const handleCreateInvoiceMeLink = async () => { + const label = newLinkName.trim() + if (label.length === 0) { setError("Link label is required") return; } - await createInvoiceMeLink({ label: newLinkName }); - setIsCreating(false); - setNewLinkName(""); - setError(null); + try { + await createInvoiceMeLink({ label }); + setIsCreating(false); + setNewLinkName(""); + setError(null); + } catch (e: unknown) { + setError(e instanceof Error ? e.message : "Failed to create link"); + } }; @@ - await handleCreateInvoiceMeLink(); + if (!isCreatingInvoiceMeLink) { + await handleCreateInvoiceMeLink(); + } }} @@ - disabled={isCreatingInvoiceMeLink} + disabled={isCreatingInvoiceMeLink || newLinkName.trim().length === 0} > - {isCreatingInvoiceMeLink ? "Creating..." : "Create"} + {isCreatingInvoiceMeLink ? "Creating..." : "Create"}Also applies to: 84-91, 88-91
src/components/payment-widget/components/receipt/styles.css (1)
1-243: Confirm CSS nesting support or flatten selectorsThis file relies on nested CSS (e.g., .receipt-container { .receipt-header { ... } }). Ensure your target browsers and build pipeline support native CSS Nesting; otherwise these rules won’t apply.
- If relying on native nesting, confirm minimum browser versions and that PostCSS/minifier in Next 14 doesn’t strip them.
- Otherwise: enable postcss-nesting/postcss-preset-env or flatten selectors (e.g., .receipt-container .receipt-header { ... }).
Example flattening pattern:
-.receipt-container { - /* base */ - .receipt-header { ... } - .company-info { ... } -} +.receipt-container { /* base styles... */ } +.receipt-container .receipt-header { /* ... */ } +.receipt-container .company-info { /* ... */ }package.json (1)
40-46: Sanity check peer deps and devtools gating
- wagmi 2.x + viem 2.x present; ensure no unmet peer deps at runtime.
- If @tanstack/react-query-devtools is used, gate its import behind NODE_ENV !== "production" to avoid extra bundle weight.
Example dynamic import:
// somewhere in app root -if (process.env.NODE_ENV !== "production") { - const { ReactQueryDevtools } = await import("@tanstack/react-query-devtools"); - // render Devtools conditionally... -} +// Conditionally render Devtools only in devAlso applies to: 73-75, 40-42
src/app/(dashboard)/ecommerce/widget-playground/_components/currency-combobox.tsx (1)
41-53: Controlled/uncontrolled sync is fine; add optional max selection guardThe value-sync via useEffect is sound. If needed, support an optional maxSelections prop to limit badges and prevent over-selection in UI-heavy flows.
src/components/ui/command.tsx (1)
115-123: Minor cleanup: duplicate selected data attributeYou have both data-[selected='true'] and data-[selected=true] in className; keep one for consistency.
- "relative flex cursor-default gap-2 select-none items-center rounded-sm px-2 py-1.5 text-sm outline-none data-[disabled=true]:pointer-events-none data-[selected='true']:bg-accent data-[selected=true]:text-accent-foreground data-[disabled=true]:opacity-50 [&_svg]:pointer-events-none [&_svg]:size-4 [&_svg]:shrink-0", + "relative flex cursor-default gap-2 select-none items-center rounded-sm px-2 py-1.5 text-sm outline-none data-[disabled=true]:pointer-events-none data-[selected=true]:bg-accent data-[selected=true]:text-accent-foreground data-[disabled=true]:opacity-50 [&_svg]:pointer-events-none [&_svg]:size-4 [&_svg]:shrink-0",src/components/payment-widget/utils/receipt.ts (4)
17-23: Avoid private types in exported interfaces (export these or use intersections).
ReceiptBuyerInfoandReceiptCompanyInfoare not exported but are referenced by exportedReceiptData. This can break declaration emit and tooling. Export them or inline as intersections.Apply one of:
Option A — export the interfaces
-interface ReceiptBuyerInfo extends BuyerInfo { +export interface ReceiptBuyerInfo extends BuyerInfo { walletAddress: string; } -interface ReceiptCompanyInfo extends CompanyInfo { +export interface ReceiptCompanyInfo extends CompanyInfo { walletAddress: string; }Option B — inline intersections (no new exports)
-export interface ReceiptData { - company: ReceiptCompanyInfo; - buyer: ReceiptBuyerInfo; +export interface ReceiptData { + company: CompanyInfo & { walletAddress: string }; + buyer: BuyerInfo & { walletAddress: string };Also applies to: 25-37
39-45: Strengthen receipt number uniqueness.
Date.now()+ 3‑digit random can collide under load. Prefer ULID/UUID.+import { ulid } from "ulid"; export const generateReceiptNumber = (prefix = "REC"): string => { - const timestamp = Date.now(); - const random = Math.floor(Math.random() * 1000) - .toString() - .padStart(3, "0"); - return `${prefix}-${timestamp}-${random}`; + return `${prefix}-${ulid()}`; };
77-89: Allow optional issueDate override; default to now.Some receipts need a transaction/settlement date. Make
issueDateoptional and default it internally.export interface CreateReceiptParams { - company: ReceiptCompanyInfo; - buyer: ReceiptBuyerInfo; + company: ReceiptCompanyInfo; + buyer: ReceiptBuyerInfo; payment: PaymentInfo; items: ReceiptItem[]; totals: { totalDiscount: string; totalTax: string; total: string; totalUSD: string; }; - metadata: Omit<ReceiptMetadata, "issueDate">; + metadata: Omit<ReceiptMetadata, "issueDate"> & { issueDate?: Date }; } export const createReceipt = (params: CreateReceiptParams): ReceiptData => { const metadata: ReceiptMetadata = { - ...params.metadata, - issueDate: new Date(), + ...params.metadata, + issueDate: params.metadata.issueDate ?? new Date(), };Also applies to: 91-96
62-67: Optional: format crypto with localized digits/precision.Current output is raw string. Consider localized formatting with a sensible max fraction (e.g., 8) or per‑currency decimals from your currencies map.
Example (generic):
export const formatCryptoAmount = ( amount: string, currency: string, ): string => { - return `${amount} ${currency}`; + const n = Number.parseFloat(amount); + if (Number.isNaN(n)) return `${amount} ${currency}`; + return `${n.toLocaleString(undefined, { maximumFractionDigits: 8 })} ${currency}`; };src/server/trpc.ts (1)
25-31: Parse x-forwarded-for safely and handle lists/fallbacks.
x-forwarded-forcan be a comma‑separated list and is untrusted. Take the first value, trim, and fall back tox-real-ip.- let ip = opts.headers.get("x-forwarded-for"); + const xff = opts.headers.get("x-forwarded-for") ?? ""; + let ip = xff.split(",")[0]?.trim() || opts.headers.get("x-real-ip"); if (ip === "::1" || ip === "127.0.0.1") { ip = "203.0.113.195"; }src/server/routers/compliance.ts (1)
104-106: Nit: rename clientUserId to email for clarity (it’s used as email).The param is used in a URL as an email and in DB
where email = .... Rename to avoid confusion.src/lib/hooks/use-cancel-recurring-payment.ts (1)
93-99: Nit: align naming with v5 state — return isPending, not isLoading.You’ve mapped to
isPending; consider renaming the exposed property for clarity.return { cancelRecurringPayment, - isLoading: + isPending: updateRecurringPaymentMutation.isPending || updateRecurringPaymentForSubscriptionMutation.isPending, };src/lib/hooks/use-switch-network.ts (2)
10-16: Tighten retry semantics: backoff + no-retry on user rejection.
- Avoid 0 ms retries; add small backoff/jitter.
- Don’t retry if the wallet action was explicitly rejected (e.g., code 4001/UserRejected).
Promise.resolve(switchNetwork(chain))is redundant.Apply:
- const switchWithRetry = async (chain: Chain, opts?: RetryHooks<void>) => { - return await retry(() => Promise.resolve(switchNetwork(chain)), { - retries: 3, - delay: 0, - ...opts, - }); - }; + const switchWithRetry = async (chain: Chain, opts?: RetryHooks<void>) => { + return retry(async () => { + try { + await switchNetwork(chain); + } catch (err: any) { + // Don't hammer the wallet if user cancels + if (err?.code === 4001 || /UserRejected/i.test(String(err?.name ?? err?.message))) { + throw err; + } + throw err; + } + }, { + retries: 3, + delay: 250, // small backoff; consider jitter inside retry impl + ...opts, + }); + };
22-26: Defensive guards when resolving target chains.Ensure lookups never pass
undefinedintoswitchNetwork; fail fast with clear errors.- const targetChain = getChainFromPaymentCurrency(paymentCurrency); + const targetChain = getChainFromPaymentCurrency(paymentCurrency); + if (!targetChain) { + throw new Error(`Unsupported payment currency: ${paymentCurrency}`); + } @@ - const targetChain = - ID_TO_APPKIT_NETWORK[targetChainId as keyof typeof ID_TO_APPKIT_NETWORK]; + const targetChain = + ID_TO_APPKIT_NETWORK[targetChainId as keyof typeof ID_TO_APPKIT_NETWORK]; + if (!targetChain) { + throw new Error(`Unsupported chain id: ${targetChainId}`); + }Also applies to: 32-37
src/components/navigation/sidebar.tsx (1)
176-185: Active state should match nested routes too.Use
startsWith("/ecommerce/widget-playground")so subpages remain highlighted.- className={`block px-3 py-2 text-sm rounded-lg transition-colors ${ - pathname === "/ecommerce/widget-playground" + className={`block px-3 py-2 text-sm rounded-lg transition-colors ${ + pathname.startsWith("/ecommerce/widget-playground") ? "bg-primary/10 text-primary" : "text-muted-foreground hover:bg-muted" }`}src/components/ui/form.tsx (1)
174-176: Announce errors to screen readers and align styling.Add
role="alert"andaria-live="polite"; consider usingtext-destructiveto match FormMessage.-const FormError = ({ children }: { children: React.ReactNode }) => ( - <p className="text-destructive-foreground text-sm">{children}</p> -); +const FormError = ({ children }: { children: React.ReactNode }) => ( + <p role="alert" aria-live="polite" className="text-sm text-destructive"> + {children} + </p> +);src/components/payment-widget/components/connection-handler.tsx (1)
13-29: LGTM; consider handling reconnect state for smoother UX.Optionally show the loading state when
useAccount().isReconnecting(orstatus === "reconnecting") to avoid a brief flicker during autoConnect.src/components/payment-widget/context/web3-context.tsx (2)
20-25: walletConnectProjectId changes are ignored; config never refreshes.Because of the
refguard, changingwalletConnectProjectIdwon’t rebuild the wagmi config (e.g., enabling WalletConnect after initial render). Update when the value changes while still avoiding StrictMode double-inits.Apply this diff:
- const configRef = useRef<ReturnType<typeof getWagmiConfig> | null>(null); + const configRef = useRef<ReturnType<typeof getWagmiConfig> | null>(null); + const lastIdRef = useRef<string | undefined>(undefined); - const wagmiConfig = useMemo(() => { - if (!configRef.current) { - configRef.current = getWagmiConfig(walletConnectProjectId); - } - return configRef.current; - }, [walletConnectProjectId]); + const wagmiConfig = useMemo(() => { + // Rebuild only when the projectId value changes + if (!configRef.current || lastIdRef.current !== walletConnectProjectId) { + configRef.current = getWagmiConfig(walletConnectProjectId); + lastIdRef.current = walletConnectProjectId; + } + return configRef.current; + }, [walletConnectProjectId]);
8-8: Avoid module-scoped QueryClient; create per-provider instance.A global
QueryClientcan leak state across test runs and complicate multi-provider usage. Create it lazily via a ref in the component.Apply this diff:
-const queryClient = new QueryClient(); +// Create once per provider instance +// (stable across re-renders and StrictMode double-invocations)And inside
Web3Provider:export function Web3Provider({ children, walletConnectProjectId, }: { children: ReactNode; walletConnectProjectId?: string; }) { // @NOTE this may seem weird, but walletConnect doesn't handle strict mode initializing it twice, so we explicitly use a ref to store the config const configRef = useRef<ReturnType<typeof getWagmiConfig> | null>(null); + const queryClientRef = useRef<QueryClient>(); + if (!queryClientRef.current) { + queryClientRef.current = new QueryClient(); + }And in the JSX:
- <WagmiProvider config={wagmiConfig}> - <QueryClientProvider client={queryClient}>{children}</QueryClientProvider> - </WagmiProvider> + <WagmiProvider config={wagmiConfig}> + <QueryClientProvider client={queryClientRef.current!}> + {children} + </QueryClientProvider> + </WagmiProvider>src/app/(dashboard)/ecommerce/widget-playground/_components/clientid-select.tsx (1)
18-26: Align to React Query v5 and add an error state.Use
isPending(v5) and surface a simple error placeholder to avoid a silent empty list on failures.Apply this diff:
-export function ClientIdSelect({ value, onChange }: ClientIdSelectProps) { - const { data: clients = [], isLoading } = api.ecommerce.getAll.useQuery(); +export function ClientIdSelect({ value, onChange }: ClientIdSelectProps) { + const { + data: clients = [], + isPending, + isError, + } = api.ecommerce.getAll.useQuery(); - if (isLoading) { + if (isPending) { return ( <Select disabled> <SelectTrigger> <SelectValue placeholder="Loading..." /> </SelectTrigger> </Select> ); } + + if (isError) { + return ( + <Select disabled> + <SelectTrigger> + <SelectValue placeholder="Failed to load client IDs" /> + </SelectTrigger> + </Select> + ); + }src/components/payment-widget/README.md (1)
321-341: Minor wording/spelling check.Give this section a quick pass for typos (e.g., “wallet client” phrasing) to satisfy linting/LanguageTool.
src/components/payment-widget/context/payment-widget-context/payment-widget-provider.tsx (1)
70-86: Stabilize memo deps for config objectsDepending on nested props can miss updates if
paymentConfig/uiConfigare mutated in place. Prefer depending on the whole object (assuming immutability), or derive stable shallow copies upfront.- [ - amountInUsd, - recipientWallet, - walletAccount, - connectedWalletAddress, - isWalletOverride, - paymentConfig.rnApiClientId, - paymentConfig.feeInfo, - paymentConfig.supportedCurrencies, - paymentConfig.reference, - uiConfig?.showReceiptDownload, - uiConfig?.showRequestScanUrl, - receiptInfo, - onPaymentSuccess, - onPaymentError, - onComplete, - ], + [ + amountInUsd, + recipientWallet, + walletAccount, + connectedWalletAddress, + isWalletOverride, + paymentConfig, + uiConfig, + receiptInfo, + onPaymentSuccess, + onPaymentError, + onComplete, + ],src/components/payment-widget/types/index.ts (2)
6-6: Decide on numeric semantics now to avoid later churnMixing
number(e.g.,quantity) with stringified numbers elsewhere is error‑prone. Recommend: on‑chain amounts asbigint(wei), UI/fiat amounts as decimal strings with a helper (e.g.,Decimal), and normalize via Zod schemas.
8-17: Prefer serializable error shape and precise chain types
- Avoid storing native
ErroronPaymentError(not serializable, loses data across boundaries). Use{ message: string; code?: string; cause?: unknown }.- Tighten
Transactionwith viem types.+import type { Address, Hex } from "viem"; export interface PaymentError { type: "wallet" | "transaction" | "api" | "unknown"; - error: Error; + message: string; + code?: string; + cause?: unknown; } export interface Transaction { - to: string; - data: string; - value: { hex: string }; + to: Address; + data: Hex; + value: Hex; // or `bigint` if you normalize to wei }src/components/payment-widget/components/buyer-info-form.tsx (2)
42-60: Normalize and sanitize form data on submit
- Trim nested address fields to avoid persisting whitespace.
- Accept 2‑letter country codes case‑insensitively and normalize to uppercase.
- const onFormSubmit = (data: BuyerInfo) => { + const onFormSubmit = (data: BuyerInfo) => { const cleanValue = (value: string | undefined) => { if (typeof value === "string") { const trimmed = value.trim(); return trimmed === "" ? undefined : trimmed; } return value; }; - // we want to send undefined for empty optional fields + // we want to send undefined for empty optional fields and trim nested address const cleanData: BuyerInfo = { email: data.email, firstName: cleanValue(data.firstName), lastName: cleanValue(data.lastName), businessName: cleanValue(data.businessName), phone: cleanValue(data.phone), - address: data.address && hasAnyAddressField ? data.address : undefined, + address: + data.address && hasAnyAddressField + ? { + street: cleanValue(data.address.street)!, + city: cleanValue(data.address.city)!, + state: cleanValue(data.address.state)!, + country: cleanValue( + data.address.country?.toUpperCase(), + ) as string, + postalCode: cleanValue(data.address.postalCode)!, + } + : undefined, }; onSubmit(cleanData); }; - // Country pattern + // Country pattern <Input id="address.country" placeholder="US" {...register("address.country", { - required: hasAnyAddressField - ? "Country is required when address is provided" - : false, - pattern: hasAnyAddressField - ? { - value: /^[A-Z]{2}$/, - message: - "Use a 2-letter ISO country code (e.g. US, CA, GB)", - } - : undefined, + required: hasAnyAddressField + ? "Country is required when address is provided" + : false, + pattern: hasAnyAddressField + ? { + value: /^[A-Za-z]{2}$/, + message: + "Use a 2-letter ISO country code (e.g. US, CA, GB)", + } + : undefined, })} />Also applies to: 193-211
69-89: A11y and IDs
- Add
aria-invalidandaria-describedbyto inputs with errors for better screen reader support.- Consider avoiding dots in
id/htmlFor(e.g., useaddress-street) to simplify selectors and tests.Also applies to: 126-141, 148-188, 191-239
src/server/context.ts (1)
5-15: Usenullfor unknown IP and type the returnEmpty string is ambiguous. Return
nullwhen the IP is unavailable and annotate the function toPromise<Context>.-export async function createContext() { +export async function createContext(): Promise<Context> { const { session, user } = await getCurrentSession(); - const ip = ""; + const ip = null; return { session, user, db, ip, }; }src/components/ui/popover.tsx (1)
12-28: Avoid viewport clippingConsider passing Radix
collisionPaddingoravoidCollisionsto improve popover positioning in tight layouts.- <PopoverPrimitive.Content + <PopoverPrimitive.Content ref={ref} align={align} sideOffset={sideOffset} + avoidCollisions + collisionPadding={8}src/components/payment-widget/components/receipt/receipt-template.tsx (1)
39-45: Title and receipt number fallback
- Consider rendering “RECEIPT” instead of “REC”.
- Guard
receiptNumberto avoid#undefined.- <h2 className="receipt-title">REC</h2> + <h2 className="receipt-title">RECEIPT</h2> <div className="receipt-number"> - #{receipt.metadata.receiptNumber} + #{receipt.metadata.receiptNumber ?? "—"} </div>Also applies to: 41-45
src/app/(dashboard)/ecommerce/widget-playground/_components/validation.ts (1)
21-28: Consider adding validation for feePercentage format.The
feePercentagefield is currently validated as a plain string with no format constraints. This could allow invalid values (e.g., non-numeric strings or percentages exceeding 100).Consider applying a regex pattern and range validation:
feeInfo: z .object({ - feePercentage: z.string(), + feePercentage: z + .string() + .regex(/^\d+(\.\d{1,2})?$/, "Enter a valid percentage") + .refine((v) => { + const num = Number.parseFloat(v); + return num >= 0 && num <= 100; + }, "Percentage must be between 0 and 100"), feeAddress: z .string() .refine(isEthereumAddress, "Invalid Ethereum address format"), }) .optional(),src/components/payment-widget/utils/currencies.ts (1)
22-42: Enhance error handling with response details.The current error handling provides a generic message without HTTP status or response details, which can make debugging API failures difficult.
if (!response.ok) { - throw new Error("Network response was not ok"); + throw new Error( + `Failed to fetch conversion currencies: ${response.status} ${response.statusText}` + ); }src/app/(dashboard)/ecommerce/widget-playground/_components/playground-form.tsx (2)
135-142: Rename cleanedreceiptInfo → cleanedReceiptInfo.Minor readability/consistency nit.
- const cleanedreceiptInfo = { + const cleanedReceiptInfo = { ...formValues.receiptInfo, buyerInfo: Object.values(formValues.receiptInfo.buyerInfo || {}).some( (val) => val, ) ? formValues.receiptInfo.buyerInfo : undefined, }; ... - receiptInfo={${formatObject(cleanedreceiptInfo, 2)}} + receiptInfo={${formatObject(cleanedReceiptInfo, 2)}}Also applies to: 153-153
76-77: Remove unused codeRef.
codeRefisn’t used for any behavior. Drop it to reduce noise.- const codeRef = useRef<HTMLPreElement>(null); ... - <pre - ref={codeRef} - className="bg-muted text-foreground p-4 rounded-lg overflow-x-auto pr-24" - > + <pre className="bg-muted text-foreground p-4 rounded-lg overflow-x-auto pr-24">Also applies to: 278-284
src/lib/helpers.ts (1)
23-28: Harden truncateEmail for edge cases (no '@', small maxLength).Prevent negative slices and support addresses without '@'.
export function truncateEmail(email: string, maxLength = 20): string { - if (email.length <= maxLength) return email; - const [user, domain] = email.split("@"); - const keep = maxLength - domain.length - 4; - return `${user.slice(0, keep)}...@${domain}`; + if (email.length <= maxLength) return email; + const at = email.indexOf("@"); + if (at === -1) { + // Not an email; generic truncation + return email.length > maxLength + ? `${email.slice(0, Math.max(1, maxLength - 3))}...` + : email; + } + const user = email.slice(0, at); + const domain = email.slice(at + 1); + const keep = Math.max(1, maxLength - domain.length - 4); // 4 for "...@" + return `${user.slice(0, keep)}...@${domain}`; }src/app/(dashboard)/ecommerce/widget-playground/_components/customize.tsx (1)
16-16: Import cn from helpers to match refactor.Standardize on
@/lib/helpersto avoid duplicate implementations.-import { cn } from "@/lib/utils"; +import { cn } from "@/lib/helpers";src/components/payment-widget/payment-widget.tsx (1)
19-38: Broaden disabled-state checks to prevent invalid flows.Also disable when amount is not a positive number or recipientWallet is invalid. Suggest using
viem'sisAddress.-import { type PropsWithChildren, useState } from "react"; +import { type PropsWithChildren, useState } from "react"; +import { isAddress } from "viem"; ... const { walletAccount, - paymentConfig: { rnApiClientId, supportedCurrencies }, + paymentConfig: { rnApiClientId, supportedCurrencies }, + amountInUsd, + recipientWallet, } = usePaymentWidgetContext(); let isButtonDisabled = false; if (!rnApiClientId || rnApiClientId === "") { console.error("PaymentWidget: rnApiClientId is required in paymentConfig"); isButtonDisabled = true; } if (supportedCurrencies.length === 0) { console.error( "PaymentWidget: supportedCurrencies is required in paymentConfig", ); isButtonDisabled = true; } + + const amount = Number(amountInUsd); + if (!Number.isFinite(amount) || amount <= 0) { + console.error("PaymentWidget: amountInUsd must be a positive number"); + isButtonDisabled = true; + } + if (!recipientWallet || !isAddress(recipientWallet)) { + console.error("PaymentWidget: recipientWallet must be a valid address"); + isButtonDisabled = true; + }src/components/payment-widget/utils/payment.ts (1)
145-146: Remove redundant|| undefinedoperators.The optional chaining operator (
?.) already returnsundefinedwhen the property doesn't exist, making the|| undefinedredundant.Apply this diff:
- feePercentage: feeInfo?.feePercentage || undefined, - feeAddress: feeInfo?.feeAddress || undefined, + feePercentage: feeInfo?.feePercentage, + feeAddress: feeInfo?.feeAddress,
| useEffect(() => { | ||
| if (isComplianceSuccess) { | ||
| setComplianceData({ | ||
| agreementUrl: (complianceApiData.data.agreementUrl as string) ?? null, | ||
| kycUrl: (complianceApiData.data.kycUrl as string) ?? null, | ||
| status: { | ||
| agreementStatus: complianceApiData.data.agreementStatus as StatusType, | ||
| kycStatus: complianceApiData.data.kycStatus as StatusType, | ||
| isCompliant: complianceApiData.data.isCompliant, | ||
| }, | ||
| }, | ||
| ); | ||
| }); | ||
| } | ||
| }, [complianceApiData, isComplianceSuccess]); |
There was a problem hiding this comment.
Guard undefined data in effect and reduce type assertions.
Add a null-guard and avoid repeated as-casts; map once and store strongly typed.
Apply:
- useEffect(() => {
- if (isComplianceSuccess) {
- setComplianceData({
- agreementUrl: (complianceApiData.data.agreementUrl as string) ?? null,
- kycUrl: (complianceApiData.data.kycUrl as string) ?? null,
- status: {
- agreementStatus: complianceApiData.data.agreementStatus as StatusType,
- kycStatus: complianceApiData.data.kycStatus as StatusType,
- isCompliant: complianceApiData.data.isCompliant,
- },
- });
- }
- }, [complianceApiData, isComplianceSuccess]);
+ useEffect(() => {
+ const d = complianceApiData?.data;
+ if (isComplianceSuccess && d) {
+ const mapped: ComplianceResponse = {
+ agreementUrl: (d.agreementUrl as string) ?? null,
+ kycUrl: (d.kycUrl as string) ?? null,
+ status: {
+ agreementStatus: d.agreementStatus as StatusType,
+ kycStatus: d.kycStatus as StatusType,
+ isCompliant: d.isCompliant,
+ },
+ };
+ setComplianceData(mapped);
+ }
+ }, [complianceApiData, isComplianceSuccess]);📝 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.
| useEffect(() => { | |
| if (isComplianceSuccess) { | |
| setComplianceData({ | |
| agreementUrl: (complianceApiData.data.agreementUrl as string) ?? null, | |
| kycUrl: (complianceApiData.data.kycUrl as string) ?? null, | |
| status: { | |
| agreementStatus: complianceApiData.data.agreementStatus as StatusType, | |
| kycStatus: complianceApiData.data.kycStatus as StatusType, | |
| isCompliant: complianceApiData.data.isCompliant, | |
| }, | |
| }, | |
| ); | |
| }); | |
| } | |
| }, [complianceApiData, isComplianceSuccess]); | |
| useEffect(() => { | |
| const d = complianceApiData?.data; | |
| if (isComplianceSuccess && d) { | |
| setComplianceData({ | |
| agreementUrl: (d.agreementUrl as string) ?? null, | |
| kycUrl: (d.kycUrl as string) ?? null, | |
| status: { | |
| agreementStatus: d.agreementStatus as StatusType, | |
| kycStatus: d.kycStatus as StatusType, | |
| isCompliant: d.isCompliant, | |
| }, | |
| }); | |
| } | |
| }, [complianceApiData, isComplianceSuccess]); |
src/app/(dashboard)/ecommerce/widget-playground/_components/currency-combobox.tsx
Show resolved
Hide resolved
| <div className="flex justify-between text-muted-foreground"> | ||
| <span>Subtotal:</span> | ||
| <span> | ||
| $ | ||
| {( | ||
| Number.parseFloat(formValues.receiptInfo.totals.total) || 0 | ||
| ).toFixed(2)} | ||
| </span> | ||
| </div> |
There was a problem hiding this comment.
"Subtotal" label shows the final total.
Display the actual subtotal derived from items.
- <span>
- $
- {(
- Number.parseFloat(formValues.receiptInfo.totals.total) || 0
- ).toFixed(2)}
- </span>
+ <span>
+ $
+ {formValues.receiptInfo.items
+ .reduce((sum, item) => {
+ const v =
+ typeof item.total === "string"
+ ? Number.parseFloat(item.total) || 0
+ : item.total;
+ return sum + v;
+ }, 0)
+ .toFixed(2)}
+ </span>📝 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.
| <div className="flex justify-between text-muted-foreground"> | |
| <span>Subtotal:</span> | |
| <span> | |
| $ | |
| {( | |
| Number.parseFloat(formValues.receiptInfo.totals.total) || 0 | |
| ).toFixed(2)} | |
| </span> | |
| </div> | |
| <div className="flex justify-between text-muted-foreground"> | |
| <span>Subtotal:</span> | |
| <span> | |
| $ | |
| {formValues.receiptInfo.items | |
| .reduce((sum, item) => { | |
| const v = | |
| typeof item.total === "string" | |
| ? Number.parseFloat(item.total) || 0 | |
| : item.total; | |
| return sum + v; | |
| }, 0) | |
| .toFixed(2)} | |
| </span> | |
| </div> |
There was a problem hiding this comment.
This is fine, the values are synced in line 107
There was a problem hiding this comment.
@bassgeta, understood! I've noted your decision. The totals calculation in the useEffect is working correctly.
src/app/(dashboard)/ecommerce/widget-playground/_components/playground-form.tsx
Outdated
Show resolved
Hide resolved
| export const DEFAULT_CLIENT_ID_DOMAIN = | ||
| process.env.NEXT_PUBLIC_DEFAULT_ECOMMERCE_DOMAIN || | ||
| "https://checkout.request.network"; | ||
| "https://easyinvoice.request.network"; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the domain change doesn't break existing integrations.
The default domain change from checkout.request.network to easyinvoice.request.network aligns with integrating the payment widget into the dashboard. Ensure the new domain is accessible and serves the intended purpose for client ID operations.
Run the following script to check for any hardcoded references to the old domain:
🏁 Script executed:
#!/bin/bash
# Description: Search for hardcoded references to the old checkout domain
# Search for hardcoded references to checkout.request.network
rg -n "checkout\.request\.network" --type ts --type tsx --type js --type jsx -g '!*.lock' -g '!node_modules/**'Length of output: 27
🏁 Script executed:
#!/bin/bash
# Description: Search for hardcoded references to the old checkout domain (corrected)
# Search for hardcoded references to checkout.request.network without specifying tsx type
rg -n "checkout\.request\.network" --type ts --type js -g '!*.lock' -g '!node_modules/**'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Description: Verify domain change impact and usage patterns
# 1. Find usages of the DEFAULT_CLIENT_ID_DOMAIN constant
echo "=== Usage of DEFAULT_CLIENT_ID_DOMAIN ==="
rg -n "DEFAULT_CLIENT_ID_DOMAIN" --type ts --type js -g '!node_modules/**'
echo ""
echo "=== References to NEXT_PUBLIC_DEFAULT_ECOMMERCE_DOMAIN ==="
rg -n "NEXT_PUBLIC_DEFAULT_ECOMMERCE_DOMAIN" --type ts --type js -g '!node_modules/**'
echo ""
echo "=== References to easyinvoice.request.network ==="
rg -n "easyinvoice\.request\.network" --type ts --type js -g '!node_modules/**'
echo ""
echo "=== Environment file references ==="
fd -e env -e env.local -e env.example | head -20Length of output: 1590
Address database migration before domain change takes effect.
This constant change creates a data consistency issue. The DEFAULT_CLIENT_ID_DOMAIN is used for string comparisons against database records (ecommerceClientTable.domain). Existing database records with domain "https://checkout.request.network" will no longer match the new default value, breaking the logic that identifies default clients.
• Database impact: Existing ecommerce client records won't be recognized as default clients after this change
• Secondary reference: src/components/app-kit.tsx:17 also hardcodes the easyinvoice domain URL
Ensure a database migration or cleanup strategy is in place to update existing records to the new domain value before deploying this change.
🤖 Prompt for AI Agents
In src/lib/constants/ecommerce.ts lines 1-3, switching DEFAULT_CLIENT_ID_DOMAIN
to the new easyinvoice URL will break existing database string comparisons
against ecommerceClientTable.domain (records still set to
"https://checkout.request.network"); add a migration/cleanup to update existing
DB rows to the new domain before this constant is deployed — either (A) write
and run a DB migration that updates ecommerceClientTable.domain where value =
"https://checkout.request.network" to the new value (ensuring transactional
safety and backup), or (B) if you need a safe rollout, temporarily change code
that compares domains to accept both the old and new domain values and then
schedule/execute a migration to normalize records to the new domain, after which
you can remove the dual-check.
There was a problem hiding this comment.
Thanks for the heads up, but we don't have anybody using this feature yet, so we'll do a manual migration if necessary, so no need to solve this
There was a problem hiding this comment.
@bassgeta, understood! Since there are no existing users, a manual migration approach makes perfect sense. Thank you for confirming—this resolves the concern. 👍
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
src/app/(dashboard)/ecommerce/widget-playground/_components/playground-form.tsx (1)
20-68: Good fix: feeInfo default now undefined.Line 28 correctly addresses the previous validation issue by defaulting
feeInfotoundefinedinstead of an object with empty strings. This prevents the Ethereum address refinement from failing on an empty string.src/app/(dashboard)/crypto-to-fiat/_components/compliance-form.tsx (1)
100-113: Avoid shadowing state and map once with strong typing.Rename the local variable to avoid shadowing complianceData state, and build a typed object before setState.
- useEffect(() => { - const complianceData = complianceApiData?.data; - if (isComplianceSuccess && complianceData) { - setComplianceData({ - agreementUrl: (complianceData.agreementUrl as string) ?? null, - kycUrl: (complianceData.kycUrl as string) ?? null, - status: { - agreementStatus: complianceData.agreementStatus as StatusType, - kycStatus: complianceData.kycStatus as StatusType, - isCompliant: complianceData.isCompliant, - }, - }); - } - }, [complianceApiData, isComplianceSuccess]); + useEffect(() => { + const d = complianceApiData?.data; + if (isComplianceSuccess && d) { + const mapped: ComplianceResponse = { + agreementUrl: (d.agreementUrl as string) ?? null, + kycUrl: (d.kycUrl as string) ?? null, + status: { + agreementStatus: d.agreementStatus as StatusType, + kycStatus: d.kycStatus as StatusType, + isCompliant: d.isCompliant, + }, + }; + setComplianceData(mapped); + } + }, [complianceApiData, isComplianceSuccess]);
🧹 Nitpick comments (5)
src/lib/helpers.ts (1)
9-15: Consider naming the key variable explicitly.The underscore prefix (
_) conventionally signals an intentionally unused variable, but here the key is used inObject.fromEntries. Consider[key, value]or[k, v]for clarity.Apply this diff:
export function filterDefinedValues<T extends Record<string, unknown>>( obj: T, ): Partial<T> { return Object.fromEntries( - Object.entries(obj).filter(([_, v]) => v !== undefined && v !== null), + Object.entries(obj).filter(([key, value]) => value !== undefined && value !== null), ) as Partial<T>; }src/app/(dashboard)/ecommerce/widget-playground/_components/playground-form.tsx (1)
17-19: ConsideronBlurvalidation mode for better UX.With
mode: "onChange", validation fires on every keystroke. When users start filling in fee fields individually, they'll see validation errors immediately (e.g., "feePercentage is required") before completing both fields. Switching to"onBlur"would defer validation until the user leaves the field.const methods = useForm<z.infer<typeof PlaygroundValidation>>({ resolver: zodResolver(PlaygroundValidation), - mode: "onChange", + mode: "onBlur", defaultValues: {src/app/(dashboard)/crypto-to-fiat/_components/compliance-form.tsx (3)
219-221: Guard mutateAsync to avoid unhandled rejections in event handler.Rely on onError for toasts, but prevent a surfaced unhandled rejection.
- await submitComplianceMutation.mutateAsync(values); + try { + await submitComplianceMutation.mutateAsync(values); + } catch { + // handled in onError + }
685-690: Good: mutation pending state wired to the button.Minor a11y enhancement: add aria-busy={submitComplianceMutation.isPending}.
397-406: Use value prop instead of defaultValue for controlled Select componentsShadCN Selects should use
value={field.value}instead ofdefaultValue={field.value}to ensure proper synchronization when the form resets or updates.-<Select onValueChange={field.onChange} defaultValue={field.value}> +<Select onValueChange={field.onChange} value={field.value}>Applies to:
- Lines 397–406 (beneficiaryType)
- Lines 585–606 (country)
- Lines 618–638 (nationality)
Verified: beneficiaryType Select item values ("individual", "business") match the enum casing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/app/(dashboard)/crypto-to-fiat/_components/compliance-form.tsx(4 hunks)src/app/(dashboard)/ecommerce/widget-playground/_components/currency-combobox.tsx(1 hunks)src/app/(dashboard)/ecommerce/widget-playground/_components/playground-form.tsx(1 hunks)src/lib/helpers.ts(1 hunks)src/server/context.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/(dashboard)/ecommerce/widget-playground/_components/currency-combobox.tsx
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#168
File: src/components/payment-widget/README.md:29-31
Timestamp: 2025-10-28T12:17:14.899Z
Learning: The payment-widget component in src/components/payment-widget/ is an external component installed via ShadCN from the Request Network registry (https://ui.request.network). Its README and documentation should not be modified as it's maintained externally.
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#168
File: src/components/payment-widget/utils/payment.ts:86-87
Timestamp: 2025-10-28T12:17:42.971Z
Learning: The src/components/payment-widget/ directory contains an external component installed via ShadCN from the requestnetwork registry (https://ui.request.network/r/{name}.json) and should be treated as a vendored dependency. Modifications to files in this directory should not be suggested during code review.
📚 Learning: 2025-10-13T19:12:34.359Z
Learnt from: MantisClone
PR: RequestNetwork/easy-invoice#0
File: :0-0
Timestamp: 2025-10-13T19:12:34.359Z
Learning: In `src/server/routers/ecommerce.ts`, the `create` procedure for client IDs should use `?? undefined` for `feePercentage` and `feeAddress` when calling the external API, because the backend create endpoint uses `.optional()` and rejects `null`. However, the `edit` procedure should use `?? null` for these fields because the backend update endpoint uses `.nullable().optional()`, which allows `null` values to support explicitly unsetting fees.
Applied to files:
src/app/(dashboard)/ecommerce/widget-playground/_components/playground-form.tsx
🧬 Code graph analysis (3)
src/server/context.ts (2)
src/server/auth/index.ts (1)
getCurrentSession(96-103)src/server/db/index.ts (1)
db(10-12)
src/app/(dashboard)/ecommerce/widget-playground/_components/playground-form.tsx (5)
src/app/(dashboard)/ecommerce/widget-playground/_components/validation.ts (1)
PlaygroundValidation(4-96)src/app/(dashboard)/ecommerce/widget-playground/_components/customize.tsx (1)
CustomizeForm(23-365)src/app/(dashboard)/ecommerce/widget-playground/_components/seller-info.tsx (1)
SellerForm(10-221)src/app/(dashboard)/ecommerce/widget-playground/_components/buyer-info.tsx (1)
BuyerForm(10-209)src/components/payment-widget/payment-widget.tsx (1)
PaymentWidget(86-113)
src/app/(dashboard)/crypto-to-fiat/_components/compliance-form.tsx (2)
src/trpc/react.tsx (1)
api(12-12)src/app/(dashboard)/crypto-to-fiat/_components/compliance-status.tsx (1)
StatusType(5-10)
⏰ 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/server/context.ts (2)
16-21: Explicit Context type is clear and type-safe.The explicit type definition properly infers
sessionandusertypes fromgetCurrentSession()and eliminates the previously flagged unused optional fields (headers?,req?,res?). This aligns well with the tRPC v11 migration.
6-6: Remove dead code:createContext()in context.ts is not used.The
createContext()function in context.ts is never imported or called. The actual context creation happens viacreateTRPCContext()in trpc.ts, which is used by the route handler. The hardcoded empty IP string is not a real issue because this function is never executed.Delete the unused
createContext()function from context.ts entirely. Keep only theContexttype, which is correctly imported and used by trpc.ts.Likely an incorrect or invalid review comment.
src/lib/helpers.ts (2)
24-34: LGTM!Both
getCanCancelPaymentandisNotFoundErrorare correctly implemented. The TRPC error detection properly handles the nestedTRPCClientError→TRPCErrorstructure, and the status check logic is sound.
36-48: LGTM!The retry mechanism type definitions are well-structured with clear separation between configuration (
RetryConfig) and lifecycle hooks (RetryHooks). The combinedRetryOptionstype provides a clean API surface.
| const { | ||
| isLoading: isLoadingStatus, | ||
| data: complianceApiData, | ||
| refetch: getComplianceStatus, | ||
| isSuccess: isComplianceSuccess, | ||
| } = api.compliance.getComplianceStatus.useQuery( | ||
| { clientUserId: user?.email ?? "" }, | ||
| { | ||
| // Only fetch if we have a user email | ||
| enabled: !!user?.email, | ||
| // Use the configurable constant for polling interval | ||
| refetchInterval: COMPLIANCE_STATUS_POLLING_INTERVAL, | ||
| // Also refetch when the window regains focus | ||
| refetchOnWindowFocus: true, | ||
| }, | ||
| ); |
There was a problem hiding this comment.
Stop polling when compliant and use isFetching; fix unreachable skeleton branch.
- refetchInterval runs forever; return false when already compliant.
- Expose isFetching to show lightweight UI during background refetches.
- The inner skeleton gated by isLoadingStatus never renders because the outer branch already checks the same flag.
Apply:
- const {
- isLoading: isLoadingStatus,
+ const {
+ isLoading: isLoadingStatus,
+ isFetching,
data: complianceApiData,
refetch: getComplianceStatus,
isSuccess: isComplianceSuccess,
} = api.compliance.getComplianceStatus.useQuery(
{ clientUserId: user?.email ?? "" },
{
enabled: !!user?.email,
- refetchInterval: COMPLIANCE_STATUS_POLLING_INTERVAL,
+ refetchInterval: (q) =>
+ q.state.data?.isCompliant ? false : COMPLIANCE_STATUS_POLLING_INTERVAL,
refetchOnWindowFocus: true,
},
);Then render with:
- Initial: show spinner only when isLoadingStatus && !complianceData.
- Background: show skeleton/inline hint when isFetching.
📝 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.
| const { | |
| isLoading: isLoadingStatus, | |
| data: complianceApiData, | |
| refetch: getComplianceStatus, | |
| isSuccess: isComplianceSuccess, | |
| } = api.compliance.getComplianceStatus.useQuery( | |
| { clientUserId: user?.email ?? "" }, | |
| { | |
| // Only fetch if we have a user email | |
| enabled: !!user?.email, | |
| // Use the configurable constant for polling interval | |
| refetchInterval: COMPLIANCE_STATUS_POLLING_INTERVAL, | |
| // Also refetch when the window regains focus | |
| refetchOnWindowFocus: true, | |
| }, | |
| ); | |
| const { | |
| isLoading: isLoadingStatus, | |
| isFetching, | |
| data: complianceApiData, | |
| refetch: getComplianceStatus, | |
| isSuccess: isComplianceSuccess, | |
| } = api.compliance.getComplianceStatus.useQuery( | |
| { clientUserId: user?.email ?? "" }, | |
| { | |
| // Only fetch if we have a user email | |
| enabled: !!user?.email, | |
| // Use the configurable constant for polling interval | |
| refetchInterval: (q) => | |
| q.state.data?.isCompliant ? false : COMPLIANCE_STATUS_POLLING_INTERVAL, | |
| // Also refetch when the window regains focus | |
| refetchOnWindowFocus: true, | |
| }, | |
| ); |
🤖 Prompt for AI Agents
In src/app/(dashboard)/crypto-to-fiat/_components/compliance-form.tsx around
lines 83–98, stop unconditional polling and expose isFetching, and fix the
unreachable skeleton branch: include isFetching in the query destructure, change
refetchInterval to return false when the returned data indicates the user is
compliant (e.g., refetchInterval: (data) => data?.isCompliant ? false :
COMPLIANCE_STATUS_POLLING_INTERVAL), and update rendering so the initial spinner
shows only when isLoadingStatus && !complianceApiData, while the lightweight
skeleton/inline hint is shown when isFetching (not when isLoadingStatus) to
reflect background refetches.
src/app/(dashboard)/ecommerce/widget-playground/_components/playground-form.tsx
Show resolved
Hide resolved
src/app/(dashboard)/ecommerce/widget-playground/_components/playground-form.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/(dashboard)/crypto-to-fiat/_components/compliance-form.tsx (1)
243-246: Remove unreachable skeleton branch.The skeleton rendering on lines 243-246 checks
isLoadingStatus && !complianceData, but this code is inside theelsebranch of line 227'sisLoadingStatuscheck. This meansisLoadingStatusis alwaysfalsehere, making the skeleton branch unreachable.Apply this diff to remove the dead code:
- {isLoadingStatus && !complianceData ? ( - <div className="w-full"> - <Skeleton className="w-full h-40" /> - </div> - ) : complianceData?.status.kycStatus !== "not_started" || + {complianceData?.status.kycStatus !== "not_started" || complianceData?.status.agreementStatus !== "not_started" ? (Based on learnings
♻️ Duplicate comments (2)
src/app/(dashboard)/crypto-to-fiat/_components/compliance-form.tsx (1)
83-100: Fix refetchInterval to use query data directly, not local state.The
refetchIntervalon line 94 referencescomplianceData?.status?.isCompliant, which is local state updated via theuseEffectbelow. This creates a timing issue where the polling decision is based on potentially stale state rather than the fresh query result.Additionally,
isFetchingis not destructured, so there's no way to indicate background refetches to the user.Apply this diff:
const { isLoading: isLoadingStatus, + isFetching, data: complianceApiData, refetch: getComplianceStatus, isSuccess: isComplianceSuccess, } = api.compliance.getComplianceStatus.useQuery( { clientUserId: user?.email ?? "" }, { enabled: !!user?.email, - refetchInterval: complianceData?.status?.isCompliant - ? false - : COMPLIANCE_STATUS_POLLING_INTERVAL, + refetchInterval: (query) => + query.state.data?.data?.isCompliant + ? false + : COMPLIANCE_STATUS_POLLING_INTERVAL, refetchOnWindowFocus: true, }, );Then optionally expose
isFetchingin the UI (e.g., show a subtle indicator during background polls).src/app/(dashboard)/ecommerce/widget-playground/_components/playground-form.tsx (1)
219-236: Apply feeInfo cleaning logic to the preview.The preview passes
formValues.paymentConfigdirectly, which may contain a partially filledfeeInfo(e.g., onlyfeeAddressset, withfeePercentageempty). This inconsistency with the code generation logic (lines 125–129) could cause unexpected preview behavior.Apply the same cleaning logic used for code generation:
+ const cleanedPaymentConfig = { + ...formValues.paymentConfig, + feeInfo: + formValues.paymentConfig.feeInfo?.feeAddress && + formValues.paymentConfig.feeInfo?.feePercentage + ? formValues.paymentConfig.feeInfo + : undefined, + }; + <PaymentWidget amountInUsd={formValues.amountInUsd} recipientWallet={formValues.recipientWallet} - paymentConfig={formValues.paymentConfig} + paymentConfig={cleanedPaymentConfig} uiConfig={formValues.uiConfig} receiptInfo={formValues.receiptInfo}
🧹 Nitpick comments (2)
src/app/(dashboard)/crypto-to-fiat/_components/compliance-form.tsx (2)
102-115: Avoid variable shadowing and reduce unsafe type assertions.Line 103 declares
const complianceData = complianceApiData?.data;which shadows the outercomplianceDatastate variable, making the code harder to reason about. Additionally, multiple type assertions (as string,as StatusType) bypass TypeScript's type safety without runtime validation.Apply this diff to rename the local variable and clarify the mapping:
useEffect(() => { - const complianceData = complianceApiData?.data; - if (isComplianceSuccess && complianceData) { + const apiData = complianceApiData?.data; + if (isComplianceSuccess && apiData) { setComplianceData({ - agreementUrl: (complianceData.agreementUrl as string) ?? null, - kycUrl: (complianceData.kycUrl as string) ?? null, + agreementUrl: (apiData.agreementUrl as string) ?? null, + kycUrl: (apiData.kycUrl as string) ?? null, status: { - agreementStatus: complianceData.agreementStatus as StatusType, - kycStatus: complianceData.kycStatus as StatusType, - isCompliant: complianceData.isCompliant, + agreementStatus: apiData.agreementStatus as StatusType, + kycStatus: apiData.kycStatus as StatusType, + isCompliant: apiData.isCompliant, }, }); } }, [complianceApiData, isComplianceSuccess]);Consider validating the status types at runtime if the API contract is not guaranteed.
221-223: Add try/catch around mutateAsync or rely on mutation callbacks.The
await submitComplianceMutation.mutateAsync(values)on line 222 will throw on error by default. While the mutation hasonErrorcallbacks defined (lines 138-143), it's safer to either wrap the call in try/catch or remove theawaitand rely solely on the callbacks.Option 1 (explicit error handling):
async function onSubmit(values: ComplianceFormValues) { - await submitComplianceMutation.mutateAsync(values); + try { + await submitComplianceMutation.mutateAsync(values); + } catch (error) { + // Error already handled by onError callback + console.error("Submission failed:", error); + } }Option 2 (callback-only, no async):
- async function onSubmit(values: ComplianceFormValues) { - await submitComplianceMutation.mutateAsync(values); + function onSubmit(values: ComplianceFormValues) { + submitComplianceMutation.mutate(values); }Option 2 is simpler if you don't need to await the result.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/(dashboard)/crypto-to-fiat/_components/compliance-form.tsx(4 hunks)src/app/(dashboard)/ecommerce/widget-playground/_components/playground-form.tsx(1 hunks)src/lib/helpers.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/helpers.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#168
File: src/components/payment-widget/README.md:29-31
Timestamp: 2025-10-28T12:17:14.899Z
Learning: The payment-widget component in src/components/payment-widget/ is an external component installed via ShadCN from the Request Network registry (https://ui.request.network). Its README and documentation should not be modified as it's maintained externally.
📚 Learning: 2025-10-13T19:12:34.359Z
Learnt from: MantisClone
PR: RequestNetwork/easy-invoice#0
File: :0-0
Timestamp: 2025-10-13T19:12:34.359Z
Learning: In `src/server/routers/ecommerce.ts`, the `create` procedure for client IDs should use `?? undefined` for `feePercentage` and `feeAddress` when calling the external API, because the backend create endpoint uses `.optional()` and rejects `null`. However, the `edit` procedure should use `?? null` for these fields because the backend update endpoint uses `.nullable().optional()`, which allows `null` values to support explicitly unsetting fees.
Applied to files:
src/app/(dashboard)/ecommerce/widget-playground/_components/playground-form.tsx
🧬 Code graph analysis (2)
src/app/(dashboard)/ecommerce/widget-playground/_components/playground-form.tsx (5)
src/app/(dashboard)/ecommerce/widget-playground/_components/validation.ts (1)
PlaygroundValidation(4-96)src/app/(dashboard)/ecommerce/widget-playground/_components/customize.tsx (1)
CustomizeForm(23-365)src/app/(dashboard)/ecommerce/widget-playground/_components/seller-info.tsx (1)
SellerForm(10-221)src/app/(dashboard)/ecommerce/widget-playground/_components/buyer-info.tsx (1)
BuyerForm(10-209)src/components/payment-widget/payment-widget.tsx (1)
PaymentWidget(86-113)
src/app/(dashboard)/crypto-to-fiat/_components/compliance-form.tsx (1)
src/app/(dashboard)/crypto-to-fiat/_components/compliance-status.tsx (1)
StatusType(5-10)
⏰ 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 (3)
src/app/(dashboard)/crypto-to-fiat/_components/compliance-form.tsx (2)
293-297: Good partial security fix; origin validation deferred.The addition of
noopener,noreferrerattributes prevents tabnabbing attacks, which is a solid security improvement. Full URL origin validation (as discussed in previous reviews) is deferred to a future refactor per your earlier comment.
687-689: Correct migration to react-query v5 mutation state.The change from
isLoadingtoisPendingaligns with react-query v5's mutation API, whereisPendingindicates in-flight mutation status. This is the correct property to use for disabling the submit button and showing loading state.src/app/(dashboard)/ecommerce/widget-playground/_components/playground-form.tsx (1)
28-28: Previous feeInfo issues resolved.Good work addressing the earlier feedback! Setting
feeInfo: undefinedin defaultValues and requiring bothfeeAddressandfeePercentage(via AND) in the cleaning logic ensures partial feeInfo objects are excluded from generated code.Also applies to: 125-129
src/app/(dashboard)/ecommerce/widget-playground/_components/playground-form.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/app/(dashboard)/crypto-to-fiat/_components/compliance-form.tsx (1)
288-292: URL validation previously flagged and deferred.This window.open call was previously flagged for URL origin validation. The concern has been acknowledged and deferred to a future refactor per the PR author's decision.
src/app/(dashboard)/ecommerce/widget-playground/_components/playground-form.tsx (1)
229-246: Apply the same cleaning logic to the preview PaymentWidget.The preview currently passes
formValues.paymentConfigandformValues.receiptInfodirectly without cleaning. This can cause the preview to break or behave unexpectedly when:
feeInfois partially filled (e.g., onlyfeeAddresswithoutfeePercentage)buyerInfocontains empty nested objectsThe code generation (lines 120-130 and 143-149) properly cleans these values, but the preview does not.
Apply the same cleaning before passing to the preview:
+ const cleanedPaymentConfig = { + ...formValues.paymentConfig, + supportedCurrencies: formValues.paymentConfig.supportedCurrencies?.length + ? formValues.paymentConfig.supportedCurrencies + : undefined, + feeInfo: + formValues.paymentConfig.feeInfo?.feeAddress && + formValues.paymentConfig.feeInfo?.feePercentage + ? formValues.paymentConfig.feeInfo + : undefined, + }; + + const buyer = formValues.receiptInfo.buyerInfo || {}; + const hasDirectFields = [ + buyer.email, + buyer.firstName, + buyer.lastName, + buyer.businessName, + buyer.phone, + ].some((v) => v); + const hasAddress = + buyer.address && Object.values(buyer.address).some((v) => v); + const cleanedReceiptInfo = { + ...formValues.receiptInfo, + buyerInfo: + hasDirectFields || hasAddress + ? formValues.receiptInfo.buyerInfo + : undefined, + }; + <PaymentWidget amountInUsd={formValues.amountInUsd} recipientWallet={formValues.recipientWallet} - paymentConfig={formValues.paymentConfig} + paymentConfig={cleanedPaymentConfig} uiConfig={formValues.uiConfig} - receiptInfo={formValues.receiptInfo} + receiptInfo={cleanedReceiptInfo} onPaymentSuccess={(requestId) =>Note: This was flagged in previous reviews and marked as addressed, but the current code still passes raw form values.
🧹 Nitpick comments (2)
src/app/(dashboard)/crypto-to-fiat/_components/compliance-form.tsx (2)
82-99: Consider using query data directly in refetchInterval for cleaner implementation.The polling stop logic correctly prevents unnecessary refetches when compliant. However, the
refetchIntervaldepends on thecomplianceDatastate variable, which is derived from the query via auseEffect. This creates coupling between the query configuration and component state.Consider refactoring to use the query's own data directly:
const { isLoading: isLoadingStatus, data: complianceApiData, refetch: getComplianceStatus, isSuccess: isComplianceSuccess, } = api.compliance.getComplianceStatus.useQuery( { clientUserId: user?.email ?? "" }, { enabled: !!user?.email, - refetchInterval: complianceData?.status?.isCompliant - ? false - : COMPLIANCE_STATUS_POLLING_INTERVAL, + refetchInterval: (query) => + query.state.data?.data?.isCompliant + ? false + : COMPLIANCE_STATUS_POLLING_INTERVAL, refetchOnWindowFocus: true, }, );Optionally, expose
isFetchingto provide visual feedback during background refetches:const { isLoading: isLoadingStatus, + isFetching, data: complianceApiData, ... }
101-114: Type assertions suggest potential type safety improvements in TRPC router.The null-guard is now in place (good improvement), but type assertions like
as stringandas StatusTyperemain. These casts suggest either:
- The TRPC router response types are too broad (e.g.,
unknownorany)- Response types don't match the expected
ComplianceResponsestructureConsider investigating whether the TRPC router definition for
compliance.getComplianceStatuscan export stronger types to eliminate the need for assertions. If the API response schema is well-defined, TypeScript should infer these types automatically.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/(dashboard)/crypto-to-fiat/_components/compliance-form.tsx(5 hunks)src/app/(dashboard)/ecommerce/widget-playground/_components/playground-form.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#168
File: src/components/payment-widget/README.md:29-31
Timestamp: 2025-10-28T12:17:14.899Z
Learning: The payment-widget component in src/components/payment-widget/ is an external component installed via ShadCN from the Request Network registry (https://ui.request.network). Its README and documentation should not be modified as it's maintained externally.
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#168
File: src/components/payment-widget/context/payment-widget-context/payment-widget-provider.tsx:43-46
Timestamp: 2025-10-28T12:17:03.639Z
Learning: The payment widget components under src/components/payment-widget/ are installed via ShadCN from the Request Network registry and should not be modified locally to maintain compatibility with upstream.
📚 Learning: 2025-10-13T19:12:34.359Z
Learnt from: MantisClone
PR: RequestNetwork/easy-invoice#0
File: :0-0
Timestamp: 2025-10-13T19:12:34.359Z
Learning: In `src/server/routers/ecommerce.ts`, the `create` procedure for client IDs should use `?? undefined` for `feePercentage` and `feeAddress` when calling the external API, because the backend create endpoint uses `.optional()` and rejects `null`. However, the `edit` procedure should use `?? null` for these fields because the backend update endpoint uses `.nullable().optional()`, which allows `null` values to support explicitly unsetting fees.
Applied to files:
src/app/(dashboard)/ecommerce/widget-playground/_components/playground-form.tsx
🧬 Code graph analysis (2)
src/app/(dashboard)/ecommerce/widget-playground/_components/playground-form.tsx (5)
src/app/(dashboard)/ecommerce/widget-playground/_components/validation.ts (1)
PlaygroundValidation(4-96)src/app/(dashboard)/ecommerce/widget-playground/_components/customize.tsx (1)
CustomizeForm(23-365)src/app/(dashboard)/ecommerce/widget-playground/_components/seller-info.tsx (1)
SellerForm(10-221)src/app/(dashboard)/ecommerce/widget-playground/_components/buyer-info.tsx (1)
BuyerForm(10-209)src/components/payment-widget/payment-widget.tsx (1)
PaymentWidget(86-113)
src/app/(dashboard)/crypto-to-fiat/_components/compliance-form.tsx (2)
src/trpc/react.tsx (1)
api(12-12)src/app/(dashboard)/crypto-to-fiat/_components/compliance-status.tsx (1)
StatusType(5-10)
⏰ 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/(dashboard)/crypto-to-fiat/_components/compliance-form.tsx (2)
220-222: LGTM: Clean mutation handling.The submission flow correctly delegates success and error handling to the mutation's
onSuccessandonErrorcallbacks (lines 117-143), maintaining proper separation of concerns.
679-687: LGTM: Correct React Query v5 mutation state.The button correctly uses
isPendingto track mutation loading state, aligning with the React Query v5 upgrade.src/app/(dashboard)/ecommerce/widget-playground/_components/playground-form.tsx (2)
28-28: LGTM: feeInfo default correctly set to undefined.This properly addresses the previous validation error where empty string defaults would fail the address validation.
125-129: LGTM: feeInfo inclusion logic correctly requires both fields.The AND condition properly ensures that
feeInfois only included when bothfeeAddressandfeePercentageare provided, preventing incomplete objects in the generated code.
Problem
Since our dashboard is evolving into an entire product, we don't want the users to hop between multiple sites to configure their checkout experience.
Solution
Port the widget into the new dashboard and add it to the nav sidebar.
Changes
The widget uses
wagmiand that usesreact-queryversion 5, while we use version 4.Naturally I had to upgrade that and
trpctoo since v10 was incompatible with react-query v5...trpclibraries to v11 and do the necessary migration steps (you'll notice ourserver.tslooking quite different.rn-checkout, but make the layout be 2 columns.Testing
Well since the libraries were updated, it would be good to check almost everything 😅
Other than that check out the new route
/ecommerce/widget-playgroundand make sure everything works the same as it did on the checkout page.Resolves #167
Summary by CodeRabbit
New Features
Improvements
Documentation
Chores