Merge pull request #18566 from eileenmcnaughton/act_contact

#2057 ActivityContact.create Remove select query that never or almost never finds something.

CRM/Activity/BAO/ActivityContact.php

@@ -37,13 +37,22 @@ class CRM_Activity_BAO_ActivityContact extends CRM_Activity_DAO_ActivityContact
    *   activity_contact object
    */
   public static function create($params) {
+    $errorScope = CRM_Core_TemporaryErrorScope::useException();
     $activityContact = new CRM_Activity_DAO_ActivityContact();
-
     $activityContact->copyValues($params);
-    if (!$activityContact->find(TRUE)) {
+    try {
       return $activityContact->save();
     }
-    return $activityContact;
+    catch (PEAR_Exception $e) {
+      // This check used to be done first, creating an extra query before each insert.
+      // However, in none of the core tests was this ever called with values that already
+      // existed, meaning that this line would never or almost never be hit.
+      // hence it's better to save the select query here.
+      if ($activityContact->find(TRUE)) {
+        return $activityContact;
+      }
+      throw $e;
+    }
   }
 
   /**

tests/phpunit/api/v3/ActivityContactTest.php

@@ -40,21 +40,28 @@ class api_v3_ActivityContactTest extends CiviUnitTestCase {
 
   /**
    * @param int $version
+   *
    * @dataProvider versionThreeAndFour
+   * @throws \CRM_Core_Exception
    */
   public function testCreateActivityContact($version) {
     $this->_apiversion = $version;
 
-    $result = $this->callAPIAndDocument('activity_contact', 'create', $this->_params, __FUNCTION__, __FILE__);
+    $result = $this->callAPIAndDocument('ActivityContact', 'create', $this->_params, __FUNCTION__, __FILE__);
     $this->assertEquals(1, $result['count']);
     $this->assertNotNull($result['values'][$result['id']]['id']);
 
+    $result = $this->callAPIAndDocument('ActivityContact', 'create', $this->_params, __FUNCTION__, __FILE__);
+    $this->assertEquals(1, $result['count']);
+
     $this->callAPISuccess('activity_contact', 'delete', ['id' => $result['id']]);
   }
 
   /**
    * @param int $version
+   *
    * @dataProvider versionThreeAndFour
+   * @throws \CRM_Core_Exception
    */
   public function testDeleteActivityContact($version) {
     $this->_apiversion = $version;
@@ -71,7 +78,9 @@ class api_v3_ActivityContactTest extends CiviUnitTestCase {
 
   /**
    * @param int $version
+   *
    * @dataProvider versionThreeAndFour
+   * @throws \CRM_Core_Exception
    */
   public function testGetActivitiesByContact($version) {
     $this->_apiversion = $version;
@@ -80,7 +89,9 @@ class api_v3_ActivityContactTest extends CiviUnitTestCase {
 
   /**
    * @param int $version
+   *
    * @dataProvider versionThreeAndFour
+   * @throws \CRM_Core_Exception
    */
   public function testGetActivitiesByActivity($version) {
     $this->_apiversion = $version;
@@ -89,8 +100,11 @@ class api_v3_ActivityContactTest extends CiviUnitTestCase {
 
   /**
    * Test civicrm_activity_contact_get with empty params.
+   *
    * @param int $version
+   *
    * @dataProvider versionThreeAndFour
+   * @throws \CRM_Core_Exception
    */
   public function testGetEmptyParams($version) {
     $this->_apiversion = $version;