Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CRM-20226: Parent Group do not inherit child group contacts #10428

Merged
merged 1 commit into from
Jun 6, 2017
Merged

CRM-20226: Parent Group do not inherit child group contacts #10428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 29 additions & 26 deletions CRM/Contact/BAO/Group.php
View file
Open in desktop
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));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@monishdeb are you sure this doesn't hit the limit = 25 bug in the api3?

Copy link
Member Author

@monishdeb monishdeb Jun 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@colemanw yes because on 'Manage Group', for some group, I am getting group-contact's count > 25, reason why I haven't added limit = 0 option.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found that for 'getcount' API action, the limit is not used here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks for checking.

}

// 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
Open in desktop
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
Open in desktop
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