dev/core#926 Fixes bug on searching for removed members of smartgroups

civicrm · May 2, 2019 · 1224cb6 · 1224cb6
1 parent d7570a7
commit 1224cb6
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 15 deletions.
diff --git 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,34 +6969,42 @@ 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) . ")";
   }
 
   /**
    * 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
@@ -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
@@ -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;