CIVIMM-463: Add instalment charge scheduled job#14
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 scalable automated system for charging recurring instalment contributions. It establishes a core service responsible for identifying eligible payments and managing their lifecycle through a new 'processing' status, while leveraging an event-driven architecture to allow payment processor extensions to handle the actual payment execution. This design significantly reduces code duplication, improves consistency across payment processors, and lays the groundwork for future enhancements in recurring payment management. Highlights
Changelog
Ignored Files
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 new scheduled job for charging due instalments, a core part of the recurring payment processing system. The changes are extensive and well-structured, including a new InstalmentChargeService, a corresponding APIv3 endpoint, and the use of Symfony events to delegate charging to processor-specific extensions. The implementation correctly uses atomic updates to prevent race conditions and includes a comprehensive suite of new unit tests.
My review focuses on a potential issue in the payment retry logic. I've identified that contributions with a failed PaymentAttempt will be permanently skipped, preventing retries. I have provided suggestions across PaymentAttempt.php and InstalmentChargeService.php to address this by allowing failed attempts to be reused for retries.
The migration from XML-based schema definitions to the new civimix-schema with PHP entity types is also a significant and positive change in this PR.
c7d8e9c to
658d991
Compare
Add Job 2 that charges due instalment contributions by selecting eligible contributions, creating PaymentAttempt records, and dispatching Symfony events for processor-specific charging. Features: - InstalmentChargeService with explicit API4 joins for multi-level FK paths - ChargeInstalmentBatchEvent for processor extensions to subscribe to - ChargeInstalmentItem DTO for batch processing - PaymentAttempt status 'processing' with atomic transitions - NULL-safe failure_count comparison - Scheduled job via managed entity Schema changes: - Migrate to civimix-schema mixin for entity definitions - Add 'processing' status to PaymentAttempt
658d991 to
964136f
Compare
Overview
Implements Job 2 in the recurring payment processing system - a scheduled job that automatically charges due instalment contributions. The core extension handles contribution selection and PaymentAttempt management, while processor-specific charging is delegated to payment processor extensions (Stripe, GoCardless) via Symfony events.
Before
No automated mechanism to charge due instalments from recurring contributions. Each payment processor extension would need to implement its own selection logic, leading to code duplication and inconsistent behavior.
After
A unified scheduled job that:
Technical Details
Job Configuration
PaymentInstalmentChargeprocessor_type=Stripe,batch_size=500,max_retry_count=3Selection Query Criteria
The service selects contributions matching ALL of these:
contribution_status_idIN (Pending, Partially Paid)(total_amount - paid_amount) > 0(outstanding balance exists)receive_date <= NOW()(due date has passed)contribution_recur_id IS NOT NULL(linked to recurring)status != Cancelled,payment_token_id IS NOT NULLcontribution_recur.failure_count <= max_retry_count(NULL treated as 0)payment_processor_type.namefor processor-specific batchesPerformance Optimizations
ORDER BY receive_date ASCensures fair processingaddJoin()for reliabilityRace Condition Prevention
Symfony Event Dispatch
Payment processor extensions subscribe to
ChargeInstalmentBatchEvent:API Parameters
New PaymentAttempt Status
Added
'processing'status for in-flight charges:pending→processing(atomic transition when job claims it)processing→completed(webhook confirms success)processing→failed(webhook confirms failure)Entity Framework Migration
Upgraded from v1 to v2 via civix:
xml/schema/*.xmltoschema/*.entityType.phpsql/auto_install.sqlandsql/auto_uninstall.sqlCivix Regeneration
After schema changes, DAO files were regenerated using:
This regenerates:
CRM/Paymentprocessingcore/DAO/*.php- Data Access Objectspaymentprocessingcore.civix.php- Extension utilitiesNote: These files are auto-generated and should not be manually edited.
Core overrides
None
Comments
ChargeInstalmentBatchEventsubscribers in separate PRs