Skip to content

Commit

Permalink
Fix pledge to not use pass-by-reference
Browse files Browse the repository at this point in the history 
We have deprecated this out of BAO classes
  • Loading branch information
@eileenmcnaughton
eileenmcnaughton committed Jan 17, 2021
1 parent efbd974 commit a4c85ec
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 54 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
13 changes: 8 additions & 5 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
42 changes: 25 additions & 17 deletions tests/phpunit/CRM/Pledge/BAO/PledgeTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@ 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,
Expand All @@ -50,10 +52,12 @@ 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);
$pledge = $this->callAPISuccess('Pledge', 'create', $this->_params);

foreach ($this->_params as $param => $value) {
$this->assertEquals($value, $pledge->$param);
Expand All @@ -63,33 +67,35 @@ public function testAdd() {
/**
* 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() {
public function testRetrieveZeroPledeID(): void {
$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");
$this->assertEquals(1, is_null($pledgeId), "Pledge Id must be greater than 0");
}

/**
Expand All @@ -105,10 +111,12 @@ public function testRetrieveStringPledgeID() {

/**
* 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 +131,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 a4c85ec

Please sign in to comment.