Skip to content

Commit

Permalink
CRM-20226: Parent Group do not inherit child group contacts
Browse files Browse the repository at this point in the history
  • Loading branch information
@monishdeb
monishdeb committed May 28, 2017
1 parent 6df83fb commit 783144b
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 55 deletions.
55 changes: 29 additions & 26 deletions CRM/Contact/BAO/Group.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -910,7 +910,7 @@ public static function getGroupList(&$params) {
$links = self::actionLinks();

$allTypes = CRM_Core_OptionGroup::values('group_type');
$values = $groupsToCount = array();
$values = array();

$visibility = CRM_Core_SelectValues::ufVisibility();

Expand Down Expand Up @@ -964,8 +964,6 @@ public static function getGroupList(&$params) {

$values[$object->id]['visibility'] = $visibility[$values[$object->id]['visibility']];

$groupsToCount[$object->saved_search_id ? 'civicrm_group_contact_cache' : 'civicrm_group_contact'][] = $object->id;

if (isset($values[$object->id]['group_type'])) {
$groupTypes = explode(CRM_Core_DAO::VALUE_SEPARATOR,
substr($values[$object->id]['group_type'], 1, -1)
Expand Down Expand Up @@ -1017,20 +1015,9 @@ public static function getGroupList(&$params) {
$contactUrl = CRM_Utils_System::url('civicrm/contact/view', "reset=1&cid={$object->created_id}");
$values[$object->id]['created_by'] = "<a href='{$contactUrl}'>{$object->created_by}</a>";
}
}

// Get group counts - executes one query for regular groups and another for smart groups
foreach ($groupsToCount as $table => $groups) {
$where = "g.group_id IN (" . implode(',', $groups) . ")";
if ($table == 'civicrm_group_contact') {
$where .= " AND g.status = 'Added'";
}
// Exclude deleted contacts
$where .= " and c.id = g.contact_id AND c.is_deleted = 0";
$dao = CRM_Core_DAO::executeQuery("SELECT g.group_id, COUNT(*) as `count` FROM $table g, civicrm_contact c WHERE $where GROUP BY g.group_id");
while ($dao->fetch()) {
$values[$dao->group_id]['count'] = $dao->count;
}
// get group contact count using Contact.GetCount API
$values[$object->id]['count'] = civicrm_api3('Contact', 'getcount', array('group' => $object->id));
}

// CRM-16905 - Sort by count cannot be done with sql
Expand Down Expand Up @@ -1356,23 +1343,39 @@ protected function assignTestValue($fieldName, &$fieldDef, $counter) {
/**
* Get child group ids
*
* @param array $ids
* @param array $regularGroupIDs
* Parent Group IDs
*
* @return array
*/
public static function getChildGroupIds($ids) {
$notFound = FALSE;
$childGroupIds = array();
foreach ($ids as $id) {
$childId = CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Group', $id, 'children');
while (!empty($childId)) {
$childGroupIds[] = $childId;
$childId = CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Group', $childId, 'children');
public static function getChildGroupIds($regularGroupIDs) {
$childGroupIDs = array();

foreach ($regularGroupIDs as $regularGroupID) {
// temporary store the child group ID(s) of regular group identified by $id,
// later merge with main child group array
$tempChildGroupIDs = array();
// check that the regular group has any child group, if not then continue
if ($childrenFound = CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Group', $regularGroupID, 'children')) {
$tempChildGroupIDs[] = $childrenFound;
}
else {
continue;
}
// as civicrm_group.children stores multiple group IDs in comma imploded string format,
// so we need to convert it into array of child group IDs first
$tempChildGroupIDs = explode(',', implode(',', $tempChildGroupIDs));
$childGroupIDs = array_merge($childGroupIDs, $tempChildGroupIDs);
// recursively fetch the child group IDs
while (count($tempChildGroupIDs)) {
$tempChildGroupIDs = self::getChildGroupIds($tempChildGroupIDs);
if (count($tempChildGroupIDs)) {
$childGroupIDs = array_merge($childGroupIDs, $tempChildGroupIDs);
}
}
}

return $childGroupIds;
return $childGroupIDs;
}

}
44 changes: 28 additions & 16 deletions CRM/Contact/BAO/Query.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2979,31 +2979,42 @@ public function group($values) {
}
$groupClause = array();
if (count($regularGroupIDs) || empty($value)) {
// include child groups IDs if any
$childGroupIds = (array) CRM_Contact_BAO_Group::getChildGroupIds($regularGroupIDs);
foreach ($childGroupIds as $key => $id) {
if (CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Group', $id, 'saved_search_id')) {
$smartGroupIDs[] = $id;
unset($childGroupIds[$key]);
}
}
if (count($childGroupIds)) {
$regularGroupIDs = array_merge($regularGroupIDs, $childGroupIds);
}

// if $regularGroupIDs is populated with regular child group IDs
// then change the mysql operator to desired
if (count($regularGroupIDs) > 1) {
$op = strpos($op, 'IN') ? $op : ($op == '!=') ? 'NOT IN' : 'IN';
}
$groupIds = implode(',', (array) $regularGroupIDs);
$gcTable = "`civicrm_group_contact-{$groupIds}`";
$joinClause = array("contact_a.id = {$gcTable}.contact_id");
if ($statii) {
$joinClause[] = "{$gcTable}.status IN (" . implode(', ', $statii) . ")";
}
$this->_tables[$gcTable] = $this->_whereTables[$gcTable] = " LEFT JOIN civicrm_group_contact {$gcTable} ON (" . implode(' AND ', $joinClause) . ")";

if (strpos($op, 'IN') !== FALSE) {
$clause = "{$gcTable}.group_id $op ( $groupIds ) %s ";
$clause = "{$gcTable}.group_id $op ( $groupIds ) ";
}
elseif ($op == '!=') {
$clause = "{$gcTable}.contact_id NOT IN (SELECT contact_id FROM civicrm_group_contact cgc WHERE cgc.group_id = $groupIds %s)";
$clause = "{$gcTable}.contact_id NOT IN (SELECT contact_id FROM civicrm_group_contact cgc WHERE cgc.group_id = $groupIds )";
}
else {
$clause = "{$gcTable}.group_id $op $groupIds %s ";
$clause = "{$gcTable}.group_id $op $groupIds ";
}
$groupClause[] = "( {$clause} )";

// include child groups IDs if any
$childGroupIds = CRM_Contact_BAO_Group::getChildGroupIds($regularGroupIDs);
$childClause = '';
if (count($childGroupIds)) {
$gcTable = ($op == '!=') ? 'cgc' : $gcTable;
$childClause = " OR {$gcTable}.group_id IN (" . implode(',', $childGroupIds) . ") ";
if ($statii) {
$joinClause[] = "{$gcTable}.status IN (" . implode(', ', $statii) . ")";
}
$groupClause[] = '(' . sprintf($clause, $childClause) . ')';
$this->_tables[$gcTable] = $this->_whereTables[$gcTable] = " LEFT JOIN civicrm_group_contact {$gcTable} ON (" . implode(' AND ', $joinClause) . ")";
}

//CRM-19589: contact(s) removed from a Smart Group, resides in civicrm_group_contact table
Expand Down Expand Up @@ -4385,8 +4396,9 @@ public static function apiQuery(

$sql = "$select $from $where $having";

// add group by
if ($query->_useGroupBy) {
// add group by only when API action is not getcount
// otherwise query fetches incorrect count
if ($query->_useGroupBy && !$count) {
$sql .= self::getGroupByFromSelectColumns($query->_select, 'contact_a.id');
}
if (!empty($sort)) {
Expand Down
63 changes: 50 additions & 13 deletions tests/phpunit/CRM/Contact/BAO/GroupContactTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,23 +94,45 @@ public function testGetGroupId() {
*/
public function testContactSearchByParentGroup() {
// create a parent group
$groupParams1 = array(
$parentGroup = $this->callAPISuccess('Group', 'create', array(
'title' => 'Parent Group',
'description' => 'Parent Group',
'visibility' => 'User and User Admin Only',
'is_active' => 1,
);
$parentGroup = $this->callAPISuccess('Group', 'create', $groupParams1);
));

// create a child group
$groupParams2 = array(
$childGroup = $this->callAPISuccess('Group', 'create', array(
'title' => 'Child Group',
'description' => 'Child Group',
'visibility' => 'User and User Admin Only',
'parents' => $parentGroup['id'],
'is_active' => 1,
);
$childGroup = $this->callAPISuccess('Group', 'create', $groupParams2);
));

// create smart group based on saved criteria Gender = Male
$batch = $this->callAPISuccess('SavedSearch', 'create', array(
'form_values' => 'a:1:{i:0;a:5:{i:0;s:9:"gender_id";i:1;s:1:"=";i:2;i:2;i:3;i:0;i:4;i:0;}}',
));
// Create contact with Gender - Male
$childSmartGroupContact = $this->individualCreate(array(
'gender_id' => "Male",
'first_name' => 'C',
), 1);
// then create smart group
$childSmartGroup = $this->callAPISuccess('Group', 'create', array(
'title' => 'Child Smart Group',
'description' => 'Child Smart Group',
'visibility' => 'User and User Admin Only',
'saved_search_id' => $batch['id'],
'is_active' => 1,
'parents' => $parentGroup['id'],
));

$this->callAPISuccess('Group', 'create', array(
'id' => $parentGroup['id'],
'children' => implode(',', array($childGroup['id'], $childSmartGroup['id'])),
));

// Create a contact within parent group
$parentContactParams = array(
Expand All @@ -129,30 +151,45 @@ public function testContactSearchByParentGroup() {
$childContact = $this->individualCreate($childContactParams);

// Check if searching by parent group returns both parent and child group contacts
$searchParams = array(
$result = $this->callAPISuccess('contact', 'get', array(
'group' => $parentGroup['id'],
);
$result = $this->callAPISuccess('contact', 'get', $searchParams);
));
$validContactIds = array($parentContact, $childContact);
$resultContactIds = array();
foreach ($result['values'] as $k => $v) {
$resultContactIds[] = $v['contact_id'];
}
$this->assertEquals(2, count($resultContactIds), 'Check the count of returned values');
$this->assertEquals(3, count($resultContactIds), 'Check the count of returned values');
$this->assertEquals(array(), array_diff($validContactIds, $resultContactIds), 'Check that the difference between two arrays should be blank array');

// Check if searching by child group returns just child group contacts
$searchParams = array(
$result = $this->callAPISuccess('contact', 'get', array(
'group' => $childGroup['id'],
);
$result = $this->callAPISuccess('contact', 'get', $searchParams);
));
$validChildContactIds = array($childContact);
$resultChildContactIds = array();
foreach ($result['values'] as $k => $v) {
$resultChildContactIds[] = $v['contact_id'];
}
$this->assertEquals(1, count($resultChildContactIds), 'Check the count of returned values');
$this->assertEquals(array(), array_diff($validChildContactIds, $resultChildContactIds), 'Check that the difference between two arrays should be blank array');

// Check if searching by smart child group returns just smart child group contacts
$result = $this->callAPISuccess('contact', 'get', array(
'group' => $childSmartGroup['id'],
));
$validChildContactIds = array($childSmartGroupContact);
$resultChildContactIds = array();
foreach ($result['values'] as $k => $v) {
$resultChildContactIds[] = $v['contact_id'];
}
$this->assertEquals(1, count($resultChildContactIds), 'Check the count of returned values');
$this->assertEquals(array(), array_diff($validChildContactIds, $resultChildContactIds), 'Check that the difference between two arrays should be blank array');

//cleanup
$this->callAPISuccess('Contact', 'delete', array('id' => $parentContact));
$this->callAPISuccess('Contact', 'delete', array('id' => $childContact));
$this->callAPISuccess('Contact', 'delete', array('id' => $childSmartGroupContact));
}


Expand Down

0 comments on commit 783144b

Please sign in to comment.