From 13f957178f16a3cfe84bb83e491e032b6259bfe4 Mon Sep 17 00:00:00 2001 From: eileen Date: Tue, 29 Aug 2017 19:31:30 +1200 Subject: [PATCH] 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 | 42 ++++----- CRM/Utils/Token.php | 5 +- api/v3/utils.php | 6 +- tests/phpunit/CRM/Utils/Token.php | 63 -------------- tests/phpunit/CRM/Utils/TokenTest.php | 121 ++++++++++++++++++++++++++ 5 files changed, 143 insertions(+), 94 deletions(-) delete mode 100644 tests/phpunit/CRM/Utils/Token.php create mode 100644 tests/phpunit/CRM/Utils/TokenTest.php diff --git a/CRM/Contact/BAO/Query.php b/CRM/Contact/BAO/Query.php index 6e4edb89321e..479099d07d10 100644 --- 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,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, @@ -435,18 +428,18 @@ 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(); } - // 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 index a32df892a5e9..0335ccfb774e 100644 --- a/CRM/Utils/Token.php +++ b/CRM/Utils/Token.php @@ -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]; 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/Token.php deleted file mode 100644 index 5f5a25c17616..000000000000 --- a/tests/phpunit/CRM/Utils/Token.php +++ /dev/null @@ -1,63 +0,0 @@ -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, - )); - - // 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'); - } - -} diff --git a/tests/phpunit/CRM/Utils/TokenTest.php b/tests/phpunit/CRM/Utils/TokenTest.php new file mode 100644 index 000000000000..f6df7d0122ea --- /dev/null +++ b/tests/phpunit/CRM/Utils/TokenTest.php @@ -0,0 +1,121 @@ +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]); + } + } + } + +}