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 · Aug 29, 2017 · 64f3607 · 64f3607
1 parent e10dd65
commit 64f3607
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 92 deletions.
diff --git a/CRM/Contact/BAO/Query.php b/CRM/Contact/BAO/Query.php
@@ -404,12 +404,6 @@ class CRM_Contact_BAO_Query {
 
   public $_pseudoConstantsSelect = array();
 
-  /**
-   * Set to true when address fields will be always on basis of primary location type
-   * @var Boolean
-   */
-  static $_alwaysSearchByPrimaryOnly = FALSE;
-
   /**
    * Class constructor which also does all the work.
    *
@@ -426,27 +420,26 @@ 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();
     }
 
-    // Search by Primary location type ONLY if searchPrimaryDetailsOnly setting is on OR
-    //  search query is trigerred via api OR
-    //  special parameter search_by_primary_details_only = TRUE
-    self::$_alwaysSearchByPrimaryOnly = (Civi::settings()->get('searchPrimaryDetailsOnly') || $apiEntity || !empty($params['search_by_primary_details_only']));
-
     if ($returnProperties === self::NO_RETURN_PROPERTIES) {
       $this->_returnProperties = array();
     }
@@ -2645,27 +2638,24 @@ public static function fromClause(&$tables, $inner = NULL, $right = NULL, $prima
         continue;
       }
 
-      $searchPrimary = self::$_alwaysSearchByPrimaryOnly ? "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':
@@ -2675,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':
@@ -4433,6 +4423,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(
@@ -4446,7 +4437,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(
@@ -4455,7 +4447,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');
   }
 
   /**
@@ -101,49 +118,4 @@ public function testGetTokenDetailsMultipleEmails() {
     }
   }
 
-  /**
-   * Test getting multiple contacts.
-   *
-   * Check for situation described in CRM-19876.
-   */
-  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,
-    ));
-
-    // when we are fetching contact details NOT ON basis of primary address fields
-    $extraParams = array('search_by_primary_details_only' => FALSE);
-    $contactIDs = array($contactID);
-    $contactDetails = CRM_Utils_Token::getTokenDetails($contactIDs, NULL, TRUE, TRUE, $extraParams);
-    $this->assertNotEquals($primaryEmail, $contactDetails[0][$contactID]['email']);
-
-    // 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 @@