From 783b62a7dfe3c94bd69bcb182732b240e53911be Mon Sep 17 00:00:00 2001 From: eileen Date: Mon, 28 Oct 2019 23:05:18 +1300 Subject: [PATCH] dev/financial#77 ++ Make contribution_id mandatory for PaymentProcessor.pay, pass incoieID Replaces https://github.com/civicrm/civicrm-core/pull/15477 & also resolves https://lab.civicrm.org/dev/financial/issues/77 by requiring contribution_id for PaymentProcessor.pay as a way to ensure that it is only called after the order is created per our preferred flow (people could still get past this but it feels like they would at least know they werre hacking our process & take responsibility for any issues if it breaks or we suddenly start enforcing that it is a valid contribution. This also sets some of the recommended variables. Note that I had to use a few more lines to ensure we were always setting contactID, contributionRecurID to an integer. I do think this stricter approach is better but it wound up more verbose --- api/v3/PaymentProcessor.php | 32 +++++++++++++- tests/phpunit/api/v3/PaymentProcessorTest.php | 42 +++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/api/v3/PaymentProcessor.php b/api/v3/PaymentProcessor.php index 435fd97e97..9b3833d622 100644 --- a/api/v3/PaymentProcessor.php +++ b/api/v3/PaymentProcessor.php @@ -124,11 +124,22 @@ function _civicrm_api3_payment_processor_getlist_defaults(&$request) { * API result array. * * @throws \API_Exception + * @throws \CiviCRM_API3_Exception */ function civicrm_api3_payment_processor_pay($params) { + /* @var CRM_Core_Payment $processor */ $processor = Civi\Payment\System::singleton()->getById($params['payment_processor_id']); $processor->setPaymentProcessor(civicrm_api3('PaymentProcessor', 'getsingle', ['id' => $params['payment_processor_id']])); try { + $processor->setContributionID($params['contribution_id']); + $processor->setInvoiceID($params['invoice_id'] ?? ''); + if (!empty($params['contact_id'])) { + $processor->setContactID((int) $params['contact_id']); + } + if (!empty($params['contribution_recur_id'])) { + $processor->setContributionRecurID((int) $params['contribution_recur_id']); + } + $result = $processor->doPayment($params); } catch (\Civi\Payment\Exception\PaymentProcessorException $e) { @@ -139,7 +150,7 @@ function civicrm_api3_payment_processor_pay($params) { } throw new API_Exception('Payment failed', $code, $errorData, $e); } - return civicrm_api3_create_success(array($result), $params); + return civicrm_api3_create_success([$result], $params); } /** @@ -149,7 +160,7 @@ function civicrm_api3_payment_processor_pay($params) { */ function _civicrm_api3_payment_processor_pay_spec(&$params) { $params['payment_processor_id'] = [ - 'api.required' => 1, + 'api.required' => TRUE, 'title' => ts('Payment processor'), 'type' => CRM_Utils_Type::T_INT, ]; @@ -158,6 +169,23 @@ function _civicrm_api3_payment_processor_pay_spec(&$params) { 'title' => ts('Amount to pay'), 'type' => CRM_Utils_Type::T_MONEY, ]; + $params['contribution_id'] = [ + 'api.required' => TRUE, + 'title' => ts('Contribution ID'), + 'type' => CRM_Utils_Type::T_INT, + ]; + $params['contact_id'] = [ + 'title' => ts('Contact ID'), + 'type' => CRM_Utils_Type::T_INT, + ]; + $params['contribution_recur_id'] = [ + 'title' => ts('Contribution Recur ID'), + 'type' => CRM_Utils_Type::T_INT, + ]; + $params['invoice_id'] = [ + 'title' => ts('Invoice ID'), + 'type' => CRM_Utils_Type::T_STRING, + ]; } /** diff --git a/tests/phpunit/api/v3/PaymentProcessorTest.php b/tests/phpunit/api/v3/PaymentProcessorTest.php index 60ccd4e5ce..6c54af7867 100644 --- a/tests/phpunit/api/v3/PaymentProcessorTest.php +++ b/tests/phpunit/api/v3/PaymentProcessorTest.php @@ -35,6 +35,11 @@ class api_v3_PaymentProcessorTest extends CiviUnitTestCase { protected $_apiversion = 3; protected $_params; + /** + * Set up for class. + * + * @throws \CRM_Core_Exception + */ public function setUp() { parent::setUp(); $this->useTransaction(TRUE); @@ -69,6 +74,8 @@ class api_v3_PaymentProcessorTest extends CiviUnitTestCase { /** * Create payment processor. + * + * @throws \Exception */ public function testPaymentProcessorCreate() { $params = $this->_params; @@ -88,6 +95,8 @@ class api_v3_PaymentProcessorTest extends CiviUnitTestCase { /** * Update payment processor. + * + * @throws \CRM_Core_Exception */ public function testPaymentProcessorUpdate() { $params = $this->_params; @@ -131,6 +140,8 @@ class api_v3_PaymentProcessorTest extends CiviUnitTestCase { /** * Check payment processor delete. + * + * @throws \CRM_Core_Exception */ public function testPaymentProcessorDelete() { $result = $this->callAPISuccess('payment_processor', 'create', $this->_params); @@ -143,6 +154,8 @@ class api_v3_PaymentProcessorTest extends CiviUnitTestCase { /** * Check with valid params array. + * + * @throws \CRM_Core_Exception */ public function testPaymentProcessorsGet() { $params = $this->_params; @@ -158,4 +171,33 @@ class api_v3_PaymentProcessorTest extends CiviUnitTestCase { $this->assertEquals('test@test.com', $results['values'][$results['id']]['user_name']); } + /** + * Test the processor passed to the hook can access the relevant variables. + * + * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception + */ + public function testPaymentProcessorPay() { + $this->hookClass->setHook('civicrm_alterPaymentProcessorParams', [$this, 'hook_civicrm_alterPaymentProcessorParams']); + $processor = $this->dummyProcessorCreate(); + $this->callAPISuccess('PaymentProcessor', 'pay', [ + 'payment_processor_id' => $processor->getID(), + 'contribution_id' => 10, + 'invoice_id' => 2, + 'contribution_recur_id' => 3, + 'amount' => 6, + ]); + } + + /** + * Implements civicrm_alterPaymentProcessorParams hook. + * + * @param \CRM_Core_Payment_Dummy $paymentObject + */ + public function hook_civicrm_alterPaymentProcessorParams($paymentObject) { + $this->assertEquals(10, $paymentObject->getContributionID()); + $this->assertEquals(2, $paymentObject->getInvoiceID()); + $this->assertEquals(3, $paymentObject->getContributionRecurID()); + } + } -- 2.25.1