fix(*): payment initiations fixes #374
Conversation
WalkthroughThis pull request makes several adjustments across the codebase by removing unused fields (like Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Workflow
participant Engine
participant Plugin
Client->>Workflow: Initiate Payment (with ScheduledAt)
Workflow->>Workflow: Check if ScheduledAt > current time
alt Payment is Scheduled
Workflow->>Workflow: Add Adjustment (SCHEDULED_FOR_PROCESSING)
Workflow->>Workflow: Sleep until ScheduledAt
end
Workflow->>Engine: Call CreateTransfer/CreatePayout (no startDelay)
Engine->>Plugin: Execute Payment Operation
Plugin-->>Engine: Return Result
Engine-->>Workflow: Return Task/Result
Workflow-->>Client: Confirm Payment Processed
Assessment against linked issues
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code Definitions (1)internal/connectors/engine/workflow/create_transfer.go (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
55f901a to
e222bf4
Compare
e222bf4 to
f21afce
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/connectors/engine/workflow/create_transfer.go (1)
55-76: Well-implemented scheduling logic for payment initiations.The added code correctly checks if the payment is scheduled for the future and handles it appropriately by:
- Creating an adjustment with the new
SCHEDULED_FOR_PROCESSINGstatus- Including the scheduled time in the metadata
- Using
workflow.Sleepto pause execution until the scheduled timeThis implementation aligns with the PR objectives by replacing the delay mechanism with a sleep function and introducing the new adjustment status.
However, I would suggest adding a comment explaining the purpose of this block to improve code readability:
+// If the payment is scheduled for a future time: +// 1. Create an adjustment with SCHEDULED_FOR_PROCESSING status +// 2. Sleep until the scheduled time before proceeding now := workflow.Now(ctx) if !pi.ScheduledAt.IsZero() && pi.ScheduledAt.After(now) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
openapi.yamlis excluded by!**/*.yamlopenapi/v1-2/v1-2.yamlis excluded by!**/*.yamlopenapi/v3/v3-schemas.yamlis excluded by!**/*.yaml
📒 Files selected for processing (19)
internal/api/backend/backend_generated.go(0 hunks)internal/api/services/payment_initiations_approve.go(1 hunks)internal/api/services/payment_initiations_approve_test.go(2 hunks)internal/api/services/payment_initiations_create.go(1 hunks)internal/api/services/payment_initiations_create_test.go(1 hunks)internal/api/services/payment_initiations_retry.go(1 hunks)internal/api/services/payment_initiations_retry_test.go(1 hunks)internal/api/v2/handler_transfer_initiations_get.go(2 hunks)internal/api/v2/handler_transfer_initiations_list.go(2 hunks)internal/connectors/engine/activities/schedule_client_generated.go(0 hunks)internal/connectors/engine/engine.go(3 hunks)internal/connectors/engine/engine_generated.go(2 hunks)internal/connectors/engine/plugins/plugin_generated.go(0 hunks)internal/connectors/engine/workers.go(1 hunks)internal/connectors/engine/workflow/create_payout.go(1 hunks)internal/connectors/engine/workflow/create_payout_test.go(2 hunks)internal/connectors/engine/workflow/create_transfer.go(1 hunks)internal/connectors/engine/workflow/create_transfer_test.go(2 hunks)internal/models/payment_initiation_adjusments_status.go(3 hunks)
💤 Files with no reviewable changes (3)
- internal/connectors/engine/plugins/plugin_generated.go
- internal/connectors/engine/activities/schedule_client_generated.go
- internal/api/backend/backend_generated.go
🧰 Additional context used
🧬 Code Definitions (12)
internal/api/services/payment_initiations_create.go (2)
internal/api/services/payment_initiations_approve.go (1) (1)
s(14-64)internal/api/services/payment_initiations_retry.go (2) (2)
s(13-54)s(67-95)
internal/api/v2/handler_transfer_initiations_get.go (1)
internal/models/payment_initiation_adjusments_status.go (3) (3)
status(128-128)PaymentInitiationAdjustmentStatus(10-10)PAYMENT_INITIATION_ADJUSTMENT_STATUS_SCHEDULED_FOR_PROCESSING(22-22)
internal/api/services/payment_initiations_approve.go (1)
internal/api/services/payment_initiations_create.go (1) (1)
s(9-46)
internal/api/services/payment_initiations_create_test.go (2)
internal/connectors/engine/workflow/create_transfer.go (1) (1)
CreateTransfer(14-18)internal/connectors/engine/workflow/create_payout.go (1) (1)
CreatePayout(14-18)
internal/connectors/engine/workflow/create_transfer.go (2)
internal/models/payment_initiation_adjusments_status.go (2) (2)
err(129-129)PAYMENT_INITIATION_ADJUSTMENT_STATUS_SCHEDULED_FOR_PROCESSING(22-22)internal/connectors/engine/workflow/create_payout.go (2) (2)
w(20-40)w(42-206)
internal/api/services/payment_initiations_approve_test.go (2)
internal/api/services/payment_initiations_approve.go (1) (1)
s(14-64)internal/api/services/payment_initiations_create.go (1) (1)
s(9-46)
internal/api/services/payment_initiations_retry.go (1)
internal/api/services/payment_initiations_create.go (1) (1)
s(9-46)
internal/connectors/engine/workflow/create_payout.go (1)
internal/connectors/engine/workflow/create_transfer.go (2) (2)
w(20-40)w(42-208)
internal/api/services/payment_initiations_retry_test.go (2)
internal/connectors/engine/workflow/create_transfer.go (1) (1)
CreateTransfer(14-18)internal/connectors/engine/workflow/create_payout.go (1) (1)
CreatePayout(14-18)
internal/connectors/engine/workflow/create_transfer_test.go (2)
internal/models/payment_initiation_adjusments_status.go (3) (3)
PAYMENT_INITIATION_ADJUSTMENT_STATUS_SCHEDULED_FOR_PROCESSING(22-22)PAYMENT_INITIATION_ADJUSTMENT_STATUS_PROCESSING(15-15)PAYMENT_INITIATION_ADJUSTMENT_STATUS_PROCESSED(16-16)internal/connectors/engine/workflow/create_transfer.go (2) (2)
RunCreateTransfer(210-210)CreateTransfer(14-18)
internal/connectors/engine/workflow/create_payout_test.go (3)
internal/models/payment_initiation_adjusments_status.go (3) (3)
PAYMENT_INITIATION_ADJUSTMENT_STATUS_SCHEDULED_FOR_PROCESSING(22-22)PAYMENT_INITIATION_ADJUSTMENT_STATUS_PROCESSING(15-15)PAYMENT_INITIATION_ADJUSTMENT_STATUS_PROCESSED(16-16)internal/models/plugin.go (2) (2)
CreatePayoutRequest(181-183)CreatePayoutResponse(185-192)internal/connectors/engine/workflow/create_payout.go (2) (2)
RunCreatePayout(208-208)CreatePayout(14-18)
internal/connectors/engine/engine_generated.go (2)
internal/connectors/engine/workflow/create_payout.go (1) (1)
CreatePayout(14-18)internal/connectors/engine/workflow/create_transfer.go (1) (1)
CreateTransfer(14-18)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Tests
🔇 Additional comments (31)
internal/connectors/engine/workers.go (1)
115-115: Good refactoring to remove unused parameter.Renaming the unused context parameter to
_is a good practice that avoids linter warnings while making it clear that the parameter is intentionally unused.internal/connectors/engine/workflow/create_payout.go (1)
55-76: Excellent implementation of payment scheduling feature.This change implements the scheduled payments feature by checking for future
ScheduledAttimestamps, creating a "SCHEDULED_FOR_PROCESSING" adjustment, and then usingworkflow.Sleepto delay execution until the scheduled time. The implementation aligns with the similar approach in thecreateTransfermethod and properly handles all necessary metadata.The new status clearly indicates that client approval has been received and the payment is waiting to be processed at the scheduled time.
internal/connectors/engine/engine.go (2)
47-47: Good API simplification by removing the startDelay parameter.Removing the
startDelayparameter from the interface methods is a good design decision since scheduling is now handled internally by the workflow using theScheduledAtfield on the payment initiation model. This change simplifies the API and makes it more intuitive for callers.Also applies to: 51-51
516-516: Correctly implemented method signatures matching the interface changes.The implementation methods have been properly updated to match the interface changes, removing the
startDelayparameter. This maintains consistency across the codebase.Also applies to: 634-634
internal/connectors/engine/engine_generated.go (1)
88-90: Successfully updated mock implementations to match the new method signatures.The mock implementations have been properly updated to reflect the removal of the
startDelayparameter from theCreateTransferandCreatePayoutmethods. This ensures that tests using these mocks will continue to work with the updated API.Also applies to: 97-99, 117-120, 126-129
internal/api/services/payment_initiations_retry.go (2)
40-40: Signature update correctly removes the startDelay parameterThe
CreateTransfermethod call has been updated to remove thestartDelayparameter, aligning with the PR's approach of handling scheduled payments differently. This change is consistent with the modifications in other service methods.
46-46: Signature update correctly removes the startDelay parameterSimilar to the
CreateTransfermethod, theCreatePayoutcall has been updated to remove thestartDelayparameter. This provides consistency across both payment initiation types.internal/api/services/payment_initiations_create.go (2)
32-32: Signature update correctly removes the startDelay parameterThe
CreateTransfermethod call has been updated to remove thestartDelayparameter, aligning with the PR's approach of handling scheduled payments within the workflow engine rather than at the API service level.
38-38: Signature update correctly removes the startDelay parameterThe
CreatePayoutmethod call has been updated to remove thestartDelayparameter, consistent with the changes inCreateTransferand maintaining uniformity across both payment types.internal/api/v2/handler_transfer_initiations_list.go (2)
53-62: Status translation for V2 API backward compatibilityThis change properly handles the new
SCHEDULED_FOR_PROCESSINGstatus by mapping it toPROCESSINGfor V2 API backward compatibility. The comment clearly explains the rationale, which aligns with the PR objectives of addressing the status discrepancy between API versions.The implementation uses a straightforward switch statement to handle the specific case while defaulting to the original behavior for other statuses, maintaining compatibility without disrupting existing functionality.
64-64: Updated to use the translated status valueThe status assignment has been correctly updated to use the newly computed
statusvariable, ensuring the status translation logic is properly applied to the response.internal/api/services/payment_initiations_create_test.go (2)
100-100: Test expectation updated to match new signatureThe test mock expectation has been updated to match the new signature of
CreateTransferthat no longer includes thestartDelayparameter, replacing it with the attempt count parameter value of1.
102-102: Test expectation updated to match new signatureThe test mock expectation for
CreatePayouthas been updated to use the specific value of1for the attempt count parameter instead ofgomock.Any(), aligning with the changes in the method signature and implementation.internal/api/services/payment_initiations_approve_test.go (5)
44-45: Updated ScheduledAt for scheduled payment testing.The test now uses a future time (current time + 1 hour) for the
ScheduledAtfield to properly test the scheduled payment functionality.
144-147: Correctly determine waitResult based on scheduling condition.This change implements conditional logic to set
waitResultbased on whether the payment initiation is scheduled for the future, which aligns with the updated implementation in thePaymentInitiationsApprovefunction.
152-153: Updated CreateTransfer call to use dynamic waitResult parameter.The test now correctly uses the
waitResultvariable based on scheduling conditions rather than a hardcoded value, ensuring that the test accurately reflects the production code behavior.
154-155: Updated CreatePayout call to use dynamic waitResult parameter.Similar to the CreateTransfer update, this ensures consistent behavior for payouts by using the dynamic
waitResultvalue determined by scheduling conditions.
159-159: Changed waitResult parameter in PaymentInitiationsApprove call from false to true.This change ensures that the test verifies the correct handling of the
waitResultparameter in the service method, allowing the test to validate that the method properly overrides this value when there's a scheduled date.internal/connectors/engine/workflow/create_payout_test.go (2)
7-7: Added time package import.Added import for the time package to support the new scheduled payout test functionality.
84-162: Added test for scheduled payout processing.This new test validates the payment initiation workflow when a payout is scheduled for a future time. It ensures:
- The workflow correctly creates an adjustment with
SCHEDULED_FOR_PROCESSINGstatus initially- After the scheduled time, it creates a
PROCESSINGadjustment- Finally, it creates a
PROCESSEDadjustment when the payment completes- The workflow handles the temporal sleep and transitions between states correctly
This comprehensively tests the new scheduled payment functionality, covering the entire state transition flow.
internal/api/services/payment_initiations_approve.go (3)
42-46: Improved handling of scheduled payments.This change correctly determines that payments scheduled for the future should not wait for results, overriding any user-provided
waitResultvalue. The code now:
- Checks if
ScheduledAtis non-zero and in the future- Sets
waitResult = falsein that case, since waiting isn't appropriate for scheduled payments- Includes helpful comments explaining the rationale
This eliminates the previous complexity of manual delay calculations and simplifies the logic.
50-50: Removed startDelay parameter from CreateTransfer call.The function now correctly passes the dynamic
waitResultvalue to the engine, allowing the workflow to handle the scheduling internally rather than using a delay mechanism in the API service.
56-56: Removed startDelay parameter from CreatePayout call.Similar to the CreateTransfer update, this change passes the dynamic
waitResultvalue to the engine for payouts, maintaining consistent behavior between different payment types.internal/api/v2/handler_transfer_initiations_get.go (3)
121-124: Added filtering for unsupported adjustment statuses in V2 API.This change enhances the V2 API to handle compatibility with newer status types by:
- Calling the new
translateStatusfunction to determine if an adjustment should be included- Skipping adjustments that shouldn't be exposed in the V2 API
This maintains backward compatibility while allowing the internal system to use new status types.
128-128: Updated Status field to use translated value.The code now uses the translated status from the
translateStatusfunction rather than directly using the status string, ensuring consistent status representation in the API response.
141-151: Added translateStatus function to handle V2 API compatibility.This new function ensures backward compatibility by:
- Taking a
PaymentInitiationAdjustmentStatusas input- Handling the new
SCHEDULED_FOR_PROCESSINGstatus which doesn't exist in V2- Returning an appropriate status string and a boolean indicating whether to include the adjustment
This is a clean approach to maintain API compatibility while allowing the internal system to evolve with new status types.
internal/models/payment_initiation_adjusments_status.go (3)
22-22: Clean addition of new payment status constant.The new constant,
PAYMENT_INITIATION_ADJUSTMENT_STATUS_SCHEDULED_FOR_PROCESSING, correctly extends the existing enum type for payment initiation adjustment statuses.
43-44: Properly implemented String() method for the new status.The method now correctly returns the string representation "SCHEDULED_FOR_PROCESSING" when the new status is encountered, maintaining consistency with the existing pattern.
69-70: Correctly updated status string parsing function.The
PaymentInitiationAdjustmentStatusFromStringfunction properly handles the new "SCHEDULED_FOR_PROCESSING" string value and returns the corresponding enum value.internal/api/services/payment_initiations_retry_test.go (1)
145-147: Correctly updated method signatures to remove the startDelay parameter.The test cases have been properly updated to reflect the removal of the
startDelayparameter from theCreateTransferandCreatePayoutmethods, which aligns with the changes made in the workflow implementation.internal/connectors/engine/workflow/create_transfer_test.go (1)
84-162: Comprehensive test for the new scheduling functionality.This test thoroughly validates the scheduled payment implementation by:
- Setting up a payment initiation with a future scheduled time
- Verifying the creation of an adjustment with
SCHEDULED_FOR_PROCESSINGstatus- Checking that all subsequent workflow steps execute correctly after the scheduled time
The test is well-structured and parallels the existing test case pattern while adding the necessary assertions for the new functionality.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #374 +/- ##
==========================================
- Coverage 72.87% 72.85% -0.02%
==========================================
Files 531 531
Lines 25817 25873 +56
==========================================
+ Hits 18813 18849 +36
- Misses 5987 6006 +19
- Partials 1017 1018 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
* fix(*): payment initiations fixes * add comments
Fixes
ScheduledAt new adjustments
Context
When creating a transfer/payout with a scheduled at in the future, we were creating the temporal workflow with the corresponding delay. But the corresponding transfer/payout was not in processing mode, and was still in waiting for validation.
What's new
I replaced the delay on the temporal workflow by a workflow.Sleep (and added some tests)
I added a new adjustment status called "SCHEDULED_FOR_PROCESSING" which will tell the client that their approval was done and that the payment is now in waiting mode
To keep the compatibility with the V2 api, this status is replaced by a PROCESSING status when getting adjustments with the v2 API
Blocking call when creating a transfer/payout in v2 with a scheduled at
Context
In the previous version of payments, we were not waiting for results when a client was creating a transfer/payout in the future thanks to the scheduledAt.
What's new
This PR introduces a fix for that, hence not waiting for temporal workflow to finish if we have a scheduledAt in the future
Related Payments of transfer in v2
What's new
This is a fix for the v2 openapi, the related payment status should be a PaymentStatus and not a TransferInitiationStatus
Fixes PMNT-71
Fixes PMNT-73
Fixes PMNT-74