From ef5fbf3eeaf4bfff3a35ca00dc8e0f8666404682 Mon Sep 17 00:00:00 2001 From: Rich Lott / Artful Robot Date: Tue, 4 Feb 2020 12:35:38 +0000 Subject: [PATCH] Use Payment.create to complete repeat payments. --- CRM/GoCardless/Page/Webhook.php | 67 ++++++++++++++++++++------------ README.md | 4 +- tests/phpunit/GoCardlessTest.php | 12 +++++- 3 files changed, 56 insertions(+), 27 deletions(-) diff --git a/CRM/GoCardless/Page/Webhook.php b/CRM/GoCardless/Page/Webhook.php index 4b5c32c..fd3c2b7 100644 --- a/CRM/GoCardless/Page/Webhook.php +++ b/CRM/GoCardless/Page/Webhook.php @@ -202,6 +202,12 @@ public function doPaymentsConfirmed($event) { // Note: the param called 'trxn_date' which is used for membership date // calculations. If it's not given, today's date gets used. + + // Shortly we will make a call to Payment.create, but this will not correctly update + // several fields in the Contribution record. We'll collect the things that + // need correcting in and submit them using Contribution.create at the end. + $fixContributionParams = []; + $pending_contribution_id = $this->getPendingContributionId($recur); if ($pending_contribution_id) { // There's an existing pending contribution. @@ -232,22 +238,27 @@ public function doPaymentsConfirmed($event) { // is_send_contribution_notification are passed on to the // Contribution.completetranscation API. // - // We want to save the trxn_id and receive_date to the contribution, so - // we do this directly now. It would be more efficient if the core API - // allowed passing in this data to avoid a 2nd call. - $fixContributionParams = [ - 'id' => $contribution['id'], - 'trxn_id' => $payment->id, - 'receive_date' => $payment->charge_date, - ]; - civicrm_api3('Contribution', 'create', $fixContributionParams); - - // We're done here. - return; + // trxn_id is not updated on the Contribution record. + $fixContributionParams['trxn_id'] = $contribution['trxn_id']; + } + else { + // This is another payment after the first. + $contribution = $this->handleRepeatTransaction($contribution, $recur); } - // This is another payment after the first. - $this->handleRepeatTransaction($contribution, $recur); + $fixContributionParams += [ + 'id' => $contribution['id'], + 'receive_date' => $contribution['receive_date'] + ]; + + // Finally, correct any Contribution records entries that are either not + // updated or set incorrectly by Payment.create. It would be more efficient + // if the core APIs allowed passing in this data to avoid a 2nd call + // (or perhaps we're not supposed to do this - but it seems important to me + // that the contribution receive_date is the date the payment was made, and + // not the date the webhook was fired - since this could be delayed, or a + // replay). + civicrm_api3('Contribution', 'create', $fixContributionParams); } /** @@ -295,6 +306,8 @@ public function updateContributionRecurRecord($recur, $payment) { * * @param Array $contribution * @param Array $recur + * + * @return Array updated $contribution */ public function handleRepeatTransaction($contribution, $recur) { @@ -354,22 +367,28 @@ public function handleRepeatTransaction($contribution, $recur) { return; } - // Now complete the transaction. - $params = [ - 'id' => $contribution['id'], - 'payment_processor_id' => $this->payment_processor->getPaymentProcessor()['id'], - 'is_email_receipt' => (empty($contribution['is_email_receipt']) ? 0 : 1), - 'trxn_id' => $contribution['trxn_id'], - 'receive_date' => $contribution['receive_date'], + // Now complete the new contribution by creating a payment for the same amount. + $paymentCreateParams = [ + 'contribution_id' => $contribution['id'], + 'is_send_contribution_notification' => (empty($contribution['is_email_receipt']) ? 0 : 1), + 'trxn_id' => $contribution['trxn_id'], + 'trxn_date' => $contribution['receive_date'], + 'total_amount' => $contribution['total_amount'], ]; - $result = civicrm_api3('Contribution', 'completetransaction', $params); + $result = civicrm_api3('Payment', 'create', $paymentCreateParams); if ($result['is_error'] ?? NULL) { CRM_Core_Error::debug_log_message( - "Webhook $this->now Failed event. completetransaction failed.", - $params, 'GoCardless', PEAR_LOG_ERR); + "Webhook $this->now Failed event. Payment.create API failed\n". + json_encode([ + 'API params' => $paymentCreateParams, + 'Result' => $result, + 'Contribution' => $contribution, + ], JSON_PRETTY_PRINT), + NULL, 'GoCardless', PEAR_LOG_ERR); return; } + return $contribution; } /** * Process webhook for 'payments' resource type, action 'failed'. diff --git a/README.md b/README.md index 2e296bb..aa6e75b 100644 --- a/README.md +++ b/README.md @@ -269,7 +269,9 @@ to build a tool for their own needs from that. ### 1.9.2 -- Move to `Payment.create` API instead of older (and deprecated) `Contribution.completetransaction` API. #70 (Thanks @mattwire). This fixes [issue #63](https://github.com/artfulrobot/uk.artfulrobot.civicrm.gocardless/issues/63) on some sites where the first contribution never gets completed. +- Move to `Payment.create` API instead of older (and deprecated) `Contribution.completetransaction` API. + - This is from PR #70 (Thanks @mattwire) which fixes [issue #63](https://github.com/artfulrobot/uk.artfulrobot.civicrm.gocardless/issues/63) on some sites where the first contribution never gets completed. + - Also this method is now used for repeat payments. - Fix some issues with the system checks (#69 thanks @mattwire) diff --git a/tests/phpunit/GoCardlessTest.php b/tests/phpunit/GoCardlessTest.php index eaeb5c5..78ce272 100644 --- a/tests/phpunit/GoCardlessTest.php +++ b/tests/phpunit/GoCardlessTest.php @@ -907,11 +907,15 @@ public function testWebhookPaymentConfirmationSubsequent() { "links":{"subscription":"SUBSCRIPTION_ID"} }')); - // Now trigger webhook. + // + // Mocks, done, now onto the code we are testing: trigger webhook. + // $controller->parseWebhookRequest(["Webhook-Signature" => $calculated_signature], $body); $controller->processWebhookEvents(TRUE); + // // Now check the changes have been made. + // $result = civicrm_api3('Contribution', 'get', [ 'contribution_recur_id' => $recur['id'], 'is_test' => 1, @@ -1033,11 +1037,15 @@ public function testWebhookPaymentConfirmationDoesNotMarkCancelledAsInProgress() "links":{"subscription":"SUBSCRIPTION_ID"} }')); - // Now trigger webhook. + // + // Mocks, done, now onto the code we are testing: trigger webhook. + // $controller->parseWebhookRequest(["Webhook-Signature" => $calculated_signature], $body); $controller->processWebhookEvents(TRUE); + // // Now check the changes have been made. + // $result = civicrm_api3('Contribution', 'get', [ 'contribution_recur_id' => $recur['id'], 'is_test' => 1,