Merge pull request #10451 from totten/master-null-caseid

CRM-20642 - Activity.get - Fix "IS NULL" filtering for case_id, tag_id, file_id
civicrm · Jun 2, 2017 · 503a995 · 503a995
2 parents a47577b + 749522a
commit 503a995
Show file tree

Hide file tree

Showing 2 changed files with 123 additions and 25 deletions.
diff --git a/api/v3/Activity.php b/api/v3/Activity.php
@@ -327,31 +327,49 @@ function civicrm_api3_activity_get($params) {
       );
     }
   }
-  if (!empty($params['tag_id'])) {
-    if (!is_array($params['tag_id'])) {
-      $params['tag_id'] = array('=' => $params['tag_id']);
-    }
-    $clause = \CRM_Core_DAO::createSQLFilter('tag_id', $params['tag_id']);
-    if ($clause) {
-      $sql->where('a.id IN (SELECT entity_id FROM civicrm_entity_tag WHERE entity_table = "civicrm_activity" AND !clause)', array('!clause' => $clause));
-    }
-  }
-  if (!empty($params['file_id'])) {
-    if (!is_array($params['file_id'])) {
-      $params['file_id'] = array('=' => $params['file_id']);
-    }
-    $clause = \CRM_Core_DAO::createSQLFilter('file_id', $params['file_id']);
-    if ($clause) {
-      $sql->where('a.id IN (SELECT entity_id FROM civicrm_entity_file WHERE entity_table = "civicrm_activity" AND !clause)', array('!clause' => $clause));
-    }
-  }
-  if (!empty($params['case_id'])) {
-    if (!is_array($params['case_id'])) {
-      $params['case_id'] = array('=' => $params['case_id']);
-    }
-    $clause = \CRM_Core_DAO::createSQLFilter('case_id', $params['case_id']);
-    if ($clause) {
-      $sql->where('a.id IN (SELECT activity_id FROM civicrm_case_activity WHERE !clause)', array('!clause' => $clause));
+
+  // Define how to handle filters on some related entities.
+  // Subqueries are nice in (a) avoiding duplicates and (b) when the result
+  // list is expected to be bite-sized. Joins are nice (a) with larger
+  // datasets and (b) checking for non-existent relations.
+  $rels = array(
+    'tag_id' => array(
+      'subquery' => 'a.id IN (SELECT entity_id FROM civicrm_entity_tag WHERE entity_table = "civicrm_activity" AND !clause)',
+      'join' => '!joinType civicrm_entity_tag !alias ON (!alias.entity_table = "civicrm_activity" AND !alias.entity_id = a.id)',
+      'column' => 'tag_id',
+    ),
+    'file_id' => array(
+      'subquery' => 'a.id IN (SELECT entity_id FROM civicrm_entity_file WHERE entity_table = "civicrm_activity" AND !clause)',
+      'join' => '!joinType civicrm_entity_file !alias ON (!alias.entity_table = "civicrm_activity" AND !alias.entity_id = a.id)',
+      'column' => 'file_id',
+    ),
+    'case_id' => array(
+      'subquery' => 'a.id IN (SELECT activity_id FROM civicrm_case_activity WHERE !clause)',
+      'join' => '!joinType civicrm_case_activity !alias ON (!alias.activity_id = a.id)',
+      'column' => 'case_id',
+    ),
+  );
+  foreach ($rels as $filter => $relSpec) {
+    if (!empty($params[$filter])) {
+      if (!is_array($params[$filter])) {
+        $params[$filter] = array('=' => $params[$filter]);
+      }
+      // $mode is one of ('LEFT JOIN', 'INNER JOIN', 'SUBQUERY')
+      $mode = isset($params[$filter]['IS NULL']) ? 'LEFT JOIN' : 'SUBQUERY';
+      if ($mode === 'SUBQUERY') {
+        $clause = \CRM_Core_DAO::createSQLFilter($relSpec['column'], $params[$filter]);
+        if ($clause) {
+          $sql->where($relSpec['subquery'], array('!clause' => $clause));
+        }
+      }
+      else {
+        $alias = 'actjoin_' . $filter;
+        $clause = \CRM_Core_DAO::createSQLFilter($alias . "." . $relSpec['column'], $params[$filter]);
+        if ($clause) {
+          $sql->join($alias, $relSpec['join'], array('!alias' => $alias, 'joinType' => $mode));
+          $sql->where($clause);
+        }
+      }
     }
   }
   $activities = _civicrm_api3_basic_get(_civicrm_api3_get_BAO(__FUNCTION__), $params, FALSE, 'Activity', $sql);

diff --git a/tests/phpunit/api/v3/ActivityCaseTest.php b/tests/phpunit/api/v3/ActivityCaseTest.php
@@ -0,0 +1,80 @@
+<?php
+
+/**
+ *  Test Activity.get API with the case_id field
+ *
+ * @package CiviCRM_APIv3
+ * @group headless
+ */
+class api_v3_ActivityCaseTest extends CiviCaseTestCase {
+  protected $_params;
+  protected $_entity;
+  protected $_cid;
+
+  /**
+   * @var array
+   *  APIv3 Result (Case.create)
+   */
+  protected $_case;
+
+  /**
+   * @var array
+   *  APIv3 Result (Activity.create)
+   */
+  protected $_otherActivity;
+
+  /**
+   * Test setup for every test.
+   *
+   * Connect to the database, truncate the tables that will be used
+   * and redirect stdin to a temporary file.
+   */
+  public function setUp() {
+    $this->_entity = 'case';
+
+    parent::setUp();
+
+    $this->_cid = $this->individualCreate();
+
+    $this->_case = $this->callAPISuccess('case', 'create', array(
+      'case_type_id' => $this->caseTypeId,
+      'subject' => __CLASS__,
+      'contact_id' => $this->_cid,
+    ));
+
+    $this->_otherActivity = $this->callAPISuccess('Activity', 'create', array(
+      'source_contact_id' => $this->_cid,
+      'activity_type_id' => 'Phone Call',
+      'subject' => 'Ask not what your API can do for you, but what you can do for your API.',
+    ));
+  }
+
+  public function testGet() {
+    $this->assertTrue(is_numeric($this->_case['id']));
+    $this->assertTrue(is_numeric($this->_otherActivity['id']));
+
+    $getByCaseId = $this->callAPIAndDocument('Activity', 'get', array(
+      'case_id' => $this->_case['id'],
+    ), __FUNCTION__, __FILE__);
+    $this->assertNotEmpty($getByCaseId['values']);
+    $getByCaseId_ids = array_keys($getByCaseId['values']);
+
+    $getByCaseNotNull = $this->callAPIAndDocument('Activity', 'get', array(
+      'case_id' => array('IS NOT NULL' => 1),
+    ), __FUNCTION__, __FILE__);
+    $this->assertNotEmpty($getByCaseNotNull['values']);
+    $getByCaseNotNull_ids = array_keys($getByCaseNotNull['values']);
+
+    $getByCaseNull = $this->callAPIAndDocument('Activity', 'get', array(
+      'case_id' => array('IS NULL' => 1),
+    ), __FUNCTION__, __FILE__);
+    $this->assertNotEmpty($getByCaseNull['values']);
+    $getByCaseNull_ids = array_keys($getByCaseNull['values']);
+
+    $this->assertTrue(in_array($this->_otherActivity['id'], $getByCaseNull_ids));
+    $this->assertNotTrue(in_array($this->_otherActivity['id'], $getByCaseId_ids));
+    $this->assertEquals($getByCaseId_ids, $getByCaseNotNull_ids);
+    $this->assertEquals(array(), array_intersect($getByCaseId_ids, $getByCaseNull_ids));
+  }
+
+}