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
Add Forum Post Support #1609
Add Forum Post Support #1609
Conversation
…s createForumPost and removeForumPost IsForum exposed to CCs to quickly check if a channel is a forum channel AvailableTags are exposed to CCs as a slice of discordgo.ForumTag AppliedTags are exposed to CCs as a slice of int64 ids
common/templates/context.go
Outdated
c.addContextFunc("createThread", c.tmplCreateThread) | ||
c.addContextFunc("deleteThread", c.tmplDeleteThread) | ||
c.addContextFunc("addThreadMember", c.tmplThreadMemberAdd) | ||
c.addContextFunc("removeThreadMember", c.tmplThreadMemberRemove) | ||
|
||
// forum functions | ||
c.addContextFunc("createForumPost", c.tmplCreateForumPost) | ||
c.addContextFunc("removeForumPost", c.tmplRemoveForumPost) |
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.
deleteForumPost
feels better (create > delete, add > remove, give > take logic), also no need for additional c.tmplRemoveForumPost
why not just c.tmplDeleteThread
one could always add a comment to that method, that it deletes both, simple thread and a forum post
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.
That makes sense, I renamed it "deleteForumPost". I also removed tmplRemoveForumPost in favor of reusing tmplDeleteThread (added a comment about being it able to delete both types of threads like you suggested).
common/templates/context_funcs.go
Outdated
return rateLimit, tags, nil | ||
} | ||
|
||
func (c *Context) tmplCreateForumPost(channel, content interface{}, name string, optional ...interface{}) (*CtxChannel, error) { |
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.
To me workflow of giving arguments > channel, name, content, (optional...) feels better. Post title is sort of first than post content. I know it needs interface{} twice then : )
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.
Latest commit switches the order to be channel, name, content, (optional...)
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.
I'm also wondering if createThread
from the last PR should be changed to follow the same format. For example
createThread channel name (optional: "message_id" arg "private" arg)
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.
looks reasonable, to have thread's name first rest is already all content and how content is shown
but at its current state, it is less code and writing createThread nil nil "threadName"
is not that cumbersome.
so let's wait what developer thinks, both ways are good for me.
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.
it think that parameter name
could also be part of type interface{} and you later ToString() it, because in some cases it is easier to use pure numbers as thread/forumPost name : )
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.
Pushed a change that switches both create functions to use interface{} for name
. It calls ToString() on it before passing it to discordgo.
…or createForumPost
…moveForumPost (reuses tmplDeleteThread instead)
DefaultReactionEmoji discordgo.ForumDefaultReaction `json:"default_reaction_emoji"` | ||
DefaultThreadRateLimitPerUser int `json:"default_thread_rate_limit_per_user"` | ||
DefaultSortOrder *discordgo.ForumSortOrderType `json:"default_sort_order"` | ||
DefaultForumLayout discordgo.ForumLayout `json:"default_forum_layout"` |
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.
I think, for IsPrivate() method few lines down, || c.Type == discordgo.ChannelTypeGuildPrivateThread
needs to be added to if
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.
Does GuildPrivateThread sit on the same level as DM privacy?
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.
looking at how this is handled in context functions, well it is a badly named field. Channel Type 12 says yeah it's private as well. so better not change this.
DefaultReactionEmoji ForumDefaultReaction `json:"default_reaction_emoji"` | ||
|
||
// The initial RateLimitPerUser to set on newly created threads in a channel. | ||
// This field is copied to the thread at creation time and does not live update. | ||
DefaultThreadRateLimitPerUser int `json:"default_thread_rate_limit_per_user"` | ||
|
||
// The default sort order type used to order posts in forum channels. | ||
// Defaults to null, which indicates a preferred sort order hasn't been set by a channel admin. | ||
DefaultSortOrder *ForumSortOrderType `json:"default_sort_order"` | ||
|
||
// The default forum layout view used to display posts in forum channels. | ||
// Defaults to ForumLayoutNotSet, which indicates a layout view has not been set by a channel admin. | ||
DefaultForumLayout ForumLayout `json:"default_forum_layout"` |
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.
Perhaps these fields should be in templates/structs.go > CtxChannel as well
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.
Yeah could add them. When I first went through it I just wasn't sure if people would need those fields in CCs. They're present in the dstate.ChannelState struct though for Go code to use
Continuation of #1604 and #1453
This PR does two things:
createForumPost
anddeleteForumPost
templatesOther Info:
createForumPost
are set to use key-value style so that order doesn't matter and we can also expose other optional args laterAPI (templates):
API (functions):
createForumPost channel name content (optional: "slowmode" arg "tags" arg)
deleteForumPost post
Examples:
(Create)
(Delete)