Skip to content

Commit

Permalink
Add / make fit for purpose email.getlist api call
Browse files Browse the repository at this point in the history
The function CRM_Contact_Page_AJAX::getContactEmail is one of our  earlier  ajax attempts & this approach has been largely
replaced with entity Reference fields. In order to switch over we need to bring Email.getlist api to parity which  means
1) searching on sortname first, if less than 10 results on emails include emails
2) appropriate respect for includeWildCardInName (this should already be in the generic getlist)
3) filter out on_hold, is_deceased, do_not_email
4) acl support (should already  be part of the api).

The trickiest of these to support is the first - because we need to avoid using a non-performant OR
My current solution is the idea of a fallback field to search if the search results are less than the limit.
in most cases this won't require a second query but when it does it should be fairly quick.
  • Loading branch information
eileenmcnaughton committed Apr 6, 2020
1 parent 834f0bd commit dace9ba
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 3 deletions.
32 changes: 32 additions & 0 deletions 1
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
Add / make fit for purpose email.getlist api call

The function CRM_Contact_Page_AJAX::getContactEmail is one of our earlier ajax attempts & this approach has been largely
replaced with entity Reference fields. In order to switch over we need to bring Email.getlist api to parity which means
1) searching on sortname first, if less than 10 results on emails include emails
2) appropriate respect for includeWildCardInName (this should already be in the generic getlist)
3) filter out on_hold, is_deceased, do_not_email
4) acl support (should already be part of the api).

The trickiest of these to support is the first - because we need to avoid using a non-performant OR
My current solution is the idea of a fallback field to search if the search results are less than the limit.
in most cases this won't require a second query but when it does it should be fairly quick.

# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
#
# Date: Mon Apr 6 15:48:00 2020 +1200
#
# On branch emailget
# Changes to be committed:
# modified: api/v3/Email.php
# modified: api/v3/Generic/Getlist.php
# modified: tests/phpunit/CRM/Contact/Form/Task/EmailCommonTest.php
# modified: tests/phpunit/api/v3/ContactTest.php
# modified: tests/phpunit/api/v3/EmailTest.php
#
# Changes not staged for commit:
# modified: api/v3/Email.php
# modified: templates/CRM/Contact/Form/Task/Email.tpl
# modified: tests/phpunit/CRM/Contact/Form/Task/EmailCommonTest.php
# modified: tests/phpunit/api/v3/EmailTest.php
#
37 changes: 36 additions & 1 deletion api/v3/Email.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
*
* @return array
* API result array
*
* @throws \API_Exception
* @throws \Civi\API\Exception\UnauthorizedException
*/
function civicrm_api3_email_create($params) {
return _civicrm_api3_basic_create(_civicrm_api3_get_BAO(__FUNCTION__), $params, 'Email');
Expand All @@ -37,7 +40,6 @@ function civicrm_api3_email_create($params) {
* Array of parameters determined by getfields.
*/
function _civicrm_api3_email_create_spec(&$params) {
// TODO a 'clever' default should be introduced
$params['is_primary']['api.default'] = 0;
$params['email']['api.required'] = 1;
$params['contact_id']['api.required'] = 1;
Expand All @@ -55,6 +57,10 @@ function _civicrm_api3_email_create_spec(&$params) {
*
* @return array
* API result array.
*
* @throws \API_Exception
* @throws \CiviCRM_API3_Exception
* @throws \Civi\API\Exception\UnauthorizedException
*/
function civicrm_api3_email_delete($params) {
return _civicrm_api3_basic_delete(_civicrm_api3_get_BAO(__FUNCTION__), $params);
Expand All @@ -72,3 +78,32 @@ function civicrm_api3_email_delete($params) {
function civicrm_api3_email_get($params) {
return _civicrm_api3_basic_get(_civicrm_api3_get_BAO(__FUNCTION__), $params);
}

/**
* Set default getlist parameters.
*
* @see _civicrm_api3_generic_getlist_defaults
*
* @param array $request
*
* @return array
*/
function _civicrm_api3_email_getlist_defaults(&$request) {
return [
'description_field' => [
'contact_id.sort_name',
'email',
],
'params' => [
'on_hold' => 0,
'contact_id.is_deleted' => 0,
'contact_id.is_deceased' => 0,
'contact_id.do_not_email' => 0,
],
'label_field' => 'contact_id.display_name',
// If no results from sort_name try email.
'search_field' => 'contact_id.sort_name',
'search_field_fallback' => 'email',
];

}
20 changes: 19 additions & 1 deletion api/v3/Generic/Getlist.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
* @param array $apiRequest
*
* @return mixed
* @throws \CiviCRM_API3_Exception
*/
function civicrm_api3_generic_getList($apiRequest) {
$entity = _civicrm_api_get_entity_name_from_camel($apiRequest['entity']);
Expand All @@ -37,6 +38,23 @@ function civicrm_api3_generic_getList($apiRequest) {

$request['params']['check_permissions'] = !empty($apiRequest['params']['check_permissions']);
$result = civicrm_api3($entity, 'get', $request['params']);
if (!empty($request['input']) && !empty($defaults['search_field_fallback']) && $result['count'] < $request['params']['options']['limit']) {
// We support a field fallback. Note we don't do this as an OR query because that could easily
// bypass an index & kill the server. We just 'pad' the results if needed with the second
// query - this is effectively the same as what the old Ajax::getContactEmail function did.
// Since these queries should be quick & often only one should be needed this is a simpler alternative
// to constructing a UNION via the api.
$request['params'][$defaults['search_field_fallback']] = $request['params'][$defaults['search_field']];
unset($request['params'][$defaults['search_field']]);
$request['params']['options']['limit'] -= $result['count'];
$result2 = civicrm_api3($entity, 'get', $request['params']);
$result['values'] = array_merge($result['values'], $result2['values']);
$result['count'] = count($result['values']);
}
else {
// Re-index to sequential = 0.
$result['values'] = array_merge($result['values']);
}

// Hey api, would you like to format the output?
$fnName = "_civicrm_api3_{$entity}_getlist_output";
Expand Down Expand Up @@ -98,7 +116,7 @@ function _civicrm_api3_generic_getList_defaults($entity, &$request, $apiDefaults
$request += $apiDefaults + $defaults;
// Default api params
$params = [
'sequential' => 1,
'sequential' => 0,
'options' => [],
];
// When searching e.g. autocomplete
Expand Down
16 changes: 15 additions & 1 deletion tests/phpunit/CRM/Contact/Form/Task/EmailCommonTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,27 @@
*/
class CRM_Contact_Form_Task_EmailCommonTest extends CiviUnitTestCase {

/**
* Set up for tests.
*
* @throws \CRM_Core_Exception
*/
protected function setUp() {
parent::setUp();
$this->_contactIds = [
$this->individualCreate(['first_name' => 'Antonia', 'last_name' => 'D`souza']),
$this->individualCreate(['first_name' => 'Anthony', 'last_name' => 'Collins']),
];
$this->_optionValue = $this->callApiSuccess('optionValue', 'create', [
$this->_optionValue = $this->callAPISuccess('optionValue', 'create', [
'label' => '"Seamus Lee" <[email protected]>',
'option_group_id' => 'from_email_address',
]);
}

/**
* Test generating domain emails
*
* @throws \CRM_Core_Exception
*/
public function testDomainEmailGeneration() {
$emails = CRM_Core_BAO_Email::domainEmails();
Expand All @@ -39,6 +46,13 @@ public function testDomainEmailGeneration() {
$this->assertEquals('"Seamus Lee" <[email protected]>', $optionValue['values'][$this->_optionValue['id']]['label']);
}

/**
* Test email uses signature.
*
* @throws \CRM_Core_Exception
* @throws \CiviCRM_API3_Exception
* @throws \Civi\API\Exception\UnauthorizedException
*/
public function testPostProcessWithSignature() {
$mut = new CiviMailUtils($this, TRUE);
Civi::settings()->set('allow_mail_from_logged_in_contact', 1);
Expand Down
2 changes: 2 additions & 0 deletions tests/phpunit/api/v3/ContactTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3683,6 +3683,8 @@ public function testReturnCityProfile() {

/**
* CRM-15443 - ensure getlist api does not return deleted contacts.
*
* @throws \CRM_Core_Exception
*/
public function testGetlistExcludeConditions() {
$name = 'Scarabée';
Expand Down
22 changes: 22 additions & 0 deletions tests/phpunit/api/v3/EmailTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -495,4 +495,26 @@ public function testSetBulkEmail() {
$this->assertEquals(1, $emails[$email2['id']]['is_bulkmail']);
}

/**
* Test getlist.
*
* @throws \CRM_Core_Exception
*/
public function testGetlist() {
$name = 'Scarabée';
$emailMatchContactID = $this->individualCreate(['last_name' => $name, 'email' => '[email protected]']);
$emailMatchEmailID = $this->callAPISuccessGetValue('Email', ['return' => 'id', 'contact_id' => $emailMatchContactID]);
$this->individualCreate(['last_name' => $name, 'email' => '[email protected]', 'is_deceased' => 1]);
$this->individualCreate(['last_name' => $name, 'email' => '[email protected]', 'is_deleted' => 1]);
$this->individualCreate(['last_name' => $name, 'api.email.create' => ['email' => '[email protected]', 'on_hold' => 1]]);
$this->individualCreate(['last_name' => $name, 'do_not_email' => 1, 'api.email.create' => ['email' => '[email protected]']]);
$nameMatchContactID = $this->individualCreate(['last_name' => 'bob', 'email' => '[email protected]']);
$nameMatchEmailID = $this->callAPISuccessGetValue('Email', ['return' => 'id', 'contact_id' => $nameMatchContactID]);
// We should get only the active live email-able contact.
$result = $this->callAPISuccess('Email', 'getlist', ['input' => 'bob'])['values'];
$this->assertCount(2, $result);
$this->assertEquals($nameMatchEmailID, $result[0]['id']);
$this->assertEquals($emailMatchEmailID, $result[1]['id']);
}

}

0 comments on commit dace9ba

Please sign in to comment.