From: eileen Date: Fri, 8 Nov 2019 21:42:00 +0000 (+1300) Subject: dev/core#1374 Fix search formValue handling on contribution search X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=e9f51713e9913ee2f5dd5f3f65af6ff027af4cca;p=civicrm-core.git dev/core#1374 Fix search formValue handling on contribution search This fixes 2 issues 1) Search params being lost when editing a related entity per https://lab.civicrm.org/dev/core/issues/1374 2) force=1&sort_name=p not working in contribution search url In digging I concluded the problem is we have 3 underlying arrays which we keep jumbling together 1) formValues - the actual submitted values, augmented by any url passed params 2) the default values - values to load by default on the form 3) our working query params - a copy of formValues that we have prepared for the query We need to stop mangling them. I added subtle code comments --- diff --git a/CRM/Contact/Form/Search/Advanced.php b/CRM/Contact/Form/Search/Advanced.php index a3f46e68de..d2d2963425 100644 --- a/CRM/Contact/Form/Search/Advanced.php +++ b/CRM/Contact/Form/Search/Advanced.php @@ -201,7 +201,7 @@ class CRM_Contact_Form_Search_Advanced extends CRM_Contact_Form_Search { $this->_ssID = $this->get('ssID'); } - $defaults = array_merge($this->_formValues, [ + $defaults = array_merge((array) $this->_formValues, [ 'privacy_toggle' => 1, 'operator' => 'AND', ], $defaults); diff --git a/CRM/Contribute/Form/Search.php b/CRM/Contribute/Form/Search.php index 6923169fe6..1e967dca2f 100644 --- a/CRM/Contribute/Form/Search.php +++ b/CRM/Contribute/Form/Search.php @@ -84,9 +84,7 @@ class CRM_Contribute_Form_Search extends CRM_Core_Form_Search { $this->_done = FALSE; - $this->loadStandardSearchOptionsFromUrl(); - - $this->_formValues = $this->getFormValues(); + parent::preProcess(); //membership ID $memberShipId = CRM_Utils_Request::retrieve('memberId', 'Positive', $this); @@ -98,10 +96,6 @@ class CRM_Contribute_Form_Search extends CRM_Core_Form_Search { $this->_formValues['contribution_participant_id'] = $participantId; } - if ($this->_force) { - $this->handleForcedSearch(); - } - $sortID = NULL; if ($this->get(CRM_Utils_Sort::SORT_ID)) { $sortID = CRM_Utils_Sort::sortIDValue($this->get(CRM_Utils_Sort::SORT_ID), @@ -277,10 +271,12 @@ class CRM_Contribute_Form_Search extends CRM_Core_Form_Search { $this->_done = TRUE; $this->setFormValues(); + // @todo - stop changing formValues - respect submitted form values, change a working array. $this->fixFormValues(); // We don't show test records in summaries or dashboards if (empty($this->_formValues['contribution_test']) && $this->_force && !empty($this->_context) && $this->_context == 'dashboard') { + // @todo - stop changing formValues - respect submitted form values, change a working array. $this->_formValues["contribution_test"] = 0; } @@ -289,11 +285,11 @@ class CRM_Contribute_Form_Search extends CRM_Core_Form_Search { 'contribution_amount_high', ] as $f) { if (isset($this->_formValues[$f])) { + // @todo - stop changing formValues - respect submitted form values, change a working array. $this->_formValues[$f] = CRM_Utils_Rule::cleanMoney($this->_formValues[$f]); } } - $config = CRM_Core_Config::singleton(); if (!empty($_POST)) { $specialParams = [ 'financial_type_id', @@ -306,10 +302,13 @@ class CRM_Contribute_Form_Search extends CRM_Core_Form_Search { 'payment_instrument_id', 'contribution_batch_id', ]; + // @todo - stop changing formValues - respect submitted form values, change a working array. CRM_Contact_BAO_Query::processSpecialFormValue($this->_formValues, $specialParams); + // @todo - stop changing formValues - respect submitted form values, change a working array. $tags = CRM_Utils_Array::value('contact_tags', $this->_formValues); if ($tags && !is_array($tags)) { + // @todo - stop changing formValues - respect submitted form values, change a working array. unset($this->_formValues['contact_tags']); $this->_formValues['contact_tags'][$tags] = 1; } @@ -317,12 +316,14 @@ class CRM_Contribute_Form_Search extends CRM_Core_Form_Search { if ($tags && is_array($tags)) { unset($this->_formValues['contact_tags']); foreach ($tags as $notImportant => $tagID) { + // @todo - stop changing formValues - respect submitted form values, change a working array. $this->_formValues['contact_tags'][$tagID] = 1; } } $group = CRM_Utils_Array::value('group', $this->_formValues); if ($group && !is_array($group)) { + // @todo - stop changing formValues - respect submitted form values, change a working array. unset($this->_formValues['group']); $this->_formValues['group'][$group] = 1; } @@ -330,16 +331,18 @@ class CRM_Contribute_Form_Search extends CRM_Core_Form_Search { if ($group && is_array($group)) { unset($this->_formValues['group']); foreach ($group as $groupID) { + // @todo - stop changing formValues - respect submitted form values, change a working array. $this->_formValues['group'][$groupID] = 1; } } } + // @todo - stop changing formValues - respect submitted form values, change a working array. CRM_Core_BAO_CustomValue::fixCustomFieldValue($this->_formValues); + // @todo - stop changing formValues - respect submitted form values, change a working array. $this->_queryParams = CRM_Contact_BAO_Query::convertFormValues($this->_formValues); - $this->set('formValues', $this->_formValues); $this->set('queryParams', $this->_queryParams); $buttonName = $this->controller->getButtonName(); @@ -360,6 +363,7 @@ class CRM_Contribute_Form_Search extends CRM_Core_Form_Search { ); } + // @todo - stop changing formValues - respect submitted form values, change a working array. $this->_queryParams = CRM_Contact_BAO_Query::convertFormValues($this->_formValues); $selector = new CRM_Contribute_Selector_Search($this->_queryParams, $this->_action, @@ -456,9 +460,6 @@ class CRM_Contribute_Form_Search extends CRM_Core_Form_Search { if ($contribPageId) { $this->_formValues['contribution_page_id'] = $contribPageId; } - - //give values to default. - $this->_defaults = $this->_formValues; } /** diff --git a/CRM/Core/Form/Search.php b/CRM/Core/Form/Search.php index c698c6d785..b03bbc46b6 100644 --- a/CRM/Core/Form/Search.php +++ b/CRM/Core/Form/Search.php @@ -124,6 +124,20 @@ class CRM_Core_Form_Search extends CRM_Core_Form { $this->searchFieldMetadata = array_merge($this->searchFieldMetadata, $searchFieldMetadata); } + /** + * Prepare for search by loading options from the url, handling force searches, retrieving form values. + * + * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception + */ + public function preProcess() { + $this->loadStandardSearchOptionsFromUrl(); + if ($this->_force) { + $this->handleForcedSearch(); + } + $this->_formValues = $this->getFormValues(); + } + /** * This virtual function is used to set the default values of various form elements. * @@ -132,7 +146,10 @@ class CRM_Core_Form_Search extends CRM_Core_Form { * @throws \CRM_Core_Exception */ public function setDefaultValues() { - $defaults = (array) $this->_formValues; + // Use the form values stored to the form. Ideally 'formValues' + // would remain 'pure' & another array would be wrangled. + // We don't do that - so we want the version of formValues stored early on. + $defaults = (array) $this->get('formValues'); foreach (array_keys($this->getSearchFieldMetadata()) as $entity) { $defaults = array_merge($this->getEntityDefaults($entity), $defaults); } @@ -146,6 +163,7 @@ class CRM_Core_Form_Search extends CRM_Core_Form { */ protected function setFormValues() { $this->_formValues = $this->getFormValues(); + $this->set('formValues', $this->_formValues); $this->convertTextStringsToUseLikeOperator(); } @@ -508,6 +526,7 @@ class CRM_Core_Form_Search extends CRM_Core_Form { */ protected function handleForcedSearch() { $this->setSearchMetadata(); + $this->addContactSearchFields(); $this->postProcess(); $this->set('force', 0); } diff --git a/tests/phpunit/CRM/Contact/BAO/SavedSearchTest.php b/tests/phpunit/CRM/Contact/BAO/SavedSearchTest.php index b939d85bac..f6df955857 100644 --- a/tests/phpunit/CRM/Contact/BAO/SavedSearchTest.php +++ b/tests/phpunit/CRM/Contact/BAO/SavedSearchTest.php @@ -58,11 +58,13 @@ class CRM_Contact_BAO_SavedSearchTest extends CiviUnitTestCase { /** * Test setDefaults for privacy radio buttons. + * + * @throws \Exception */ public function testDefaultValues() { $sg = new CRM_Contact_Form_Search_Advanced(); $sg->controller = new CRM_Core_Controller(); - $sg->_formValues = [ + $formValues = [ 'group_search_selected' => 'group', 'privacy_options' => ['do_not_email'], 'privacy_operator' => 'OR', @@ -71,14 +73,15 @@ class CRM_Contact_BAO_SavedSearchTest extends CiviUnitTestCase { 'component_mode' => 1, ]; CRM_Core_DAO::executeQuery( - "INSERT INTO civicrm_saved_search (form_values) VALUES('" . serialize($sg->_formValues) . "')" + "INSERT INTO civicrm_saved_search (form_values) VALUES('" . serialize($formValues) . "')" ); $ssID = CRM_Core_DAO::singleValueQuery('SELECT LAST_INSERT_ID()'); $sg->set('ssID', $ssID); + $sg->set('formValues', $formValues); $defaults = $sg->setDefaultValues(); - $this->checkArrayEquals($defaults, $sg->_formValues); + $this->checkArrayEquals($defaults, $formValues); } /**