CRM-20855: Disabling "Search Primary Details Only" causes partial Civ…

…iMail delivery failure CRM-20855: Adjust variables for clarity I'm concerned about the use of $apiEntity to mean 'do something unconnected' so suggest a specific paramteter.
civicrm · Sep 4, 2017 · 0606198 · 0606198
1 parent f7a4b13
commit 0606198
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 39 deletions.
diff --git a/CRM/Contact/BAO/Query.php b/CRM/Contact/BAO/Query.php
@@ -420,17 +420,21 @@ class CRM_Contact_BAO_Query {
    * @param null $displayRelationshipType
    * @param string $operator
    * @param string $apiEntity
-   *
-   * @return \CRM_Contact_BAO_Query
+   * @param bool|NULL $primaryLocationOnly
    */
   public function __construct(
     $params = NULL, $returnProperties = NULL, $fields = NULL,
     $includeContactIds = FALSE, $strict = FALSE, $mode = 1,
     $skipPermission = FALSE, $searchDescendentGroups = TRUE,
     $smartGroupCache = TRUE, $displayRelationshipType = NULL,
     $operator = 'AND',
-    $apiEntity = NULL
+    $apiEntity = NULL,
+    $primaryLocationOnly = NULL
   ) {
+    if ($primaryLocationOnly === NULL) {
+      $primaryLocationOnly = Civi::settings()->get('searchPrimaryDetailsOnly');
+    }
+    $this->_primaryLocation = $primaryLocationOnly;
     $this->_params = &$params;
     if ($this->_params == NULL) {
       $this->_params = array();
@@ -2633,29 +2637,25 @@ public static function fromClause(&$tables, $inner = NULL, $right = NULL, $prima
         }
         continue;
       }
-      $searchPrimary = '';
-      if (Civi::settings()->get('searchPrimaryDetailsOnly') || $apiEntity) {
-        $searchPrimary = "AND {$name}.is_primary = 1";
-      }
+
+      $limitToPrimaryClause = $primaryLocation ? "AND {$name}.is_primary = 1" : '';
+
       switch ($name) {
         case 'civicrm_address':
           //CRM-14263 further handling of address joins further down...
-          if (!$primaryLocation) {
-            $searchPrimary = '';
-          }
-          $from .= " $side JOIN civicrm_address ON ( contact_a.id = civicrm_address.contact_id {$searchPrimary} )";
+          $from .= " $side JOIN civicrm_address ON ( contact_a.id = civicrm_address.contact_id {$limitToPrimaryClause} )";
           continue;
 
         case 'civicrm_phone':
-          $from .= " $side JOIN civicrm_phone ON (contact_a.id = civicrm_phone.contact_id {$searchPrimary}) ";
+          $from .= " $side JOIN civicrm_phone ON (contact_a.id = civicrm_phone.contact_id {$limitToPrimaryClause}) ";
           continue;
 
         case 'civicrm_email':
-          $from .= " $side JOIN civicrm_email ON (contact_a.id = civicrm_email.contact_id {$searchPrimary})";
+          $from .= " $side JOIN civicrm_email ON (contact_a.id = civicrm_email.contact_id {$limitToPrimaryClause})";
           continue;
 
         case 'civicrm_im':
-          $from .= " $side JOIN civicrm_im ON (contact_a.id = civicrm_im.contact_id {$searchPrimary}) ";
+          $from .= " $side JOIN civicrm_im ON (contact_a.id = civicrm_im.contact_id {$limitToPrimaryClause}) ";
           continue;
 
         case 'im_provider':
@@ -2665,7 +2665,7 @@ public static function fromClause(&$tables, $inner = NULL, $right = NULL, $prima
           continue;
 
         case 'civicrm_openid':
-          $from .= " $side JOIN civicrm_openid ON ( civicrm_openid.contact_id = contact_a.id {$searchPrimary} )";
+          $from .= " $side JOIN civicrm_openid ON ( civicrm_openid.contact_id = contact_a.id {$limitToPrimaryClause} )";
           continue;
 
         case 'civicrm_worldregion':
@@ -4428,6 +4428,7 @@ public static function getQuery($params = NULL, $returnProperties = NULL, $count
    *   The api entity being called.
    *   This sort-of duplicates $mode in a confusing way. Probably not by design.
    *
+   * @param bool|null $primaryLocationOnly
    * @return array
    */
   public static function apiQuery(
@@ -4441,7 +4442,8 @@ public static function apiQuery(
     $count = FALSE,
     $skipPermissions = TRUE,
     $mode = CRM_Contact_BAO_Query::MODE_CONTACTS,
-    $apiEntity = NULL
+    $apiEntity = NULL,
+    $primaryLocationOnly = NULL
   ) {
 
     $query = new CRM_Contact_BAO_Query(
@@ -4450,7 +4452,7 @@ public static function apiQuery(
       $skipPermissions,
       TRUE, $smartGroupCache,
       NULL, 'AND',
-      $apiEntity
+      $apiEntity, $primaryLocationOnly
     );
 
     //this should add a check for view deleted if permissions are enabled

diff --git a/CRM/Utils/Token.php b/CRM/Utils/Token.php
@@ -1235,7 +1235,7 @@ public static function getTokenDetails(
       }
     }
 
-    $details = CRM_Contact_BAO_Query::apiQuery($params, $returnProperties, NULL, NULL, 0, count($contactIDs));
+    $details = CRM_Contact_BAO_Query::apiQuery($params, $returnProperties, NULL, NULL, 0, count($contactIDs), TRUE, FALSE, TRUE, CRM_Contact_BAO_Query::MODE_CONTACTS, NULL, TRUE);
 
     $contactDetails = &$details[0];
 

diff --git a/api/v3/utils.php b/api/v3/utils.php
@@ -570,7 +570,8 @@ function _civicrm_api3_get_using_query_object($entity, $params, $additional_opti
     $getCount,
     $skipPermissions,
     $mode,
-    $entity
+    $entity,
+    TRUE
   );
 
   return $entities;
@@ -603,7 +604,8 @@ function _civicrm_api3_get_query_object($params, $mode, $entity) {
   $newParams = CRM_Contact_BAO_Query::convertFormValues($inputParams, 0, FALSE, $entity);
   $query = new CRM_Contact_BAO_Query($newParams, $returnProperties, NULL,
     FALSE, FALSE, $mode,
-    empty($params['check_permissions'])
+    empty($params['check_permissions']),
+    TRUE, TRUE, NULL, 'AND', 'NULL', TRUE
   );
   list($select, $from, $where, $having) = $query->query();
 

diff --git a/tests/phpunit/CRM/Utils/Token.php → tests/phpunit/CRM/Utils/TokenTest.php b/tests/phpunit/CRM/Utils/Token.php → tests/phpunit/CRM/Utils/TokenTest.php
@@ -16,27 +16,44 @@ public function testGetTokenDetails() {
   }
 
   /**
-   * Test for replaceGreetingTokens.
+   * Test getting contacts w/o primary location type
    *
+   * Check for situation described in CRM-19876.
    */
-  public function testReplaceGreetingTokens() {
-    $tokenString = 'First Name: {contact.first_name} Last Name: {contact.last_name} Birth Date: {contact.birth_date} Prefix: {contact.prefix_id} Suffix: {contact.individual_suffix}';
-    $contactDetails = array(
-      array(
-        2811 => array(
-          'id' => '2811',
-          'contact_type' => 'Individual',
-          'first_name' => 'Morticia',
-          'last_name' => 'Addams',
-          'prefix_id' => 2,
-        ),
-      ),
-    );
-    $contactId = 2811;
-    $className = 'CRM_Contact_BAO_Contact';
-    $escapeSmarty = TRUE;
-    CRM_Utils_Token::replaceGreetingTokens($tokenString, $contactDetails, $contactId, $className, $escapeSmarty);
-    $this->assertEquals($tokenString, 'First Name: Morticia Last Name: Addams Birth Date:  Prefix: Ms. Suffix: ');
+  public function testSearchByPrimaryLocation() {
+    // Disable searchPrimaryDetailsOnly civi settings so we could test the functionality without it.
+    Civi::settings()->set('searchPrimaryDetailsOnly', '0');
+
+    // create a contact with multiple email address and among which one is primary
+    $contactID = $this->individualCreate();
+    $primaryEmail = uniqid() . '@primary.com';
+    $this->callAPISuccess('Email', 'create', array(
+      'contact_id' => $contactID,
+      'email' => $primaryEmail,
+      'location_type_id' => 'Other',
+      'is_primary' => 1,
+    ));
+    $this->callAPISuccess('Email', 'create', array(
+      'contact_id' => $contactID,
+      'email' => uniqid() . '@galaxy.com',
+      'location_type_id' => 'Work',
+      'is_primary' => 0,
+    ));
+    $this->callAPISuccess('Email', 'create', array(
+      'contact_id' => $contactID,
+      'email' => uniqid() . '@galaxy.com',
+      'location_type_id' => 'Work',
+      'is_primary' => 0,
+    ));
+
+    $contactIDs = array($contactID);
+
+    // when we are fetching contact details ON basis of primary address fields
+    $contactDetails = CRM_Utils_Token::getTokenDetails($contactIDs);
+    $this->assertEquals($primaryEmail, $contactDetails[0][$contactID]['email']);
+
+    // restore setting
+    Civi::settings()->set('searchPrimaryDetailsOnly', '1');
   }
 
   /**
-Original file line number
+Diff line change
@@ Expand Up / @@ -1235,7 +1235,7 @@ public static function getTokenDetails( @@
           }
         }
-        $details = CRM_Contact_BAO_Query::apiQuery($params, $returnProperties, NULL, NULL, 0, count($contactIDs));
+        $details = CRM_Contact_BAO_Query::apiQuery($params, $returnProperties, NULL, NULL, 0, count($contactIDs), TRUE, FALSE, TRUE, CRM_Contact_BAO_Query::MODE_CONTACTS, NULL, TRUE);
         $contactDetails = &$details[0];
@@ Expand Down @@