Fixed book-tree-gen page visibility issue

When book trees were generated, pages in chapters where ALL pages within
were not supposed to be visibile, would be visible due to the code
falling back on the raw relation which would not account for
permissions.

This has now been changed so that a custom 'visible_pages' attribute is set and used by any book tree structures, to ensure it does not fall back to the raw relation.

Added an extra test to cover.

For #2414

diff --git a/app/Entities/Chapter.php b/app/Entities/Chapter.php
index 3290afcfa6b90b0b01b653f99dfd7f93598f6556..5f5509efa22843925d218524183726924b2d8cb1 100644 (file)
--- a/app/Entities/Chapter.php
+++ b/app/Entities/Chapter.php
@@ -5,7 +5,6 @@ use Illuminate\Support\Collection;
 /**
  * Class Chapter
  * @property Collection<Page> $pages
- * @package BookStack\Entities
  */
 class Chapter extends BookChild
 {
@@ -52,15 +51,6 @@ class Chapter extends BookChild
         return mb_strlen($description) > $length ? mb_substr($description, 0, $length-3) . '...' : $description;
     }
 
-    /**
-     * Check if this chapter has any child pages.
-     * @return bool
-     */
-    public function hasChildren()
-    {
-        return count($this->pages) > 0;
-    }
-
     /**
      * Get the visible pages in this chapter.
      */

diff --git a/app/Entities/Managers/BookContents.php b/app/Entities/Managers/BookContents.php
index 52447e43bf0b397d4c69782fed8453793e3a13bf..b9844da5774baacccaf74b8543ec8b2054ca4ff5 100644 (file)
--- a/app/Entities/Managers/BookContents.php
+++ b/app/Entities/Managers/BookContents.php
@@ -53,12 +53,16 @@ class BookContents
         $pages->groupBy('chapter_id')->each(function ($pages, $chapter_id) use ($chapterMap, &$lonePages) {
             $chapter = $chapterMap->get($chapter_id);
             if ($chapter) {
-                $chapter->setAttribute('pages', collect($pages)->sortBy($this->bookChildSortFunc()));
+                $chapter->setAttribute('visible_pages', collect($pages)->sortBy($this->bookChildSortFunc()));
             } else {
                 $lonePages = $lonePages->concat($pages);
             }
         });
 
+        $chapters->whereNull('visible_pages')->each(function (Chapter $chapter) {
+            $chapter->setAttribute('visible_pages', collect([]));
+        });
+
         $all->each(function (Entity $entity) use ($renderPages) {
             $entity->setRelation('book', $this->book);
 

diff --git a/resources/views/books/export.blade.php b/resources/views/books/export.blade.php
index e86a24e816a339e0a443bffef08bbf28b7ab6685..f62b895827b7c87b0a3f47ed82d66037fd1d5ad1 100644 (file)
--- a/resources/views/books/export.blade.php
+++ b/resources/views/books/export.blade.php
@@ -41,9 +41,9 @@
         <ul class="contents">
             @foreach($bookChildren as $bookChild)
                 <li><a href="#{{$bookChild->getType()}}-{{$bookChild->id}}">{{ $bookChild->name }}</a></li>
-                @if($bookChild->isA('chapter') && count($bookChild->pages) > 0)
+                @if($bookChild->isA('chapter') && count($bookChild->visible_pages) > 0)
                     <ul>
-                        @foreach($bookChild->pages as $page)
+                        @foreach($bookChild->visible_pages as $page)
                             <li><a href="#page-{{$page->id}}">{{ $page->name }}</a></li>
                         @endforeach
                     </ul>
@@ -59,8 +59,8 @@
         @if($bookChild->isA('chapter'))
             <p>{{ $bookChild->description }}</p>
 
-            @if(count($bookChild->pages) > 0)
-                @foreach($bookChild->pages as $page)
+            @if(count($bookChild->visible_pages) > 0)
+                @foreach($bookChild->visible_pages as $page)
                     <div class="page-break"></div>
                     <div class="chapter-hint">{{$bookChild->name}}</div>
                     <h1 id="page-{{$page->id}}">{{ $page->name }}</h1>

diff --git a/resources/views/books/sort-box.blade.php b/resources/views/books/sort-box.blade.php
index 98f0af87eeeaa711408a6c010956bb68d9c273a3..c5d461629826221145ccffd5769088233be01ab9 100644 (file)
--- a/resources/views/books/sort-box.blade.php
+++ b/resources/views/books/sort-box.blade.php
@@ -28,7 +28,7 @@
                 </div>
                 @if($bookChild->isA('chapter'))
                     <ul>
-                        @foreach($bookChild->pages as $page)
+                        @foreach($bookChild->visible_pages as $page)
                             <li class="text-page"
                                 data-id="{{$page->id}}" data-type="page"
                                 data-name="{{ $page->name }}" data-created="{{ $page->created_at->timestamp }}"

diff --git a/resources/views/chapters/child-menu.blade.php b/resources/views/chapters/child-menu.blade.php
index 6137c34e8fce357653db2583e3a21cb413f01aba..a1358e1db4e0398cc8d339a79213df190a2ab3f5 100644 (file)
--- a/resources/views/chapters/child-menu.blade.php
+++ b/resources/views/chapters/child-menu.blade.php
@@ -1,10 +1,10 @@
 <div class="chapter-child-menu">
     <button chapter-toggle type="button" aria-expanded="{{ $isOpen ? 'true' : 'false' }}"
             class="text-muted @if($isOpen) open @endif">
-        @icon('caret-right') @icon('page') <span>{{ trans_choice('entities.x_pages', $bookChild->pages->count()) }}</span>
+        @icon('caret-right') @icon('page') <span>{{ trans_choice('entities.x_pages', $bookChild->visible_pages->count()) }}</span>
     </button>
     <ul class="sub-menu inset-list @if($isOpen) open @endif" @if($isOpen) style="display: block;" @endif role="menu">
-        @foreach($bookChild->pages as $childPage)
+        @foreach($bookChild->visible_pages as $childPage)
             <li class="list-item-page {{ $childPage->isA('page') && $childPage->draft ? 'draft' : '' }}" role="presentation">
                 @include('partials.entity-list-item-basic', ['entity' => $childPage, 'classes' => $current->matches($childPage)? 'selected' : '' ])
             </li>

diff --git a/resources/views/chapters/list-item.blade.php b/resources/views/chapters/list-item.blade.php
index 7e2e0e1c539c9dca666540a245c5cf2342ea9384..9186983332eaae842d8af07aff34abbfcfd6f994 100644 (file)
--- a/resources/views/chapters/list-item.blade.php
+++ b/resources/views/chapters/list-item.blade.php
@@ -1,4 +1,6 @@
-<a href="{{ $chapter->getUrl() }}" class="chapter entity-list-item @if($chapter->hasChildren()) has-children @endif" data-entity-type="chapter" data-entity-id="{{$chapter->id}}">
+{{--This view display child pages in a list if pre-loaded onto a 'visible_pages' property,--}}
+{{--To ensure that the pages have been loaded efficiently with permissions taken into account.--}}
+<a href="{{ $chapter->getUrl() }}" class="chapter entity-list-item @if($chapter->visible_pages->count() > 0) has-children @endif" data-entity-type="chapter" data-entity-id="{{$chapter->id}}">
     <span class="icon text-chapter">@icon('chapter')</span>
     <div class="content">
         <h4 class="entity-list-item-name break-text">{{ $chapter->name }}</h4>
@@ -7,16 +9,16 @@
         </div>
     </div>
 </a>
-@if ($chapter->hasChildren())
+@if ($chapter->visible_pages->count() > 0)
     <div class="chapter chapter-expansion">
         <span class="icon text-chapter">@icon('page')</span>
         <div class="content">
             <button type="button" chapter-toggle
                     aria-expanded="false"
-                    class="text-muted chapter-expansion-toggle">@icon('caret-right') <span>{{ trans_choice('entities.x_pages', $chapter->pages->count()) }}</span></button>
+                    class="text-muted chapter-expansion-toggle">@icon('caret-right') <span>{{ trans_choice('entities.x_pages', $chapter->visible_pages->count()) }}</span></button>
             <div class="inset-list">
                 <div class="entity-list-item-children">
-                    @include('partials.entity-list', ['entities' => $chapter->pages])
+                    @include('partials.entity-list', ['entities' => $chapter->visible_pages])
                 </div>
             </div>
         </div>

diff --git a/resources/views/partials/book-tree.blade.php b/resources/views/partials/book-tree.blade.php
index 5131af1aa36ae5728f12cd6c52e429e53aeaadd2..6e308bb09f06447f588b62e512edd5d3d588dc99 100644 (file)
--- a/resources/views/partials/book-tree.blade.php
+++ b/resources/views/partials/book-tree.blade.php
@@ -15,7 +15,7 @@
             <li class="list-item-{{ $bookChild->getClassName() }} {{ $bookChild->getClassName() }} {{ $bookChild->isA('page') && $bookChild->draft ? 'draft' : '' }}">
                 @include('partials.entity-list-item-basic', ['entity' => $bookChild, 'classes' => $current->matches($bookChild)? 'selected' : ''])
 
-                @if($bookChild->isA('chapter') && count($bookChild->pages) > 0)
+                @if($bookChild->isA('chapter') && count($bookChild->visible_pages) > 0)
                     <div class="entity-list-item no-hover">
                         <span role="presentation" class="icon text-chapter"></span>
                         <div class="content">

diff --git a/resources/views/search/book.blade.php b/resources/views/search/book.blade.php
deleted file mode 100644 (file)
index 36732c2..0000000
--- a/resources/views/search/book.blade.php
+++ /dev/null
@@ -1,24 +0,0 @@
-<div class="page-list">
-    @if(count($pages) > 0)
-        @foreach($pages as $pageIndex => $page)
-            <div class="anim searchResult" style="animation-delay: {{$pageIndex*50 . 'ms'}};">
-                @include('pages.list-item', ['page' => $page])
-                <hr>
-            </div>
-        @endforeach
-    @else
-        <p class="text-muted">{{ trans('entities.search_no_pages') }}</p>
-    @endif
-</div>
-
-@if(count($chapters) > 0)
-    <div class="page-list">
-        @foreach($chapters as $chapterIndex => $chapter)
-            <div class="anim searchResult" style="animation-delay: {{($chapterIndex+count($pages))*50 . 'ms'}};">
-                @include('chapters.list-item', ['chapter' => $chapter, 'hidePages' => true])
-                <hr>
-            </div>
-        @endforeach
-    </div>
-@endif
-

diff --git a/tests/Permissions/RestrictionsTest.php b/tests/Permissions/RestrictionsTest.php
index 7d6c1831afdd0eeab6ecceb36b3b5f8127f61981..2dcc0ea695e207887692525fcf3bd460fd8ef23c 100644 (file)
--- a/tests/Permissions/RestrictionsTest.php
+++ b/tests/Permissions/RestrictionsTest.php
@@ -490,6 +490,22 @@ class RestrictionsTest extends BrowserKitTest
             ->dontSee($page->name);
     }
 
+    public function test_restricted_chapter_pages_not_visible_on_book_page()
+    {
+        $chapter = Chapter::query()->first();
+        $this->actingAs($this->user)
+            ->visit($chapter->book->getUrl())
+            ->see($chapter->pages->first()->name);
+
+        foreach ($chapter->pages as $page) {
+            $this->setEntityRestrictions($page, []);
+        }
+
+        $this->actingAs($this->user)
+            ->visit($chapter->book->getUrl())
+            ->dontSee($chapter->pages->first()->name);
+    }
+
     public function test_bookshelf_update_restriction_override()
     {
         $shelf = Bookshelf::first();