Introduce Forum Management API #1308
Conversation
Allows easier access & management of forum posting. See Together-Java#1307
Introduced a forum management API. TransferQuestionCommand now makes use of it. See Together-Java#1307
|
|
| .orElseThrow(() -> new IllegalStateException( | ||
| "Did not find a forum channel with ID %s while trying to create a forum post. Make sure the config is setup properly.")); |
There was a problem hiding this comment.
I personally prefer an IllegalArgumentException.
Why is this exception message mentioning config? That looks like guessing how this method is used.
Same for #findForumPost.
There was a problem hiding this comment.
It was basically a copy & paste from the original code. For channels, it's possible the issue could be from a config issue, so it's nice to let the user know. I removed that part from findForumPost, because it's more common for that to be used for temporary stuff
EDIT: Seems like I didn't push the findForumPost message change. I'll fix all that up
I agree, if it was a bad ID given as an argument, it should be IllegalArgumentException. Good catch, I'll update it
|
|
||
| import java.util.*; | ||
|
|
||
| public final class ForumPoster { |
There was a problem hiding this comment.
i find the name of the class confusing. for quite some time it sounded to me like its referring to a person posting in the forum, i.e. Poster being something like OP, author of a post.
but it is referring to this being an utility to post stuff in forums. i would probably have renamed it to Forums, but since the API is builder-like and wants to create instances, a more suitable name is perhaps ForumPostBuilder, also similar to JDAs MessageCreateBuilder and MessageEditBuilder that we are all used to.
or alternatively ForumPost, since its not actually a builder in the classic sense.
| return new ForumPoster(jda); | ||
| } | ||
|
|
||
| private ForumChannel findForumChannel(String channelId) { |
There was a problem hiding this comment.
all these channelId and postIds in the class should probably be long instead
| return Optional.ofNullable(jda.getForumChannelById(channelId)) | ||
| .orElseThrow(() -> new IllegalArgumentException( | ||
| "Did not find a forum channel with ID %s while trying to create a forum post. Make sure the config is setup properly.")); | ||
| } | ||
|
|
||
| private ThreadChannel findForumPost(String postId) { | ||
| return Optional.ofNullable(jda.getThreadChannelById(postId)) | ||
| .orElseThrow(() -> new IllegalArgumentException( | ||
| "Did not find the forum post with ID %s while trying to reply to a post.")); | ||
|
|
||
| } |
There was a problem hiding this comment.
do we want this to throw if not found or return Optional instead, so the caller can deal with how to properly display error messages and the like to the user.
| return Optional.ofNullable(jda.getThreadChannelById(postId)) | ||
| .orElseThrow(() -> new IllegalArgumentException( | ||
| "Did not find the forum post with ID %s while trying to reply to a post.")); | ||
|
|
| public MessageCreateAction sendPost(String postId, MessageCreateData messageData) { | ||
| return sendPost(findForumPost(postId), messageData); | ||
| } |
There was a problem hiding this comment.
i dont quite understand this. does this reply to a given message? or is postId reffering to a channel and not a "message"?
| public MessageCreateAction sendPost(ThreadChannel post, MessageCreateData messageData) { | ||
| return post.sendMessage(messageData); | ||
| } | ||
|
|
||
| public ForumPostAction createPost(String channelId, String title, MessageCreateData message) { | ||
| return createPost(findForumChannel(channelId), title, message); | ||
| } | ||
|
|
||
| public ForumPostAction createPost(ForumChannel channel, String title, | ||
| MessageCreateData message) { | ||
| return channel.createForumPost(title, message); | ||
| } |
There was a problem hiding this comment.
potentially a more convenient API that also aligns more with what JDA already provides and all contributors are used to, would be to create an actual builder. so something where you can do
ForumPost.using(jda)
.forum(channel)
.title(title)
.message(messageCreateData)
.post();given that you need at least one object coming from JDA within this chain, you could even drop the using(jda). For example, it could extract it from messageCreateData or from the channel it finds.
|
|
||
| import java.util.*; | ||
|
|
||
| public final class ForumPoster { |
There was a problem hiding this comment.
(public classes and methods require meaningful/useful JavaDoc in this project)
Introduces
ForumPosterand updatesTransferQuestionCommandto make use of it.Addresses issue #1307