From c4f19d0f8296491babad027c870a1223acf46e9a Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Mon, 6 Dec 2021 11:09:33 -0500 Subject: [PATCH] Apiv4 - Make Groups a managed entity, fix 'null' bugs in BAO_Group Adds Group as an APIv4 managed entity, with tests. This allows SmartGroups to be exported along with Saved Searches. The tests turned up bugs in the Group BAO, where the API passes the string 'null' to represent null values but the BAO misinterprets it as a value. --- CRM/Contact/BAO/Group.php | 41 +++++++------------ CRM/Core/ManagedEntities.php | 2 +- Civi/Api4/Group.php | 1 + .../api/v4/Entity/ManagedEntityTest.php | 27 ++++++++++++ 4 files changed, 44 insertions(+), 27 deletions(-) diff --git a/CRM/Contact/BAO/Group.php b/CRM/Contact/BAO/Group.php index 3079dda7c9..9a096e1ec1 100644 --- a/CRM/Contact/BAO/Group.php +++ b/CRM/Contact/BAO/Group.php @@ -336,13 +336,13 @@ class CRM_Contact_BAO_Group extends CRM_Contact_DAO_Group { * The new group BAO (if created) */ public static function create(&$params) { + $params += [ + 'group_type' => NULL, + 'parents' => NULL, + ]; - if (!empty($params['id'])) { - CRM_Utils_Hook::pre('edit', 'Group', $params['id'], $params); - } - else { - CRM_Utils_Hook::pre('create', 'Group', NULL, $params); - } + $hook = empty($params['id']) ? 'create' : 'edit'; + CRM_Utils_Hook::pre($hook, 'Group', $params['id'] ?? NULL, $params); // If title isn't specified, retrieve it because we use it later, e.g. // for RecentItems. But note we use array_key_exists not isset or empty @@ -369,7 +369,7 @@ class CRM_Contact_BAO_Group extends CRM_Contact_DAO_Group { 'is_active' => 1, ]); if (count($parentIds) >= 1 && $activeParentsCount <= 1) { - $setDisable = self::setIsActive($childValue, CRM_Utils_Array::value('is_active', $params, 1)); + self::setIsActive($childValue, (int) ($params['is_active'] ?? 1)); } } } @@ -379,17 +379,14 @@ class CRM_Contact_BAO_Group extends CRM_Contact_DAO_Group { $params['name'] = CRM_Utils_String::titleToVar($params['title']); } - if (!empty($params['parents'])) { + if (!CRM_Utils_System::isNull($params['parents'])) { $params['parents'] = CRM_Utils_Array::convertCheckboxFormatToArray((array) $params['parents']); } // convert params if array type - if (isset($params['group_type'])) { + if (!CRM_Utils_System::isNull($params['group_type'])) { $params['group_type'] = CRM_Utils_Array::convertCheckboxFormatToArray((array) $params['group_type']); } - else { - $params['group_type'] = NULL; - } $session = CRM_Core_Session::singleton(); $cid = $session->get('userID'); @@ -404,7 +401,7 @@ class CRM_Contact_BAO_Group extends CRM_Contact_DAO_Group { // CRM-19068. // Validate parents parameter when creating group. - if (!empty($params['parents'])) { + if (!CRM_Utils_System::isNull($params['parents'])) { $parents = is_array($params['parents']) ? array_keys($params['parents']) : (array) $params['parents']; foreach ($parents as $parent) { CRM_Utils_Type::validate($parent, 'Integer'); @@ -413,9 +410,7 @@ class CRM_Contact_BAO_Group extends CRM_Contact_DAO_Group { $group = new CRM_Contact_BAO_Group(); $group->copyValues($params); - if (empty($params['id']) && - !$nameParam - ) { + if (empty($params['id']) && !$nameParam) { $group->name .= "_tmp"; } $group->save(); @@ -424,9 +419,8 @@ class CRM_Contact_BAO_Group extends CRM_Contact_DAO_Group { return NULL; } - if (empty($params['id']) && - !$nameParam - ) { + // Enforce unique name by appending id + if (empty($params['id']) && !$nameParam) { $group->name = substr($group->name, 0, -4) . "_{$group->id}"; } @@ -450,7 +444,7 @@ class CRM_Contact_BAO_Group extends CRM_Contact_DAO_Group { $params['parents'] = [$domainGroupID]; } - if (!empty($params['parents'])) { + if (!CRM_Utils_System::isNull($params['parents'])) { foreach ($params['parents'] as $parentId) { if ($parentId && !CRM_Contact_BAO_GroupNesting::isParentChild($parentId, $group->id)) { CRM_Contact_BAO_GroupNesting::add($parentId, $group->id); @@ -481,12 +475,7 @@ class CRM_Contact_BAO_Group extends CRM_Contact_DAO_Group { self::flushCaches(); CRM_Contact_BAO_GroupContactCache::invalidateGroupContactCache($group->id); - if (!empty($params['id'])) { - CRM_Utils_Hook::post('edit', 'Group', $group->id, $group); - } - else { - CRM_Utils_Hook::post('create', 'Group', $group->id, $group); - } + CRM_Utils_Hook::post($hook, 'Group', $group->id, $group); $recentOther = []; if (CRM_Core_Permission::check('edit groups')) { diff --git a/CRM/Core/ManagedEntities.php b/CRM/Core/ManagedEntities.php index 983bcbd93a..bcf5ea770d 100644 --- a/CRM/Core/ManagedEntities.php +++ b/CRM/Core/ManagedEntities.php @@ -315,7 +315,7 @@ class CRM_Core_ManagedEntities { $todo['params']['checkPermissions'] = FALSE; } - $result = civicrm_api($todo['entity_type'], 'create', $todo['params']); + $result = civicrm_api($todo['entity_type'], 'create', ['debug' => TRUE] + $todo['params']); if (!empty($result['is_error'])) { $this->onApiError($todo['entity_type'], 'create', $todo['params'], $result); } diff --git a/Civi/Api4/Group.php b/Civi/Api4/Group.php index 098d48d8fb..e92567a9d4 100644 --- a/Civi/Api4/Group.php +++ b/Civi/Api4/Group.php @@ -20,5 +20,6 @@ namespace Civi\Api4; * @package Civi\Api4 */ class Group extends Generic\DAOEntity { + use Generic\Traits\ManagedEntity; } diff --git a/tests/phpunit/api/v4/Entity/ManagedEntityTest.php b/tests/phpunit/api/v4/Entity/ManagedEntityTest.php index cdb4fd6df5..481beb99af 100644 --- a/tests/phpunit/api/v4/Entity/ManagedEntityTest.php +++ b/tests/phpunit/api/v4/Entity/ManagedEntityTest.php @@ -20,6 +20,7 @@ namespace api\v4\Entity; use api\v4\UnitTestCase; use Civi\Api4\Domain; +use Civi\Api4\Group; use Civi\Api4\Navigation; use Civi\Api4\OptionGroup; use Civi\Api4\OptionValue; @@ -560,6 +561,31 @@ class ManagedEntityTest extends UnitTestCase implements TransactionalInterface, $this->assertEquals(TRUE, $nav['is_active']); } + public function testExportAndCreateGroup() { + $original = Group::create(FALSE) + ->addValue('title', 'My Managed Group') + ->execute()->single(); + + $export = Group::export(FALSE) + ->setId($original['id']) + ->execute()->single(); + + Group::delete(FALSE)->addWhere('id', '=', $original['id'])->execute(); + + $this->_managedEntities = [ + ['module' => 'civicrm'] + $export, + ]; + \CRM_Core_ManagedEntities::singleton(TRUE)->reconcile(); + + $created = Group::get(FALSE) + ->addWhere('name', '=', $original['name']) + ->execute()->single(); + + $this->assertEquals('My Managed Group', $created['title']); + $this->assertEquals($original['name'], $created['name']); + $this->assertGreaterThan($original['id'], $created['id']); + } + /** * @dataProvider sampleEntityTypes * @param string $entityName @@ -585,6 +611,7 @@ class ManagedEntityTest extends UnitTestCase implements TransactionalInterface, 'ContactType' => TRUE, 'CustomField' => TRUE, 'CustomGroup' => TRUE, + 'Group' => TRUE, 'MembershipType' => TRUE, 'Navigation' => TRUE, 'OptionGroup' => TRUE, -- 2.25.1