From 0606198bdac72ea958414e8560ab26550de4f6fd Mon Sep 17 00:00:00 2001 From: "deb.monish" Date: Tue, 25 Jul 2017 01:46:38 +0530 Subject: [PATCH] CRM-20855: Disabling "Search Primary Details Only" causes partial CiviMail 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. --- CRM/Contact/BAO/Query.php | 36 ++++++------ CRM/Utils/Token.php | 2 +- api/v3/utils.php | 6 +- .../CRM/Utils/{Token.php => TokenTest.php} | 55 ++++++++++++------- 4 files changed, 60 insertions(+), 39 deletions(-) rename tests/phpunit/CRM/Utils/{Token.php => TokenTest.php} (65%) diff --git a/CRM/Contact/BAO/Query.php b/CRM/Contact/BAO/Query.php index dabb838b8488..ab28ca3de959 100644 --- a/CRM/Contact/BAO/Query.php +++ b/CRM/Contact/BAO/Query.php @@ -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 @@ public function __construct( $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 index 904fa40ad8a7..7bd729b2b0cc 100644 --- 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 index 9f4288670b74..cdbc837f2ac7 100644 --- 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 b/tests/phpunit/CRM/Utils/TokenTest.php similarity index 65% rename from tests/phpunit/CRM/Utils/Token.php rename to tests/phpunit/CRM/Utils/TokenTest.php index ab0b33403786..f6df7d0122ea 100644 --- a/tests/phpunit/CRM/Utils/Token.php +++ b/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'); } /**