Skip to content

Conversation

@mayur1377
Copy link
Contributor

@mayur1377 mayur1377 commented Dec 21, 2025

Fixes #182

Moved feePercentage and feeAddress from inside the requests.map() callback to the body level where the API expects them. This matches the pattern used in the single pay endpoint.

Summary by CodeRabbit

  • Refactor
    • Optimized payment processing logic by improving how fees are handled in batch payment requests for better performance and code efficiency.

✏️ Tip: You can customize this high-level summary in your review settings.

Fixes RequestNetwork#182

- Move feePercentage and feeAddress from requests array to body level
- Ensures platform fees are collected for both batch payouts and batch invoice payments
- Matches the pattern used in the single pay endpoint
- Prevents revenue loss on batch transactions
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 21, 2025

Walkthrough

Modified src/server/routers/payment.ts to fix batch payment fee collection. Moved platform fee environment variables from per-payout level to request body level in batchPay. Also refactored pay mutation to use property shorthand notation and optimized fee variable retrieval in batch operations.

Changes

Cohort / File(s) Summary
Batch payment fee collection fix
src/server/routers/payment.ts
Restructured fee handling in batchPay mutation: retrieved FEE_PERCENTAGE_FOR_PAYMENT and FEE_ADDRESS_FOR_PAYMENT environment variables once before API call and conditionally spread them at request body level (instead of per-payout level). Refactored pay mutation to use ES6 property shorthand notation for fee fields while preserving conditional inclusion logic.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Areas requiring attention:
    • Verify the conditional spread logic correctly checks both environment variables are defined before including fees
    • Confirm the fee fields are now properly positioned at the request body level to match the API contract expectations
    • Validate that the pay mutation's property shorthand refactoring preserves existing conditional logic for single payouts
    • Test both requests array path (batch payouts) and requestIds array path (batch invoice payments) to ensure fees are collected in both scenarios

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: moving platform fee fields from inside request entries to body level in the batchPay endpoint.
Linked Issues check ✅ Passed The PR addresses issue #182 by moving feePercentage and feeAddress from within requests.map() to body level with conditional spreading, matching the single pay endpoint pattern and fixing the revenue loss bug.
Out of Scope Changes check ✅ Passed All changes are directly focused on fixing the platform fee collection issue in batchPay. The modifications to the pay mutation's object shorthand and the batchPay fee field relocation are all within scope of issue #182.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4d82506 and 59e06da.

📒 Files selected for processing (1)
  • src/server/routers/payment.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*

⚙️ CodeRabbit configuration file

**/*: - Only comment on issues that would block merging — ignore minor or stylistic concerns.

  • Restrict feedback to errors, security risks, or functionality-breaking problems.
  • Do not post comments on code style, formatting, or non-critical improvements.
  • Keep reviews short: flag only issues that make the PR unsafe to merge.
  • Limit review comments to 3–5 items maximum, unless additional blockers exist.
  • Group similar issues into a single comment instead of posting multiple notes.
  • Skip repetition — if a pattern repeats, mention it once at a summary level only.
  • Do not add general suggestions; focus strictly on merge-blocking concerns.
  • If there are no critical problems, respond with minimal approval (e.g., 'Looks good'). Do not add additional review.
  • Avoid line-by-line commentary unless it highlights a critical bug or security hole.
  • Highlight only issues that could cause runtime errors, data loss, or severe maintainability issues.
  • Ignore minor optimization opportunities — focus solely on correctness and safety.
  • Provide a top-level summary of critical blockers rather than detailed per-line notes.
  • Comment only when the issue must be resolved before merge — otherwise, remain silent.
  • When in doubt, err on the side of fewer comments — brevity and blocking issues only.
  • Avoid posting any refactoring issues

Files:

  • src/server/routers/payment.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: MantisClone
Repo: RequestNetwork/easy-invoice PR: 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.
Learnt from: aimensahnoun
Repo: RequestNetwork/easy-invoice PR: 67
File: src/server/routers/payment.ts:47-49
Timestamp: 2025-06-04T12:02:39.411Z
Learning: In `src/server/routers/payment.ts`, the batchPay input validation already handles empty arrays correctly. The `batchPaymentFormSchema.shape.payouts.optional()` inherits the `.min(1, "At least one payment is required")` validation from the original schema, so empty payouts arrays are automatically rejected even when the field is made optional.
Learnt from: aimensahnoun
Repo: RequestNetwork/easy-invoice PR: 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.
📚 Learning: 2025-10-13T19:12:34.359Z
Learnt from: MantisClone
Repo: RequestNetwork/easy-invoice PR: 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/server/routers/payment.ts
📚 Learning: 2025-06-04T12:02:39.411Z
Learnt from: aimensahnoun
Repo: RequestNetwork/easy-invoice PR: 67
File: src/server/routers/payment.ts:47-49
Timestamp: 2025-06-04T12:02:39.411Z
Learning: In `src/server/routers/payment.ts`, the batchPay input validation already handles empty arrays correctly. The `batchPaymentFormSchema.shape.payouts.optional()` inherits the `.min(1, "At least one payment is required")` validation from the original schema, so empty payouts arrays are automatically rejected even when the field is made optional.

Applied to files:

  • src/server/routers/payment.ts
📚 Learning: 2025-02-12T12:40:14.742Z
Learnt from: aimensahnoun
Repo: RequestNetwork/easy-invoice PR: 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/payment.ts
🧬 Code graph analysis (1)
src/server/routers/payment.ts (1)
src/lib/axios.ts (1)
  • apiClient (3-8)
⏰ 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). (3)
  • GitHub Check: Lint
  • GitHub Check: Build
  • GitHub Check: Greptile Review
🔇 Additional comments (1)
src/server/routers/payment.ts (1)

60-79: Core fix looks correct.

Fee fields are now properly placed at the request body level for both batch payment paths (requests and requestIds), matching the single pay endpoint pattern. This should resolve the revenue loss issue.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link

greptile-apps bot commented Dec 21, 2025

Greptile Summary

This PR fixes a critical revenue loss bug where batch payments (batchPay endpoint) were not collecting platform fees. The fix moves feePercentage and feeAddress from inside the requests array to the body level of the API request, matching the correct pattern used in the single pay endpoint.

Key changes:

  • Extracted fee environment variables (FEE_PERCENTAGE_FOR_PAYMENT, FEE_ADDRESS_FOR_PAYMENT) before the API call
  • Removed fee fields from inside requests.map() callback where they were incorrectly nested
  • Added fee fields at body level using conditional spreading (only when both values are present)
  • Also simplified the single pay endpoint by using object shorthand notation

Impact:

  • Fixes revenue loss for both batch payment paths: batch payouts (via requests array) and batch invoice payments (via requestIds array)
  • Aligns batch payment implementation with single payment implementation
  • No breaking changes to the API contract

Confidence Score: 5/5

  • This PR is safe to merge with no risk
  • The fix is straightforward and critical: it correctly moves platform fee fields to the body level where the API expects them. The change mirrors the existing pattern from the single pay endpoint (lines 22-36), ensures consistent fee handling across both payment methods, and includes proper conditional spreading to handle missing environment variables gracefully. No logic errors, security issues, or edge cases identified.
  • No files require special attention

Important Files Changed

Filename Overview
src/server/routers/payment.ts Fixed critical platform fee bug by moving feePercentage and feeAddress from request array to body level in batchPay endpoint, matching the pattern used in single pay endpoint

Sequence Diagram

sequenceDiagram
    participant Client
    participant batchPay as batchPay Endpoint
    participant Env as Environment Variables
    participant API as v2/payouts/batch API
    
    Client->>batchPay: Request batch payment
    Note over Client,batchPay: input: { payouts/requestIds, payer }
    
    batchPay->>batchPay: Check user authorization
    
    batchPay->>Env: Read FEE_PERCENTAGE_FOR_PAYMENT
    Env-->>batchPay: feePercentage value
    
    batchPay->>Env: Read FEE_ADDRESS_FOR_PAYMENT
    Env-->>batchPay: feeAddress value
    
    alt Both fee values present
        batchPay->>API: POST with fee fields at body level
        Note over batchPay,API: { requests/requestIds, payer,<br/>feePercentage, feeAddress }
    else Missing fee values
        batchPay->>API: POST without fee fields
        Note over batchPay,API: { requests/requestIds, payer }
    end
    
    API-->>batchPay: Payment response
    batchPay-->>Client: Return response.data
Loading

@greptile-apps
Copy link

greptile-apps bot commented Dec 21, 2025

Greptile's behavior is changing!

From now on, if a review finishes with no comments, we will not post an additional "statistics" comment to confirm that our review found nothing to comment on. However, you can confirm that we reviewed your changes in the status check section.

This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Batch payments not collecting platform fee

1 participant