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 a3f46e68de23c863c65d0926baf6c35ba2b50314..d2d29634255658f18541c6655b31454902f753fb 100644 (file)
--- 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 6923169fe6b902cc1ba28a44347e1ea7cdcdd59d..1e967dca2f2e8b7f66801da8e6e0d6715017d1d1 100644 (file)
--- 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 c698c6d78555e9410c2a38346299a896eb1dd8d0..b03bbc46b6eecb70fad0077082590c7df76b96be 100644 (file)
--- 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 b939d85bac89800f3667ff19e3891911a3a3c3d9..f6df95585794c37271bffa368b2e4a00416b3cc9 100644 (file)
--- 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);
   }
 
   /**