CiviCRM Community Forums (archive)

I'd like to propose the following patch to the multisite module.

What it does is instead of finding ALL the child groups of the multisite group & generating permissions based on that it ONLY finds the child groups of type ACL. I think this makes sense - to only look at ACL groups when implementing ACLs. There is an edge case where the domain group has a child group which is not an ACL which has a child group which is an ACL which will no longer work as a multisite ACL- but I think that's OK. The main situation where I feel a hierarchical ACL is relevant is when someone is in a local chapter group which is a child of a state group - so someone in the state (level 2 group) should be able to find the chapter people even though they aren't directly in the state group.

(I have tried to keep the behaviour the same for mailing groups at this stage)

This patch is actually eliminating 300 queries for us (125 child groups of which NONE are ACL groups are having their name resolved 3 times each per search) - see end for backtrace on that


Index: tools/drupal/modules/multisite/multisite.module
===================================================================
--- tools/drupal/modules/multisite/multisite.module   (revision 528)
+++ tools/drupal/modules/multisite/multisite.module   (working copy)
@@ -105,7 +105,7 @@
   }
   require_once 'CRM/Core/BAO/Domain.php';
   $groupID = CRM_Core_BAO_Domain::getGroupId();
-  $currentGroups = _multisite_get_all_child_groups($groupID, FALSE);
+  $currentGroups = _multisite_get_all_child_groups($groupID,$type, FALSE);
 }
 
 function multisite_civicrm_aclWhereClause($type, &$tables, &$whereTables, &$contactID, &$where) {
@@ -114,7 +114,7 @@
   }
   require_once 'CRM/Core/BAO/Domain.php';
   $groupID = CRM_Core_BAO_Domain::getGroupId();
-  $childOrgs = _multisite_get_all_child_groups($groupID);
+  $childOrgs = _multisite_get_all_child_groups($groupID, $type);
 
   if (!empty($childOrgs)) {
     $groupTable = 'civicrm_group_contact';
@@ -233,10 +233,32 @@
 
   return $_cache[$orgID];
 }
-
-function _multisite_get_all_child_groups($groupID, $includeParent = TRUE) {
+     /*
+     *  Return all child groups of type specified - note that we are assuming here
+     * that a hierarchical chain of ACL groups 'breaks' if there is a non acl_group along the way.
+     * (Mailing or other groups ignore the concept of type since this we can't assume a mailing
+     * group is always child of a mailing group & it seems less efficient to filter on type as it requires more
+     * queries)
+     * This seems to make sense as
+     *
+     * 1) people are added to the domain group when they are entered on that domain/ url
+     * 2) the hierarchical model allows groups to go several levels deep -i.e member of local
+     * group is member of chapter is member of state group. So, resolving several levels is good
+     * but it can get out of hand. Generally the cascading effect should be restricted
+     * to deliberate design choice. Any group created in a site as an ACL group will
+     * automatically have that group as parent so it should be invisible to users
+     * 3) original model was to resolve ALL children but mailing groups & smart groups seem to
+     * 'mushroom' in a live environment - causing huge queries - e.g we are seeing 125 children of a state group
+     *
+     * @param integer $groupID parent group to be resolved
+     * @param bool $includeParent - should parent be in array of groups
+     * @return array $childGroups array of child groups
+     */
+function _multisite_get_all_child_groups($groupID, $type = 0, $includeParent = TRUE) {
   static $_cache = array();
-
+  if($type == 2){
+     $type = "AND group_type LIKE '%2%' ";
+  }
   if (!array_key_exists($groupID, $_cache)) {
     require_once 'CRM/Core/BAO/Cache.php';
     $childGroups = &CRM_Core_BAO_Cache::getItem('descendant groups for an org', $groupID);
@@ -245,10 +267,12 @@
       $childGroups = array();
 
       $query = "
-SELECT children
+SELECT group_concat(civicrm_group.id) as children
 FROM   civicrm_group
-WHERE  children IS NOT NULL
-AND    id IN ";
+INNER JOIN civicrm_group_nesting
+ON civicrm_group_nesting.child_group_id = civicrm_group.id
+$type
+WHERE parent_group_id  IN ";
 
       if (!is_array($groupID)) {
         $groupIDs = array($groupID);
@@ -257,7 +281,7 @@
       while (!empty($groupIDs)) {
         $groupIDString = implode(',', $groupIDs);
 
-        $realQuery = $query . " ( $groupIDString )";
+        $realQuery = $query . " ( $groupIDString )  GROUP BY parent_group_id";
         $dao       = CRM_Core_DAO::executeQuery($realQuery);
         $groupIDs  = array();
         while ($dao->fetch()) {





Addendum - this is the query that was being done 3 times per group - mentioned about - it is a pseudoconstant but there is no caching on it

May 10 12:59:37  [info] $SELECT  id, title 
 FROM civicrm_group
 
 WHERE (  civicrm_group.id = 119 ) 
 
  /var/htdocs/sites/all/modules/civicrm/packages/DB/DataObject.php, backtrace, 2358
/var/htdocs/sites/all/modules/civicrm/packages/DB/DataObject.php, _query, 442
/var/htdocs/sites/all/modules/civicrm/CRM/Core/DAO.php, find, 734
/var/htdocs/sites/all/modules/civicrm/CRM/Core/Permission/Drupal.php, getFieldValue, 108
/var/htdocs/sites/all/modules/civicrm/CRM/Core/Permission.php(123) : eval()'d code, group, 1
/var/htdocs/sites/all/modules/civicrm/CRM/Core/Permission.php, eval, 123
/var/htdocs/sites/all/modules/civicrm/CRM/Core/PseudoConstant.php, group, 1026
/var/htdocs/sites/all/modules/civicrm/CRM/Contact/Form/Search.php, group, 524
/var/htdocs/sites/all/modules/civicrm/CRM/Contact/Form/Search/Advanced.php, preProcess, 61
/var/htdocs/sites/all/modules/civicrm/CRM/Core/Form.php, preProcess, 325
/var/htdocs/sites/all/modules/civicrm/CRM/Core/QuickForm/Action/Refresh.php, buildForm, 65
/var/htdocs/sites/all/modules/civicrm/packages/HTML/QuickForm/Controller.php, perform, 203
/var/htdocs/sites/all/modules/civicrm/packages/HTML/QuickForm/Page.php, handle, 103
/var/htdocs/sites/all/modules/civicrm/CRM/Core/Controller.php, handle, 284
/var/htdocs/sites/all/modules/civicrm/CRM/Core/Invoke.php, run, 223
/var/htdocs/sites/all/modules/civicrm/drupal/civicrm.module, invoke, 361

Our testing on this patch seems to show that in some circumstances if a user doesn't have permission to a group & saves a contact then the contact gets removed from the group. This doesn't seem to be caused by the patch but it does seem to have made it visible - am doing more testing