From 4865a48e22ae1567a3e71d79023bd9c845b3ae36 Mon Sep 17 00:00:00 2001 From: Angie Byron Date: Thu, 5 Aug 2010 07:11:15 +0000 Subject: #567100 by jhodgdon, pwolanin, ezra-g: Fixed Search module is assuming node is default search even when it's disabled. --- modules/search/search.admin.inc | 36 +++++-- modules/search/search.module | 142 ++++++++++++++++++++------ modules/search/search.pages.inc | 52 +++++++--- modules/search/search.test | 119 +++++++++++++++++++-- modules/search/tests/search_extra_type.info | 8 ++ modules/search/tests/search_extra_type.module | 34 ++++++ 6 files changed, 328 insertions(+), 63 deletions(-) create mode 100644 modules/search/tests/search_extra_type.info create mode 100644 modules/search/tests/search_extra_type.module (limited to 'modules/search') diff --git a/modules/search/search.admin.inc b/modules/search/search.admin.inc index 45f1db44e..a67836eca 100644 --- a/modules/search/search.admin.inc +++ b/modules/search/search.admin.inc @@ -31,7 +31,7 @@ function search_reindex_confirm_submit(&$form, &$form_state) { */ function _search_get_module_names() { - $search_info = search_get_info(); + $search_info = search_get_info(TRUE); $modules = db_select('system', 's') ->fields('s', array('name', 'info')) ->condition('s.status', 1) @@ -48,10 +48,11 @@ function _search_get_module_names() { } /** - * Menu callback; displays the search module settings page. + * Menu callback: displays the search module settings page. * * @ingroup forms - * @see system_settings_form() + * + * @see search_admin_settings_validate() * @see search_admin_settings_submit() * @see search_admin_reindex_submit() */ @@ -118,9 +119,16 @@ function search_admin_settings($form) { '#type' => 'checkboxes', '#default_value' => variable_get('search_active_modules', array('node', 'user')), '#options' => _search_get_module_names(), - '#description' => t('Determine which search modules are active from the available modules.') + '#description' => t('Choose which search modules are active from the available modules.') ); - + $form['active']['search_default_module'] = array( + '#title' => t('Default search module'), + '#type' => 'radios', + '#default_value' => 'node', + '#options' => _search_get_module_names(), + '#description' => t('Choose which search module is the default.') + ); + $form['#validate'][] = 'search_admin_settings_validate'; $form['#submit'][] = 'search_admin_settings_submit'; // Per module settings @@ -135,7 +143,21 @@ function search_admin_settings($form) { } /** - * Submit callback. + * Form validation handler for search_admin_settings(). + */ +function search_admin_settings_validate($form, &$form_state) { + // Check whether we selected a valid default. + if ($form_state['clicked_button']['#value'] != t('Reset to defaults')) { + $new_modules = array_filter($form_state['values']['search_active_modules']); + $default = $form_state['values']['search_default_module']; + if (!in_array($default, $new_modules, TRUE)) { + form_set_error('search_default_module', t('Your default search module is not selected as an active module.')); + } + } +} + +/** + * Form submission handler for search_admin_settings(). */ function search_admin_settings_submit($form, &$form_state) { // If these settings change, the index needs to be rebuilt. @@ -159,7 +181,7 @@ function search_admin_settings_submit($form, &$form_state) { } /** - * Submit callback. + * Form submission handler for reindex button on search_admin_settings_form(). */ function search_admin_reindex_submit($form, &$form_state) { // send the user to the confirmation page diff --git a/modules/search/search.module b/modules/search/search.module index 2138562a6..e37ba95c4 100644 --- a/modules/search/search.module +++ b/modules/search/search.module @@ -170,7 +170,7 @@ function search_menu() { $items['search'] = array( 'title' => 'Search', 'page callback' => 'search_view', - 'access arguments' => array('search content'), + 'access callback' => 'search_is_active', 'type' => MENU_SUGGESTED_ITEM, 'file' => 'search.pages.inc', ); @@ -201,8 +201,9 @@ function search_menu() { // http://drupal.org/node/245103 for details. drupal_static_reset('search_get_info'); - if ($active = variable_get('search_active_modules', array('node', 'user'))) { - foreach (array_intersect_key(search_get_info(), array_flip($active)) as $module => $search_info) { + $default_info = search_get_default_module_info(); + if ($default_info) { + foreach (search_get_info() as $module => $search_info) { $path = 'search/' . $search_info['path']; $items[$path] = array( 'title' => $search_info['title'], @@ -210,9 +211,9 @@ function search_menu() { 'page arguments' => array($module), 'access callback' => '_search_menu_access', 'access arguments' => array($module), - 'type' => $module == 'node' ? MENU_DEFAULT_LOCAL_TASK : MENU_LOCAL_TASK, + 'type' => MENU_LOCAL_TASK, 'file' => 'search.pages.inc', - 'weight' => $module == 'node' ? -10 : 0, + 'weight' => $module == $default_info['module'] ? -10 : 0, ); $items["$path/%menu_tail"] = array( 'title' => $search_info['title'], @@ -226,9 +227,9 @@ function search_menu() { 'file' => 'search.pages.inc', 'weight' => 0, // These tabs are not subtabs. - 'tab_root' => 'search/node/%', + 'tab_root' => 'search/' . $default_info['path'] . '/%', // These tabs need to display at the same level. - 'tab_parent' => 'search', + 'tab_parent' => 'search/' . $default_info['path'], ); } } @@ -236,19 +237,63 @@ function search_menu() { } /** - * Get information about all available search hooks. + * Determines access for the ?q=search path. */ -function search_get_info() { +function search_is_active() { + // This path cannot be accessed if there are no active modules. + return user_access('search content') && search_get_info(); +} + +/** + * Returns information about available search modules. + * + * @param $all + * If TRUE, information about all enabled modules implementing + * hook_search_info() will be returned. If FALSE (default), only modules that + * have been set to active on the search settings page will be returned. + * + * @return + * Array of hook_search_info() return values, keyed by module name. The + * 'title' and 'path' array elements will be set to defaults for each module + * if not supplied by hook_search_info(), and an additional array element of + * 'module' will be added (set to the module name). + */ +function search_get_info($all = FALSE) { $search_hooks = &drupal_static(__FUNCTION__); if (!isset($search_hooks)) { foreach (module_implements('search_info') as $module) { $search_hooks[$module] = call_user_func($module . '_search_info'); - // Use module name as the default. + // Use module name as the default value. $search_hooks[$module] += array('title' => $module, 'path' => $module); + // Include the module name itself in the array. + $search_hooks[$module]['module'] = $module; } } - return $search_hooks; + + if ($all) { + return $search_hooks; + } + + $active = variable_get('search_active_modules', array('node', 'user')); + return array_intersect_key($search_hooks, array_flip($active)); +} + +/** + * Returns information about the default search module. + * + * @return + * The search_get_info() array element for the default search module, if any. + */ +function search_get_default_module_info() { + $info = search_get_info(); + $default = variable_get('search_default_module', 'node'); + if (isset($info[$default])) { + return $info[$default]; + } + // The variable setting does not match any active module, so just return + // the info for the first active module (if any). + return reset($info); } /** @@ -867,23 +912,42 @@ function search_get_keys() { */ /** - * Render a search form. + * Builds a search form. * * @param $action - * Form action. Defaults to "search/$type". This will be run through url(). + * Form action. Defaults to "search/$path", where $path is the search path + * associated with the $type module in its hook_search_info(). This will be + * run through url(). * @param $keys * The search string entered by the user, containing keywords for the search. * @param $type - * The type of search to render the node for. Must be the name of module - * which implements hook_search(). Defaults to 'node'. + * The search module to render the form for: a module that implements + * hook_search_info(). If not supplied, the default search module is used. * @param $prompt - * A piece of text to put before the form (e.g. "Enter your keywords") + * Label for the keywords field. Defaults to t('Enter your keywords') if NULL. + * Supply '' to omit. + * * @return - * An HTML string containing the search form. + * A Form API array for the search form. */ function search_form($form, &$form_state, $action = '', $keys = '', $type = NULL, $prompt = NULL) { + $module_info = FALSE; + if (!$type) { + $module_info = search_get_default_module_info(); + } + else { + $info = search_get_info(); + $module_info = isset($info[$type]) ? $info[$type] : FALSE; + } + + // Sanity check. + if (!$module_info) { + form_set_error(NULL, t('Search is currently disabled.'), 'error'); + return $form; + } + if (!$action) { - $action = 'search/' . $type; + $action = 'search/' . $module_info['path']; } if (is_null($prompt)) { $prompt = t('Enter your keywords'); @@ -945,7 +1009,13 @@ function search_box_form_submit($form, &$form_state) { } $form_id = $form['form_id']['#value']; - $form_state['redirect'] = 'search/node/' . trim($form_state['values'][$form_id]); + $info = search_get_default_module_info(); + if ($info) { + $form_state['redirect'] = 'search/' . $info['path'] . '/' . trim($form_state['values'][$form_id]); + } + else { + form_set_error(NULL, t('Search is currently disabled.'), 'error'); + } } /** @@ -976,19 +1046,25 @@ function template_preprocess_search_block_form(&$variables) { } /** - * Perform a standard search on the given keys, and return the formatted results. + * Performs a search by calling hook_search_execute(). + * + * @param $keys + * Keyword query to search on. + * @param $module + * Search module to search. + * + * @return + * Formatted search results. No return value if $keys are not supplied or + * if the given search module is not active. */ -function search_data($keys = NULL, $type = 'node') { - - if (isset($keys)) { - if (module_hook($type, 'search_execute')) { - $results = module_invoke($type, 'search_execute', $keys); - if (module_hook($type, 'search_page')) { - return module_invoke($type, 'search_page', $results); - } - else { - return theme('search_results', array('results' => $results, 'type' => $type)); - } +function search_data($keys, $module) { + if (module_hook($module, 'search_execute')) { + $results = module_invoke($module, 'search_execute', $keys); + if (module_hook($module, 'search_page')) { + return module_invoke($module, 'search_page', $results); + } + else { + return theme('search_results', array('results' => $results, 'type' => $module)); } } } @@ -1116,6 +1192,9 @@ function _search_excerpt_replace(&$text) { $text = preg_quote($text, '/'); } +/** + * Implements hook_forms(). + */ function search_forms() { $forms['search_block_form']= array( 'callback' => 'search_box', @@ -1123,3 +1202,4 @@ function search_forms() { ); return $forms; } + diff --git a/modules/search/search.pages.inc b/modules/search/search.pages.inc index 646431b43..0d9fb4bc3 100644 --- a/modules/search/search.pages.inc +++ b/modules/search/search.pages.inc @@ -8,39 +8,60 @@ /** * Menu callback; presents the search form and/or search results. + * + * @param $module + * Search module to use for the search. */ -function search_view($type = 'node') { +function search_view($module = NULL) { + $info = FALSE; + $redirect = FALSE; + + if (!empty($module)) { + $active_module_info = search_get_info(); + if (isset($active_module_info[$module])) { + $info = $active_module_info[$module]; + } + } + + if (empty($info)) { + // No path or invalid path: find the default module. Note that if there + // are no enabled search modules, this function should never be called, + // since hook_menu() would not have defined any search paths. + $info = search_get_default_module_info(); + $redirect = TRUE; + } + // Search form submits with POST but redirects to GET. This way we can keep // the search query URL clean as a whistle: - // search/type/keyword+keyword + // search/type_path/keyword+keyword if (!isset($_POST['form_id'])) { $keys = search_get_keys(); - if ($_GET['q'] != 'search' && $type == 'node' && !$keys) { - // Due to how search_menu() sets up the tabs, path search/node would - // display two sets of tabs. So instead, if there are no keywords and - // we're on the node tab, just redirect to the bare 'search' path. - drupal_goto('search'); + if ($redirect) { + // Redirect from bare /search or an invalid path to the default search path. + $path = 'search/' . $info['path']; + if ($keys) { + $path .= '/' . $keys; + } + drupal_goto($path); } - - // Only perform search if there is non-whitespace search term: + // Only perform search if there is a non-whitespace search term. $results = ''; if (trim($keys)) { // Log the search keys. - $info = search_get_info(); - watchdog('search', 'Searched %type for %keys.', array('%keys' => $keys, '%type' => $info[$type]['title']), WATCHDOG_NOTICE, l(t('results'), 'search/' . $type . '/' . $keys)); + watchdog('search', 'Searched %type for %keys.', array('%keys' => $keys, '%type' => $info['title']), WATCHDOG_NOTICE, l(t('results'), 'search/' . $info['path'] . '/' . $keys)); // Collect the search results. - $results = search_data($keys, $type); + $results = search_data($keys, $info['module']); // Construct the search form. - $build['search_form'] = drupal_get_form('search_form', NULL, $keys, $type); + $build['search_form'] = drupal_get_form('search_form', NULL, $keys, $info['module']); $build['search_results'] = array('#markup' => $results); return $build; } } - return drupal_get_form('search_form', NULL, empty($keys) ? '' : $keys, $type); + return drupal_get_form('search_form', NULL, empty($keys) ? '' : $keys, $info['module']); } /** @@ -113,10 +134,9 @@ function search_form_submit($form, &$form_state) { $keys = $form_state['values']['processed_keys']; if ($keys == '') { form_set_error('keys', t('Please enter some keywords.')); - // Fall through to the drupal_goto() call. + // Fall through to the form redirect. } - $type = $form_state['values']['module'] ? $form_state['values']['module'] : 'node'; $form_state['redirect'] = $form_state['action'] . '/' . $keys; return; } diff --git a/modules/search/search.test b/modules/search/search.test index b6a72d0bd..9079326a1 100644 --- a/modules/search/search.test +++ b/modules/search/search.test @@ -992,6 +992,9 @@ class SearchSimplifyTestCase extends DrupalWebTestCase { * Test config page. */ class SearchConfigSettingsForm extends DrupalWebTestCase { + public $search_user; + public $search_node; + public static function getInfo() { return array( 'name' => 'Config settings form', @@ -1001,27 +1004,36 @@ class SearchConfigSettingsForm extends DrupalWebTestCase { } function setUp() { - parent::setUp('search'); + parent::setUp('search', 'search_extra_type'); // Login as a user that can create and search content. - $this->drupalLogin($this->drupalCreateUser(array('search content', 'administer search', 'administer nodes', 'bypass node access'))); - } + $this->search_user = $this->drupalCreateUser(array('search content', 'administer search', 'administer nodes', 'bypass node access', 'access user profiles', 'administer users', 'administer blocks')); + $this->drupalLogin($this->search_user); - /** - * Verify the search settings form. - */ - function testSearchSettingsPage() { // Add a single piece of content and index it. $node = $this->drupalCreateNode(); - // Link the node to itself to test that it's only indexed once. + $this->search_node = $node; + // Link the node to itself to test that it's only indexed once. The content + // also needs the word "pizza" so we can use it as the search keyword. $langcode = LANGUAGE_NONE; $body_key = "body[$langcode][0][value]"; - $edit[$body_key] = l($node->title, 'node/' . $node->nid); + $edit[$body_key] = l($node->title, 'node/' . $node->nid) . ' pizza sandwich'; $this->drupalPost('node/' . $node->nid . '/edit', $edit, t('Save')); node_update_index(); search_update_totals(); + // Enable the search block. + $edit = array(); + $edit['search_form[region]'] = 'content'; + $this->drupalPost('admin/structure/block', $edit, t('Save blocks')); + } + + /** + * Verify the search settings form. + */ + function testSearchSettingsPage() { + // Test that the settings form displays the correct count of items left to index. $this->drupalGet('admin/config/search/settings'); $this->assertText(t('There are @count items left to index.', array('@count' => 0))); @@ -1034,6 +1046,95 @@ class SearchConfigSettingsForm extends DrupalWebTestCase { $this->drupalGet('admin/config/search/settings'); $this->assertText(t('There is 1 item left to index.')); } + + /** + * Verify that you can disable individual search modules. + */ + function testSearchModuleDisabling() { + // Array of search types to test: 'path' is the search path, 'title' is + // the tab title, 'keys' are the keywords to search for, and 'text' is + // the text to assert is on the results page. + $module_info = array( + 'node' => array( + 'path' => 'node', + 'title' => 'Content', + 'keys' => 'pizza', + 'text' => $this->search_node->title, + ), + 'user' => array( + 'path' => 'user', + 'title' => 'User', + 'keys' => $this->search_user->name, + 'text' => $this->search_user->mail, + ), + 'search_extra_type' => array( + 'path' => 'dummy_path', + 'title' => 'Dummy search type', + 'keys' => 'foo', + 'text' => 'Dummy search snippet to display', + ), + ); + $modules = array_keys($module_info); + + // Test each module if it's enabled as the only search module. + foreach ($modules as $module) { + // Enable the one module and disable other ones. + $info = $module_info[$module]; + $edit = array(); + foreach ($modules as $other) { + $edit['search_active_modules[' . $other . ']'] = (($other == $module) ? $module : FALSE); + } + $edit['search_default_module'] = $module; + $this->drupalPost('admin/config/search/settings', $edit, t('Save configuration')); + + // Run a search from the correct search URL. + $this->drupalGet('search/' . $info['path'] . '/' . $info['keys']); + $this->assertNoText('no results', $info['title'] . ' search found results'); + $this->assertText($info['text'], 'Correct search text found'); + + // Verify that other module search tab titles are not visible. + foreach ($modules as $other) { + if ($other != $module) { + $title = $module_info[$other]['title']; + $this->assertNoText($title, $title . ' search tab is not shown'); + } + } + + // Run a search from the search block on the node page. Verify you get + // to this module's search results page. + $terms = array('search_block_form' => $info['keys']); + $this->drupalPost('node', $terms, t('Search')); + $this->assertEqual( + $this->getURL(), + url('search/' . $info['path'] . '/' . $info['keys'], array('absolute' => TRUE)), + 'Block redirected to right search page'); + + // Try an invalid search path. Should redirect to our active module. + $this->drupalGet('search/not_a_module_path'); + $this->assertEqual( + $this->getURL(), + url('search/' . $info['path'], array('absolute' => TRUE)), + 'Invalid search path redirected to default search page'); + } + + // Test with all search modules enabled. When you go to the search + // page or run search, all modules should be shown. + $edit = array(); + foreach ($modules as $module) { + $edit['search_active_modules[' . $module . ']'] = $module; + } + $edit['search_default_module'] = 'node'; + + $this->drupalPost('admin/config/search/settings', $edit, t('Save configuration')); + + foreach (array('search/node/pizza', 'search/node') as $path) { + $this->drupalGet($path); + foreach ($modules as $module) { + $title = $module_info[$module]['title']; + $this->assertText($title, $title . ' search tab is shown'); + } + } + } } /** diff --git a/modules/search/tests/search_extra_type.info b/modules/search/tests/search_extra_type.info new file mode 100644 index 000000000..3a9a1f323 --- /dev/null +++ b/modules/search/tests/search_extra_type.info @@ -0,0 +1,8 @@ +; $Id$ +name = "Test search type" +description = "Support module for search module testing." +package = Testing +version = VERSION +core = 7.x +files[] = search_extra_type.module +hidden = TRUE diff --git a/modules/search/tests/search_extra_type.module b/modules/search/tests/search_extra_type.module new file mode 100644 index 000000000..0aaa9260a --- /dev/null +++ b/modules/search/tests/search_extra_type.module @@ -0,0 +1,34 @@ + 'Dummy search type', + 'path' => 'dummy_path', + ); +} + +/** + * Implements hook_search_execute(). + * + * This is a dummy search, so when search "executes", we just return a dummy + * result. + */ +function search_extra_type_search_execute() { + return array( + array( + 'link' => url('node'), + 'type' => 'Dummy result type', + 'title' => 'Dummy title', + 'snippet' => 'Dummy search snippet to display', + ), + ); +} -- cgit v1.2.3