Updated shelf sort to allow default sort, added testing

Done during review of #2515

diff --git a/app/Http/Controllers/BookshelfController.php b/app/Http/Controllers/BookshelfController.php
index 14bc9d94efc61e8704f8691b368b7d56799d1f50..03b3cad54cc0a538fd8de9794e3d647b2b4dca0b 100644 (file)
--- a/app/Http/Controllers/BookshelfController.php
+++ b/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 = $visibleShelfBooks
-            ->sortBy($sort, SORT_REGULAR, $order === 'desc')
+        $sortedVisibleShelfBooks = $shelf->visibleBooks()->get()
+            ->sortBy($sort === 'default' ? 'pivot.order' : $sort, SORT_REGULAR, $order === 'desc')
             ->values()
             ->all();
 

diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php
index 59c6d6edfc90485c30cd1d97717b4cb39af6c9d9..ba35904378a6f2ca04a76baf26f78cc94b56388a 100644 (file)
--- a/app/Http/Controllers/UserController.php
+++ b/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';
         }
 

diff --git a/resources/lang/en/common.php b/resources/lang/en/common.php
index e048db90f4c2ff13d17a5fe29d449dac302c95e2..3b003c64ad814ad97d36912c366850995c97488e 100644 (file)
--- a/resources/lang/en/common.php
+++ b/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',
 

diff --git a/resources/views/shelves/show.blade.php b/resources/views/shelves/show.blade.php
index a7a55a4270bddd0ca91f69a66845e2a6a5b88472..7ed36c90685ef1d8a978b7fdf8a4a00d50deec61 100644 (file)
--- a/resources/views/shelves/show.blade.php
+++ b/resources/views/shelves/show.blade.php
@@ -10,10 +10,12 @@
 
     <main class="card content-wrap">
 
-        <div class="grid half v-center">
-            <h1 class="break-text">{{$shelf->name}}</h1>
-            <div class="text-m-right my-m">
+        <div class="flex-container-row wrap v-center">
+            <h1 class="flex fit-content break-text">{{ $shelf->name }}</h1>
+            <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>

diff --git a/tests/Entity/BookShelfTest.php b/tests/Entity/BookShelfTest.php
index 29fac5ec56c21777465d2550299f7cf99eba57fe..60658f6b2b8a87ecf36609a32f79bda7b46434e6 100644 (file)
--- a/tests/Entity/BookShelfTest.php
+++ b/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();

diff --git a/tests/TestResponse.php b/tests/TestResponse.php
index 9c6b78782b4c91bb8576bfb535666559a588d772..bf7ee0f69add042497b6862569354b1f1629427f 100644 (file)
--- a/tests/TestResponse.php
+++ b/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.