feat: add refund reason handling to payment order responses and webhook#683
feat: add refund reason handling to payment order responses and webhook#683onahprosper wants to merge 3 commits intomainfrom
Conversation
- Introduced RefundReason field in SenderOrderResponse and PaymentOrderWebhookData types to include refund reasons in API responses. - Implemented refundReasonFromOrder function to conditionally format refund reasons based on payment order status and cancellation reasons. - Updated relevant test cases to validate the inclusion of refund reasons in responses.
📝 WalkthroughWalkthroughAdds RefundReason propagation: new optional RefundReason fields in response and webhook structs, helper logic to compute a semicolon-joined refund reason from CancellationReasons, tests updated to validate refundReason type, and a pre-refund step that ensures CancellationReasons is populated before issuing refunds. Changes
Sequence Diagram(s)sequenceDiagram
participant Aggregator
participant SenderController as Sender Controller
participant Utils
participant Storage
participant StaleOps as StaleOps Task
participant RefundService as Refund Service
Aggregator->>SenderController: webhook (may include CancellationReasons)
SenderController->>Utils: build webhook/payment payload
Utils-->>SenderController: payload with RefundReason (if refunded)
SenderController->>Storage: persist webhook/order (includes RefundReason)
StaleOps->>Storage: find stale orders
StaleOps->>Storage: conditional update CancellationReasons if empty ("Order timed out")
StaleOps->>RefundService: RefundOrder(orderID, ... with CancellationReasons)
RefundService-->>Storage: record refund outcome
RefundService-->>SenderController: emit update/response (may include RefundReason)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tasks/stale_ops.go`:
- Around line 332-344: The current UpdateOneID(...).SetCancellationReasons(...)
unconditionally overwrites cancellation reasons from the in-memory snapshot and
can clobber a concurrent update; change the update to be conditional so it only
sets the default when the DB row still has empty reasons. Replace the
unconditional
storage.Client.PaymentOrder.UpdateOneID(order.ID).SetCancellationReasons(...)
call with a conditional update (e.g.
storage.Client.PaymentOrder.Update().Where(paymentorder.IDEQ(order.ID),
paymentorder.CancellationReasonsLenEQ(0) or an equivalent "is empty" predicate).
Keep the same error handling path but skip/continue when the conditional update
affects 0 rows so you don’t overwrite newer reasons for
order.CancellationReasons.
🧹 Nitpick comments (2)
controllers/sender/sender.go (1)
1710-1716: Centralize refund-reason derivation to avoid drift.
You now compute refundReason here and separately in webhook construction. Consider extracting a shared helper (e.g., utils.RefundReasonFromOrder) and reusing it to keep status rules in sync.utils/utils.go (1)
302-309: Deduplicate refundReason logic across webhook and sender responses.
Consider reusing a shared helper (e.g., from utils) so webhook and API responses stay aligned if status rules evolve.Also applies to: 339-339
- Updated the RetryStaleUserOperations function to log errors when setting cancellation reasons for payment orders, ensuring that refunds still proceed even if an error occurs. - Refactored the update logic to use a more explicit query structure for better clarity and maintainability.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
- Refactored the query in the RetryStaleUserOperations function to remove unnecessary conditions, enhancing clarity and maintainability. - Ensured that the cancellation reasons are set correctly for payment orders with a streamlined update process.
| if (paymentOrder.Status == paymentorder.StatusRefunded || paymentOrder.Status == paymentorder.StatusRefunding) && len(reasons) > 0 { | ||
| refundReason = strings.Join(reasons, "; ") | ||
| } |
There was a problem hiding this comment.
@onahprosper this could contain duplicate reasons
| if status != paymentorder.StatusRefunded || reasons == nil || len(reasons) == 0 { | ||
| return "" | ||
| } | ||
| return strings.Join(reasons, "; ") |
There was a problem hiding this comment.
@onahprosper this could contain duplicate reasons
Description
This PR adds refund reason to the
payment_order.refundingandpayment_order.refundedwebhooks and to the transaction details/list API when status is refunded. The reason is derived from existingcancellation_reasonson the PaymentOrder table; no schema or migration.refundReasontoPaymentOrderWebhookData; set from joinedCancellationReasonswhen status is refunding or refunded.refundReasontoSenderOrderResponse; set in GetPaymentOrderByID and list when status is refunded.References
Testing
Existing sender tests pass. Added assertions: when
refundReasonis present in GetPaymentOrderByID or in list order items, it is a string.Manual: trigger a refund (stale-ops or provider cancel), verify webhook payload and GET /sender/orders/:id and list include
refundReasonwhen status is refunded/refunding.This change adds test coverage for new/changed/fixed functionality
Checklist
mainBy submitting a PR, I agree to Paycrest's Contributor Code of Conduct and Contribution Guide.
Summary by CodeRabbit
New Features
Bug Fixes
Tests