diff --git a/admin_views.info b/admin_views.info index c8f6f8c..c1a47af 100644 --- a/admin_views.info +++ b/admin_views.info @@ -4,5 +4,5 @@ package = Administration core = 7.x dependencies[] = views dependencies[] = views_bulk_operations -files[] = admin_views.module files[] = plugins/views_plugin_display_system.inc +files[] = tests/admin_views.test diff --git a/plugins/views_plugin_display_system.inc b/plugins/views_plugin_display_system.inc index ba8f9de..d4d7faf 100644 --- a/plugins/views_plugin_display_system.inc +++ b/plugins/views_plugin_display_system.inc @@ -8,6 +8,24 @@ /** * Plugin to handle replacement of existing system paths. * + * The System display is mostly identical to Views' native Page display. The + * only differences are: + * - No menu link options. + * - No tab/local task options. + * These options make no sense, because the System display is supposed to + * replace the main page output of an existing system path only. This plugin + * code should therefore always be kept in sync with the Page display plugin + * code (excluding the support code for the removed functionality). + * + * @see views_plugin_display_page + * + * To achieve a correct replacement of an existing system path, + * execute_hook_menu() performs some advanced processing on menu router + * definitions to account for possible child router items that would normally + * inherit properties of the original system path. + * + * @see views_plugin_display_system::execute_hook_menu() + * * @ingroup views_display_plugins */ class views_plugin_display_system extends views_plugin_display { @@ -18,7 +36,6 @@ class views_plugin_display_system extends views_plugin_display { return TRUE; } - // @todo Needed? function uses_breadcrumb() { return TRUE; } @@ -35,6 +52,11 @@ class views_plugin_display_system extends views_plugin_display { /** * Add this display's path information to Drupal's menu system. + * + * @param array $callbacks + * All existing menu router items defined by modules. Taken by reference, in + * order to adjust any possibly existing child router items of the replaced + * system path. (This method only returns new router items normally.) */ function execute_hook_menu(&$callbacks) { $items = array(); @@ -62,6 +84,10 @@ class views_plugin_display_system extends views_plugin_display { if (!$path) { return $items; } + // Only existing system paths can be replaced. + if (!isset($callbacks[$path])) { + return $items; + } $access_plugin = $this->get_plugin('access'); if (!isset($access_plugin)) { @@ -108,45 +134,55 @@ class views_plugin_display_system extends views_plugin_display { 'load arguments' => array($this->view->name, $this->display->id, '%index'), ); - // Setup a list of keys on the router item for $path, which we are going to - // replace. Since the menu system inherits router item properties to child - // paths, we need to ensure that child paths do not inherit the properties - // for the view, but instead, get the original properties of $path. - $keys = array_fill_keys(array_keys($items[$path]), NULL); - $keys += array( + // List of router item default property values, which are inherited to + // children. These default values are only applied if the original parent + // item does not define them (see below). + // @see _menu_router_build() + $defaults = array( + 'access callback' => 'user_access', + 'menu_name' => NULL, 'file' => NULL, 'file path' => NULL, + 'delivery callback' => NULL, + 'theme callback' => NULL, + 'theme arguments' => array(), ); - // Grep all callbacks for router items below the target path. + // Grep all router items below the target path. + $parent = &$callbacks[$path]; $num_parent_parts = count(explode('/', $path)); $children = preg_grep('@^' . preg_quote($path, '@') . '/@', array_keys($callbacks)); + // Ensure correct inheritance of properties on the original parent path // (being replaced) to child items. foreach ($children as $child_path) { - // Default local tasks can be skipped, since the only properties that - // matter for them are 'title', 'description', and 'weight'. - if (!isset($callbacks[$child_path]['type']) || $callbacks[$child_path]['type'] != MENU_DEFAULT_LOCAL_TASK) { - // If this is a direct child, copy original properties form parent path. - $num_child_parts = count(explode('/', $child_path)); - if (isset($callbacks[$path]) && ($num_parent_parts + 1 == $num_child_parts)) { - // Copy properties from the parent item, which are not set on the - // child. The menu system would normally inherit these to the child. - $callbacks[$child_path] += array_intersect_key($callbacks[$path], $keys); - // Some properties may not be set on the original parent item, but - // are set on the new views parent item. The menu system would inherit - // these to children; prevent that by setting NULL values. - $callbacks[$child_path] += $keys; - } + // Only apply property inheritance to direct children of the parent path. + $num_child_parts = count(explode('/', $child_path)); + if ($num_parent_parts == $num_child_parts - 1) { + $child = &$callbacks[$child_path]; + // Copy all properties from the original parent that will be replaced + // with new values. + // This typically resets 'access arguments', etc. + $child += array_intersect_key($parent, $items[$path]); + // Copy all properties from the original parent, for which the router + // system would inherit parent values or fill in default values. + // This typically adds back 'file' and other properties defined on the + // parent but not on $items[$path]. (The two operations could be + // combined with $items[$path] + $defaults, but are separated for + // documentation purposes and clarity.) + $child += array_intersect_key($parent, $defaults); + // Last catch-22, insert new default properties and their default values + // for the child, which may not be defined on the original parent. + // This typically inserts 'access callback', which can be omitted in + // router item definitions and only gets a default of user_access() in + // the final _menu_router_build(). Without this, the new access callback + // views_access() in $items[$path] would be inherited to all children. + $child += $defaults; } } - // If the original parent path already existed, copy over its remaining // properties. - // @todo Above inheritance code can probably be moved into this condition. - if (isset($callbacks[$path])) { - $items[$path] += $callbacks[$path]; - } + $items[$path] += $parent; return $items; } @@ -158,6 +194,7 @@ class views_plugin_display_system extends views_plugin_display { * Also set the page title, because original page callbacks might do this. */ function execute() { + // Let the world know that this is the page view we're using. views_set_page_view($this->view); // Prior to this being called, the $view should already be set to this @@ -166,16 +203,22 @@ class views_plugin_display_system extends views_plugin_display { if (!empty($this->view->build_info['fail'])) { return drupal_not_found(); } + + if (!empty($this->view->build_info['denied'])) { + return drupal_access_denied(); + } + // @todo Needed? $this->view->get_breadcrumb(TRUE); - drupal_set_title(filter_xss_admin($this->view->get_title()), PASS_THROUGH); + // And now render the view. + $render = $this->view->render(); // @todo This is super-likely the totally wrong place to add our CSS; better // suggestions and patches welcome. drupal_add_css(drupal_get_path('module', 'admin_views') . '/admin_views.css'); - return $this->view->render(); + return $render; } /** @@ -213,7 +256,7 @@ class views_plugin_display_system extends views_plugin_display { $form['#title'] .= t('An existing menu path this view replaces'); $form['path'] = array( '#type' => 'textfield', - '#description' => t('This view replaces this path on your site. You may use "%" for dynamic arguments. For example: "node/%/feed".'), + '#description' => t('This view replaces this path on your site. You may use "%" in your URL to represent values that will be used for contextual filters. For example: "node/%/feed".'), '#default_value' => $this->get_option('path'), '#field_prefix' => '' . url(NULL, array('absolute' => TRUE)) . (variable_get('clean_url', 0) ? '' : '?q='), '#field_suffix' => '‎', @@ -228,6 +271,10 @@ class views_plugin_display_system extends views_plugin_display { switch ($form_state['section']) { case 'path': + if (strpos($form_state['values']['path'], '$arg') !== FALSE) { + form_error($form['path'], t('"$arg" is no longer supported. Use % instead.')); + } + if (strpos($form_state['values']['path'], '%') === 0) { form_error($form['path'], t('"%" may not be used for the first segment of a path.')); } diff --git a/tests/admin_views.test b/tests/admin_views.test new file mode 100644 index 0000000..28709a6 --- /dev/null +++ b/tests/admin_views.test @@ -0,0 +1,73 @@ + array('administer users', 'administer permissions'), + 'node' => array('access content overview'), + 'comment' => array('administer comments'), + 'taxonomy' => array('administer taxonomy'), + ); + + function setUp() { + $modules = func_get_args(); + $modules = isset($modules[0]) && is_array($modules[0]) ? $modules[0] : $modules; + $modules[] = 'admin_views'; + parent::setUp($modules); + + $permissions = array( + 'access administration pages', + ); + foreach ($this->permissionMap as $module => $module_permissions) { + if (module_exists($module)) { + $permissions = array_merge($permissions, $module_permissions); + } + } + $this->admin_user = $this->drupalCreateUser($permissions); + } +} + +/** + * Tests System display functionality. + */ +class AdminViewsSystemDisplayTestCase extends AdminViewsWebTestCase { + public static function getInfo() { + return array( + 'name' => 'Views System display plugin', + 'description' => 'Tests Views System display plugin functionality.', + 'group' => 'Administration views', + ); + } + + /** + * Tests proper inheritance of router item properties. + */ + function testRouterItemInheritance() { + $path = 'admin/people'; + $item = menu_get_item($path); + + $this->drupalLogin($this->admin_user); + $out = $this->drupalGet($path); + // Verify a regular page response. + $this->assertResponse(200); + $this->assertText($item['title']); + + // Verify that local tasks and actions exist and can be accessed. + foreach (array(t('List'), t('Permissions'), t('Add user')) as $link) { + $this->drupalSetContent($out); + $this->assertLink($link); + $this->clickLink($link); + $this->assertResponse(200); + } + } +} +