From a89e0a3e110e8ff459a9737c2022395645906459 Mon Sep 17 00:00:00 2001 From: eileen Date: Wed, 29 Jul 2020 12:14:30 +1200 Subject: [PATCH] Reduce calls to CRM_Member_PseudoConstant::membershipType This function defaults to not using the cached version so that's a pretty good argument for making a more active effort to deprecate it. The createMembership function flushes it so we don't need to do that so much in tests now either. I also included swapping status over in the test --- CRM/Activity/BAO/Activity.php | 12 +++++----- CRM/Member/BAO/Membership.php | 4 ++-- .../phpunit/CRM/Core/Payment/BaseIPNTest.php | 1 - tests/phpunit/api/v3/MembershipTest.php | 22 +++++++++---------- 4 files changed, 18 insertions(+), 21 deletions(-) diff --git a/CRM/Activity/BAO/Activity.php b/CRM/Activity/BAO/Activity.php index a4065942ca..0046a5baa3 100644 --- a/CRM/Activity/BAO/Activity.php +++ b/CRM/Activity/BAO/Activity.php @@ -1783,16 +1783,14 @@ WHERE activity.id IN ($activityIds)"; * particular component object. * * @return string + * @throws \CRM_Core_Exception */ public static function getActivitySubject($entityObj) { + // @todo determine the subject on the appropriate entity rather than from the activity. switch ($entityObj->__table) { case 'civicrm_membership': - $membershipType = CRM_Member_PseudoConstant::membershipType($entityObj->membership_type_id); - $subject = $membershipType ? $membershipType : ts('Membership'); - - if (is_array($subject)) { - $subject = implode(", ", $subject); - } + $membershipType = CRM_Core_PseudoConstant::getLabel('CRM_Member_BAO_Membership', 'membership_type_id', $entityObj->membership_type_id); + $subject = $membershipType ?: ts('Membership'); if (!CRM_Utils_System::isNull($entityObj->source)) { $subject .= " - {$entityObj->source}"; @@ -1803,7 +1801,7 @@ WHERE activity.id IN ($activityIds)"; $subject .= sprintf(' (by %s)', $displayName); } - $subject .= " - Status: " . CRM_Core_DAO::getFieldValue('CRM_Member_DAO_MembershipStatus', $entityObj->status_id, 'label'); + $subject .= ' - Status: ' . CRM_Core_PseudoConstant::getLabel('CRM_Member_BAO_Membership', 'status_id', $entityObj->status_id); return $subject; case 'civicrm_participant': diff --git a/CRM/Member/BAO/Membership.php b/CRM/Member/BAO/Membership.php index 18cffb95a5..6640b4b113 100644 --- a/CRM/Member/BAO/Membership.php +++ b/CRM/Member/BAO/Membership.php @@ -403,8 +403,8 @@ class CRM_Member_BAO_Membership extends CRM_Member_DAO_Membership { // ie in an update situation. $membership->find(TRUE); } - $membershipTypes = CRM_Member_PseudoConstant::membershipType(); - $title = CRM_Contact_BAO_Contact::displayName($membership->contact_id) . ' - ' . ts('Membership Type:') . ' ' . $membershipTypes[$membership->membership_type_id]; + $title = CRM_Contact_BAO_Contact::displayName($membership->contact_id) . ' - ' . ts('Membership Type:') + . ' ' . CRM_Core_PseudoConstant::getLabel('CRM_Member_BAO_Membership', 'membership_type_id', $membership->membership_type_id); $recentOther = []; if (CRM_Core_Permission::checkActionPermission('CiviMember', CRM_Core_Action::UPDATE)) { diff --git a/tests/phpunit/CRM/Core/Payment/BaseIPNTest.php b/tests/phpunit/CRM/Core/Payment/BaseIPNTest.php index 746dc8c1a7..758503c6c5 100644 --- a/tests/phpunit/CRM/Core/Payment/BaseIPNTest.php +++ b/tests/phpunit/CRM/Core/Payment/BaseIPNTest.php @@ -93,7 +93,6 @@ class CRM_Core_Payment_BaseIPNTest extends CiviUnitTestCase { */ public function tearDown() { $this->quickCleanUpFinancialEntities(); - CRM_Member_PseudoConstant::membershipType(NULL, TRUE); CRM_Member_PseudoConstant::membershipStatus(NULL, NULL, 'name', TRUE); } diff --git a/tests/phpunit/api/v3/MembershipTest.php b/tests/phpunit/api/v3/MembershipTest.php index 5d0bc09510..bd956ebf6d 100644 --- a/tests/phpunit/api/v3/MembershipTest.php +++ b/tests/phpunit/api/v3/MembershipTest.php @@ -50,7 +50,6 @@ class api_v3_MembershipTest extends CiviUnitTestCase { ]); $this->_membershipStatusID = $this->membershipStatusCreate('test status'); - CRM_Member_PseudoConstant::membershipType(NULL, TRUE); CRM_Member_PseudoConstant::membershipStatus(NULL, NULL, 'name', TRUE); CRM_Core_PseudoConstant::activityType(TRUE, TRUE, TRUE, 'name'); @@ -114,7 +113,7 @@ class api_v3_MembershipTest extends CiviUnitTestCase { 'total_amount' => 100, 'contact_id' => $this->_params['contact_id'], ]); - $membershipPaymentCreate = $this->callAPISuccess('MembershipPayment', 'create', [ + $this->callAPISuccess('MembershipPayment', 'create', [ 'sequential' => 1, 'contribution_id' => $ContributionCreate['values'][0]['id'], 'membership_id' => $membershipID, @@ -139,20 +138,17 @@ class api_v3_MembershipTest extends CiviUnitTestCase { public function testActivityForCancelledContribution() { $contactId = $this->createLoggedInUser(); $membershipID = $this->contactMembershipCreate($this->_params); - $this->assertDBRowExist('CRM_Member_DAO_Membership', $membershipID); $ContributionCreate = $this->callAPISuccess('Contribution', 'create', [ - 'financial_type_id' => "Member Dues", + 'financial_type_id' => 'Member Dues', 'total_amount' => 100, 'contact_id' => $this->_params['contact_id'], ]); - $membershipPaymentCreate = $this->callAPISuccess('MembershipPayment', 'create', [ + $this->callAPISuccess('MembershipPayment', 'create', [ 'sequential' => 1, 'contribution_id' => $ContributionCreate['id'], 'membership_id' => $membershipID, ]); - $instruments = $this->callAPISuccess('contribution', 'getoptions', ['field' => 'payment_instrument_id']); - $this->paymentInstruments = $instruments['values']; $form = new CRM_Contribute_Form_Contribution(); $form->_id = $ContributionCreate['id']; @@ -160,16 +156,20 @@ class api_v3_MembershipTest extends CiviUnitTestCase { 'total_amount' => 100, 'financial_type_id' => 1, 'contact_id' => $contactId, - 'payment_instrument_id' => array_search('Check', $this->paymentInstruments), + 'payment_instrument_id' => CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'payment_instrument_id', 'Check'), 'contribution_status_id' => 3, ], CRM_Core_Action::UPDATE); - $activity = $this->callAPISuccess('Activity', 'get', [ - 'activity_type_id' => "Change Membership Status", + $this->callAPISuccessGetSingle('Activity', [ + 'activity_type_id' => 'Membership Signup', + 'source_record_id' => $membershipID, + 'subject' => 'General - Payment - Status: test status', + ]); + $this->callAPISuccessGetSingle('Activity', [ + 'activity_type_id' => 'Change Membership Status', 'source_record_id' => $membershipID, ]); - $this->assertNotEmpty($activity['values']); } /** -- 2.25.1