Skip to content

GitLab

D
drupal
Commit 4e871329 authored by webchick's avatar webchick
Browse files
Options

Issue #1366020 by pwolanin, dawehner, rcaracaus, jhodgdon | tstoeckler:... 

Issue #1366020 by pwolanin, dawehner, rcaracaus, jhodgdon | tstoeckler: Overhaul SearchQuery; make search redirects use GET query params for keywords.
parent d4ff24a6
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 629 additions and 278 deletions
core/modules/node/lib/Drupal/node/Plugin/Search/NodeSearch.php
View file @ 4e871329
......@@ -20,6 +20,7 @@
use Drupal\node\NodeInterface;
use Drupal\search\Plugin\ConfigurableSearchPluginBase;
use Drupal\search\Plugin\SearchIndexingInterface;
use Drupal\Search\SearchQuery;
use Symfony\Component\DependencyInjection\ContainerInterface;
/**
......@@ -157,6 +158,17 @@ public function access($operation = 'view', AccountInterface $account = NULL) {
return !empty($account) && $account->hasPermission('access content');
}
/**
* {@inheritdoc}
*/
public function isSearchExecutable() {
// Node search is executable if we have keywords or an advanced parameter.
// At least, we should parse out the parameters and see if there are any
// keyword matches in that case, rather than just printing out the
// "Please enter keywords" message.
return !empty($this->keywords) || (isset($this->searchParameters['f']) && count($this->searchParameters['f']));
}
/**
* {@inheritdoc}
*/
......@@ -182,7 +194,8 @@ public function execute() {
// the URL: ?f[]=type:page&f[]=term:27&f[]=term:13&f[]=langcode:en
// So $parameters['f'] looks like:
// array('type:page', 'term:27', 'term:13', 'langcode:en');
// We need to parse this out into query conditions.
// We need to parse this out into query conditions, some of which go into
// the keywords string, and some of which are separate conditions.
$parameters = $this->getParameters();
if (!empty($parameters['f']) && is_array($parameters['f'])) {
$filters = array();
......@@ -195,6 +208,7 @@ public function execute() {
$filters[$m[1]][$m[2]] = $m[2];
}
}
// Now turn these into query conditions. This assumes that everything in
// $filters is a known type of advanced search.
foreach ($filters as $option => $matched) {
......@@ -211,15 +225,11 @@ public function execute() {
}
}
}
// Only continue if the first pass query matches.
if (!$query->executeFirstPass()) {
return array();
}
// Add the ranking expressions.
$this->addNodeRankings($query);
// Load results.
// Run the query and load results.
$find = $query
// Add the language code of the indexed item to the result of the query,
// since the node will be rendered using the respective language.
......@@ -231,6 +241,21 @@ public function execute() {
->limit(10)
->execute();
// Check query status and set messages if needed.
$status = $query->getStatus();
if ($status & SearchQuery::EXPRESSIONS_IGNORED) {
drupal_set_message($this->t('Your search used too many AND/OR expressions. Only the first @count terms were included in this search.', array('@count' => $this->searchSettings->get('and_or_limit'))), 'warning');
}
if ($status & SearchQuery::LOWER_CASE_OR) {
drupal_set_message($this->t('Search for either of the two terms with uppercase <strong>OR</strong>. For example, <strong>cats OR dogs</strong>.'), 'warning');
}
if ($status & SearchQuery::NO_POSITIVE_KEYWORDS) {
drupal_set_message(\Drupal::translation()->formatPlural($this->searchSettings->get('index.minimum_word_size'), 'You must include at least one positive keyword with 1 character or more.', 'You must include at least one positive keyword with @count characters or more.'), 'warning');
}
$node_storage = $this->entityManager->getStorage('node');
$node_render = $this->entityManager->getViewBuilder('node');
......@@ -370,7 +395,7 @@ public function indexStatus() {
* {@inheritdoc}
*/
public function searchFormAlter(array &$form, array &$form_state) {
// Add keyword boxes.
// Add advanced search keyword-related boxes.
$form['advanced'] = array(
'#type' => 'details',
'#title' => t('Advanced search'),
......@@ -445,25 +470,18 @@ public function searchFormAlter(array &$form, array &$form_state) {
'#options' => $language_options,
);
}
// Add a submit handler.
$form['#submit'][] = array($this, 'searchFormSubmit');
}
/**
* Handles submission of elements added in searchFormAlter().
*
* @param array $form
* Nested array of form elements that comprise the form.
* @param array $form_state
* A keyed array containing the current state of the form.
/*
* {@inheritdoc}
*/
public function searchFormSubmit(array &$form, array &$form_state) {
// Initialize using any existing basic search keywords.
$keys = $form_state['values']['processed_keys'];
$filters = array();
public function buildSearchUrlQuery($form_state) {
// Read keyword and advanced search information from the form values,
// and put these into the GET parameters.
$keys = trim($form_state['values']['keys']);
// Collect extra restrictions.
// Collect extra filters.
$filters = array();
if (isset($form_state['values']['type']) && is_array($form_state['values']['type'])) {
// Retrieve selected types - Form API sets the value of unselected
// checkboxes to 0.
......@@ -499,21 +517,17 @@ public function searchFormSubmit(array &$form, array &$form_state) {
if ($form_state['values']['phrase'] != '') {
$keys .= ' "' . str_replace('"', ' ', $form_state['values']['phrase']) . '"';
}
if (!empty($keys)) {
form_set_value($form['basic']['processed_keys'], trim($keys), $form_state);
}
$options = array();
$keys = trim($keys);
// Put the keywords and advanced parameters into GET parameters. Make sure
// to put keywords into the query even if it is empty, because the page
// controller uses that to decide it's time to check for search results.
$query = array('keys' => $keys);
if ($filters) {
$options['query'] = array('f' => $filters);
$query['f'] = $filters;
}
$form_state['redirect_route'] = array(
'route_name' => 'search.view_' . $form_state['search_page_id'],
'route_parameters' => array(
'keys' => $keys,
),
'options' => $options,
);
return $query;
}
/**
......
core/modules/search/lib/Drupal/search/Controller/SearchController.php
View file @ 4e871329
......@@ -51,42 +51,42 @@ public static function create(ContainerInterface $container) {
* The request object.
* @param \Drupal\search\SearchPageInterface $entity
* The search page entity.
* @param string $keys
* (optional) Search keywords, defaults to an empty string.
*
* @return array|\Symfony\Component\HttpFoundation\RedirectResponse
* The search form and search results or redirect response.
* @return array
* The search form and search results build array.
*/
public function view(Request $request, SearchPageInterface $entity, $keys = '') {
// Also try to pull search keywords from the request to support old GET
// format of searches for existing links.
if (!$keys && $request->query->has('keys')) {
$keys = $request->query->get('keys');
public function view(Request $request, SearchPageInterface $entity) {
$build = array();
$plugin = $entity->getPlugin();
// Build the form first, because it may redirect during the submit,
// and we don't want to build the results based on last time's request.
if ($request->query->has('keys')) {
$keys = trim($request->get('keys'));
$plugin->setSearch($keys, $request->query->all(), $request->attributes->all());
}
$keys = trim($keys);
$build['#title'] = $this->t('Search');
$plugin = $entity->getPlugin();
$plugin->setSearch($keys, $request->query->all(), $request->attributes->all());
$results = array();
$build['search_form'] = $this->entityFormBuilder()->getForm($entity, 'search');
// Process the search form. Note that if there is
// \Drupal::request()->request data, search_form_submit() will cause a
// redirect to search/[path]/[keys], which will get us back to this page
// callback. In other words, the search form submits with POST but redirects
// to GET. This way we can keep the search query URL clean as a whistle.
if ($request->request->has('form_id') || $request->request->get('form_id') != 'search_form') {
// Only search if there are keywords or non-empty conditions.
// Build search results, if keywords or other search parameters are in the
// GET parameters. Note that we need to try the search if 'keys' is in
// there at all, vs. being empty, due to advanced search.
$results = array();
if ($request->query->has('keys')) {
if ($plugin->isSearchExecutable()) {
// Log the search keys.
watchdog('search', 'Searched %type for %keys.', array('%keys' => $keys, '%type' => $entity->label()), WATCHDOG_NOTICE, $this->l(t('results'), 'search.view_' . $entity->id(), array('keys' => $keys)));
// Log the search.
watchdog('search', 'Searched %type for %keys.', array('%keys' => $keys, '%type' => $entity->label()), WATCHDOG_NOTICE);
// Collect the search results.
$results = $plugin->buildResults();
}
else {
// The search not being executable means that no keywords or other
// conditions were entered.
drupal_set_message($this->t('Please enter some keywords.'), 'error');
}
}
// The form may be altered based on whether the search was run.
$build['search_form'] = $this->entityFormBuilder()->getForm($entity, 'search');
if (count($results)) {
$build['search_results_title'] = array(
'#markup' => '<h2>' . $this->t('Search results') . '</h2>',
......
core/modules/search/lib/Drupal/search/Form/SearchBlockForm.php
View file @ 4e871329
......@@ -53,7 +53,21 @@ public function getFormId() {
* {@inheritdoc}
*/
public function buildForm(array $form, array &$form_state) {
$form['search_block_form'] = array(
// Set up the form to submit using GET to the correct search page.
$entity_id = $this->searchPageRepository->getDefaultSearchPage();
if (!$entity_id) {
$form['message'] = array(
'#markup' => $this->t('Search is currently disabled'),
);
return $form;
}
$route = 'search.view_' . $entity_id;
$form['#action'] = $this->url($route);
$form['#token'] = FALSE;
$form['#method'] = 'get';
$form['keys'] = array(
'#type' => 'search',
'#title' => $this->t('Search'),
'#title_display' => 'invisible',
......@@ -61,8 +75,14 @@ public function buildForm(array $form, array &$form_state) {
'#default_value' => '',
'#attributes' => array('title' => $this->t('Enter the terms you wish to search for.')),
);
$form['actions'] = array('#type' => 'actions');
$form['actions']['submit'] = array('#type' => 'submit', '#value' => $this->t('Search'));
$form['actions']['submit'] = array(
'#type' => 'submit',
'#value' => $this->t('Search'),
// Prevent op from showing up in the query string.
'#name' => '',
);
return $form;
}
......@@ -71,36 +91,6 @@ public function buildForm(array $form, array &$form_state) {
* {@inheritdoc}
*/
public function submitForm(array &$form, array &$form_state) {
// The search form relies on control of the redirect destination for its
// functionality, so we override any static destination set in the request.
// See http://drupal.org/node/292565.
$request = $this->getRequest();
if ($request->query->has('destination')) {
$request->query->remove('destination');
}
// Check to see if the form was submitted empty.
// If it is empty, display an error message.
// (This method is used instead of setting #required to TRUE for this field
// because that results in a confusing error message. It would say a plain
// "field is required" because the search keywords field has no title.
// The error message would also complain about a missing #title field.)
if ($form_state['values']['search_block_form'] == '') {
drupal_set_message($this->t('Please enter some keywords.'), 'error');
}
$form_id = $form['form_id']['#value'];
if ($entity_id = $this->searchPageRepository->getDefaultSearchPage()) {
$form_state['redirect_route'] = array(
'route_name' => 'search.view_' . $entity_id,
'route_parameters' => array(
'keys' => trim($form_state['values'][$form_id]),
),
);
}
else {
drupal_set_message($this->t('Search is currently disabled.'), 'error');
}
// This form submits to the search page, so processing happens there.
}
}
core/modules/search/lib/Drupal/search/Form/SearchPageForm.php
View file @ 4e871329
......@@ -11,6 +11,12 @@
/**
* Provides a search form for site wide search.
*
* Search plugins can define method searchFormAlter() to alter the form. If they
* have additional or substitute fields, they will need to override the form
* submit, making sure to redirect with a GET parameter of 'keys' included, to
* trigger the search being processed by the controller, and adding in any
* additional query parameters they need to execute search.
*/
class SearchPageForm extends EntityFormController {
......@@ -33,8 +39,8 @@ public function getFormID() {
*/
public function form(array $form, array &$form_state) {
$plugin = $this->entity->getPlugin();
$form_state['search_page_id'] = $this->entity->id();
$form['basic'] = array(
'#type' => 'container',
'#attributes' => array(
......@@ -58,6 +64,7 @@ public function form(array $form, array &$form_state) {
'#type' => 'submit',
'#value' => $this->t('Search'),
);
// Allow the plugin to add to or alter the search form.
$plugin->searchFormAlter($form, $form_state);
......@@ -72,29 +79,20 @@ protected function actions(array $form, array &$form_state) {
return array();
}
/**
* {@inheritdoc}
*/
public function validateForm(array &$form, array &$form_state) {
form_set_value($form['basic']['processed_keys'], trim($form_state['values']['keys']), $form_state);
}
/**
* {@inheritdoc}
*/
public function submitForm(array &$form, array &$form_state) {
$keys = $form_state['values']['processed_keys'];
if ($keys == '') {
$this->setFormError('keys', $form_state, $this->t('Please enter some keywords.'));
// Fall through to the form redirect.
}
// Redirect to the search page with keywords in the GET parameters.
// Plugins with additional search parameters will need to provide their
// own form submit handler to replace this, so they can put their values
// into the GET as well. If so, make sure to put 'keys' into the GET
// parameters so that the search results generation is triggered.
$query = $this->entity->getPlugin()->buildSearchUrlQuery($form_state);
$route = 'search.view_' . $form_state['search_page_id'];
$form_state['redirect_route'] = array(
'route_name' => 'search.view_' . $this->entity->id(),
'route_parameters' => array(
'keys' => $keys,
),
'route_name' => $route,
'options' => array('query' => $query),
);
}
}
core/modules/search/lib/Drupal/search/Plugin/SearchInterface.php
View file @ 4e871329
......@@ -20,7 +20,7 @@ interface SearchInterface extends PluginInspectionInterface {
* @param string $keywords
* The keywords to use in a search.
* @param array $parameters
* Array of parameters as am associative array. This is expected to
* Array of parameters as an associative array. This is expected to
* be the query string from the current request.
* @param array $attributes
* Array of attributes, usually from the current request object.
......@@ -85,9 +85,9 @@ public function buildResults();
* Alters the search form when being built for a given plugin.
*
* The core search module only invokes this method on active module plugins
* when building a form for them in search_form(). A plugin implementing
* this needs to add validate and submit callbacks to the form if it needs
* to act after form submission.
* when building a form for them in
* \Drupal\search\Form\SearchPageForm::form(). A plugin implementing this
* will also need to implement the buildSearchUrlQuery() method.
*
* @param array $form
* Nested array of form elements that comprise the form.
......@@ -95,7 +95,30 @@ public function buildResults();
* A keyed array containing the current state of the form. The arguments
* that \Drupal::formBuilder()->getForm() was originally called with are
* available in the array $form_state['build_info']['args'].
*
* @see SearchInterface::buildSearchUrlQuery()
*/
public function searchFormAlter(array &$form, array &$form_state);
/**
* Builds the URL GET query parameters array for search.
*
* When the search form is submitted, a redirect is generated with the
* search input as GET query parameters. Plugins using the searchFormAlter()
* method to add form elements to the search form will need to override this
* method to gather the form input and add it to the GET query parameters.
*
* @param array $form_state
* The form state, with submitted form information.
*
* @return array
* An array of GET query parameters containing all relevant form values
* to process the search. The 'keys' element must be present in order to
* trigger generation of search results, even if it is empty or unused by
* the search plugin.
*
* @see SearchInterface::searchFormAlter()
*/
public function buildSearchUrlQuery($form_state);
}
core/modules/search/lib/Drupal/search/Plugin/SearchPluginBase.php
View file @ 4e871329
......@@ -101,11 +101,21 @@ public function buildResults() {
return $built;
}
/**
/**
* {@inheritdoc}
*/
public function searchFormAlter(array &$form, array &$form_state) {
// Empty default implementation.
}
/*
* {@inheritdoc}
*/
public function buildSearchUrlQuery($form_state) {
// Grab the keywords entered in the form and put them as 'keys' in the GET.
$keys = trim($form_state['values']['keys']);
$query = array('keys' => $keys);
return $query;
}
}
core/modules/search/lib/Drupal/search/Routing/SearchPageRoutes.php
View file @ 4e871329
......@@ -77,15 +77,13 @@ public function routes() {
$active_pages = $this->searchPageRepository->getActiveSearchPages();
foreach ($active_pages as $entity_id => $entity) {
$routes["search.view_$entity_id"] = new Route(
'/search/' . $entity->getPath() . '/{keys}',
'/search/' . $entity->getPath(),
array(
'_content' => 'Drupal\search\Controller\SearchController::view',
'_title' => $entity->label(),
'_title' => 'Search',
'entity' => $entity_id,
'keys' => '',
),
array(
'keys' => '.+',
'_entity_access' => 'entity.view',
'_permission' => 'search content',
),
......
core/modules/search/lib/Drupal/search/Tests/SearchAdvancedSearchFormTest.php
View file @ 4e871329
......@@ -51,11 +51,11 @@ function testNodeType() {
$this->assertNotEqual($dummy_title, $this->node->label(), "Dummy title doesn't equal node title.");
// Search for the dummy title with a GET query.
$this->drupalGet('search/node/' . $dummy_title);
$this->drupalGet('search/node', array('query' => array('keys' => $dummy_title)));
$this->assertNoText($this->node->label(), 'Basic page node is not found with dummy title.');
// Search for the title of the node with a GET query.
$this->drupalGet('search/node/' . $this->node->label());
$this->drupalGet('search/node', array('query' => array('keys' => $this->node->label())));
$this->assertText($this->node->label(), 'Basic page node is found with GET query.');
// Search for the title of the node with a POST query.
......
core/modules/search/lib/Drupal/search/Tests/SearchBlockTest.php
View file @ 4e871329
......@@ -2,7 +2,7 @@
/**
* @file
* Definition of Drupal\search\Tests\SearchBlockTest.
* Contains \Drupal\search\Tests\SearchBlockTest.
*/
namespace Drupal\search\Tests;
......@@ -51,15 +51,15 @@ protected function testSearchFormBlock() {
$this->assertText($block->label(), 'Block title was found.');
// Test a normal search via the block form, from the front page.
$terms = array('search_block_form' => 'test');
$this->drupalPostForm('', $terms, t('Search'));
$terms = array('keys' => 'test');
$this->submitGetForm('', $terms, t('Search'));
$this->assertResponse(200);
$this->assertText('Your search yielded no results');
// Test a search from the block on a 404 page.
$this->drupalGet('foo');
$this->assertResponse(404);
$this->drupalPostForm(NULL, $terms, t('Search'));
$this->submitGetForm(NULL, $terms, t('Search'));
$this->assertResponse(200);
$this->assertText('Your search yielded no results');
......@@ -67,20 +67,23 @@ protected function testSearchFormBlock() {
$visibility['path']['pages'] = 'search';
$block->set('visibility', $visibility);
$this->drupalPostForm('', $terms, t('Search'));
$this->submitGetForm('', $terms, t('Search'));
$this->assertResponse(200);
$this->assertText('Your search yielded no results');
// Confirm that the user is redirected to the search page.
// Confirm that the form submits to the default search page.
/** @var $search_page_repository \Drupal\search\SearchPageRepositoryInterface */
$search_page_repository = \Drupal::service('search.search_page_repository');
$entity_id = $search_page_repository->getDefaultSearchPage();
$this->assertEqual(
$this->getUrl(),
url('search/node/' . $terms['search_block_form'], array('absolute' => TRUE)),
'Redirected to correct url.'
\Drupal::url('search.view_' . $entity_id, array(), array('query' => array('keys' => $terms['keys']), 'absolute' => TRUE)),
'Submitted to correct url.'
);
// Test an empty search via the block form, from the front page.
$terms = array('search_block_form' => '');
$this->drupalPostForm('', $terms, t('Search'));
$terms = array('keys' => '');
$this->submitGetForm('', $terms, t('Search'));
$this->assertResponse(200);
$this->assertText('Please enter some keywords');
......@@ -88,9 +91,24 @@ protected function testSearchFormBlock() {
// submitted empty.
$this->assertEqual(
$this->getUrl(),
url('search/node/', array('absolute' => TRUE)),
\Drupal::url('search.view_' . $entity_id, array(), array('query' => array('keys' => ''), 'absolute' => TRUE)),
'Redirected to correct url.'
);
// Test that after entering a too-short keyword in the form, you can then
// search again with a longer keyword. First test using the block form.
$this->submitGetForm('node', array('keys' => $this->randomName(1)), t('Search'));
$this->assertText('You must include at least one positive keyword', 'Keyword message is displayed when searching for short word');
$this->assertNoText(t('Please enter some keywords'), 'With short word entered, no keywords message is not displayed');
$this->submitGetForm(NULL, array('keys' => $this->randomName()), t('Search'), 'search-block-form');
$this->assertNoText('You must include at least one positive keyword', 'Keyword message is not displayed when searching for long word after short word search');
// Same test again, using the search page form for the second search this
// time.
$this->submitGetForm('node', array('keys' => $this->randomName(1)), t('Search'));
$this->drupalPostForm(NULL, array('keys' => $this->randomName()), t('Search'), array(), array(), 'search-form');
$this->assertNoText('You must include at least one positive keyword', 'Keyword message is not displayed when searching for long word after short word search');
}
}
core/modules/search/lib/Drupal/search/Tests/SearchCommentCountToggleTest.php
View file @ 4e871329
......@@ -82,11 +82,11 @@ function setUp() {
function testSearchCommentCountToggle() {
// Search for the nodes by string in the node body.
$edit = array(
'search_block_form' => "'SearchCommentToggleTestCase'",
'keys' => "'SearchCommentToggleTestCase'",
);
// Test comment count display for nodes with comment status set to Open
$this->drupalPostForm('', $edit, t('Search'));
$this->submitGetForm('', $edit, t('Search'));
$this->assertText(t('0 comments'), 'Empty comment count displays for nodes with comment status set to Open');
$this->assertText(t('1 comment'), 'Non-empty comment count displays for nodes with comment status set to Open');
......@@ -96,7 +96,7 @@ function testSearchCommentCountToggle() {
$this->searchable_nodes['1 comment']->set('comment', CommentItemInterface::CLOSED);
$this->searchable_nodes['1 comment']->save();
$this->drupalPostForm('', $edit, t('Search'));
$this->submitGetForm('', $edit, t('Search'));
$this->assertNoText(t('0 comments'), 'Empty comment count does not display for nodes with comment status set to Closed');
$this->assertText(t('1 comment'), 'Non-empty comment count displays for nodes with comment status set to Closed');
......@@ -106,7 +106,7 @@ function testSearchCommentCountToggle() {
$this->searchable_nodes['1 comment']->set('comment', CommentItemInterface::HIDDEN);
$this->searchable_nodes['1 comment']->save();
$this->drupalPostForm('', $edit, t('Search'));
$this->submitGetForm('', $edit, t('Search'));
$this->assertNoText(t('0 comments'), 'Empty comment count does not display for nodes with comment status set to Hidden');
$this->assertNoText(t('1 comment'), 'Non-empty comment count does not display for nodes with comment status set to Hidden');
}
......
core/modules/search/lib/Drupal/search/Tests/SearchCommentTest.php
View file @ 4e871329
......@@ -94,18 +94,18 @@ function testSearchResultsComment() {
// Search for the comment subject.
$edit = array(
'search_block_form' => "'" . $edit_comment['subject'] . "'",
'keys' => "'" . $edit_comment['subject'] . "'",
);
$this->drupalPostForm('', $edit, t('Search'));
$this->submitGetForm('', $edit, t('Search'));
$node2 = node_load($node->id(), TRUE);
$this->assertText($node2->label(), 'Node found in search results.');
$this->assertText($edit_comment['subject'], 'Comment subject found in search results.');
// Search for the comment body.
$edit = array(
'search_block_form' => "'" . $comment_body . "'",
'keys' => "'" . $comment_body . "'",
);
$this->drupalPostForm('', $edit, t('Search'));
$this->submitGetForm('', $edit, t('Search'));
$this->assertText($node2->label(), 'Node found in search results.');
// Verify that comment is rendered using proper format.
......@@ -123,7 +123,7 @@ function testSearchResultsComment() {
$this->cronRun();
// Search for $title.
$this->drupalPostForm('', $edit, t('Search'));
$this->submitGetForm('', $edit, t('Search'));
$this->assertNoText($comment_body, 'Comment body text not found in search results.');
}
......@@ -209,9 +209,9 @@ function assertCommentAccess($assume_access, $message) {
// Search for the comment subject.
$edit = array(
'search_block_form' => "'" . $this->comment_subject . "'",
'keys' => "'" . $this->comment_subject . "'",
);
$this->drupalPostForm('', $edit, t('Search'));
$this->submitGetForm('', $edit, t('Search'));
if ($assume_access) {
$expected_node_result = $this->assertText($this->node->label());
......