Skip to content

Commit 2b4932c

Browse files
[2.x] feat: add setting to disable sticky pinning on All Discussions page (#4609)
* feat(sticky): add setting to disable sticky pinning on All Discussions Adds an admin toggle (default: enabled) that gates sticky pinning on the All Discussions page. When disabled, stickied discussions appear at their natural last_posted_at position. Tag pages are unaffected. Restructures PinStickiedDiscussionsToTop so the new toggle is the master gate for /all and properly subordinates only_sticky_unread_- discussions, which is shown disabled in the admin UI when the master toggle is off. Consolidates multiple Extend\Settings declarations into one block. * test(sticky): pin no-sticky-pinning behaviour on a non-tag filter With only_sticky_unread_discussions off, the reordered mutator no longer pins stickied discussions on filtered listings other than a single tag page (e.g. a by-author search) — pinning is scoped to /all and single-tag pages. This locks that behaviour, which previously went untested. --------- Co-authored-by: IanM <ian@morland.me>
1 parent 0a9ef63 commit 2b4932c

5 files changed

Lines changed: 133 additions & 20 deletions

File tree

extensions/sticky/extend.php

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,11 @@
3333
new Extend\Locales(__DIR__.'/locale'),
3434

3535
(new Extend\Settings())
36+
->default('flarum-sticky.enable_display_excerpt', true)
37+
->default('flarum-sticky.only_sticky_unread_discussions', true)
38+
->default('flarum-sticky.pin_sticky_on_all_discussions', true)
3639
->serializeToForum('excerptDisplayEnabled', 'flarum-sticky.enable_display_excerpt', 'boolval')
37-
->default('flarum-sticky.enable_display_excerpt', true),
40+
->serializeToForum('onlyStickyUnreadDiscussions', 'flarum-sticky.only_sticky_unread_discussions', 'boolval'),
3841

3942
(new Extend\Model(Discussion::class))
4043
->cast('is_sticky', 'bool'),
@@ -56,10 +59,6 @@
5659
->addFilter(DiscussionSearcher::class, StickyFilter::class)
5760
->addMutator(DiscussionSearcher::class, PinStickiedDiscussionsToTop::class),
5861

59-
(new Extend\Settings())
60-
->default('flarum-sticky.only_sticky_unread_discussions', true)
61-
->serializeToForum('onlyStickyUnreadDiscussions', 'flarum-sticky.only_sticky_unread_discussions', 'boolval'),
62-
6362
(new Extend\Conditional())
6463
->whenExtensionEnabled('flarum-realtime', fn () => [
6564
(new RealtimeExtend())

extensions/sticky/js/src/admin/extend.tsx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,25 @@ export default [
1515
'moderate',
1616
95
1717
)
18+
.setting(
19+
() => ({
20+
setting: 'flarum-sticky.pin_sticky_on_all_discussions',
21+
name: 'pinStickyOnAllDiscussions',
22+
type: 'boolean',
23+
label: app.translator.trans('flarum-sticky.admin.settings.pin_sticky_on_all_discussions_label'),
24+
help: app.translator.trans('flarum-sticky.admin.settings.pin_sticky_on_all_discussions_help'),
25+
}),
26+
95
27+
)
1828
.setting(
1929
() => ({
2030
setting: 'flarum-sticky.only_sticky_unread_discussions',
2131
name: 'onlyStickyUnreadDiscussions',
2232
type: 'boolean',
2333
label: app.translator.trans('flarum-sticky.admin.settings.only_sticky_unread_discussions_label'),
2434
help: app.translator.trans('flarum-sticky.admin.settings.only_sticky_unread_discussions_help'),
35+
// Only meaningful when sticky pinning is enabled on the All Discussions page.
36+
disabled: app.data.settings['flarum-sticky.pin_sticky_on_all_discussions'] === '0',
2537
}),
2638
90
2739
)

extensions/sticky/locale/en.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ flarum-sticky:
1010
enable_display_excerpt: Show an excerpt of the first post when a sticky discussion is unread
1111
only_sticky_unread_discussions_label: Only sticky unread discussions
1212
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.
13+
pin_sticky_on_all_discussions_label: Pin stickied discussions on the All Discussions page
14+
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.
1315

1416
# These translations are used in the Permissions page of the admin interface.
1517
permissions:

extensions/sticky/src/PinStickiedDiscussionsToTop.php

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -27,18 +27,8 @@ public function __invoke(DatabaseSearchState $state, SearchCriteria $criteria):
2727
{
2828
if ($criteria->sortIsDefault && ! $state->isFulltextSearch()) {
2929
$query = $state->getQuery()->getQuery();
30-
$onlyStickyUnread = $this->settings->get('flarum-sticky.only_sticky_unread_discussions');
3130

32-
// If only sticky unread discussions is disabled, then pin all stickied
33-
// discussions to the top whether they are read or not.
34-
if (! $onlyStickyUnread) {
35-
$this->pinStickiedToTop($query);
36-
37-
return;
38-
}
39-
40-
// If we are viewing a specific tag, then pin all stickied
41-
// discussions to the top no matter what.
31+
// Tag pages always pin stickied discussions to the top.
4232
$filters = $state->getActiveFilters();
4333

4434
if ($count = count($filters)) {
@@ -49,10 +39,27 @@ public function __invoke(DatabaseSearchState $state, SearchCriteria $criteria):
4939
return;
5040
}
5141

52-
// Otherwise, if we are viewing "all discussions", only pin stickied
53-
// discussions to the top if they are unread. To do this in a
54-
// performant way we create another query which will select all
55-
// stickied discussions, marry them into the main query, and then
42+
// The remainder of this method handles the All Discussions page only.
43+
44+
// Admins can disable sticky pinning on this page entirely. When disabled,
45+
// stickied discussions appear at their natural last_posted_at position
46+
// and the only_sticky_unread_discussions setting becomes a no-op (the
47+
// distinction between read and unread sticky no longer matters).
48+
if (! $this->settings->get('flarum-sticky.pin_sticky_on_all_discussions', true)) {
49+
return;
50+
}
51+
52+
// If unread-only floating is disabled, pin all stickied discussions to
53+
// the top regardless of read state.
54+
if (! $this->settings->get('flarum-sticky.only_sticky_unread_discussions')) {
55+
$this->pinStickiedToTop($query);
56+
57+
return;
58+
}
59+
60+
// Otherwise, only pin stickied discussions to the top if they are unread.
61+
// To do this in a performant way we create another query which will select
62+
// all stickied discussions, marry them into the main query, and then
5663
// reorder the unread ones up to the top.
5764
$sticky = clone $query;
5865
$sticky->where('is_sticky', true);

extensions/sticky/tests/integration/api/ListDiscussionsTest.php

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,4 +193,97 @@ public function list_discussions_shows_stick_first_on_a_tag()
193193

194194
$this->assertEquals([3, 1, 2, 4], Arr::pluck($data['data'], 'id'));
195195
}
196+
197+
#[Test]
198+
public function list_discussions_does_not_pin_sticky_on_all_when_pin_setting_disabled_as_guest()
199+
{
200+
$this->setting('flarum-sticky.pin_sticky_on_all_discussions', false);
201+
202+
$response = $this->send(
203+
$this->request('GET', '/api/discussions')
204+
);
205+
206+
$this->assertEquals(200, $response->getStatusCode(), $body = $response->getBody()->getContents());
207+
208+
$data = json_decode($body, true);
209+
210+
$this->assertEquals([2, 4, 3, 1], Arr::pluck($data['data'], 'id'));
211+
}
212+
213+
#[Test]
214+
public function list_discussions_pin_setting_disabled_overrides_only_unread_setting_on_all()
215+
{
216+
// pin_sticky_on_all_discussions is the master gate for /all and must
217+
// override only_sticky_unread_discussions when both are flipped.
218+
$this->setting('flarum-sticky.pin_sticky_on_all_discussions', false);
219+
$this->setting('flarum-sticky.only_sticky_unread_discussions', false);
220+
221+
$response = $this->send(
222+
$this->request('GET', '/api/discussions', [
223+
'authenticatedAs' => 2
224+
])
225+
);
226+
227+
$this->assertEquals(200, $response->getStatusCode(), $body = $response->getBody()->getContents());
228+
229+
$data = json_decode($body, true);
230+
231+
$this->assertEquals([2, 4, 3, 1], Arr::pluck($data['data'], 'id'));
232+
}
233+
234+
#[Test]
235+
public function list_discussions_pin_setting_disabled_does_not_affect_tag_pages()
236+
{
237+
$this->setting('flarum-sticky.pin_sticky_on_all_discussions', false);
238+
239+
$response = $this->send(
240+
$this->request('GET', '/api/discussions', [
241+
'authenticatedAs' => 3
242+
])->withQueryParams([
243+
'filter' => [
244+
'tag' => 'general'
245+
]
246+
])
247+
);
248+
249+
$this->assertEquals(200, $response->getStatusCode(), $body = $response->getBody()->getContents());
250+
251+
$data = json_decode($body, true);
252+
253+
$this->assertEquals([3, 1, 2, 4], Arr::pluck($data['data'], 'id'));
254+
}
255+
256+
#[Test]
257+
public function list_discussions_only_unread_off_does_not_pin_sticky_on_a_non_tag_filter()
258+
{
259+
// With only_sticky_unread_discussions off, pinning applies to the
260+
// All Discussions page and to a single-tag page — but NOT to other
261+
// filtered listings (e.g. a by-author search). Previously the
262+
// "pin all when only_unread is off" branch ran ahead of the filter
263+
// check, so it pinned sticky on any filtered listing; pinning is now
264+
// scoped to /all and single-tag pages, which is the intended behaviour.
265+
$this->setting('flarum-sticky.only_sticky_unread_discussions', false);
266+
267+
$response = $this->send(
268+
$this->request('GET', '/api/discussions', [
269+
'authenticatedAs' => 1,
270+
])->withQueryParams([
271+
'filter' => [
272+
'author' => 'Muralf',
273+
],
274+
])
275+
);
276+
277+
$this->assertEquals(200, $response->getStatusCode(), $body = $response->getBody()->getContents());
278+
279+
$data = json_decode($body, true);
280+
281+
// Natural last_posted_at order — no sticky pinning on a non-tag filter.
282+
// The sticky discussions (1 and 3) sit last, in their natural position,
283+
// rather than floating to the top. (5 is also sticky and appears here
284+
// because the author filter isn't tag-scoped and the actor is an admin;
285+
// it leads only because it is the most recently posted, not because it
286+
// is stickied.)
287+
$this->assertEquals([5, 2, 4, 3, 1], Arr::pluck($data['data'], 'id'));
288+
}
196289
}

0 commit comments

Comments
 (0)