Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix pledge to not use pass-by-reference #19400

Merged
merged 1 commit into from
Jan 18, 2021
Merged

Fix pledge to not use pass-by-reference #19400

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 10 additions & 32 deletions CRM/Pledge/BAO/Pledge.php
View file
Open in desktop
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 = [
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has not been needed for some years

'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
Open in desktop
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
Open in desktop
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
Open in desktop
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