From 3fc37a30874fcec1680e6e4a2fc89c4ed730211d Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 21 Nov 2019 21:39:50 +1300 Subject: [PATCH] Fix CRM_Contribute_BAO_ContributionTest to no longer use unreliable legacy set up method The whole partial_amount_to_pay params thing works badly & was actually the cause of other fixes stalling for 6 months. This is part of an effort to deprecate & eliminate it --- CRM/Contribute/BAO/Contribution.php | 8 ++- .../CRM/Contribute/BAO/ContributionTest.php | 51 ++++++++++--------- .../CRM/Member/Form/MembershipRenewalTest.php | 2 + tests/phpunit/CiviTest/CiviUnitTestCase.php | 2 + 4 files changed, 38 insertions(+), 25 deletions(-) diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index 0f1a0c6359..73d8eb5029 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -4841,12 +4841,16 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac } /** - * Function to add payments for contribution - * for Partially Paid status + * Function to add payments for contribution for Partially Paid status + * + * @deprecated this is known to be flawed and possibly buggy. + * + * Replace with Order.create->Payment.create flow. * * @param array $contributions * @param string $contributionStatusId * + * @throws \CiviCRM_API3_Exception */ public static function addPayments($contributions, $contributionStatusId = NULL) { // get financial trxn which is a payment diff --git a/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php b/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php index 60519a8882..5e2ca3972c 100644 --- a/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php +++ b/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php @@ -28,6 +28,8 @@ class CRM_Contribute_BAO_ContributionTest extends CiviUnitTestCase { /** * Test create method (create and update modes). + * + * @throws \CRM_Core_Exception */ public function testCreate() { $contactId = $this->individualCreate(); @@ -671,10 +673,16 @@ class CRM_Contribute_BAO_ContributionTest extends CiviUnitTestCase { } /** - * addPayments() method (add and edit modes of participant) + * addPayments() method (add and edit modes of participant). + * + * Add Payments is part of an old, flawed, code flow. */ public function testAddPayments() { - list($lineItems, $contribution) = $this->addParticipantWithContribution(); + $contribution = $this->addParticipantWithContribution(); + // Delete existing financial_trxns. This is because we are testing a code flow we + // want to deprecate & remove & the test relies on bad data asa starting point. + // End goal is the Order.create->Payment.create flow. + CRM_Core_DAO::executeQuery('DELETE FROM civicrm_entity_financial_trxn WHERE entity_table = "civicrm_financial_item"'); CRM_Contribute_BAO_Contribution::addPayments([$contribution]); $this->checkItemValues($contribution); } @@ -701,9 +709,16 @@ WHERE eft.entity_id = %1 AND ft.to_financial_account_id <> %2"; /** * assignProportionalLineItems() method (add and edit modes of participant) + * + * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception */ public function testAssignProportionalLineItems() { - list($lineItems, $contribution) = $this->addParticipantWithContribution(); + $contribution = $this->addParticipantWithContribution(); + // Delete existing financial_trxns. This is because we are testing a code flow we + // want to deprecate & remove & the test relies on bad data asa starting point. + // End goal is the Order.create->Payment.create flow. + CRM_Core_DAO::executeQuery('DELETE FROM civicrm_entity_financial_trxn WHERE entity_table = "civicrm_financial_item"'); $params = [ 'contribution_id' => $contribution->id, 'total_amount' => 150.00, @@ -719,25 +734,16 @@ WHERE eft.entity_id = %1 AND ft.to_financial_account_id <> %2"; * Add participant with contribution * * @return array + * + * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception */ public function addParticipantWithContribution() { // creating price set, price field $this->_contactId = $this->individualCreate(); - $event = $this->eventCreate(); + $event = $this->eventCreatePaid([]); $this->_eventId = $event['id']; - $paramsSet['title'] = 'Price Set' . substr(sha1(rand()), 0, 4); - $paramsSet['name'] = CRM_Utils_String::titleToVar($paramsSet['title']); - $paramsSet['is_active'] = TRUE; - $paramsSet['financial_type_id'] = 4; - $paramsSet['extends'] = 1; - - $priceset = CRM_Price_BAO_PriceSet::create($paramsSet); - $priceSetId = $priceset->id; - - //Checking for priceset added in the table. - $this->assertDBCompareValue('CRM_Price_BAO_PriceSet', $priceSetId, 'title', - 'id', $paramsSet['title'], 'Check DB for created priceset' - ); + $priceSetId = $this->priceSetID; $paramsField = [ 'label' => 'Price Field', 'name' => CRM_Utils_String::titleToVar('Price Field'), @@ -751,7 +757,7 @@ WHERE eft.entity_id = %1 AND ft.to_financial_account_id <> %2"; 'weight' => 1, 'options_per_line' => 1, 'is_active' => ['1' => 1, '2' => 1], - 'price_set_id' => $priceset->id, + 'price_set_id' => $this->priceSetID, 'is_enter_qty' => 1, 'financial_type_id' => CRM_Core_DAO::getFieldValue('CRM_Financial_DAO_FinancialType', 'Event Fee', 'id', 'name'), ]; @@ -775,16 +781,15 @@ WHERE eft.entity_id = %1 AND ft.to_financial_account_id <> %2"; ]; $participant = CRM_Event_BAO_Participant::add($participantParams); $contributionParams = [ - 'total_amount' => 150, + 'total_amount' => 300, 'currency' => 'USD', 'contact_id' => $this->_contactId, 'financial_type_id' => 4, - 'contribution_status_id' => 1, - 'partial_payment_total' => 300.00, - 'partial_amount_to_pay' => 150, + 'contribution_status_id' => 'Pending', 'contribution_mode' => 'participant', 'participant_id' => $participant->id, 'sequential' => TRUE, + 'api.Payment.create' => ['total_amount' => 150], ]; foreach ($priceFields['values'] as $key => $priceField) { @@ -812,7 +817,7 @@ WHERE eft.entity_id = %1 AND ft.to_financial_account_id <> %2"; $contributionObject->id = $contribution['id']; $contributionObject->find(TRUE); - return [$lineItems, $contributionObject]; + return $contributionObject; } /** diff --git a/tests/phpunit/CRM/Member/Form/MembershipRenewalTest.php b/tests/phpunit/CRM/Member/Form/MembershipRenewalTest.php index cf06ce363d..197bd8cd7e 100644 --- a/tests/phpunit/CRM/Member/Form/MembershipRenewalTest.php +++ b/tests/phpunit/CRM/Member/Form/MembershipRenewalTest.php @@ -98,6 +98,8 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase { * Clean up after each test. */ public function tearDown() { + $this->validateAllPayments(); + $this->validateAllContributions(); $this->quickCleanUpFinancialEntities(); $this->quickCleanup( [ diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php index 273c243b18..7b5cabb790 100644 --- a/tests/phpunit/CiviTest/CiviUnitTestCase.php +++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php @@ -1042,6 +1042,8 @@ class CiviUnitTestCase extends PHPUnit\Framework\TestCase { * @param array $params * * @return array + * + * @throws \CRM_Core_Exception */ protected function eventCreatePaid($params) { $event = $this->eventCreate($params); -- 2.25.1