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 significant automation feature to CiviCRM by implementing a scheduled job that generates instalment contributions for recurring donations. This job streamlines the process of creating "invoice" records for non-membership recurring contributions that are due, ensuring timely record creation without manual intervention or reliance on processor-specific logic. It focuses solely on record generation, explicitly avoiding any payment processor communication, and includes robust checks for idempotency and membership exclusion. Highlights
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 scheduled job to generate instalments for recurring contributions, which is a valuable addition. The implementation is well-structured, featuring a dedicated service, an API endpoint, and a managed job definition. The inclusion of comprehensive unit tests is particularly commendable.
I have identified two high-severity issues. The first is a logic bug in how due contributions are queried, which could lead to instalments being processed a day late. The second concerns overly defensive code that can mask data problems and lead to incorrect behavior when checking for existing instalments. I've provided specific suggestions to address these points. With these changes, this will be a very solid feature.
Civi/Paymentprocessingcore/Service/InstalmentGenerationService.php
Outdated
Show resolved
Hide resolved
Civi/Paymentprocessingcore/Service/InstalmentGenerationService.php
Outdated
Show resolved
Hide resolved
e4000ea to
4fdc210
Compare
4fdc210 to
d37b3a1
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This is an excellent pull request that introduces a valuable new feature for generating instalments for recurring contributions. The implementation in InstalmentGenerationService is robust, with great attention to details like transaction atomicity, idempotency, and edge-case handling for date calculations. The new API endpoint and scheduled job are well-defined.
I'm particularly impressed by the comprehensive test suite, which covers a wide range of scenarios and ensures the reliability of the new service. The work to improve the PHPStan configuration with stubs for Api4 entities is also a significant contribution to the project's long-term code quality and maintainability.
I have one minor suggestion for improving code clarity, but overall this is a high-quality contribution.
Civi/Paymentprocessingcore/Service/InstalmentGenerationService.php
Outdated
Show resolved
Hide resolved
017eb9b to
00c0582
Compare
00c0582 to
aef0cf8
Compare
f92339b to
48736b0
Compare
Overview
Adds a scheduled job that creates Pending contribution records for each due In Progress recurring contribution not linked to a membership. No payment processor calls are made.
Also introduces a typed
RecurringContributionDTO, fixes PHPStan stub resolution, and adds AI playbook configuration for multi-agent development.Before
CLAUDE.mdwith no shared standards for other AI toolsAfter
Instalment generation
A new scheduled job (
InstalmentGenerator.Run) queries due In Progress recurring contributions, excludes membership-linked ones, checks idempotency, creates a Pending contribution, and advancesnext_sched_contribution_datewith correct end-of-month handling.Typed DTO boundary
RecurringContributionDTO::fromApiResult()converts CiviCRM's untyped Api4 arrays into a typed object at the service boundary, eliminating manual validation and "Cannot cast mixed" PHPStan errors.AI playbook
Restructured
CLAUDE.mdinto a thin wrapper referencing shared standards in.ai/. Added configs for Gemini, Codex, and Copilot so all AI tools follow the same conventions.Technical Details
DTO pattern:
RecurringContributionDTOvalidates required fields (id,contact_id,next_sched_contribution_date), applies defaults (frequency_unit→'month',frequency_interval→1), and guarantees types via a private constructor with a static factory method.Contribution.create over repeattransaction:
repeattransactioncopies from a template contribution and triggers side effects (receipts, hooks, tax). We need a clean Pending record with fields from the recurring contribution directly.Atomicity: Each instalment creation + schedule advance is wrapped in
CRM_Core_Transaction. If either fails, both roll back.End-of-month handling: Monthly advancement uses
first day of +N monththen clamps to the target month's last day, avoiding PHP DateTime overflow (Jan 31 + 1 month → Mar 3).AI playbook architecture: Tool-specific thin wrappers (
CLAUDE.md,AGENTS.md,GEMINI.md) reference shared standards in.ai/..claude/settings.jsonand slash commands (/pre-commit,/review) are tracked in the repo.settings.local.jsonis gitignored for per-developer overrides.Core overrides
None.
Comments
processor_type=Stripe,batch_size=500(both configurable)