From eb151aab7216259a9995618fdffc65b5f18f512a Mon Sep 17 00:00:00 2001 From: eileen Date: Sun, 18 Aug 2019 09:39:19 +1200 Subject: [PATCH] Fix readability & caching on CRM_Contact_BAO_Relationship::isInheritedMembershipInvalidated This removes an sql query & some hard wrangling & uses the api - wrapped in a caching helper. --- CRM/Contact/BAO/Relationship.php | 24 ++++++++++-------------- CRM/Member/BAO/MembershipType.php | 31 +++++++++++++++++++++++++++++++ CRM/Utils/System.php | 1 + Civi/Core/Container.php | 3 ++- api/v3/MembershipType.php | 1 + 5 files changed, 45 insertions(+), 15 deletions(-) diff --git a/CRM/Contact/BAO/Relationship.php b/CRM/Contact/BAO/Relationship.php index cc46c9c1fa..f9a49f68ca 100644 --- a/CRM/Contact/BAO/Relationship.php +++ b/CRM/Contact/BAO/Relationship.php @@ -2321,22 +2321,18 @@ AND cc.sort_name LIKE '%$name%'"; * @param int $mainRelatedContactId * * @return array + * @throws \CiviCRM_API3_Exception */ private static function isInheritedMembershipInvalidated($membershipValues, array $values, $cid, $mainRelatedContactId): array { - // Delete memberships of the related contacts only if relationship type exists for membership type - $query = " -SELECT relationship_type_id, relationship_direction - FROM civicrm_membership_type - WHERE id = {$membershipValues['membership_type_id']}"; - $dao = CRM_Core_DAO::executeQuery($query); - while ($dao->fetch()) { - $relTypeId = $dao->relationship_type_id; - } - $relTypeIds = explode(CRM_Core_DAO::VALUE_SEPARATOR, $relTypeId); - $isDeletable = in_array($values[$cid]['relationshipTypeId'], $relTypeIds - //CRM-16300 check if owner membership exist for related membership - ) && !empty($membershipValues['owner_membership_id']) && !empty($values[$mainRelatedContactId]['memberships'][$membershipValues['owner_membership_id']]); - return [$relTypeId, $isDeletable]; + $membershipType = CRM_Member_BAO_MembershipType::getMembershipType($membershipValues['membership_type_id']); + $relTypeIds = $membershipType['relationship_type_id']; + $membshipInheritedFrom = $membershipValues['owner_membership_id'] ?? NULL; + if (!$membshipInheritedFrom || !in_array($values[$cid]['relationshipTypeId'], $relTypeIds)) { + return [implode(',', $relTypeIds), FALSE]; + } + //CRM-16300 check if owner membership exist for related membership + $isDeletable = !empty($values[$mainRelatedContactId]['memberships'][$membshipInheritedFrom]); + return [implode(',', $relTypeIds), $isDeletable]; } } diff --git a/CRM/Member/BAO/MembershipType.php b/CRM/Member/BAO/MembershipType.php index 7452946151..7c16cd5995 100644 --- a/CRM/Member/BAO/MembershipType.php +++ b/CRM/Member/BAO/MembershipType.php @@ -156,6 +156,7 @@ class CRM_Member_BAO_MembershipType extends CRM_Member_DAO_MembershipType { CRM_Member_PseudoConstant::membershipType(NULL, TRUE); civicrm_api3('membership', 'getfields', ['cache_clear' => 1, 'fieldname' => 'membership_type_id']); civicrm_api3('profile', 'getfields', ['action' => 'submit', 'cache_clear' => 1]); + Civi::cache('metadata')->clear(); } /** @@ -266,6 +267,8 @@ class CRM_Member_BAO_MembershipType extends CRM_Member_DAO_MembershipType { /** * Get membership Types. * + * @deprecated use getAllMembershipTypes. + * * @param bool $public * * @return array @@ -288,6 +291,8 @@ class CRM_Member_BAO_MembershipType extends CRM_Member_DAO_MembershipType { /** * Get membership Type Details. * + * @deprecated use getMembershipType. + * * @param int $membershipTypeId * * @return array|null @@ -841,4 +846,30 @@ class CRM_Member_BAO_MembershipType extends CRM_Member_DAO_MembershipType { } } + /** + * Cached wrapper for membership types. + * + * Since this is used from the batched script caching helps. + * + * @throws \CiviCRM_API3_Exception + */ + public static function getAllMembershipTypes() { + if (!Civi::cache('metadata')->has(__CLASS__ . __FUNCTION__)) { + Civi::cache('metadata')->set(__CLASS__ . __FUNCTION__, civicrm_api3('MembershipType', 'get', ['options' => ['limit' => 0, 'sort' => 'weight']])['values']); + } + return Civi::cache('metadata')->get(__CLASS__ . __FUNCTION__); + } + + /** + * Get a specific membership type (leveraging the cache). + * + * @param int $id + * + * @return mixed + * @throws \CiviCRM_API3_Exception + */ + public static function getMembershipType($id) { + return self::getAllMembershipTypes()[$id]; + } + } diff --git a/CRM/Utils/System.php b/CRM/Utils/System.php index 53ccf5f2c6..dc3b5466f6 100644 --- a/CRM/Utils/System.php +++ b/CRM/Utils/System.php @@ -1443,6 +1443,7 @@ class CRM_Utils_System { Civi::cache('navigation')->flush(); Civi::cache('customData')->flush(); Civi::cache('contactTypes')->clear(); + Civi::cache('metadata')->clear(); CRM_Extension_System::singleton()->getCache()->flush(); CRM_Cxn_CiviCxnHttp::singleton()->getCache()->flush(); } diff --git a/Civi/Core/Container.php b/Civi/Core/Container.php index 0c67eb5a53..0dfebb73ed 100644 --- a/Civi/Core/Container.php +++ b/Civi/Core/Container.php @@ -160,6 +160,7 @@ class Container { 'customData' => 'custom data', 'fields' => 'contact fields', 'contactTypes' => 'contactTypes', + 'metadata' => 'metadata', ]; foreach ($basicCaches as $cacheSvc => $cacheGrp) { $definitionParams = [ @@ -169,7 +170,7 @@ class Container { // For Caches that we don't really care about the ttl for and/or maybe accessed // fairly often we use the fastArrayDecorator which improves reads and writes, these // caches should also not have concurrency risk. - $fastArrayCaches = ['groups', 'navigation', 'customData', 'fields', 'contactTypes']; + $fastArrayCaches = ['groups', 'navigation', 'customData', 'fields', 'contactTypes', 'metadata']; if (in_array($cacheSvc, $fastArrayCaches)) { $definitionParams['withArray'] = 'fast'; } diff --git a/api/v3/MembershipType.php b/api/v3/MembershipType.php index 386626c548..c0247f45f0 100644 --- a/api/v3/MembershipType.php +++ b/api/v3/MembershipType.php @@ -87,6 +87,7 @@ function civicrm_api3_membership_type_get($params) { // Workaround for fields using nonstandard serialization foreach (['relationship_type_id', 'relationship_direction'] as $field) { if (isset($item[$field]) && !is_array($item[$field])) { + // @todo - this should be handled by the serialization now... $item[$field] = (array) $item[$field]; } } -- 2.25.1