dev/financial#77 ++ Make contribution_id mandatory for PaymentProcessor.pay, pass...

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

diff --git a/api/v3/PaymentProcessor.php b/api/v3/PaymentProcessor.php
index 435fd97e977ea7779ec1167afe0c86e509f1fb6a..9b3833d622e5edb471e6351e95528e29d1443282 100644 (file)
--- 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 60ccd4e5ceacb8cf1607e4d1f7b92d6242637e00..6c54af7867b30e9bf7e1cdbf7528e20b7d283701 100644 (file)
--- 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());
+  }
+
 }