From aba748e5599b058abc2d7a5934039f0f9894f508 Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 25 Mar 2021 11:29:47 +1300 Subject: [PATCH] [REF] Simplify calculation of recurring membership parameters We need frequency_unit & frequency_interval for the params we pass to the payment process & those we use to create the recurring - this simplifies them to be calculated in a function rather than being passed around For the legacyProcessRecurringContribution function to be reached auto_renew must be true --- CRM/Financial/BAO/Order.php | 32 +++++++++ CRM/Member/Form/Membership.php | 69 +++++++++++-------- .../CRM/Member/Form/MembershipTest.php | 15 ++-- 3 files changed, 83 insertions(+), 33 deletions(-) diff --git a/CRM/Financial/BAO/Order.php b/CRM/Financial/BAO/Order.php index 47085372b7..d4916e7e4e 100644 --- a/CRM/Financial/BAO/Order.php +++ b/CRM/Financial/BAO/Order.php @@ -340,6 +340,38 @@ class CRM_Financial_BAO_Order { return $lines; } + /** + * Get an array of all membership types included in the order. + * + * @return array + * + * @throws \CiviCRM_API3_Exception + */ + public function getMembershipTypes(): array { + $types = []; + foreach ($this->getMembershipLineItems() as $line) { + $types[$line['membership_type_id']] = CRM_Member_BAO_MembershipType::getMembershipType((int) $line['membership_type_id']); + } + return $types; + } + + /** + * Get an array of all membership types included in the order. + * + * @return array + * + * @throws \CiviCRM_API3_Exception + */ + public function getRenewableMembershipTypes(): array { + $types = []; + foreach ($this->getMembershipTypes() as $id => $type) { + if (!empty($type['auto_renew'])) { + $types[$id] = $type; + } + } + return $types; + } + /** * @return array * @throws \CiviCRM_API3_Exception diff --git a/CRM/Member/Form/Membership.php b/CRM/Member/Form/Membership.php index 8737dd70cb..242cf55deb 100644 --- a/CRM/Member/Form/Membership.php +++ b/CRM/Member/Form/Membership.php @@ -1019,30 +1019,6 @@ DESC limit 1"); $membershipTypeValues[$memType]['membership_type_id'] = $memType; } - //take the required membership recur values. - if ($this->isCreateRecurringContribution()) { - $count = 0; - foreach ($this->_memTypeSelected as $memType) { - $recurMembershipTypeValues = CRM_Utils_Array::value($memType, - $this->allMembershipTypeDetails, [] - ); - if (!$recurMembershipTypeValues['auto_renew']) { - continue; - } - foreach ([ - 'frequency_interval' => 'duration_interval', - 'frequency_unit' => 'duration_unit', - ] as $mapVal => $mapParam) { - if (!$count) { - $formValues[$mapVal] = CRM_Utils_Array::value($mapParam, - $recurMembershipTypeValues - ); - } - } - $count++; - } - } - $isQuickConfig = $this->_priceSet['is_quick_config']; $termsByType = []; @@ -1177,6 +1153,9 @@ DESC limit 1"); // all the payment processors expect the name and address to be in the // so we copy stuff over to first_name etc. $paymentParams = $formValues; + $paymentParams['frequency_unit'] = $this->getFrequencyUnit(); + $paymentParams['frequency_interval'] = $this->getFrequencyInterval(); + $paymentParams['contactID'] = $this->_contributorContactID; //CRM-10377 if payment is by an alternate contact then we need to set that person // as the contact in the payment params @@ -1774,9 +1753,10 @@ DESC limit 1"); } $recurParams = ['contact_id' => $contactID]; $recurParams['amount'] = $this->order->getTotalAmount(); - $recurParams['auto_renew'] = $params['auto_renew'] ?? NULL; - $recurParams['frequency_unit'] = $params['frequency_unit'] ?? NULL; - $recurParams['frequency_interval'] = $params['frequency_interval'] ?? NULL; + // for the legacyProcessRecurringContribution function to be reached auto_renew must be true + $recurParams['auto_renew'] = TRUE; + $recurParams['frequency_unit'] = $this->getFrequencyUnit(); + $recurParams['frequency_interval'] = $this->getFrequencyInterval(); $recurParams['installments'] = $params['installments'] ?? NULL; $recurParams['financial_type_id'] = $this->getFinancialTypeID(); $recurParams['currency'] = $this->getCurrency(); @@ -1819,6 +1799,41 @@ DESC limit 1"); return (int) $this->getSubmittedValue('financial_type_id') ?: $this->order->getFinancialTypeID(); } + /** + * Get the membership type, if any, to be recurred. + * + * @return array + * @throws \CiviCRM_API3_Exception + */ + protected function getRecurMembershipType(): array { + foreach ($this->order->getRenewableMembershipTypes() as $type) { + return $type; + } + return []; + } + + /** + * Get the frequency interval. + * + * @return int|null + * @throws \CiviCRM_API3_Exception + */ + protected function getFrequencyInterval(): ?int { + $membershipType = $this->getRecurMembershipType(); + return empty($membershipType) ? NULL : (int) $membershipType['duration_interval']; + } + + /** + * Get the frequency interval. + * + * @return string|null + * @throws \CiviCRM_API3_Exception + */ + protected function getFrequencyUnit(): ?string { + $membershipType = $this->getRecurMembershipType(); + return empty($membershipType) ? NULL : (string) $membershipType['duration_unit']; + } + /** * Get values that should be passed to all membership create actions. * diff --git a/tests/phpunit/CRM/Member/Form/MembershipTest.php b/tests/phpunit/CRM/Member/Form/MembershipTest.php index c85d66598e..e649cd86ec 100644 --- a/tests/phpunit/CRM/Member/Form/MembershipTest.php +++ b/tests/phpunit/CRM/Member/Form/MembershipTest.php @@ -18,6 +18,7 @@ */ use Civi\Api4\FinancialType; +use Civi\Api4\MembershipType; /** * Test CRM_Member_Form_Membership functions. @@ -83,7 +84,7 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase { * @throws \CRM_Core_Exception * @throws \CiviCRM_API3_Exception */ - public function setUp() { + public function setUp(): void { $this->_apiversion = 3; parent::setUp(); @@ -188,12 +189,12 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase { $obj = new CRM_Member_Form_Membership(); $rc = CRM_Member_Form_Membership::formRule($params, $files, $obj); $this->assertType('array', $rc); - $this->assertTrue(array_key_exists('membership_type_id', $rc)); + $this->assertArrayHasKey('membership_type_id', $rc); $params['membership_type_id'] = [1 => 3]; $rc = CRM_Member_Form_Membership::formRule($params, $files, $obj); $this->assertType('array', $rc); - $this->assertTrue(array_key_exists('join_date', $rc)); + $this->assertArrayHasKey('join_date', $rc); } /** @@ -203,7 +204,7 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase { * that has an start date before the join date and a rolling * membership type. */ - public function testFormRuleRollingEarlyStart() { + public function testFormRuleRollingEarlyStart(): void { $unixNow = time(); $unixYesterday = $unixNow - (24 * 60 * 60); $ymdYesterday = date('Y-m-d', $unixYesterday); @@ -827,16 +828,18 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase { * * @throws \CRM_Core_Exception * @throws \CiviCRM_API3_Exception + * @throws \API_Exception */ public function testSubmitRecurTwoRows(): void { $pfvIDs = $this->createMembershipPriceSet(); + MembershipType::update() + ->addWhere('id', '=', $this->ids['membership_type']['AnnualRollingOrg2']) + ->setValues(['frequency_interval' => 1, 'frequency_unit' => 'month', 'auto_renew' => 1])->execute(); $form = $this->getForm(); $form->_mode = 'live'; $priceParams = [ 'price_' . $this->getPriceFieldID() => $pfvIDs, 'price_set_id' => $this->getPriceSetID(), - 'frequency_interval' => 1, - 'frequency_unit' => 'month', 'membership_type_id' => NULL, // Set financial type id to null to check it is retrieved from the price set. 'financial_type_id' => NULL, -- 2.25.1