From 8514a7e283b65f1085e0f3a96e1e701157532e88 Mon Sep 17 00:00:00 2001 From: Dries Buytaert <dries@buytaert.net> Date: Sat, 14 Nov 2009 07:58:50 +0000 Subject: [PATCH] - Patch #301902 by TheRec, beeradb, catch, sun, skilip, alpritt, JacobSingh, Senpai: usability improvement: allow more users to see the node admin page. --- modules/node/node.admin.inc | 169 ++++++++++++++------ modules/node/node.install | 10 ++ modules/node/node.module | 6 +- modules/node/node.test | 117 ++++++++++---- modules/simpletest/drupal_web_test_case.php | 40 +++++ modules/tracker/tracker.test | 2 +- 6 files changed, 265 insertions(+), 79 deletions(-) diff --git a/modules/node/node.admin.inc b/modules/node/node.admin.inc index 9079936b2698..f9c21155dd32 100644 --- a/modules/node/node.admin.inc +++ b/modules/node/node.admin.inc @@ -396,7 +396,7 @@ function node_admin_content($form, $form_state) { '#access' => _node_add_access(), '#markup' => theme('links', array('links' => array(array('title' => t('Add new content'), 'href' => 'node/add')), 'attributes' => array('class' => array('action-links')))), ); - $form[] = node_filter_form(); + $form['filter'] = node_filter_form(); $form['#submit'][] = 'node_filter_form_submit'; $form['#theme'] = 'node_filter_form'; $form['admin'] = node_admin_nodes(); @@ -408,33 +408,7 @@ function node_admin_content($form, $form_state) { * Form builder: Builds the node administration overview. */ function node_admin_nodes() { - // Enable language column if translation module is enabled - // or if we have any node with language. - $multilanguage = (module_exists('translation') || db_query("SELECT COUNT(*) FROM {node} WHERE language <> ''")->fetchField()); - - // Build the sortable table header. - $header = array( - 'title' => array('data' => t('Title'), 'field' => 'n.title'), - 'type' => array('data' => t('Type'), 'field' => 'n.type'), - 'author' => array('data' => t('Author'), 'field' => 'u.name'), - 'status' => array('data' => t('Status'), 'field' => 'n.status'), - 'changed' => array('data' => t('Updated'), 'field' => 'n.changed', 'sort' => 'desc') - ); - if ($multilanguage) { - $header['language'] = array('data' => t('Language'), 'field' => 'n.language'); - } - $header['operations'] = array('data' => t('Operations')); - - $query = db_select('node', 'n')->extend('PagerDefault')->extend('TableSort'); - $query->join('users', 'u', 'n.uid = u.uid'); - node_build_filter_query($query); - - $result = $query - ->fields('n') - ->fields('u', array('name')) - ->limit(50) - ->orderByHeader($header) - ->execute(); + $admin_access = user_access('administer nodes'); // Build the 'Update options' form. $form['options'] = array( @@ -442,6 +416,7 @@ function node_admin_nodes() { '#title' => t('Update options'), '#prefix' => '<div class="container-inline">', '#suffix' => '</div>', + '#access' => $admin_access, ); $options = array(); foreach (module_invoke_all('node_operations') as $operation => $array) { @@ -452,52 +427,148 @@ function node_admin_nodes() { '#options' => $options, '#default_value' => 'approve', ); - $options = array(); $form['options']['submit'] = array( '#type' => 'submit', '#value' => t('Update'), - '#submit' => array('node_admin_nodes_submit'), '#validate' => array('node_admin_nodes_validate'), + '#submit' => array('node_admin_nodes_submit'), ); + // Enable language column if translation module is enabled + // or if we have any node with language. + $multilanguage = (module_exists('translation') || db_query("SELECT COUNT(*) FROM {node} WHERE language <> ''")->fetchField()); + + // Build the sortable table header. + $header = array( + 'title' => array('data' => t('Title'), 'field' => 'n.title'), + 'type' => array('data' => t('Type'), 'field' => 'n.type'), + 'author' => array('data' => t('Author'), 'field' => 'u.name'), + 'status' => array('data' => t('Status'), 'field' => 'n.status'), + 'changed' => array('data' => t('Updated'), 'field' => 'n.changed', 'sort' => 'desc') + ); + if ($multilanguage) { + $header['language'] = array('data' => t('Language'), 'field' => 'n.language'); + } + $header['operations'] = array('data' => t('Operations')); + + $query = db_select('node', 'n')->extend('PagerDefault')->extend('TableSort'); + node_build_filter_query($query); + + if (!user_access('bypass node access')) { + // If the user is able to view their own unpublished nodes, allow them + // to see these in addition to published nodes. Check that they actually + // have some unpublished nodes to view before adding the condition. + if (user_access('view own unpublished content') && $own_unpublished = db_query('SELECT nid FROM {node} WHERE uid = :uid AND status = :status', array(':uid' => $GLOBALS['user']->uid, ':status' => 0))->fetchCol()) { + $query->condition(db_or() + ->condition('n.status', 1) + ->condition('n.nid', $own_unpublished, 'IN') + ); + } + else { + // If not, restrict the query to published nodes. + $query->condition('n.status', 1); + } + } + $nids = $query + ->fields('n',array('nid')) + ->limit(50) + ->orderByHeader($header) + ->execute() + ->fetchCol(); + $nodes = node_load_multiple($nids); + + // Prepare the list of nodes. $languages = language_list(); $destination = drupal_get_destination(); - $nodes = array(); - foreach ($result as $node) { - $l_options = empty($node->language) ? array() : array('language' => $languages[$node->language]); + $options = array(); + foreach ($nodes as $node) { + $l_options = !empty($node->language) ? array('language' => $languages[$node->language]) : array(); $options[$node->nid] = array( 'title' => array( 'data' => array( '#type' => 'link', - '#title' => $node->title, + '#title' => $node->title[FIELD_LANGUAGE_NONE][0]['value'], '#href' => 'node/' . $node->nid, '#options' => $l_options, '#suffix' => ' ' . theme('mark', array('type' => node_mark($node->nid, $node->changed))), ), ), - 'type' => check_plain(node_type_get_name($node)), + 'type' => check_plain(node_type_get_name($node)), 'author' => theme('username', array('account' => $node)), - 'status' => $node->status ? t('published') : t('not published'), + 'status' => $node->status ? t('published') : t('not published'), 'changed' => format_date($node->changed, 'short'), ); if ($multilanguage) { $options[$node->nid]['language'] = empty($node->language) ? t('Language neutral') : t($languages[$node->language]->name); } - $options[$node->nid]['operations'] = array( - 'data' => array( - '#type' => 'link', - '#title' => t('edit'), - '#href' => 'node/' . $node->nid . '/edit', - '#options' => array('query' => $destination), - ), + // Build a list of all the accessible operations for the current node. + $operations = array(); + if (node_access('update', $node)) { + $operations['edit'] = array( + 'title' => t('edit'), + 'href' => 'node/' . $node->nid . '/edit', + 'query' => $destination, + ); + } + if (node_access('delete', $node)) { + $operations['delete'] = array( + 'title' => t('delete'), + 'href' => 'node/' . $node->nid . '/delete', + 'query' => $destination, + ); + } + $options[$node->nid]['operations'] = array(); + if (count($operations) > 1) { + // Render an unordered list of operations links. + $options[$node->nid]['operations'] = array( + 'data' => array( + '#theme' => 'links', + '#links' => $operations, + '#attributes' => array('class' => array('links', 'inline')), + ), + ); + } + elseif (!empty($operations)) { + // Render the first and only operation as a link. + $link = reset($operations); + $options[$node->nid]['operations'] = array( + 'data' => array( + '#type' => 'link', + '#title' => $link['title'], + '#href' => $link['href'], + '#options' => array('query' => $link['query']), + ), + ); + } + } + + // Only use a tableselect when the current user is able to perform any + // operations. + if ($admin_access) { + $form['nodes'] = array( + '#type' => 'tableselect', + '#header' => $header, + '#options' => $options, + '#empty' => t('No content available.'), ); } - $form['nodes'] = array( - '#type' => 'tableselect', - '#header' => $header, - '#options' => $options, - '#empty' => t('No content available.'), - ); + // Otherwise, use a simple table. + else { + // Display an empty message like in the tableselect. + if (empty($options)) { + $row = array( + 'data' => t('No content available.'), + 'colspan' => count($header), + ); + $options = array($row); + } + $form['nodes'] = array( + '#theme' => 'table', + '#header' => $header, + '#rows' => $options, + ); + } + $form['pager'] = array('#markup' => theme('pager', array('tags' => NULL))); return $form; } diff --git a/modules/node/node.install b/modules/node/node.install index 9c4c2201f979..272c1fbd39b4 100644 --- a/modules/node/node.install +++ b/modules/node/node.install @@ -555,6 +555,16 @@ function node_update_7007() { db_drop_field('node_type', 'min_word_count'); } +/** + * Split the 'administer nodes' permission from 'access content overview'. + */ +function node_update_7008() { + $roles = user_roles(FALSE, 'administer nodes'); + foreach ($roles as $rid => $role) { + user_role_grant_permissions($rid, array('access content overview')); + } +} + /** * @} End of "defgroup updates-6.x-to-7.x" * The next series of updates should start at 8000. diff --git a/modules/node/node.module b/modules/node/node.module index 53e285ce623c..90509ffb5d55 100644 --- a/modules/node/node.module +++ b/modules/node/node.module @@ -1391,6 +1391,10 @@ function node_permission() { 'title' => t('Access content'), 'description' => t('View published content.'), ), + 'access content overview' => array( + 'title' => t('Access content overview'), + 'description' => t('Access the content overview page.'), + ), 'bypass node access' => array( 'title' => t('Bypass node access'), 'description' => t('View, edit and delete all site content. Users with this permission will bypass any content-related access control. %warning', array('%warning' => t('Warning: Give to trusted roles only; this permission has security implications.'))), @@ -1741,7 +1745,7 @@ function node_menu() { 'description' => 'Find and manage content.', 'page callback' => 'drupal_get_form', 'page arguments' => array('node_admin_content'), - 'access arguments' => array('administer nodes'), + 'access arguments' => array('access content overview'), 'weight' => -10, 'file' => 'node.admin.inc', ); diff --git a/modules/node/node.test b/modules/node/node.test index d3bc04f54fed..807dc5f652d9 100644 --- a/modules/node/node.test +++ b/modules/node/node.test @@ -950,57 +950,118 @@ class NodeAccessRebuildTestCase extends DrupalWebTestCase { * Test node administration page functionality. */ class NodeAdminTestCase extends DrupalWebTestCase { - protected $admin_user; - public static function getInfo() { return array( 'name' => 'Node administration', 'description' => 'Test node administration page functionality.', - 'group' => 'Node' + 'group' => 'Node', ); } function setUp() { parent::setUp(); - $this->admin_user = $this->drupalCreateUser(array('administer nodes', 'create article content', 'create page content')); - $this->drupalLogin($this->admin_user); + + // Remove the "view own unpublished content" permission which is set + // by default for authenticated users so we can test this permission + // correctly. + user_role_revoke_permissions(DRUPAL_AUTHENTICATED_RID, array('view own unpublished content')); + + $this->admin_user = $this->drupalCreateUser(array('access administration pages', 'access content overview', 'administer nodes', 'bypass node access')); + $this->base_user_1 = $this->drupalCreateUser(array('access content overview')); + $this->base_user_2 = $this->drupalCreateUser(array('access content overview', 'view own unpublished content')); + $this->base_user_3 = $this->drupalCreateUser(array('access content overview', 'bypass node access')); } /** - * Create 3 nodes and test if they are listed on the node admistration page. + * Tests content overview with different user permissions. */ - function testNodeAdmin() { - $this->drupalPost('admin/content', array(), t('Update')); - $this->assertText(t('No items selected.'), t('Clicking update with no nodes displays error message on the node administration listing.')); + function testContentAdminPages() { + $this->drupalLogin($this->admin_user); - $node1 = $this->drupalCreateNode(array('type' => 'article', 'status' => 1)); - $node2 = $this->drupalCreateNode(array('type' => 'article', 'status' => 0)); - $node3 = $this->drupalCreateNode(array('type' => 'page')); + $nodes['published_page'] = $this->drupalCreateNode(array('type' => 'page')); + $nodes['published_article'] = $this->drupalCreateNode(array('type' => 'article')); + $nodes['unpublished_page_1'] = $this->drupalCreateNode(array('type' => 'page', 'uid' => $this->base_user_1->uid, 'status' => 0)); + $nodes['unpublished_page_2'] = $this->drupalCreateNode(array('type' => 'page', 'uid' => $this->base_user_2->uid, 'status' => 0)); + // Verify view, edit, and delete links for any content. $this->drupalGet('admin/content'); - $this->assertText($node1->title[FIELD_LANGUAGE_NONE][0]['value'], t('Node appears on the node administration listing.')); - - $this->drupalPost('admin/content', array(), t('Update')); - $this->assertText(t('No items selected.'), t('Clicking update with no selected nodes displays error message on the node administration listing.')); + $this->assertResponse(200); + foreach ($nodes as $node) { + $this->assertLinkByHref('node/' . $node->nid); + $this->assertLinkByHref('node/' . $node->nid . '/edit'); + $this->assertLinkByHref('node/' . $node->nid . '/delete'); + // Verify tableselect. + $this->assertFieldByName('nodes[' . $node->nid . ']', '', t('Tableselect found.')); + } - // Filter the node listing by status. + // Verify filtering by publishing status. $edit = array( 'status' => 'status-1', ); - $this->drupalPost('admin/content', $edit, t('Filter')); - $this->assertRaw(t('<strong>%type</strong> is <strong>%value</strong>', array('%type' => t('status'), '%value' => t('published'))), t('The node administration listing is filtered by status.')); - $this->assertText($node1->title[FIELD_LANGUAGE_NONE][0]['value'], t('Published node appears on the node administration listing.')); - $this->assertNoText($node2->title[FIELD_LANGUAGE_NONE][0]['value'], t('Unpublished node does not appear on the node administration listing.')); + $this->drupalPost(NULL, $edit, t('Filter')); + $this->assertRaw(t('<strong>%type</strong> is <strong>%value</strong>', array('%type' => t('status'), '%value' => t('published'))), t('Content list is filtered by status.')); + $this->assertLinkByHref('node/' . $nodes['published_page']->nid . '/edit'); + $this->assertLinkByHref('node/' . $nodes['published_article']->nid . '/edit'); + $this->assertNoLinkByHref('node/' . $nodes['unpublished_page_1']->nid . '/edit'); - // Filter the node listing by content type. + // Verify filtering by status and content type. $edit = array( - 'type' => 'article', + 'type' => 'page', ); - $this->drupalPost('admin/content', $edit, t('Refine')); - $this->assertRaw(t('<strong>%type</strong> is <strong>%value</strong>', array('%type' => t('status'), '%value' => t('published'))), t('The node administration listing is filtered by status.')); - $this->assertRaw(t('<strong>%type</strong> is <strong>%value</strong>', array('%type' => t('type'), '%value' => 'Article')), t('The node administration listing is filtered by content type.')); - $this->assertText($node1->title[FIELD_LANGUAGE_NONE][0]['value'], t('Article node appears on the node administration listing.')); - $this->assertNoText($node3->title[FIELD_LANGUAGE_NONE][0]['value'], t('Page node does not appear on the node administration listing.')); + $this->drupalPost(NULL, $edit, t('Refine')); + $this->assertRaw(t('<strong>%type</strong> is <strong>%value</strong>', array('%type' => t('status'), '%value' => t('published'))), t('Content list is filtered by status.')); + $this->assertRaw(t('<strong>%type</strong> is <strong>%value</strong>', array('%type' => t('type'), '%value' => 'Page')), t('Content list is filtered by content type.')); + $this->assertLinkByHref('node/' . $nodes['published_page']->nid . '/edit'); + $this->assertNoLinkByHref('node/' . $nodes['published_article']->nid . '/edit'); + + // Verify no operation links are displayed for regular users. + $this->drupalLogout(); + $this->drupalLogin($this->base_user_1); + $this->drupalGet('admin/content'); + $this->assertResponse(200); + $this->assertLinkByHref('node/' . $nodes['published_page']->nid); + $this->assertLinkByHref('node/' . $nodes['published_article']->nid); + $this->assertNoLinkByHref('node/' . $nodes['published_page']->nid . '/edit'); + $this->assertNoLinkByHref('node/' . $nodes['published_page']->nid . '/delete'); + $this->assertNoLinkByHref('node/' . $nodes['published_article']->nid . '/edit'); + $this->assertNoLinkByHref('node/' . $nodes['published_article']->nid . '/delete'); + + // Verify no unpublished content is displayed without permission. + $this->assertNoLinkByHref('node/' . $nodes['unpublished_page_1']->nid); + $this->assertNoLinkByHref('node/' . $nodes['unpublished_page_1']->nid . '/edit'); + $this->assertNoLinkByHref('node/' . $nodes['unpublished_page_1']->nid . '/delete'); + + // Verify no tableselect. + $this->assertNoFieldByName('nodes[' . $nodes['published_page']->nid . ']', '', t('No tableselect found.')); + + // Verify unpublished content is displayed with permission. + $this->drupalLogout(); + $this->drupalLogin($this->base_user_2); + $this->drupalGet('admin/content'); + $this->assertResponse(200); + $this->assertLinkByHref('node/' . $nodes['unpublished_page_2']->nid); + // Verify no operation links are displayed. + $this->assertNoLinkByHref('node/' . $nodes['unpublished_page_2']->nid . '/edit'); + $this->assertNoLinkByHref('node/' . $nodes['unpublished_page_2']->nid . '/delete'); + + // Verify user cannot see unpublished content of other users. + $this->assertNoLinkByHref('node/' . $nodes['unpublished_page_1']->nid); + $this->assertNoLinkByHref('node/' . $nodes['unpublished_page_1']->nid . '/edit'); + $this->assertNoLinkByHref('node/' . $nodes['unpublished_page_1']->nid . '/delete'); + + // Verify no tableselect. + $this->assertNoFieldByName('nodes[' . $nodes['unpublished_page_2']->nid . ']', '', t('No tableselect found.')); + + // Verify node access can be bypassed. + $this->drupalLogout(); + $this->drupalLogin($this->base_user_3); + $this->drupalGet('admin/content'); + $this->assertResponse(200); + foreach ($nodes as $node) { + $this->assertLinkByHref('node/' . $node->nid); + $this->assertLinkByHref('node/' . $node->nid . '/edit'); + $this->assertLinkByHref('node/' . $node->nid . '/delete'); + } } } diff --git a/modules/simpletest/drupal_web_test_case.php b/modules/simpletest/drupal_web_test_case.php index 98f4445ba607..05d4e58281e4 100644 --- a/modules/simpletest/drupal_web_test_case.php +++ b/modules/simpletest/drupal_web_test_case.php @@ -1763,6 +1763,46 @@ protected function assertNoLink($label, $message = '', $group = 'Other') { return $this->assert(empty($links), $message, $group); } + /** + * Pass if a link containing a given href (part) is found. + * + * @param $href + * The full or partial value of the 'href' attribute of the anchor tag. + * @param $index + * Link position counting from zero. + * @param $message + * Message to display. + * @param $group + * The group this message belongs to, defaults to 'Other'. + * + * @return + * TRUE if the assertion succeeded, FALSE otherwise. + */ + protected function assertLinkByHref($href, $index = 0, $message = '', $group = 'Other') { + $links = $this->xpath('//a[contains(@href, "' . $href . '")]'); + $message = ($message ? $message : t('Link containing href %href found.', array('%href' => $href))); + return $this->assert(isset($links[$index]), $message, $group); + } + + /** + * Pass if a link containing a given href (part) is not found. + * + * @param $href + * The full or partial value of the 'href' attribute of the anchor tag. + * @param $message + * Message to display. + * @param $group + * The group this message belongs to, defaults to 'Other'. + * + * @return + * TRUE if the assertion succeeded, FALSE otherwise. + */ + protected function assertNoLinkByHref($href, $message = '', $group = 'Other') { + $links = $this->xpath('//a[contains(@href, "' . $href . '")]'); + $message = ($message ? $message : t('No link containing href %href found.', array('%href' => $href))); + return $this->assert(empty($links), $message, $group); + } + /** * Follows a link by name. * diff --git a/modules/tracker/tracker.test b/modules/tracker/tracker.test index e304be3103ee..2e67b9860241 100644 --- a/modules/tracker/tracker.test +++ b/modules/tracker/tracker.test @@ -213,7 +213,7 @@ class TrackerTest extends DrupalWebTestCase { * Test that publish/unpublish works at admin/content/node */ function testTrackerAdminUnpublish() { - $admin_user = $this->drupalCreateUser(array('administer nodes')); + $admin_user = $this->drupalCreateUser(array('access content overview', 'administer nodes', 'bypass node access')); $this->drupalLogin($admin_user); $node = $this->drupalCreateNode(array( -- GitLab