From fcb291b42e9694f8185ea8817251ad1e04a6d7eb Mon Sep 17 00:00:00 2001 From: Raed Shomali Date: Wed, 26 Jul 2023 22:12:13 -0400 Subject: [PATCH] fix: Improve Reply to be Thread Aware (#139) --- context.go | 26 +++++++++++++++++++++++-- examples/interaction-middleware/main.go | 3 +-- examples/job-middleware/main.go | 5 ++--- examples/message-error/main.go | 2 +- examples/message-thread/main.go | 2 +- message_event.go | 9 +++------ options.go | 8 ++++---- response_replier.go | 8 +++++--- util.go | 9 +++++++++ 9 files changed, 50 insertions(+), 22 deletions(-) create mode 100644 util.go diff --git a/context.go b/context.go index d3d9cf4..1f87c27 100644 --- a/context.go +++ b/context.go @@ -18,7 +18,7 @@ func newCommandContext( ) *CommandContext { request := newRequest(parameters) writer := newWriter(ctx, logger, slackClient) - replier := newReplier(event.ChannelID, event.UserID, event.TimeStamp, writer) + replier := newReplier(event.ChannelID, event.UserID, event.InThread(), event.TimeStamp, writer) response := newResponseReplier(writer, replier) return &CommandContext{ @@ -28,6 +28,7 @@ func newCommandContext( definition: definition, request: request, response: response, + logger: logger, } } @@ -39,6 +40,7 @@ type CommandContext struct { definition *CommandDefinition request *Request response *ResponseReplier + logger Logger } // Context returns the context @@ -71,6 +73,11 @@ func (r *CommandContext) Response() *ResponseReplier { return r.response } +// Logger returns the logger +func (r *CommandContext) Logger() Logger { + return r.logger +} + // newInteractionContext creates a new interaction context func newInteractionContext( ctx context.Context, @@ -79,8 +86,9 @@ func newInteractionContext( callback *slack.InteractionCallback, definition *InteractionDefinition, ) *InteractionContext { + inThread := isMessageInThread(callback.OriginalMessage.ThreadTimestamp, callback.OriginalMessage.Timestamp) writer := newWriter(ctx, logger, slackClient) - replier := newReplier(callback.Channel.ID, callback.User.ID, callback.MessageTs, writer) + replier := newReplier(callback.Channel.ID, callback.User.ID, inThread, callback.MessageTs, writer) response := newResponseReplier(writer, replier) return &InteractionContext{ ctx: ctx, @@ -88,6 +96,7 @@ func newInteractionContext( callback: callback, slackClient: slackClient, response: response, + logger: logger, } } @@ -98,6 +107,7 @@ type InteractionContext struct { callback *slack.InteractionCallback slackClient *slack.Client response *ResponseReplier + logger Logger } // Context returns the context @@ -125,6 +135,11 @@ func (r *InteractionContext) SlackClient() *slack.Client { return r.slackClient } +// Logger returns the logger +func (r *InteractionContext) Logger() Logger { + return r.logger +} + // newJobContext creates a new bot context func newJobContext(ctx context.Context, logger Logger, slackClient *slack.Client, definition *JobDefinition) *JobContext { writer := newWriter(ctx, logger, slackClient) @@ -134,6 +149,7 @@ func newJobContext(ctx context.Context, logger Logger, slackClient *slack.Client definition: definition, slackClient: slackClient, response: response, + logger: logger, } } @@ -143,6 +159,7 @@ type JobContext struct { definition *JobDefinition slackClient *slack.Client response *ResponseWriter + logger Logger } // Context returns the context @@ -164,3 +181,8 @@ func (r *JobContext) Response() *ResponseWriter { func (r *JobContext) SlackClient() *slack.Client { return r.slackClient } + +// Logger returns the logger +func (r *JobContext) Logger() Logger { + return r.logger +} diff --git a/examples/interaction-middleware/main.go b/examples/interaction-middleware/main.go index ca93110..1ad3908 100644 --- a/examples/interaction-middleware/main.go +++ b/examples/interaction-middleware/main.go @@ -2,7 +2,6 @@ package main import ( "context" - "fmt" "log" "os" @@ -66,7 +65,7 @@ func slackerInteractive(ctx *slacker.InteractionContext) { func LoggingInteractionMiddleware() slacker.InteractionMiddlewareHandler { return func(next slacker.InteractionHandler) slacker.InteractionHandler { return func(ctx *slacker.InteractionContext) { - fmt.Printf( + ctx.Logger().Infof( "%s initiated \"%s\" with action \"%v\" in channel %s\n", ctx.Callback().User.ID, ctx.Definition().BlockID, diff --git a/examples/job-middleware/main.go b/examples/job-middleware/main.go index f2d4f21..8a585be 100644 --- a/examples/job-middleware/main.go +++ b/examples/job-middleware/main.go @@ -2,7 +2,6 @@ package main import ( "context" - "fmt" "log" "os" @@ -54,12 +53,12 @@ func main() { func LoggingJobMiddleware() slacker.JobMiddlewareHandler { return func(next slacker.JobHandler) slacker.JobHandler { return func(ctx *slacker.JobContext) { - fmt.Printf( + ctx.Logger().Infof( "%s started\n", ctx.Definition().Name, ) next(ctx) - fmt.Printf( + ctx.Logger().Infof( "%s ended\n", ctx.Definition().Name, ) diff --git a/examples/message-error/main.go b/examples/message-error/main.go index 9d186f1..53c53a0 100644 --- a/examples/message-error/main.go +++ b/examples/message-error/main.go @@ -27,7 +27,7 @@ func main() { Command: "thread", Description: "Tests errors in threads", Handler: func(ctx *slacker.CommandContext) { - ctx.Response().ReplyError(errors.New("oops, an error occurred"), slacker.WithInThread()) + ctx.Response().ReplyError(errors.New("oops, an error occurred"), slacker.WithInThread(true)) }, } diff --git a/examples/message-thread/main.go b/examples/message-thread/main.go index 36018db..8404442 100644 --- a/examples/message-thread/main.go +++ b/examples/message-thread/main.go @@ -18,7 +18,7 @@ func main() { Description: "Ping!", Examples: []string{"ping"}, Handler: func(ctx *slacker.CommandContext) { - ctx.Response().Reply("pong", slacker.WithInThread()) + ctx.Response().Reply("pong", slacker.WithInThread(true)) }, } diff --git a/message_event.go b/message_event.go index 19e1757..fe95054 100644 --- a/message_event.go +++ b/message_event.go @@ -50,12 +50,9 @@ type MessageEvent struct { BotID string } -// IsThread indicates if a message event took place in a thread. -func (e *MessageEvent) IsThread() bool { - if e.ThreadTimeStamp == "" || e.ThreadTimeStamp == e.TimeStamp { - return false - } - return true +// InThread indicates if a message event took place in a thread. +func (e *MessageEvent) InThread() bool { + return isMessageInThread(e.ThreadTimeStamp, e.TimeStamp) } // IsBot indicates if the message was sent by a bot diff --git a/options.go b/options.go index dae30d9..5ab0b85 100644 --- a/options.go +++ b/options.go @@ -81,9 +81,9 @@ func WithAttachments(attachments []slack.Attachment) ReplyOption { } // WithInThread specifies whether to reply inside a thread of the original message -func WithInThread() ReplyOption { +func WithInThread(inThread bool) ReplyOption { return func(defaults *replyOptions) { - defaults.InThread = true + defaults.InThread = &inThread } } @@ -110,7 +110,7 @@ func WithSchedule(timestamp time.Time) ReplyOption { type replyOptions struct { Attachments []slack.Attachment - InThread bool + InThread *bool ReplaceMessageTS string IsEphemeral bool ScheduleTime *time.Time @@ -120,7 +120,7 @@ type replyOptions struct { func newReplyOptions(options ...ReplyOption) *replyOptions { config := &replyOptions{ Attachments: []slack.Attachment{}, - InThread: false, + InThread: nil, } for _, option := range options { diff --git a/response_replier.go b/response_replier.go index 5bc6951..9184454 100644 --- a/response_replier.go +++ b/response_replier.go @@ -5,14 +5,15 @@ import ( ) // newReplier creates a new replier structure -func newReplier(channelID string, userID string, eventTS string, writer *Writer) *Replier { - return &Replier{channelID: channelID, userID: userID, eventTS: eventTS, writer: writer} +func newReplier(channelID string, userID string, inThread bool, eventTS string, writer *Writer) *Replier { + return &Replier{channelID: channelID, userID: userID, inThread: inThread, eventTS: eventTS, writer: writer} } // Replier sends messages to the same channel the event came from type Replier struct { channelID string userID string + inThread bool eventTS string writer *Writer } @@ -41,7 +42,8 @@ func (r *Replier) convertOptions(options ...ReplyOption) []PostOption { SetAttachments(replyOptions.Attachments), } - if replyOptions.InThread { + // If the original message came from a thread, reply in a thread, unless there is an override + if (replyOptions.InThread == nil && r.inThread) || (replyOptions.InThread != nil && *replyOptions.InThread) { responseOptions = append(responseOptions, SetThreadTS(r.eventTS)) } diff --git a/util.go b/util.go new file mode 100644 index 0000000..408a062 --- /dev/null +++ b/util.go @@ -0,0 +1,9 @@ +package slacker + +// isMessageInThread determines if a message is in a thread +func isMessageInThread(threadTimestamp string, messageTimestamp string) bool { + if threadTimestamp == "" || threadTimestamp == messageTimestamp { + return false + } + return true +}