diff --git a/extensions/sticky/extend.php b/extensions/sticky/extend.php index fac39e5018..25f1d23a8f 100644 --- a/extensions/sticky/extend.php +++ b/extensions/sticky/extend.php @@ -32,8 +32,11 @@ new Extend\Locales(__DIR__.'/locale'), (new Extend\Settings()) + ->default('flarum-sticky.enable_display_excerpt', true) + ->default('flarum-sticky.only_sticky_unread_discussions', true) + ->default('flarum-sticky.pin_sticky_on_all_discussions', true) ->serializeToForum('excerptDisplayEnabled', 'flarum-sticky.enable_display_excerpt', 'boolval') - ->default('flarum-sticky.enable_display_excerpt', true), + ->serializeToForum('onlyStickyUnreadDiscussions', 'flarum-sticky.only_sticky_unread_discussions', 'boolval'), (new Extend\Model(Discussion::class)) ->cast('is_sticky', 'bool'), @@ -54,8 +57,4 @@ (new Extend\SearchDriver(DatabaseSearchDriver::class)) ->addFilter(DiscussionSearcher::class, StickyFilter::class) ->addMutator(DiscussionSearcher::class, PinStickiedDiscussionsToTop::class), - - (new Extend\Settings()) - ->default('flarum-sticky.only_sticky_unread_discussions', true) - ->serializeToForum('onlyStickyUnreadDiscussions', 'flarum-sticky.only_sticky_unread_discussions', 'boolval'), ]; diff --git a/extensions/sticky/js/src/admin/extend.tsx b/extensions/sticky/js/src/admin/extend.tsx index 3ca276305e..971a959ff1 100644 --- a/extensions/sticky/js/src/admin/extend.tsx +++ b/extensions/sticky/js/src/admin/extend.tsx @@ -15,6 +15,16 @@ export default [ 'moderate', 95 ) + .setting( + () => ({ + setting: 'flarum-sticky.pin_sticky_on_all_discussions', + name: 'pinStickyOnAllDiscussions', + type: 'boolean', + label: app.translator.trans('flarum-sticky.admin.settings.pin_sticky_on_all_discussions_label'), + help: app.translator.trans('flarum-sticky.admin.settings.pin_sticky_on_all_discussions_help'), + }), + 95 + ) .setting( () => ({ setting: 'flarum-sticky.only_sticky_unread_discussions', @@ -22,6 +32,8 @@ export default [ type: 'boolean', label: app.translator.trans('flarum-sticky.admin.settings.only_sticky_unread_discussions_label'), help: app.translator.trans('flarum-sticky.admin.settings.only_sticky_unread_discussions_help'), + // Only meaningful when sticky pinning is enabled on the All Discussions page. + disabled: app.data.settings['flarum-sticky.pin_sticky_on_all_discussions'] === '0', }), 90 ) diff --git a/extensions/sticky/locale/en.yml b/extensions/sticky/locale/en.yml index 03868879a3..0765312b16 100644 --- a/extensions/sticky/locale/en.yml +++ b/extensions/sticky/locale/en.yml @@ -10,6 +10,8 @@ flarum-sticky: enable_display_excerpt: Show an excerpt of the first post when a sticky discussion is unread only_sticky_unread_discussions_label: Only sticky unread discussions only_sticky_unread_discussions_help: On the All Discussions page, unread sticky discussions pin to the top, while read sticky discussions follow the regular order. + pin_sticky_on_all_discussions_label: Pin stickied discussions on the All Discussions page + pin_sticky_on_all_discussions_help: When enabled (default), stickied discussions are pinned at the top of the All Discussions page. When disabled, they appear at their natural position by latest activity. Tag pages always pin stickied discussions to the top regardless of this setting. # These translations are used in the Permissions page of the admin interface. permissions: diff --git a/extensions/sticky/src/PinStickiedDiscussionsToTop.php b/extensions/sticky/src/PinStickiedDiscussionsToTop.php index 2c3dd55e96..2829cb374e 100755 --- a/extensions/sticky/src/PinStickiedDiscussionsToTop.php +++ b/extensions/sticky/src/PinStickiedDiscussionsToTop.php @@ -27,18 +27,8 @@ public function __invoke(DatabaseSearchState $state, SearchCriteria $criteria): { if ($criteria->sortIsDefault && ! $state->isFulltextSearch()) { $query = $state->getQuery()->getQuery(); - $onlyStickyUnread = $this->settings->get('flarum-sticky.only_sticky_unread_discussions'); - // If only sticky unread discussions is disabled, then pin all stickied - // discussions to the top whether they are read or not. - if (! $onlyStickyUnread) { - $this->pinStickiedToTop($query); - - return; - } - - // If we are viewing a specific tag, then pin all stickied - // discussions to the top no matter what. + // Tag pages always pin stickied discussions to the top. $filters = $state->getActiveFilters(); if ($count = count($filters)) { @@ -49,10 +39,27 @@ public function __invoke(DatabaseSearchState $state, SearchCriteria $criteria): return; } - // Otherwise, if we are viewing "all discussions", only pin stickied - // discussions to the top if they are unread. To do this in a - // performant way we create another query which will select all - // stickied discussions, marry them into the main query, and then + // The remainder of this method handles the All Discussions page only. + + // Admins can disable sticky pinning on this page entirely. When disabled, + // stickied discussions appear at their natural last_posted_at position + // and the only_sticky_unread_discussions setting becomes a no-op (the + // distinction between read and unread sticky no longer matters). + if (! $this->settings->get('flarum-sticky.pin_sticky_on_all_discussions', true)) { + return; + } + + // If unread-only floating is disabled, pin all stickied discussions to + // the top regardless of read state. + if (! $this->settings->get('flarum-sticky.only_sticky_unread_discussions')) { + $this->pinStickiedToTop($query); + + return; + } + + // Otherwise, only pin stickied discussions to the top if they are unread. + // To do this in a performant way we create another query which will select + // all stickied discussions, marry them into the main query, and then // reorder the unread ones up to the top. $sticky = clone $query; $sticky->where('is_sticky', true); diff --git a/extensions/sticky/tests/integration/api/ListDiscussionsTest.php b/extensions/sticky/tests/integration/api/ListDiscussionsTest.php index 1352390f5c..5822b40ca3 100644 --- a/extensions/sticky/tests/integration/api/ListDiscussionsTest.php +++ b/extensions/sticky/tests/integration/api/ListDiscussionsTest.php @@ -193,4 +193,63 @@ public function list_discussions_shows_stick_first_on_a_tag() $this->assertEquals([3, 1, 2, 4], Arr::pluck($data['data'], 'id')); } + + #[Test] + public function list_discussions_does_not_pin_sticky_on_all_when_pin_setting_disabled_as_guest() + { + $this->setting('flarum-sticky.pin_sticky_on_all_discussions', false); + + $response = $this->send( + $this->request('GET', '/api/discussions') + ); + + $this->assertEquals(200, $response->getStatusCode(), $body = $response->getBody()->getContents()); + + $data = json_decode($body, true); + + $this->assertEquals([2, 4, 3, 1], Arr::pluck($data['data'], 'id')); + } + + #[Test] + public function list_discussions_pin_setting_disabled_overrides_only_unread_setting_on_all() + { + // pin_sticky_on_all_discussions is the master gate for /all and must + // override only_sticky_unread_discussions when both are flipped. + $this->setting('flarum-sticky.pin_sticky_on_all_discussions', false); + $this->setting('flarum-sticky.only_sticky_unread_discussions', false); + + $response = $this->send( + $this->request('GET', '/api/discussions', [ + 'authenticatedAs' => 2 + ]) + ); + + $this->assertEquals(200, $response->getStatusCode(), $body = $response->getBody()->getContents()); + + $data = json_decode($body, true); + + $this->assertEquals([2, 4, 3, 1], Arr::pluck($data['data'], 'id')); + } + + #[Test] + public function list_discussions_pin_setting_disabled_does_not_affect_tag_pages() + { + $this->setting('flarum-sticky.pin_sticky_on_all_discussions', false); + + $response = $this->send( + $this->request('GET', '/api/discussions', [ + 'authenticatedAs' => 3 + ])->withQueryParams([ + 'filter' => [ + 'tag' => 'general' + ] + ]) + ); + + $this->assertEquals(200, $response->getStatusCode(), $body = $response->getBody()->getContents()); + + $data = json_decode($body, true); + + $this->assertEquals([3, 1, 2, 4], Arr::pluck($data['data'], 'id')); + } }