diff --git a/CRM/ACL/API.php b/CRM/ACL/API.php index d9739d48d781..ce2fc6ae7f53 100644 --- a/CRM/ACL/API.php +++ b/CRM/ACL/API.php @@ -148,7 +148,7 @@ public static function whereClause( public static function group( $type, $contactID = NULL, - $tableName = 'civicrm_saved_search', + $tableName = 'civicrm_group', $allGroups = NULL, $includedGroups = [] ) { @@ -181,7 +181,7 @@ public static function groupPermission( $type, $groupID, $contactID = NULL, - $tableName = 'civicrm_saved_search', + $tableName = 'civicrm_group', $allGroups = NULL, $includedGroups = NULL ) { diff --git a/CRM/ACL/BAO/ACL.php b/CRM/ACL/BAO/ACL.php index 10ff5e51bda5..77311c8cf741 100644 --- a/CRM/ACL/BAO/ACL.php +++ b/CRM/ACL/BAO/ACL.php @@ -234,7 +234,7 @@ public static function whereClause($type, &$tables, &$whereTables, $contactID = FROM civicrm_acl_cache c, civicrm_acl a WHERE c.acl_id = a.id AND a.is_active = 1 - AND a.object_table = 'civicrm_saved_search' + AND a.object_table = 'civicrm_group' AND a.id IN ( $aclKeys ) AND a.deny = 0 ORDER BY a.object_id @@ -259,7 +259,7 @@ public static function whereClause($type, &$tables, &$whereTables, $contactID = FROM civicrm_acl_cache c, civicrm_acl a WHERE c.acl_id = a.id AND a.is_active = 1 - AND a.object_table = 'civicrm_saved_search' + AND a.object_table = 'civicrm_group' AND a.id IN ( $aclKeys ) AND a.deny = 1 AND a.object_id IN (%1) @@ -333,7 +333,7 @@ public static function whereClause($type, &$tables, &$whereTables, $contactID = public static function group( $type, $contactID = NULL, - $tableName = 'civicrm_saved_search', + $tableName = 'civicrm_group', $allGroups = NULL, $includedGroups = [] ) { diff --git a/CRM/ACL/Form/ACL.php b/CRM/ACL/Form/ACL.php index a86ebb1fcb86..7b80b81543a7 100644 --- a/CRM/ACL/Form/ACL.php +++ b/CRM/ACL/Form/ACL.php @@ -37,7 +37,7 @@ public function setDefaultValues() { if (isset($defaults['object_table'])) { switch ($defaults['object_table']) { - case 'civicrm_saved_search': + case 'civicrm_group': $defaults['group_id'] = $defaults['object_id']; $defaults['object_type'] = 1; $showHide->addShow("id-group-acl"); @@ -265,7 +265,7 @@ public function postProcess() { // Figure out which type of object we're permissioning on and set object_table and object_id. switch ($params['object_type']) { case 1: - $params['object_table'] = 'civicrm_saved_search'; + $params['object_table'] = 'civicrm_group'; $params['object_id'] = $params['group_id']; break; diff --git a/CRM/ACL/Page/ACL.php b/CRM/ACL/Page/ACL.php index 9aea58591908..78c7fe26e7df 100644 --- a/CRM/ACL/Page/ACL.php +++ b/CRM/ACL/Page/ACL.php @@ -142,7 +142,7 @@ public function browse() { } switch ($acl[$dao->id]['object_table']) { - case 'civicrm_saved_search': + case 'civicrm_group': $acl[$dao->id]['object'] = $group[$acl[$dao->id]['object_id']] ?? NULL; $acl[$dao->id]['object_name'] = ts('Group'); break; diff --git a/CRM/Bridge/OG/Drupal.php b/CRM/Bridge/OG/Drupal.php index 7a8b0be8b574..6933ac0fcc77 100644 --- a/CRM/Bridge/OG/Drupal.php +++ b/CRM/Bridge/OG/Drupal.php @@ -175,7 +175,7 @@ public static function updateCiviACLEntityRole(&$params, $op) { public static function updateCiviACL(&$params, $op) { $dao = new CRM_ACL_DAO_ACL(); - $dao->object_table = 'civicrm_saved_search'; + $dao->object_table = 'civicrm_group'; $dao->object_id = $params['civicrm_group_id']; if ($op == 'delete') { diff --git a/CRM/Contact/BAO/Group.php b/CRM/Contact/BAO/Group.php index 6563b61d57c2..f2011648444f 100644 --- a/CRM/Contact/BAO/Group.php +++ b/CRM/Contact/BAO/Group.php @@ -285,7 +285,7 @@ public static function checkPermission($id, $excludeHidden = FALSE) { $permissions = NULL; if (CRM_Core_Permission::check('edit all contacts') || CRM_ACL_API::groupPermission(CRM_ACL_API::EDIT, $id, NULL, - 'civicrm_saved_search', $allGroups + 'civicrm_group', $allGroups ) ) { $permissions[] = CRM_Core_Permission::EDIT; @@ -293,7 +293,7 @@ public static function checkPermission($id, $excludeHidden = FALSE) { if (CRM_Core_Permission::check('view all contacts') || CRM_ACL_API::groupPermission(CRM_ACL_API::VIEW, $id, NULL, - 'civicrm_saved_search', $allGroups + 'civicrm_group', $allGroups ) ) { $permissions[] = CRM_Core_Permission::VIEW; @@ -316,8 +316,7 @@ public function addSelectWhereClause() { $clauses = []; if (!CRM_Core_Permission::check([['edit all contacts', 'view all contacts']])) { $allGroups = CRM_Core_PseudoConstant::allGroup(NULL, FALSE); - // FIXME: TableName 'civicrm_saved_search' seems wrong but is consistent with self::checkPermission - $allowedGroups = \CRM_ACL_API::group(CRM_ACL_API::VIEW, NULL, 'civicrm_saved_search', $allGroups); + $allowedGroups = \CRM_ACL_API::group(CRM_ACL_API::VIEW, NULL, 'civicrm_group', $allGroups); $groupsIn = $allowedGroups ? implode(',', $allowedGroups) : '0'; $clauses['id'][] = "IN ($groupsIn)"; } diff --git a/CRM/Core/Permission/Base.php b/CRM/Core/Permission/Base.php index 9b1ae5709bbc..ac9fbea5499a 100644 --- a/CRM/Core/Permission/Base.php +++ b/CRM/Core/Permission/Base.php @@ -169,7 +169,7 @@ public function group($groupType = NULL, $excludeHidden = TRUE) { Civi::$statics[__CLASS__]['viewPermissionedGroups_' . $domainId . '_' . $userId][$groupKey] = $groups; } - $ids = CRM_ACL_API::group(CRM_Core_Permission::VIEW, NULL, 'civicrm_saved_search', $groups); + $ids = CRM_ACL_API::group(CRM_Core_Permission::VIEW, NULL, 'civicrm_group', $groups); if (!empty($ids)) { foreach (array_values($ids) as $id) { $title = CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Group', $id, 'title'); @@ -178,7 +178,7 @@ public function group($groupType = NULL, $excludeHidden = TRUE) { } } - $ids = CRM_ACL_API::group(CRM_Core_Permission::EDIT, NULL, 'civicrm_saved_search', $groups); + $ids = CRM_ACL_API::group(CRM_Core_Permission::EDIT, NULL, 'civicrm_group', $groups); if (!empty($ids)) { foreach (array_values($ids) as $id) { $title = CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Group', $id, 'title'); diff --git a/CRM/Upgrade/Incremental/sql/5.64.alpha1.mysql.tpl b/CRM/Upgrade/Incremental/sql/5.64.alpha1.mysql.tpl index f1f12629b23b..d44f0b8c4fcd 100644 --- a/CRM/Upgrade/Incremental/sql/5.64.alpha1.mysql.tpl +++ b/CRM/Upgrade/Incremental/sql/5.64.alpha1.mysql.tpl @@ -4,7 +4,10 @@ UPDATE `civicrm_acl` SET `priority` = `id`; -- Remove obsolete "Basic ACLs" DELETE FROM civicrm_acl -WHERE object_table NOT IN ('civicrm_saved_search', 'civicrm_uf_group', 'civicrm_custom_group', 'civicrm_event'); +WHERE object_table NOT IN ('civicrm_group', 'civicrm_saved_search', 'civicrm_uf_group', 'civicrm_custom_group', 'civicrm_event'); + +-- Fix wrong table name +UPDATE `civicrm_acl` SET `object_table` = 'civicrm_group' WHERE `object_table` = 'civicrm_saved_search'; -- fix mis-casing of field name. Note the php function doesn't permit the name change hence it is here -- but field is not localised. diff --git a/CRM/Utils/Hook.php b/CRM/Utils/Hook.php index 94039470135b..5e6005b77bdf 100644 --- a/CRM/Utils/Hook.php +++ b/CRM/Utils/Hook.php @@ -598,9 +598,7 @@ public static function aclWhereClause($type, &$tables, &$whereTables, &$contactI * @param int $contactID * User contactID for whom the check is made. * @param string $tableName - * Table name of group, e.g. `civicrm_uf_group` or `civicrm_custom_group`. - * Note: for some weird reason when this hook is called for contact groups, this - * value will be `civicrm_saved_search` instead of `civicrm_group` as you'd expect. + * Table name of group, e.g. 'civicrm_group' or 'civicrm_uf_group' or 'civicrm_custom_group'. * @param array $allGroups * All groups from the above table, keyed by id. * @param int[] $currentGroups @@ -611,6 +609,17 @@ public static function aclWhereClause($type, &$tables, &$whereTables, &$contactI */ public static function aclGroup($type, $contactID, $tableName, &$allGroups, &$currentGroups) { $null = NULL; + // Legacy support for hooks that still expect 'civicrm_group' to be 'civicrm_saved_search' + // This was changed in 5.64 + if ($tableName === 'civicrm_group') { + $initialValue = $currentGroups; + $legacyTableName = 'civicrm_saved_search'; + self::singleton() + ->invoke(['type', 'contactID', 'tableName', 'allGroups', 'currentGroups'], $type, $contactID, $legacyTableName, $allGroups, $currentGroups, $null, 'civicrm_aclGroup'); + if ($initialValue != $currentGroups) { + CRM_Core_Error::deprecatedWarning('Since 5.64 hook_civicrm_aclGroup passes "civicrm_group" instead of "civicrm_saved_search" for the $tableName when referring to Groups. Hook listeners should be updated.'); + } + } return self::singleton() ->invoke(['type', 'contactID', 'tableName', 'allGroups', 'currentGroups'], $type, $contactID, $tableName, $allGroups, $currentGroups, $null, 'civicrm_aclGroup'); } diff --git a/Civi/Test/ACLPermissionTrait.php b/Civi/Test/ACLPermissionTrait.php index 91b6681cf6d7..7ff1705e3107 100644 --- a/Civi/Test/ACLPermissionTrait.php +++ b/Civi/Test/ACLPermissionTrait.php @@ -125,7 +125,7 @@ public function aclWhereGreaterThan($type, &$tables, &$whereTables, &$contactID, * @throws CRM_Core_Exception */ public function setupCoreACLPermittedAcl($permissionedEntities = [], $groupAllowedAccess = 'Everyone', $operation = 'View', $entity = 'Group') { - $tableMap = ['Group' => 'civicrm_saved_search', 'CustomGroup' => 'civicrm_custom_group', 'Profile' => 'civicrm_uf_match', 'Event' => 'civicrm_event']; + $tableMap = ['Group' => 'civicrm_group', 'CustomGroup' => 'civicrm_custom_group', 'Profile' => 'civicrm_uf_group', 'Event' => 'civicrm_event']; $entityTable = $tableMap[$entity]; $permittedRoleID = ($groupAllowedAccess === 'Everyone') ? 0 : $groupAllowedAccess; diff --git a/tests/phpunit/CRM/Group/Page/AjaxTest.php b/tests/phpunit/CRM/Group/Page/AjaxTest.php index acb7ad4bc73a..6f52b0805cda 100644 --- a/tests/phpunit/CRM/Group/Page/AjaxTest.php +++ b/tests/phpunit/CRM/Group/Page/AjaxTest.php @@ -666,6 +666,9 @@ public function testGroupDontRegenerateSmartGroups() { * @param array $currentGroups */ public function hook_civicrm_aclGroup($type, $contactID, $tableName, &$allGroups, &$currentGroups) { + if ($tableName !== 'civicrm_group') { + return; + } //don't use api - you will get a loop $sql = " SELECT * FROM civicrm_group WHERE name LIKE '%pick%'"; $groups = []; @@ -734,7 +737,7 @@ public function setupEditAllGroupsACL() { `name`, `entity_table`, `entity_id`, `operation`, `object_table`, `object_id`, `is_active` ) VALUES ( - 'core-580', 'civicrm_acl_role', 55, 'Edit', 'civicrm_saved_search', 0, 1 + 'core-580', 'civicrm_acl_role', 55, 'Edit', 'civicrm_group', 0, 1 ); "); diff --git a/tests/phpunit/CRM/Mailing/BAO/MailingTest.php b/tests/phpunit/CRM/Mailing/BAO/MailingTest.php index 93d228953998..9af4831d169a 100644 --- a/tests/phpunit/CRM/Mailing/BAO/MailingTest.php +++ b/tests/phpunit/CRM/Mailing/BAO/MailingTest.php @@ -225,6 +225,9 @@ public function aclWhereAllowedOnlyOne($type, &$tables, &$whereTables, &$contact * @param array $currentGroups */ public function hook_civicrm_aclGroup($type, $contactID, $tableName, &$allGroups, &$currentGroups) { + if ($tableName !== 'civicrm_group') { + return; + } //don't use api - you will get a loop $sql = " SELECT * FROM civicrm_group"; $groups = []; diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php index df2a5f27742f..48eb3612f9d0 100644 --- a/tests/phpunit/CiviTest/CiviUnitTestCase.php +++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php @@ -2182,7 +2182,7 @@ public function setupACL($isProfile = FALSE) { `name`, `entity_table`, `entity_id`, `operation`, `object_table`, `object_id`, `is_active` ) VALUES ( - 'view picked', 'civicrm_group', $this->_permissionedGroup , 'Edit', 'civicrm_saved_search', {$this->_permissionedGroup}, 1 + 'view picked', 'civicrm_group', $this->_permissionedGroup , 'Edit', 'civicrm_group', {$this->_permissionedGroup}, 1 ); "); @@ -2191,7 +2191,7 @@ public function setupACL($isProfile = FALSE) { `name`, `entity_table`, `entity_id`, `operation`, `object_table`, `object_id`, `is_active` ) VALUES ( - 'view picked', 'civicrm_group', $this->_permissionedGroup, 'Edit', 'civicrm_saved_search', {$this->_permissionedDisabledGroup}, 1 + 'view picked', 'civicrm_group', $this->_permissionedGroup, 'Edit', 'civicrm_group', {$this->_permissionedDisabledGroup}, 1 ); "); } diff --git a/tests/phpunit/api/v3/GroupTest.php b/tests/phpunit/api/v3/GroupTest.php index 70b542dfd9ec..26f721a6c146 100644 --- a/tests/phpunit/api/v3/GroupTest.php +++ b/tests/phpunit/api/v3/GroupTest.php @@ -362,8 +362,10 @@ public function testGroupGetACLs($version) { * @param array $ids */ public function aclGroupAllGroups($type, $contactID, $tableName, $allGroups, &$ids) { - $group = $this->callAPISuccess('Group', 'get', ['name' => 'Test Group 1']); - $ids = array_keys($group['values']); + if ($tableName === 'civicrm_group') { + $group = $this->callAPISuccess('Group', 'get', ['name' => 'Test Group 1']); + $ids = array_keys($group['values']); + } } } diff --git a/tests/phpunit/api/v3/RelationshipTest.php b/tests/phpunit/api/v3/RelationshipTest.php index 91ace3a2e372..09b6d694a8a5 100644 --- a/tests/phpunit/api/v3/RelationshipTest.php +++ b/tests/phpunit/api/v3/RelationshipTest.php @@ -1477,8 +1477,7 @@ public function testCreateWithLesserPermissions() { * this from civicrm_generated.mysql */ private function setUpACLByCheating() { - CRM_Core_DAO::executeQuery("INSERT INTO civicrm_acl (name, deny, entity_table, entity_id, operation, object_table, object_id, acl_table, acl_id, is_active) VALUES ('Edit All Contacts', 0, 'civicrm_acl_role', 1, 'Edit', 'civicrm_saved_search', 0, NULL, NULL, 1)"); - CRM_Core_DAO::executeQuery("INSERT INTO civicrm_acl (name, deny, entity_table, entity_id, operation, object_table, object_id, acl_table, acl_id, is_active) VALUES ('Core ACL',0,'civicrm_acl_role',0,'All','access CiviMail subscribe/unsubscribe pages',NULL,NULL,NULL,1)"); + CRM_Core_DAO::executeQuery("INSERT INTO civicrm_acl (name, deny, entity_table, entity_id, operation, object_table, object_id, acl_table, acl_id, is_active) VALUES ('Edit All Contacts', 0, 'civicrm_acl_role', 1, 'Edit', 'civicrm_group', 0, NULL, NULL, 1)"); } } diff --git a/tests/phpunit/api/v3/ReportTemplateTest.php b/tests/phpunit/api/v3/ReportTemplateTest.php index 723dd7f7c313..f824fd346d32 100644 --- a/tests/phpunit/api/v3/ReportTemplateTest.php +++ b/tests/phpunit/api/v3/ReportTemplateTest.php @@ -1811,7 +1811,9 @@ private function doQuestionableStuffInASeparateFunctionSoNobodyNotices(): void { * @param array $ids */ public function aclGroupOnly($type, $contactID, $tableName, $allGroups, &$ids) { - $ids = [$this->aclGroupID]; + if ($tableName === 'civicrm_group') { + $ids = [$this->aclGroupID]; + } } /** diff --git a/xml/templates/civicrm_acl.tpl b/xml/templates/civicrm_acl.tpl index 0877ac3d4685..afa070c7a540 100644 --- a/xml/templates/civicrm_acl.tpl +++ b/xml/templates/civicrm_acl.tpl @@ -14,7 +14,7 @@ -- Create ACL to edit and view contacts in all groups INSERT INTO civicrm_acl (name, deny, entity_table, entity_id, operation, object_table, object_id, acl_table, acl_id, is_active, priority) VALUES -('Edit All Contacts', 0, 'civicrm_acl_role', 1, 'Edit', 'civicrm_saved_search', 0, NULL, NULL, 1, 1); +('Edit All Contacts', 0, 'civicrm_acl_role', 1, 'Edit', 'civicrm_group', 0, NULL, NULL, 1, 1); -- Create default Groups for User Permissioning INSERT INTO civicrm_group (`id`, `name`, `title`, `description`, `source`, `saved_search_id`, `is_active`, `visibility`, `group_type`)