Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
More things for 2024 #254
More things for 2024 #254
Changes from all commits
e06119d
131f442
4057f5a
a80c36f
e5d0c73
53c3109
5038759
bf5fe33
d7edefe
3eef127
41c28a4
1b13959
25037f9
62ca93d
1ca19fe
09d58eb
6135d20
313be25
946c0f0
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 aForumData
, this call is difficult to use by itself without also callingGET /api/v3/forum/ID
. Not that big of a deal by itself, but if you're getting to a forum viaGET /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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call regarding the blocks/mutes/words and buildForumData. I'll add that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 aForum
and does its own query to build posts from there. IMO adding apinsOnly = true
mode adds excessive complexity to an already complex function. Going through the major features: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 theForumData
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 apinnedPostCount
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.