diff --git a/Civi/Paymentprocessingcore/Event/ReconcilePaymentAttemptBatchEvent.php b/Civi/Paymentprocessingcore/Event/ReconcilePaymentAttemptBatchEvent.php index d5148bc..0ed756c 100644 --- a/Civi/Paymentprocessingcore/Event/ReconcilePaymentAttemptBatchEvent.php +++ b/Civi/Paymentprocessingcore/Event/ReconcilePaymentAttemptBatchEvent.php @@ -12,10 +12,12 @@ * their API for the real status of stuck payments and report results. * * Two usage patterns: - * - Stripe: Uses getAttempts() to iterate PaymentAttempt records, calls - * setAttemptResult() for each. - * - GoCardless: Uses getProcessorType()/getThresholdDays()/getRemainingBudget() - * as trigger + config; queries its own data internally. + * - PaymentAttempt-based (e.g. Stripe): iterate getAttempts() and call + * setAttemptResult() for each; the core builds the run summary from those + * results. + * - Custom-query (e.g. GoCardless): use getProcessorType()/getThresholdDays()/ + * getRemainingBudget() as trigger + config, reconcile own data internally, and + * report totals via reportCounts() so they are included in the run summary. */ class ReconcilePaymentAttemptBatchEvent extends GenericHookEvent { @@ -31,6 +33,18 @@ class ReconcilePaymentAttemptBatchEvent extends GenericHookEvent { */ private array $results = []; + /** + * Counts reported directly by custom-query handlers. + * + * @var array{reconciled: int, unchanged: int, errored: int, unhandled: int} + */ + private array $reportedCounts = [ + 'reconciled' => 0, + 'unchanged' => 0, + 'errored' => 0, + 'unhandled' => 0, + ]; + /** * Constructor. * @@ -151,4 +165,37 @@ public function hasAttemptResult(int $attemptId): bool { return array_key_exists($attemptId, $this->results); } + /** + * Report reconciliation counts directly, for handlers that do not use + * PaymentAttempt records (the custom-query pattern, e.g. GoCardless). + * + * Counts are additive, so a handler may call this more than once. Handlers + * using setAttemptResult() must not call this — the core counts their results + * separately, and mixing the two would double-count. + * + * @param int $reconciled + * @param int $unchanged + * @param int $errored + * @param int $unhandled + */ + public function reportCounts(int $reconciled, int $unchanged = 0, int $errored = 0, int $unhandled = 0): void { + if ($reconciled < 0 || $unchanged < 0 || $errored < 0 || $unhandled < 0) { + throw new \InvalidArgumentException('Reconciliation counts must be non-negative'); + } + + $this->reportedCounts['reconciled'] += $reconciled; + $this->reportedCounts['unchanged'] += $unchanged; + $this->reportedCounts['errored'] += $errored; + $this->reportedCounts['unhandled'] += $unhandled; + } + + /** + * Get the counts reported by custom-query handlers. + * + * @return array{reconciled: int, unchanged: int, errored: int, unhandled: int} + */ + public function getReportedCounts(): array { + return $this->reportedCounts; + } + } diff --git a/Civi/Paymentprocessingcore/Service/PaymentAttemptReconcileService.php b/Civi/Paymentprocessingcore/Service/PaymentAttemptReconcileService.php index 5c3836d..77ec418 100644 --- a/Civi/Paymentprocessingcore/Service/PaymentAttemptReconcileService.php +++ b/Civi/Paymentprocessingcore/Service/PaymentAttemptReconcileService.php @@ -269,6 +269,16 @@ private function reconcileByProcessor(string $processorType, int $thresholdDays, } } + // Fold in counts reported by custom-query handlers (e.g. GoCardless), which + // reconcile their own data and report totals directly rather than through + // PaymentAttempt records. Handlers on the setAttemptResult() path (e.g. + // Stripe) do not call reportCounts(), so this adds zero for them. + $reported = $event->getReportedCounts(); + $summary['reconciled'] += $reported['reconciled']; + $summary['unchanged'] += $reported['unchanged']; + $summary['errored'] += $reported['errored']; + $summary['unhandled'] += $reported['unhandled']; + return $summary; } diff --git a/tests/phpunit/Civi/Paymentprocessingcore/Event/ReconcilePaymentAttemptBatchEventTest.php b/tests/phpunit/Civi/Paymentprocessingcore/Event/ReconcilePaymentAttemptBatchEventTest.php index 46417cc..d47a9fb 100644 --- a/tests/phpunit/Civi/Paymentprocessingcore/Event/ReconcilePaymentAttemptBatchEventTest.php +++ b/tests/phpunit/Civi/Paymentprocessingcore/Event/ReconcilePaymentAttemptBatchEventTest.php @@ -242,4 +242,75 @@ public function testGetCompletionDataReturnsNullByDefault(): void { $this->assertNull($result->getCompletionData()); } + // ------------------------------------------------------------------------- + // Reported-counts tests (custom-query pattern, e.g. GoCardless) + // ------------------------------------------------------------------------- + + /** + * Tests reported counts default to zero. + */ + public function testReportedCountsDefaultToZero(): void { + $event = new ReconcilePaymentAttemptBatchEvent('GoCardless', [], 7, 50); + + $this->assertEquals( + ['reconciled' => 0, 'unchanged' => 0, 'errored' => 0, 'unhandled' => 0], + $event->getReportedCounts() + ); + } + + /** + * Tests reportCounts records the reported totals. + */ + public function testReportCountsRecordsTotals(): void { + $event = new ReconcilePaymentAttemptBatchEvent('GoCardless', [], 7, 50); + + $event->reportCounts(99, 1, 2, 3); + + $this->assertEquals( + ['reconciled' => 99, 'unchanged' => 1, 'errored' => 2, 'unhandled' => 3], + $event->getReportedCounts() + ); + } + + /** + * Tests reportCounts is additive across multiple calls. + */ + public function testReportCountsIsAdditive(): void { + $event = new ReconcilePaymentAttemptBatchEvent('GoCardless', [], 7, 50); + + $event->reportCounts(10, 1, 0, 0); + $event->reportCounts(5, 0, 2, 1); + + $this->assertEquals( + ['reconciled' => 15, 'unchanged' => 1, 'errored' => 2, 'unhandled' => 1], + $event->getReportedCounts() + ); + } + + /** + * Tests reportCounts defaults the optional buckets to zero. + */ + public function testReportCountsDefaultsOptionalBucketsToZero(): void { + $event = new ReconcilePaymentAttemptBatchEvent('GoCardless', [], 7, 50); + + $event->reportCounts(4); + + $this->assertEquals( + ['reconciled' => 4, 'unchanged' => 0, 'errored' => 0, 'unhandled' => 0], + $event->getReportedCounts() + ); + } + + /** + * Tests reportCounts rejects negative counts. + */ + public function testReportCountsRejectsNegativeValues(): void { + $event = new ReconcilePaymentAttemptBatchEvent('GoCardless', [], 7, 50); + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Reconciliation counts must be non-negative'); + + $event->reportCounts(-1); + } + } diff --git a/tests/phpunit/Civi/Paymentprocessingcore/Service/PaymentAttemptReconcileServiceTest.php b/tests/phpunit/Civi/Paymentprocessingcore/Service/PaymentAttemptReconcileServiceTest.php index cf92fe5..926973a 100644 --- a/tests/phpunit/Civi/Paymentprocessingcore/Service/PaymentAttemptReconcileServiceTest.php +++ b/tests/phpunit/Civi/Paymentprocessingcore/Service/PaymentAttemptReconcileServiceTest.php @@ -874,6 +874,113 @@ public function testCancelledResultLeavesPayLaterContributionAlone(): void { } } + // ------------------------------------------------------------------------- + // Reported-counts tests (custom-query pattern, e.g. GoCardless) + // ------------------------------------------------------------------------- + + /** + * Tests that counts reported via reportCounts() appear in the run summary, + * even with zero PaymentAttempt records (the GoCardless pattern). + */ + public function testReportedCountsFromCustomQueryHandlerAppearInSummary(): void { + // No PaymentAttempt records are created — GoCardless does not use them. + $subscriber = function (ReconcilePaymentAttemptBatchEvent $event): void { + if ($event->getProcessorType() === 'GoCardless') { + $event->reportCounts(99, 1, 2, 3); + } + }; + \Civi::dispatcher()->addListener(ReconcilePaymentAttemptBatchEvent::NAME, $subscriber, -10); + + try { + $result = $this->service->reconcileStuckAttempts(['GoCardless' => 7], 100); + + $this->assertEquals(99, $result['reconciled']); + $this->assertEquals(1, $result['unchanged']); + $this->assertEquals(2, $result['errored']); + $this->assertEquals(3, $result['unhandled']); + } + finally { + \Civi::dispatcher()->removeListener(ReconcilePaymentAttemptBatchEvent::NAME, $subscriber); + } + } + + /** + * Regression guard for the PaymentAttempt/setAttemptResult() path (e.g. + * Stripe): a handler that reconciles via setAttemptResult() and never calls + * reportCounts() must produce exactly the same summary as before, and the + * reported-counts channel must stay empty. + */ + public function testSetAttemptResultPathUnaffectedWhenNoCountsReported(): void { + $this->createStuckPaymentAttempt([ + 'processor_type' => 'dummy', + 'days_ago' => 5, + ]); + + $reportedDuringRun = NULL; + $subscriber = function (ReconcilePaymentAttemptBatchEvent $event) use (&$reportedDuringRun): void { + foreach ($event->getAttempts() as $attemptId => $attempt) { + $event->setAttemptResult($attemptId, new ReconcileAttemptResult('completed', 'PI succeeded')); + } + // A PaymentAttempt-based handler never touches the reported-counts channel. + $reportedDuringRun = $event->getReportedCounts(); + }; + \Civi::dispatcher()->addListener(ReconcilePaymentAttemptBatchEvent::NAME, $subscriber, -10); + + try { + $result = $this->service->reconcileStuckAttempts([self::PROCESSOR_TYPE => 3], 100); + + $this->assertEquals(1, $result['reconciled']); + $this->assertEquals(0, $result['unchanged']); + $this->assertEquals(0, $result['errored']); + $this->assertEquals(0, $result['unhandled']); + $this->assertEquals( + ['reconciled' => 0, 'unchanged' => 0, 'errored' => 0, 'unhandled' => 0], + $reportedDuringRun + ); + } + finally { + \Civi::dispatcher()->removeListener(ReconcilePaymentAttemptBatchEvent::NAME, $subscriber); + } + } + + /** + * Tests that a PaymentAttempt-based processor (setAttemptResult) and a + * custom-query processor (reportCounts) in the same run sum correctly, + * without double-counting. + */ + public function testSetAttemptResultAndReportedCountsCombineWithoutDoubleCounting(): void { + // One Dummy attempt reconciled via setAttemptResult(). + $this->createStuckPaymentAttempt([ + 'processor_type' => 'dummy', + 'days_ago' => 5, + ]); + + $subscriber = function (ReconcilePaymentAttemptBatchEvent $event): void { + if ($event->getProcessorType() === self::PROCESSOR_TYPE) { + foreach ($event->getAttempts() as $attemptId => $attempt) { + $event->setAttemptResult($attemptId, new ReconcileAttemptResult('completed', 'PI succeeded')); + } + } + if ($event->getProcessorType() === 'GoCardless') { + $event->reportCounts(50, 0, 0, 0); + } + }; + \Civi::dispatcher()->addListener(ReconcilePaymentAttemptBatchEvent::NAME, $subscriber, -10); + + try { + $result = $this->service->reconcileStuckAttempts([ + self::PROCESSOR_TYPE => 3, + 'GoCardless' => 7, + ], 100); + + // 1 from the Dummy attempt path + 50 reported by the custom-query path. + $this->assertEquals(51, $result['reconciled']); + } + finally { + \Civi::dispatcher()->removeListener(ReconcilePaymentAttemptBatchEvent::NAME, $subscriber); + } + } + // ------------------------------------------------------------------------- // Helper methods // -------------------------------------------------------------------------