diff --git a/CRM/Contact/BAO/Query.php b/CRM/Contact/BAO/Query.php index b7f353ad7fdb..d02aad2b6558 100644 --- a/CRM/Contact/BAO/Query.php +++ b/CRM/Contact/BAO/Query.php @@ -3006,7 +3006,7 @@ public function group($values) { $isNotOp = ($op == 'NOT IN' || $op == '!='); - $statusJoinClause = $this->getGroupStatusClause($grouping); + $statusJoinClause = $this->getGroupStatusClause($grouping, $hasNonSmartGroups); $groupClause = []; if ($hasNonSmartGroups || empty($value)) { // include child groups IDs if any @@ -3069,6 +3069,9 @@ public function group($values) { $groupIds = implode(',', (array) $smartGroupIDs); $gcTable = "civicrm_group_contact_{$this->_groupUniqueKey}"; $joinClause = ["contact_a.id = {$gcTable}.contact_id"]; + if ($statusJoinClause) { + $joinClause[] = "{$gcTable}.$statusJoinClause"; + } $this->_tables[$gcTable] = $this->_whereTables[$gcTable] = " LEFT JOIN civicrm_group_contact {$gcTable} ON (" . implode(' AND ', $joinClause) . ")"; if (strpos($op, 'IN') !== FALSE) { $groupClause[] = "{$gcTable}.group_id $op ( $groupIds ) AND {$gccTableAlias}.group_id IS NULL"; @@ -3089,7 +3092,7 @@ public function group($values) { list($qillop, $qillVal) = CRM_Contact_BAO_Query::buildQillForFieldValue('CRM_Contact_DAO_Group', 'id', $value, $op); $this->_qill[$grouping][] = ts("Group(s) %1 %2", [1 => $qillop, 2 => $qillVal]); if (strpos($op, 'NULL') === FALSE) { - $this->_qill[$grouping][] = ts("Group Status %1", [1 => implode(' ' . ts('or') . ' ', $this->getSelectedGroupStatuses($grouping))]); + $this->_qill[$grouping][] = ts("Group Status %1", [1 => implode(' ' . ts('or') . ' ', $this->getSelectedGroupStatuses($grouping, $hasNonSmartGroups))]); } } @@ -6966,11 +6969,16 @@ protected function addAddressTable($tableKey, $joinCondition) { * Get the clause for group status. * * @param int $grouping + * @param int $hasNonSmartGroups + * Are there any non-smart groups in the listing. Since we only support + * one status clause for form-oriented/ limited signature/legacy reasons this + * basically means 'should we add 'status = 'Added'' to the filter as that + * is otherwise handled for smart groups. * * @return string */ - protected function getGroupStatusClause($grouping) { - $statuses = $this->getSelectedGroupStatuses($grouping); + protected function getGroupStatusClause($grouping, $hasNonSmartGroups) { + $statuses = $this->getSelectedGroupStatuses($grouping, $hasNonSmartGroups); return "status IN (" . implode(', ', $statuses) . ")"; } @@ -6978,22 +6986,25 @@ protected function getGroupStatusClause($grouping) { * Get an array of the statuses that have been selected. * * @param string $grouping + * @param int $hasNonSmartGroups + * Are there any non-smart groups in the listing. Since we only support + * one status clause for form-oriented/ limited signature/legacy reasons this + * basically means 'should we add 'status = 'Added'' to the filter as that + * is otherwise handled for smart groups. * * @return array */ - protected function getSelectedGroupStatuses($grouping) { + protected function getSelectedGroupStatuses($grouping, $hasNonSmartGroups) { $statuses = []; $gcsValues = $this->getWhereValues('group_contact_status', $grouping); - if ($gcsValues && - is_array($gcsValues[2]) - ) { + if ($gcsValues && is_array($gcsValues[2])) { foreach ($gcsValues[2] as $k => $v) { - if ($v) { + if ($v && ($hasNonSmartGroups || $v !== 'Added')) { $statuses[] = "'" . CRM_Utils_Type::escape($k, 'String') . "'"; } } } - else { + elseif ($hasNonSmartGroups) { $statuses[] = "'Added'"; } return $statuses; diff --git a/tests/phpunit/CRM/Contact/BAO/QueryTest.php b/tests/phpunit/CRM/Contact/BAO/QueryTest.php index d3d42b94843a..f324b164b84a 100644 --- a/tests/phpunit/CRM/Contact/BAO/QueryTest.php +++ b/tests/phpunit/CRM/Contact/BAO/QueryTest.php @@ -645,6 +645,29 @@ public function testGetByGroupWithStatus() { $this->assertEquals(2, $resultDAO->N); } + /** + * Test we can narrow a group get by status. + */ + public function testGetByGroupWithStatusSmartGroup() { + $groupID = $this->smartGroupCreate(); + // This means they are actually all hard-added, which is fine for this purpose. + $this->groupContactCreate($groupID, 3); + $groupContactID = $this->callAPISuccessGetSingle('GroupContact', ['group_id' => $groupID, 'options' => ['limit' => 1]])['id']; + $this->callAPISuccess('GroupContact', 'create', ['id' => $groupContactID, 'status' => 'Removed']); + + $queryObj = new CRM_Contact_BAO_Query([['group', '=', $groupID, 0, 0], ['group_contact_status', 'IN', ['Removed' => 1], 0, 0]]); + $resultDAO = $queryObj->searchQuery(); + $this->assertEquals(1, $resultDAO->N); + + $queryObj = new CRM_Contact_BAO_Query([['group', '=', $groupID, 0, 0], ['group_contact_status', 'IN', ['Added' => 1], 0, 0]]); + $resultDAO = $queryObj->searchQuery(); + $this->assertEquals(2, $resultDAO->N); + + $queryObj = new CRM_Contact_BAO_Query([['group', '=', $groupID, 0, 0]]); + $resultDAO = $queryObj->searchQuery(); + $this->assertEquals(2, $resultDAO->N); + } + /** * Test the group contact clause does not contain an OR. * diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php index db21ca346ab6..5a396e1e5f2f 100644 --- a/tests/phpunit/CiviTest/CiviUnitTestCase.php +++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php @@ -1177,13 +1177,15 @@ protected function cleanUpAfterACLs() { * * @param array $smartGroupParams * @param array $groupParams + * @param string $contactType + * * @return int */ - public function smartGroupCreate($smartGroupParams = array(), $groupParams = array()) { - $smartGroupParams = array_merge(array( - 'formValues' => array('contact_type' => array('IN' => array('Household'))), - ), - $smartGroupParams); + public function smartGroupCreate($smartGroupParams = [], $groupParams = [], $contactType = 'Household') { + $smartGroupParams = array_merge([ + 'formValues' => ['contact_type' => ['IN' => [$contactType]]], + ], + $smartGroupParams); $savedSearch = CRM_Contact_BAO_SavedSearch::create($smartGroupParams); $groupParams['saved_search_id'] = $savedSearch->id;