From 0623c095190ea3bd9452fdd31bc90ce62dcc19e6 Mon Sep 17 00:00:00 2001 From: Alex Pott <alex.a.pott@googlemail.com> Date: Tue, 14 Jul 2015 09:11:33 +0100 Subject: [PATCH] Issue #278958 by jhodgdon, pwolanin, mgifford: Advanced search form has usability issues --- .../node/src/Plugin/Search/NodeSearch.php | 95 +++++++++++++++++++ .../Tests/SearchAdvancedSearchFormTest.php | 53 ++++++++++- 2 files changed, 143 insertions(+), 5 deletions(-) diff --git a/core/modules/node/src/Plugin/Search/NodeSearch.php b/core/modules/node/src/Plugin/Search/NodeSearch.php index d787cb513f29..87a3851c5d14 100644 --- a/core/modules/node/src/Plugin/Search/NodeSearch.php +++ b/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(). * diff --git a/core/modules/search/src/Tests/SearchAdvancedSearchFormTest.php b/core/modules/search/src/Tests/SearchAdvancedSearchFormTest.php index 1465f5227701..3650a93bf328 100644 --- a/core/modules/search/src/Tests/SearchAdvancedSearchFormTest.php +++ b/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"); + } + } + } + } -- GitLab