List page settings: Review of #5606

ssddanbrown · ssddanbrown · commit 10f5ceee35c2 · 2025-12-06T23:10:54.000Z
Updated setting display to show mulitple number inputs under one heading
group.
Updated settings to use general number field form view template.
Updated translations to match display changes, and to advise on counts.
Added page count control for search results.
Added setting service method, to get settings as integers, with
min/max/default control.
Updating sorting group to be names "Lists &amp; Sorting".
Added tests to cover.
diff --git a/app/App/HomeController.php b/app/App/HomeController.php
@@ -83,7 +83,7 @@ public function index(
         if ($homepageOption === 'bookshelves') {
             $shelves = $this->queries->shelves->visibleForListWithCover()
                 ->orderBy($commonData['listOptions']->getSort(), $commonData['listOptions']->getOrder())
-                ->paginate(intval(setting('sorting-shelves-per-page', '18')));
+                ->paginate(setting()->getInteger('lists-page-count-shelves', 18, 1, 1000));
             $data = array_merge($commonData, ['shelves' => $shelves]);
 
             return view('home.shelves', $data);
@@ -92,7 +92,7 @@ public function index(
         if ($homepageOption === 'books') {
             $books = $this->queries->books->visibleForListWithCover()
                 ->orderBy($commonData['listOptions']->getSort(), $commonData['listOptions']->getOrder())
-                ->paginate(intval(setting('sorting-books-per-page', '18')));
+                ->paginate(setting()->getInteger('lists-page-count-books', 18, 1, 1000));
             $data = array_merge($commonData, ['books' => $books]);
 
             return view('home.books', $data);
diff --git a/app/Entities/Controllers/BookController.php b/app/Entities/Controllers/BookController.php
@@ -52,7 +52,7 @@ public function index(Request $request)
 
         $books = $this->queries->visibleForListWithCover()
             ->orderBy($listOptions->getSort(), $listOptions->getOrder())
-            ->paginate(intval(setting('sorting-books-per-page', '18')));
+            ->paginate(setting()->getInteger('lists-page-count-books', 18, 1, 1000));
         $recents = $this->isSignedIn() ? $this->queries->recentlyViewedForCurrentUser()->take(4)->get() : false;
         $popular = $this->queries->popularForList()->take(4)->get();
         $new = $this->queries->visibleForList()->orderBy('created_at', 'desc')->take(4)->get();
diff --git a/app/Entities/Controllers/BookshelfController.php b/app/Entities/Controllers/BookshelfController.php
@@ -45,7 +45,7 @@ public function index(Request $request)
 
         $shelves = $this->queries->visibleForListWithCover()
             ->orderBy($listOptions->getSort(), $listOptions->getOrder())
-            ->paginate(intval(setting('sorting-shelves-per-page', '18')));
+            ->paginate(setting()->getInteger('lists-page-count-shelves', 18, 1, 1000));
         $recents = $this->isSignedIn() ? $this->queries->recentlyViewedForCurrentUser()->get() : false;
         $popular = $this->queries->popularForList()->get();
         $new = $this->queries->visibleForList()
diff --git a/app/Search/SearchController.php b/app/Search/SearchController.php
@@ -25,10 +25,11 @@ public function search(Request $request, SearchResultsFormatter $formatter)
         $searchOpts = SearchOptions::fromRequest($request);
         $fullSearchString = $searchOpts->toString();
         $page = intval($request->get('page', '0')) ?: 1;
+        $count = setting()->getInteger('lists-page-count-search', 18, 1, 1000);
 
-        $results = $this->searchRunner->searchEntities($searchOpts, 'all', $page, 20);
+        $results = $this->searchRunner->searchEntities($searchOpts, 'all', $page, $count);
         $formatter->format($results['results']->all(), $searchOpts);
-        $paginator = new LengthAwarePaginator($results['results'], $results['total'], 20, $page);
+        $paginator = new LengthAwarePaginator($results['results'], $results['total'], $count, $page);
         $paginator->setPath('/search');
         $paginator->appends($request->except('page'));
 
diff --git a/app/Settings/AppSettingsStore.php b/app/Settings/AppSettingsStore.php
@@ -14,7 +14,7 @@ public function __construct(
     ) {
     }
 
-    public function storeFromUpdateRequest(Request $request, string $category)
+    public function storeFromUpdateRequest(Request $request, string $category): void
     {
         $this->storeSimpleSettings($request);
         if ($category === 'customization') {
@@ -76,7 +76,7 @@ protected function updateAppLogo(Request $request): void
     protected function storeSimpleSettings(Request $request): void
     {
         foreach ($request->all() as $name => $value) {
-            if (strpos($name, 'setting-') !== 0) {
+            if (!str_starts_with($name, 'setting-')) {
                 continue;
             }
 
@@ -85,7 +85,7 @@ protected function storeSimpleSettings(Request $request): void
         }
     }
 
-    protected function destroyExistingSettingImage(string $settingKey)
+    protected function destroyExistingSettingImage(string $settingKey): void
     {
         $existingVal = setting()->get($settingKey);
         if ($existingVal) {
diff --git a/app/Settings/SettingService.php b/app/Settings/SettingService.php
@@ -28,6 +28,21 @@ public function get(string $key, $default = null): mixed
         return $this->formatValue($value, $default);
     }
 
+    /**
+     * Get a setting from the database as an integer.
+     * Returns the default value if not found or not an integer, and clamps the value to the given min/max range.
+     */
+    public function getInteger(string $key, int $default, int $min = 0, int $max = PHP_INT_MAX): int
+    {
+        $value = $this->get($key, $default);
+        if (!is_numeric($value)) {
+            return $default;
+        }
+
+        $int = intval($value);
+        return max($min, min($max, $int));
+    }
+
     /**
      * Get a value from the session instead of the main store option.
      */
diff --git a/lang/en/settings.php b/lang/en/settings.php
@@ -75,12 +75,8 @@
     'reg_confirm_restrict_domain_placeholder' => 'No restriction set',
 
     // Sorting Settings
-    'sorting' => 'Sorting',
-    'sorting_shelves_per_page' => 'Shelves Per Page',
-    'sorting_shelves_per_page_desc' => 'Sets the number of shelves shown per page in shelf listings.',
-    'sorting_books_per_page' => 'Books Per Page',
-    'sorting_books_per_page_desc' => 'Sets the number of books shown per page in book listings.',
-    'sorting_book_default' => 'Default Book Sort',
+    'sorting' => 'Lists & Sorting',
+    'sorting_book_default' => 'Default Book Sort Rule',
     'sorting_book_default_desc' => 'Select the default sort rule to apply to new books. This won\'t affect existing books, and can be overridden per-book.',
     'sorting_rules' => 'Sort Rules',
     'sorting_rules_desc' => 'These are predefined sorting operations which can be applied to content in the system.',
@@ -107,6 +103,8 @@
     'sort_rule_op_updated_date' => 'Updated Date',
     'sort_rule_op_chapters_first' => 'Chapters First',
     'sort_rule_op_chapters_last' => 'Chapters Last',
+    'sorting_page_limits' => 'Per-Page Display Limits',
+    'sorting_page_limits_desc' => 'Set how many items to show per-page in various lists within the system. Typically a lower amount will be more performant, while a higher amount avoids the need to click through multiple pages. Using an even multiple of 3 (18, 24, 30, etc...) is recommended.',
 
     // Maintenance settings
     'maint' => 'Maintenance',
diff --git a/resources/sass/_forms.scss b/resources/sass/_forms.scss
@@ -348,6 +348,10 @@ input[type=color] {
   }
 }
 
+.small-inputs input {
+  width: 150px;
+}
+
 .simple-code-input {
   background-color: #F8F8F8;
   font-family: monospace;
diff --git a/resources/views/form/number.blade.php b/resources/views/form/number.blade.php
@@ -6,7 +6,8 @@
        @if($readonly ?? false) readonly="readonly" @endif
        @if($min ?? false) min="{{ $min }}" @endif
        @if($max ?? false) max="{{ $max }}" @endif
-       @if(isset($model) || old($name)) value="{{ old($name) ? old($name) : $model->$name}}" @endif>
+       @if($step ?? false) step="{{ $step }}" @endif
+       @if(isset($model) || old($name) || isset($value)) value="{{ old($name) ?? $model->$name ?? $value }}" @endif>
 @if($errors->has($name))
     <div class="text-neg text-small">{{ $errors->first($name) }}</div>
 @endif
diff --git a/resources/views/settings/categories/sorting.blade.php b/resources/views/settings/categories/sorting.blade.php
@@ -10,41 +10,30 @@
         {{ csrf_field() }}
         <input type="hidden" name="section" value="sorting">
 
-        <div class="grid half gap-xl items-center">
-            <div>
-                <label for="setting-sorting-shelves-per-page"
-                    class="setting-list-label">{{ trans('settings.sorting_shelves_per_page') }}</label>
-                <p class="small">{{ trans('settings.sorting_shelves_per_page_desc') }}</p>
-            </div>
-            <div>
-                <input type="number"
-                    id="setting-sorting-shelves-per-page"
-                    name="setting-sorting-shelves-per-page"
-                    value="{{ intval(setting('sorting-shelves-per-page', 18)) }}"
-                    min="1"
-                    step="1"
-                    class="@if($errors->has('setting-sorting-shelves-per-page')) neg @endif">
-            </div>
-        </div>
-
-        <div class="grid half gap-xl items-center">
-            <div>
-                <label for="setting-sorting-books-per-page"
-                    class="setting-list-label">{{ trans('settings.sorting_books_per_page') }}</label>
-                <p class="small">{{ trans('settings.sorting_books_per_page_desc') }}</p>
-            </div>
+        <div class="setting-list">
             <div>
-                <input type="number"
-                    id="setting-sorting-books-per-page"
-                    name="setting-sorting-books-per-page"
-                    value="{{ intval(setting('sorting-books-per-page', 18)) }}"
-                    min="1"
-                    step="1"
-                    class="@if($errors->has('setting-sorting-books-per-page')) neg @endif">
+                <div class="mb-m">
+                    <label class="setting-list-label">{{ trans('settings.sorting_page_limits') }}</label>
+                    <p class="small">{{ trans('settings.sorting_page_limits_desc') }}</p>
+                </div>
+                <div class="flex-container-row wrap gap-m small-inputs">
+                    @php
+                        $labelByKey = ['shelves' => trans('entities.shelves'), 'books' => trans('entities.books'), 'search' => trans('entities.search_results')];
+                    @endphp
+                    @foreach($labelByKey as $key => $label)
+                        <div>
+                            <label for="setting-lists-page-count-{{ $key }}">{{ $label }}</label>
+                            @include('form.number', [
+                                'name' => 'setting-lists-page-count-' . $key,
+                                'value' => setting()->getInteger('lists-page-count-' . $key, 18, 1, 1000),
+                                'min' => 1,
+                                'step' => 1,
+                            ])
+                        </div>
+                    @endforeach
+                </div>
             </div>
-        </div>
 
-        <div class="setting-list">
             <div class="grid half gap-xl items-center">
                 <div>
                     <label for="setting-sorting-book-default"
diff --git a/tests/Settings/PageListLimitsTest.php b/tests/Settings/PageListLimitsTest.php
@@ -0,0 +1,81 @@
+<?php
+
+namespace Settings;
+
+use BookStack\Entities\Models\Book;
+use BookStack\Entities\Models\Bookshelf;
+use Tests\TestCase;
+
+class PageListLimitsTest extends TestCase
+{
+    public function test_saving_setting_and_loading()
+    {
+        $resp = $this->asAdmin()->post('/settings/sorting', [
+            'setting-lists-page-count-shelves' => '3',
+            'setting-lists-page-count-books' => '6',
+            'setting-lists-page-count-search' => '9',
+        ]);
+        $resp->assertRedirect('/settings/sorting');
+
+        $this->assertEquals(3, setting()->getInteger('lists-page-count-shelves', 18));
+        $this->assertEquals(6, setting()->getInteger('lists-page-count-books', 18));
+        $this->assertEquals(9, setting()->getInteger('lists-page-count-search', 18));
+
+        $resp = $this->get('/settings/sorting');
+        $html = $this->withHtml($resp);
+
+        $html->assertFieldHasValue('setting-lists-page-count-shelves', '3');
+        $html->assertFieldHasValue('setting-lists-page-count-books', '6');
+        $html->assertFieldHasValue('setting-lists-page-count-search', '9');
+    }
+
+    public function test_invalid_counts_will_use_default_when_fetched_as_an_integer()
+    {
+        $this->asAdmin()->post('/settings/sorting', [
+            'setting-lists-page-count-shelves' => 'cat',
+        ]);
+
+        $this->assertEquals(18, setting()->getInteger('lists-page-count-shelves', 18));
+    }
+
+    public function test_shelf_count_is_used_on_shelves_view()
+    {
+        $resp = $this->asAdmin()->get('/shelves');
+        $defaultCount = min(Bookshelf::query()->count(), 18);
+        $this->withHtml($resp)->assertElementCount('main [data-entity-type="bookshelf"]', $defaultCount);
+
+        $this->post('/settings/sorting', [
+            'setting-lists-page-count-shelves' => '1',
+        ]);
+
+        $resp = $this->get('/shelves');
+        $this->withHtml($resp)->assertElementCount('main [data-entity-type="bookshelf"]', 1);
+    }
+
+    public function test_shelf_count_is_used_on_books_view()
+    {
+        $resp = $this->asAdmin()->get('/books');
+        $defaultCount = min(Book::query()->count(), 18);
+        $this->withHtml($resp)->assertElementCount('main [data-entity-type="book"]', $defaultCount);
+
+        $this->post('/settings/sorting', [
+            'setting-lists-page-count-books' => '1',
+        ]);
+
+        $resp = $this->get('/books');
+        $this->withHtml($resp)->assertElementCount('main [data-entity-type="book"]', 1);
+    }
+
+    public function test_shelf_count_is_used_on_search_view()
+    {
+        $resp = $this->asAdmin()->get('/search');
+        $this->withHtml($resp)->assertElementCount('.entity-list [data-entity-id]', 18);
+
+        $this->post('/settings/sorting', [
+            'setting-lists-page-count-search' => '1',
+        ]);
+
+        $resp = $this->get('/search');
+        $this->withHtml($resp)->assertElementCount('.entity-list [data-entity-id]', 1);
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,7 @@ public function __construct(`
`14`	`14`	`) {`
`15`	`15`	`}`
`16`	`16`
`17`		`- public function storeFromUpdateRequest(Request $request, string $category)`
	`17`	`+ public function storeFromUpdateRequest(Request $request, string $category): void`
`18`	`18`	`{`
`19`	`19`	`$this->storeSimpleSettings($request);`
`20`	`20`	`if ($category === 'customization') {`
`@@ -76,7 +76,7 @@ protected function updateAppLogo(Request $request): void`
`76`	`76`	`protected function storeSimpleSettings(Request $request): void`
`77`	`77`	`{`
`78`	`78`	`foreach ($request->all() as $name => $value) {`
`79`		`- if (strpos($name, 'setting-') !== 0) {`
	`79`	`+ if (!str_starts_with($name, 'setting-')) {`
`80`	`80`	`continue;`
`81`	`81`	`}`
`82`	`82`
`@@ -85,7 +85,7 @@ protected function storeSimpleSettings(Request $request): void`
`85`	`85`	`}`
`86`	`86`	`}`
`87`	`87`
`88`		`- protected function destroyExistingSettingImage(string $settingKey)`
	`88`	`+ protected function destroyExistingSettingImage(string $settingKey): void`
`89`	`89`	`{`
`90`	`90`	`$existingVal = setting()->get($settingKey);`
`91`	`91`	`if ($existingVal) {`
Original file line number	Diff line number	Diff line change
`@@ -348,6 +348,10 @@ input[type=color] {`
`348`	`348`	`}`
`349`	`349`	`}`
`350`	`350`
	`351`	`+.small-inputs input {`
	`352`	`+ width: 150px;`
	`353`	`+}`
	`354`	`+`
`351`	`355`	`.simple-code-input {`
`352`	`356`	`background-color: #F8F8F8;`
`353`	`357`	`font-family: monospace;`