Skip to content

Commit 13a79b3

Browse files
committed
Shelves: Addressed book edits removing non-visible books
Tracks the non-visible existing books on change, to retain as part of the assigned books sync. Added test to cover. For #5728
1 parent 7c79b10 commit 13a79b3

File tree

2 files changed

+52
-6
lines changed

2 files changed

+52
-6
lines changed

app/Entities/Repos/BookshelfRepo.php

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,20 +56,37 @@ public function update(Bookshelf $shelf, array $input, ?array $bookIds): Bookshe
5656

5757
/**
5858
* Update which books are assigned to this shelf by syncing the given book ids.
59-
* Function ensures the books are visible to the current user and existing.
59+
* Function ensures the managed books are visible to the current user and existing,
60+
* and that the user does not alter the assignment of books that are not visible to them.
6061
*/
61-
protected function updateBooks(Bookshelf $shelf, array $bookIds)
62+
protected function updateBooks(Bookshelf $shelf, array $bookIds): void
6263
{
6364
$numericIDs = collect($bookIds)->map(function ($id) {
6465
return intval($id);
6566
});
6667

67-
$syncData = $this->bookQueries->visibleForList()
68+
$existingBookIds = $shelf->books()->pluck('id')->toArray();
69+
$visibleExistingBookIds = $this->bookQueries->visibleForList()
70+
->whereIn('id', $existingBookIds)
71+
->pluck('id')
72+
->toArray();
73+
$nonVisibleExistingBookIds = array_values(array_diff($existingBookIds, $visibleExistingBookIds));
74+
75+
$newIdsToAssign = $this->bookQueries->visibleForList()
6876
->whereIn('id', $bookIds)
6977
->pluck('id')
70-
->mapWithKeys(function ($bookId) use ($numericIDs) {
71-
return [$bookId => ['order' => $numericIDs->search($bookId)]];
72-
});
78+
->toArray();
79+
80+
$maxNewIndex = max($numericIDs->keys()->toArray() ?: [0]);
81+
82+
$syncData = [];
83+
foreach ($newIdsToAssign as $id) {
84+
$syncData[$id] = ['order' => $numericIDs->search($id)];
85+
}
86+
87+
foreach ($nonVisibleExistingBookIds as $index => $id) {
88+
$syncData[$id] = ['order' => $maxNewIndex + ($index + 1)];
89+
}
7390

7491
$shelf->books()->sync($syncData);
7592
}

tests/Entity/BookShelfTest.php

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,35 @@ public function test_shelf_edit()
259259
$this->assertDatabaseHas('bookshelves_books', ['bookshelf_id' => $shelf->id, 'book_id' => $booksToInclude[1]->id]);
260260
}
261261

262+
public function test_shelf_edit_does_not_alter_books_we_dont_have_access_to()
263+
{
264+
$shelf = $this->entities->shelf();
265+
$shelf->books()->detach();
266+
$this->entities->book();
267+
$this->entities->book();
268+
269+
$newBooks = [$this->entities->book(), $this->entities->book()];
270+
$originalBooks = [$this->entities->book(), $this->entities->book()];
271+
foreach ($originalBooks as $book) {
272+
$this->permissions->disableEntityInheritedPermissions($book);
273+
$shelf->books()->attach($book->id);
274+
}
275+
276+
$this->asEditor()->put($shelf->getUrl(), [
277+
'name' => $shelf->name,
278+
'books' => "{$newBooks[0]->id},{$newBooks[1]->id}",
279+
])->assertRedirect($shelf->getUrl());
280+
281+
$resultingBooksById = $shelf->books()->get()->keyBy('id')->toArray();
282+
$this->assertCount(4, $resultingBooksById);
283+
foreach ($newBooks as $book) {
284+
$this->assertArrayHasKey($book->id, $resultingBooksById);
285+
}
286+
foreach ($originalBooks as $book) {
287+
$this->assertArrayHasKey($book->id, $resultingBooksById);
288+
}
289+
}
290+
262291
public function test_shelf_create_new_book()
263292
{
264293
$shelf = $this->entities->shelf();

0 commit comments

Comments
 (0)