Conversation
Summary of ChangesHello @erawat, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a robust and extensible system to automatically reconcile payment attempts that become stuck in a 'processing' status. By establishing a scheduled job and an event-driven service, the system can now proactively detect and resolve stalled transactions, significantly enhancing payment processing reliability. The architecture allows individual payment processor extensions to integrate their unique reconciliation logic, ensuring adaptability and ease of maintenance. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a robust and well-designed scheduled job for reconciling payment attempts that are stuck in a 'processing' state. The implementation is clean, leveraging services, DTOs, and Symfony events to create a flexible and extensible system. The code is accompanied by a comprehensive suite of unit tests, ensuring its reliability. My review found the code to be of high quality. I have one suggestion to simplify the code in the PaymentAttemptReconcileService by making better use of the APIv4 result object, which will improve conciseness and maintainability.
8b57253 to
e056c01
Compare
50738c3 to
fcdd2b0
Compare
Add core post-reconciliation pipeline that handles contribution completion, failure counting, and status transitions for handlers that report results via setAttemptResult().
fcdd2b0 to
5c2c2a9
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This is an excellent pull request that introduces a robust and extensible reconciliation pipeline for stuck payment attempts. The code is very well-structured, leveraging DTOs, Symfony events, and services to create a clean separation of concerns. The implementation is of high quality, with careful attention to performance (avoiding N+1 queries with batch pre-fetching), error handling, and edge cases like pay-later contributions. The addition of a comprehensive test suite covering the new service, DTOs, and events is commendable and ensures the reliability of this new core feature. I have reviewed the changes thoroughly and could not find any issues of medium or higher severity. The design is solid and the implementation is clean. Great work!
Overview
Adds a scheduled job to reconcile stuck payment attempts, now with a core post-reconciliation pipeline that handles contribution completion, failure counting, and status transitions. Handlers only need to check their processor API and report status — core handles everything else.
Before
processingstatusAfter
A daily scheduled job (
PaymentAttemptReconcile.Run) finds stuck attempts, dispatches events for each processor type, collects results, and runs a standard post-reconciliation pipeline:CompletedReconcileResult(subclass)ContributionCompletionServiceReconcileAttemptResult('completed')(base)failedfailure_count. If threshold exceeded AND NOT pay-later → mark contribution FailedcancelledunchangedDefault parameters:
processor_parameters=[Stripe,2],[GoCardless,6]— processor type and stuck threshold in daysbatch_size=100— max attempts to process per run (shared across all processors)max_retry_count=3— max failures before marking recurring contribution as FailedTechnical Details
Subclass polymorphism
CompletedReconcileResult extends ReconcileAttemptResult— overridesgetCompletionData()to return aCompletionDataVO (transactionId + optional feeAmount). The base class returns NULL.Core calls the polymorphic method:
Handler intent via type choice:
new CompletedReconcileResult($action, $transactionId, $feeAmount)→ core completes contributionnew ReconcileAttemptResult('completed', $action)→ handler completed it itself (opt-out)Batch pre-fetching
After collecting results from handlers, the service batch-fetches all required Contribution and ContributionRecur records in 2 queries (not N+1) before processing results.
maxRetryCounton the eventmaxRetryCountis passed to the event even though core handles all failure threshold logic. This is intentional for:ChargeInstalmentBatchEventwhich follows the same patternTwo integration patterns
Pattern A — PaymentAttempt-based (Stripe, Deluxe, future processors):
Handler checks API, maps status, returns result via
setAttemptResult(). Core handles everything else. Handler does NOT need to complete contributions, increment failure counts, or mark contributions Failed.Pattern B — Trigger-only (GoCardless):
Uses event as trigger + config source only. Never calls
setAttemptResult(). Manages its ownpayment_in_progressflow internally.How to implement a new processor handler
ReconcilePaymentAttemptBatchEvent::NAME$event->getAttempts()new CompletedReconcileResult($action, $transactionId, $feeAmount)— core completes contributionnew ReconcileAttemptResult('failed', $action)— core handles failure pipelinenew ReconcileAttemptResult('cancelled', $action)— core handles cancellationnew ReconcileAttemptResult('unchanged', $action)— no action$event->setAttemptResult($attemptId, $result)Comments
Dummyprocessor type — no external API dependencies.