CRM-20855: Disabling "Search Primary Details Only" causes partial CiviMail delivery...
authordeb.monish <monish.deb@jmaconsulting.biz>
Mon, 24 Jul 2017 20:16:38 +0000 (01:46 +0530)
committereileen <emcnaughton@wikimedia.org>
Mon, 4 Sep 2017 07:06:37 +0000 (19:06 +1200)
CRM-20855: Adjust variables for clarity

I'm concerned about the use of $apiEntity to mean 'do something unconnected' so suggest
a specific paramteter.

CRM/Contact/BAO/Query.php
CRM/Utils/Token.php
api/v3/utils.php
tests/phpunit/CRM/Utils/TokenTest.php [moved from tests/phpunit/CRM/Utils/Token.php with 65% similarity]

index dabb838b848836b2501965d3c209c4c05ce59257..ab28ca3de959dd47a298796719d059d0a0d1e174 100644 (file)
@@ -420,8 +420,7 @@ 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,
@@ -429,8 +428,13 @@ class CRM_Contact_BAO_Query {
     $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 @@ class CRM_Contact_BAO_Query {
         }
         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 @@ class CRM_Contact_BAO_Query {
           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 @@ civicrm_relationship.is_permission_a_b = 0
    *   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 @@ civicrm_relationship.is_permission_a_b = 0
     $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 @@ civicrm_relationship.is_permission_a_b = 0
       $skipPermissions,
       TRUE, $smartGroupCache,
       NULL, 'AND',
-      $apiEntity
+      $apiEntity, $primaryLocationOnly
     );
 
     //this should add a check for view deleted if permissions are enabled
index 904fa40ad8a790a943a82ed24bc239830812f879..7bd729b2b0cca34c236e47291c9bd1b813d24e01 100644 (file)
@@ -1235,7 +1235,7 @@ class CRM_Utils_Token {
       }
     }
 
-    $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];
 
index 9f4288670b7402085ea4ece1ab20c6fbbd21ca3e..cdbc837f2ac7aaf3a5dda043e0bcca43e03fa983 100644 (file)
@@ -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();
 
similarity index 65%
rename from tests/phpunit/CRM/Utils/Token.php
rename to tests/phpunit/CRM/Utils/TokenTest.php
index ab0b3340378629b8c56a8508aedb1996220e39f4..f6df7d0122ea03ddc2a1d4dc125db29a3ea580b4 100644 (file)
@@ -16,27 +16,44 @@ class CRM_Utils_TokenTest extends CiviUnitTestCase {
   }
 
   /**
-   * 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');
   }
 
   /**