Skip to content

Commit

Permalink
Merge pull request #19400 from eileenmcnaughton/pledge_ref
Browse files Browse the repository at this point in the history 
Fix pledge to not use pass-by-reference
  • Loading branch information
@seamuslee001
seamuslee001 authored Jan 18, 2021
2 parents 343b1e7 + 45ae836 commit 98cd4c3
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 84 deletions.
42 changes: 10 additions & 32 deletions CRM/Pledge/BAO/Pledge.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,12 @@ public static function retrieve(&$params, &$defaults) {
* @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();

Expand All @@ -85,12 +81,7 @@ public static function add(&$params) {

$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;
}
Expand Down Expand Up @@ -121,25 +112,12 @@ public static function &getValues(&$params, &$values, $returnProperties = NULL)
* 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();
Expand Down
4 changes: 2 additions & 2 deletions CRM/Pledge/BAO/PledgePayment.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public static function createMultiple(array $params) {
$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 = [];
Expand All @@ -110,7 +110,7 @@ public static function createMultiple(array $params) {
}

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);
Expand Down
19 changes: 11 additions & 8 deletions tests/phpunit/CRM/Pledge/BAO/PledgePaymentTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -422,10 +422,13 @@ public function testCreatePledgePaymentForMultipleInstallments() {
* 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;

Expand All @@ -446,7 +449,7 @@ public function testCreatePledgePaymentForMultipleInstallments2() {
'sequential' => 1,
];

$pledge = CRM_Pledge_BAO_Pledge::create($params);
$pledge = $this->callAPISuccess('Pledge', 'create', $params);

$contributionID = $this->contributionCreate([
'contact_id' => $contact_id,
Expand All @@ -462,7 +465,7 @@ public function testCreatePledgePaymentForMultipleInstallments2() {

// Fetch the first planned pledge payment/installment
$pledgePayments = civicrm_api3('PledgePayment', 'get', [
'pledge_id' => $pledge->id,
'pledge_id' => $pledge['id'],
'sequential' => 1,
]);

Expand Down Expand Up @@ -492,7 +495,7 @@ public function testCreatePledgePaymentForMultipleInstallments2() {
// 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,
]);

Expand All @@ -511,11 +514,11 @@ public function testCreatePledgePaymentForMultipleInstallments2() {
$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'],
]);
}

Expand Down
73 changes: 31 additions & 42 deletions tests/phpunit/CRM/Pledge/BAO/PledgeTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -50,65 +52,52 @@ protected function tearDown() {

/**
* 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,
Expand All @@ -123,14 +112,14 @@ public function testRetrieveKnownPledgeID() {
'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");
}

/**
Expand Down

0 comments on commit 98cd4c3

Please sign in to comment.