-
Notifications
You must be signed in to change notification settings - Fork 3
feat: set invoice to "processing" status after being paid. #133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: set invoice to "processing" status after being paid. #133
Conversation
WalkthroughAdds a Redis-backed transient "processing" state for invoices, a new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant C as Client (payment-section)
participant S as TRPC invoiceRouter
participant R as Redis
participant DB as DB
rect rgba(230,245,255,0.6)
note over C: After successful payment confirmation
U->>C: Confirm payment
C->>S: setInvoiceAsProcessing({ id }) -- wrapped in inner try/catch
S->>DB: find invoice by id
alt invoice found
S->>R: SETEX processing:{id} = "true" (TTL)
S-->>C: OK
else not found
S-->>C: ERROR (NOT_FOUND)
end
end
rect rgba(240,255,240,0.6)
note over C,S: Reading invoice status (getById)
C->>S: getById({ id })
S->>DB: fetch invoice
S->>R: GET processing:{id}
alt redis key exists AND DB.status == "pending"
S-->>C: invoice with status "processing"
else
S-->>C: invoice with DB.status
end
end
rect rgba(255,245,230,0.6)
note over C: UI behavior
C-->>U: Show steps only if status == "pending"
C-->>U: Disable pay button when progress != "idle" or no routes
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/app/api/webhook/route.ts (1)
20-37
: Guard status transitions to prevent out-of-order webhooks from reverting paid invoicesRight now,
updateRequestStatus()
unconditionally overwrites status. If a delayedpayment.failed
arrives afterpayment.confirmed
/fiat_sent
, you could regresspaid
→pending
. Add optional allowed-from guards at the DB level to enforce a monotonic state machine and make calls likepayment.failed
only transition from the right predecessors.-import { and, eq, not } from "drizzle-orm"; +import { and, eq, not, inArray } from "drizzle-orm"; @@ -async function updateRequestStatus( - requestId: string, - status: (typeof requestStatusEnum.enumValues)[number], -) { +async function updateRequestStatus( + requestId: string, + status: (typeof requestStatusEnum.enumValues)[number], + allowedFrom?: (typeof requestStatusEnum.enumValues)[number][], +) { await db.transaction(async (tx) => { - const result = await tx - .update(requestTable) - .set({ status }) - .where(eq(requestTable.requestId, requestId)) - .returning({ id: requestTable.id }); + const whereClause = allowedFrom && allowedFrom.length + ? and(eq(requestTable.requestId, requestId), inArray(requestTable.status, allowedFrom)) + : eq(requestTable.requestId, requestId); + + const result = await tx + .update(requestTable) + .set({ status }) + .where(whereClause) + .returning({ id: requestTable.id, status: requestTable.status }); @@ - if (!result.length) { + if (!result.length) { throw new ResourceNotFoundError( `No request found with request ID: ${requestId}`, ); } }); }Example call sites to adopt this (illustrative):
- payment.failed non-offramp:
await updateRequestStatus(requestId, "pending", ["processing"]);
- payment.failed offramp:
await updateRequestStatus(requestId, "offramp_failed", ["offramp_initiated","offramp_pending"]);
- fiat_sent:
await updateRequestStatus(requestId, "paid", ["offramp_pending"]);
src/components/payment-section.tsx (3)
126-137
: UI can get stuck in “processing” after a failed payment; allow processing → pending updates from backendThe onSuccess guard suppresses setting status to "pending". With the new webhook resetting failed payments to "pending", this prevents the UI from re-enabling the payment steps.
- onSuccess: (data) => { - setInvoice(data); - if (data.status !== "pending") { - setPaymentStatus(data.status); - } - if (data.status === "paid") { - setPolling(false); - } - }, + onSuccess: (data) => { + setInvoice(data); + setPaymentStatus((prev) => { + // Allow retry if backend moved from processing -> pending + if (prev === "processing" && data.status === "pending") { + return "pending" as InvoiceStatus; + } + // Always adopt non-pending statuses from backend + if (data.status !== "pending") { + return data.status as InvoiceStatus; + } + // Otherwise keep current UI state + return prev; + }); + if (data.status === "paid") { + setPolling(false); + } + },
289-324
: Set “processing” immediately upon broadcast and decouple UI from mutation errorsMove the state flip and server mutation to right after broadcasting the tx (before awaiting confirmations), and ensure a failed mutation doesn’t surface as “Payment Failed”. This reduces race with the webhook setting “paid” and avoids misleading UX if only the status update fails.
- const paymentTransaction = await signer.sendTransaction( - paymentData.transactions[isApprovalNeeded ? 1 : 0], - ); - - await paymentTransaction.wait(); - - toast("Payment is being processed", { - description: "You can safely close this page.", - }); - - setPaymentStatus("processing"); + const paymentTransaction = await signer.sendTransaction( + paymentData.transactions[isApprovalNeeded ? 1 : 0], + ); + + // Optimistically mark as processing immediately after broadcast + setPaymentStatus("processing"); + try { + await setInvoiceAsProcessing({ id: invoice.id }); + } catch (e) { + console.warn("[setInvoiceAsProcessing] failed", e); + // Do not treat as payment failure + } + + await paymentTransaction.wait(); + + toast("Payment is being processed", { + description: "You can safely close this page.", + });
275-287
: Do the same for cross-chain: call setInvoiceAsProcessing after sending the intentApply the same pattern here so status flips server-side as soon as the intent is accepted.
setPaymentProgress("paying"); await sendPaymentIntent({ paymentIntent: paymentData.paymentIntentId, payload: signedPermit, }); setPaymentStatus("processing"); toast("Payment is being processed", { description: "You can safely close this page.", }); + + try { + await setInvoiceAsProcessing({ id: invoice.id }); + } catch (e) { + console.warn("[setInvoiceAsProcessing] failed", e); + }
🧹 Nitpick comments (6)
src/app/api/webhook/route.ts (1)
170-177
: Use contextual logging and avoid silent state regressions on payment.failedReplace the bare debug log with a contextual warning, so failed events are visible in production logs and searchable by requestId. Also include
subStatus
to aid triage.- console.debug("Handling failed"); + console.warn("payment.failed: resetting status", { + requestId, + isCryptoToFiat, + subStatus, + });src/components/payment-section.tsx (4)
378-381
: Avoid duplicate status writes; remove post-payment setInvoiceAsProcessingAfter moving the mutation into the payment flows, this call becomes redundant and can race with webhook updates. Remove it.
- await setInvoiceAsProcessing({ - id: invoice.id, - });
474-474
: Reconsider gating strictly to pending; offer retry UX for offramp_failedOnly rendering steps when
paymentStatus === "pending"
blocks any retry path for crypto-to-fiat failures (offramp_failed
). If that’s intentional, consider showing a lightweight “Retry payment” CTA that re-enters the flow; otherwise, allow steps forofframp_failed
as well.-{paymentStatus === "pending" && ( +{(paymentStatus === "pending" /* || paymentStatus === "offramp_failed" */) && (If you keep steps hidden for
offramp_failed
, at least render a banner with a retry button that triggershandlePayment
.
656-663
: Minor: redundant disable checkInside this gated block,
paymentStatus
is guaranteed to be"pending"
, sopaymentStatus === "processing"
is always false. You can drop this condition.- disabled={ - paymentProgress !== "idle" || - !hasRoutes || - paymentStatus === "processing" - } + disabled={paymentProgress !== "idle" || !hasRoutes}
145-147
: Name/usage clarity and error handling for status mutationThe hook is fine, but given it mutates global state, consider a name like
markInvoiceProcessing
and always wrap its call in a try/catch (as suggested above) to avoid surfacing it as a payment failure.src/server/routers/invoice.ts (1)
465-495
: Return value consistencyMost mutations here return data; this one returns nothing. Returning the updated invoice simplifies client-side optimistic updates and makes the API consistent.
📜 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/api/webhook/route.ts
(1 hunks)src/components/payment-section.tsx
(3 hunks)src/server/routers/invoice.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: MantisClone
PR: RequestNetwork/easy-invoice#59
File: src/app/api/webhook/route.ts:77-95
Timestamp: 2025-05-22T18:19:12.366Z
Learning: For webhook handlers in the Easy Invoice project, unknown or unexpected subStatus values in payment processing should be treated as errors (using console.error) rather than warnings, and should return a 422 Unprocessable Entity status code.
📚 Learning: 2025-02-12T12:40:14.742Z
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#2
File: src/server/routers/invoice.ts:88-109
Timestamp: 2025-02-12T12:40:14.742Z
Learning: The payRequest endpoint in src/server/routers/invoice.ts is intentionally kept public (using publicProcedure) to allow invoice sharing and payment by anyone with the payment reference, similar to how payment links work in other payment systems.
Applied to files:
src/server/routers/invoice.ts
src/components/payment-section.tsx
📚 Learning: 2025-02-12T13:04:46.135Z
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#2
File: src/server/routers/invoice.ts:0-0
Timestamp: 2025-02-12T13:04:46.135Z
Learning: The `payRequest` function in the invoice router retrieves payment transactions calldata from the Request API and does not process the payment itself. The actual payment status update happens through the webhook handler.
Applied to files:
src/components/payment-section.tsx
🧬 Code graph analysis (1)
src/server/routers/invoice.ts (4)
src/server/trpc.ts (1)
publicProcedure
(12-12)src/server/db/index.ts (1)
db
(10-12)src/server/db/schema.ts (1)
requestTable
(179-216)src/lib/errors.ts (1)
toTRPCError
(37-78)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved 👍 Pending comment resolution 🚧
We decided to investigate the potential security issue further.
…ng-on-db-when-awaiting-confirmed-webhook
…0-easyinvoice---change-invoice-to-processing-on-db-when-awaiting-confirmed-webhook
…en-awaiting-confirmed-webhook' of github.com:RequestNetwork/easy-invoice into 130-easyinvoice---change-invoice-to-processing-on-db-when-awaiting-confirmed-webhook
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (5)
.env.example (2)
20-20
: Fix dotenv-linter warning: remove quotes from empty valuedotenv-linter flags QuoteCharacter for INVOICE_PROCESSING_TTL="". Use an empty assignment without quotes.
-INVOICE_PROCESSING_TTL="" +INVOICE_PROCESSING_TTL=Optional: set a documented sample value to reduce ambiguity:
-# INVOICE_PROCESSING_TTL= +# INVOICE_PROCESSING_TTL=60
15-15
: Align Redis URL port in.env.example
to the code default (6379)The Redis client in
src/lib/redis/index.ts
falls back toredis://localhost:6379
whenREDIS_URL
isn’t set, but.env.example
currently points at port 7379. To avoid confusion during local setup, update the example to match the code’s default.• File:
.env.example
– Line 15: change port from 7379 to 6379Suggested diff:
- REDIS_URL=redis://localhost:7379 + REDIS_URL=redis://localhost:6379src/lib/redis/index.ts (1)
5-17
: Prevent multiple Redis clients in Next.js dev/HMR and serverless by using a global singletonModule-scoped singletons can multiply under hot-reload or serverless invocations. Cache the client on globalThis to avoid connection leaks.
-export function getRedis(): Redis { - if (!redis) { - redis = new Redis(process.env.REDIS_URL || "redis://localhost:6379", { - lazyConnect: true, - }); - - redis.on("error", (err) => { - console.error("Redis connection error:", err); - }); - } - - return redis; -} +declare global { + // eslint-disable-next-line no-var + var __easyInvoiceRedis__: Redis | undefined; +} + +export function getRedis(): Redis { + if (!globalThis.__easyInvoiceRedis__) { + const client = new Redis(process.env.REDIS_URL || "redis://localhost:6379", { + lazyConnect: true, + }); + client.on("error", (err) => { + console.error("Redis connection error:", err); + }); + globalThis.__easyInvoiceRedis__ = client; + } + return globalThis.__easyInvoiceRedis__!; +}Nice-to-have: detect rediss:// and set TLS options accordingly.
src/server/routers/invoice.ts (2)
475-506
: Reduce public attack surface: require both id and requestId to set processingSince this is public, requiring both id and requestId (or paymentReference) makes enumeration harder without sacrificing unauthenticated payability.
- .input( - z.object({ - id: z.string().ulid(), - }), - ) + .input( + z.object({ + id: z.string().ulid(), + requestId: z.string(), + }), + ) .mutation(async ({ ctx, input }) => { const { db } = ctx; try { - const invoice = await db.query.requestTable.findFirst({ - where: (requestTable, { eq }) => eq(requestTable.id, input.id), - }); + const invoice = await db.query.requestTable.findFirst({ + where: (requestTable, { and, eq }) => + and(eq(requestTable.id, input.id), eq(requestTable.requestId, input.requestId)), + });If you’d like, I can update the client call site in src/components/payment-section.tsx accordingly.
271-294
: Optional: Extend API-level InvoiceStatus to include “processing”Although the database enum (
requestStatusEnum
) only includes"pending"
,"paid"
, and"rejected"
(see src/server/db/schema.ts lines 48–50), thegetById
resolver returns"processing"
when Redis indicates a pending invoice is being processed. To keep your API types in sync and prevent client-side type drift, consider one of the following refactors:• Define an API-level
InvoiceStatus
union that includes"processing"
alongside the DB enum values. For example, in your router or a shared types file:export const invoiceStatusEnum = z.enum(["pending", "paid", "rejected"]); export const apiInvoiceStatus = z.union([invoiceStatusEnum, z.literal("processing")]);• Apply an explicit
.output()
schema on the procedure to enforce the extended status type:getById: publicProcedure .input(z.string()) .output( z.object({ id: z.string(), /* …other fields… */ status: apiInvoiceStatus, }) ) .query(async ({ ctx, input }) => { … })• Update any shared TypeScript interfaces or client-side types to accept
"processing"
in addition to the DB statuses.This change is optional but will help ensure your clients never encounter an unrecognized status value.
📜 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 (4)
.env.example
(1 hunks)src/components/payment-section.tsx
(4 hunks)src/lib/redis/index.ts
(1 hunks)src/server/routers/invoice.ts
(3 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-02-12T12:40:14.742Z
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#2
File: src/server/routers/invoice.ts:88-109
Timestamp: 2025-02-12T12:40:14.742Z
Learning: The payRequest endpoint in src/server/routers/invoice.ts is intentionally kept public (using publicProcedure) to allow invoice sharing and payment by anyone with the payment reference, similar to how payment links work in other payment systems.
Applied to files:
src/server/routers/invoice.ts
src/components/payment-section.tsx
📚 Learning: 2025-05-22T18:19:12.366Z
Learnt from: MantisClone
PR: RequestNetwork/easy-invoice#59
File: src/app/api/webhook/route.ts:77-95
Timestamp: 2025-05-22T18:19:12.366Z
Learning: For webhook handlers in the Easy Invoice project, unknown or unexpected subStatus values in payment processing should be treated as errors (using console.error) rather than warnings, and should return a 422 Unprocessable Entity status code.
Applied to files:
src/components/payment-section.tsx
📚 Learning: 2025-06-23T09:14:42.979Z
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#83
File: src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsx:127-138
Timestamp: 2025-06-23T09:14:42.979Z
Learning: In the RequestNetwork/easy-invoice codebase, when suggesting improvements like error handling for transaction approvals, the user bassgeta prefers consistency over isolated improvements. They prefer not to implement error handling in one place unless it's done consistently across all similar transaction flows in the codebase.
Applied to files:
src/components/payment-section.tsx
📚 Learning: 2025-02-12T13:04:46.135Z
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#2
File: src/server/routers/invoice.ts:0-0
Timestamp: 2025-02-12T13:04:46.135Z
Learning: The `payRequest` function in the invoice router retrieves payment transactions calldata from the Request API and does not process the payment itself. The actual payment status update happens through the webhook handler.
Applied to files:
src/components/payment-section.tsx
🧬 Code graph analysis (1)
src/server/routers/invoice.ts (2)
src/lib/redis/index.ts (1)
getRedis
(5-17)src/server/db/schema.ts (1)
requestTable
(179-216)
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 20-20: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🔇 Additional comments (3)
src/components/payment-section.tsx (3)
145-147
: LGTM: mutation hook addition is clean and idiomaticThe new setInvoiceAsProcessing useMutation is correctly declared and used.
671-672
: LGTM: disable logic simplified appropriatelySince the section is hidden once status becomes "processing", tying the disabled state to progress and routes is sufficient.
486-688
: Confirm UX: Include “offramp_failed” in retryable payment statesThe payment‐steps UI in src/components/payment‐section.tsx only renders when paymentStatus === "pending". If an invoice lands in “offramp_failed” (and the server doesn’t reset it to pending), users have no visible way to retry the payment flow.
Please verify how “offramp_failed” is handled downstream (e.g., via webhook logic in src/server/routers/invoice.ts) and choose one of:
- Client-side fix: adjust the rendering condition to include off-ramp failures when crypto-to-fiat is enabled
- Server-side fix: ensure failed off-ramps are reset to “pending” for retryable errors
Locations to update (approx. lines 486–688):
• In src/components/payment‐section.tsx, change:
- {paymentStatus === "pending" && ( + {(paymentStatus === "pending" || (paymentStatus === "offramp_failed" && isCryptoToFiatAvailable)) && (• (Optional) Verify or add logic in src/server/routers/invoice.ts to reset offramp_failed → pending for retryable failures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved 👍 Pending comment resolution 🚧
Summary
Changes
setInvoiceAsProcessing
TRPC mutation to update invoice status to "processing"payment.failed
webhook event handler to reset regular payments to "pending" statusTest plan
Demo video:
CleanShot.2025-08-21.at.16.02.50.mp4
Summary by CodeRabbit
New Features
Bug Fixes
Documentation