Merge pull request #19400 from eileenmcnaughton/pledge_ref

Fix pledge to not use pass-by-reference

CRM/Pledge/BAO/Pledge.php

@@ -58,16 +58,12 @@ class CRM_Pledge_BAO_Pledge extends CRM_Pledge_DAO_Pledge {
    * @param array $params
    *   Reference array contains the values submitted by the form.
    *
-   *
-   * @return object
+   * @return CRM_Pledge_DAO_Pledge
    */
-  public static function add(&$params) {
-    if (!empty($params['id'])) {
-      CRM_Utils_Hook::pre('edit', 'Pledge', $params['id'], $params);
-    }
-    else {
-      CRM_Utils_Hook::pre('create', 'Pledge', NULL, $params);
-    }
+  public static function add(array $params): CRM_Pledge_DAO_Pledge {
+
+    $hook = empty($params['id']) ? 'create' : 'edit';
+    CRM_Utils_Hook::pre($hook, 'Pledge', $params['id'] ?? NULL, $params);
 
     $pledge = new CRM_Pledge_DAO_Pledge();
 
@@ -85,12 +81,7 @@ class CRM_Pledge_BAO_Pledge extends CRM_Pledge_DAO_Pledge {
 
     $result = $pledge->save();
 
-    if (!empty($params['id'])) {
-      CRM_Utils_Hook::post('edit', 'Pledge', $pledge->id, $pledge);
-    }
-    else {
-      CRM_Utils_Hook::post('create', 'Pledge', $pledge->id, $pledge);
-    }
+    CRM_Utils_Hook::post($hook, 'Pledge', $pledge->id, $pledge);
 
     return $result;
   }
@@ -121,25 +112,12 @@ class CRM_Pledge_BAO_Pledge extends CRM_Pledge_DAO_Pledge {
    * Takes an associative array and creates a pledge object.
    *
    * @param array $params
-   *   (reference ) an assoc array of name/value pairs.
+   *   Assoc array of name/value pairs.
    *
-   * @return CRM_Pledge_BAO_Pledge
+   * @return CRM_Pledge_DAO_Pledge
+   * @throws \CRM_Core_Exception
    */
-  public static function create(&$params) {
-    // FIXME: a cludgy hack to fix the dates to MySQL format
-    $dateFields = [
-      'start_date',
-      'create_date',
-      'acknowledge_date',
-      'modified_date',
-      'cancel_date',
-      'end_date',
-    ];
-    foreach ($dateFields as $df) {
-      if (isset($params[$df])) {
-        $params[$df] = CRM_Utils_Date::isoToMysql($params[$df]);
-      }
-    }
+  public static function create($params) {
 
     $isRecalculatePledgePayment = self::isPaymentsRequireRecalculation($params);
     $transaction = new CRM_Core_Transaction();

CRM/Pledge/BAO/PledgePayment.php

@@ -86,7 +86,7 @@ WHERE     pledge_id = %1
     $transaction = new CRM_Core_Transaction();
     $overdueStatusID = CRM_Core_PseudoConstant::getKey('CRM_Pledge_BAO_PledgePayment', 'status_id', 'Overdue');
     $pendingStatusId = CRM_Core_PseudoConstant::getKey('CRM_Pledge_BAO_PledgePayment', 'status_id', 'Pending');
-
+    $currency = $params['currency'] ?? CRM_Core_Config::singleton()->defaultCurrency;
     //calculate the scheduled date for every installment
     $now = date('Ymd') . '000000';
     $statues = $prevScheduledDate = [];
@@ -110,7 +110,7 @@ WHERE     pledge_id = %1
     }
 
     if ($params['installment_amount']) {
-      $params['scheduled_amount'] = $params['installment_amount'];
+      $params['scheduled_amount'] = round($params['installment_amount'], CRM_Utils_Money::getCurrencyPrecision($currency));
     }
     else {
       $params['scheduled_amount'] = round(($params['amount'] / $params['installments']), 2);

tests/phpunit/CRM/Pledge/BAO/PledgePaymentTest.php

@@ -422,10 +422,13 @@ class CRM_Pledge_BAO_PledgePaymentTest extends CiviUnitTestCase {
    * More specifically, in the UI this would be equivalent to creating a $100
    * pledge to be paid in 11 installments of $8.33 and one installment of $8.37
    * (to compensate the missing $0.04 from round(100/12)*12.
-   * The API does not allow to do this kind of pledge, because the BAO recalculates
-   * the 'amount' using original_installment_amount * installment.
+   * The API does not allow to do this kind of pledge, because the BAO
+   * recalculates the 'amount' using original_installment_amount * installment.
+   *
+   * @throws \CRM_Core_Exception
+   * @throws \CiviCRM_API3_Exception
    */
-  public function testCreatePledgePaymentForMultipleInstallments2() {
+  public function testCreatePledgePaymentForMultipleInstallments2(): void {
     $scheduled_date = date('Ymd', mktime(0, 0, 0, date("m"), date("d") + 2, date("y")));
     $contact_id = 2;
 
@@ -446,7 +449,7 @@ class CRM_Pledge_BAO_PledgePaymentTest extends CiviUnitTestCase {
       'sequential' => 1,
     ];
 
-    $pledge = CRM_Pledge_BAO_Pledge::create($params);
+    $pledge = $this->callAPISuccess('Pledge', 'create', $params);
 
     $contributionID = $this->contributionCreate([
       'contact_id' => $contact_id,
@@ -462,7 +465,7 @@ class CRM_Pledge_BAO_PledgePaymentTest extends CiviUnitTestCase {
 
     // Fetch the first planned pledge payment/installment
     $pledgePayments = civicrm_api3('PledgePayment', 'get', [
-      'pledge_id' => $pledge->id,
+      'pledge_id' => $pledge['id'],
       'sequential' => 1,
     ]);
 
@@ -492,7 +495,7 @@ class CRM_Pledge_BAO_PledgePaymentTest extends CiviUnitTestCase {
     // Fetch the pledge payments again to see if the amounts and statuses
     // have been updated correctly.
     $pledgePayments = $this->callAPISuccess('pledge_payment', 'get', [
-      'pledge_id' => $pledge->id,
+      'pledge_id' => $pledge['id'],
       'sequential' => 1,
     ]);
 
@@ -511,11 +514,11 @@ class CRM_Pledge_BAO_PledgePaymentTest extends CiviUnitTestCase {
       $this->assertEquals(1, $pp['status_id']);
     }
 
-    $this->assertEquals(count($pledgePayments['values']), CRM_Pledge_BAO_Pledge::pledgeHasFinancialTransactions($pledge->id, 2));
+    $this->assertEquals(count($pledgePayments['values']), CRM_Pledge_BAO_Pledge::pledgeHasFinancialTransactions($pledge['id'], 2));
 
     // Cleanup
     civicrm_api3('Pledge', 'delete', [
-      'id' => $pledge->id,
+      'id' => $pledge['id'],
     ]);
   }
 

tests/phpunit/CRM/Pledge/BAO/PledgeTest.php

@@ -20,21 +20,23 @@ class CRM_Pledge_BAO_PledgeTest extends CiviUnitTestCase {
   /**
    * Sets up the fixture, for example, opens a network connection.
    * This method is called before a test is executed.
+   *
+   * @throws \CiviCRM_API3_Exception
    */
   protected function setUp() {
     parent::setUp();
-    $this->_contactId = $this->individualCreate();
+    $this->ids['Contact'][0] = $this->individualCreate();
     $this->_params = [
-      'contact_id' => $this->_contactId,
+      'contact_id' => $this->ids['Contact'][0],
       'frequency_unit' => 'month',
       'original_installment_amount' => 25.00,
       'frequency_interval' => 1,
       'frequency_day' => 1,
       'installments' => 12,
       'financial_type_id' => 1,
-      'create_date' => '20100513000000',
-      'acknowledge_date' => '20100513000000',
-      'start_date' => '20100513000000',
+      'create_date' => '2010-05-13 00:00:00',
+      'acknowledge_date' => '2010-05-13 00:00:00',
+      'start_date' => '2010-05-13 00:00:00',
       'status_id' => 2,
       'currency' => 'USD',
       'amount' => 300,
@@ -50,65 +52,52 @@ class CRM_Pledge_BAO_PledgeTest extends CiviUnitTestCase {
 
   /**
    *  Test for Add/Update Pledge.
+   *
+   * @throws \CRM_Core_Exception
    */
-  public function testAdd() {
+  public function testAdd(): void {
     //do test for normal add.
-    $pledge = CRM_Pledge_BAO_Pledge::add($this->_params);
-
+    $pledgeID = $this->callAPISuccess('Pledge', 'create', $this->_params)['id'];
+    $pledge = new CRM_Pledge_DAO_Pledge();
+    $pledge->id = $pledgeID;
+    $pledge->find(TRUE);
+    unset($this->_params['status_id']);
     foreach ($this->_params as $param => $value) {
-      $this->assertEquals($value, $pledge->$param);
+      $this->assertEquals($value, $pledge->$param, $param);
     }
   }
 
   /**
    * Test Pledge Payment Status with 1 installment
    * and not passing status id.
+   *
+   * @throws \CRM_Core_Exception
    */
-  public function testPledgePaymentStatus() {
+  public function testPledgePaymentStatus(): void {
     $scheduledDate = date('Ymd', mktime(0, 0, 0, date("m"), date("d") + 2, date("y")));
     $this->_params['installments'] = 1;
     $this->_params['scheduled_date'] = $scheduledDate;
 
     unset($this->_params['status_id']);
-    $pledge = CRM_Pledge_BAO_Pledge::create($this->_params);
-    $pledgePayment = CRM_Pledge_BAO_PledgePayment::getPledgePayments($pledge->id);
+    $pledge = $this->callAPISuccess('Pledge', 'create', $this->_params);
+    $pledgePayment = CRM_Pledge_BAO_PledgePayment::getPledgePayments($pledge['id']);
 
-    $this->assertEquals(count($pledgePayment), 1);
+    $this->assertCount(1, $pledgePayment);
     $payment = array_pop($pledgePayment);
     // Assert that we actually have no pledge Payments
-    $this->assertEquals(0, CRM_Pledge_BAO_Pledge::pledgeHasFinancialTransactions($pledge->id, array_search('Pending', CRM_Contribute_PseudoConstant::contributionStatus(NULL, 'name'))));
-    $this->assertEquals($payment['status'], 'Pending');
+    $this->assertEquals(0, CRM_Pledge_BAO_Pledge::pledgeHasFinancialTransactions($pledge['id'], array_search('Pending', CRM_Contribute_PseudoConstant::contributionStatus(NULL, 'name'))));
+    $this->assertEquals('Pending', $payment['status']);
     $this->assertEquals($payment['scheduled_date'], date('Y-m-d 00:00:00', strtotime($scheduledDate)));
   }
 
-  /**
-   *  Retrieve a pledge based on a pledge id = 0
-   */
-  public function testRetrieveZeroPledeID() {
-    $defaults = [];
-    $params = ['pledge_id' => 0];
-    $pledgeId = CRM_Pledge_BAO_Pledge::retrieve($params, $defaults);
-
-    $this->assertEquals(is_null($pledgeId), 1, "Pledge Id must be greater than 0");
-  }
-
-  /**
-   *  Retrieve a payment based on a Null pledge id random string.
-   */
-  public function testRetrieveStringPledgeID() {
-    $defaults = [];
-    $params = ['pledge_id' => 'random text'];
-    $pledgeId = CRM_Pledge_BAO_Pledge::retrieve($params, $defaults);
-
-    $this->assertEquals(is_null($pledgeId), 1, "Pledge Id must be a string");
-  }
-
   /**
    *  Test that payment retrieve wrks based on known pledge id.
+   *
+   * @throws \CRM_Core_Exception
    */
-  public function testRetrieveKnownPledgeID() {
+  public function testRetrieveKnownPledgeID(): void {
     $params = [
-      'contact_id' => $this->_contactId,
+      'contact_id' => $this->ids['Contact'][0],
       'frequency_unit' => 'month',
       'frequency_interval' => 1,
       'frequency_day' => 1,
@@ -123,14 +112,14 @@ class CRM_Pledge_BAO_PledgeTest extends CiviUnitTestCase {
       'amount' => 300,
     ];
 
-    $pledge = CRM_Pledge_BAO_Pledge::add($params);
+    $pledge = $this->callAPISuccess('Pledge', 'create', $params);
 
     $defaults = [];
-    $pledgeParams = ['pledge_id' => $pledge->id];
+    $pledgeParams = ['pledge_id' => $pledge['id']];
 
     $pledgeId = CRM_Pledge_BAO_Pledge::retrieve($pledgeParams, $defaults);
 
-    $this->assertEquals($pledgeId->N, 1, "Pledge was retrieved");
+    $this->assertEquals(1, $pledgeId->N, "Pledge was retrieved");
   }
 
   /**