From f7adc66bb435fac633961155fe60bd2bc902ff5f Mon Sep 17 00:00:00 2001 From: William Hearn Date: Sun, 31 Jan 2021 22:27:46 -0500 Subject: [PATCH] Issue #26552 by jhedstrom: Allow users with access to unpublished nodes to create unpublished books --- core/modules/book/book.module | 2 +- core/modules/book/src/BookManager.php | 25 +++---- .../book/tests/src/Functional/BookTest.php | 68 +++++++++++++++++-- 3 files changed, 72 insertions(+), 23 deletions(-) diff --git a/core/modules/book/book.module b/core/modules/book/book.module index 8bb1bb38a5..eef06e9ce9 100644 --- a/core/modules/book/book.module +++ b/core/modules/book/book.module @@ -97,7 +97,7 @@ function book_node_links_alter(array &$links, NodeInterface $node, array &$conte if ($context['view_mode'] == 'full' && node_is_page($node)) { $child_type = \Drupal::config('book.settings')->get('child_type'); $access_control_handler = \Drupal::entityTypeManager()->getAccessControlHandler('node'); - if (($account->hasPermission('add content to books') || $account->hasPermission('administer book outlines')) && $access_control_handler->createAccess($child_type) && $node->isPublished() && $node->book['depth'] < BookManager::BOOK_MAX_DEPTH) { + if (($account->hasPermission('add content to books') || $account->hasPermission('administer book outlines')) && $access_control_handler->createAccess($child_type) && $node->book['depth'] < BookManager::BOOK_MAX_DEPTH) { $book_links['book_add_child'] = [ 'title' => t('Add child page'), 'url' => Url::fromRoute('node.add', ['node_type' => $child_type], ['query' => ['parent' => $node->id()]]), diff --git a/core/modules/book/src/BookManager.php b/core/modules/book/src/BookManager.php index 0d6e205de7..504f042ea5 100644 --- a/core/modules/book/src/BookManager.php +++ b/core/modules/book/src/BookManager.php @@ -957,12 +957,10 @@ public function bookTreeCheckAccess(&$tree, $node_links = []) { // @todo This should be actually filtering on the desired node status // field language and just fall back to the default language. - $nids = \Drupal::entityQuery('node') - ->condition('nid', $nids, 'IN') - ->condition('status', 1) - ->execute(); + $book_links = $this->bookOutlineStorage->loadMultiple($nids); - foreach ($nids as $nid) { + foreach ($book_links as $book_link) { + $nid = $book_link['nid']; foreach ($node_links[$nid] as $mlid => $link) { $node_links[$nid][$mlid]['access'] = TRUE; } @@ -1002,19 +1000,14 @@ protected function doBookTreeCheckAccess(&$tree) { * {@inheritdoc} */ public function bookLinkTranslate(&$link) { - $node = NULL; - // Access will already be set in the tree functions. - if (!isset($link['access'])) { - $node = $this->entityTypeManager->getStorage('node')->load($link['nid']); - $link['access'] = $node && $node->access('view'); - } + // Check access via the api, since the query node_access tag doesn't check + // for unpublished nodes. + // @todo - load the nodes en-mass rather than individually. + // @see https://www.drupal.org/project/drupal/issues/2470896 + $node = $this->entityTypeManager->getStorage('node')->load($link['nid']); + $link['access'] = $node && $node->access('view'); // For performance, don't localize a link the user can't access. if ($link['access']) { - // @todo - load the nodes en-mass rather than individually. - if (!$node) { - $node = $this->entityTypeManager->getStorage('node') - ->load($link['nid']); - } // The node label will be the value for the current user's language. $link['title'] = $node->label(); $link['options'] = []; diff --git a/core/modules/book/tests/src/Functional/BookTest.php b/core/modules/book/tests/src/Functional/BookTest.php index 2937c355b1..69a308b7b3 100644 --- a/core/modules/book/tests/src/Functional/BookTest.php +++ b/core/modules/book/tests/src/Functional/BookTest.php @@ -37,14 +37,14 @@ class BookTest extends BrowserTestBase { /** * A user with permission to view a book and access printer-friendly version. * - * @var object + * @var \Drupal\user\UserInterface */ protected $webUser; /** * A user with permission to create and edit books and to administer blocks. * - * @var object + * @var \Drupal\user\UserInterface */ protected $adminUser; @@ -217,6 +217,14 @@ public function testBook() { $this->drupalLogin($this->bookAuthor); $book = $this->createBookNode('new'); $book->save(); + + // Confirm that an unpublished book page has the 'Add child page' link. + $this->drupalGet('node/' . $nodes[4]->id()); + $this->assertSession()->linkExists('Add child page'); + $nodes[4]->setUnPublished(); + $nodes[4]->save(); + $this->drupalGet('node/' . $nodes[4]->id()); + $this->assertSession()->linkExists('Add child page'); } /** @@ -285,6 +293,31 @@ public function testBookNavigationBlock() { $this->assertText($block->label(), 'Book navigation block is displayed.'); $this->assertText($this->book->label(), new FormattableMarkup('Link to book root (@title) is displayed.', ['@title' => $nodes[0]->label()])); $this->assertNoText($nodes[0]->label(), 'No links to individual book pages are displayed.'); + + // Ensure that an unpublished node does not appear in the navigation for a + // user without access. By unpublishing a parent page, child pages should + // not appear in the navigation. The node_access_test module is disabled + // since it interferes with this logic. + + /** @var \Drupal\Core\Extension\ModuleInstaller $installer */ + $installer = \Drupal::service('module_installer'); + $installer->uninstall(['node_access_test']); + node_access_rebuild(); + + $nodes[0]->setUnPublished(); + $nodes[0]->save(); + + // Verify the user does not have access to the unpublished node. + $this->assertFalse($nodes[0]->access('view', $this->webUser)); + + // Verify the unpublished book page does not appear in the navigation. + $this->drupalLogin($this->webUser); + $this->drupalGet($nodes[0]->toUrl()); + $this->assertSession()->statusCodeEquals(403); + $this->drupalGet($this->book->toUrl()); + $this->assertSession()->responseNotContains($nodes[0]->getTitle()); + $this->assertSession()->responseNotContains($nodes[1]->getTitle()); + $this->assertSession()->responseNotContains($nodes[2]->getTitle()); } /** @@ -487,14 +520,14 @@ public function testSaveBookLink() { $book_manager = \Drupal::service('book.manager'); // Mock a link for a new book. - $link = ['nid' => 1, 'has_children' => 0, 'original_bid' => 0, 'parent_depth_limit' => 8, 'pid' => 0, 'weight' => 0, 'bid' => 1]; + $link = ['nid' => 1, 'has_children' => 0, 'original_bid' => 0, 'pid' => 0, 'weight' => 0, 'bid' => 0]; $new = TRUE; // Save the link. $return = $book_manager->saveBookLink($link, $new); // Add the link defaults to $link so we have something to compare to the return from saveBookLink(). - $link += $book_manager->getLinkDefaults($link['nid']); + $link = $book_manager->getLinkDefaults($link['nid']); // Test the return from saveBookLink. $this->assertEqual($return, $link); @@ -559,7 +592,7 @@ public function testBookListing() { */ public function testAdminBookListing() { // Create a new book. - $this->createBook(); + $nodes = $this->createBook(); // Load the book page and assert the created book title is displayed. $this->drupalLogin($this->adminUser); @@ -572,7 +605,7 @@ public function testAdminBookListing() { */ public function testAdminBookNodeListing() { // Create a new book. - $this->createBook(); + $nodes = $this->createBook(); $this->drupalLogin($this->adminUser); // Load the book page list and assert the created book title is displayed @@ -582,6 +615,29 @@ public function testAdminBookNodeListing() { $elements = $this->xpath('//table//ul[@class="dropbutton"]/li/a'); $this->assertEqual($elements[0]->getText(), 'View', 'View link is found from the list.'); + $this->assertEquals(count($nodes), count($elements), 'All the book pages are displayed on the book outline page.'); + + // Unpublish a book in the hierarchy. + $nodes[0]->setUnPublished(); + $nodes[0]->save(); + + // Node should still appear on the outline for admins. + $this->drupalGet('admin/structure/book/' . $this->book->id()); + $elements = $this->xpath('//table//ul[@class="dropbutton"]/li/a'); + $this->assertEquals(count($nodes), count($elements), 'All the book pages are displayed on the book outline page.'); + + // Saving a book page not as the current version shouldn't effect the book. + $old_title = $nodes[1]->getTitle(); + $new_title = $this->randomGenerator->name(); + $nodes[1]->isDefaultRevision(FALSE); + $nodes[1]->setNewRevision(TRUE); + $nodes[1]->setTitle($new_title); + $nodes[1]->save(); + $this->drupalGet('admin/structure/book/' . $this->book->id()); + $elements = $this->xpath('//table//ul[@class="dropbutton"]/li/a'); + $this->assertEquals(count($nodes), count($elements), 'All the book pages are displayed on the book outline page.'); + $this->assertSession()->responseNotContains($new_title); + $this->assertSession()->responseContains($old_title); } /** -- 2.21.0 (Apple Git-122)