CIVIPLMMSR-655: Include custom-query handler counts in reconcile summary

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;
+  }
+
 }

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;
   }
 

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);
+  }
+
 }

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
   // -------------------------------------------------------------------------