Updated shelf sort to allow default sort, added testing

Done during review of #2515

app/Http/Controllers/BookshelfController.php

@@ -101,12 +101,11 @@ class BookshelfController extends Controller
         $shelf = $this->bookshelfRepo->getBySlug($slug);
         $this->checkOwnablePermission('book-view', $shelf);
 
-        $sort = setting()->getForCurrentUser('shelf_books_sort', 'name');
+        $sort = setting()->getForCurrentUser('shelf_books_sort', 'default');
         $order = setting()->getForCurrentUser('shelf_books_sort_order', 'asc');
 
-        $visibleShelfBooks = $shelf->visibleBooks()->get();
+        $sortedVisibleShelfBooks = $shelf->visibleBooks()->get()
-        $sortedVisibleShelfBooks = $visibleShelfBooks
+            ->sortBy($sort === 'default' ? 'pivot.order' : $sort, SORT_REGULAR, $order === 'desc')
-            ->sortBy($sort, SORT_REGULAR, $order === 'desc')
             ->values()
             ->all();
 

app/Http/Controllers/UserController.php

@@ -343,7 +343,7 @@ class UserController extends Controller
         $this->checkPermissionOrCurrentUser('users-manage', $userId);
 
         $sort = $request->get('sort');
-        if (!in_array($sort, ['name', 'created_at', 'updated_at'])) {
+        if (!in_array($sort, ['name', 'created_at', 'updated_at', 'default'])) {
             $sort = 'name';
         }
 

resources/lang/en/common.php

@@ -47,6 +47,7 @@ return [
     'sort_ascending' => 'Sort Ascending',
     'sort_descending' => 'Sort Descending',
     'sort_name' => 'Name',
+    'sort_default' => 'Default',
     'sort_created_at' => 'Created Date',
     'sort_updated_at' => 'Updated Date',
 

resources/views/shelves/show.blade.php

@@ -10,10 +10,12 @@
 
     <main class="card content-wrap">
 
-        <div class="grid half v-center">
+        <div class="flex-container-row wrap v-center">
-            <h1 class="break-text">{{$shelf->name}}</h1>
+            <h1 class="flex fit-content break-text">{{ $shelf->name }}</h1>
-            <div class="text-m-right my-m">
+            <div class="flex"></div>
+            <div class="flex fit-content text-m-right my-m ml-m">
                 @include('partials.sort', ['options' => [
+                    'default' => trans('common.sort_default'),
                     'name' => trans('common.sort_name'),
                     'created_at' => trans('common.sort_created_at'),
                     'updated_at' => trans('common.sort_updated_at'),
@@ -32,7 +34,7 @@
                     </div>
                 @else
                     <div class="grid third">
-                        @foreach($sortedVisibleShelfBooks as $key => $book)
+                        @foreach($sortedVisibleShelfBooks as $book)
                             @include('partials.entity-grid-item', ['entity' => $book])
                         @endforeach
                     </div>

tests/Entity/BookShelfTest.php

@@ -156,6 +156,47 @@ class BookShelfTest extends TestCase
         $resp->assertDontSee($shelf->getUrl('/permissions'));
     }
 
+    public function test_shelf_view_has_sort_control_that_defaults_to_default()
+    {
+        $shelf = Bookshelf::query()->first();
+        $resp = $this->asAdmin()->get($shelf->getUrl());
+        $resp->assertElementExists('form[action$="change-sort/shelf_books"]');
+        $resp->assertElementContains('form[action$="change-sort/shelf_books"] [aria-haspopup="true"]', 'Default');
+    }
+
+    public function test_shelf_view_sort_takes_action()
+    {
+        $shelf = Bookshelf::query()->whereHas('books')->with('books')->first();
+        $books = Book::query()->take(3)->get(['id', 'name']);
+        $books[0]->fill(['name' => 'bsfsdfsdfsd'])->save();
+        $books[1]->fill(['name' => 'adsfsdfsdfsd'])->save();
+        $books[2]->fill(['name' => 'hdgfgdfg'])->save();
+
+        // Set book ordering
+        $this->asAdmin()->put($shelf->getUrl(), [
+            'books' => $books->implode('id', ','),
+            'tags' => [], 'description' => 'abc', 'name' => 'abc'
+        ]);
+        $this->assertEquals(3, $shelf->books()->count());
+        $shelf->refresh();
+
+        $resp = $this->asEditor()->get($shelf->getUrl());
+        $resp->assertElementContains('.book-content a.grid-card', $books[0]->name, 1);
+        $resp->assertElementNotContains('.book-content a.grid-card', $books[0]->name, 3);
+
+        setting()->putUser($this->getEditor(), 'shelf_books_sort_order', 'desc');
+        $resp = $this->asEditor()->get($shelf->getUrl());
+        $resp->assertElementNotContains('.book-content a.grid-card', $books[0]->name, 1);
+        $resp->assertElementContains('.book-content a.grid-card', $books[0]->name, 3);
+
+        setting()->putUser($this->getEditor(), 'shelf_books_sort_order', 'desc');
+        setting()->putUser($this->getEditor(), 'shelf_books_sort', 'name');
+        $resp = $this->asEditor()->get($shelf->getUrl());
+        $resp->assertElementContains('.book-content a.grid-card', 'hdgfgdfg', 1);
+        $resp->assertElementContains('.book-content a.grid-card', 'bsfsdfsdfsd', 2);
+        $resp->assertElementContains('.book-content a.grid-card', 'adsfsdfsdfsd', 3);
+    }
+
     public function test_shelf_edit()
     {
         $shelf = Bookshelf::first();

tests/TestResponse.php

@@ -60,13 +60,20 @@ class TestResponse extends BaseTestResponse {
 
     /**
      * Assert the response includes a specific element containing the given text.
+     * If an nth match is provided, only that will be checked otherwise all matching
+     * elements will be checked for the given text.
      * @return $this
      */
-    public function assertElementContains(string $selector, string $text)
+    public function assertElementContains(string $selector, string $text, ?int $nthMatch = null)
     {
         $elements = $this->crawler()->filter($selector);
         $matched = false;
         $pattern = $this->getEscapedPattern($text);
+
+        if (!is_null($nthMatch)) {
+            $elements = $elements->eq($nthMatch - 1);
+        }
+
         foreach ($elements as $element) {
             $element = new Crawler($element);
             if (preg_match("/$pattern/i", $element->html())) {
@@ -78,6 +85,7 @@ class TestResponse extends BaseTestResponse {
         PHPUnit::assertTrue(
             $matched,
             'Unable to find element of selector: '.PHP_EOL.PHP_EOL.
+            ($nthMatch ? ("at position {$nthMatch}".PHP_EOL.PHP_EOL) : '') .
             "[{$selector}]".PHP_EOL.PHP_EOL.
             'containing text'.PHP_EOL.PHP_EOL.
             "[{$text}]".PHP_EOL.PHP_EOL.
@@ -90,13 +98,20 @@ class TestResponse extends BaseTestResponse {
 
     /**
      * Assert the response does not include a specific element containing the given text.
+     * If an nth match is provided, only that will be checked otherwise all matching
+     * elements will be checked for the given text.
      * @return $this
      */
-    public function assertElementNotContains(string $selector, string $text)
+    public function assertElementNotContains(string $selector, string $text, ?int $nthMatch = null)
     {
         $elements = $this->crawler()->filter($selector);
         $matched = false;
         $pattern = $this->getEscapedPattern($text);
+
+        if (!is_null($nthMatch)) {
+            $elements = $elements->eq($nthMatch - 1);
+        }
+
         foreach ($elements as $element) {
             $element = new Crawler($element);
             if (preg_match("/$pattern/i", $element->html())) {
@@ -108,6 +123,7 @@ class TestResponse extends BaseTestResponse {
         PHPUnit::assertTrue(
             !$matched,
             'Found element of selector: '.PHP_EOL.PHP_EOL.
+            ($nthMatch ? ("at position {$nthMatch}".PHP_EOL.PHP_EOL) : '') .
             "[{$selector}]".PHP_EOL.PHP_EOL.
             'containing text'.PHP_EOL.PHP_EOL.
             "[{$text}]".PHP_EOL.PHP_EOL.