From 2d09a0c34e59d879a30239064365ce957b166420 Mon Sep 17 00:00:00 2001 From: eileen Date: Fri, 25 Sep 2020 14:46:31 +1200 Subject: [PATCH] [REF] Consolidate retrieval of searchFormValues Currently there are 3 ways in which the values from the searchForm are retrieved 1) the search form sets a value 'searchFormName' on the form during search which can be later retrieved 2) the search task has an action that reflects the form it came from 3) as per 2 but there is additional handling for the fact it might be the contact search (knowable because the entity has been set to 'Contact' I'm not 100% sure at this stage that the selected method (3) in this PR is the best - but it seems 'as good' as 1 with less form-state stuff and this PR consolidates it into 1 place so it is handled consistently --- CRM/Activity/Form/Task.php | 4 ++-- CRM/Case/Form/Search.php | 3 +++ CRM/Contact/Form/Search/Advanced.php | 3 +++ CRM/Contact/Form/Search/Basic.php | 3 +++ CRM/Contact/Form/Search/Builder.php | 3 +++ CRM/Contact/Form/Search/Custom.php | 3 +++ CRM/Contact/Form/Task.php | 2 +- CRM/Contribute/Form/Search.php | 3 +++ CRM/Contribute/Form/Task.php | 6 ++++-- CRM/Core/Form/Task.php | 7 +++++-- CRM/Event/Form/Search.php | 3 +++ CRM/Event/Form/Task.php | 4 ++-- CRM/Export/Form/Select.php | 19 +------------------ CRM/Grant/Form/Task.php | 6 ++++-- CRM/Mailing/Form/Task.php | 6 ++++-- CRM/Member/Form/Search.php | 3 +++ CRM/Member/Form/Task.php | 4 ++-- .../phpunit/CRM/Contribute/Form/TaskTest.php | 3 +-- 18 files changed, 50 insertions(+), 35 deletions(-) diff --git a/CRM/Activity/Form/Task.php b/CRM/Activity/Form/Task.php index 462499c38f..470e3692e8 100644 --- a/CRM/Activity/Form/Task.php +++ b/CRM/Activity/Form/Task.php @@ -38,14 +38,14 @@ class CRM_Activity_Form_Task extends CRM_Core_Form_Task { /** * Common pre-process function. * - * @param CRM_Core_Form $form + * @param \CRM_Core_Form_Task $form * * @throws \CRM_Core_Exception */ public static function preProcessCommon(&$form) { $form->_activityHolderIds = []; - $values = $form->controller->exportValues($form->get('searchFormName')); + $values = $form->getSearchFormValues(); $form->_task = $values['task']; $activityTasks = CRM_Activity_Task::tasks(); diff --git a/CRM/Case/Form/Search.php b/CRM/Case/Form/Search.php index b22ab8c08e..d3ba7a07c4 100644 --- a/CRM/Case/Form/Search.php +++ b/CRM/Case/Form/Search.php @@ -58,6 +58,9 @@ class CRM_Case_Form_Search extends CRM_Core_Form_Search { * Processing needed for buildForm and later. */ public function preProcess() { + // SearchFormName is deprecated & to be removed - the replacement is for the task to + // call $this->form->getSearchFormValues() + // A couple of extensions use it. $this->set('searchFormName', 'Search'); //check for civicase access. diff --git a/CRM/Contact/Form/Search/Advanced.php b/CRM/Contact/Form/Search/Advanced.php index e579c74331..95e981a63d 100644 --- a/CRM/Contact/Form/Search/Advanced.php +++ b/CRM/Contact/Form/Search/Advanced.php @@ -24,6 +24,9 @@ class CRM_Contact_Form_Search_Advanced extends CRM_Contact_Form_Search { * Processing needed for buildForm and later. */ public function preProcess() { + // SearchFormName is deprecated & to be removed - the replacement is for the task to + // call $this->form->getSearchFormValues() + // A couple of extensions use it. $this->set('searchFormName', 'Advanced'); parent::preProcess(); diff --git a/CRM/Contact/Form/Search/Basic.php b/CRM/Contact/Form/Search/Basic.php index c1e8438231..6deec9e2a4 100644 --- a/CRM/Contact/Form/Search/Basic.php +++ b/CRM/Contact/Form/Search/Basic.php @@ -117,6 +117,9 @@ class CRM_Contact_Form_Search_Basic extends CRM_Contact_Form_Search { * Processing needed for buildForm and later. */ public function preProcess() { + // SearchFormName is deprecated & to be removed - the replacement is for the task to + // call $this->form->getSearchFormValues() + // A couple of extensions use it. $this->set('searchFormName', 'Basic'); parent::preProcess(); diff --git a/CRM/Contact/Form/Search/Builder.php b/CRM/Contact/Form/Search/Builder.php index fc51f6580d..0eccbcb504 100644 --- a/CRM/Contact/Form/Search/Builder.php +++ b/CRM/Contact/Form/Search/Builder.php @@ -38,6 +38,9 @@ class CRM_Contact_Form_Search_Builder extends CRM_Contact_Form_Search { * Build the form object. */ public function preProcess() { + // SearchFormName is deprecated & to be removed - the replacement is for the task to + // call $this->form->getSearchFormValues() + // A couple of extensions use it. $this->set('searchFormName', 'Builder'); $this->set('context', 'builder'); diff --git a/CRM/Contact/Form/Search/Custom.php b/CRM/Contact/Form/Search/Custom.php index cfc78ef835..d2b4fd4cdd 100644 --- a/CRM/Contact/Form/Search/Custom.php +++ b/CRM/Contact/Form/Search/Custom.php @@ -19,6 +19,9 @@ class CRM_Contact_Form_Search_Custom extends CRM_Contact_Form_Search { protected $_customClass = NULL; public function preProcess() { + // SearchFormName is deprecated & to be removed - the replacement is for the task to + // call $this->form->getSearchFormValues() + // A couple of extensions use it. $this->set('searchFormName', 'Custom'); $this->set('context', 'custom'); diff --git a/CRM/Contact/Form/Task.php b/CRM/Contact/Form/Task.php index a7bd173701..8263094ea0 100644 --- a/CRM/Contact/Form/Task.php +++ b/CRM/Contact/Form/Task.php @@ -112,7 +112,7 @@ class CRM_Contact_Form_Task extends CRM_Core_Form_Task { $fragment .= '/custom'; } if (!$isStandAlone) { - self::$_searchFormValues = $form->getFormValues(); + self::$_searchFormValues = $form->getSearchFormValues(); } //set the user context for redirection of task actions diff --git a/CRM/Contribute/Form/Search.php b/CRM/Contribute/Form/Search.php index 341c54c868..866c178848 100644 --- a/CRM/Contribute/Form/Search.php +++ b/CRM/Contribute/Form/Search.php @@ -61,6 +61,9 @@ class CRM_Contribute_Form_Search extends CRM_Core_Form_Search { * @throws \CRM_Core_Exception */ public function preProcess() { + // SearchFormName is deprecated & to be removed - the replacement is for the task to + // call $this->form->getSearchFormValues() + // A couple of extensions use it. $this->set('searchFormName', 'Search'); $this->_actionButtonName = $this->getButtonName('next', 'action'); diff --git a/CRM/Contribute/Form/Task.php b/CRM/Contribute/Form/Task.php index 16d6116667..ce5b071fa9 100644 --- a/CRM/Contribute/Form/Task.php +++ b/CRM/Contribute/Form/Task.php @@ -50,12 +50,14 @@ class CRM_Contribute_Form_Task extends CRM_Core_Form_Task { } /** - * @param CRM_Core_Form $form + * @param \CRM_Core_Form_Task $form + * + * @throws \CRM_Core_Exception */ public static function preProcessCommon(&$form) { $form->_contributionIds = []; - $values = $form->controller->exportValues($form->get('searchFormName')); + $values = $form->getSearchFormValues(); $form->_task = $values['task'] ?? NULL; $contributeTasks = CRM_Contribute_Task::tasks(); diff --git a/CRM/Core/Form/Task.php b/CRM/Core/Form/Task.php index a3bde1d8b6..65a105d387 100644 --- a/CRM/Core/Form/Task.php +++ b/CRM/Core/Form/Task.php @@ -92,7 +92,7 @@ abstract class CRM_Core_Form_Task extends CRM_Core_Form { public static function preProcessCommon(&$form) { $form->_entityIds = []; - $searchFormValues = $form->controller->exportValues($form->get('searchFormName')); + $searchFormValues = $form->getSearchFormValues(); $form->_task = $searchFormValues['task']; $className = 'CRM_' . ucfirst($form::$entityShortname) . '_Task'; @@ -256,7 +256,7 @@ SELECT contact_id * * @return array */ - public function getFormValues() { + public function getSearchFormValues() { if ($this->_action === CRM_Core_Action::ADVANCED) { return $this->controller->exportValues('Advanced'); } @@ -266,6 +266,9 @@ SELECT contact_id if ($this->_action == CRM_Core_Action::COPY) { return $this->controller->exportValues('Custom'); } + if ($this->get('entity') !== 'Contact') { + return $this->controller->exportValues('Search'); + } return $this->controller->exportValues('Basic'); } diff --git a/CRM/Event/Form/Search.php b/CRM/Event/Form/Search.php index 3000110687..ac4404ca6d 100644 --- a/CRM/Event/Form/Search.php +++ b/CRM/Event/Form/Search.php @@ -79,6 +79,9 @@ class CRM_Event_Form_Search extends CRM_Core_Form_Search { * @throws \CiviCRM_API3_Exception */ public function preProcess() { + // SearchFormName is deprecated & to be removed - the replacement is for the task to + // call $this->form->getSearchFormValues() + // A couple of extensions use it. $this->set('searchFormName', 'Search'); /** diff --git a/CRM/Event/Form/Task.php b/CRM/Event/Form/Task.php index 18962108a6..ddb8953815 100644 --- a/CRM/Event/Form/Task.php +++ b/CRM/Event/Form/Task.php @@ -40,12 +40,12 @@ class CRM_Event_Form_Task extends CRM_Core_Form_Task { } /** - * @param CRM_Core_Form $form + * @param CRM_Core_Form_Task $form */ public static function preProcessCommon(&$form) { $form->_participantIds = []; - $values = $form->controller->exportValues($form->get('searchFormName')); + $values = $form->getSearchFormValues(); $form->_task = $values['task']; $tasks = CRM_Event_Task::permissionedTaskTitles(CRM_Core_Permission::getPermission()); diff --git a/CRM/Export/Form/Select.php b/CRM/Export/Form/Select.php index abc1f3b2de..4772536ab7 100644 --- a/CRM/Export/Form/Select.php +++ b/CRM/Export/Form/Select.php @@ -108,24 +108,7 @@ class CRM_Export_Form_Select extends CRM_Core_Form_Task { $this::$tableName = CRM_Core_DAO_AllCoreTables::getTableForClass(CRM_Core_DAO_AllCoreTables::getFullName($this->getDAOName())); } - // get the submitted values based on search - if ($this->_action == CRM_Core_Action::ADVANCED) { - $values = $this->controller->exportValues('Advanced'); - } - elseif ($this->_action == CRM_Core_Action::PROFILE) { - $values = $this->controller->exportValues('Builder'); - } - elseif ($this->_action == CRM_Core_Action::COPY) { - $values = $this->controller->exportValues('Custom'); - } - else { - if ($entityShortname !== 'Contact') { - $values = $this->controller->exportValues('Search'); - } - else { - $values = $this->controller->exportValues('Basic'); - } - } + $values = $this->getSearchFormValues(); $count = 0; $this->_matchingContacts = FALSE; diff --git a/CRM/Grant/Form/Task.php b/CRM/Grant/Form/Task.php index b983cec3d7..3917ac01f8 100644 --- a/CRM/Grant/Form/Task.php +++ b/CRM/Grant/Form/Task.php @@ -34,12 +34,14 @@ class CRM_Grant_Form_Task extends CRM_Core_Form_Task { } /** - * @param CRM_Core_Form $form + * @param \CRM_Core_Form_Task $form + * + * @throws \CRM_Core_Exception */ public static function preProcessCommon(&$form) { $form->_grantIds = []; - $values = $form->controller->exportValues($form->get('searchFormName')); + $values = $form->getSearchFormValues(); $form->_task = $values['task']; $tasks = CRM_Grant_Task::tasks(); diff --git a/CRM/Mailing/Form/Task.php b/CRM/Mailing/Form/Task.php index 4a84746aaa..f7f37eee39 100644 --- a/CRM/Mailing/Form/Task.php +++ b/CRM/Mailing/Form/Task.php @@ -29,10 +29,12 @@ class CRM_Mailing_Form_Task extends CRM_Core_Form_Task { } /** - * @param CRM_Core_Form $form + * @param \CRM_Core_Form_Task $form + * + * @throws \CRM_Core_Exception */ public static function preProcessCommon(&$form) { - $values = $form->controller->exportValues($form->get('searchFormName')); + $values = $form->getSearchFormValues(); $form->_task = $values['task'] ?? NULL; $mailingTasks = CRM_Mailing_Task::tasks(); diff --git a/CRM/Member/Form/Search.php b/CRM/Member/Form/Search.php index fc0ba94de0..dd937142ff 100644 --- a/CRM/Member/Form/Search.php +++ b/CRM/Member/Form/Search.php @@ -63,6 +63,9 @@ class CRM_Member_Form_Search extends CRM_Core_Form_Search { * @throws \CiviCRM_API3_Exception */ public function preProcess() { + // SearchFormName is deprecated & to be removed - the replacement is for the task to + // call $this->form->getSearchFormValues() + // A couple of extensions use it. $this->set('searchFormName', 'Search'); $this->_actionButtonName = $this->getButtonName('next', 'action'); diff --git a/CRM/Member/Form/Task.php b/CRM/Member/Form/Task.php index 5f5791d01a..86eda194dd 100644 --- a/CRM/Member/Form/Task.php +++ b/CRM/Member/Form/Task.php @@ -41,14 +41,14 @@ class CRM_Member_Form_Task extends CRM_Core_Form_Task { } /** - * @param CRM_Core_Form $form + * @param \CRM_Core_Form_Task $form * * @throws \CRM_Core_Exception */ public static function preProcessCommon(&$form) { $form->_memberIds = []; - $values = $form->controller->exportValues($form->get('searchFormName')); + $values = $form->getSearchFormValues(); $form->_task = $values['task']; $tasks = CRM_Member_Task::permissionedTaskTitles(CRM_Core_Permission::getPermission()); diff --git a/tests/phpunit/CRM/Contribute/Form/TaskTest.php b/tests/phpunit/CRM/Contribute/Form/TaskTest.php index acd140f5bb..51605fe923 100644 --- a/tests/phpunit/CRM/Contribute/Form/TaskTest.php +++ b/tests/phpunit/CRM/Contribute/Form/TaskTest.php @@ -66,8 +66,7 @@ class CRM_Contribute_Form_TaskTest extends CiviUnitTestCase { } // Assert contribIds are returned in a sorted order. - $form = new CRM_Contribute_Form_Task(); - $form->controller = new CRM_Core_Controller(); + $form = $this->getFormObject('CRM_Contribute_Form_Task', [], 'Search'); foreach ($fields as $val) { $form->set(CRM_Utils_Sort::SORT_ORDER, "`{$val}` asc"); CRM_Contribute_Form_Task::preProcessCommon($form); -- 2.25.1