diff --git a/.ai/ai-code-review.md b/.ai/ai-code-review.md new file mode 100644 index 0000000..15251fc --- /dev/null +++ b/.ai/ai-code-review.md @@ -0,0 +1,159 @@ + + +# AI Code Review Guide + +This document defines how **any AI tool** (Claude, Gemini, Codex, or others) should review code changes. The review process is tool-agnostic -- the diff is the universal input, the standards are shared. + +> **Standards reference:** [shared-development-guide.md](shared-development-guide.md) + +--- + +## 1. Review Modes + +### Pre-Push Review (Developer-Initiated) + +Review changes **before** pushing to remote. Works with any AI tool or interface (IDE, CLI, web chat). + +**Step 1: Generate the diff** + +Determine your base branch first (e.g., `master`, `main`, `develop`). Use the base branch your PR will target. + +```bash +# All committed changes on current branch vs base branch +git diff ...HEAD + +# Only staged changes (not yet committed) +git diff --cached + +# Only unstaged changes +git diff + +# Specific files +git diff ...HEAD -- path/to/file.php + +# Example with master as base +git diff master...HEAD +``` + +> **Tip:** For a complete review, check both committed changes (`git diff ...HEAD`) and any uncommitted work (`git diff`, `git diff --cached`). + +**Step 2: Request the review** + +Use this prompt template (adapt wording for your tool): + +``` +Review the following code changes against our project's coding standards +in .ai/shared-development-guide.md. + +Check for: +- Security issues (SQL injection, XSS, hardcoded secrets) +- Performance problems (N+1 queries, inefficient loops) +- Code quality (SRP, naming, error handling, type safety) +- Test coverage (are new features/fixes tested?) +- PHPStan compliance (proper types, no mixed where avoidable) +- Linting compliance (CiviCRM Drupal standards) + +For each issue, provide: +- Severity: BLOCKER / WARNING / SUGGESTION / QUESTION +- File and line reference +- What the issue is and why it matters +- Suggested fix + + +``` + +**Step 3: Act on feedback** + +- Fix BLOCKERs before pushing +- Address WARNINGs where practical +- Consider SUGGESTIONs for follow-up +- Answer QUESTIONs with code comments or commit messages + +### GitHub PR Review (Automated) + +Review changes **after** pushing, on the Pull Request. The AI tool reads the PR diff automatically (base branch is known from the PR target). + +Same checklist applies. The AI tool should post comments on specific lines. + +--- + +## 2. Review Checklist + +### Standards Compliance +- [ ] Commit messages follow `CIVIMM-###:` format +- [ ] PR uses `.github/PULL_REQUEST_TEMPLATE.md` +- [ ] All required sections filled (Overview, Before, After, Technical Details) +- [ ] No AI attribution in commits + +### Security +- [ ] No hardcoded secrets, API keys, or credentials +- [ ] Parameterized queries (no SQL injection) +- [ ] User input sanitized before rendering (no XSS) +- [ ] Webhook signatures verified +- [ ] Authentication/authorization on API endpoints +- [ ] Sensitive files not committed (`civicrm.settings.php`, `.env`) + +### Performance +- [ ] No N+1 query issues +- [ ] No inefficient loops over large datasets +- [ ] Unnecessary API calls avoided +- [ ] Database queries optimized + +### Code Quality +- [ ] Single responsibility principle followed +- [ ] Meaningful names following project conventions +- [ ] Proper exception handling +- [ ] Return type declarations on service methods +- [ ] Dependency injection used +- [ ] Proper types in PHPDoc (no `mixed` where avoidable) +- [ ] No `assert()` in production code + +### Testing +- [ ] New features and bug fixes include tests +- [ ] Tests cover positive, negative, and edge cases +- [ ] No tests removed or weakened +- [ ] Error message changes reflected in test assertions + +### Static Analysis & Linting +- [ ] Code passes PHPStan level 9 +- [ ] Coding standards followed +- [ ] Files end with newlines +- [ ] `@phpstan-param` / `@phpstan-var` used where linter and PHPStan conflict + +--- + +## 3. Review Severity Levels + +| Level | Meaning | Action Required | +|-------|---------|-----------------| +| **BLOCKER** | Security vulnerability, data loss risk, broken functionality | Must fix before merge | +| **WARNING** | Affects quality, performance, or maintainability | Should fix before merge | +| **SUGGESTION** | Optional improvement | Nice to have, can be follow-up | +| **QUESTION** | Needs clarification from the author | Author must respond | + +--- + +## 4. Review Feedback Principles + +- **Be specific** -- reference file paths and line numbers +- **Explain why** -- not just what to change, but why it matters +- **Think critically** -- don't suggest changes that contradict architectural decisions +- **Consider implications** -- type changes, database constraints, performance trade-offs +- **Distinguish severity** -- not everything is a blocker +- **Be constructive** -- suggest fixes, not just problems + +--- + +## 5. Cross-Tool Review Workflow + +Any combination works: + +| Developer Tool | Reviewer Tool | How | +|---------------|---------------|-----| +| Claude | Gemini | Push branch, Gemini reviews PR or diff | +| Gemini | Claude | Feed diff to Claude in new session | +| Human | Any AI | Feed diff or create PR | +| Codex | Claude/Gemini | Push branch, other tool reviews | +| Any tool | Same tool (new session) | Feed diff to fresh session for unbiased review | + +**Best practice:** Use a **different tool or session** for review than was used for development. Fresh context catches more issues. diff --git a/.ai/civicrm.md b/.ai/civicrm.md new file mode 100644 index 0000000..4e1b328 --- /dev/null +++ b/.ai/civicrm.md @@ -0,0 +1,225 @@ + + +# CiviCRM Core Reference + +This file provides CiviCRM-specific patterns and conventions for AI tools and developers. + +> **Extension structure & testing:** See [extension.md](extension.md) for extension directory layout, schema changes, and testing patterns. + +--- + +## 1. Namespaces + +| Namespace | Directory | Purpose | +|-----------|-----------|---------| +| `CRM_*` | `CRM/` | Traditional CiviCRM (DAO, BAO, Pages, Forms) | +| `Civi\Paymentprocessingcore\*` | `Civi/` | Modern services, hooks, factories | + +--- + +## 2. CiviCRM API Usage + +### Prefer API4 Over API3 + +API4 is the modern CiviCRM API with better type safety and cleaner syntax. Use it for all new code. + +```php +// API4 with permission bypass for internal/IPN operations +$contribution = \Civi\Api4\Contribution::get(FALSE) + ->addSelect('id', 'contribution_page_id', 'contribution_status_id:name') + ->addWhere('id', '=', $contributionId) + ->execute() + ->first(); + +// API3 (legacy - avoid in new code) +$contribution = civicrm_api3('Contribution', 'getsingle', [ + 'id' => $contributionId, + 'return' => ['id', 'contribution_page_id'], +]); +``` + +### Key API4 Differences +- `FALSE` as first parameter bypasses permission checks +- API3 requires `'check_permissions' => 0` (easy to forget) +- API4 uses `snake_case` field names consistently +- API4 status fields use pseudoconstant syntax: `contribution_status_id:name` + +### When to Bypass Permissions (API4 `FALSE`) +- IPN/webhook handlers (anonymous context) +- Return URLs from payment processors +- Internal service operations +- Background processing jobs + +### When API3 Is Acceptable +- Entity not yet available in API4 (rare) +- Maintaining consistency with existing code in same method +- `Payment.create` API (commonly used, works well) + +--- + +## 3. API4 Result Handling & PHPStan + +API4's `->first()` and `->single()` return `mixed`. Use these patterns for PHPStan compliance: + +### In Services - `is_array()` Guard +```php +$result = \Civi\Api4\PaymentToken::create(FALSE) + ->setValues($tokenParams) + ->execute() + ->first(); + +if (!is_array($result) || empty($result['id'])) { + throw new \CRM_Core_Exception('Failed to create payment token'); +} +return $result; +``` + +### In Tests - `assertNotNull()` Before Accessing +```php +$updated = \Civi\Api4\ContributionRecur::get(FALSE) + ->addSelect('payment_token_id') + ->addWhere('id', '=', $recurId) + ->execute() + ->first(); + +$this->assertNotNull($updated); +$this->assertEquals($tokenId, $updated['payment_token_id']); +``` + +### Avoid These Patterns +```php +// BAD: Inline @var annotations - not the project pattern +/** @var array{id: int}|null $result */ +$result = $api->execute()->first(); + +// BAD: ->single() throws if not exactly 1 result +// Use ->first() with is_array() check instead +$result = $api->execute()->single(); +``` + +--- + +## 4. PHPStan Stub Files for Dynamic Classes + +CiviCRM Api4 entity classes are **dynamically generated at runtime** and have no physical PHP files. Use stub files in `stubs/`: + +```php +// stubs/CiviApi4.stub.php +namespace Civi\Api4; + +use Civi\Api4\Generic\DAOGetAction; +use Civi\Api4\Generic\DAOCreateAction; +use Civi\Api4\Generic\DAOUpdateAction; +use Civi\Api4\Generic\DAODeleteAction; + +/** + * @method static DAOGetAction get(bool $checkPermissions = TRUE) + * @method static DAOCreateAction create(bool $checkPermissions = TRUE) + * @method static DAOUpdateAction update(bool $checkPermissions = TRUE) + * @method static DAODeleteAction delete(bool $checkPermissions = TRUE) + */ +class Activity { + +} +``` + +**Rules:** +- Do NOT use `extends AbstractEntity` in stubs (causes CI errors) +- Add new Api4 entities to the stub file when first used +- Stubs are loaded via `scanFiles` in both `phpstan.neon` and CI-generated `phpstan-ci.neon` +- Must use `scanFiles` (not `stubFiles`) because `stubFiles` are overridden when `scanDirectories` loads the `Civi\Api4` namespace from core + +--- + +## 5. Hooks and Integration Points + +Common CiviCRM hooks used in this extension: + +| Hook | Purpose | +|------|---------| +| `civicrm_buildForm` | Inject scripts/templates into forms | +| `civicrm_validateForm` | Validate form submissions | +| `civicrm_permission` | Register custom permissions | +| `civicrm_permission_check` | Grant permissions dynamically | +| `civicrm_container` | Register services with DI container | +| `civicrm_postCommit` | Handle post-transaction tasks | +| `civicrm_check` | System status checks | + +--- + +## 6. Service Registration + +Services are registered via Symfony DI container in hook implementations: + +```php +// In Civi/Paymentprocessingcore/Hook/Container/ServiceContainer.php +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Definition; + +class ServiceContainer { + public function register(ContainerBuilder $container): void { + $definition = new Definition(MyService::class); + $definition->addArgument(new Reference('other.service')); + $container->setDefinition('my.service', $definition); + } +} +``` + +Usage: +```php +$service = \Civi::service('my.service'); +``` + +--- + +## 7. Common CiviCRM Commands + +```bash +# Enable extension +cv en paymentprocessingcore + +# Disable extension +cv dis paymentprocessingcore + +# Upgrade extension +cv api Extension.upgrade + +# Clear cache +cv flush + +# Run cv commands via Docker +./scripts/run.sh cv api Contact.get +./scripts/run.sh shell # Shell into container +``` + +--- + +## 8. PHPStan vs Linter Compatibility + +The Drupal linter and PHPStan have conflicting requirements for array type annotations. + +**Problem:** `@var array` fails linter, but `@var array` fails PHPStan. + +**Solution:** Use `@phpstan-var` / `@phpstan-param` which PHPStan reads but linter ignores: + +```php +/** + * @param array $params + * @phpstan-param array $params + */ +public function process(array $params): void { + // ... +} + +/** + * {@inheritdoc} + * + * @phpstan-var array + */ +protected static $defaultParams = [ + 'financial_type_id' => 'Donation', + 'frequency_interval' => 1, +]; +``` + +For inherited properties, use `{@inheritdoc}` plus `@phpstan-var` for the specific type. diff --git a/.ai/extension.md b/.ai/extension.md new file mode 100644 index 0000000..4072a57 --- /dev/null +++ b/.ai/extension.md @@ -0,0 +1,141 @@ + + +# CiviCRM Extension Development + +This file covers extension-specific structure, schema management, and testing patterns. + +> **CiviCRM core patterns:** See [civicrm.md](civicrm.md) for API4, hooks, services, and PHPStan patterns. + +--- + +## 1. Extension Structure + +``` +io.compuco.paymentprocessingcore/ + info.xml # Extension metadata, dependencies, version + paymentprocessingcore.php # Hook implementations entry point + paymentprocessingcore.civix.php # Auto-generated CiviX boilerplate (DO NOT EDIT) + CRM/ # Traditional CiviCRM namespace (CRM_*) + Paymentprocessingcore/ + DAO/ # Database Access Objects (auto-generated from XML) + BAO/ # Business Access Objects (business logic) + Page/ # UI pages + Form/ # Form handlers + Hook/ # Hook implementations + Helper/ # Constants and helper classes + Civi/ # Modern namespace (Civi\Paymentprocessingcore\*) + Paymentprocessingcore/ + Service/ # Business logic services + DTO/ # Data Transfer Objects (typed API4 boundary) + Utils/ # Utility classes + Helper/ # Traits for shared functionality + Hook/ # Modern hook implementations + Factory/ # Factory classes + Exception/ # Custom exception classes + xml/schema/ # Entity schema definitions + xml/Menu/ # Menu definitions + sql/ # Database schema and upgrade scripts + templates/ # Smarty templates for UI + js/ # JavaScript files + css/ # Stylesheets + tests/phpunit/ # PHPUnit tests (mirrors source structure) + stubs/ # PHPStan stub files for dynamic classes + scripts/ # Dev scripts (run.sh, lint.sh) + phpstan.neon # PHPStan configuration + phpcs-ruleset.xml # PHPCS linting rules +``` + +--- + +## 2. Key Services + +| Service | Purpose | +|---------|---------| +| `ContributionCompletionService` | Complete pending contributions | +| `ContributionFailureService` | Handle failed contributions | +| `InstalmentGenerationService` | Generate instalment contributions for recurring | +| `WebhookQueueService` | Queue webhook events | +| `WebhookHandlerRegistry` | Map events to handlers | +| `PaymentProcessorCustomerService` | Manage customer IDs | + +--- + +## 3. Webhook Handler Pattern + +Handlers can implement `WebhookHandlerInterface` directly (preferred) or use duck typing (fallback). The registry uses the Adapter pattern for duck-typed handlers. + +```php +// Option 1: Implement interface (preferred) +class MyHandler implements WebhookHandlerInterface { + public function handle(int $webhookId, array $params): string { + return 'applied'; + } +} + +// Option 2: Duck typing (fallback - wrapped in Adapter) +class MyHandler { + public function handle(int $webhookId, array $params): string { + return 'applied'; + } +} +``` + +--- + +## 4. DTO Pattern + +Use Data Transfer Objects at the boundary between CiviCRM's untyped Api4 arrays and typed service code: + +```php +// Private constructor with static factory +$dto = RecurringContributionDTO::fromApiResult($apiRecord); + +// Type-safe access via getters +$dto->getId(); +$dto->getAmount(); +$dto->getCampaignId(); // nullable +``` + +--- + +## 5. Database Schema Changes + +When modifying entities in `xml/schema/`: + +```bash +# Regenerate DAO files using Docker test environment +./scripts/run.sh setup # One-time setup +./scripts/run.sh civix # Regenerate DAO files +``` + +**Notes:** +- DAO files are auto-regenerated during extension installation/upgrade +- Always regenerate DAO files after modifying XML schemas +- Never edit DAO files manually + +--- + +## 6. Testing Patterns + +- Extend `BaseHeadlessTest` for all test classes +- Use fabricators in `tests/phpunit/Fabricator/` to create test data +- Mock external API calls where applicable +- Test positive, negative, and edge cases +- Store tests mirroring source structure + +```php +class MyServiceTest extends BaseHeadlessTest { + public function testSuccessCase(): void { + // Arrange + $fabricator = new MyEntityFabricator(); + $entity = $fabricator->fabricate(); + + // Act + $result = $this->service->process($entity['id']); + + // Assert + $this->assertNotNull($result); + $this->assertEquals('expected', $result['status']); + } +} +``` diff --git a/.ai/shared-development-guide.md b/.ai/shared-development-guide.md new file mode 100644 index 0000000..a47d771 --- /dev/null +++ b/.ai/shared-development-guide.md @@ -0,0 +1,302 @@ + + + + + +# Development Standards + +--- + +## 1. Quick Reference + +| Resource | Location | +|----------|----------| +| Project Overview | [README.md](../README.md) | +| PR Template | `.github/PULL_REQUEST_TEMPLATE.md` | +| Linting Rules | `phpcs-ruleset.xml` | +| PHPStan Config | `phpstan.neon` | +| CiviCRM Patterns | [.ai/civicrm.md](civicrm.md) | +| Extension Development | [.ai/extension.md](extension.md) | +| AI Code Review | [.ai/ai-code-review.md](ai-code-review.md) | + +--- + +## 2. Development Environment + +### Running Tests, Linter, PHPStan + +```bash +# Setup Docker environment (one-time) +./scripts/run.sh setup + +# Run all tests +./scripts/run.sh tests + +# Run specific test +./scripts/run.sh test tests/phpunit/path/to/Test.php + +# Run linter on changed files +./scripts/lint.sh check + +# Auto-fix linting issues +./scripts/lint.sh fix + +# Run PHPStan on changed files +./scripts/run.sh phpstan-changed + +# Run PHPStan on full codebase +./scripts/run.sh phpstan + +# Regenerate DAO files after schema changes +./scripts/run.sh civix +``` + +--- + +## 3. Pull Request Guidelines + +### PR Title Format +``` +CIVIMM-###: Short description +``` + +### Required Sections (from template) +- **Overview**: Non-technical description +- **Before**: Current state with screenshots/gifs where appropriate +- **After**: What changed with screenshots/gifs where appropriate +- **Technical Details**: Noteworthy technical changes, code snippets +- **Core overrides**: If any CiviCRM core files are patched +- **Comments**: Any additional notes for reviewers + +### When Drafting PRs +- Reference the ticket ID in the PR title +- Fill all required template sections +- Keep summaries factual — avoid assumptions +- Include before/after screenshots for UI changes + +--- + +## 4. Handling Pull Request Review Feedback + +**NEVER blindly implement feedback.** Always think critically and ask questions. + +### Required Process + +1. **Analyze Each Suggestion:** + - Does this suggestion make technical sense? + - What are the implications (database constraints, type safety, performance)? + - Could this break existing functionality? + - Is this consistent with the project's architecture? + +2. **Ask Clarifying Questions:** + - If unsure about the reasoning, ask: "Why is this change recommended?" + - If there are trade-offs, present them: "This suggestion would fix X but might break Y - which is preferred?" + - If the suggestion seems incorrect, explain why: "I think this might cause issues because..." + +3. **Explain Your Analysis:** + - For each change, explain WHY you're making it (or not making it) + - Present technical reasoning + - Highlight potential issues + +4. **Get Approval Before Implementing:** + - Show what you plan to change + - Wait for explicit confirmation before committing + - Never batch commit multiple review changes without review + +### Red Flags - Stop and Ask Questions +- Changes that affect database constraints (NOT NULL, foreign keys) +- Changes to type checking logic (null checks, empty checks, isset) +- Suggestions that seem to contradict architectural decisions +- "Consistency" arguments without technical justification +- Changes that would alter existing behavior +- Automated tool suggestions without context + +--- + +## 5. Commit Message Convention + +``` +CIVIMM-###: Short imperative description +``` + +**Rules:** +- Keep under 72 characters +- Use present tense ("Add", "Fix", "Refactor") +- Include the correct issue key when committing +- Be specific and descriptive +- **DO NOT add any AI attribution or co-authorship lines** + +**Examples:** +``` +CIVIMM-456: Add PaymentAttempt entity with multi-processor support +CIVIMM-789: Fix webhook deduplication race condition +CIVIMM-101: Refactor PaymentService to use dependency injection +``` + +--- + +## 6. Code Quality Standards + +### Unit Testing +- **Mandatory** for all new features and bug fixes +- Never modify or skip tests just to make them pass. Fix the underlying code. +- Test positive, negative, and edge cases +- See framework-specific docs for test base classes and patterns + +### Linting (PHPCS) +- Follow project coding standards (`phpcs-ruleset.xml`) +- All files must end with a newline +- Run `./scripts/lint.sh fix` to auto-fix issues + +### Static Analysis (PHPStan) +- Never regenerate baseline to "fix" errors - fix the code +- Goal is to **reduce** the baseline over time, not grow it +- Use `@phpstan-param` for generic types that linter doesn't support + +### PHPStan Baseline Management + +- The baseline (`phpstan-baseline.neon`) tracks pre-existing errors +- **Never add new entries** to work around errors — fix the code or add stubs +- Regenerate baseline after fixing code: + ```bash + ./scripts/run.sh shell + cd /path/to/extension + php /tmp/phpstan.phar analyse -c phpstan.neon --generate-baseline=phpstan-baseline.neon + ``` +- `reportUnmatchedIgnoredErrors: false` is set because local and CI scan different paths + +### PHPStan CI vs Local Configuration + +- **Local**: `phpstan.neon` with Docker paths (`/build/site/...`) +- **CI**: `phpstan-ci.neon` generated in `.github/workflows/phpstan.yml` with CI paths (`$GITHUB_WORKSPACE/site/...`) +- Both must stay in sync for `level`, `scanFiles`, `excludePaths`, `reportUnmatchedIgnoredErrors`, and `includes` +- When changing `phpstan.neon`, always check if the CI workflow needs the same change + +### Linter + PHPStan Compatibility Pattern +```php +/** + * @param array $params // Linter sees this + * @phpstan-param array $params // PHPStan sees this + */ +``` + +--- + +## 7. Critical Coding Guidelines + +### Security +- Never log or expose API keys, tokens, or sensitive data +- Validate all input amounts and parameters before processing +- Use parameterized queries (prevent SQL injection) +- Sanitize all user input before rendering (XSS prevention) +- Verify webhook signatures +- Ensure proper authentication/authorization for API endpoints + +### Performance +- Identify N+1 query issues +- Detect inefficient loops or algorithms +- Avoid unnecessary API calls (use cached records) +- Recommend caching for expensive operations +- Review database queries for optimization + +### Code Quality +- Services should follow single responsibility principle +- Use meaningful, descriptive names +- Handle exceptions properly (use custom exception classes) +- All service methods should have proper return type declarations +- Use dependency injection for service dependencies +- **Use proper types instead of `mixed`** in PHPDoc annotations +- **Never use `assert()` in production code** - use proper type checks with exceptions + +--- + +## 8. CI Requirements + +All code must pass before merging: + +| Check | Command | CI Workflow | +|-------|---------|-------------| +| Tests | `./scripts/run.sh tests` | `unit-test.yml` | +| Linting | `./scripts/lint.sh check` | `linters.yml` | +| PHPStan | `./scripts/run.sh phpstan-changed` | `phpstan.yml` | + +--- + +## 9. Git Discipline + +**NEVER use `git add -A` or `git add .`** - always add specific files: +```bash +# BAD +git add -A +git add . + +# GOOD +git add Civi/Service/MyService.php tests/phpunit/Civi/Service/MyServiceTest.php +``` + +**Always run linter before committing:** +```bash +./scripts/lint.sh check +git add +git commit -m "CIVIMM-###: Description" +``` + +**Always run tests before committing code changes:** +- When modifying source files, run `./scripts/run.sh tests` BEFORE committing +- When modifying error messages, verify affected tests expect the new message + +--- + +## 10. Safety Rules + +- Never commit code without running **tests** and **linting** +- Never remove or weaken tests to make them pass +- Never push commits automatically without human review +- If unsure, stop and consult a senior developer + +### Sensitive Files (Never Commit) +- `civicrm.settings.php` (contains API keys) +- `.env` files +- Any files with credentials or secrets + +### Auto-Generated Files (Do Not Edit Manually) +- `paymentprocessingcore.civix.php` (regenerate with `civix`) +- `CRM/Paymentprocessingcore/DAO/*.php` (regenerate from XML schemas) +- Files in `xml/schema/*.entityType.php` (auto-generated) + +--- + +## 11. Pre-Merge Validation Checklist + +| Check | Requirement | +|-------|-------------| +| Tests pass | All green in CI | +| Linting passes | No violations | +| PHPStan passes | Static analysis clean on changed files | +| Commit prefix | Uses CIVIMM-### format | +| PR template used | `.github/PULL_REQUEST_TEMPLATE.md` completed | +| No sensitive data | No API keys or credentials in code | +| Code reviewed | At least one approval from team member | + +--- + +## 12. Common Commands + +```bash +# Git +git status && git diff + +# Testing +./scripts/run.sh setup # One-time Docker setup +./scripts/run.sh tests # Run all tests +./scripts/run.sh test path/to/Test.php # Run specific test + +# Code Quality +./scripts/lint.sh check # Check linting +./scripts/lint.sh fix # Auto-fix linting +./scripts/run.sh phpstan-changed # PHPStan on changed files + +# CiviCRM +./scripts/run.sh civix # Regenerate DAO files +./scripts/run.sh shell # Shell into container +``` diff --git a/.claude/commands/pre-commit.md b/.claude/commands/pre-commit.md new file mode 100644 index 0000000..a37f0d0 --- /dev/null +++ b/.claude/commands/pre-commit.md @@ -0,0 +1,10 @@ +Check my changes are ready to commit. + +1. Run `git diff --cached` to see staged changes +2. Verify commit message follows `CIVIMM-###:` format +3. Check for sensitive files (civicrm.settings.php, .env) +4. Check for auto-generated files that shouldn't be edited manually +5. Verify files end with newlines +6. Flag any issues before committing + +$ARGUMENTS \ No newline at end of file diff --git a/.claude/commands/review.md b/.claude/commands/review.md new file mode 100644 index 0000000..e2e020d --- /dev/null +++ b/.claude/commands/review.md @@ -0,0 +1,11 @@ +Review my current changes against the project coding standards. + +The user may provide a base branch name as an argument. If not provided, ask which base branch to diff against before proceeding. + +1. Run `git diff ...HEAD` to see all committed changes on this branch +2. Also run `git diff` and `git diff --cached` to catch any uncommitted/staged changes +3. Apply the review checklist from .ai/ai-code-review.md +4. Flag issues by severity: BLOCKER, WARNING, SUGGESTION, QUESTION +5. Reference specific file paths and line numbers + +$ARGUMENTS \ No newline at end of file diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 0000000..754414b --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,17 @@ +{ + "permissions": { + "allow": [ + "Bash(git status*)", + "Bash(git diff*)", + "Bash(git log*)", + "Bash(git branch*)", + "Bash(./scripts/run.sh *)", + "Bash(./scripts/lint.sh *)" + ], + "deny": [ + "Bash(git push*)", + "Bash(git reset --hard*)", + "Bash(rm -rf*)" + ] + } +} \ No newline at end of file diff --git a/.gemini/config.yaml b/.gemini/config.yaml new file mode 100644 index 0000000..95a3732 --- /dev/null +++ b/.gemini/config.yaml @@ -0,0 +1,41 @@ +# Gemini Code Assist configuration +# See: https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github + +# Files/directories Gemini should ignore during review +ignore_patterns: + - "paymentprocessingcore.civix.php" + - "CRM/Paymentprocessingcore/DAO/*.php" + - "xml/schema/*.entityType.php" + - "vendor/**" + - "node_modules/**" + - "*.min.js" + - "*.min.css" + +# Persistent memory for this repository (learns from past reviews) +memory_config: + disabled: false + +# Code review settings +code_review: + # Set to true to disable Gemini on PRs + disable: false + + # Minimum severity for review comments (LOW, MEDIUM, HIGH, CRITICAL) + comment_severity_threshold: MEDIUM + + # Maximum number of review comments (-1 for unlimited) + max_review_comments: -1 + + # What happens when a PR is opened + pull_request_opened: + # Post a help message explaining how to interact with Gemini + help: false + + # Post a PR summary (overview of changes) + summary: true + + # Post code review comments + code_review: true + + # Also review draft PRs + include_drafts: false diff --git a/.gemini/styleguide.md b/.gemini/styleguide.md new file mode 100644 index 0000000..b7d1cae --- /dev/null +++ b/.gemini/styleguide.md @@ -0,0 +1,56 @@ +# Code Review Style Guide + +This style guide defines how Gemini Code Assist should review Pull Requests. + +## Standards + +Follow all standards in [.ai/shared-development-guide.md](../.ai/shared-development-guide.md). + +For CiviCRM extensions, also follow [.ai/civicrm.md](../.ai/civicrm.md) and [.ai/extension.md](../.ai/extension.md). + +For the full review checklist and severity definitions, see [.ai/ai-code-review.md](../.ai/ai-code-review.md). + +## Review Focus + +### Must Check (BLOCKER if violated) +- No hardcoded secrets, API keys, or credentials in code +- No SQL injection (use parameterized queries) +- No XSS (sanitize user input before rendering) +- Webhook signatures verified +- No auto-generated files edited manually (`paymentprocessingcore.civix.php`, `CRM/Paymentprocessingcore/DAO/*.php`) +- No sensitive files committed (`civicrm.settings.php`, `.env`) +- Tests not removed or weakened to pass + +### Should Check (WARNING) +- Commit messages follow `CIVIMM-###:` format +- New features and bug fixes include unit tests +- PHPStan level 9 compliance (proper types, no `mixed` where avoidable) +- `is_array()` guard on API4 `->first()` results +- `@phpstan-param` / `@phpstan-var` used where linter and PHPStan conflict +- No `assert()` in production code +- No N+1 query issues +- Services follow single responsibility principle +- Dependency injection used for service dependencies +- No AI attribution in commits + +### Nice to Have (SUGGESTION) +- Code readability improvements +- Better variable naming +- Additional edge case test coverage +- Documentation improvements + +## Severity Labels + +Use these in review comments: +- **BLOCKER**: Must fix before merge (security, data loss, broken functionality) +- **WARNING**: Should fix (quality, performance, maintainability) +- **SUGGESTION**: Optional improvement +- **QUESTION**: Needs clarification from the author + +## Review Style + +- Be specific -- reference file paths and line numbers +- Explain **why** something is an issue, not just what to change +- Think critically -- don't suggest changes that contradict architectural decisions +- Consider implications of type changes, database constraints, performance trade-offs +- Distinguish severity clearly -- not everything is a blocker diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 0000000..3794d82 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,76 @@ + + +# GitHub Copilot PR Review Guide + +This file defines how **GitHub Copilot** assists in pull request reviews for this repository. +Copilot must follow our engineering standards, CI workflows, and commit conventions when reviewing code. + +--- + +## 1. Review Objectives + +Copilot should: + +- Verify code quality, readability, and maintainability +- Confirm compliance with CI (PHPStan, PHPUnit, Linters) +- Ensure PRs follow commit, testing, and documentation guidelines +- Never auto-approve — only provide actionable feedback + +--- + +## 2. Pull Request Format + +All PRs must follow `.github/PULL_REQUEST_TEMPLATE.md`. + +**Checklist:** + +- PR title includes issue key (e.g., `CIVIMM-123: Fix summary bug`) +- All template sections (Overview, Before, After, Technical Details) are completed +- Linked to correct issue + +If incomplete, Copilot should suggest precise edits or missing fields. + +--- + +## 3. Review Checklist + +| Category | Requirement | Example Feedback | +|----------------|--------------------------------------|---------------------------------------------------------------------| +| **Code Quality** | Clear, maintainable logic | "Consider extracting this logic into a helper." | +| **Testing** | PHPUnit tests included and passing | "Missing test for `InstalmentGenerationService::createInstalment()`." | +| **Static Analysis** | PHPStan passes at CI level | "Check for PHPStan level 9 compliance." | +| **Style** | Follows PSR-12 & naming conventions | "Rename `$obj` to `$contactData` for clarity." | +| **Docs** | Public methods include PHPDoc | "Add PHPDoc for `ContributionCompletionService::complete()`." | + +--- + +## 4. Critical Review Areas + +### Security + +- Detect hardcoded secrets or API keys +- Check for SQL injection and XSS +- Validate user input & sanitize output +- Review authentication/authorization logic + +### Performance + +- Identify N+1 query issues +- Detect inefficient loops or algorithms +- Spot memory leaks or unfreed resources +- Recommend caching for expensive ops + +### Code Quality + +- Functions should be focused and testable +- Use meaningful, descriptive names +- Handle errors properly + +--- + +## 5. Review Style Tips + +- Be specific and actionable +- Explain the "why" behind your suggestions +- Acknowledge good patterns +- Ask clarifying questions if intent is unclear diff --git a/.github/workflows/phpstan.yml b/.github/workflows/phpstan.yml index 0a64562..571a324 100644 --- a/.github/workflows/phpstan.yml +++ b/.github/workflows/phpstan.yml @@ -65,7 +65,7 @@ jobs: - name: Run PHPStan working-directory: ${{ env.CIVICRM_EXTENSIONS_DIR }}/io.compuco.paymentprocessingcore run: | - # Create phpstan.neon with CI-specific paths + # Create phpstan-ci.neon with CI-specific paths cat > phpstan-ci.neon < + +# Codex Development Guide + +> **Shared standards:** Read [.ai/shared-development-guide.md](.ai/shared-development-guide.md) for all coding standards, CI requirements, commit conventions, and best practices. +> +> **CiviCRM reference:** See [.ai/civicrm.md](.ai/civicrm.md) for CiviCRM core patterns and [.ai/extension.md](.ai/extension.md) for extension structure and testing. +> +> **Code review:** See [.ai/ai-code-review.md](.ai/ai-code-review.md) for review checklist and process. + +This file contains **Codex-specific** instructions only. + +--- + +## Codex Environment Constraints + +Codex runs in a **sandboxed environment** with limited capabilities: + +- No network access during execution +- Cannot run Docker commands (`./scripts/run.sh` will not work) +- Cannot access CI workflows directly +- Can read all files in the repository +- Can write and modify files + +--- + +## Codex-Specific Rules + +- **DO NOT add any AI attribution** to commits +- Follow the `CIVIMM-###:` commit format from the shared guide +- Since you cannot run tests, ensure code is correct by careful analysis +- Write tests alongside code changes +- Always flag verification steps the developer must run + +--- + +## Codex Workflow + +1. **Plan** -- Outline the approach before making changes +2. **Edit** -- Apply changes following all shared standards +3. **Flag** -- Note what the developer must verify + +### Always Remind the Developer To +```bash +./scripts/run.sh tests # Run tests before committing +./scripts/lint.sh check # Check linting before committing +./scripts/run.sh phpstan-changed # Check static analysis +``` + +--- + +## Codex for Code Review + +Codex can review code changes using the process in [.ai/ai-code-review.md](.ai/ai-code-review.md): + +- Feed `git diff master...HEAD` output for pre-push review +- Review PRs when integrated with GitHub +- Apply the same checklist and severity levels as any other tool +- **Never auto-approve** -- provide actionable feedback +- Think critically about suggestions (shared guide, Section 4) diff --git a/CLAUDE.md b/CLAUDE.md index 54ccfa1..ea1af18 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,268 +1,72 @@ -# Claude Code Development Guide + -This file defines how **Claude Code (Anthropic)** should assist with this project. +# Claude Code Instructions -> **For project overview, installation, and usage examples, see [README.md](README.md).** +> **Shared standards:** Read [.ai/shared-development-guide.md](.ai/shared-development-guide.md) for all coding standards, CI requirements, commit conventions, and best practices. +> +> **CiviCRM reference:** See [.ai/civicrm.md](.ai/civicrm.md) for CiviCRM core patterns and [.ai/extension.md](.ai/extension.md) for extension structure and testing. +> +> **Code review:** See [.ai/ai-code-review.md](.ai/ai-code-review.md) for review checklist and process. -Claude Code can edit files, plan changes, and run commands — but must follow all development standards described here. +This file contains **Claude Code-specific** instructions only. --- -## 1. Quick Reference +## Claude Code Workflow -| Resource | Location | -|----------|----------| -| Project Overview | [README.md](README.md) | -| Usage Examples | [README.md](README.md#usage) | -| PR Template | `.github/PULL_REQUEST_TEMPLATE.md` | -| Linting Rules | `phpcs-ruleset.xml` | -| PHPStan Config | `phpstan.neon` | +### Plan Mode and Execution Mode ---- - -## 2. Development Environment - -### Running Tests, Linter, PHPStan - -```bash -# Setup Docker environment (one-time) -./scripts/run.sh setup - -# Run all tests -./scripts/run.sh tests - -# Run linter on changed files -./scripts/lint.sh check - -# Run PHPStan on changed files -./scripts/run.sh phpstan-changed - -# Regenerate DAO files after schema changes -./scripts/run.sh civix -``` - -### CiviCRM Core Reference (Optional) - -For debugging CiviCRM API issues: - -```bash -git clone https://github.com/compucorp/civicrm-core.git -cd civicrm-core && git checkout 6.4.1-patches -``` - ---- - -## 3. Pull Request Guidelines - -### PR Title Format -``` -CIVIMM-###: Short description -``` - -### Required Sections (from template) -- **Overview**: Non-technical description -- **Before**: Current state -- **After**: What changed -- **Technical Details**: Code snippets, patterns used -- **Core overrides**: If any CiviCRM core files are patched - -### Handling Review Feedback - -**NEVER blindly implement feedback.** Always: - -1. Analyze if the suggestion makes technical sense -2. Consider implications (database constraints, type safety, performance) -3. Ask clarifying questions if unsure -4. Explain your reasoning for accepting or rejecting -5. Get user approval before committing changes - ---- - -## 4. Commit Message Convention - -``` -CIVIMM-###: Short imperative description -``` - -**Rules:** -- Keep under 72 characters -- Use present tense ("Add", "Fix", "Refactor") -- **NO AI attribution** (no "Co-Authored-By: Claude") - -**Examples:** -``` -CIVIMM-456: Add PaymentAttempt entity with multi-processor support -CIVIMM-789: Fix webhook deduplication race condition -``` - ---- - -## 5. Code Quality Standards - -### Unit Testing -- **Mandatory** for all new features and bug fixes -- Extend `BaseHeadlessTest` for test classes -- Use fabricators in `tests/phpunit/Fabricator/` -- Never modify tests just to make them pass - -### Linting (PHPCS) -- Follow CiviCRM Drupal coding standards -- All files must end with a newline -- Run `./scripts/lint.sh fix` to auto-fix issues - -### Static Analysis (PHPStan Level 9) -- Strictest PHP type checking -- Never regenerate baseline to "fix" errors - fix the code -- Use `@phpstan-param` for generic types that linter doesn't support - -### Linter + PHPStan Compatibility Pattern -```php -/** - * @param array $params // Linter sees this - * @phpstan-param array $params // PHPStan sees this - */ -``` - ---- - -## 6. CI Requirements - -All code must pass before merging: - -| Check | Command | CI Workflow | -|-------|---------|-------------| -| Tests | `./scripts/run.sh tests` | `unit-test.yml` | -| Linting | `./scripts/lint.sh check` | `linters.yml` | -| PHPStan | `./scripts/run.sh phpstan-changed` | `phpstan.yml` | - ---- - -## 7. Architecture Overview - -### Namespaces - -| Namespace | Directory | Purpose | -|-----------|-----------|---------| -| `CRM_*` | `CRM/` | Traditional CiviCRM (DAO, BAO) | -| `Civi\Paymentprocessingcore\*` | `Civi/` | Modern services | - -### Key Services - -| Service | Purpose | -|---------|---------| -| `ContributionCompletionService` | Complete pending contributions | -| `ContributionFailureService` | Handle failed contributions | -| `WebhookQueueService` | Queue webhook events | -| `WebhookHandlerRegistry` | Map events to handlers | -| `PaymentProcessorCustomerService` | Manage customer IDs | - -### Webhook Handler Pattern - -Handlers can implement `WebhookHandlerInterface` directly (preferred) or use duck typing (fallback). The registry uses the Adapter pattern for duck-typed handlers. - -```php -// Option 1: Implement interface (preferred) -class MyHandler implements WebhookHandlerInterface { - public function handle(int $webhookId, array $params): string { - return 'applied'; - } -} - -// Option 2: Duck typing (fallback - wrapped in Adapter) -class MyHandler { - public function handle(int $webhookId, array $params): string { - return 'applied'; - } -} -``` - ---- - -## 8. Workflow with Claude Code - -### Recommended Flow -1. **Explain** – Describe the issue -2. **Plan** – Use Plan Mode for complex tasks -3. **Review** – Verify plan before implementation -4. **Implement** – Apply changes -5. **Verify** – Run tests and linting +1. **Explain** -- Ask Claude to describe the issue in its own words +2. **Plan** -- Use Plan Mode (`Shift + Tab` twice) for complex tasks +3. **Review** -- Verify and edit the plan before implementation +4. **Implement** -- Disable Plan Mode and apply changes +5. **Verify** -- Run tests and linting ### Request Confirmation Before - Deleting or overwriting files - Database migrations -- Modifying auto-generated files -- Changes to `xml/schema/` files - ---- +- Modifying auto-generated files (see shared guide, Section 10) -## 9. Safety Rules -### CRITICAL: Run Tests Before Committing -```bash -./scripts/run.sh tests -``` - -### Never Commit -- `civicrm.settings.php` (credentials) -- `.env` files -- Any secrets or API keys +--- -### Never Edit Manually -- `paymentprocessingcore.civix.php` -- `CRM/PaymentProcessingCore/DAO/*.php` -- Files in `xml/schema/*.entityType.php` +## Environment Constraints -### Other Rules -- Never push without running tests and linting -- Never remove tests to make them pass -- Always prefix commits with issue ID -- Never push automatically without human review +- Cannot run tests or PHPStan directly without Docker environment +- Can write test files following existing patterns +- Can fix errors based on CI output +- Suggest: "Push changes to trigger CI" or "Run tests via Docker" --- -## 10. Pre-Merge Checklist +## Claude-Specific Rules -| Check | Requirement | -|-------|-------------| -| ✅ Tests pass | PHPUnit all green | -| ✅ Linting passes | PHPCS no violations | -| ✅ PHPStan passes | Level 9 clean | -| ✅ Commit prefix | `CIVIMM-###` format | -| ✅ PR template used | All sections filled | -| ✅ No sensitive data | No credentials | -| ✅ Code reviewed | At least one approval | +- **DO NOT add `Co-Authored-By: Claude` or any AI attribution** to commits +- Never push commits automatically without human review +- When proposing commits, use the `CIVIMM-###:` format from the shared guide +- Always read files before editing them --- -## 11. Common Commands +## Pre-Push Self-Review -```bash -# Git -git status && git diff +Before proposing a commit, Claude can review its own changes in the same session: -# Testing -./scripts/run.sh setup # One-time Docker setup -./scripts/run.sh tests # Run all tests -./scripts/run.sh test path/to/Test.php # Run specific test - -# Code Quality -./scripts/lint.sh check # Check linting -./scripts/lint.sh fix # Auto-fix linting -./scripts/run.sh phpstan-changed # PHPStan on changed files - -# CiviCRM -./scripts/run.sh civix # Regenerate DAO files -./scripts/run.sh cv api Contact.get # Run cv commands -./scripts/run.sh shell # Shell into container +```bash +# Replace with your PR target (master, main, develop, etc.) +git diff ...HEAD ``` +For unbiased review, use a **separate Claude session** and follow [.ai/ai-code-review.md](.ai/ai-code-review.md). + --- -## 12. Example Prompts +## Developer Prompts (Examples) | Task | Prompt | |------|--------| -| Generate tests | "Create PHPUnit tests for `PaymentAttemptService::createAttempt()` covering success and error cases." | +| Generate tests | "Create PHPUnit tests for `ContributionCompletionService::complete()` covering success and error cases." | | Summarize PR | "Summarize commits into PR description using template for CIVIMM-123." | | Fix linting | "Fix PHPCS violations in `ContributionCompletionService.php`." | -| Add service | "Create `RefundService` following existing service patterns." | +| Fix PHPStan | "PHPStan reports type mismatch in `Contribution.php:132`. Suggest a safe fix." | +| Review changes | "Review my changes against our coding standards: `git diff master...HEAD`" | diff --git a/CRM/Paymentprocessingcore/BAO/PaymentProcessorCustomer.php b/CRM/Paymentprocessingcore/BAO/PaymentProcessorCustomer.php index 0a14748..f7bdd5d 100644 --- a/CRM/Paymentprocessingcore/BAO/PaymentProcessorCustomer.php +++ b/CRM/Paymentprocessingcore/BAO/PaymentProcessorCustomer.php @@ -52,7 +52,9 @@ public static function findByContactAndProcessor(int $contactId, int $paymentPro $customer->payment_processor_id = $paymentProcessorId; if ($customer->find(TRUE)) { - return $customer->toArray(); + /** @var array{id: int, contact_id: int, payment_processor_id: int, processor_customer_id: string, created_date: string} $data */ + $data = $customer->toArray(); + return $data; } return NULL; @@ -78,7 +80,9 @@ public static function findByProcessorCustomerId(string $processorCustomerId, in $customer->payment_processor_id = $paymentProcessorId; if ($customer->find(TRUE)) { - return $customer->toArray(); + /** @var array{id: int, contact_id: int, payment_processor_id: int, processor_customer_id: string, created_date: string} $data */ + $data = $customer->toArray(); + return $data; } return NULL; diff --git a/Civi/Paymentprocessingcore/DTO/RecurringContributionDTO.php b/Civi/Paymentprocessingcore/DTO/RecurringContributionDTO.php new file mode 100644 index 0000000..a02f716 --- /dev/null +++ b/Civi/Paymentprocessingcore/DTO/RecurringContributionDTO.php @@ -0,0 +1,227 @@ +id = $id; + $this->contactId = $contactId; + $this->amount = $amount; + $this->currency = $currency; + $this->financialTypeId = $financialTypeId; + $this->campaignId = $campaignId; + $this->nextSchedContributionDate = $nextSchedContributionDate; + $this->frequencyUnit = $frequencyUnit; + $this->frequencyInterval = $frequencyInterval; + } + + /** + * Create a DTO from a CiviCRM Api4 result array. + * + * @param array $record + * Api4 result row with keys: id, contact_id, + * next_sched_contribution_date, amount, currency, + * financial_type_id, campaign_id, frequency_unit:name, + * frequency_interval. + * + * @return self + * Typed DTO instance. + * + * @throws \InvalidArgumentException + * If required fields are missing or invalid. + */ + public static function fromApiResult(array $record): self { + if (empty($record['id']) || !is_numeric($record['id'])) { + throw new \InvalidArgumentException('Recurring contribution record must have a numeric id.'); + } + + if (empty($record['contact_id']) || !is_numeric($record['contact_id'])) { + throw new \InvalidArgumentException('Recurring contribution record must have a numeric contact_id.'); + } + + if (empty($record['next_sched_contribution_date'])) { + throw new \InvalidArgumentException('Recurring contribution record must have a next_sched_contribution_date.'); + } + + /** @var int|string|float $id */ + $id = $record['id']; + /** @var int|string|float $contactId */ + $contactId = $record['contact_id']; + /** @var int|string|float|null $amount */ + $amount = $record['amount'] ?? 0; + /** @var string $currency */ + $currency = $record['currency'] ?? ''; + /** @var int|string|float|null $financialTypeId */ + $financialTypeId = $record['financial_type_id'] ?? 0; + /** @var string $nextSchedDate */ + $nextSchedDate = $record['next_sched_contribution_date']; + /** @var string $frequencyUnit */ + $frequencyUnit = $record['frequency_unit:name'] ?? 'month'; + /** @var int|string|float|null $frequencyInterval */ + $frequencyInterval = $record['frequency_interval'] ?? 1; + + $campaignId = isset($record['campaign_id']) && is_numeric($record['campaign_id']) + ? (int) $record['campaign_id'] + : NULL; + + return new self( + (int) $id, + (int) $contactId, + (float) $amount, + (string) $currency, + (int) $financialTypeId, + $campaignId, + (string) $nextSchedDate, + (string) $frequencyUnit, + (int) $frequencyInterval + ); + } + + /** + * Get the recurring contribution ID. + */ + public function getId(): int { + return $this->id; + } + + /** + * Get the contact ID. + */ + public function getContactId(): int { + return $this->contactId; + } + + /** + * Get the recurring amount. + */ + public function getAmount(): float { + return $this->amount; + } + + /** + * Get the currency code. + */ + public function getCurrency(): string { + return $this->currency; + } + + /** + * Get the financial type ID. + */ + public function getFinancialTypeId(): int { + return $this->financialTypeId; + } + + /** + * Get the campaign ID, or NULL if not set. + */ + public function getCampaignId(): ?int { + return $this->campaignId; + } + + /** + * Get the next scheduled contribution date. + */ + public function getNextSchedContributionDate(): string { + return $this->nextSchedContributionDate; + } + + /** + * Get the frequency unit. + */ + public function getFrequencyUnit(): string { + return $this->frequencyUnit; + } + + /** + * Get the frequency interval. + */ + public function getFrequencyInterval(): int { + return $this->frequencyInterval; + } + +} diff --git a/Civi/Paymentprocessingcore/Hook/Container/ServiceContainer.php b/Civi/Paymentprocessingcore/Hook/Container/ServiceContainer.php index 5afe070..d4b84f4 100644 --- a/Civi/Paymentprocessingcore/Hook/Container/ServiceContainer.php +++ b/Civi/Paymentprocessingcore/Hook/Container/ServiceContainer.php @@ -94,6 +94,17 @@ public function register(): void { 'Civi\Paymentprocessingcore\Service\WebhookHealthService', 'paymentprocessingcore.webhook_health' ); + + // Register InstalmentGenerationService + $this->container->setDefinition( + 'paymentprocessingcore.instalment_generation', + new Definition(\Civi\Paymentprocessingcore\Service\InstalmentGenerationService::class) + )->setAutowired(TRUE)->setPublic(TRUE); + + $this->container->setAlias( + 'Civi\Paymentprocessingcore\Service\InstalmentGenerationService', + 'paymentprocessingcore.instalment_generation' + ); } } diff --git a/Civi/Paymentprocessingcore/Service/InstalmentGenerationService.php b/Civi/Paymentprocessingcore/Service/InstalmentGenerationService.php new file mode 100644 index 0000000..5c49bed --- /dev/null +++ b/Civi/Paymentprocessingcore/Service/InstalmentGenerationService.php @@ -0,0 +1,309 @@ +, message: string} + * Summary of processing results. + */ + public function generateInstalments(string $processorType, int $batchSize, ?string $referenceDate = NULL): array { + $referenceDate = $referenceDate ?? date('Y-m-d'); + + $this->logDebug('InstalmentGenerationService::generateInstalments: Starting', [ + 'processorType' => $processorType, + 'batchSize' => $batchSize, + 'referenceDate' => $referenceDate, + ]); + + $summary = [ + 'created' => 0, + 'skipped' => 0, + 'errored' => 0, + 'errors' => [], + ]; + + $dueRecurrings = $this->getDueRecurringContributions($processorType, $batchSize, $referenceDate); + + $this->logDebug('InstalmentGenerationService::generateInstalments: Found due recurrings', [ + 'count' => count($dueRecurrings), + ]); + + foreach ($dueRecurrings as $recur) { + try { + $recurId = $recur->getId(); + $receiveDate = $recur->getNextSchedContributionDate(); + + if ($this->instalmentExists($recurId, $receiveDate)) { + $this->logDebug('InstalmentGenerationService::generateInstalments: Skipping duplicate instalment', [ + 'recurId' => $recurId, + 'receiveDate' => $receiveDate, + ]); + $summary['skipped']++; + continue; + } + + $tx = new \CRM_Core_Transaction(); + try { + $this->createInstalment($recur, $receiveDate); + $this->advanceScheduleDate($recurId, $receiveDate, $recur->getFrequencyUnit(), $recur->getFrequencyInterval()); + $tx->commit(); + $summary['created']++; + } + catch (\Throwable $e) { + $tx->rollback(); + throw $e; + } + + $this->logDebug('InstalmentGenerationService::generateInstalments: Created instalment', [ + 'recurId' => $recurId, + 'receiveDate' => $receiveDate, + ]); + } + catch (\Throwable $e) { + $recurId = isset($recurId) ? $recurId : 0; + $summary['errored']++; + $summary['errors'][$recurId] = $e->getMessage(); + + $this->logDebug('InstalmentGenerationService::generateInstalments: Error processing recur', [ + 'recurId' => $recurId, + 'error' => $e->getMessage(), + ]); + } + } + + $summary['message'] = sprintf( + 'Processed %d due recurring contributions: %d instalments created, %d skipped (contribution already exists for scheduled date), %d errors.', + $summary['created'] + $summary['skipped'] + $summary['errored'], + $summary['created'], + $summary['skipped'], + $summary['errored'] + ); + + $this->logDebug('InstalmentGenerationService::generateInstalments: Completed', $summary); + + return $summary; + } + + /** + * Get recurring contributions that are due for instalment generation. + * + * @param string $processorType + * Payment processor type name. + * @param int $batchSize + * Maximum number of records to return. + * @param string|null $referenceDate + * Optional reference date (Y-m-d). Defaults to today. + * + * @return array + * Array of recurring contribution DTOs. + */ + public function getDueRecurringContributions(string $processorType, int $batchSize, ?string $referenceDate = NULL): array { + $dateOnly = $referenceDate ?? date('Y-m-d'); + $nextDayTimestamp = strtotime($dateOnly . ' +1 day'); + if ($nextDayTimestamp === FALSE) { + throw new \InvalidArgumentException('Invalid reference date: ' . $dateOnly); + } + $nextDay = date('Y-m-d', $nextDayTimestamp); + + $result = \Civi\Api4\ContributionRecur::get(FALSE) + ->addSelect( + 'id', + 'next_sched_contribution_date', + 'frequency_unit:name', + 'frequency_interval', + 'contact_id', + 'amount', + 'currency', + 'financial_type_id', + 'campaign_id' + ) + ->addJoin( + 'PaymentProcessor AS pp', + 'INNER', + NULL, + ['payment_processor_id', '=', 'pp.id'] + ) + ->addJoin( + 'PaymentProcessorType AS ppt', + 'INNER', + NULL, + ['pp.payment_processor_type_id', '=', 'ppt.id'] + ) + ->addJoin( + 'Membership AS m', + 'LEFT', + NULL, + ['id', '=', 'm.contribution_recur_id'] + ) + ->addWhere('contribution_status_id:name', '=', 'In Progress') + ->addWhere('next_sched_contribution_date', '<', $nextDay . ' 00:00:00') + ->addWhere('ppt.name', '=', $processorType) + ->addWhere('m.id', 'IS NULL') + ->setLimit($batchSize) + ->execute() + ->getArrayCopy(); + + $dtos = []; + foreach ($result as $record) { + /** @var array $record */ + $dtos[] = RecurringContributionDTO::fromApiResult($record); + } + + return $dtos; + } + + /** + * Check if an instalment already exists for the given recur and date. + * + * @param int $recurId + * The recurring contribution ID. + * @param string $receiveDate + * The receive date to check. + * + * @return bool + * TRUE if an instalment already exists. + */ + public function instalmentExists(int $recurId, string $receiveDate): bool { + $dateTimestamp = strtotime($receiveDate); + if ($dateTimestamp === FALSE) { + throw new \InvalidArgumentException('Invalid receive date: ' . $receiveDate); + } + $dateOnly = date('Y-m-d', $dateTimestamp); + $nextDayTimestamp = strtotime($dateOnly . ' +1 day'); + if ($nextDayTimestamp === FALSE) { + throw new \InvalidArgumentException('Invalid date calculation for: ' . $dateOnly); + } + $nextDay = date('Y-m-d', $nextDayTimestamp); + $count = \Civi\Api4\Contribution::get(FALSE) + ->selectRowCount() + ->addWhere('contribution_recur_id', '=', $recurId) + ->addWhere('receive_date', '>=', $dateOnly . ' 00:00:00') + ->addWhere('receive_date', '<', $nextDay . ' 00:00:00') + ->addWhere('contribution_status_id:name', 'IN', [ + 'Pending', + 'Completed', + 'Failed', + 'Cancelled', + ]) + ->execute() + ->rowCount; + + return $count > 0; + } + + /** + * Create an instalment contribution from recurring contribution fields. + * + * Copies contact_id, financial_type_id, total_amount, currency, and + * campaign_id from the recurring contribution record. Sets is_pay_later + * to 0 and invoice_id to NULL. + * + * @param \Civi\Paymentprocessingcore\DTO\RecurringContributionDTO $recur + * The recurring contribution DTO. + * @param string $receiveDate + * The receive date for the new contribution. + * + * @return int + * The new contribution ID. + * + * @throws \RuntimeException + */ + public function createInstalment(RecurringContributionDTO $recur, string $receiveDate): int { + $createAction = \Civi\Api4\Contribution::create(FALSE) + ->addValue('contact_id', $recur->getContactId()) + ->addValue('financial_type_id', $recur->getFinancialTypeId()) + ->addValue('total_amount', $recur->getAmount()) + ->addValue('currency', $recur->getCurrency()) + ->addValue('contribution_recur_id', $recur->getId()) + ->addValue('receive_date', $receiveDate) + ->addValue('contribution_status_id:name', 'Pending') + ->addValue('is_pay_later', 0) + ->addValue('invoice_id', NULL); + + if ($recur->getCampaignId() !== NULL) { + $createAction->addValue('campaign_id', $recur->getCampaignId()); + } + + $result = $createAction->execute()->first(); + + if (!is_array($result)) { + throw new \RuntimeException( + 'Failed to create instalment for recurring contribution ' . $recur->getId() + ); + } + + return (int) $result['id']; + } + + /** + * Advance the next scheduled contribution date. + * + * @param int $recurId + * The recurring contribution ID. + * @param string $currentDate + * The current scheduled date. + * @param string $frequencyUnit + * The frequency unit (day, week, month, year). + * @param int $frequencyInterval + * The frequency interval. + */ + public function advanceScheduleDate( + int $recurId, + string $currentDate, + string $frequencyUnit, + int $frequencyInterval + ): void { + $date = new \DateTime($currentDate); + + if ($frequencyUnit === 'month') { + $originalDay = (int) $date->format('j'); + $date->modify('first day of +' . $frequencyInterval . ' month'); + $lastDayOfTargetMonth = (int) $date->format('t'); + $targetDay = min($originalDay, $lastDayOfTargetMonth); + $date->setDate((int) $date->format('Y'), (int) $date->format('n'), $targetDay); + } + else { + $date->modify('+' . $frequencyInterval . ' ' . $frequencyUnit); + } + + \Civi\Api4\ContributionRecur::update(FALSE) + ->addWhere('id', '=', $recurId) + ->addValue('next_sched_contribution_date', $date->format('Y-m-d H:i:s')) + ->execute(); + } + +} diff --git a/GEMINI.md b/GEMINI.md new file mode 100644 index 0000000..ec85c46 --- /dev/null +++ b/GEMINI.md @@ -0,0 +1,44 @@ + + +# Gemini Instructions + +> **Shared standards:** Read [.ai/shared-development-guide.md](.ai/shared-development-guide.md) for all coding standards, CI requirements, commit conventions, and best practices. +> +> **CiviCRM reference:** See [.ai/civicrm.md](.ai/civicrm.md) for CiviCRM core patterns and [.ai/extension.md](.ai/extension.md) for extension structure and testing. +> +> **Code review:** See [.ai/ai-code-review.md](.ai/ai-code-review.md) for the full review checklist, severity levels, and process. + +This file contains **Gemini-specific** instructions only. + +--- + +## Gemini's Primary Role + +Gemini is used for **code review** -- both pre-push (developer-initiated) and on GitHub PRs. It can also assist with development. + +--- + +## Pre-Push Review (Any Interface) + +Developers can request review before pushing, using any Gemini interface (IDE extension, web, CLI). Follow the process in [.ai/ai-code-review.md](.ai/ai-code-review.md). + +--- + +## GitHub PR Review + +When reviewing PRs on GitHub, Gemini should: + +- Apply the full checklist from [.ai/ai-code-review.md](.ai/ai-code-review.md) +- Post comments on specific lines with severity labels +- **Never auto-approve** -- provide actionable feedback +- Think critically (shared guide, Section 4) + +--- + +## Gemini-Specific Rules + +- **DO NOT add any AI attribution** to commits if writing code +- Follow the `CIVIMM-###:` commit format from the shared guide +- When reviewing, distinguish clearly between BLOCKERs and SUGGESTIONs +- Be specific -- reference file paths and line numbers +- Explain **why** something is an issue, not just what to change diff --git a/api/v3/InstalmentGenerator/Run.php b/api/v3/InstalmentGenerator/Run.php new file mode 100644 index 0000000..03aa567 --- /dev/null +++ b/api/v3/InstalmentGenerator/Run.php @@ -0,0 +1,55 @@ +generateInstalments($processorType, $batchSize); + + return civicrm_api3_create_success($result, $params, 'InstalmentGenerator', 'Run'); +} + +/** + * InstalmentGenerator.Run API specification. + * + * @param array $spec + * API specification array. + */ +function _civicrm_api3_instalment_generator_Run_spec(array &$spec): void { + $spec['processor_type'] = [ + 'title' => 'Processor Type', + 'description' => 'Payment processor type name. Default: "Stripe".', + 'type' => CRM_Utils_Type::T_STRING, + 'api.default' => InstalmentGenerationService::DEFAULT_PROCESSOR_TYPE, + ]; + $spec['batch_size'] = [ + 'title' => 'Batch Size', + 'description' => 'Maximum number of recurring contributions to process. Default: 500.', + 'type' => CRM_Utils_Type::T_INT, + 'api.default' => InstalmentGenerationService::DEFAULT_BATCH_SIZE, + ]; +} diff --git a/composer.json b/composer.json index 38a84c0..6a12abf 100644 --- a/composer.json +++ b/composer.json @@ -13,6 +13,7 @@ "require-dev": { "phpstan/phpstan": "^1.12", "phpstan/phpstan-phpunit": "^1.4", - "phpunit/phpunit": "^9.5" + "phpunit/phpunit": "^9.5", + "psr/log": "^1.1" } } diff --git a/composer.lock b/composer.lock index ddc1580..9786629 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "5d031c74e57aa1b66b463856c07bfcea", + "content-hash": "71a71f11250d468942f9b0d3098e39ee", "packages": [], "packages-dev": [ { @@ -848,6 +848,56 @@ ], "time": "2025-09-24T06:29:11+00:00" }, + { + "name": "psr/log", + "version": "1.1.4", + "source": { + "type": "git", + "url": "https://github.com/php-fig/log.git", + "reference": "d49695b909c3b7628b6289db5479a1c204601f11" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/php-fig/log/zipball/d49695b909c3b7628b6289db5479a1c204601f11", + "reference": "d49695b909c3b7628b6289db5479a1c204601f11", + "shasum": "" + }, + "require": { + "php": ">=5.3.0" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "1.1.x-dev" + } + }, + "autoload": { + "psr-4": { + "Psr\\Log\\": "Psr/Log/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "PHP-FIG", + "homepage": "https://www.php-fig.org/" + } + ], + "description": "Common interface for logging libraries", + "homepage": "https://github.com/php-fig/log", + "keywords": [ + "log", + "psr", + "psr-3" + ], + "support": { + "source": "https://github.com/php-fig/log/tree/1.1.4" + }, + "time": "2021-05-03T11:20:27+00:00" + }, { "name": "sebastian/cli-parser", "version": "1.0.2", diff --git a/managed/Job_InstalmentGenerator.mgd.php b/managed/Job_InstalmentGenerator.mgd.php new file mode 100644 index 0000000..4423dce --- /dev/null +++ b/managed/Job_InstalmentGenerator.mgd.php @@ -0,0 +1,27 @@ + 'Job:InstalmentGenerator', + 'entity' => 'Job', + 'params' => [ + 'version' => 3, + 'name' => 'Generate instalments for recurring contributions (non-membership)', + 'description' => 'Creates Pending contribution records for each due In Progress recurring contribution that is not linked to a membership. No payment processor calls are made.', + 'run_frequency' => 'Always', + 'api_entity' => 'InstalmentGenerator', + 'api_action' => 'Run', + 'api_version' => 3, + 'parameters' => "processor_type=Stripe\nbatch_size=500", + 'is_active' => 1, + ], + ], +]; diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 3a6d98e..13ce0d8 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1,5 +1,15 @@ parameters: ignoreErrors: + - + message: "#^Call to an undefined method CRM_Paymentprocessingcore_DAO_PaymentAttempt\\:\\:find\\(\\)\\.$#" + count: 3 + path: CRM/Paymentprocessingcore/BAO/PaymentAttempt.php + + - + message: "#^Call to an undefined method CRM_Paymentprocessingcore_DAO_PaymentAttempt\\:\\:toArray\\(\\)\\.$#" + count: 3 + path: CRM/Paymentprocessingcore/BAO/PaymentAttempt.php + - message: "#^Method CRM_Paymentprocessingcore_BAO_PaymentAttempt\\:\\:create\\(\\) has parameter \\$params with no value type specified in iterable type array\\.$#" count: 1 @@ -10,20 +20,30 @@ parameters: count: 1 path: CRM/Paymentprocessingcore/BAO/PaymentAttempt.php + - + message: "#^Call to an undefined method CRM_Paymentprocessingcore_BAO_PaymentProcessorCustomer\\:\\:find\\(\\)\\.$#" + count: 2 + path: CRM/Paymentprocessingcore/BAO/PaymentProcessorCustomer.php + + - + message: "#^Call to an undefined method CRM_Paymentprocessingcore_BAO_PaymentProcessorCustomer\\:\\:toArray\\(\\)\\.$#" + count: 2 + path: CRM/Paymentprocessingcore/BAO/PaymentProcessorCustomer.php + - message: "#^Method CRM_Paymentprocessingcore_BAO_PaymentProcessorCustomer\\:\\:create\\(\\) has parameter \\$params with no value type specified in iterable type array\\.$#" count: 1 path: CRM/Paymentprocessingcore/BAO/PaymentProcessorCustomer.php - - message: "#^Method CRM_Paymentprocessingcore_BAO_PaymentProcessorCustomer\\:\\:findByContactAndProcessor\\(\\) should return array\\{id\\: int, contact_id\\: int, payment_processor_id\\: int, processor_customer_id\\: string, created_date\\: string\\}\\|null but returns array\\.$#" + message: "#^Call to an undefined method CRM_Paymentprocessingcore_BAO_PaymentWebhook\\:\\:find\\(\\)\\.$#" count: 1 - path: CRM/Paymentprocessingcore/BAO/PaymentProcessorCustomer.php + path: CRM/Paymentprocessingcore/BAO/PaymentWebhook.php - - message: "#^Method CRM_Paymentprocessingcore_BAO_PaymentProcessorCustomer\\:\\:findByProcessorCustomerId\\(\\) should return array\\{id\\: int, contact_id\\: int, payment_processor_id\\: int, processor_customer_id\\: string, created_date\\: string\\}\\|null but returns array\\.$#" + message: "#^Call to an undefined method CRM_Paymentprocessingcore_BAO_PaymentWebhook\\:\\:toArray\\(\\)\\.$#" count: 1 - path: CRM/Paymentprocessingcore/BAO/PaymentProcessorCustomer.php + path: CRM/Paymentprocessingcore/BAO/PaymentWebhook.php - message: "#^Comparison operation \"\\>\" between int\\<1, max\\> and 0 is always true\\.$#" @@ -106,25 +126,40 @@ parameters: path: Civi/Paymentprocessingcore/Exception/PaymentProcessorCustomerException.php - - message: "#^Call to method getExtraParams\\(\\) on an unknown class CiviCRM_API3_Exception\\.$#" - count: 1 - path: Civi/Paymentprocessingcore/Service/ContributionCompletionService.php + message: "#^Call to method setAlias\\(\\) on an unknown class Symfony\\\\Component\\\\DependencyInjection\\\\ContainerBuilder\\.$#" + count: 7 + path: Civi/Paymentprocessingcore/Hook/Container/ServiceContainer.php - - message: "#^Call to method getMessage\\(\\) on an unknown class CiviCRM_API3_Exception\\.$#" + message: "#^Call to method setDefinition\\(\\) on an unknown class Symfony\\\\Component\\\\DependencyInjection\\\\ContainerBuilder\\.$#" + count: 7 + path: Civi/Paymentprocessingcore/Hook/Container/ServiceContainer.php + + - + message: "#^Instantiated class Symfony\\\\Component\\\\DependencyInjection\\\\Definition not found\\.$#" + count: 7 + path: Civi/Paymentprocessingcore/Hook/Container/ServiceContainer.php + + - + message: "#^Parameter \\$container of method Civi\\\\Paymentprocessingcore\\\\Hook\\\\Container\\\\ServiceContainer\\:\\:__construct\\(\\) has invalid type Symfony\\\\Component\\\\DependencyInjection\\\\ContainerBuilder\\.$#" count: 2 - path: Civi/Paymentprocessingcore/Service/ContributionCompletionService.php + path: Civi/Paymentprocessingcore/Hook/Container/ServiceContainer.php - - message: "#^Call to static method get\\(\\) on an unknown class Civi\\\\Api4\\\\Contribution\\.$#" + message: "#^Property Civi\\\\Paymentprocessingcore\\\\Hook\\\\Container\\\\ServiceContainer\\:\\:\\$container has unknown class Symfony\\\\Component\\\\DependencyInjection\\\\ContainerBuilder as its type\\.$#" count: 1 - path: Civi/Paymentprocessingcore/Service/ContributionCompletionService.php + path: Civi/Paymentprocessingcore/Hook/Container/ServiceContainer.php - - message: "#^Call to static method get\\(\\) on an unknown class Civi\\\\Api4\\\\ContributionPage\\.$#" + message: "#^Call to method getExtraParams\\(\\) on an unknown class CiviCRM_API3_Exception\\.$#" count: 1 path: Civi/Paymentprocessingcore/Service/ContributionCompletionService.php + - + message: "#^Call to method getMessage\\(\\) on an unknown class CiviCRM_API3_Exception\\.$#" + count: 2 + path: Civi/Paymentprocessingcore/Service/ContributionCompletionService.php + - message: "#^Caught class CiviCRM_API3_Exception not found\\.$#" count: 1 @@ -156,49 +191,54 @@ parameters: path: Civi/Paymentprocessingcore/Service/ContributionCompletionService.php - - message: "#^Access to an undefined property object\\:\\:\\$data\\.$#" - count: 2 - path: Civi/Paymentprocessingcore/Service/WebhookQueueRunnerService.php + message: "#^Access to an undefined property CRM_Core_DAO\\:\\:\\$errors\\.$#" + count: 1 + path: Civi/Paymentprocessingcore/Service/WebhookHealthService.php - - message: "#^Comparison operation \"\\>\" between int\\<1, max\\> and 0 is always true\\.$#" + message: "#^Access to an undefined property CRM_Core_DAO\\:\\:\\$pending\\.$#" count: 1 - path: Civi/Paymentprocessingcore/Service/WebhookQueueRunnerService.php + path: Civi/Paymentprocessingcore/Service/WebhookHealthService.php - - message: "#^Method Civi\\\\Paymentprocessingcore\\\\Service\\\\WebhookQueueRunnerService\\:\\:runAllQueues\\(\\) return type has no value type specified in iterable type array\\.$#" + message: "#^Access to an undefined property CRM_Core_DAO\\:\\:\\$processor_type\\.$#" count: 1 - path: Civi/Paymentprocessingcore/Service/WebhookQueueRunnerService.php + path: Civi/Paymentprocessingcore/Service/WebhookHealthService.php - - message: "#^Method Civi\\\\Paymentprocessingcore\\\\Service\\\\WebhookQueueRunnerService\\:\\:runTask\\(\\) has parameter \\$params with no value type specified in iterable type array\\.$#" + message: "#^Access to an undefined property CRM_Core_DAO\\:\\:\\$stuck\\.$#" count: 1 - path: Civi/Paymentprocessingcore/Service/WebhookQueueRunnerService.php + path: Civi/Paymentprocessingcore/Service/WebhookHealthService.php - - message: "#^Negated boolean expression is always false\\.$#" - count: 1 + message: "#^Call to an undefined method CRM_Core_DAO\\:\\:fetch\\(\\)\\.$#" + count: 2 + path: Civi/Paymentprocessingcore/Service/WebhookHealthService.php + + - + message: "#^Access to an undefined property object\\:\\:\\$data\\.$#" + count: 2 path: Civi/Paymentprocessingcore/Service/WebhookQueueRunnerService.php - - message: "#^Access to an undefined property CRM_Core_DAO\\:\\:\\$processor_type\\.$#" + message: "#^Comparison operation \"\\>\" between int\\<1, max\\> and 0 is always true\\.$#" count: 1 - path: Civi/Paymentprocessingcore/Service/WebhookHealthService.php + path: Civi/Paymentprocessingcore/Service/WebhookQueueRunnerService.php - - message: "#^Access to an undefined property CRM_Core_DAO\\:\\:\\$pending\\.$#" + message: "#^Method Civi\\\\Paymentprocessingcore\\\\Service\\\\WebhookQueueRunnerService\\:\\:runAllQueues\\(\\) return type has no value type specified in iterable type array\\.$#" count: 1 - path: Civi/Paymentprocessingcore/Service/WebhookHealthService.php + path: Civi/Paymentprocessingcore/Service/WebhookQueueRunnerService.php - - message: "#^Access to an undefined property CRM_Core_DAO\\:\\:\\$stuck\\.$#" + message: "#^Method Civi\\\\Paymentprocessingcore\\\\Service\\\\WebhookQueueRunnerService\\:\\:runTask\\(\\) has parameter \\$params with no value type specified in iterable type array\\.$#" count: 1 - path: Civi/Paymentprocessingcore/Service/WebhookHealthService.php + path: Civi/Paymentprocessingcore/Service/WebhookQueueRunnerService.php - - message: "#^Access to an undefined property CRM_Core_DAO\\:\\:\\$errors\\.$#" + message: "#^Negated boolean expression is always false\\.$#" count: 1 - path: Civi/Paymentprocessingcore/Service/WebhookHealthService.php + path: Civi/Paymentprocessingcore/Service/WebhookQueueRunnerService.php - message: "#^Method Civi\\\\Paymentprocessingcore\\\\Service\\\\WebhookQueueService\\:\\:addTask\\(\\) has parameter \\$params with no value type specified in iterable type array\\.$#" @@ -225,11 +265,6 @@ parameters: count: 1 path: tests/phpunit/BaseHeadlessTest.php - - - message: "#^Call to static method create\\(\\) on an unknown class Civi\\\\Api4\\\\Contribution\\.$#" - count: 1 - path: tests/phpunit/CRM/Paymentprocessingcore/BAO/PaymentAttemptTest.php - - message: "#^Cannot access property \\$contact_id on CRM_Paymentprocessingcore_DAO_PaymentAttempt\\|null\\.$#" count: 1 @@ -327,14 +362,9 @@ parameters: - message: "#^Offset 'id' does not exist on array\\|null\\.$#" - count: 1 + count: 2 path: tests/phpunit/CRM/Paymentprocessingcore/BAO/PaymentAttemptTest.php - - - message: "#^Call to static method create\\(\\) on an unknown class Civi\\\\Api4\\\\Contribution\\.$#" - count: 1 - path: tests/phpunit/CRM/Paymentprocessingcore/BAO/PaymentWebhookTest.php - - message: "#^Cannot access property \\$event_id on CRM_Paymentprocessingcore_DAO_PaymentWebhook\\|null\\.$#" count: 3 @@ -452,7 +482,7 @@ parameters: - message: "#^Offset 'id' does not exist on array\\|null\\.$#" - count: 1 + count: 2 path: tests/phpunit/CRM/Paymentprocessingcore/BAO/PaymentWebhookTest.php - @@ -461,17 +491,7 @@ parameters: path: tests/phpunit/CRM/Paymentprocessingcore/BAO/PaymentWebhookTest.php - - message: "#^Call to static method create\\(\\) on an unknown class Civi\\\\Api4\\\\Contribution\\.$#" - count: 5 - path: tests/phpunit/Civi/Api4/PaymentAttemptTest.php - - - - message: "#^Call to static method create\\(\\) on an unknown class Civi\\\\Api4\\\\PaymentProcessor\\.$#" - count: 1 - path: tests/phpunit/Civi/Api4/PaymentAttemptTest.php - - - - message: "#^Call to static method delete\\(\\) on an unknown class Civi\\\\Api4\\\\Contribution\\.$#" + message: "#^Call to an undefined method Civi\\\\Api4\\\\Generic\\\\DAODeleteAction\\:\\:setUseTrash\\(\\)\\.$#" count: 1 path: tests/phpunit/Civi/Api4/PaymentAttemptTest.php @@ -552,7 +572,7 @@ parameters: - message: "#^Offset 'id' does not exist on array\\|null\\.$#" - count: 13 + count: 19 path: tests/phpunit/Civi/Api4/PaymentAttemptTest.php - @@ -575,6 +595,11 @@ parameters: count: 2 path: tests/phpunit/Civi/Api4/PaymentAttemptTest.php + - + message: "#^Parameter \\#1 \\$exception of method PHPUnit\\\\Framework\\\\TestCase\\:\\:expectException\\(\\) expects class\\-string\\, string given\\.$#" + count: 3 + path: tests/phpunit/Civi/Api4/PaymentAttemptTest.php + - message: "#^Property Civi_Api4_PaymentAttemptTest\\:\\:\\$contactId has no type specified\\.$#" count: 1 @@ -590,11 +615,6 @@ parameters: count: 1 path: tests/phpunit/Civi/Api4/PaymentAttemptTest.php - - - message: "#^Call to static method create\\(\\) on an unknown class Civi\\\\Api4\\\\PaymentProcessor\\.$#" - count: 1 - path: tests/phpunit/Civi/Api4/PaymentProcessorCustomerTest.php - - message: "#^Method Civi_Api4_PaymentProcessorCustomerTest\\:\\:skipTestCascadeDeleteWhenContactDeleted\\(\\) has no return type specified\\.$#" count: 1 @@ -640,11 +660,6 @@ parameters: count: 1 path: tests/phpunit/Civi/Api4/PaymentProcessorCustomerTest.php - - - message: "#^Call to static method create\\(\\) on an unknown class Civi\\\\Api4\\\\Contribution\\.$#" - count: 1 - path: tests/phpunit/Civi/Api4/PaymentWebhookTest.php - - message: "#^Method Civi_Api4_PaymentWebhookTest\\:\\:testAllResults\\(\\) has no return type specified\\.$#" count: 1 @@ -747,7 +762,7 @@ parameters: - message: "#^Offset 'id' does not exist on array\\|null\\.$#" - count: 13 + count: 14 path: tests/phpunit/Civi/Api4/PaymentWebhookTest.php - @@ -775,6 +790,11 @@ parameters: count: 3 path: tests/phpunit/Civi/Api4/PaymentWebhookTest.php + - + message: "#^Parameter \\#1 \\$exception of method PHPUnit\\\\Framework\\\\TestCase\\:\\:expectException\\(\\) expects class\\-string\\, string given\\.$#" + count: 4 + path: tests/phpunit/Civi/Api4/PaymentWebhookTest.php + - message: "#^Property Civi_Api4_PaymentWebhookTest\\:\\:\\$attemptId has no type specified\\.$#" count: 1 @@ -791,27 +811,27 @@ parameters: path: tests/phpunit/Civi/Api4/PaymentWebhookTest.php - - message: "#^Call to static method create\\(\\) on an unknown class Civi\\\\Api4\\\\Contribution\\.$#" + message: "#^Offset 'contribution_status…' does not exist on array\\|null\\.$#" count: 1 path: tests/phpunit/Civi/Paymentprocessingcore/Service/ContributionCompletionServiceTest.php - - message: "#^Call to static method create\\(\\) on an unknown class Civi\\\\Api4\\\\ContributionPage\\.$#" + message: "#^Offset 'fee_amount' does not exist on array\\|null\\.$#" count: 1 path: tests/phpunit/Civi/Paymentprocessingcore/Service/ContributionCompletionServiceTest.php - - message: "#^Call to static method get\\(\\) on an unknown class Civi\\\\Api4\\\\Contribution\\.$#" - count: 2 + message: "#^Offset 'id' does not exist on array\\|null\\.$#" + count: 3 path: tests/phpunit/Civi/Paymentprocessingcore/Service/ContributionCompletionServiceTest.php - - message: "#^Call to static method update\\(\\) on an unknown class Civi\\\\Api4\\\\Contribution\\.$#" + message: "#^Offset 'net_amount' does not exist on array\\|null\\.$#" count: 1 path: tests/phpunit/Civi/Paymentprocessingcore/Service/ContributionCompletionServiceTest.php - - message: "#^Offset 'id' does not exist on array\\|null\\.$#" + message: "#^Offset 'trxn_id' does not exist on array\\|null\\.$#" count: 1 path: tests/phpunit/Civi/Paymentprocessingcore/Service/ContributionCompletionServiceTest.php @@ -825,11 +845,6 @@ parameters: count: 1 path: tests/phpunit/Civi/Paymentprocessingcore/Service/ContributionCompletionServiceTest.php - - - message: "#^Call to static method create\\(\\) on an unknown class Civi\\\\Api4\\\\PaymentProcessor\\.$#" - count: 1 - path: tests/phpunit/Civi/Paymentprocessingcore/Service/PaymentProcessorCustomerServiceTest.php - - message: "#^Method Civi\\\\Paymentprocessingcore\\\\Service\\\\TestWebhookReceiverService\\:\\:publicQueueWebhook\\(\\) has parameter \\$eventData with no value type specified in iterable type array\\.$#" count: 1 @@ -965,11 +980,6 @@ parameters: count: 1 path: tests/phpunit/Civi/Paymentprocessingcore/Service/WebhookQueueServiceTest.php - - - message: "#^Call to static method create\\(\\) on an unknown class Civi\\\\Api4\\\\Contribution\\.$#" - count: 1 - path: tests/phpunit/Civi/Paymentprocessingcore/Service/WebhookReceiverServiceTest.php - - message: "#^Cannot call method getQueueCount\\(\\) on mixed\\.$#" count: 2 @@ -1032,7 +1042,7 @@ parameters: - message: "#^Offset 'id' does not exist on array\\|null\\.$#" - count: 2 + count: 3 path: tests/phpunit/Civi/Paymentprocessingcore/Service/WebhookReceiverServiceTest.php - @@ -1144,3 +1154,8 @@ parameters: message: "#^Parameter \\#1 \\$string of function trim expects string, string\\|false given\\.$#" count: 2 path: tests/phpunit/bootstrap.php + + - + message: "#^Parameter \\#2 \\$value of function ini_set expects string, int given\\.$#" + count: 1 + path: tests/phpunit/bootstrap.php diff --git a/phpstan.neon b/phpstan.neon index 81f6a4d..7f78664 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -4,6 +4,7 @@ includes: parameters: level: 9 + reportUnmatchedIgnoredErrors: false paths: - Civi - CRM @@ -17,6 +18,10 @@ parameters: - tests/bootstrap.php scanFiles: - paymentprocessingcore.civix.php + # CiviCRM Api4 entity classes that are dynamically generated at runtime. + # Must be scanFiles (not stubFiles) because stubFiles are overridden + # when scanDirectories loads the Civi\Api4 namespace from core. + - stubs/CiviApi4.stub.php - /build/site/web/sites/all/modules/civicrm/Civi.php # Test base classes needed for type information - tests/phpunit/BaseHeadlessTest.php diff --git a/stubs/CiviApi4.stub.php b/stubs/CiviApi4.stub.php new file mode 100644 index 0000000..5521bb9 --- /dev/null +++ b/stubs/CiviApi4.stub.php @@ -0,0 +1,91 @@ + 42, + 'contact_id' => 7, + 'amount' => 99.50, + 'currency' => 'GBP', + 'financial_type_id' => 3, + 'campaign_id' => 5, + 'next_sched_contribution_date' => '2025-07-15', + 'frequency_unit:name' => 'week', + 'frequency_interval' => 2, + ]; + + $dto = RecurringContributionDTO::fromApiResult($record); + + $this->assertSame(42, $dto->getId()); + $this->assertSame(7, $dto->getContactId()); + $this->assertSame(99.50, $dto->getAmount()); + $this->assertSame('GBP', $dto->getCurrency()); + $this->assertSame(3, $dto->getFinancialTypeId()); + $this->assertSame(5, $dto->getCampaignId()); + $this->assertSame('2025-07-15', $dto->getNextSchedContributionDate()); + $this->assertSame('week', $dto->getFrequencyUnit()); + $this->assertSame(2, $dto->getFrequencyInterval()); + } + + /** + * Tests that null campaign_id returns null. + */ + public function testFromApiResultWithNullCampaignId(): void { + $record = [ + 'id' => 1, + 'contact_id' => 2, + 'amount' => 10.00, + 'currency' => 'USD', + 'financial_type_id' => 1, + 'campaign_id' => NULL, + 'next_sched_contribution_date' => '2025-07-15', + 'frequency_unit:name' => 'month', + 'frequency_interval' => 1, + ]; + + $dto = RecurringContributionDTO::fromApiResult($record); + + $this->assertNull($dto->getCampaignId()); + } + + /** + * Tests default frequency values when not provided. + */ + public function testFromApiResultWithDefaultFrequency(): void { + $record = [ + 'id' => 1, + 'contact_id' => 2, + 'amount' => 10.00, + 'currency' => 'USD', + 'financial_type_id' => 1, + 'next_sched_contribution_date' => '2025-07-15', + ]; + + $dto = RecurringContributionDTO::fromApiResult($record); + + $this->assertSame('month', $dto->getFrequencyUnit()); + $this->assertSame(1, $dto->getFrequencyInterval()); + } + + /** + * Tests that missing id throws InvalidArgumentException. + */ + public function testFromApiResultThrowsOnMissingId(): void { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('numeric id'); + + RecurringContributionDTO::fromApiResult([ + 'contact_id' => 2, + 'next_sched_contribution_date' => '2025-07-15', + ]); + } + + /** + * Tests that missing next_sched_contribution_date throws. + */ + public function testFromApiResultThrowsOnMissingDate(): void { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('next_sched_contribution_date'); + + RecurringContributionDTO::fromApiResult([ + 'id' => 1, + 'contact_id' => 2, + ]); + } + + /** + * Tests that missing contact_id throws InvalidArgumentException. + */ + public function testFromApiResultThrowsOnMissingContactId(): void { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('contact_id'); + + RecurringContributionDTO::fromApiResult([ + 'id' => 1, + 'next_sched_contribution_date' => '2025-07-15', + ]); + } + +} diff --git a/tests/phpunit/Civi/Paymentprocessingcore/Service/InstalmentGenerationServiceTest.php b/tests/phpunit/Civi/Paymentprocessingcore/Service/InstalmentGenerationServiceTest.php new file mode 100644 index 0000000..9a88430 --- /dev/null +++ b/tests/phpunit/Civi/Paymentprocessingcore/Service/InstalmentGenerationServiceTest.php @@ -0,0 +1,911 @@ +service = new InstalmentGenerationService(); + } + + /** + * Tests happy path: generates instalment for due recurring contribution. + */ + public function testGeneratesInstalmentForDueRecurring(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + $recurId = $this->createRecurringContribution($contactId, $processorId, [ + 'contribution_status_id:name' => 'In Progress', + 'next_sched_contribution_date' => date('Y-m-d', strtotime('-1 day')), + ]); + + $result = $this->service->generateInstalments(self::PROCESSOR_TYPE, 500); + + $this->assertEquals(1, $result['created']); + $this->assertEquals(0, $result['skipped']); + $this->assertEquals(0, $result['errored']); + + // Verify the new contribution exists with correct fields from recurring. + $contributions = \Civi\Api4\Contribution::get(FALSE) + ->addSelect('contribution_status_id:name', 'contact_id', 'total_amount', 'currency', 'contribution_recur_id', 'is_pay_later') + ->addWhere('contribution_recur_id', '=', $recurId) + ->addWhere('contribution_status_id:name', '=', 'Pending') + ->execute(); + + $this->assertCount(1, $contributions); + $newContribution = $contributions->first(); + $this->assertIsArray($newContribution); + $this->assertEquals($contactId, $newContribution['contact_id']); + $this->assertEquals(50.00, (float) $newContribution['total_amount']); + $this->assertEquals('GBP', $newContribution['currency']); + $this->assertEquals($recurId, $newContribution['contribution_recur_id']); + $this->assertEquals(0, (int) $newContribution['is_pay_later']); + + // Verify schedule date was advanced. + $recur = \Civi\Api4\ContributionRecur::get(FALSE) + ->addSelect('next_sched_contribution_date') + ->addWhere('id', '=', $recurId) + ->execute() + ->first(); + + $this->assertNotNull($recur); + $expectedDate = date('Y-m-d', strtotime('+1 month -1 day')); + $this->assertEquals($expectedDate, date('Y-m-d', strtotime($recur['next_sched_contribution_date']))); + } + + /** + * Tests that membership-linked recurring contributions are skipped. + */ + public function testSkipsMembershipLinkedRecurring(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + $recurId = $this->createRecurringContribution($contactId, $processorId, [ + 'contribution_status_id:name' => 'In Progress', + 'next_sched_contribution_date' => date('Y-m-d', strtotime('-1 day')), + ]); + + // Link to membership. + $this->createMembership($contactId, $recurId); + + $result = $this->service->generateInstalments(self::PROCESSOR_TYPE, 500); + + // Membership-linked recurrings are excluded at the DB query level, + // so they don't appear in any counter. + $this->assertEquals(0, $result['created']); + $this->assertEquals(0, $result['skipped']); + $this->assertEquals(0, $result['errored']); + } + + /** + * Tests idempotency: skips when Pending contribution already exists. + */ + public function testIdempotencySkipsDuplicate(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + $schedDate = date('Y-m-d', strtotime('-1 day')); + $recurId = $this->createRecurringContribution($contactId, $processorId, [ + 'contribution_status_id:name' => 'In Progress', + 'next_sched_contribution_date' => $schedDate, + ]); + + // Create existing Pending contribution with same date. + $this->createContribution($contactId, $recurId, [ + 'contribution_status_id:name' => 'Pending', + 'receive_date' => $schedDate, + ]); + + $result = $this->service->generateInstalments(self::PROCESSOR_TYPE, 500); + + $this->assertEquals(0, $result['created']); + $this->assertEquals(1, $result['skipped']); + } + + /** + * Tests that non-matching processor type is not processed. + */ + public function testSkipsNonMatchingProcessorType(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + $this->createRecurringContribution($contactId, $processorId, [ + 'contribution_status_id:name' => 'In Progress', + 'next_sched_contribution_date' => date('Y-m-d', strtotime('-1 day')), + ]); + + // Query with a processor type that doesn't match Dummy. + $result = $this->service->generateInstalments('NonExistentProcessor', 500); + + $this->assertEquals(0, $result['created']); + $this->assertEquals(0, $result['skipped']); + } + + /** + * Tests that future scheduled dates are not processed. + */ + public function testSkipsFutureScheduledDate(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + $this->createRecurringContribution($contactId, $processorId, [ + 'contribution_status_id:name' => 'In Progress', + 'next_sched_contribution_date' => date('Y-m-d', strtotime('+7 days')), + ]); + + $result = $this->service->generateInstalments(self::PROCESSOR_TYPE, 500); + + $this->assertEquals(0, $result['created']); + $this->assertEquals(0, $result['skipped']); + } + + /** + * Tests that non-In Progress statuses are not processed. + */ + public function testSkipsNonInProgressStatus(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + + foreach (['Cancelled', 'Pending', 'Failed'] as $status) { + $this->createRecurringContribution($contactId, $processorId, [ + 'contribution_status_id:name' => $status, + 'next_sched_contribution_date' => date('Y-m-d', strtotime('-1 day')), + ]); + } + + $result = $this->service->generateInstalments(self::PROCESSOR_TYPE, 500); + + $this->assertEquals(0, $result['created']); + } + + /** + * Tests that schedule date advances correctly for different intervals. + */ + public function testAdvancesScheduleDateCorrectly(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + $baseDate = '2025-06-15'; + + $cases = [ + ['unit' => 'month', 'interval' => 1, 'expected' => '2025-07-15'], + ['unit' => 'week', 'interval' => 2, 'expected' => '2025-06-29'], + ['unit' => 'year', 'interval' => 1, 'expected' => '2026-06-15'], + ['unit' => 'day', 'interval' => 30, 'expected' => '2025-07-15'], + ]; + + foreach ($cases as $case) { + $recurId = $this->createRecurringContribution($contactId, $processorId, [ + 'contribution_status_id:name' => 'In Progress', + 'next_sched_contribution_date' => $baseDate, + 'frequency_unit:name' => $case['unit'], + 'frequency_interval' => $case['interval'], + ]); + + $this->service->advanceScheduleDate( + $recurId, + $baseDate, + $case['unit'], + $case['interval'] + ); + + $recur = \Civi\Api4\ContributionRecur::get(FALSE) + ->addSelect('next_sched_contribution_date') + ->addWhere('id', '=', $recurId) + ->execute() + ->first(); + + $this->assertNotNull($recur); + $this->assertEquals( + $case['expected'], + date('Y-m-d', strtotime($recur['next_sched_contribution_date'])), + sprintf('Failed for unit=%s interval=%d', $case['unit'], $case['interval']) + ); + } + } + + /** + * Tests that batch size limits the number of records processed. + */ + public function testBatchSizeLimitsProcessing(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + + for ($i = 0; $i < 3; $i++) { + $this->createRecurringContribution($contactId, $processorId, [ + 'contribution_status_id:name' => 'In Progress', + 'next_sched_contribution_date' => date('Y-m-d', strtotime('-' . ($i + 1) . ' days')), + ]); + } + + $result = $this->service->generateInstalments(self::PROCESSOR_TYPE, 2); + + $this->assertLessThanOrEqual(2, $result['created']); + } + + /** + * Tests that one failure does not stop the batch. + */ + public function testContinuesOnSingleRecordError(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + + // Create a valid recurring contribution. + $recurId1 = $this->createRecurringContribution($contactId, $processorId, [ + 'contribution_status_id:name' => 'In Progress', + 'next_sched_contribution_date' => date('Y-m-d', strtotime('-1 day')), + ]); + + // Create a recurring contribution without financial_type_id (will cause error). + $recurId2 = $this->createRecurringContribution($contactId, $processorId, [ + 'contribution_status_id:name' => 'In Progress', + 'next_sched_contribution_date' => date('Y-m-d', strtotime('-2 days')), + ]); + + // Remove financial_type_id to trigger error during contribution creation. + \CRM_Core_DAO::executeQuery( + 'UPDATE civicrm_contribution_recur SET financial_type_id = NULL WHERE id = %1', + [1 => [$recurId2, 'Integer']] + ); + + $result = $this->service->generateInstalments(self::PROCESSOR_TYPE, 500); + + // At least one should succeed or error - batch continues. + $totalProcessed = $result['created'] + $result['errored']; + $this->assertGreaterThan(0, $totalProcessed); + $this->assertGreaterThan(0, $result['errored']); + $this->assertEmpty($result['errors'][$recurId1] ?? NULL); + } + + /** + * Tests that Failed contribution also blocks re-creation (idempotency). + */ + public function testIdempotencyChecksAllStatuses(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + $schedDate = date('Y-m-d', strtotime('-1 day')); + + // Test instalmentExists directly for each blocking status. + $statuses = ['Pending', 'Completed', 'Failed', 'Cancelled']; + foreach ($statuses as $status) { + $recurId = $this->createRecurringContribution($contactId, $processorId, [ + 'contribution_status_id:name' => 'In Progress', + 'next_sched_contribution_date' => $schedDate, + ]); + + // Create contribution with the blocking status. + $this->createContribution($contactId, $recurId, [ + 'contribution_status_id:name' => $status, + 'receive_date' => $schedDate, + ]); + + $this->assertTrue( + $this->service->instalmentExists($recurId, $schedDate), + "Expected instalmentExists to return TRUE for status: $status" + ); + } + } + + /** + * Tests getDueRecurringContributions returns correct fields. + */ + public function testGetDueRecurringContributionsReturnsCorrectFields(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + $this->createRecurringContribution($contactId, $processorId, [ + 'contribution_status_id:name' => 'In Progress', + 'next_sched_contribution_date' => date('Y-m-d', strtotime('-1 day')), + 'frequency_unit:name' => 'week', + 'frequency_interval' => 2, + ]); + + $results = $this->service->getDueRecurringContributions(self::PROCESSOR_TYPE, 500); + + $this->assertCount(1, $results); + $record = $results[0]; + $this->assertInstanceOf(RecurringContributionDTO::class, $record); + $this->assertGreaterThan(0, $record->getId()); + $this->assertNotEmpty($record->getNextSchedContributionDate()); + $this->assertGreaterThan(0, $record->getContactId()); + $this->assertGreaterThan(0, $record->getAmount()); + $this->assertNotEmpty($record->getCurrency()); + $this->assertGreaterThan(0, $record->getFinancialTypeId()); + $this->assertEquals('week', $record->getFrequencyUnit()); + $this->assertEquals(2, $record->getFrequencyInterval()); + } + + /** + * Tests getDueRecurringContributions excludes membership-linked records. + */ + public function testGetDueRecurringContributionsExcludesMembership(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + + // Non-membership recur (should be returned). + $this->createRecurringContribution($contactId, $processorId, [ + 'contribution_status_id:name' => 'In Progress', + 'next_sched_contribution_date' => date('Y-m-d', strtotime('-1 day')), + ]); + + // Membership-linked recur (should be excluded). + $membershipRecurId = $this->createRecurringContribution($contactId, $processorId, [ + 'contribution_status_id:name' => 'In Progress', + 'next_sched_contribution_date' => date('Y-m-d', strtotime('-1 day')), + ]); + $this->createMembership($contactId, $membershipRecurId); + + $results = $this->service->getDueRecurringContributions(self::PROCESSOR_TYPE, 500); + + $this->assertCount(1, $results); + } + + /** + * Tests getDueRecurringContributions respects batch size. + */ + public function testGetDueRecurringContributionsRespectsBatchSize(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + + for ($i = 0; $i < 5; $i++) { + $this->createRecurringContribution($contactId, $processorId, [ + 'contribution_status_id:name' => 'In Progress', + 'next_sched_contribution_date' => date('Y-m-d', strtotime('-' . ($i + 1) . ' days')), + ]); + } + + $results = $this->service->getDueRecurringContributions(self::PROCESSOR_TYPE, 3); + + $this->assertCount(3, $results); + } + + /** + * Tests getDueRecurringContributions filters by processor type. + */ + public function testGetDueRecurringContributionsFiltersByProcessorType(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + $this->createRecurringContribution($contactId, $processorId, [ + 'contribution_status_id:name' => 'In Progress', + 'next_sched_contribution_date' => date('Y-m-d', strtotime('-1 day')), + ]); + + $results = $this->service->getDueRecurringContributions('NonExistent', 500); + + $this->assertCount(0, $results); + } + + /** + * Tests getDueRecurringContributions only returns In Progress status. + */ + public function testGetDueRecurringContributionsFiltersStatus(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + + $this->createRecurringContribution($contactId, $processorId, [ + 'contribution_status_id:name' => 'In Progress', + 'next_sched_contribution_date' => date('Y-m-d', strtotime('-1 day')), + ]); + $this->createRecurringContribution($contactId, $processorId, [ + 'contribution_status_id:name' => 'Cancelled', + 'next_sched_contribution_date' => date('Y-m-d', strtotime('-1 day')), + ]); + $this->createRecurringContribution($contactId, $processorId, [ + 'contribution_status_id:name' => 'Pending', + 'next_sched_contribution_date' => date('Y-m-d', strtotime('-1 day')), + ]); + + $results = $this->service->getDueRecurringContributions(self::PROCESSOR_TYPE, 500); + + $this->assertCount(1, $results); + } + + /** + * Tests getDueRecurringContributions excludes future dates. + */ + public function testGetDueRecurringContributionsExcludesFutureDates(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + + $this->createRecurringContribution($contactId, $processorId, [ + 'contribution_status_id:name' => 'In Progress', + 'next_sched_contribution_date' => date('Y-m-d', strtotime('-1 day')), + ]); + $this->createRecurringContribution($contactId, $processorId, [ + 'contribution_status_id:name' => 'In Progress', + 'next_sched_contribution_date' => date('Y-m-d', strtotime('+7 days')), + ]); + + $results = $this->service->getDueRecurringContributions(self::PROCESSOR_TYPE, 500); + + $this->assertCount(1, $results); + } + + /** + * Tests getDueRecurringContributions returns empty array when no matches. + */ + public function testGetDueRecurringContributionsReturnsEmptyWhenNoMatches(): void { + $results = $this->service->getDueRecurringContributions(self::PROCESSOR_TYPE, 500); + + $this->assertCount(0, $results); + } + + /** + * Tests instalmentExists returns FALSE when no contribution exists. + */ + public function testInstalmentExistsReturnsFalseWhenNone(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + $recurId = $this->createRecurringContribution($contactId, $processorId); + + $this->assertFalse($this->service->instalmentExists($recurId, date('Y-m-d'))); + } + + /** + * Tests instalmentExists returns FALSE for different date. + */ + public function testInstalmentExistsReturnsFalseForDifferentDate(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + $recurId = $this->createRecurringContribution($contactId, $processorId); + + $this->createContribution($contactId, $recurId, [ + 'contribution_status_id:name' => 'Pending', + 'receive_date' => '2025-06-15', + ]); + + // Check a different date. + $this->assertFalse($this->service->instalmentExists($recurId, '2025-07-15')); + } + + /** + * Tests instalmentExists returns FALSE for different recur ID. + */ + public function testInstalmentExistsReturnsFalseForDifferentRecur(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + $recurId1 = $this->createRecurringContribution($contactId, $processorId); + $recurId2 = $this->createRecurringContribution($contactId, $processorId); + + $date = date('Y-m-d', strtotime('-1 day')); + $this->createContribution($contactId, $recurId1, [ + 'contribution_status_id:name' => 'Pending', + 'receive_date' => $date, + ]); + + // Different recur ID should not match. + $this->assertFalse($this->service->instalmentExists($recurId2, $date)); + } + + /** + * Tests instalmentExists matches datetime receive_date against date-only input. + */ + public function testInstalmentExistsHandlesDatetimeVsDate(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + $recurId = $this->createRecurringContribution($contactId, $processorId); + + // Contribution stored with full datetime. + $this->createContribution($contactId, $recurId, [ + 'contribution_status_id:name' => 'Pending', + 'receive_date' => '2025-06-15 14:30:00', + ]); + + // Check with date-only string should still match. + $this->assertTrue($this->service->instalmentExists($recurId, '2025-06-15')); + } + + /** + * Tests createInstalment returns a contribution ID. + */ + public function testCreateInstalmentReturnsContributionId(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + $recurId = $this->createRecurringContribution($contactId, $processorId); + + $recur = $this->getRecurRecord($recurId); + $receiveDate = date('Y-m-d'); + $contributionId = $this->service->createInstalment($recur, $receiveDate); + + $this->assertGreaterThan(0, $contributionId); + } + + /** + * Tests createInstalment creates contribution with Pending status. + */ + public function testCreateInstalmentCreatesPendingContribution(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + $recurId = $this->createRecurringContribution($contactId, $processorId); + + $recur = $this->getRecurRecord($recurId); + $receiveDate = date('Y-m-d'); + $contributionId = $this->service->createInstalment($recur, $receiveDate); + + $contribution = \Civi\Api4\Contribution::get(FALSE) + ->addSelect('contribution_status_id:name', 'contribution_recur_id') + ->addWhere('id', '=', $contributionId) + ->execute() + ->first(); + + $this->assertNotNull($contribution); + $this->assertEquals('Pending', $contribution['contribution_status_id:name']); + $this->assertEquals($recurId, $contribution['contribution_recur_id']); + } + + /** + * Tests createInstalment copies campaign_id when present in recur record. + */ + public function testCreateInstalmentCopiesCampaignId(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + + /** @var array{id: int, values: array} $campaignResult */ + $campaignResult = civicrm_api3('Campaign', 'create', [ + 'title' => 'Test Campaign', + 'name' => 'test_campaign', + ]); + $campaignId = (int) $campaignResult['values'][$campaignResult['id']]['id']; + + $recurId = $this->createRecurringContribution($contactId, $processorId); + $recur = RecurringContributionDTO::fromApiResult([ + 'id' => $recurId, + 'contact_id' => $contactId, + 'amount' => 50.00, + 'currency' => 'GBP', + 'financial_type_id' => 1, + 'campaign_id' => $campaignId, + 'next_sched_contribution_date' => date('Y-m-d'), + ]); + + $contributionId = $this->service->createInstalment($recur, date('Y-m-d')); + + $result = \CRM_Core_DAO::singleValueQuery( + 'SELECT campaign_id FROM civicrm_contribution WHERE id = %1', + [1 => [$contributionId, 'Integer']] + ); + + $this->assertEquals($campaignId, (int) $result); + } + + /** + * Tests createInstalment sets is_pay_later to 0. + */ + public function testCreateInstalmentSetsIsPayLaterToZero(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + $recurId = $this->createRecurringContribution($contactId, $processorId); + + $recur = $this->getRecurRecord($recurId); + $contributionId = $this->service->createInstalment($recur, date('Y-m-d')); + + $contribution = \Civi\Api4\Contribution::get(FALSE) + ->addSelect('is_pay_later') + ->addWhere('id', '=', $contributionId) + ->execute() + ->first(); + + $this->assertNotNull($contribution); + $this->assertEquals(0, (int) $contribution['is_pay_later']); + } + + /** + * Tests end-of-month handling for monthly schedule advancement. + */ + public function testAdvanceScheduleDateHandlesEndOfMonth(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + + $cases = [ + ['from' => '2025-01-31', 'expected' => '2025-02-28'], + ['from' => '2025-02-28', 'expected' => '2025-03-28'], + ['from' => '2025-03-31', 'expected' => '2025-04-30'], + ['from' => '2024-01-31', 'expected' => '2024-02-29'], + ['from' => '2025-01-30', 'expected' => '2025-02-28'], + ['from' => '2025-05-31', 'expected' => '2025-06-30'], + ]; + + foreach ($cases as $case) { + $recurId = $this->createRecurringContribution($contactId, $processorId, [ + 'next_sched_contribution_date' => $case['from'], + ]); + + $this->service->advanceScheduleDate($recurId, $case['from'], 'month', 1); + + $recur = \Civi\Api4\ContributionRecur::get(FALSE) + ->addSelect('next_sched_contribution_date') + ->addWhere('id', '=', $recurId) + ->execute() + ->first(); + + $this->assertNotNull($recur); + $this->assertEquals( + $case['expected'], + date('Y-m-d', strtotime($recur['next_sched_contribution_date'])), + sprintf('Failed for %s + 1 month', $case['from']) + ); + } + } + + /** + * Tests advanceScheduleDate updates the database record. + */ + public function testAdvanceScheduleDateUpdatesRecord(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + $recurId = $this->createRecurringContribution($contactId, $processorId, [ + 'next_sched_contribution_date' => '2025-03-01', + ]); + + $this->service->advanceScheduleDate($recurId, '2025-03-01', 'month', 1); + + $recur = \Civi\Api4\ContributionRecur::get(FALSE) + ->addSelect('next_sched_contribution_date') + ->addWhere('id', '=', $recurId) + ->execute() + ->first(); + + $this->assertNotNull($recur); + $this->assertEquals('2025-04-01', date('Y-m-d', strtotime($recur['next_sched_contribution_date']))); + } + + /** + * Tests advanceScheduleDate does not affect other recurring records. + */ + public function testAdvanceScheduleDateDoesNotAffectOtherRecords(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + $recurId1 = $this->createRecurringContribution($contactId, $processorId, [ + 'next_sched_contribution_date' => '2025-03-01', + ]); + $recurId2 = $this->createRecurringContribution($contactId, $processorId, [ + 'next_sched_contribution_date' => '2025-03-01', + ]); + + $this->service->advanceScheduleDate($recurId1, '2025-03-01', 'month', 1); + + $recur2 = \Civi\Api4\ContributionRecur::get(FALSE) + ->addSelect('next_sched_contribution_date') + ->addWhere('id', '=', $recurId2) + ->execute() + ->first(); + + $this->assertNotNull($recur2); + $this->assertEquals('2025-03-01', date('Y-m-d', strtotime($recur2['next_sched_contribution_date']))); + } + + /** + * Tests injectable reference date controls which contributions are due. + */ + public function testInjectableReferenceDateControlsDueContributions(): void { + $processorId = $this->createPaymentProcessor(); + $contactId = $this->createContact(); + $this->createRecurringContribution($contactId, $processorId, [ + 'contribution_status_id:name' => 'In Progress', + 'next_sched_contribution_date' => '2025-06-15', + ]); + + // Reference date before the scheduled date — should not pick it up. + $result = $this->service->generateInstalments(self::PROCESSOR_TYPE, 500, '2025-06-14'); + $this->assertEquals(0, $result['created']); + + // Reference date on the scheduled date — should pick it up. + $result = $this->service->generateInstalments(self::PROCESSOR_TYPE, 500, '2025-06-15'); + $this->assertEquals(1, $result['created']); + } + + /** + * Get a recurring contribution record with fields needed by createInstalment. + * + * @param int $recurId + * The recurring contribution ID. + * + * @return \Civi\Paymentprocessingcore\DTO\RecurringContributionDTO + * The recurring contribution DTO. + */ + private function getRecurRecord(int $recurId): RecurringContributionDTO { + $recur = \Civi\Api4\ContributionRecur::get(FALSE) + ->addSelect('id', 'contact_id', 'amount', 'currency', 'financial_type_id', 'campaign_id', 'next_sched_contribution_date') + ->addWhere('id', '=', $recurId) + ->execute() + ->first(); + + if (!is_array($recur)) { + throw new \RuntimeException('Failed to get ContributionRecur ' . $recurId); + } + + return RecurringContributionDTO::fromApiResult($recur); + } + + /** + * Create a payment processor using the built-in Dummy type. + * + * @return int + * The payment processor ID. + */ + private function createPaymentProcessor(): int { + $processor = \Civi\Api4\PaymentProcessor::create(FALSE) + ->addValue('name', 'Test Processor') + ->addValue('payment_processor_type_id:name', self::PROCESSOR_TYPE) + ->addValue('class_name', 'Payment_Dummy') + ->addValue('is_active', TRUE) + ->addValue('is_test', FALSE) + ->addValue('domain_id', 1) + ->execute() + ->first(); + + if (!is_array($processor)) { + throw new \RuntimeException('Failed to create PaymentProcessor'); + } + + return (int) $processor['id']; + } + + /** + * Create a contact. + * + * @return int + * The contact ID. + */ + private function createContact(): int { + $contact = \Civi\Api4\Contact::create(FALSE) + ->addValue('contact_type', 'Individual') + ->addValue('first_name', 'Test') + ->addValue('last_name', 'User') + ->execute() + ->first(); + + if (!is_array($contact)) { + throw new \RuntimeException('Failed to create Contact'); + } + + return (int) $contact['id']; + } + + /** + * Create a recurring contribution. + * + * @param int $contactId + * The contact ID. + * @param int $processorId + * The payment processor ID. + * @param array $params + * Additional parameters. + * + * @return int + * The recurring contribution ID. + */ + private function createRecurringContribution(int $contactId, int $processorId, array $params = []): int { + $defaults = [ + 'contact_id' => $contactId, + 'payment_processor_id' => $processorId, + 'amount' => 50.00, + 'currency' => 'GBP', + 'financial_type_id:name' => 'Donation', + 'frequency_unit:name' => 'month', + 'frequency_interval' => 1, + 'start_date' => date('Y-m-d', strtotime('-6 months')), + 'contribution_status_id:name' => 'In Progress', + 'next_sched_contribution_date' => date('Y-m-d'), + ]; + + $values = array_merge($defaults, $params); + + $recur = \Civi\Api4\ContributionRecur::create(FALSE) + ->setValues($values) + ->execute() + ->first(); + + if (!is_array($recur)) { + throw new \RuntimeException('Failed to create ContributionRecur'); + } + + return (int) $recur['id']; + } + + /** + * Create a contribution. + * + * @param int $contactId + * The contact ID. + * @param int $recurId + * The recurring contribution ID. + * @param array $params + * Additional parameters. + * + * @return int + * The contribution ID. + */ + private function createContribution(int $contactId, int $recurId, array $params = []): int { + $defaults = [ + 'contact_id' => $contactId, + 'contribution_recur_id' => $recurId, + 'financial_type_id:name' => 'Donation', + 'total_amount' => 50.00, + 'currency' => 'GBP', + 'contribution_status_id:name' => 'Completed', + 'receive_date' => date('Y-m-d'), + ]; + + $values = array_merge($defaults, $params); + + $contribution = \Civi\Api4\Contribution::create(FALSE) + ->setValues($values) + ->execute() + ->first(); + + if (!is_array($contribution)) { + throw new \RuntimeException('Failed to create Contribution'); + } + + return (int) $contribution['id']; + } + + /** + * Create a membership linked to a recurring contribution. + * + * @param int $contactId + * The contact ID. + * @param int $recurId + * The recurring contribution ID. + * + * @return int + * The membership ID. + */ + private function createMembership(int $contactId, int $recurId): int { + // Get or create membership type. + $membershipType = \Civi\Api4\MembershipType::get(FALSE) + ->addSelect('id') + ->execute() + ->first(); + + if (!is_array($membershipType)) { + $membershipType = \Civi\Api4\MembershipType::create(FALSE) + ->addValue('name', 'Test Membership') + ->addValue('member_of_contact_id', $contactId) + ->addValue('financial_type_id:name', 'Member Dues') + ->addValue('duration_unit', 'year') + ->addValue('duration_interval', 1) + ->addValue('period_type', 'rolling') + ->execute() + ->first(); + } + + if (!is_array($membershipType)) { + throw new \RuntimeException('Failed to create MembershipType'); + } + + $membership = \Civi\Api4\Membership::create(FALSE) + ->addValue('contact_id', $contactId) + ->addValue('membership_type_id', $membershipType['id']) + ->addValue('contribution_recur_id', $recurId) + ->addValue('join_date', date('Y-m-d')) + ->addValue('start_date', date('Y-m-d')) + ->addValue('status_id:name', 'Current') + ->execute() + ->first(); + + if (!is_array($membership)) { + throw new \RuntimeException('Failed to create Membership'); + } + + return (int) $membership['id']; + } + +}