Fix unreleased regression where processors using getPaymentDescription get a fatal

We added the new property bag in https://github.com/civicrm/civicrm-core/pull/15697 but in the back & forth the
call to the function getDescription was retained but the function was removed. This would leave the
core processor that calls it (Paypal std) with a fatal. The known external processor using it (Omnipay)
also hits a notice because the signature changed ($params became optional). From the Omnipay
pov I think it makes sense just to stop overriding the function - hence I am sneaking a tiny
change into this to make that possible.

Note the doPayment function ensures that propertyBag is set. There is a small chance that an external processor
is fully overriding doPayment AND calling this - that would be the case for Omnipay except I'll remove
the override in conjunction with this - so I've added a few lines to ensure it is set.

Note the test would fail with the changes to Payment_Dummmy & not the changes to Core_Payment so it provides
test cover

diff --git a/CRM/Core/Payment.php b/CRM/Core/Payment.php
index 5ae135f13d37eea3261ba8195636c53b1ccc48f6..1af72a2301f03e52b8dbd79952d03d886dea4fe9 100644 (file)
--- a/CRM/Core/Payment.php
+++ b/CRM/Core/Payment.php
@@ -1268,7 +1268,7 @@ abstract class CRM_Core_Payment {
    *  For the current status see: https://lab.civicrm.org/dev/financial/issues/53
    * If we DO have a contribution ID, then the payment processor can (and should) update parameters on the contribution record as necessary.
    *
-   * @param array $params
+   * @param array|PropertyBag $params
    *
    * @param string $component
    *
@@ -1664,25 +1664,19 @@ INNER JOIN civicrm_contribution con ON ( con.contribution_recur_id = rec.id )
    * @return string
    */
   protected function getPaymentDescription($params = [], $length = 24) {
+    $propertyBag = PropertyBag::cast($params);
     $parts = [
-      'contactID',
-      'contributionID',
-      'description',
-      'billing_first_name',
-      'billing_last_name',
+      $propertyBag->getter('contactID', TRUE),
+      $propertyBag->getter('contributionID', TRUE),
+      $propertyBag->getter('description', TRUE) ?: ($propertyBag->getter('isRecur', TRUE) ? ts('Recurring payment') : NULL),
+      $propertyBag->getter('billing_first_name', TRUE),
+      $propertyBag->getter('billing_last_name', TRUE),
     ];
-    $validParts = [];
-    $params['description'] = $this->getDescription();
-    foreach ($parts as $part) {
-      if ((!empty($params[$part]))) {
-        $validParts[] = $params[$part];
-      }
-    }
-    return substr(implode('-', $validParts), 0, $length);
+    return substr(implode('-', array_filter($parts)), 0, $length);
   }
 
   /**
-   * Checks if backoffice recurring edit is allowed
+   * Checks if back-office recurring edit is allowed
    *
    * @return bool
    */

diff --git a/CRM/Core/Payment/Dummy.php b/CRM/Core/Payment/Dummy.php
index 717c4f3c6fb8a15bf5698cb08564e77fd7b693cb..b2b66125fbdd866217a1ef944cb959473e588952 100644 (file)
--- a/CRM/Core/Payment/Dummy.php
+++ b/CRM/Core/Payment/Dummy.php
@@ -128,6 +128,7 @@ class CRM_Core_Payment_Dummy extends CRM_Core_Payment {
     // Add a fee_amount so we can make sure fees are handled properly in underlying classes.
     $params['fee_amount'] = 1.50;
     $params['net_amount'] = $params['gross_amount'] - $params['fee_amount'];
+    $params['description'] = $this->getPaymentDescription($params);
 
     return $params;
   }

diff --git a/Civi/Payment/PropertyBag.php b/Civi/Payment/PropertyBag.php
index 1419892d0a19b85d81f7b45968cf22c458547634..c27e0af4dc051498c51003b29893b93ace520139 100644 (file)
--- a/Civi/Payment/PropertyBag.php
+++ b/Civi/Payment/PropertyBag.php
@@ -67,6 +67,25 @@ class PropertyBag implements \ArrayAccess {
     'trxnResultCode'              => TRUE,
   ];
 
+  /**
+   * Get the property bag.
+   *
+   * This allows us to swap a 'might be an array might be a property bag'
+   * variable for a known PropertyBag.
+   *
+   * @param \Civi\Payment\PropertyBag|array $params
+   *
+   * @return \Civi\Payment\PropertyBag
+   */
+  public static function cast($params) {
+    if ($params instanceof self) {
+      return $params;
+    }
+    $propertyBag = new self();
+    $propertyBag->mergeLegacyInputParams($params);
+    return $propertyBag;
+  }
+
   /**
    * @var string Just for unit testing.
    */

diff --git a/tests/phpunit/api/v3/PaymentProcessorTest.php b/tests/phpunit/api/v3/PaymentProcessorTest.php
index b707f6772029c62500a10e37ae1764d26f3b04f1..e3c8a713d72fca51a86f5f7f3e4d82b6c455a573 100644 (file)
--- a/tests/phpunit/api/v3/PaymentProcessorTest.php
+++ b/tests/phpunit/api/v3/PaymentProcessorTest.php
@@ -16,9 +16,13 @@
  */
 class api_v3_PaymentProcessorTest extends CiviUnitTestCase {
   protected $_paymentProcessorType;
-  protected $_apiversion = 3;
   protected $_params;
 
+  /**
+   * Set up class.
+   *
+   * @throws \CRM_Core_Exception
+   */
   public function setUp() {
     parent::setUp();
     $this->useTransaction(TRUE);
@@ -42,17 +46,16 @@ class api_v3_PaymentProcessorTest extends CiviUnitTestCase {
   }
 
   /**
-   * Check with no name.
+   * Check create with no name specified.
    */
   public function testPaymentProcessorCreateWithoutName() {
-    $payProcParams = [
-      'is_active' => 1,
-    ];
-    $this->callAPIFailure('payment_processor', 'create', $payProcParams);
+    $this->callAPIFailure('payment_processor', 'create', ['is_active' => 1]);
   }
 
   /**
    * Create payment processor.
+   *
+   * @throws \CRM_Core_Exception
    */
   public function testPaymentProcessorCreate() {
     $params = $this->_params;
@@ -72,6 +75,8 @@ class api_v3_PaymentProcessorTest extends CiviUnitTestCase {
 
   /**
    * Update payment processor.
+   *
+   * @throws \CRM_Core_Exception
    */
   public function testPaymentProcessorUpdate() {
     $params = $this->_params;
@@ -115,6 +120,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);
@@ -127,6 +134,8 @@ class api_v3_PaymentProcessorTest extends CiviUnitTestCase {
 
   /**
    * Check with valid params array.
+   *
+   * @throws \CRM_Core_Exception
    */
   public function testPaymentProcessorsGet() {
     $params = $this->_params;