Reviewed and added testing for BookShelf API implementation

- Tweaked how books are passed on update to prevent unassignment if
parameter is not provided.
- Added books to validation so they show in docs.
- Added request/response examples.
- Added tests to cover.
- Added child book info to shelf info.

Review of #1908

app/Auth/User.php

@@ -47,7 +47,10 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon
      * The attributes excluded from the model's JSON form.
      * @var array
      */
-    protected $hidden = ['password', 'remember_token', 'system_name', 'email_confirmed', 'external_auth_id', 'email'];
+    protected $hidden = [
+        'password', 'remember_token', 'system_name', 'email_confirmed', 'external_auth_id', 'email',
+        'created_at', 'updated_at',
+    ];
 
     /**
      * This holds the user's permissions when loaded.

app/Entities/Book.php

@@ -19,7 +19,7 @@ class Book extends Entity implements HasCoverImage
     public $searchFactor = 2;
 
     protected $fillable = ['name', 'description'];
-    protected $hidden = ['restricted'];
+    protected $hidden = ['restricted', 'pivot'];
 
     /**
      * Get the url for this book.

app/Entities/Bookshelf.php

@@ -12,6 +12,8 @@ class Bookshelf extends Entity implements HasCoverImage
 
     protected $fillable = ['name', 'description', 'image_id'];
 
+    protected $hidden = ['restricted'];
+
     /**
      * Get the books in this shelf.
      * Should not be used directly since does not take into account permissions.

app/Entities/Repos/BookshelfRepo.php

@@ -91,10 +91,14 @@ class BookshelfRepo
     /**
      * Create a new shelf in the system.
      */
-    public function update(Bookshelf $shelf, array $input, array $bookIds): Bookshelf
+    public function update(Bookshelf $shelf, array $input, ?array $bookIds): Bookshelf
     {
         $this->baseRepo->update($shelf, $input);
-        $this->updateBooks($shelf, $bookIds);
+
+        if (!is_null($bookIds)) {
+            $this->updateBooks($shelf, $bookIds);
+        }
+
         return $shelf;
     }
 

app/Http/Controllers/Api/BookshelfApiController.php

@@ -4,10 +4,10 @@ use BookStack\Facades\Activity;
 use BookStack\Entities\Repos\BookshelfRepo;
 use BookStack\Entities\Bookshelf;
 use Exception;
+use Illuminate\Database\Eloquent\Relations\BelongsToMany;
 use Illuminate\Http\Request;
 use Illuminate\Validation\ValidationException;
 
-
 class BookshelfApiController extends ApiController
 {
 
@@ -20,10 +20,12 @@ class BookshelfApiController extends ApiController
         'create' => [
             'name' => 'required|string|max:255',
             'description' => 'string|max:1000',
+            'books' => 'array',
         ],
         'update' => [
             'name' => 'string|min:1|max:255',
             'description' => 'string|max:1000',
+            'books' => 'array',
         ],
     ];
 
@@ -49,6 +51,8 @@ class BookshelfApiController extends ApiController
 
     /**
      * Create a new shelf in the system.
+     * An array of books IDs can be provided in the request. These
+     * will be added to the shelf in the same order as provided.
      * @throws ValidationException
      */
     public function create(Request $request)
@@ -57,10 +61,9 @@ class BookshelfApiController extends ApiController
         $requestData = $this->validate($request, $this->rules['create']);
 
         $bookIds = $request->get('books', []);
+        $shelf = $this->bookshelfRepo->create($requestData, $bookIds);
 
-        $shelf = $this->bookshelfRepo->create($requestData,$bookIds);
         Activity::add($shelf, 'bookshelf_create', $shelf->id);
-
         return response()->json($shelf);
     }
 
@@ -69,12 +72,20 @@ class BookshelfApiController extends ApiController
      */
     public function read(string $id)
     {
-        $shelf = Bookshelf::visible()->with(['tags', 'cover', 'createdBy', 'updatedBy'])->findOrFail($id);
+        $shelf = Bookshelf::visible()->with([
+            'tags', 'cover', 'createdBy', 'updatedBy',
+            'books' => function (BelongsToMany $query) {
+                $query->visible()->get(['id', 'name', 'slug']);
+            }
+        ])->findOrFail($id);
         return response()->json($shelf);
     }
 
     /**
      * Update the details of a single shelf.
+     * An array of books IDs can be provided in the request. These
+     * will be added to the shelf in the same order as provided and overwrite
+     * any existing book assignments.
      * @throws ValidationException
      */
     public function update(Request $request, string $id)
@@ -84,9 +95,9 @@ class BookshelfApiController extends ApiController
 
         $requestData = $this->validate($request, $this->rules['update']);
 
-        $bookIds = $request->get('books', []);
+        $bookIds = $request->get('books', null);
 
-        $shelf = $this->bookshelfRepo->update($shelf, $requestData,$bookIds);
+        $shelf = $this->bookshelfRepo->update($shelf, $requestData, $bookIds);
         Activity::add($shelf, 'bookshelf_update', $shelf->id);
 
         return response()->json($shelf);
@@ -96,8 +107,6 @@ class BookshelfApiController extends ApiController
 
     /**
      * Delete a single shelf from the system.
-     * @param string $id
-     * @return \Illuminate\Contracts\Routing\ResponseFactory|\Illuminate\Http\Response
      * @throws Exception
      */
     public function delete(string $id)
@@ -106,7 +115,7 @@ class BookshelfApiController extends ApiController
         $this->checkOwnablePermission('bookshelf-delete', $shelf);
 
         $this->bookshelfRepo->destroy($shelf);
-        Activity::addMessage('bookshelf-delete', $shelf->name);
+        Activity::addMessage('bookshelf_delete', $shelf->name);
 
         return response('', 204);
     }

dev/api/requests/shelves-create.json

@@ -0,0 +1,5 @@
+{
+  "name": "My shelf",
+  "description": "This is my shelf with some books",
+  "books": [5,1,3]
+}

dev/api/requests/shelves-update.json

@@ -0,0 +1,5 @@
+{
+  "name": "My updated shelf",
+  "description": "This is my update shelf with some books",
+  "books": [5,1,3]
+}

dev/api/responses/books-read.json

@@ -8,15 +8,11 @@
   "created_by": {
     "id": 1,
     "name": "Admin",
-    "created_at": "2019-05-05 21:15:13",
-    "updated_at": "2019-12-16 12:18:37",
     "image_id": 48
   },
   "updated_by": {
     "id": 1,
     "name": "Admin",
-    "created_at": "2019-05-05 21:15:13",
-    "updated_at": "2019-12-16 12:18:37",
     "image_id": 48
   },
   "image_id": 452,

dev/api/responses/shelves-create.json

@@ -0,0 +1,10 @@
+{
+  "name": "My shelf",
+  "description": "This is my shelf with some books",
+  "created_by": 1,
+  "updated_by": 1,
+  "slug": "my-shelf",
+  "updated_at": "2020-04-10 13:24:09",
+  "created_at": "2020-04-10 13:24:09",
+  "id": 14
+}

dev/api/responses/shelves-list.json

@@ -0,0 +1,38 @@
+{
+  "data": [
+    {
+      "id": 8,
+      "name": "Qui qui aspernatur autem molestiae libero necessitatibus molestias.",
+      "slug": "qui-qui-aspernatur-autem-molestiae-libero-necessitatibus-molestias",
+      "description": "Enim dolor ut quia error dolores est. Aut distinctio consequuntur non nisi nostrum. Labore cupiditate error labore aliquid provident impedit voluptatibus. Quaerat impedit excepturi eius qui eius voluptatem reiciendis.",
+      "created_at": "2019-05-05 22:10:16",
+      "updated_at": "2020-04-10 13:00:45",
+      "created_by": 4,
+      "updated_by": 1,
+      "image_id": 31
+    },
+    {
+      "id": 9,
+      "name": "Ipsum aut inventore fuga libero non facilis.",
+      "slug": "ipsum-aut-inventore-fuga-libero-non-facilis",
+      "description": "Labore culpa modi perspiciatis harum sit. Maxime non et nam est. Quae ut laboriosam repellendus sunt quisquam. Velit at est perspiciatis nesciunt adipisci nobis illo. Sed possimus odit optio officiis nisi voluptates officiis dolor.",
+      "created_at": "2019-05-05 22:10:16",
+      "updated_at": "2020-04-10 13:00:58",
+      "created_by": 4,
+      "updated_by": 1,
+      "image_id": 28
+    },
+    {
+      "id": 10,
+      "name": "Omnis reiciendis aut molestias sint accusantium.",
+      "slug": "omnis-reiciendis-aut-molestias-sint-accusantium",
+      "description": "Qui ea occaecati alias est dolores voluptatem doloribus. Ad reiciendis corporis vero nostrum omnis et. Non doloribus ut eaque ut quos dolores.",
+      "created_at": "2019-05-05 22:10:16",
+      "updated_at": "2020-04-10 13:00:53",
+      "created_by": 4,
+      "updated_by": 1,
+      "image_id": 30
+    }
+  ],
+  "total": 3
+}

dev/api/responses/shelves-read.json

@@ -0,0 +1,60 @@
+{
+  "id": 14,
+  "name": "My shelf",
+  "slug": "my-shelf",
+  "description": "This is my shelf with some books",
+  "created_by": {
+    "id": 1,
+    "name": "Admin",
+    "image_id": 48
+  },
+  "updated_by": {
+    "id": 1,
+    "name": "Admin",
+    "image_id": 48
+  },
+  "image_id": 501,
+  "created_at": "2020-04-10 13:24:09",
+  "updated_at": "2020-04-10 13:31:04",
+  "tags": [
+    {
+      "id": 16,
+      "entity_id": 14,
+      "entity_type": "BookStack\\Bookshelf",
+      "name": "Category",
+      "value": "Guide",
+      "order": 0,
+      "created_at": "2020-04-10 13:31:04",
+      "updated_at": "2020-04-10 13:31:04"
+    }
+  ],
+  "cover": {
+    "id": 501,
+    "name": "anafrancisconi_Sp04AfFCPNM.jpg",
+    "url": "http://bookstack.local/uploads/images/cover_book/2020-04/anafrancisconi_Sp04AfFCPNM.jpg",
+    "created_at": "2020-04-10 13:31:04",
+    "updated_at": "2020-04-10 13:31:04",
+    "created_by": 1,
+    "updated_by": 1,
+    "path": "/uploads/images/cover_book/2020-04/anafrancisconi_Sp04AfFCPNM.jpg",
+    "type": "cover_book",
+    "uploaded_to": 14
+  },
+  "books": [
+    {
+      "id": 5,
+      "name": "Sint explicabo alias sunt.",
+      "slug": "jbsQrzuaXe"
+    },
+    {
+      "id": 1,
+      "name": "BookStack User Guide",
+      "slug": "bookstack-user-guide"
+    },
+    {
+      "id": 3,
+      "name": "Molestiae doloribus sint velit suscipit dolorem.",
+      "slug": "H99QxALaoG"
+    }
+  ]
+}

dev/api/responses/shelves-update.json

@@ -0,0 +1,11 @@
+{
+  "id": 14,
+  "name": "My updated shelf",
+  "slug": "my-updated-shelf",
+  "description": "This is my update shelf with some books",
+  "created_by": 1,
+  "updated_by": 1,
+  "image_id": 501,
+  "created_at": "2020-04-10 13:24:09",
+  "updated_at": "2020-04-10 13:48:22"
+}

tests/Api/ShelvesApiTest.php

@@ -0,0 +1,136 @@
+<?php namespace Tests\Api;
+
+use BookStack\Entities\Book;
+use BookStack\Entities\Bookshelf;
+use Tests\TestCase;
+
+class ShelvesApiTest extends TestCase
+{
+    use TestsApi;
+
+    protected $baseEndpoint = '/api/shelves';
+
+    public function test_index_endpoint_returns_expected_shelf()
+    {
+        $this->actingAsApiEditor();
+        $firstBookshelf = Bookshelf::query()->orderBy('id', 'asc')->first();
+
+        $resp = $this->getJson($this->baseEndpoint . '?count=1&sort=+id');
+        $resp->assertJson(['data' => [
+            [
+                'id' => $firstBookshelf->id,
+                'name' => $firstBookshelf->name,
+                'slug' => $firstBookshelf->slug,
+            ]
+        ]]);
+    }
+
+    public function test_create_endpoint()
+    {
+        $this->actingAsApiEditor();
+        $books = Book::query()->take(2)->get();
+
+        $details = [
+            'name' => 'My API shelf',
+            'description' => 'A shelf created via the API',
+        ];
+
+        $resp = $this->postJson($this->baseEndpoint, array_merge($details, ['books' => [$books[0]->id, $books[1]->id]]));
+        $resp->assertStatus(200);
+        $newItem = Bookshelf::query()->orderByDesc('id')->where('name', '=', $details['name'])->first();
+        $resp->assertJson(array_merge($details, ['id' => $newItem->id, 'slug' => $newItem->slug]));
+        $this->assertActivityExists('bookshelf_create', $newItem);
+        foreach ($books as $index => $book) {
+            $this->assertDatabaseHas('bookshelves_books', [
+                'bookshelf_id' => $newItem->id,
+                'book_id' => $book->id,
+                'order' => $index,
+            ]);
+        }
+    }
+
+    public function test_shelf_name_needed_to_create()
+    {
+        $this->actingAsApiEditor();
+        $details = [
+            'description' => 'A shelf created via the API',
+        ];
+
+        $resp = $this->postJson($this->baseEndpoint, $details);
+        $resp->assertStatus(422);
+        $resp->assertJson([
+            "error" => [
+                "message" => "The given data was invalid.",
+                "validation" => [
+                    "name" => ["The name field is required."]
+                ],
+                "code" => 422,
+            ],
+        ]);
+    }
+
+    public function test_read_endpoint()
+    {
+        $this->actingAsApiEditor();
+        $shelf = Bookshelf::visible()->first();
+
+        $resp = $this->getJson($this->baseEndpoint . "/{$shelf->id}");
+
+        $resp->assertStatus(200);
+        $resp->assertJson([
+            'id' => $shelf->id,
+            'slug' => $shelf->slug,
+            'created_by' => [
+                'name' => $shelf->createdBy->name,
+            ],
+            'updated_by' => [
+                'name' => $shelf->createdBy->name,
+            ]
+        ]);
+    }
+
+    public function test_update_endpoint()
+    {
+        $this->actingAsApiEditor();
+        $shelf = Bookshelf::visible()->first();
+        $details = [
+            'name' => 'My updated API shelf',
+            'description' => 'A shelf created via the API',
+        ];
+
+        $resp = $this->putJson($this->baseEndpoint . "/{$shelf->id}", $details);
+        $shelf->refresh();
+
+        $resp->assertStatus(200);
+        $resp->assertJson(array_merge($details, ['id' => $shelf->id, 'slug' => $shelf->slug]));
+        $this->assertActivityExists('bookshelf_update', $shelf);
+    }
+
+    public function test_update_only_assigns_books_if_param_provided()
+    {
+        $this->actingAsApiEditor();
+        $shelf = Bookshelf::visible()->first();
+        $this->assertTrue($shelf->books()->count() > 0);
+        $details = [
+            'name' => 'My updated API shelf',
+        ];
+
+        $resp = $this->putJson($this->baseEndpoint . "/{$shelf->id}", $details);
+        $resp->assertStatus(200);
+        $this->assertTrue($shelf->books()->count() > 0);
+
+        $resp = $this->putJson($this->baseEndpoint . "/{$shelf->id}", ['books' => []]);
+        $resp->assertStatus(200);
+        $this->assertTrue($shelf->books()->count() === 0);
+    }
+
+    public function test_delete_endpoint()
+    {
+        $this->actingAsApiEditor();
+        $shelf = Bookshelf::visible()->first();
+        $resp = $this->deleteJson($this->baseEndpoint . "/{$shelf->id}");
+
+        $resp->assertStatus(204);
+        $this->assertActivityExists('bookshelf_delete');
+    }
+}