More things for 2024 #254
More things for 2024 #254
Conversation
for jocosocial#174 add support for pinned posts make forum pins an api endpoint rather than a query help text
| .filter(\.$forum.$id == forum.requireID()) | ||
| .filter(\.$pinned == true) | ||
| .all() | ||
| return try await buildPostData(query, userID: user.userID, on: req) |
There was a problem hiding this comment.
The fact that the forum creator can pin posts in their forum greatly increases the likelihood that someone makes a forum with more than a page of pinned posts. Mods aren't likely to do it, but thread creators just might.
There was a problem hiding this comment.
I think if they do they risk pissing off their readers, which could also attract moderator attention. Do you think we should restrict pinning posts to just moderators? I feel like that'll make the feature significantly less effective.
There was a problem hiding this comment.
No, I'm just saying that I'd prefer a solution that doesn't buckle with a ton of pinned posts.
| /// | ||
| /// - Parameter forumID: In the URL path. | ||
| /// - Returns array of `PostData`. | ||
| func forumPinnedPostsHandler(_ req: Request) async throws -> [PostData] { |
There was a problem hiding this comment.
There's a bunch of stuff that buildForumData does that this method doesn't, besides pagination. It appears that this fn doesn't respect blocks, mutes, or mutewords, and because it returns an array of PostData instead of a ForumData, this call is difficult to use by itself without also calling GET /api/v3/forum/ID. Not that big of a deal by itself, but if you're getting to a forum via GET /api/v3/forum/post/ID/forum (the call that is used to get from a search results post to the forum containing that post), you'd have to call the post-to-forum call first, and then use that to get the forum ID you need to get the pinned posts in that forum.
There was a problem hiding this comment.
Good call regarding the blocks/mutes/words and buildForumData. I'll add that.
There was a problem hiding this comment.
Ok i've added blocks/mutes/muteword support.
Unless I'm totally high: buildForumData doesn't apply here. That function as currently designed starts from a Forum and does its own query to build posts from there. IMO adding a pinsOnly = true mode adds excessive complexity to an already complex function. Going through the major features:
- blocks/mutes/mutewords can already be done in the request handler query and matches patterns done elsewher.e
- start/limit doesn't really apply since this usage really shouldn't support pagination. If someone has paginated the number of pinned posts in a thread, jezus lord we're off the rails. I'd rather institute a "no more than 5 pins per thread" sort of limit on the pin create handler at that point.
- lastPostID and event data is irrelevant for this use.
As for the call being difficult to use by itself, I'd say "sort-of".
In the web UI we display pinned posts in the normal "thread view" and in the "thread from post view". In the former case, the forumID is in the URL so that's an easy async GET to both /forum/:forum_ID /forum/:forum_ID/pinnedposts. In the latter case, it's a [not-A]sync GET since we have to wait for the ForumData response in the /forum/post/:postID/forum call to then do the /forum/:forum_ID/pinnedposts call. IMO this minor sub-optimization is acceptable.
Tricordarr doesn't even go this far. There is a dedicated "pinned posts" button in the header of the /forum/:forum_ID and /forum/post/:post_ID/forum screens that takes you to the /forum/:forum_ID/pinnedposts screen. Either one doesn't execute until the user does something anyway.
There was a problem hiding this comment.
Waxing poetically here for a moment: I'm not even sure the pinned posts at the top of the screen should remain in the "long term" in the Web UI. It could rapidly get overwhelmed and should likely end up in a sub view. At which point perhaps buildForumData returns a pinnedPostCount that can give the user a more useful hint that there's pinned content elsewhere. And a button will take them there. But either way it still requires a dedicated API call that IMO should not take in any more info than it needs to.
|
Chall and I had a good phone call involving lobsters and pinned posts. Good to merge. |
Fixed Issues:
Touched Issues:
Misc: