Issue #278958 by jhodgdon, pwolanin, mgifford: Advanced search form has usability issues

core/modules/node/src/Plugin/Search/NodeSearch.php

@@ -114,6 +114,11 @@ class NodeSearch extends ConfigurableSearchPluginBase implements AccessibleInter
     'term' => array('column' => 'ti.tid', 'join' => array('table' => 'taxonomy_index', 'alias' => 'ti', 'condition' => 'n.nid = ti.nid')),
   );
 
+  /**
+   * A constant for setting and checking the query string.
+   */
+  const ADVANCED_FORM = 'advanced-form';
+
   /**
    * {@inheritdoc}
    */
@@ -490,38 +495,59 @@ public function indexStatus() {
    * {@inheritdoc}
    */
   public function searchFormAlter(array &$form, FormStateInterface $form_state) {
+    $parameters = $this->getParameters();
+    $keys = $this->getKeywords();
+    $used_advanced = !empty($parameters[self::ADVANCED_FORM]);
+    if ($used_advanced) {
+      $f = isset($parameters['f']) ? (array) $parameters['f'] : array();
+      $defaults =  $this->parseAdvancedDefaults($f, $keys);
+    }
+    else {
+      $defaults = array('keys' => $keys);
+    }
+
+    $form['basic']['keys']['#default_value'] = $defaults['keys'];
+
     // Add advanced search keyword-related boxes.
     $form['advanced'] = array(
       '#type' => 'details',
       '#title' => t('Advanced search'),
       '#attributes' => array('class' => array('search-advanced')),
       '#access' => $this->account && $this->account->hasPermission('use advanced search'),
+      '#open' => $used_advanced,
     );
     $form['advanced']['keywords-fieldset'] = array(
       '#type' => 'fieldset',
       '#title' => t('Keywords'),
     );
+
     $form['advanced']['keywords'] = array(
       '#prefix' => '<div class="criterion">',
       '#suffix' => '</div>',
     );
+
     $form['advanced']['keywords-fieldset']['keywords']['or'] = array(
       '#type' => 'textfield',
       '#title' => t('Containing any of the words'),
       '#size' => 30,
       '#maxlength' => 255,
+      '#default_value' => isset($defaults['or']) ? $defaults['or'] : '',
     );
+
     $form['advanced']['keywords-fieldset']['keywords']['phrase'] = array(
       '#type' => 'textfield',
       '#title' => t('Containing the phrase'),
       '#size' => 30,
       '#maxlength' => 255,
+      '#default_value' => isset($defaults['phrase']) ? $defaults['phrase'] : '',
     );
+
     $form['advanced']['keywords-fieldset']['keywords']['negative'] = array(
       '#type' => 'textfield',
       '#title' => t('Containing none of the words'),
       '#size' => 30,
       '#maxlength' => 255,
+      '#default_value' => isset($defaults['negative']) ? $defaults['negative'] : '',
     );
 
     // Add node types.
@@ -536,7 +562,9 @@ public function searchFormAlter(array &$form, FormStateInterface $form_state) {
       '#prefix' => '<div class="criterion">',
       '#suffix' => '</div>',
       '#options' => $types,
+      '#default_value' => isset($defaults['type']) ? $defaults['type'] : array(),
     );
+
     $form['advanced']['submit'] = array(
       '#type' => 'submit',
       '#value' => t('Advanced search'),
@@ -563,6 +591,7 @@ public function searchFormAlter(array &$form, FormStateInterface $form_state) {
         '#prefix' => '<div class="criterion">',
         '#suffix' => '</div>',
         '#options' => $language_options,
+        '#default_value' => isset($defaults['language']) ? $defaults['language'] : array(),
       );
     }
   }
@@ -574,6 +603,7 @@ public function buildSearchUrlQuery(FormStateInterface $form_state) {
     // Read keyword and advanced search information from the form values,
     // and put these into the GET parameters.
     $keys = trim($form_state->getValue('keys'));
+    $advanced = FALSE;
 
     // Collect extra filters.
     $filters = array();
@@ -582,6 +612,7 @@ public function buildSearchUrlQuery(FormStateInterface $form_state) {
       // checkboxes to 0.
       foreach ($form_state->getValue('type') as $type) {
         if ($type) {
+          $advanced = TRUE;
           $filters[] = 'type:' . $type;
         }
       }
@@ -590,11 +621,13 @@ public function buildSearchUrlQuery(FormStateInterface $form_state) {
     if ($form_state->hasValue('term') && is_array($form_state->getValue('term'))) {
       foreach ($form_state->getValue('term') as $term) {
         $filters[] = 'term:' . $term;
+        $advanced = TRUE;
       }
     }
     if ($form_state->hasValue('language') && is_array($form_state->getValue('language'))) {
       foreach ($form_state->getValue('language') as $language) {
         if ($language) {
+          $advanced = TRUE;
           $filters[] = 'language:' . $language;
         }
       }
@@ -602,15 +635,18 @@ public function buildSearchUrlQuery(FormStateInterface $form_state) {
     if ($form_state->getValue('or') != '') {
       if (preg_match_all('/ ("[^"]+"|[^" ]+)/i', ' ' . $form_state->getValue('or'), $matches)) {
         $keys .= ' ' . implode(' OR ', $matches[1]);
+        $advanced = TRUE;
       }
     }
     if ($form_state->getValue('negative') != '') {
       if (preg_match_all('/ ("[^"]+"|[^" ]+)/i', ' ' . $form_state->getValue('negative'), $matches)) {
         $keys .= ' -' . implode(' -', $matches[1]);
+        $advanced = TRUE;
       }
     }
     if ($form_state->getValue('phrase') != '') {
       $keys .= ' "' . str_replace('"', ' ', $form_state->getValue('phrase')) . '"';
+      $advanced = TRUE;
     }
     $keys = trim($keys);
 
@@ -621,10 +657,69 @@ public function buildSearchUrlQuery(FormStateInterface $form_state) {
     if ($filters) {
       $query['f'] = $filters;
     }
+    // Record that the person used the advanced search form, if they did.
+    if ($advanced) {
+      $query[self::ADVANCED_FORM] = '1';
+    }
 
     return $query;
   }
 
+  /**
+   * Parses the advanced search form default values.
+   *
+   * @param array $f
+   *   The 'f' query parameter set up in self::buildUrlSearchQuery(), which
+   *   contains the advanced query values.
+   * @param string $keys
+   *   The search keywords string, which contains some information from the
+   *   advanced search form.
+   *
+   * @return array
+   *   Array of default form values for the advanced search form, including
+   *   a modified 'keys' element for the bare search keywords.
+   */
+  protected function parseAdvancedDefaults($f, $keys) {
+    $defaults = array();
+
+    // Split out the advanced search parameters.
+    foreach ($f as $advanced) {
+      list($key, $value) = explode(':', $advanced, 2);
+      if (!isset($defaults[$key])) {
+        $defaults[$key] = array();
+      }
+      $defaults[$key][] = $value;
+    }
+
+    // Split out the negative, phrase, and OR parts of keywords.
+
+    // For phrases, the form only supports one phrase.
+    $matches = array();
+    $keys = ' ' . $keys . ' ';
+    if (preg_match('/ "([^"]+)" /', $keys, $matches)) {
+      $keys = str_replace($matches[0], ' ', $keys);
+      $defaults['phrase'] = $matches[1];
+    }
+
+    // Negative keywords: pull all of them out.
+    if (preg_match_all('/ -([^ ]+)/', $keys, $matches)) {
+      $keys = str_replace($matches[0], ' ', $keys);
+      $defaults['negative'] = implode(' ', $matches[1]);
+    }
+
+    // OR keywords: pull up to one set of them out of the query.
+    if (preg_match('/ [^ ]+( OR [^ ]+)+ /', $keys, $matches)) {
+      $keys = str_replace($matches[0], ' ', $keys);
+      $words = explode(' OR ', trim($matches[0]));
+      $defaults['or'] = implode(' ', $words);
+    }
+
+    // Put remaining keywords string back into keywords.
+    $defaults['keys'] = trim($keys);
+
+    return $defaults;
+  }
+
   /**
    * Gathers ranking definitions from hook_ranking().
    *

core/modules/search/src/Tests/SearchAdvancedSearchFormTest.php

@@ -40,13 +40,11 @@ protected function setUp() {
   }
 
   /**
-   * Test using the search form with GET and POST queries.
-   * Test using the advanced search form to limit search to nodes of type "Basic page".
+   * Tests advanced search by node type.
    */
   function testNodeType() {
+    // Verify some properties of the node that was created.
     $this->assertTrue($this->node->getType() == 'page', 'Node type is Basic page.');
-
-    // Assert that the dummy title doesn't equal the real title.
     $dummy_title = 'Lorem ipsum';
     $this->assertNotEqual($dummy_title, $this->node->label(), "Dummy title doesn't equal node title.");
 
@@ -63,11 +61,56 @@ function testNodeType() {
     $this->drupalPostForm('search/node', $edit, t('Advanced search'));
     $this->assertText($this->node->label(), 'Basic page node is found with POST query.');
 
-    // Advanced search type option.
+    // Search by node type.
     $this->drupalPostForm('search/node', array_merge($edit, array('type[page]' => 'page')), t('Advanced search'));
     $this->assertText($this->node->label(), 'Basic page node is found with POST query and type:page.');
 
     $this->drupalPostForm('search/node', array_merge($edit, array('type[article]' => 'article')), t('Advanced search'));
     $this->assertText('search yielded no results', 'Article node is not found with POST query and type:article.');
   }
+
+  /**
+   * Tests that after submitting the advanced search form, the form is refilled.
+   */
+  function testFormRefill() {
+    $edit = array(
+      'keys' => 'cat',
+      'or' => 'dog gerbil',
+      'phrase' => 'pets are nice',
+      'negative' => 'fish snake',
+      'type[page]' => 'page',
+    );
+    $this->drupalPostForm('search/node', $edit, t('Advanced search'));
+
+    // Test that the encoded query appears in the page title. Only test the
+    // part not including the quote, because assertText() cannot seem to find
+    // the quote marks successfully.
+    $this->assertText('Search for cat dog OR gerbil -fish -snake');
+
+    // Verify that all of the form fields are filled out.
+    foreach ($edit as $key => $value) {
+      if ($key != 'type[page]') {
+        $elements = $this->xpath('//input[@name=:name]', array(':name' => $key));
+        $this->assertTrue(isset($elements[0]) && $elements[0]['value'] == $value, "Field $key is set to $value");
+      }
+      else {
+        $elements = $this->xpath('//input[@name=:name]', array(':name' => $key));
+        $this->assertTrue(isset($elements[0]) && !empty($elements[0]['checked']), "Field $key is checked");
+      }
+    }
+
+    // Now test by submitting the or/not part of the query in the main
+    // search box, and verify that the advanced form is not filled out.
+    // (It shouldn't be filled out unless you submit values in those fields.)
+    $edit2 = array('keys' => 'cat dog OR gerbil -fish -snake');
+    $this->drupalPostForm('search/node', $edit2, t('Advanced search'));
+    $this->assertText('Search for cat dog OR gerbil -fish -snake');
+    foreach ($edit as $key => $value) {
+      if ($key != 'type[page]') {
+        $elements = $this->xpath('//input[@name=:name]', array(':name' => $key));
+        $this->assertFalse(isset($elements[0]) && $elements[0]['value'] == $value, "Field $key is not set to $value");
+      }
+    }
+  }
+
 }