From d544ffcd1dffd2c6e5c8c66e881e425817efc581 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Thu, 13 Jul 2017 10:49:30 -0400 Subject: [PATCH] Code cleanup --- CRM/Activity/BAO/Activity.php | 2 +- api/v3/Activity.php | 112 +++++++++++++++----------- tests/phpunit/api/v3/ActivityTest.php | 18 ++++- 3 files changed, 80 insertions(+), 52 deletions(-) diff --git a/CRM/Activity/BAO/Activity.php b/CRM/Activity/BAO/Activity.php index e136c7f55f..5befb93b57 100644 --- a/CRM/Activity/BAO/Activity.php +++ b/CRM/Activity/BAO/Activity.php @@ -2457,7 +2457,7 @@ AND cl.modified_id = c.id public static function getStatusesByType($type) { if (!isset(Civi::$statics[__CLASS__][__FUNCTION__])) { $statuses = civicrm_api3('OptionValue', 'get', array( - 'option_group_id' => "activity_status", + 'option_group_id' => 'activity_status', 'return' => array('value', 'name', 'filter'), 'options' => array('limit' => 0), )); diff --git a/api/v3/Activity.php b/api/v3/Activity.php index a2e4cb8007..1b675c265e 100644 --- a/api/v3/Activity.php +++ b/api/v3/Activity.php @@ -300,17 +300,9 @@ function _civicrm_api3_activity_get_spec(&$params) { * @throws \Civi\API\Exception\UnauthorizedException */ function civicrm_api3_activity_get($params) { - $options = _civicrm_api3_get_options_from_params($params, FALSE, 'Activity', 'get'); $sql = CRM_Utils_SQL_Select::fragment(); - $recordTypes = civicrm_api3('ActivityContact', 'getoptions', array('field' => 'record_type_id')); - $recordTypes = $recordTypes['values']; - $activityContactOptions = array( - 'contact_id' => NULL, - 'target_contact_id' => array_search('Activity Targets', $recordTypes), - 'source_contact_id' => array_search('Activity Source', $recordTypes), - 'assignee_contact_id' => array_search('Activity Assignees', $recordTypes), - ); + if (empty($params['target_contact_id']) && empty($params['source_contact_id']) && empty($params['assignee_contact_id']) && !empty($params['check_permissions']) && !CRM_Core_Permission::check('view all activities') @@ -321,6 +313,67 @@ function civicrm_api3_activity_get($params) { //$params['contact_id'] = array('IS NOT NULL' => TRUE); } + _civicrm_api3_activity_get_extraFilters($params, $sql); + + // Handle is_overdue sort + if (!empty($options['sort'])) { + $sort = explode(', ', $options['sort']); + + foreach ($sort as $index => &$sortString) { + // Get sort field and direction + list($sortField, $dir) = array_pad(explode(' ', $sortString), 2, 'ASC'); + if ($sortField == 'is_overdue') { + $incomplete = implode(',', array_keys(CRM_Activity_BAO_Activity::getStatusesByType(CRM_Activity_BAO_Activity::INCOMPLETE))); + $sql->orderBy("IF((a.activity_date_time >= NOW() OR a.status_id NOT IN ($incomplete)), 0, 1) $dir", NULL, $index); + // Replace the sort with a placeholder which will be ignored by sql + $sortString = '(1)'; + } + } + $params['options']['sort'] = implode(', ', $sort); + } + + // Ensure there's enough data for calculating is_overdue + if (!empty($options['return']['is_overdue']) && (empty($options['return']['status_id']) || empty($options['return']['activity_date_time']))) { + $options['return']['status_id'] = $options['return']['activity_date_time'] = 1; + $params['return'] = array_keys($options['return']); + } + + $activities = _civicrm_api3_basic_get(_civicrm_api3_get_BAO(__FUNCTION__), $params, FALSE, 'Activity', $sql); + if (!empty($params['check_permissions']) && !CRM_Core_Permission::check('view all activities')) { + // @todo get this to work at the query level - see contact_id join above. + foreach ($activities as $activity) { + if (!CRM_Activity_BAO_Activity::checkPermission($activity['id'], CRM_Core_Action::VIEW)) { + unset($activities[$activity['id']]); + } + } + } + if ($options['is_count']) { + return civicrm_api3_create_success($activities, $params, 'Activity', 'get'); + } + + $activities = _civicrm_api3_activity_get_formatResult($params, $activities, $options); + //legacy custom data get - so previous formatted response is still returned too + return civicrm_api3_create_success($activities, $params, 'Activity', 'get'); +} + +/** + * Support filters beyond what basic_get can do. + * + * @param array $params + * @param CRM_Utils_SQL_Select $sql + * @throws \CiviCRM_API3_Exception + * @throws \Exception + */ +function _civicrm_api3_activity_get_extraFilters(&$params, &$sql) { + // Filter by activity contacts + $recordTypes = civicrm_api3('ActivityContact', 'getoptions', array('field' => 'record_type_id')); + $recordTypes = $recordTypes['values']; + $activityContactOptions = array( + 'contact_id' => NULL, + 'target_contact_id' => array_search('Activity Targets', $recordTypes), + 'source_contact_id' => array_search('Activity Source', $recordTypes), + 'assignee_contact_id' => array_search('Activity Assignees', $recordTypes), + ); foreach ($activityContactOptions as $activityContactName => $activityContactValue) { if (!empty($params[$activityContactName])) { if (!is_array($params[$activityContactName])) { @@ -335,6 +388,7 @@ function civicrm_api3_activity_get($params) { } // Handle is_overdue filter + // Boolean calculated field - does not support operators if (isset($params['is_overdue'])) { $incomplete = implode(',', array_keys(CRM_Activity_BAO_Activity::getStatusesByType(CRM_Activity_BAO_Activity::INCOMPLETE))); if ($params['is_overdue']) { @@ -346,23 +400,6 @@ function civicrm_api3_activity_get($params) { } } - // Handle is_overdue sort - if (!empty($options['sort'])) { - $sort = explode(', ', $options['sort']); - - // For each one of our special fields we swap it for the placeholder (1) so it will be ignored by the case api. - foreach ($sort as $index => &$sortString) { - // Get sort field and direction - list($sortField, $dir) = array_pad(explode(' ', $sortString), 2, 'ASC'); - if ($sortField == 'is_overdue') { - $incomplete = implode(',', array_keys(CRM_Activity_BAO_Activity::getStatusesByType(CRM_Activity_BAO_Activity::INCOMPLETE))); - $sql->orderBy("IF((a.activity_date_time >= NOW() OR a.status_id NOT IN ($incomplete)), 0, 1) $dir", NULL, $index); - $sortString = '(1)'; - } - } - $params['options']['sort'] = implode(', ', $sort); - } - // Define how to handle filters on some related entities. // Subqueries are nice in (a) avoiding duplicates and (b) when the result // list is expected to be bite-sized. Joins are nice (a) with larger @@ -407,29 +444,6 @@ function civicrm_api3_activity_get($params) { } } } - - // Ensure there's enough data for calculating is_overdue - if (!empty($options['return']['is_overdue']) && (empty($options['return']['status_id']) || empty($options['return']['activity_date_time']))) { - $options['return']['status_id'] = $options['return']['activity_date_time'] = 1; - $params['return'] = array_keys($options['return']); - } - - $activities = _civicrm_api3_basic_get(_civicrm_api3_get_BAO(__FUNCTION__), $params, FALSE, 'Activity', $sql); - if (!empty($params['check_permissions']) && !CRM_Core_Permission::check('view all activities')) { - // @todo get this to work at the query level - see contact_id join above. - foreach ($activities as $activity) { - if (!CRM_Activity_BAO_Activity::checkPermission($activity['id'], CRM_Core_Action::VIEW)) { - unset($activities[$activity['id']]); - } - } - } - if ($options['is_count']) { - return civicrm_api3_create_success($activities, $params, 'Activity', 'get'); - } - - $activities = _civicrm_api3_activity_get_formatResult($params, $activities, $options); - //legacy custom data get - so previous formatted response is still returned too - return civicrm_api3_create_success($activities, $params, 'Activity', 'get'); } /** diff --git a/tests/phpunit/api/v3/ActivityTest.php b/tests/phpunit/api/v3/ActivityTest.php index bda00a1a7d..98b9b2f8bb 100644 --- a/tests/phpunit/api/v3/ActivityTest.php +++ b/tests/phpunit/api/v3/ActivityTest.php @@ -1374,6 +1374,9 @@ class api_v3_ActivityTest extends CiviUnitTestCase { } } + /** + * Test or operator in api params + */ public function testGetWithOr() { $acts = array( 'test or 1' => 'orOperator', @@ -1403,9 +1406,20 @@ class api_v3_ActivityTest extends CiviUnitTestCase { $this->assertEquals(3, $result['count']); } + /** + * Test handling of is_overdue calculated field + */ public function testGetOverdue() { - $overdueAct = $this->callAPISuccess('Activity', 'create', array('activity_date_time' => 'now - 1 week', 'status_id' => 'Scheduled') + $this->_params); - $completedAct = $this->callAPISuccess('Activity', 'create', array('activity_date_time' => 'now - 1 week', 'status_id' => 'Completed') + $this->_params); + $overdueAct = $this->callAPISuccess('Activity', 'create', array( + 'activity_date_time' => 'now - 1 week', + 'status_id' => 'Scheduled', + ) + $this->_params + ); + $completedAct = $this->callAPISuccess('Activity', 'create', array( + 'activity_date_time' => 'now - 1 week', + 'status_id' => 'Completed', + ) + $this->_params + ); $ids = array($overdueAct['id'], $completedAct['id']); // Test sorting -- 2.25.1