Skip to content

Commit

Permalink
Merge pull request #9301 from eileenmcnaughton/quicksearch
Browse files Browse the repository at this point in the history 
CRM-19547 rationalise expensive use of exactMatch search on quicksearch
  • Loading branch information
@colemanw
colemanw authored Nov 1, 2016
2 parents cf159f0 + 613643e commit 696d853
Show file tree
Hide file tree
Showing 2 changed files with 164 additions and 35 deletions.
74 changes: 60 additions & 14 deletions api/v3/Contact.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -806,6 +806,7 @@ function civicrm_api3_contact_getquick($params) {
if ($aclWhere) {
$where .= " AND $aclWhere ";
}
$isPrependWildcard = \Civi::settings()->get('includeWildCardInName');

if (!empty($params['org'])) {
$where .= " AND contact_type = \"Organization\"";
Expand All @@ -818,7 +819,7 @@ function civicrm_api3_contact_getquick($params) {
(int) $params['employee_id'],
'employer_id'
)) {
if ($config->includeWildCardInName) {
if ($isPrependWildcard) {
$strSearch = "%$name%";
}
else {
Expand Down Expand Up @@ -865,7 +866,7 @@ function civicrm_api3_contact_getquick($params) {
$rel = CRM_Utils_Type::escape($relation[2], 'String');
}

if ($config->includeWildCardInName) {
if ($isPrependWildcard) {
$strSearch = "%$name%";
}
else {
Expand All @@ -880,15 +881,13 @@ function civicrm_api3_contact_getquick($params) {
//CRM-10687
if (!empty($params['field_name']) && !empty($params['table_name'])) {
$whereClause = " WHERE ( $table_name.$field_name LIKE '$strSearch') {$where}";
$exactWhereClause = " WHERE ( $table_name.$field_name = '$name') {$where}";
// Search by id should be exact
if ($field_name == 'id' || $field_name == 'external_identifier') {
$whereClause = $exactWhereClause;
$whereClause = " WHERE ( $table_name.$field_name = '$name') {$where}";
}
}
else {
$whereClause = " WHERE ( sort_name LIKE '$strSearch' $includeNickName ) {$where} ";
$exactWhereClause = " WHERE ( sort_name LIKE '$name' $exactIncludeNickName ) {$where} ";
if ($config->includeEmailInName) {
if (!in_array('email', $list)) {
$includeEmailFrom = "LEFT JOIN civicrm_email eml ON ( cc.id = eml.contact_id AND eml.is_primary = 1 )";
Expand All @@ -913,12 +912,7 @@ function civicrm_api3_contact_getquick($params) {
INNER JOIN civicrm_uf_match um ON (um.contact_id=cc.id)
";
}

$orderByInner = $orderByOuter = "ORDER BY exactFirst";
if ($config->includeOrderByClause) {
$orderByInner = "ORDER BY exactFirst, sort_name";
$orderByOuter .= ", sort_name";
}
$orderBy = _civicrm_api3_quicksearch_get_order_by($name, $isPrependWildcard, $field_name);

//CRM-5954
$query = "
Expand All @@ -932,7 +926,7 @@ function civicrm_api3_contact_getquick($params) {
{$aclFrom}
{$additionalFrom}
{$whereClause}
{$orderByInner}
{$orderBy}
LIMIT 0, {$limit} )
";

Expand All @@ -947,13 +941,13 @@ function civicrm_api3_contact_getquick($params) {
{$aclFrom}
{$additionalFrom} {$includeEmailFrom}
{$emailWhere} AND cc.is_deleted = 0 " . ($aclWhere ? " AND $aclWhere " : '') . "
{$orderByInner}
{$orderBy}
LIMIT 0, {$limit}
)
";
}
$query .= ") t
{$orderByOuter}
{$orderBy}
LIMIT 0, {$limit}
";

Expand Down Expand Up @@ -1008,6 +1002,58 @@ function civicrm_api3_contact_getquick($params) {
return civicrm_api3_create_success($contactList, $params, 'Contact', 'getquick');
}

/**
* Get the order by string for the quicksearch query.
*
* Get the order by string. The string might be
* - sort name if there is no search value provided and the site is configured
* to search by sort name
* - empty if there is no search value provided and the site is not configured
* to search by sort name
* - exactFirst and then sort name if a search value is provided and the site is configured
* to search by sort name
* - exactFirst if a search value is provided and the site is not configured
* to search by sort name
*
* exactFirst means 'yes if the search value exactly matches the searched field. else no'.
* It is intended to prioritise exact matches for the entered string so on a first name search
* for 'kath' contacts with a first name of exactly Kath rise to the top.
*
* On short strings it is expensive. Per CRM-19547 there is still an open question
* as to whether we should only do exactMatch on a minimum length or on certain fields.
*
* However, we have mitigated this somewhat by not doing an exact match search on
* empty strings, non-wildcard sort-name searches and email searches where there is
* no @ after the first character.
*
* For the user it is further mitigated by the fact they just don't know the
* slower queries are firing. If they type 'smit' slowly enough 4 queries will trigger
* but if the first 3 are slow the first result they see may be off the 4th query.
*
* @param string $name
* @param bool $isPrependWildcard
* @param string $field_name
*
* @return string
*/
function _civicrm_api3_quicksearch_get_order_by($name, $isPrependWildcard, $field_name) {
$skipExactMatch = ($name === '%');
if ($field_name === 'email' && !strpos('@', $name)) {
$skipExactMatch = TRUE;
}

if (!\Civi::settings()->get('includeOrderByClause')) {
return $skipExactMatch ? '' : "ORDER BY exactFirst";
}
if ($skipExactMatch || (!$isPrependWildcard && $field_name === 'sort_name')) {
// If there is no wildcard then sorting by exactFirst would have the same
// effect as just a sort_name search, but slower.
return "ORDER BY sort_name";
}

return "ORDER BY exactFirst, sort_name";
}

/**
* Declare deprecated api functions.
*
Expand Down
125 changes: 104 additions & 21 deletions tests/phpunit/api/v3/ContactTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2407,19 +2407,100 @@ public function testGetquickPermissionCRM13744() {
*
* The search string 'b' & 'bob' both return ordered by sort_name if includeOrderByClause
* is true (default) but if it is false then matches are returned in ID order.
*
* @dataProvider getSearchSortOptions
*/
public function testGetQuickExactFirst() {
public function testGetQuickExactFirst($searchParameters, $settings, $firstContact, $secondContact = NULL) {
$this->getQuickSearchSampleData();
$result = $this->callAPISuccess('contact', 'getquick', array('name' => 'b'));
$this->assertEquals('A Bobby, Bobby', $result['values'][0]['sort_name']);
$this->assertEquals('B Bobby, Bobby', $result['values'][1]['sort_name']);
$result = $this->callAPISuccess('contact', 'getquick', array('name' => 'bob'));
$this->assertEquals('A Bobby, Bobby', $result['values'][0]['sort_name']);
$this->assertEquals('B Bobby, Bobby', $result['values'][1]['sort_name']);
$this->callAPISuccess('Setting', 'create', array('includeOrderByClause' => FALSE));
$result = $this->callAPISuccess('contact', 'getquick', array('name' => 'bob'));
$this->assertEquals('Bob, Bob', $result['values'][0]['sort_name']);
$this->assertEquals('A Bobby, Bobby', $result['values'][1]['sort_name']);
$this->callAPISuccess('Setting', 'create', $settings);
$result = $this->callAPISuccess('contact', 'getquick', $searchParameters);
$this->assertEquals($firstContact, $result['values'][0]['sort_name']);
$this->assertEquals($secondContact, $result['values'][1]['sort_name']);
$this->callAPISuccess('Setting', 'create', array('includeWildCardInName' => TRUE, 'includeOrderByClause' => TRUE));
}

public function getSearchSortOptions() {
$firstAlphabeticalContactBySortName = 'A Bobby, Bobby';
$secondAlphabeticalContactBySortName = 'Aadvark, Bob';
$secondAlphabeticalContactWithEmailBySortName = 'Bob, Bob';
$firstAlphabeticalContactFirstNameBob = 'Aadvark, Bob';
$secondAlphabeticalContactFirstNameBob = 'Bob, Bob';
$firstByIDContactFirstNameBob = 'Bob, Bob';
$secondByIDContactFirstNameBob = 'K Bobby, Bob';
$firstContactByID = 'Bob, Bob';
$secondContactByID = 'E Bobby, Bobby';
$bobLikeEmail = 'A Bobby, Bobby';

return array(
'empty_search_basic' => array(
'search_parameters' => array('name' => '%'),
'settings' => array('includeWildCardInName' => TRUE, 'includeOrderByClause' => TRUE),
'first_contact' => $firstAlphabeticalContactBySortName,
'second_contact' => $secondAlphabeticalContactBySortName,
),
'empty_search_basic_no_wildcard' => array(
'search_parameters' => array('name' => '%'),
'settings' => array('includeWildCardInName' => FALSE, 'includeOrderByClause' => TRUE),
'first_contact' => $firstAlphabeticalContactBySortName,
'second_contact' => $secondAlphabeticalContactBySortName,
),
'single_letter_search_basic' => array(
'search_parameters' => array('name' => 'b'),
'settings' => array('includeWildCardInName' => TRUE, 'includeOrderByClause' => TRUE),
'first_contact' => $firstAlphabeticalContactBySortName,
'second_contact' => $secondAlphabeticalContactBySortName,
),
'bob_search_basic' => array(
'search_parameters' => array('name' => 'bob'),
'settings' => array('includeWildCardInName' => TRUE, 'includeOrderByClause' => TRUE),
'first_contact' => $firstAlphabeticalContactBySortName,
'second_contact' => $secondAlphabeticalContactBySortName,
),
'bob_search_no_orderby' => array(
'search_parameters' => array('name' => 'bob'),
'settings' => array('includeWildCardInName' => TRUE, 'includeOrderByClause' => FALSE),
'first_contact' => $firstContactByID,
'second_contact' => $secondContactByID,
),
'bob_search_no_wildcard' => array(
'search_parameters' => array('name' => 'bob'),
'settings' => array('includeWildCardInName' => FALSE, 'includeOrderByClause' => TRUE),
'second_contact' => $bobLikeEmail,
'first_contact' => $secondAlphabeticalContactFirstNameBob,
),
// This should be the same as just no wildcard as if we had an exactMatch while searching by
// sort name it would rise to the top CRM-19547
'bob_search_no_wildcard_no_orderby' => array(
'search_parameters' => array('name' => 'bob'),
'settings' => array('includeWildCardInName' => FALSE, 'includeOrderByClause' => TRUE),
'second_contact' => $bobLikeEmail,
'first_contact' => $secondAlphabeticalContactFirstNameBob,
),
'first_name_search_basic' => array(
'search_parameters' => array('name' => 'bob', 'field_name' => 'first_name'),
'settings' => array('includeWildCardInName' => TRUE, 'includeOrderByClause' => TRUE),
'first_contact' => $firstAlphabeticalContactFirstNameBob,
'second_contact' => $secondAlphabeticalContactFirstNameBob,
),
'first_name_search_no_wildcard' => array(
'search_parameters' => array('name' => 'bob', 'field_name' => 'first_name'),
'settings' => array('includeWildCardInName' => FALSE, 'includeOrderByClause' => TRUE),
'first_contact' => $firstAlphabeticalContactFirstNameBob,
'second_contact' => $secondAlphabeticalContactFirstNameBob,
),
'first_name_search_no_orderby' => array(
'search_parameters' => array('name' => 'bob', 'field_name' => 'first_name'),
'settings' => array('includeWildCardInName' => TRUE, 'includeOrderByClause' => FALSE),
'first_contact' => $firstByIDContactFirstNameBob,
'second_contact' => $secondByIDContactFirstNameBob,
),
'email_search_basic' => array(
'search_parameters' => array('name' => 'bob', 'field_name' => 'email', 'table_name' => 'eml'),
'settings' => array('includeWildCardInName' => FALSE, 'includeOrderByClause' => TRUE),
'first_contact' => $firstAlphabeticalContactBySortName,
'second_contact' => $secondAlphabeticalContactWithEmailBySortName,
),
);
}

/**
Expand All @@ -2432,9 +2513,9 @@ public function testGetQuickEmail() {
'name' => 'c',
));
$expectedData = array(
'A Bobby, Bobby :: bob@bobby.com',
'Bob, Bob :: bob@bob.com',
'C Bobby, Bobby',
'E Bobby, Bobby :: bob@bobby.com',
'H Bobby, Bobby :: bob@h.com',
'Second Domain',
$this->callAPISuccessGetValue('Contact', array('id' => $loggedInContactID, 'return' => 'last_name')) . ', Logged In :: anthony_anderson@civicrm.org',
Expand Down Expand Up @@ -2481,9 +2562,9 @@ public function testGetQuickEmailACL() {
// Without the acl it would be 6 like the previous email getquick test.
$this->assertEquals(5, $result['count']);
$expectedData = array(
'A Bobby, Bobby :: bob@bobby.com',
'Bob, Bob :: bob@bob.com',
'C Bobby, Bobby',
'E Bobby, Bobby :: bob@bobby.com',
'Second Domain',
$this->callAPISuccessGetValue('Contact', array('id' => $loggedInContactID, 'return' => 'last_name')) . ', Logged In :: anthony_anderson@civicrm.org',
);
Expand Down Expand Up @@ -2524,14 +2605,14 @@ public function testGetQuickID() {
'table_name' => 'cc',
));
$this->assertEquals(1, $result['count']);
$this->assertEquals('A Bobby, Bobby', $result['values'][0]['sort_name']);
$this->assertEquals('E Bobby, Bobby', $result['values'][0]['sort_name']);
$result = $this->callAPISuccess('contact', 'getquick', array(
'name' => $max + 2,
'field_name' => 'contact_id',
'table_name' => 'cc',
));
$this->assertEquals(1, $result['count']);
$this->assertEquals('A Bobby, Bobby', $result['values'][0]['sort_name']);
$this->assertEquals('E Bobby, Bobby', $result['values'][0]['sort_name']);
}

/**
Expand All @@ -2548,6 +2629,7 @@ public function testGetQuickFirstName() {
'table_name' => 'cc',
));
$expected = array(
'Aadvark, Bob',
'Bob, Bob',
'K Bobby, Bob',
'A Bobby, Bobby',
Expand All @@ -2559,7 +2641,7 @@ public function testGetQuickFirstName() {
$this->callAPISuccess('Setting', 'create', array('includeOrderByClause' => FALSE));
$result = $this->callAPISuccess('contact', 'getquick', array('name' => 'bob'));
$this->assertEquals('Bob, Bob', $result['values'][0]['sort_name']);
$this->assertEquals('A Bobby, Bobby', $result['values'][1]['sort_name']);
$this->assertEquals('E Bobby, Bobby', $result['values'][1]['sort_name']);
}

/**
Expand All @@ -2568,7 +2650,7 @@ public function testGetQuickFirstName() {
public function testGetQuickFirstNameACLs() {
$this->getQuickSearchSampleData();
$userID = $this->createLoggedInUser();
$this->callAPISuccess('Setting', 'create', array('includeOrderByClause' => TRUE));
$this->callAPISuccess('Setting', 'create', array('includeOrderByClause' => TRUE, 'search_autocomplete_count' => 15));
CRM_Core_Config::singleton()->userPermissionClass->permissions = array();
$result = $this->callAPISuccess('contact', 'getquick', array(
'name' => 'Bob',
Expand All @@ -2584,9 +2666,9 @@ public function testGetQuickFirstNameACLs() {
'field_name' => 'first_name',
'table_name' => 'cc',
));
$this->assertEquals('K Bobby, Bob', $result['values'][1]['sort_name']);
$this->assertEquals('K Bobby, Bob', $result['values'][2]['sort_name']);
// Without the ACL 9 would be bob@h.com.
$this->assertEquals('I Bobby, Bobby', $result['values'][9]['sort_name']);
$this->assertEquals('I Bobby, Bobby', $result['values'][10]['sort_name']);
}

/**
Expand Down Expand Up @@ -2655,7 +2737,7 @@ public function testGetQuickCity() {
public function getQuickSearchSampleData() {
$contacts = array(
array('first_name' => 'Bob', 'last_name' => 'Bob', 'external_identifier' => 'abc', 'email' => 'bob@bob.com'),
array('first_name' => 'Bobby', 'last_name' => 'A Bobby', 'external_identifier' => 'abcd'),
array('first_name' => 'Bobby', 'last_name' => 'E Bobby', 'external_identifier' => 'abcd'),
array(
'first_name' => 'Bobby',
'last_name' => 'B Bobby',
Expand All @@ -2677,13 +2759,14 @@ public function getQuickSearchSampleData() {
),
),
array('first_name' => 'Bobby', 'last_name' => 'D Bobby', 'external_identifier' => 'efg'),
array('first_name' => 'Bobby', 'last_name' => 'E Bobby', 'external_identifier' => 'hij', 'email' => 'bob@bobby.com'),
array('first_name' => 'Bobby', 'last_name' => 'A Bobby', 'external_identifier' => 'hij', 'email' => 'bob@bobby.com'),
array('first_name' => 'Bobby', 'last_name' => 'F Bobby', 'external_identifier' => 'klm'),
array('first_name' => 'Bobby', 'last_name' => 'G Bobby', 'external_identifier' => 'nop'),
array('first_name' => 'Bobby', 'last_name' => 'H Bobby', 'external_identifier' => 'qrs', 'email' => 'bob@h.com'),
array('first_name' => 'Bobby', 'last_name' => 'I Bobby'),
array('first_name' => 'Bobby', 'last_name' => 'J Bobby'),
array('first_name' => 'Bob', 'last_name' => 'K Bobby', 'external_identifier' => 'bcdef'),
array('first_name' => 'Bob', 'last_name' => 'Aadvark'),
);
foreach ($contacts as $type => $contact) {
$contact['contact_type'] = 'Individual';
Expand Down

0 comments on commit 696d853

Please sign in to comment.