Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CRM-21720 Cleanup search classes to use enumerators instead of hardcoded values #11600

Merged
merged 1 commit into from
Mar 11, 2018
Merged

CRM-21720 Cleanup search classes to use enumerators instead of hardcoded values #11600

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion CRM/Contact/BAO/Query.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ class CRM_Contact_BAO_Query {
// There is no 4,
MODE_MEMBER = 8,
MODE_EVENT = 16,
// No 32, no 64.
MODE_CONTACTSRELATED = 32,
// no 64.
MODE_GRANT = 128,
MODE_PLEDGEBANK = 256,
MODE_PLEDGE = 512,
Expand All @@ -67,6 +68,13 @@ class CRM_Contact_BAO_Query {
MODE_MAILING = 16384,
MODE_ALL = 17407;

/**
* Constants for search operators
*/
const
SEARCH_OPERATOR_AND = 'AND',
SEARCH_OPERATOR_OR = 'OR';

/**
* The default set of return properties.
*
Expand Down
99 changes: 44 additions & 55 deletions CRM/Contact/Form/Search.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ public static function isSearchContext($context) {
public static function setModeValues() {
if (!self::$_modeValues) {
self::$_modeValues = array(
1 => array(
CRM_Contact_BAO_Query::MODE_CONTACTS => array(
'selectorName' => self::$_selectorName,
'selectorLabel' => ts('Contacts'),
'taskFile' => 'CRM/Contact/Form/Search/ResultTasks.tpl',
Expand All @@ -210,7 +210,7 @@ public static function setModeValues() {
'resultContext' => NULL,
'taskClassName' => 'CRM_Contact_Task',
),
2 => array(
CRM_Contact_BAO_Query::MODE_CONTRIBUTE => array(
'selectorName' => 'CRM_Contribute_Selector_Search',
'selectorLabel' => ts('Contributions'),
'taskFile' => 'CRM/common/searchResultTasks.tpl',
Expand All @@ -219,7 +219,7 @@ public static function setModeValues() {
'resultContext' => 'Search',
'taskClassName' => 'CRM_Contribute_Task',
),
3 => array(
CRM_Contact_BAO_Query::MODE_EVENT => array(
'selectorName' => 'CRM_Event_Selector_Search',
'selectorLabel' => ts('Event Participants'),
'taskFile' => 'CRM/common/searchResultTasks.tpl',
Expand All @@ -228,7 +228,7 @@ public static function setModeValues() {
'resultContext' => 'Search',
'taskClassName' => 'CRM_Event_Task',
),
4 => array(
CRM_Contact_BAO_Query::MODE_ACTIVITY => array(
'selectorName' => 'CRM_Activity_Selector_Search',
'selectorLabel' => ts('Activities'),
'taskFile' => 'CRM/common/searchResultTasks.tpl',
Expand All @@ -237,7 +237,7 @@ public static function setModeValues() {
'resultContext' => 'Search',
'taskClassName' => 'CRM_Activity_Task',
),
5 => array(
CRM_Contact_BAO_Query::MODE_MEMBER => array(
'selectorName' => 'CRM_Member_Selector_Search',
'selectorLabel' => ts('Memberships'),
'taskFile' => "CRM/common/searchResultTasks.tpl",
Expand All @@ -246,7 +246,7 @@ public static function setModeValues() {
'resultContext' => 'Search',
'taskClassName' => 'CRM_Member_Task',
),
6 => array(
CRM_Contact_BAO_Query::MODE_CASE => array(
'selectorName' => 'CRM_Case_Selector_Search',
'selectorLabel' => ts('Cases'),
'taskFile' => "CRM/common/searchResultTasks.tpl",
Expand All @@ -255,7 +255,7 @@ public static function setModeValues() {
'resultContext' => 'Search',
'taskClassName' => 'CRM_Case_Task',
),
7 => array(
CRM_Contact_BAO_Query::MODE_CONTACTSRELATED => array(
'selectorName' => self::$_selectorName,
'selectorLabel' => ts('Related Contacts'),
'taskFile' => 'CRM/Contact/Form/Search/ResultTasks.tpl',
Expand All @@ -264,7 +264,7 @@ public static function setModeValues() {
'resultContext' => NULL,
'taskClassName' => 'CRM_Contact_Task',
),
8 => array(
CRM_Contact_BAO_Query::MODE_MAILING => array(
'selectorName' => 'CRM_Mailing_Selector_Search',
'selectorLabel' => ts('Mailings'),
'taskFile' => "CRM/common/searchResultTasks.tpl",
Expand All @@ -282,11 +282,11 @@ public static function setModeValues() {
*
* @return mixed
*/
public static function getModeValue($mode = 1) {
public static function getModeValue($mode = CRM_Contact_BAO_Query::MODE_CONTACTS) {
self::setModeValues();

if (!array_key_exists($mode, self::$_modeValues)) {
$mode = 1;
$mode = CRM_Contact_BAO_Query::MODE_CONTACTS;
}

return self::$_modeValues[$mode];
Expand All @@ -298,25 +298,36 @@ public static function getModeValue($mode = 1) {
public static function getModeSelect() {
self::setModeValues();

$select = array();
$componentModes = array();
foreach (self::$_modeValues as $id => & $value) {
$select[$id] = $value['selectorLabel'];
$componentModes[$id] = $value['selectorLabel'];
}

// unset contributions or participants if user does not have
// permission on them
$enabledComponents = CRM_Core_Component::getEnabledComponents();

// unset disabled components
if (!array_key_exists('CiviMail', $enabledComponents)) {
unset($componentModes[CRM_Contact_BAO_Query::MODE_MAILING]);
}

// unset contributions or participants if user does not have permission on them
if (!CRM_Core_Permission::access('CiviContribute')) {
unset($select['2']);
unset($componentModes[CRM_Contact_BAO_Query::MODE_CONTRIBUTE]);
}

if (!CRM_Core_Permission::access('CiviEvent')) {
unset($select['3']);
unset($componentModes[CRM_Contact_BAO_Query::MODE_EVENT]);
}

if (!CRM_Core_Permission::access('CiviMember')) {
unset($componentModes[CRM_Contact_BAO_Query::MODE_MEMBER]);
}

if (!CRM_Core_Permission::check('view all activities')) {
unset($select['4']);
unset($componentModes[CRM_Contact_BAO_Query::MODE_ACTIVITY]);
}
return $select;

return $componentModes;
}

/**
Expand All @@ -325,26 +336,17 @@ public static function getModeSelect() {
* @return array
*/
public function buildTaskList() {
// amtg = 'Add members to group'
if ($this->_context !== 'amtg') {
$permission = CRM_Core_Permission::getPermission();

if ($this->_componentMode == 1 || $this->_componentMode == 7) {
$this->_taskList += CRM_Contact_Task::permissionedTaskTitles($permission,
CRM_Utils_Array::value('deleted_contacts', $this->_formValues)
);
}
else {
$className = $this->_modeValue['taskClassName'];
$this->_taskList += $className::permissionedTaskTitles($permission, FALSE);
}

// Only offer the "Update Smart Group" task if a smart group/saved search is already in play
if (isset($this->_ssID) && $permission == CRM_Core_Permission::EDIT) {
$this->_taskList += CRM_Contact_Task::optionalTaskTitle();
$taskParams['deletedContacts'] = FALSE;
if ($this->_componentMode == CRM_Contact_BAO_Query::MODE_CONTACTS || $this->_componentMode == CRM_Contact_BAO_Query::MODE_CONTACTSRELATED) {
$taskParams['deletedContacts'] = CRM_Utils_Array::value('deleted_contacts', $this->_formValues);
}
$className = $this->_modeValue['taskClassName'];
$taskParams['ssID'] = isset($this->_ssID) ? $this->_ssID : NULL;
$this->_taskList += $className::permissionedTaskTitles(CRM_Core_Permission::getPermission(), $taskParams);
}

asort($this->_taskList);
return $this->_taskList;
}

Expand All @@ -357,24 +359,11 @@ public function buildQuickForm() {
// jsTree is needed for tags popup
->addScriptFile('civicrm', 'packages/jquery/plugins/jstree/jquery.jstree.js', 0, 'html-header', FALSE)
->addStyleFile('civicrm', 'packages/jquery/plugins/jstree/themes/default/style.css', 0, 'html-header');
$permission = CRM_Core_Permission::getPermission();

// some tasks.. what do we want to do with the selected contacts ?
$tasks = array();
if ($this->_componentMode == 1 || $this->_componentMode == 7) {
$tasks += CRM_Contact_Task::permissionedTaskTitles($permission,
CRM_Utils_Array::value('deleted_contacts', $this->_formValues)
);
}
else {
$className = $this->_modeValue['taskClassName'];
$tasks += $className::permissionedTaskTitles($permission, FALSE);
}
$this->_taskList = $this->buildTaskList();

if (isset($this->_ssID)) {
if ($permission == CRM_Core_Permission::EDIT) {
$tasks = $tasks + CRM_Contact_Task::optionalTaskTitle();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are still 2 places where optionalTaskTitle() is referred to that can go

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think those are removed via #11764

}

$search_custom_id
= CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_SavedSearch', $this->_ssID, 'search_custom_id');

Expand Down Expand Up @@ -464,7 +453,7 @@ public function buildQuickForm() {
'class' => 'crm-form-submit',
)
);
$this->add('hidden', 'task', CRM_Contact_Task::GROUP_CONTACTS);
$this->add('hidden', 'task', CRM_Contact_Task::GROUP_ADD);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still reading but GROUP_ADD doesn't exist on my version that I can see

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It exists once #11240 is merged

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is now in CRM/Core/Task.php which is part of core.

$selectedRowsRadio = $this->addElement('radio', 'radio_ts', NULL, '', 'ts_sel', array('checked' => 'checked'));
$allRowsRadio = $this->addElement('radio', 'radio_ts', NULL, '', 'ts_all');
$this->assign('ts_sel_id', $selectedRowsRadio->_attributes['id']);
Expand All @@ -474,7 +463,7 @@ public function buildQuickForm() {
$selectedContactIds = array();
$qfKeyParam = CRM_Utils_Array::value('qfKey', $this->_formValues);
// We use ajax to handle selections only if the search results component_mode is set to "contacts"
if ($qfKeyParam && ($this->get('component_mode') <= 1 || $this->get('component_mode') == 7)) {
if ($qfKeyParam && ($this->get('component_mode') <= CRM_Contact_BAO_Query::MODE_CONTACTS || $this->get('component_mode') == CRM_Contact_BAO_Query::MODE_CONTACTSRELATED)) {
$this->addClass('crm-ajax-selection-form');
$qfKeyParam = "civicrm search {$qfKeyParam}";
$selectedContactIdsArr = CRM_Core_BAO_PrevNextCache::getSelection($qfKeyParam);
Expand Down Expand Up @@ -515,8 +504,8 @@ public function preProcess() {
$this->_ssID = CRM_Utils_Request::retrieve('ssID', 'Positive', $this);
$this->_sortByCharacter = CRM_Utils_Request::retrieve('sortByCharacter', 'String', $this);
$this->_ufGroupID = CRM_Utils_Request::retrieve('id', 'Positive', $this);
$this->_componentMode = CRM_Utils_Request::retrieve('component_mode', 'Positive', $this, FALSE, 1, $_REQUEST);
$this->_operator = CRM_Utils_Request::retrieve('operator', 'String', $this, FALSE, 1, $_REQUEST, 'AND');
$this->_componentMode = CRM_Utils_Request::retrieve('component_mode', 'Positive', $this, FALSE, CRM_Contact_BAO_Query::MODE_CONTACTS, $_REQUEST);
$this->_operator = CRM_Utils_Request::retrieve('operator', 'String', $this, FALSE, CRM_Contact_BAO_Query::SEARCH_OPERATOR_AND, 'REQUEST');

/**
* set the button names
Expand Down Expand Up @@ -648,9 +637,9 @@ public function preProcess() {
$this->assign('id',
CRM_Utils_Array::value('uf_group_id', $this->_formValues)
);
$operator = CRM_Utils_Array::value('operator', $this->_formValues, 'AND');
$operator = CRM_Utils_Array::value('operator', $this->_formValues, CRM_Contact_BAO_Query::SEARCH_OPERATOR_AND);
$this->set('queryOperator', $operator);
if ($operator == 'OR') {
if ($operator == CRM_Contact_BAO_Query::SEARCH_OPERATOR_OR) {
$this->assign('operator', ts('OR'));
}
else {
Expand Down
2 changes: 1 addition & 1 deletion CRM/Contact/Form/Search/Advanced.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ public function setDefaultValues() {
$this->normalizeDefaultValues($defaults);

if ($this->_context === 'amtg') {
$defaults['task'] = CRM_Contact_Task::GROUP_CONTACTS;
$defaults['task'] = CRM_Contact_Task::GROUP_ADD;
}

return $defaults;
Expand Down
2 changes: 1 addition & 1 deletion CRM/Contact/Form/Search/Basic.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public function setDefaultValues() {
}

if ($this->_context === 'amtg') {
$defaults['task'] = CRM_Contact_Task::GROUP_CONTACTS;
$defaults['task'] = CRM_Contact_Task::GROUP_ADD;
}

if ($this->_context === 'smog') {
Expand Down
30 changes: 2 additions & 28 deletions CRM/Contact/Form/Search/Criteria.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,32 +158,6 @@ public static function basic(&$form) {
);

$componentModes = CRM_Contact_Form_Search::getModeSelect();
$enabledComponents = CRM_Core_Component::getEnabledComponents();

// unset disabled components that must should have been enabled
// to the option be viable
if (!array_key_exists('CiviMail', $enabledComponents)) {
unset($componentModes['8']);
}

// unset contributions or participants if user does not have
// permission on them
if (!CRM_Core_Permission::access('CiviContribute')) {
unset($componentModes['2']);
}

if (!CRM_Core_Permission::access('CiviEvent')) {
unset($componentModes['3']);
}

if (!CRM_Core_Permission::access('CiviMember')) {
unset($componentModes['5']);
}

if (!CRM_Core_Permission::check('view all activities')) {
unset($componentModes['4']);
}

if (count($componentModes) > 1) {
$form->add('select',
'component_mode',
Expand All @@ -198,8 +172,8 @@ public static function basic(&$form) {
'operator',
ts('Search Operator'),
array(
'AND' => ts('AND'),
'OR' => ts('OR'),
CRM_Contact_BAO_Query::SEARCH_OPERATOR_AND => ts('AND'),
CRM_Contact_BAO_Query::SEARCH_OPERATOR_OR => ts('OR'),
),
array('allowClear' => FALSE)
);
Expand Down
14 changes: 7 additions & 7 deletions CRM/Contact/Form/Task/SaveSearch.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,12 @@ public function preProcess() {
$values = $this->controller->exportValues('Basic');
}

// Get Task name
$modeValue = CRM_Contact_Form_Search::getModeValue($values['component_mode']);
$className = $modeValue['taskClassName'];
$taskList = $className::taskTitles();
$this->_task = CRM_Utils_Array::value('task', $values);
$crmContactTaskTasks = CRM_Contact_Task::taskTitles();
$this->assign('taskName', CRM_Utils_Array::value($this->_task, $crmContactTaskTasks));
$this->assign('taskName', CRM_Utils_Array::value($this->_task, $taskList));
}

/**
Expand Down Expand Up @@ -208,12 +211,9 @@ public function postProcess() {
$params = array();
$params['title'] = $formValues['title'];
$params['description'] = $formValues['description'];
if (isset($formValues['group_type']) &&
is_array($formValues['group_type'])
) {
if (isset($formValues['group_type']) && is_array($formValues['group_type']) && count($formValues['group_type'])) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is an unrelated change. It makes sense though from what I can see - if there is no group_type but it is set & is an array save as 'no group type'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it fixes a PHP notice as the code was not checking for an empty array, but looking at it I believe it intended to as it doesn't make sense otherwise. It's pulling a group type from the form into a variable, if there isn't one it can't do that so sets an empty string in the else below.

$params['group_type'] = CRM_Core_DAO::VALUE_SEPARATOR . implode(CRM_Core_DAO::VALUE_SEPARATOR,
array_keys($formValues['group_type'])
) . CRM_Core_DAO::VALUE_SEPARATOR;
array_keys($formValues['group_type'])) . CRM_Core_DAO::VALUE_SEPARATOR;
}
else {
$params['group_type'] = '';
Expand Down
13 changes: 4 additions & 9 deletions CRM/Contact/StateMachine/Search.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,15 +103,10 @@ public function taskName($controller, $formName = 'Search') {
}
$this->_controller->set('task', $value);

if ($value) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any idea how to get to this line without $value to test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was causing a bug in some situations, as it was falling back to the contact task and would try and return the default task for contact tasks no matter what context we were in (eg. in member context). I think you could trigger it by editing a smart search and changing the "View results as" to be something other than contacts. But it should not be possible to get there with no value for $value unless you are doing something wierd and it doesn't make sense (in my opinion) to fall back to the contact task handler, as it defines a different set of tasks which are potentially not applicable to the current context.

By removing the check for $value we are always calling the relevant getTask() handler for the context.

$componentMode = $this->_controller->get('component_mode');
$modeValue = CRM_Contact_Form_Search::getModeValue($componentMode);
$taskClassName = $modeValue['taskClassName'];
return $taskClassName::getTask($value);
}
else {
return CRM_Contact_Task::getTask($value);
}
$componentMode = $this->_controller->get('component_mode');
$modeValue = CRM_Contact_Form_Search::getModeValue($componentMode);
$taskClassName = $modeValue['taskClassName'];
return $taskClassName::getTask($value);
}

/**
Expand Down