Skip to content

Commit

Permalink
CRM-20855: Adjust variables for clarity
Browse files Browse the repository at this point in the history
I'm concerned about the use of $apiEntity to mean 'do something unconnected' so suggest
a specific paramteter.
  • Loading branch information
eileenmcnaughton committed Aug 29, 2017
1 parent 0c84e13 commit 13f9571
Show file tree
Hide file tree
Showing 5 changed files with 143 additions and 94 deletions.
42 changes: 17 additions & 25 deletions CRM/Contact/BAO/Query.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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();
}
Expand Down Expand Up @@ -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':
Expand All @@ -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':
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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
Expand Down
5 changes: 1 addition & 4 deletions CRM/Utils/Token.php
Original file line number Diff line number Diff line change
Expand Up @@ -1235,10 +1235,7 @@ public static function getTokenDetails(
}
}

$params['search_by_primary_details_only'] = CRM_Utils_Array::value('search_by_primary_details_only', $params, TRUE);
$query = new CRM_Contact_BAO_Query($params, $returnProperties);

$details = $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
6 changes: 4 additions & 2 deletions api/v3/utils.php
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,8 @@ function _civicrm_api3_get_using_query_object($entity, $params, $additional_opti
$getCount,
$skipPermissions,
$mode,
$entity
$entity,
TRUE
);

return $entities;
Expand Down Expand Up @@ -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();

Expand Down
63 changes: 0 additions & 63 deletions tests/phpunit/CRM/Utils/Token.php

This file was deleted.

121 changes: 121 additions & 0 deletions tests/phpunit/CRM/Utils/TokenTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
<?php

/**
* Class CRM_Utils_TokenTest
* @group headless
*/
class CRM_Utils_TokenTest extends CiviUnitTestCase {

/**
* Basic test on getTokenDetails function.
*/
public function testGetTokenDetails() {
$contactID = $this->individualCreate(array('preferred_communication_method' => array('Phone', 'Fax')));
$resolvedTokens = CRM_Utils_Token::getTokenDetails(array($contactID));
$this->assertEquals('Phone, Fax', $resolvedTokens[0][$contactID]['preferred_communication_method']);
}

/**
* Test getting contacts w/o primary location type
*
* 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,
));

$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');
}

/**
* Test getting multiple contacts.
*
* Check for situation described in CRM-19876.
*/
public function testGetTokenDetailsMultipleEmails() {
$i = 0;

$params = array(
'do_not_phone' => 1,
'do_not_email' => 0,
'do_not_mail' => 1,
'do_not_sms' => 1,
'do_not_trade' => 1,
'is_opt_out' => 0,
'email' => 'guardians@galaxy.com',
'legal_identifier' => 'convict 56',
'nick_name' => 'bob',
'contact_source' => 'bargain basement',
'formal_title' => 'Your silliness',
'job_title' => 'World Saviour',
'gender_id' => '1',
'birth_date' => '2017-01-01',
// 'city' => 'Metropolis',
);
$contactIDs = array();
while ($i < 27) {
$contactIDs[] = $contactID = $this->individualCreate($params);
$this->callAPISuccess('Email', 'create', array(
'contact_id' => $contactID,
'email' => 'goodguy@galaxy.com',
'location_type_id' => 'Other',
'is_primary' => 0,
));
$this->callAPISuccess('Email', 'create', array(
'contact_id' => $contactID,
'email' => 'villain@galaxy.com',
'location_type_id' => 'Work',
'is_primary' => 1,
));
$i++;
}
unset($params['email']);

$resolvedTokens = CRM_Utils_Token::getTokenDetails($contactIDs);
foreach ($contactIDs as $contactID) {
$resolvedContactTokens = $resolvedTokens[0][$contactID];
$this->assertEquals('Individual', $resolvedContactTokens['contact_type']);
$this->assertEquals('Anderson, Anthony', $resolvedContactTokens['sort_name']);
$this->assertEquals('en_US', $resolvedContactTokens['preferred_language']);
$this->assertEquals('Both', $resolvedContactTokens['preferred_mail_format']);
$this->assertEquals(3, $resolvedContactTokens['prefix_id']);
$this->assertEquals(3, $resolvedContactTokens['suffix_id']);
$this->assertEquals('Mr. Anthony J. Anderson II', $resolvedContactTokens['addressee_display']);
$this->assertEquals('villain@galaxy.com', $resolvedContactTokens['email']);

foreach ($params as $key => $value) {
$this->assertEquals($value, $resolvedContactTokens[$key]);
}
}
}

}

0 comments on commit 13f9571

Please sign in to comment.