CRM-17123 remove damaging OR from smart group query

Note that I added a group refresh to pick up the 'hard-adds'. I think the correct place to deal with this is in fact to alter the GroupContact Add functionality - ie.

IF (CRM_Core_DAO::executeQuery('SELECT id FROM civicrm_group_contact_cache WHERE group_id = 1 LIMIT 1)) {
  // Add the contact just added to the group to the group_contact_cache table.
}

I started that a bit but I think it should be dealt with separately after this is resolved

Author: eileenmcnaughton

CRM/Contact/BAO/Group.php

@@ -460,7 +460,6 @@ public static function create(&$params) {
     }
 
     if (!empty($params['organization_id'])) {
-      $groupOrg = array();
       $groupOrg = $params;
       $groupOrg['group_id'] = $group->id;
       CRM_Contact_BAO_GroupOrganization::add($groupOrg);

CRM/Contact/BAO/Query.php

@@ -586,7 +586,10 @@ public function buildParamsLookup() {
       if (!array_key_exists($value[0], $this->_paramLookup)) {
         $this->_paramLookup[$value[0]] = array();
       }
-      $this->_paramLookup[$value[0]][] = $value;
+      if ($value[0] !== 'group') {
+        // Just trying to unravel how group interacts here! This whole function is wieid.
+        $this->_paramLookup[$value[0]][] = $value;
+      }
     }
   }
 
@@ -2911,7 +2914,6 @@ public function group($values) {
 
     $groupIds = NULL;
 
-    $isSmart = FALSE;
     $isNotOp = ($op == 'NOT IN' || $op == '!=');
 
     $statii = array();
@@ -2929,7 +2931,8 @@ public function group($values) {
       $statii[] = '"Added"';
     }
 
-    $skipGroup = FALSE;
+    $ssClause = $this->addGroupContactCache($value, NULL, "contact_a", $op);
+    $isSmart = (!$ssClause) ? FALSE : TRUE;
     if (!is_array($value) &&
       count($statii) == 1 &&
       $statii[0] == '"Added"' &&
@@ -2939,9 +2942,6 @@ public function group($values) {
         $isSmart = TRUE;
       }
     }
-
-    $ssClause = $this->addGroupContactCache($value, NULL, "contact_a", $op);
-    $isSmart = (!$ssClause) ? FALSE : $isSmart;
     $groupClause = NULL;
 
     if (!$isSmart) {
@@ -3038,6 +3038,9 @@ public function addGroupContactCache($groups, $tableAlias = NULL, $joinTable = "
         CRM_Contact_BAO_GroupContactCache::load($group);
       }
     }
+    if ($group->N == 0) {
+      return NULL;
+    }
 
     if (!$tableAlias) {
       $tableAlias = "`civicrm_group_contact_cache_";

tests/phpunit/CRM/Contact/BAO/GroupContactTest.php

@@ -110,7 +110,7 @@ public function testContactSearchByParentGroup() {
       'title' => 'Child Group',
       'description' => 'Child Group',
       'visibility' => 'User and User Admin Only',
-      'parents' => $parentGroup->id,
+      'parents' => $parentGroup['id'],
       'is_active' => 1,
     );
     $childGroup = $this->callAPISuccess('Group', 'create', $groupParams2);
@@ -119,21 +119,21 @@ public function testContactSearchByParentGroup() {
     $parentContactParams = array(
       'first_name' => 'Parent1 Fname',
       'last_name' => 'Parent1 Lname',
-      'group' => array($parentGroup->id => 1),
+      'group' => array($parentGroup['id'] => 1),
     );
     $parentContact = Contact::createIndividual($parentContactParams);
 
     // create a contact within child dgroup
     $childContactParams = array(
       'first_name' => 'Child1 Fname',
       'last_name' => 'Child2 Lname',
-      'group' => array($childGroup->id => 1),
+      'group' => array($childGroup['id'] => 1),
     );
     $childContact = Contact::createIndividual($childContactParams);
 
     // Check if searching by parent group  returns both parent and child group contacts
     $searchParams = array(
-      'group' => $parentGroup->id,
+      'group' => $parentGroup['id'],
     );
     $result = $this->callAPISuccess('contact', 'get', $searchParams);
     $validContactIds = array($parentContact, $childContact);
@@ -146,7 +146,7 @@ public function testContactSearchByParentGroup() {
 
     // Check if searching by child group returns just child group contacts
     $searchParams = array(
-      'group' => $childGroup->id,
+      'group' => $childGroup['id'],
     );
     $result = $this->callAPISuccess('contact', 'get', $searchParams);
     $validChildContactIds = array($childContact);

tests/phpunit/CRM/Contact/BAO/QueryTest.php

@@ -290,6 +290,8 @@ public function testGroupClause() {
     $this->callAPISuccess('GroupContact', 'create', array('group_id' => $groupID, 'contact_id' => $individualID, 'status' => 'Added'));
     $this->callAPISuccess('GroupContact', 'create', array('group_id' => $groupID, 'contact_id' => $householdID, 'status' => 'Added'));
 
+    // Refresh the cache for test purposes. It would be better to alter to alter the GroupContact add function to add contacts to the cache.
+    CRM_Contact_BAO_GroupContactCache::remove($groupID, FALSE);
     $query = new CRM_Contact_BAO_Query(
       array(array('group', 'IN', array($groupID), 0, 0)),
       array('contact_id')
@@ -299,6 +301,7 @@ public function testGroupClause() {
     $queryString = implode(' ', $sql);
     $dao = CRM_Core_DAO::executeQuery($queryString);
     $this->assertEquals(3, $dao->N);
+    $this->assertFalse(strstr(implode(' ', $sql), ' OR '));
   }
 
 }

tests/phpunit/CRM/Event/BAO/ParticipantTest.php

@@ -177,18 +177,18 @@ public function testimportableFields() {
    * ParticipantDetails() method ( Checking the Participant Details )
    */
   public function testparticipantDetails() {
-    $participantId = Participant::create($this->_contactId, $this->_eventId);
-    $params = array('name' => 'Doe, John', 'title' => 'Test Event');
+    $participantId = $this->callAPISuccess('Participant', 'create', array('contact_id' => $this->_contactId, 'event_id' => $this->_eventId));
+    $params = array('name' => 'Doe, John', 'title' => 'Annual CiviCRM meet');
 
     $participantDetails = CRM_Event_BAO_Participant::participantDetails($participantId);
 
     $this->assertEquals(count($participantDetails), 3, 'Equating the array contains.');
     $this->assertEquals($participantDetails['name'], $params['name'], 'Checking Name of Participant.');
     $this->assertEquals($participantDetails['title'], $params['title'], 'Checking Event Title in which participant is enroled.');
 
-    Participant::delete($participantId);
+    $this->callAPISuccess('Participant', 'delete', array('id' => $participantId));
     $this->contactDelete($this->_contactId);
-    Event::delete($this->_eventId);
+    $this->callAPISuccess('Event', 'delete', array('id' => $this->_eventId));
   }
 
   /**
@@ -222,7 +222,7 @@ public function testdeleteParticipant() {
     $this->assertDBNull('CRM_Event_BAO_Participant', $participant->id, 'contact_id', 'id', 'Check DB for deleted Participant.');
 
     $this->contactDelete($this->_contactId);
-    Event::delete($this->_eventId);
+    $this->callAPISuccess('Event', 'delete', array('id' => $this->_eventId));
   }
 
   /**