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
This commit is contained in:
Dan Brown 2020-12-17 17:31:18 +00:00
parent 884664bfe9
commit 3f3fad7113
No known key found for this signature in database
GPG Key ID: 46D9F943C24A2EF9
9 changed files with 35 additions and 47 deletions

View File

@ -5,7 +5,6 @@ use Illuminate\Support\Collection;
/** /**
* Class Chapter * Class Chapter
* @property Collection<Page> $pages * @property Collection<Page> $pages
* @package BookStack\Entities
*/ */
class Chapter extends BookChild class Chapter extends BookChild
{ {
@ -52,15 +51,6 @@ class Chapter extends BookChild
return mb_strlen($description) > $length ? mb_substr($description, 0, $length-3) . '...' : $description; 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. * Get the visible pages in this chapter.
*/ */

View File

@ -53,12 +53,16 @@ class BookContents
$pages->groupBy('chapter_id')->each(function ($pages, $chapter_id) use ($chapterMap, &$lonePages) { $pages->groupBy('chapter_id')->each(function ($pages, $chapter_id) use ($chapterMap, &$lonePages) {
$chapter = $chapterMap->get($chapter_id); $chapter = $chapterMap->get($chapter_id);
if ($chapter) { if ($chapter) {
$chapter->setAttribute('pages', collect($pages)->sortBy($this->bookChildSortFunc())); $chapter->setAttribute('visible_pages', collect($pages)->sortBy($this->bookChildSortFunc()));
} else { } else {
$lonePages = $lonePages->concat($pages); $lonePages = $lonePages->concat($pages);
} }
}); });
$chapters->whereNull('visible_pages')->each(function (Chapter $chapter) {
$chapter->setAttribute('visible_pages', collect([]));
});
$all->each(function (Entity $entity) use ($renderPages) { $all->each(function (Entity $entity) use ($renderPages) {
$entity->setRelation('book', $this->book); $entity->setRelation('book', $this->book);

View File

@ -41,9 +41,9 @@
<ul class="contents"> <ul class="contents">
@foreach($bookChildren as $bookChild) @foreach($bookChildren as $bookChild)
<li><a href="#{{$bookChild->getType()}}-{{$bookChild->id}}">{{ $bookChild->name }}</a></li> <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> <ul>
@foreach($bookChild->pages as $page) @foreach($bookChild->visible_pages as $page)
<li><a href="#page-{{$page->id}}">{{ $page->name }}</a></li> <li><a href="#page-{{$page->id}}">{{ $page->name }}</a></li>
@endforeach @endforeach
</ul> </ul>
@ -59,8 +59,8 @@
@if($bookChild->isA('chapter')) @if($bookChild->isA('chapter'))
<p>{{ $bookChild->description }}</p> <p>{{ $bookChild->description }}</p>
@if(count($bookChild->pages) > 0) @if(count($bookChild->visible_pages) > 0)
@foreach($bookChild->pages as $page) @foreach($bookChild->visible_pages as $page)
<div class="page-break"></div> <div class="page-break"></div>
<div class="chapter-hint">{{$bookChild->name}}</div> <div class="chapter-hint">{{$bookChild->name}}</div>
<h1 id="page-{{$page->id}}">{{ $page->name }}</h1> <h1 id="page-{{$page->id}}">{{ $page->name }}</h1>

View File

@ -28,7 +28,7 @@
</div> </div>
@if($bookChild->isA('chapter')) @if($bookChild->isA('chapter'))
<ul> <ul>
@foreach($bookChild->pages as $page) @foreach($bookChild->visible_pages as $page)
<li class="text-page" <li class="text-page"
data-id="{{$page->id}}" data-type="page" data-id="{{$page->id}}" data-type="page"
data-name="{{ $page->name }}" data-created="{{ $page->created_at->timestamp }}" data-name="{{ $page->name }}" data-created="{{ $page->created_at->timestamp }}"

View File

@ -1,10 +1,10 @@
<div class="chapter-child-menu"> <div class="chapter-child-menu">
<button chapter-toggle type="button" aria-expanded="{{ $isOpen ? 'true' : 'false' }}" <button chapter-toggle type="button" aria-expanded="{{ $isOpen ? 'true' : 'false' }}"
class="text-muted @if($isOpen) open @endif"> 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> </button>
<ul class="sub-menu inset-list @if($isOpen) open @endif" @if($isOpen) style="display: block;" @endif role="menu"> <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"> <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' : '' ]) @include('partials.entity-list-item-basic', ['entity' => $childPage, 'classes' => $current->matches($childPage)? 'selected' : '' ])
</li> </li>

View File

@ -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> <span class="icon text-chapter">@icon('chapter')</span>
<div class="content"> <div class="content">
<h4 class="entity-list-item-name break-text">{{ $chapter->name }}</h4> <h4 class="entity-list-item-name break-text">{{ $chapter->name }}</h4>
@ -7,16 +9,16 @@
</div> </div>
</div> </div>
</a> </a>
@if ($chapter->hasChildren()) @if ($chapter->visible_pages->count() > 0)
<div class="chapter chapter-expansion"> <div class="chapter chapter-expansion">
<span class="icon text-chapter">@icon('page')</span> <span class="icon text-chapter">@icon('page')</span>
<div class="content"> <div class="content">
<button type="button" chapter-toggle <button type="button" chapter-toggle
aria-expanded="false" 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="inset-list">
<div class="entity-list-item-children"> <div class="entity-list-item-children">
@include('partials.entity-list', ['entities' => $chapter->pages]) @include('partials.entity-list', ['entities' => $chapter->visible_pages])
</div> </div>
</div> </div>
</div> </div>

View File

@ -15,7 +15,7 @@
<li class="list-item-{{ $bookChild->getClassName() }} {{ $bookChild->getClassName() }} {{ $bookChild->isA('page') && $bookChild->draft ? 'draft' : '' }}"> <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' : '']) @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"> <div class="entity-list-item no-hover">
<span role="presentation" class="icon text-chapter"></span> <span role="presentation" class="icon text-chapter"></span>
<div class="content"> <div class="content">

View File

@ -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

View File

@ -490,6 +490,22 @@ class RestrictionsTest extends BrowserKitTest
->dontSee($page->name); ->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() public function test_bookshelf_update_restriction_override()
{ {
$shelf = Bookshelf::first(); $shelf = Bookshelf::first();