From 61dace0d5883be09c9b5dd8c221c2e356e651202 Mon Sep 17 00:00:00 2001 From: Erawat Chamanont Date: Fri, 25 Sep 2026 09:13:40 +0100 Subject: [PATCH] MAE-1384: Record the payment processor when completing a contribution Payments completed through this extension were recorded without the payment processor that took them, because ContributionCompletionService did not pass payment_processor_id to Contribution.completetransaction. Core then left the column empty on the financial transaction it created, and Finance Extras, which reads the processor back off that transaction to decide whether a refund is possible, offered no refund action at all. Staff could not refund a Stripe Checkout payment from CiviCRM. Callers can now pass the processor. When they do not, it is resolved from the contribution's payment attempt, and failing that from its recurring contribution, so no caller has to change and every processor using this service is fixed. Upgrade step 1001 repairs the payments already recorded without a processor, taking it from the payment attempt alongside each one. It only touches payment transactions that have no processor, so it is safe to run again. --- CRM/Paymentprocessingcore/Upgrader.php | 19 ++ .../Upgrader/PaymentProcessorBackfill.php | 77 ++++++ .../Service/ContributionCompletionService.php | 79 +++++- stubs/CiviApi4.stub.php | 10 + .../Upgrader/PaymentProcessorBackfillTest.php | 247 ++++++++++++++++++ .../Paymentprocessingcore/UpgraderTest.php | 152 +++++++++++ .../ContributionCompletionServiceTest.php | 232 +++++++++++++++- 7 files changed, 809 insertions(+), 7 deletions(-) create mode 100644 CRM/Paymentprocessingcore/Upgrader/PaymentProcessorBackfill.php create mode 100644 tests/phpunit/CRM/Paymentprocessingcore/Upgrader/PaymentProcessorBackfillTest.php create mode 100644 tests/phpunit/CRM/Paymentprocessingcore/UpgraderTest.php diff --git a/CRM/Paymentprocessingcore/Upgrader.php b/CRM/Paymentprocessingcore/Upgrader.php index ffc19f3..010bbe7 100644 --- a/CRM/Paymentprocessingcore/Upgrader.php +++ b/CRM/Paymentprocessingcore/Upgrader.php @@ -8,4 +8,23 @@ */ class CRM_Paymentprocessingcore_Upgrader extends CRM_Extension_Upgrader_Base { + /** + * Backfill the payment processor on payments this extension completed without one. + * + * Until now ContributionCompletionService did not pass the payment processor to + * Contribution.completetransaction, so core left payment_processor_id empty on the financial + * transaction it created, and Finance Extras would not offer a refund for those payments. + * + * @return bool + */ + public function upgrade_1001(): bool { + \Civi::log()->info('Payment Processing Core upgrade 1001: backfilling the payment processor on payment transactions'); + + $backfilled = (new CRM_Paymentprocessingcore_Upgrader_PaymentProcessorBackfill())->run(); + + \Civi::log()->info("Payment Processing Core upgrade 1001: payment transactions given a payment processor: {$backfilled}"); + + return TRUE; + } + } diff --git a/CRM/Paymentprocessingcore/Upgrader/PaymentProcessorBackfill.php b/CRM/Paymentprocessingcore/Upgrader/PaymentProcessorBackfill.php new file mode 100644 index 0000000..d36f3ce --- /dev/null +++ b/CRM/Paymentprocessingcore/Upgrader/PaymentProcessorBackfill.php @@ -0,0 +1,77 @@ +countTransactionsMissingProcessor(); + + if ($missingBefore === 0) { + return 0; + } + + CRM_Core_DAO::executeQuery( + 'UPDATE ' . self::TABLES + . ' SET ft.payment_processor_id = attempt.payment_processor_id ' + . self::CONDITION + ); + + return $missingBefore - $this->countTransactionsMissingProcessor(); + } + + /** + * How many payment transactions could still take a processor from a payment attempt. + * + * @return int + */ + public function countTransactionsMissingProcessor(): int { + return (int) CRM_Core_DAO::singleValueQuery( + 'SELECT COUNT(*) FROM ' . self::TABLES . self::CONDITION + ); + } + +} diff --git a/Civi/Paymentprocessingcore/Service/ContributionCompletionService.php b/Civi/Paymentprocessingcore/Service/ContributionCompletionService.php index c2213c6..93a3f16 100644 --- a/Civi/Paymentprocessingcore/Service/ContributionCompletionService.php +++ b/Civi/Paymentprocessingcore/Service/ContributionCompletionService.php @@ -4,6 +4,7 @@ use Civi\Api4\Contribution; use Civi\Api4\ContributionPage; +use Civi\Api4\PaymentAttempt; use Civi\Paymentprocessingcore\Exception\ContributionCompletionException; /** @@ -26,6 +27,9 @@ class ContributionCompletionService { * @param string $transactionId Payment processor transaction ID (e.g., Stripe charge ID ch_..., GoCardless payment ID pm_...) * @param float|null $feeAmount Optional fee amount charged by payment processor * @param bool|null $sendReceipt Whether to send email receipt. If NULL, will check contribution page settings. Default: NULL + * @param int|null $paymentProcessorId Payment processor that took the payment. If NULL, it is resolved from the + * contribution's payment attempts or its recurring contribution. Recording it on the financial transaction is what + * allows downstream features (for example Finance Extras refunds) to know which processor to refund through. * * @return array Completion result with keys: 'success' => TRUE, 'contribution_id' => int, 'already_completed' => bool * @@ -33,7 +37,7 @@ class ContributionCompletionService { * * @throws \Civi\Paymentprocessingcore\Exception\ContributionCompletionException If completion fails */ - public function complete(int $contributionId, string $transactionId, ?float $feeAmount = NULL, ?bool $sendReceipt = NULL): array { + public function complete(int $contributionId, string $transactionId, ?float $feeAmount = NULL, ?bool $sendReceipt = NULL, ?int $paymentProcessorId = NULL): array { $contribution = $this->getContribution($contributionId); // Check if already completed (idempotency) @@ -58,8 +62,12 @@ public function complete(int $contributionId, string $transactionId, ?float $fee $sendReceipt = $this->shouldSendReceipt($contribution); } + if ($paymentProcessorId === NULL) { + $paymentProcessorId = $this->resolvePaymentProcessorId($contribution); + } + // Complete the transaction - $this->completeTransaction($contribution, $transactionId, $feeAmount, $sendReceipt); + $this->completeTransaction($contribution, $transactionId, $feeAmount, $sendReceipt, $paymentProcessorId); return [ 'success' => TRUE, @@ -80,7 +88,7 @@ public function complete(int $contributionId, string $transactionId, ?float $fee private function getContribution(int $contributionId): array { try { $contribution = Contribution::get(FALSE) - ->addSelect('id', 'contribution_status_id:name', 'total_amount', 'currency', 'contribution_page_id', 'trxn_id') + ->addSelect('id', 'contribution_status_id:name', 'total_amount', 'currency', 'contribution_page_id', 'trxn_id', 'contribution_recur_id.payment_processor_id') ->addWhere('id', '=', $contributionId) ->execute() ->first(); @@ -106,6 +114,61 @@ private function getContribution(int $contributionId): array { } } + /** + * Work out which payment processor took the payment. + * + * Callers that know the processor should pass it in. When they do not, the payment attempt recorded for the + * contribution is the most reliable source, since every processor that uses this service records one. Recurring + * contributions carry the processor themselves, so they are used as a second source. + * + * A contribution can only have one payment attempt - contribution_id is unique on the table. + * + * Resolution never blocks completion: if it fails, the contribution still completes, only without the processor + * recorded on the financial transaction. + * + * @param array $contribution Contribution data + * + * @phpstan-param array $contribution + * + * @return int|null Payment processor ID, or NULL when it cannot be determined + */ + private function resolvePaymentProcessorId(array $contribution): ?int { + try { + $attempt = PaymentAttempt::get(FALSE) + ->addSelect('payment_processor_id') + ->addWhere('contribution_id', '=', $contribution['id']) + ->addWhere('payment_processor_id', 'IS NOT NULL') + ->addOrderBy('id', 'DESC') + ->setLimit(1) + ->execute() + ->first(); + + $attemptProcessorId = is_array($attempt) ? ($attempt['payment_processor_id'] ?? NULL) : NULL; + if (is_numeric($attemptProcessorId)) { + return (int) $attemptProcessorId; + } + } + catch (\Exception $e) { + \Civi::log()->warning('ContributionCompletionService: Failed to resolve payment processor from payment attempts', [ + 'contribution_id' => $contribution['id'], + 'error' => $e->getMessage(), + ]); + } + + $recurProcessorId = $contribution['contribution_recur_id.payment_processor_id'] ?? NULL; + if (is_numeric($recurProcessorId)) { + return (int) $recurProcessorId; + } + + // Back office payments have neither a payment attempt nor a recurring contribution, so this is + // an ordinary outcome rather than something to flag. + \Civi::log()->info('ContributionCompletionService: No payment processor to record against the contribution', [ + 'contribution_id' => $contribution['id'], + ]); + + return NULL; + } + /** * Check if contribution is already completed (idempotency). * @@ -183,12 +246,13 @@ private function shouldSendReceipt(array $contribution): bool { * @param string $transactionId Payment processor transaction ID * @param float|null $feeAmount Optional fee amount * @param bool $sendReceipt Whether to send email receipt + * @param int|null $paymentProcessorId Payment processor that took the payment, recorded on the financial transaction * * @return void * * @throws \Civi\Paymentprocessingcore\Exception\ContributionCompletionException If completion fails */ - private function completeTransaction(array $contribution, string $transactionId, ?float $feeAmount, bool $sendReceipt): void { + private function completeTransaction(array $contribution, string $transactionId, ?float $feeAmount, bool $sendReceipt, ?int $paymentProcessorId = NULL): void { try { $params = [ 'id' => $contribution['id'], @@ -201,6 +265,12 @@ private function completeTransaction(array $contribution, string $transactionId, $params['fee_amount'] = $feeAmount; } + // Core records this against the financial transaction it creates, which is how refunds know + // which processor to go back through. Without it the transaction is left with no processor. + if ($paymentProcessorId !== NULL) { + $params['payment_processor_id'] = $paymentProcessorId; + } + civicrm_api3('Contribution', 'completetransaction', $params); \Civi::log()->info('ContributionCompletionService: Contribution completed successfully', [ @@ -210,6 +280,7 @@ private function completeTransaction(array $contribution, string $transactionId, 'amount' => $contribution['total_amount'], 'currency' => $contribution['currency'], 'receipt_sent' => $sendReceipt, + 'payment_processor_id' => $paymentProcessorId, ]); } catch (\CiviCRM_API3_Exception $e) { diff --git a/stubs/CiviApi4.stub.php b/stubs/CiviApi4.stub.php index 2ddc83b..69f699e 100644 --- a/stubs/CiviApi4.stub.php +++ b/stubs/CiviApi4.stub.php @@ -107,6 +107,16 @@ class PaymentToken { } + /** + * @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 EntityFinancialTrxn { + + } + /** * @method static DAOGetAction get(bool $checkPermissions = TRUE) * @method static DAOCreateAction create(bool $checkPermissions = TRUE) diff --git a/tests/phpunit/CRM/Paymentprocessingcore/Upgrader/PaymentProcessorBackfillTest.php b/tests/phpunit/CRM/Paymentprocessingcore/Upgrader/PaymentProcessorBackfillTest.php new file mode 100644 index 0000000..b9aea76 --- /dev/null +++ b/tests/phpunit/CRM/Paymentprocessingcore/Upgrader/PaymentProcessorBackfillTest.php @@ -0,0 +1,247 @@ +backfill = new CRM_Paymentprocessingcore_Upgrader_PaymentProcessorBackfill(); + + /** @var \Civi\Paymentprocessingcore\Service\ContributionCompletionService $completionService */ + $completionService = \Civi::service('paymentprocessingcore.contribution_completion'); + $this->completionService = $completionService; + + $this->contactId = $this->idOf(Contact::create(FALSE) + ->addValue('contact_type', 'Individual') + ->addValue('first_name', 'Test') + ->addValue('last_name', 'Donor') + ->execute() + ->first()); + } + + /** + * Tests a payment left without a processor takes it from the contribution's payment attempt. + */ + public function testBackfillsTheProcessorFromThePaymentAttempt(): void { + $processorId = $this->createPaymentProcessor(); + $contributionId = $this->createPaymentMissingItsProcessor($processorId); + + $outstanding = $this->backfill->countTransactionsMissingProcessor(); + $this->assertGreaterThan(0, $outstanding); + $this->assertEquals($outstanding, $this->backfill->run()); + $this->assertEquals(0, $this->backfill->countTransactionsMissingProcessor()); + $this->assertEquals([$processorId], $this->getRecordedPaymentProcessorIds($contributionId)); + } + + /** + * Tests nothing is reported as repaired when every payment already has its processor. + */ + public function testReportsNothingRepairedWhenThereIsNothingToRepair(): void { + $processorId = $this->createPaymentProcessor(); + $contributionId = $this->createPendingContribution(); + $this->createPaymentAttempt($contributionId, $processorId); + $this->completionService->complete($contributionId, 'ch_test_already_recorded', NULL, FALSE, $processorId); + + $this->assertEquals(0, $this->backfill->run()); + $this->assertEquals([$processorId], $this->getRecordedPaymentProcessorIds($contributionId)); + } + + /** + * Tests running the backfill twice does not change anything the second time. + */ + public function testIsSafeToRunAgain(): void { + $processorId = $this->createPaymentProcessor(); + $contributionId = $this->createPaymentMissingItsProcessor($processorId); + + $this->assertGreaterThan(0, $this->backfill->run()); + $this->assertEquals(0, $this->backfill->run()); + $this->assertEquals([$processorId], $this->getRecordedPaymentProcessorIds($contributionId)); + } + + /** + * Tests a payment that already names a processor keeps the one it has. + */ + public function testLeavesAPaymentThatAlreadyNamesAProcessorAlone(): void { + $recordedProcessorId = $this->createPaymentProcessor(); + $attemptProcessorId = $this->createPaymentProcessor(); + $contributionId = $this->createPendingContribution(); + $this->completionService->complete($contributionId, 'ch_test_keeps_processor', NULL, FALSE, $recordedProcessorId); + $this->createPaymentAttempt($contributionId, $attemptProcessorId); + + $this->backfill->run(); + + $this->assertEquals([$recordedProcessorId], $this->getRecordedPaymentProcessorIds($contributionId)); + } + + /** + * Tests a payment with no payment attempt to learn from is left as it is. + */ + public function testLeavesAPaymentWithNoPaymentAttemptAlone(): void { + $contributionId = $this->createPendingContribution(); + $this->completionService->complete($contributionId, 'ch_test_no_attempt', NULL, FALSE); + + $this->backfill->run(); + + $this->assertEquals([NULL], $this->getRecordedPaymentProcessorIds($contributionId)); + } + + /** + * Tests attempts recorded without a processor are not used. + */ + public function testIgnoresPaymentAttemptsThatHaveNoProcessor(): void { + $contributionId = $this->createPendingContribution(); + $this->completionService->complete($contributionId, 'ch_test_attempt_no_processor', NULL, FALSE); + $this->createPaymentAttempt($contributionId, NULL); + + $this->backfill->run(); + + $this->assertEquals([NULL], $this->getRecordedPaymentProcessorIds($contributionId)); + } + + /** + * Tests each contribution is repaired with its own processor. + */ + public function testRepairsEachContributionWithItsOwnProcessor(): void { + $firstProcessorId = $this->createPaymentProcessor(); + $secondProcessorId = $this->createPaymentProcessor(); + $firstContributionId = $this->createPaymentMissingItsProcessor($firstProcessorId); + $secondContributionId = $this->createPaymentMissingItsProcessor($secondProcessorId); + + $this->backfill->run(); + + $this->assertEquals([$firstProcessorId], $this->getRecordedPaymentProcessorIds($firstContributionId)); + $this->assertEquals([$secondProcessorId], $this->getRecordedPaymentProcessorIds($secondContributionId)); + } + + /** + * Helper: Complete a contribution the way the extension used to, without a processor. + * + * The payment attempt is created after completion so that the attempt cannot be used to resolve + * the processor while the payment is being recorded, which reproduces the data left behind by + * the versions this backfill exists to repair. + * + * @return int The contribution ID + */ + private function createPaymentMissingItsProcessor(int $paymentProcessorId): int { + $contributionId = $this->createPendingContribution(); + $this->completionService->complete($contributionId, 'ch_test_' . uniqid(), NULL, FALSE); + $this->createPaymentAttempt($contributionId, $paymentProcessorId, 'completed'); + + // Guard the fixture: the payment must really be missing its processor. + $this->assertEquals([NULL], $this->getRecordedPaymentProcessorIds($contributionId)); + + return $contributionId; + } + + /** + * Helper: The id of a record the API just returned, as an int. + * + * @phpstan-param array|null $record + */ + private function idOf(?array $record): int { + $id = $record['id'] ?? NULL; + + return is_numeric($id) ? (int) $id : 0; + } + + /** + * Helper: Create a Pending contribution. + */ + private function createPendingContribution(float $amount = 100.00): int { + return $this->idOf(Contribution::create(FALSE) + ->addValue('contact_id', $this->contactId) + ->addValue('financial_type_id:name', 'Donation') + ->addValue('total_amount', $amount) + ->addValue('currency', 'GBP') + ->addValue('contribution_status_id:name', 'Pending') + ->execute() + ->first()); + } + + /** + * Helper: Create an active payment processor. + */ + private function createPaymentProcessor(): int { + return $this->idOf(PaymentProcessor::create(FALSE) + ->addValue('name', 'Test Processor ' . uniqid()) + ->addValue('payment_processor_type_id:name', 'Dummy') + ->addValue('class_name', 'Payment_Dummy') + ->addValue('is_active', TRUE) + ->addValue('is_test', FALSE) + ->addValue('domain_id', 1) + ->execute() + ->first()); + } + + /** + * Helper: Create a payment attempt against a contribution. + */ + private function createPaymentAttempt(int $contributionId, ?int $paymentProcessorId, string $status = 'pending'): int { + $attempt = PaymentAttempt::create(FALSE) + ->addValue('contribution_id', $contributionId) + ->addValue('contact_id', $this->contactId) + ->addValue('processor_type', 'dummy') + ->addValue('status', $status); + + if ($paymentProcessorId !== NULL) { + $attempt->addValue('payment_processor_id', $paymentProcessorId); + } + + return $this->idOf($attempt->execute()->first()); + } + + /** + * Helper: The distinct payment processors recorded across a contribution's payment transactions. + * + * Completing a contribution writes several payment transactions, and they all carry the same + * processor, so the distinct values are what the assertions are about. + * + * @return array + */ + private function getRecordedPaymentProcessorIds(int $contributionId): array { + $entityTrxns = EntityFinancialTrxn::get(FALSE) + ->addSelect('financial_trxn_id.payment_processor_id') + ->addWhere('entity_table', '=', 'civicrm_contribution') + ->addWhere('entity_id', '=', $contributionId) + ->addWhere('financial_trxn_id.is_payment', '=', TRUE) + ->execute(); + + $processorIds = []; + foreach ($entityTrxns as $entityTrxn) { + $processorId = is_array($entityTrxn) ? ($entityTrxn['financial_trxn_id.payment_processor_id'] ?? NULL) : NULL; + $processorIds[] = is_numeric($processorId) ? (int) $processorId : NULL; + } + + return array_values(array_unique($processorIds, SORT_REGULAR)); + } + +} diff --git a/tests/phpunit/CRM/Paymentprocessingcore/UpgraderTest.php b/tests/phpunit/CRM/Paymentprocessingcore/UpgraderTest.php new file mode 100644 index 0000000..13f3cdd --- /dev/null +++ b/tests/phpunit/CRM/Paymentprocessingcore/UpgraderTest.php @@ -0,0 +1,152 @@ +upgrader = new CRM_Paymentprocessingcore_Upgrader(); + + /** @var \Civi\Paymentprocessingcore\Service\ContributionCompletionService $completionService */ + $completionService = \Civi::service('paymentprocessingcore.contribution_completion'); + $this->completionService = $completionService; + + $this->contactId = $this->idOf(Contact::create(FALSE) + ->addValue('contact_type', 'Individual') + ->addValue('first_name', 'Test') + ->addValue('last_name', 'Donor') + ->execute() + ->first()); + } + + /** + * Tests step 1001 backfills the payment processor on a payment recorded without one. + */ + public function testUpgrade1001BackfillsThePaymentProcessor(): void { + $processorId = $this->createPaymentProcessor(); + + $contributionId = $this->createPendingContribution(); + $this->completionService->complete($contributionId, 'ch_test_' . uniqid(), NULL, FALSE); + $this->createPaymentAttempt($contributionId, $processorId); + $this->assertEquals([NULL], $this->getRecordedPaymentProcessorIds($contributionId)); + + $this->assertTrue($this->upgrader->upgrade_1001()); + $this->assertEquals([$processorId], $this->getRecordedPaymentProcessorIds($contributionId)); + } + + /** + * Tests step 1001 succeeds on a site that has nothing to repair. + */ + public function testUpgrade1001SucceedsWithNothingToRepair(): void { + $this->assertTrue($this->upgrader->upgrade_1001()); + } + + /** + * Helper: The id of a record the API just returned, as an int. + * + * @phpstan-param array|null $record + */ + private function idOf(?array $record): int { + $id = $record['id'] ?? NULL; + + return is_numeric($id) ? (int) $id : 0; + } + + /** + * Helper: Create a Pending contribution. + */ + private function createPendingContribution(float $amount = 100.00): int { + return $this->idOf(Contribution::create(FALSE) + ->addValue('contact_id', $this->contactId) + ->addValue('financial_type_id:name', 'Donation') + ->addValue('total_amount', $amount) + ->addValue('currency', 'GBP') + ->addValue('contribution_status_id:name', 'Pending') + ->execute() + ->first()); + } + + /** + * Helper: Create an active payment processor. + */ + private function createPaymentProcessor(): int { + return $this->idOf(PaymentProcessor::create(FALSE) + ->addValue('name', 'Test Processor ' . uniqid()) + ->addValue('payment_processor_type_id:name', 'Dummy') + ->addValue('class_name', 'Payment_Dummy') + ->addValue('is_active', TRUE) + ->addValue('is_test', FALSE) + ->addValue('domain_id', 1) + ->execute() + ->first()); + } + + /** + * Helper: Create a payment attempt against a contribution. + */ + private function createPaymentAttempt(int $contributionId, int $paymentProcessorId): int { + return $this->idOf(PaymentAttempt::create(FALSE) + ->addValue('contribution_id', $contributionId) + ->addValue('contact_id', $this->contactId) + ->addValue('processor_type', 'dummy') + ->addValue('status', 'completed') + ->addValue('payment_processor_id', $paymentProcessorId) + ->execute() + ->first()); + } + + /** + * Helper: The distinct payment processors recorded across a contribution's payment transactions. + * + * @return array + */ + private function getRecordedPaymentProcessorIds(int $contributionId): array { + $entityTrxns = EntityFinancialTrxn::get(FALSE) + ->addSelect('financial_trxn_id.payment_processor_id') + ->addWhere('entity_table', '=', 'civicrm_contribution') + ->addWhere('entity_id', '=', $contributionId) + ->addWhere('financial_trxn_id.is_payment', '=', TRUE) + ->execute(); + + $processorIds = []; + foreach ($entityTrxns as $entityTrxn) { + $processorId = is_array($entityTrxn) ? ($entityTrxn['financial_trxn_id.payment_processor_id'] ?? NULL) : NULL; + $processorIds[] = is_numeric($processorId) ? (int) $processorId : NULL; + } + + return array_values(array_unique($processorIds, SORT_REGULAR)); + } + +} diff --git a/tests/phpunit/Civi/Paymentprocessingcore/Service/ContributionCompletionServiceTest.php b/tests/phpunit/Civi/Paymentprocessingcore/Service/ContributionCompletionServiceTest.php index af06446..ab8982c 100644 --- a/tests/phpunit/Civi/Paymentprocessingcore/Service/ContributionCompletionServiceTest.php +++ b/tests/phpunit/Civi/Paymentprocessingcore/Service/ContributionCompletionServiceTest.php @@ -5,6 +5,10 @@ use Civi\Api4\Contact; use Civi\Api4\Contribution; use Civi\Api4\ContributionPage; +use Civi\Api4\ContributionRecur; +use Civi\Api4\EntityFinancialTrxn; +use Civi\Api4\PaymentAttempt; +use Civi\Api4\PaymentProcessor; use Civi\Paymentprocessingcore\Exception\ContributionCompletionException; /** @@ -147,6 +151,140 @@ public function testRecordsFeeAmount(): void { $this->assertEquals(96.80, $contribution['net_amount']); } + /** + * Tests the payment processor passed by the caller is recorded on the payment transaction. + * + * This is what lets features that refund a payment - Finance Extras in particular - work out + * which processor to send the refund through. + */ + public function testRecordsGivenPaymentProcessorOnPaymentTransaction(): void { + $processorId = $this->createPaymentProcessor(); + $contributionId = $this->createPendingContribution(); + + $this->service->complete($contributionId, 'ch_test_given_processor', NULL, FALSE, $processorId); + + $this->assertEquals([$processorId], $this->getRecordedPaymentProcessorIds($contributionId)); + } + + /** + * Tests the processor is taken from the contribution's payment attempt when the caller omits it. + */ + public function testResolvesPaymentProcessorFromPaymentAttempt(): void { + $processorId = $this->createPaymentProcessor(); + $contributionId = $this->createPendingContribution(); + $this->createPaymentAttempt($contributionId, $processorId); + + $this->service->complete($contributionId, 'ch_test_attempt_processor', NULL, FALSE); + + $this->assertEquals([$processorId], $this->getRecordedPaymentProcessorIds($contributionId)); + } + + /** + * Tests the caller's processor is used even when a payment attempt names a different one. + */ + public function testGivenPaymentProcessorTakesPrecedenceOverPaymentAttempt(): void { + $attemptProcessorId = $this->createPaymentProcessor(); + $givenProcessorId = $this->createPaymentProcessor(); + $contributionId = $this->createPendingContribution(); + $this->createPaymentAttempt($contributionId, $attemptProcessorId); + + $this->service->complete($contributionId, 'ch_test_caller_wins', NULL, FALSE, $givenProcessorId); + + $this->assertEquals([$givenProcessorId], $this->getRecordedPaymentProcessorIds($contributionId)); + } + + /** + * Tests attempts recorded without a processor do not stop the recurring contribution being used. + */ + public function testFallsBackToRecurringContributionWhenPaymentAttemptHasNoProcessor(): void { + $processorId = $this->createPaymentProcessor(); + $recurId = $this->createRecurringContribution($processorId); + $contributionId = $this->createPendingContribution(100.00, NULL, $recurId); + $this->createPaymentAttempt($contributionId, NULL); + + $this->service->complete($contributionId, 'ch_test_recur_processor', NULL, FALSE); + + $this->assertEquals([$processorId], $this->getRecordedPaymentProcessorIds($contributionId)); + } + + /** + * Tests the processor is taken from the recurring contribution when there is no payment attempt. + */ + public function testResolvesPaymentProcessorFromRecurringContribution(): void { + $processorId = $this->createPaymentProcessor(); + $recurId = $this->createRecurringContribution($processorId); + $contributionId = $this->createPendingContribution(100.00, NULL, $recurId); + + $this->service->complete($contributionId, 'ch_test_recur_only', NULL, FALSE); + + $this->assertEquals([$processorId], $this->getRecordedPaymentProcessorIds($contributionId)); + } + + /** + * Tests another contribution's payment attempt is not used. + */ + public function testIgnoresPaymentAttemptsBelongingToAnotherContribution(): void { + $processorId = $this->createPaymentProcessor(); + $otherContributionId = $this->createPendingContribution(); + $this->createPaymentAttempt($otherContributionId, $processorId); + $contributionId = $this->createPendingContribution(); + + $this->service->complete($contributionId, 'ch_test_other_contribution', NULL, FALSE); + + $this->assertEquals([NULL], $this->getRecordedPaymentProcessorIds($contributionId)); + } + + /** + * Tests a contribution still completes when the processor cannot be worked out. + * + * Back office payments have neither a payment attempt nor a recurring contribution, and they + * must not start failing because of this. + */ + public function testCompletesWithoutAProcessorWhenNoneCanBeResolved(): void { + $contributionId = $this->createPendingContribution(); + + $result = $this->service->complete($contributionId, 'ch_test_no_processor', NULL, FALSE); + + $this->assertTrue($result['success']); + $this->assertEquals([NULL], $this->getRecordedPaymentProcessorIds($contributionId)); + + $contribution = Contribution::get(FALSE) + ->addSelect('contribution_status_id:name') + ->addWhere('id', '=', $contributionId) + ->execute() + ->first() ?? []; + + $this->assertEquals('Completed', $contribution['contribution_status_id:name']); + } + + /** + * Tests the contribution takes its payment instrument from the processor. + * + * Core does this whenever a processor is passed to Contribution.completetransaction, so it is a + * consequence of the fix rather than something this extension asks for. Pinned here so that any + * future change in that behaviour is noticed. + */ + public function testContributionTakesItsPaymentInstrumentFromTheProcessor(): void { + $processorId = $this->createPaymentProcessor(); + $contributionId = $this->createPendingContribution(); + + $this->service->complete($contributionId, 'ch_test_instrument', NULL, FALSE, $processorId); + + $processor = PaymentProcessor::get(FALSE) + ->addSelect('payment_instrument_id') + ->addWhere('id', '=', $processorId) + ->execute() + ->first() ?? []; + + $contribution = Contribution::get(FALSE) + ->addSelect('payment_instrument_id') + ->addWhere('id', '=', $contributionId) + ->execute() + ->first() ?? []; + + $this->assertEquals($processor['payment_instrument_id'], $contribution['payment_instrument_id']); + } + /** * Tests service is accessible via container. */ @@ -156,10 +294,21 @@ public function testServiceAccessibleViaContainer(): void { $this->assertInstanceOf(ContributionCompletionService::class, $service); } + /** + * Helper: The id of a record the API just returned, as an int. + * + * @phpstan-param array|null $record + */ + private function idOf(?array $record): int { + $id = $record['id'] ?? NULL; + + return is_numeric($id) ? (int) $id : 0; + } + /** * Helper: Create Pending contribution. */ - private function createPendingContribution(float $amount = 100.00, ?int $contributionPageId = NULL): int { + private function createPendingContribution(float $amount = 100.00, ?int $contributionPageId = NULL, ?int $contributionRecurId = NULL): int { $params = [ 'contact_id' => $this->contactId, 'financial_type_id:name' => 'Donation', @@ -172,10 +321,87 @@ private function createPendingContribution(float $amount = 100.00, ?int $contrib $params['contribution_page_id'] = $contributionPageId; } - return Contribution::create(FALSE) + if ($contributionRecurId !== NULL) { + $params['contribution_recur_id'] = $contributionRecurId; + } + + return $this->idOf(Contribution::create(FALSE) ->setValues($params) ->execute() - ->first()['id']; + ->first()); + } + + /** + * Helper: Create an active payment processor. + */ + private function createPaymentProcessor(): int { + return $this->idOf(PaymentProcessor::create(FALSE) + ->addValue('name', 'Test Processor ' . uniqid()) + ->addValue('payment_processor_type_id:name', 'Dummy') + ->addValue('class_name', 'Payment_Dummy') + ->addValue('is_active', TRUE) + ->addValue('is_test', FALSE) + ->addValue('domain_id', 1) + ->execute() + ->first()); + } + + /** + * Helper: Create a payment attempt against a contribution. + */ + private function createPaymentAttempt(int $contributionId, ?int $paymentProcessorId, string $status = 'pending'): int { + $attempt = PaymentAttempt::create(FALSE) + ->addValue('contribution_id', $contributionId) + ->addValue('contact_id', $this->contactId) + ->addValue('processor_type', 'dummy') + ->addValue('status', $status); + + if ($paymentProcessorId !== NULL) { + $attempt->addValue('payment_processor_id', $paymentProcessorId); + } + + return $this->idOf($attempt->execute()->first()); + } + + /** + * Helper: Create a recurring contribution against a payment processor. + */ + private function createRecurringContribution(int $paymentProcessorId): int { + return $this->idOf(ContributionRecur::create(FALSE) + ->addValue('contact_id', $this->contactId) + ->addValue('amount', 100.00) + ->addValue('currency', 'GBP') + ->addValue('frequency_unit:name', 'month') + ->addValue('frequency_interval', 1) + ->addValue('payment_processor_id', $paymentProcessorId) + ->addValue('contribution_status_id:name', 'Pending') + ->execute() + ->first()); + } + + /** + * Helper: The distinct payment processors recorded across a contribution's payment transactions. + * + * Completing a contribution writes several payment transactions, and they all carry the same + * processor, so the distinct values are what the assertions are about. + * + * @return array + */ + private function getRecordedPaymentProcessorIds(int $contributionId): array { + $entityTrxns = EntityFinancialTrxn::get(FALSE) + ->addSelect('financial_trxn_id.payment_processor_id') + ->addWhere('entity_table', '=', 'civicrm_contribution') + ->addWhere('entity_id', '=', $contributionId) + ->addWhere('financial_trxn_id.is_payment', '=', TRUE) + ->execute(); + + $processorIds = []; + foreach ($entityTrxns as $entityTrxn) { + $processorId = is_array($entityTrxn) ? ($entityTrxn['financial_trxn_id.payment_processor_id'] ?? NULL) : NULL; + $processorIds[] = is_numeric($processorId) ? (int) $processorId : NULL; + } + + return array_values(array_unique($processorIds, SORT_REGULAR)); } }