From 6ed7ff330eab09d0e9b390966d347662795f3bfd Mon Sep 17 00:00:00 2001 From: Steven Arnott Date: Wed, 23 Oct 2024 13:51:48 -0400 Subject: [PATCH 1/8] Add reaction handling --- core/matcher.go | 12 ++-- core/matcher_test.go | 19 +++--- models/message.go | 2 + models/rule.go | 1 + remote/slack/helper.go | 131 ++++++++++++++++++++++++++++++++++++++++- 5 files changed, 151 insertions(+), 14 deletions(-) diff --git a/core/matcher.go b/core/matcher.go index 237ab7cf..0fcaee8d 100644 --- a/core/matcher.go +++ b/core/matcher.go @@ -38,7 +38,7 @@ RuleSearch: // Only check active rules. if rule.Active { // Init some variables for use below - processedInput, hit := getProccessedInputAndHitValue(message.Input, rule.Respond, rule.Hear) + processedInput, hit := getProccessedInputAndHitValue(message.Input, rule.Respond, rule.Hear, message.Reaction, rule.ReactionMatch) // Determine what service we are processing the rule for switch message.Service { case models.MsgServiceChat, models.MsgServiceCLI: @@ -63,12 +63,14 @@ RuleSearch: } // getProccessedInputAndHitValue gets the processed input from the message input and the true/false if it was a successfully hit rule. -func getProccessedInputAndHitValue(messageInput, ruleRespondValue, ruleHearValue string) (string, bool) { +func getProccessedInputAndHitValue(messageInput, ruleRespondValue, ruleHearValue, messageReaction, ruleReactionMatch string) (string, bool) { processedInput, hit := "", false if ruleRespondValue != "" { processedInput, hit = utils.Match(ruleRespondValue, messageInput, true) } else if ruleHearValue != "" { // Are we listening to everything? _, hit = utils.Match(ruleHearValue, messageInput, false) + } else if ruleReactionMatch != "" { + processedInput, hit = utils.Match(ruleReactionMatch, messageReaction, false) } return processedInput, hit @@ -80,8 +82,8 @@ func getProccessedInputAndHitValue(messageInput, ruleRespondValue, ruleHearValue func handleChatServiceRule(outputMsgs chan<- models.Message, message models.Message, hitRule chan<- models.Rule, rule models.Rule, processedInput string, hit bool, bot *models.Bot) (bool, bool) { match, stopSearch := false, false - if rule.Respond != "" || rule.Hear != "" { - // You can only use 'respond' OR 'hear' + if rule.Respond != "" || rule.Hear != "" || rule.ReactionMatch != "" { + // You can only use 'respond', 'hear', or 'reaction match' if rule.Respond != "" && rule.Hear != "" { log.Debug().Msgf("rule %#q has both 'hear' and 'match' or 'respond' defined. please choose one or the other", rule.Name) } @@ -231,7 +233,7 @@ func isValidHitChatRule(message *models.Message, rule models.Rule, processedInpu } // If this wasn't a 'hear' rule, handle the args - if rule.Hear == "" { + if rule.Hear == "" && rule.ReactionMatch == "" { // Get all the args that the message sender supplied args := utils.RuleArgTokenizer(processedInput) diff --git a/core/matcher_test.go b/core/matcher_test.go index e7c1d71d..40649811 100644 --- a/core/matcher_test.go +++ b/core/matcher_test.go @@ -584,9 +584,11 @@ func TestUpdateReaction(t *testing.T) { func Test_getProccessedInputAndHitValue(t *testing.T) { type args struct { - messageInput string - ruleRespondValue string - ruleHearValue string + messageInput string + ruleRespondValue string + ruleHearValue string + messageReaction string + ruleReactionMatch string } tests := []struct { @@ -595,15 +597,16 @@ func Test_getProccessedInputAndHitValue(t *testing.T) { want string want1 bool }{ - {"hit", args{"hello foo", "hello", "hello"}, "foo", true}, - {"hit no hear value", args{"hello foo", "hello", ""}, "foo", true}, - {"hit no respond value - drops args", args{"hello foo", "", "hello"}, "", true}, - {"no match", args{"hello foo", "", ""}, "", false}, + {"hit", args{"hello foo", "hello", "hello", "", ""}, "foo", true}, + {"hit no hear value", args{"hello foo", "hello", "", "", ""}, "foo", true}, + {"hit no respond value - drops args", args{"hello foo", "", "hello", "", ""}, "", true}, + {"no match", args{"hello foo", "", "", "", ""}, "", false}, + {"hit reaction", args{"", "", "", ":hello:", ":hello:"}, ":hello:", true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, got1 := getProccessedInputAndHitValue(tt.args.messageInput, tt.args.ruleRespondValue, tt.args.ruleHearValue) + got, got1 := getProccessedInputAndHitValue(tt.args.messageInput, tt.args.ruleRespondValue, tt.args.ruleHearValue, tt.args.messageReaction, tt.args.ruleReactionMatch) if got != tt.want { t.Errorf("getProccessedInputAndHitValue() got = %v, want %v", got, tt.want) } diff --git a/models/message.go b/models/message.go index b75f1e05..3065e0ee 100644 --- a/models/message.go +++ b/models/message.go @@ -17,6 +17,8 @@ type Message struct { ChannelName string Input string Output string + ReactionAction string + Reaction string Error string Timestamp string ThreadID string diff --git a/models/rule.go b/models/rule.go index 5aadbef7..45f39e61 100644 --- a/models/rule.go +++ b/models/rule.go @@ -7,6 +7,7 @@ type Rule struct { Name string `mapstructure:"name" binding:"required"` Respond string `mapstructure:"respond" binding:"omitempty"` Hear string `mapstructure:"hear" binding:"omitempty"` + ReactionMatch string `mapstructure:"reaction_match" binding:"omitempty"` Schedule string `mapstructure:"schedule"` Args []string `mapstructure:"args" binding:"required"` DirectMessageOnly bool `mapstructure:"direct_message_only" binding:"required"` diff --git a/remote/slack/helper.go b/remote/slack/helper.go index 5c6efd22..a95626fa 100644 --- a/remote/slack/helper.go +++ b/remote/slack/helper.go @@ -445,6 +445,75 @@ func populateMessage(message models.Message, msgType models.MessageType, channel } } +// populateMessage - populates the 'Message' object to be passed on for processing/sending. +func populateReaction(message models.Message, msgType models.MessageType, channel, action, reaction, timeStamp, link string, user *slack.User, bot *models.Bot) models.Message { + switch msgType { + case models.MsgTypeDirect, models.MsgTypeChannel, models.MsgTypePrivateChannel: + // Populate message attributes + message.Type = msgType + message.Service = models.MsgServiceChat + message.ChannelID = channel + message.ReactionAction = action + message.Reaction = reaction + message.Input = "" + message.Output = "" + message.Timestamp = timeStamp + message.SourceLink = link + + // If the message read was not a dm, get the name of the channel it came from + if msgType != models.MsgTypeDirect { + name, ok := findKey(bot.Rooms, channel) + if !ok { + log.Error().Msgf("could not find name of channel %#q", channel) + } + + message.ChannelName = name + } + + message.Vars["_reaction.action"] = action + message.Vars["_reaction"] = reaction + // make channel variables available + message.Vars["_channel.id"] = message.ChannelID + message.Vars["_channel.name"] = message.ChannelName // will be empty if it came via DM + + // make link to trigger message available + message.Vars["_source.link"] = message.SourceLink + + // make timestamp information available + message.Vars["_source.timestamp"] = timeStamp + + // Populate message with user information (i.e. who sent the message) + // These will be accessible on rules via ${_user.email}, ${_user.id}, etc. + if user != nil { // nil user implies a message from an api/bot (i.e. not an actual user) + message.Vars["_user.id"] = user.ID + message.Vars["_user.teamid"] = user.TeamID + message.Vars["_user.name"] = user.Name + message.Vars["_user.color"] = user.Color + message.Vars["_user.realname"] = user.RealName + message.Vars["_user.tz"] = user.TZ + message.Vars["_user.tzlabel"] = user.TZLabel + message.Vars["_user.tzoffset"] = strconv.Itoa(user.TZOffset) + message.Vars["_user.firstname"] = user.Profile.FirstName + message.Vars["_user.lastname"] = user.Profile.LastName + message.Vars["_user.realnamenormalized"] = user.Profile.RealNameNormalized + message.Vars["_user.displayname"] = user.Profile.DisplayName + message.Vars["_user.displaynamenormalized"] = user.Profile.DisplayNameNormalized + message.Vars["_user.email"] = user.Profile.Email + message.Vars["_user.skype"] = user.Profile.Skype + message.Vars["_user.phone"] = user.Profile.Phone + message.Vars["_user.title"] = user.Profile.Title + message.Vars["_user.statustext"] = user.Profile.StatusText + message.Vars["_user.statusemoji"] = user.Profile.StatusEmoji + message.Vars["_user.team"] = user.Profile.Team + } + + return message + default: + log.Debug().Msgf("read message of unsupported type '%T' - unable to populate message attributes", msgType) + return message + } +} + // readFromEventsAPI utilizes the Slack API client to read event-based messages. // This method of reading is preferred over the RTM method. func readFromEventsAPI(api *slack.Client, vToken string, inputMsgs chan<- models.Message, bot *models.Bot) { @@ -509,7 +578,7 @@ func readFromSocketMode(sm *slack.Client, inputMsgs chan<- models.Message, bot * innerEvent := eventsAPIEvent.InnerEvent switch ev := innerEvent.Data.(type) { - case *slackevents.AppMentionEvent, *slackevents.ReactionAddedEvent: + case *slackevents.AppMentionEvent: continue case *slackevents.MessageEvent: senderID := ev.User @@ -564,6 +633,66 @@ func readFromSocketMode(sm *slack.Client, inputMsgs chan<- models.Message, bot * inputMsgs <- populateMessage(models.NewMessage(), msgType, channel, text, timestamp, threadTimestamp, link, mentioned, user, bot) } + case *slackevents.ReactionAddedEvent: + senderID := ev.User + + if senderID != "" && bot.ID != senderID { + channel := ev.Item.Channel + + // determine the message type + msgType, err := getMessageType(channel) + if err != nil { + log.Error().Msg(err.Error()) + } + + // get information on the user + user, err := sm.GetUserInfo(senderID) + if err != nil { + log.Error().Msgf("did not get slack user info: %s", err.Error()) + } + + timestamp := ev.Item.Timestamp + + reaction := ev.Reaction + + // get the link to the message, will be empty string if there's an error + link, err := sm.GetPermalink(&slack.PermalinkParameters{Channel: channel, Ts: timestamp}) + if err != nil { + log.Error().Msgf("unable to retrieve link to message: %s", err.Error()) + } + + inputMsgs <- populateReaction(models.NewMessage(), msgType, channel, "added", reaction, timestamp, link, user, bot) + } + case *slackevents.ReactionRemovedEvent: + senderID := ev.User + + if senderID != "" && bot.ID != senderID { + channel := ev.Item.Channel + + // determine the message type + msgType, err := getMessageType(channel) + if err != nil { + log.Error().Msg(err.Error()) + } + + // get information on the user + user, err := sm.GetUserInfo(senderID) + if err != nil { + log.Error().Msgf("did not get slack user info: %s", err.Error()) + } + + timestamp := ev.Item.Timestamp + + reaction := ev.Reaction + + // get the link to the message, will be empty string if there's an error + link, err := sm.GetPermalink(&slack.PermalinkParameters{Channel: channel, Ts: timestamp}) + if err != nil { + log.Error().Msgf("unable to retrieve link to message: %s", err.Error()) + } + + inputMsgs <- populateReaction(models.NewMessage(), msgType, channel, "removed", reaction, timestamp, link, user, bot) + } case *slackevents.MemberJoinedChannelEvent: // limit to our bot if ev.User == bot.ID { From 839a42de8d1089bf62755eb8b608c8af06c402df Mon Sep 17 00:00:00 2001 From: Steven Arnott Date: Wed, 23 Oct 2024 13:59:50 -0400 Subject: [PATCH 2/8] Example --- config-example/rules/reaction.yaml | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 config-example/rules/reaction.yaml diff --git a/config-example/rules/reaction.yaml b/config-example/rules/reaction.yaml new file mode 100644 index 00000000..4289c97a --- /dev/null +++ b/config-example/rules/reaction.yaml @@ -0,0 +1,11 @@ +name: plus-one-reaction +active: true +reaction_match: ":+1:" +args: +# actions +actions: # this rule takes no actions, just collects and responds with whats laid out in the format_response section + +# response +format_output: "Oh a reaction?" # message to send to your user when they say hello +start_message_thread: true # start a thread with the response +direct_message_only: false # allow messaging inside channels From 4e684eec8cd9043b860fed9f5eb65ccb921d8609 Mon Sep 17 00:00:00 2001 From: Steven Arnott Date: Wed, 23 Oct 2024 15:42:22 -0400 Subject: [PATCH 3/8] Some lint clean up --- core/matcher.go | 2 ++ remote/slack/helper.go | 56 +++--------------------------------------- 2 files changed, 6 insertions(+), 52 deletions(-) diff --git a/core/matcher.go b/core/matcher.go index 0fcaee8d..3f44a411 100644 --- a/core/matcher.go +++ b/core/matcher.go @@ -211,6 +211,8 @@ func handleNoMatch(outputMsgs chan<- models.Message, message models.Message, hit } // isValidHitChatRule does additional checks on a successfully hit rule that came from the chat or CLI service. +// +//nolint:gocyclo // refactor func isValidHitChatRule(message *models.Message, rule models.Rule, processedInput string, bot *models.Bot) bool { // Check to honor allow_users or allow_usergroups canRunRule := utils.CanTrigger(message.Vars["_user.name"], message.Vars["_user.id"], rule, bot) diff --git a/remote/slack/helper.go b/remote/slack/helper.go index a95626fa..b70916a7 100644 --- a/remote/slack/helper.go +++ b/remote/slack/helper.go @@ -449,69 +449,21 @@ func populateMessage(message models.Message, msgType models.MessageType, channel func populateReaction(message models.Message, msgType models.MessageType, channel, action, reaction, timeStamp, link string, user *slack.User, bot *models.Bot) models.Message { switch msgType { case models.MsgTypeDirect, models.MsgTypeChannel, models.MsgTypePrivateChannel: - // Populate message attributes - message.Type = msgType - message.Service = models.MsgServiceChat - message.ChannelID = channel message.ReactionAction = action message.Reaction = reaction message.Input = "" message.Output = "" message.Timestamp = timeStamp message.SourceLink = link - - // If the message read was not a dm, get the name of the channel it came from - if msgType != models.MsgTypeDirect { - name, ok := findKey(bot.Rooms, channel) - if !ok { - log.Error().Msgf("could not find name of channel %#q", channel) - } - - message.ChannelName = name - } - message.Vars["_reaction.action"] = action message.Vars["_reaction"] = reaction - // make channel variables available - message.Vars["_channel.id"] = message.ChannelID - message.Vars["_channel.name"] = message.ChannelName // will be empty if it came via DM - - // make link to trigger message available - message.Vars["_source.link"] = message.SourceLink - - // make timestamp information available - message.Vars["_source.timestamp"] = timeStamp - - // Populate message with user information (i.e. who sent the message) - // These will be accessible on rules via ${_user.email}, ${_user.id}, etc. - if user != nil { // nil user implies a message from an api/bot (i.e. not an actual user) - message.Vars["_user.id"] = user.ID - message.Vars["_user.teamid"] = user.TeamID - message.Vars["_user.name"] = user.Name - message.Vars["_user.color"] = user.Color - message.Vars["_user.realname"] = user.RealName - message.Vars["_user.tz"] = user.TZ - message.Vars["_user.tzlabel"] = user.TZLabel - message.Vars["_user.tzoffset"] = strconv.Itoa(user.TZOffset) - message.Vars["_user.firstname"] = user.Profile.FirstName - message.Vars["_user.lastname"] = user.Profile.LastName - message.Vars["_user.realnamenormalized"] = user.Profile.RealNameNormalized - message.Vars["_user.displayname"] = user.Profile.DisplayName - message.Vars["_user.displaynamenormalized"] = user.Profile.DisplayNameNormalized - message.Vars["_user.email"] = user.Profile.Email - message.Vars["_user.skype"] = user.Profile.Skype - message.Vars["_user.phone"] = user.Profile.Phone - message.Vars["_user.title"] = user.Profile.Title - message.Vars["_user.statustext"] = user.Profile.StatusText - message.Vars["_user.statusemoji"] = user.Profile.StatusEmoji - message.Vars["_user.team"] = user.Profile.Team - } - return message + message = populateMessage(message, msgType, channel, "", timeStamp, "", link, false, user, bot) default: log.Debug().Msgf("read message of unsupported type '%T' - unable to populate message attributes", msgType) - return message } + + return message } // readFromEventsAPI utilizes the Slack API client to read event-based messages. @@ -548,7 +500,7 @@ func readFromEventsAPI(api *slack.Client, vToken string, inputMsgs chan<- models // // https://api.slack.com/apis/connections/socket // -//nolint:gocyclo // needs refactor +//nolint:gocyclo,funlen // needs refactor func readFromSocketMode(sm *slack.Client, inputMsgs chan<- models.Message, bot *models.Bot) { // setup the client client := socketmode.New(sm) From 2e944adb0495ca53eb02183be4caa5c2453c1e8c Mon Sep 17 00:00:00 2001 From: Steven Arnott Date: Thu, 24 Oct 2024 15:57:40 -0400 Subject: [PATCH 4/8] Split add and remove action to seeperate places Update example Some refactoring in matcher.go --- Makefile | 3 +- config-example/rules/reaction.yaml | 10 +++--- core/matcher.go | 53 +++++++++++++++++++++++++----- core/matcher_test.go | 39 +++++++++++++++------- models/message.go | 4 +-- models/rule.go | 3 +- remote/slack/helper.go | 17 +++++----- 7 files changed, 92 insertions(+), 37 deletions(-) diff --git a/Makefile b/Makefile index 6687d3dd..aec14cda 100644 --- a/Makefile +++ b/Makefile @@ -63,7 +63,8 @@ test-coverage: clean: validate @echo "Running $@ tasks" -rm -v ./flottbot* - -rm -v ./debug + +## -rm -v ./debug #Not created # ┌┐ ┬ ┬┬┬ ┌┬┐ # ├┴┐│ │││ ││ diff --git a/config-example/rules/reaction.yaml b/config-example/rules/reaction.yaml index 4289c97a..76f910fe 100644 --- a/config-example/rules/reaction.yaml +++ b/config-example/rules/reaction.yaml @@ -1,11 +1,11 @@ -name: plus-one-reaction +name: question-reaction active: true -reaction_match: ":+1:" +reactions_added: "question" args: # actions -actions: # this rule takes no actions, just collects and responds with whats laid out in the format_response section - +actions: # no actions # response -format_output: "Oh a reaction?" # message to send to your user when they say hello +format_output: ${_user.firstname} could you elaborate more on your request? + start_message_thread: true # start a thread with the response direct_message_only: false # allow messaging inside channels diff --git a/core/matcher.go b/core/matcher.go index 3f44a411..c96a9f81 100644 --- a/core/matcher.go +++ b/core/matcher.go @@ -38,7 +38,7 @@ RuleSearch: // Only check active rules. if rule.Active { // Init some variables for use below - processedInput, hit := getProccessedInputAndHitValue(message.Input, rule.Respond, rule.Hear, message.Reaction, rule.ReactionMatch) + processedInput, hit := getProccessedInputAndHitValue(message, rule) // Determine what service we are processing the rule for switch message.Service { case models.MsgServiceChat, models.MsgServiceCLI: @@ -63,27 +63,62 @@ RuleSearch: } // getProccessedInputAndHitValue gets the processed input from the message input and the true/false if it was a successfully hit rule. -func getProccessedInputAndHitValue(messageInput, ruleRespondValue, ruleHearValue, messageReaction, ruleReactionMatch string) (string, bool) { +func getProccessedInputAndHitValue(message models.Message, rule models.Rule) (string, bool) { processedInput, hit := "", false - if ruleRespondValue != "" { + + ruleRespondValue := rule.Respond + ruleHearValue := rule.Hear + + if rule.Respond != "" { + log.Debug().Msgf("Respond Rule %s", rule.Name) + + messageInput := message.Input processedInput, hit = utils.Match(ruleRespondValue, messageInput, true) - } else if ruleHearValue != "" { // Are we listening to everything? + } else if rule.Hear != "" { // Are we listening to everything? + log.Debug().Msgf("HearRule %s", rule.Name) + + messageInput := message.Input _, hit = utils.Match(ruleHearValue, messageInput, false) - } else if ruleReactionMatch != "" { - processedInput, hit = utils.Match(ruleReactionMatch, messageReaction, false) + } else if rule.ReactionsAdded != "" { + log.Debug().Msgf("ReactionAdded Rule %s", rule.Name) + + messageReaction := message.ReactionAdded + processedInput, hit = utils.Match(rule.ReactionsAdded, messageReaction, false) + } else if rule.ReactionsRemoved != "" { + log.Debug().Msgf("ReactionRemoved Rule %s", rule.Name) + + messageReaction := message.ReactionRemoved + processedInput, hit = utils.Match(rule.ReactionsRemoved, messageReaction, false) } return processedInput, hit } +// isChatRule checks that the rule applies to chat. +func isValidChatRule(rule models.Rule) bool { + if rule.Respond != "" { + return true + } + + if rule.Hear != "" { + return true + } + + if rule.ReactionsAdded != "" || rule.ReactionsRemoved != "" { + return true + } + + return false +} + // handleChatServiceRule handles the processing logic for a rule that came from either the chat application or CLI remote. // //nolint:gocyclo // refactor candidate func handleChatServiceRule(outputMsgs chan<- models.Message, message models.Message, hitRule chan<- models.Rule, rule models.Rule, processedInput string, hit bool, bot *models.Bot) (bool, bool) { match, stopSearch := false, false - if rule.Respond != "" || rule.Hear != "" || rule.ReactionMatch != "" { - // You can only use 'respond', 'hear', or 'reaction match' + if isValidChatRule(rule) { + // You can only use 'respond', 'hear', or 'reactions' if rule.Respond != "" && rule.Hear != "" { log.Debug().Msgf("rule %#q has both 'hear' and 'match' or 'respond' defined. please choose one or the other", rule.Name) } @@ -235,7 +270,7 @@ func isValidHitChatRule(message *models.Message, rule models.Rule, processedInpu } // If this wasn't a 'hear' rule, handle the args - if rule.Hear == "" && rule.ReactionMatch == "" { + if rule.Hear == "" && rule.ReactionsAdded == "" { // Get all the args that the message sender supplied args := utils.RuleArgTokenizer(processedInput) diff --git a/core/matcher_test.go b/core/matcher_test.go index 40649811..062c518c 100644 --- a/core/matcher_test.go +++ b/core/matcher_test.go @@ -584,11 +584,13 @@ func TestUpdateReaction(t *testing.T) { func Test_getProccessedInputAndHitValue(t *testing.T) { type args struct { - messageInput string - ruleRespondValue string - ruleHearValue string - messageReaction string - ruleReactionMatch string + messageInput string + ruleRespondValue string + ruleHearValue string + messageReactionAdded string + messageReactionRemoved string + ruleReactionAdded string + ruleReactionRemoved string } tests := []struct { @@ -597,16 +599,31 @@ func Test_getProccessedInputAndHitValue(t *testing.T) { want string want1 bool }{ - {"hit", args{"hello foo", "hello", "hello", "", ""}, "foo", true}, - {"hit no hear value", args{"hello foo", "hello", "", "", ""}, "foo", true}, - {"hit no respond value - drops args", args{"hello foo", "", "hello", "", ""}, "", true}, - {"no match", args{"hello foo", "", "", "", ""}, "", false}, - {"hit reaction", args{"", "", "", ":hello:", ":hello:"}, ":hello:", true}, + {"hit", args{"hello foo", "hello", "hello", "", "", "", ""}, "foo", true}, + {"hit no hear value", args{"hello foo", "hello", "", "", "", "", ""}, "foo", true}, + {"hit no respond value - drops args", args{"hello foo", "", "hello", "", "", "", ""}, "", true}, + {"no match", args{"hello foo", "", "", "", "", "", ""}, "", false}, + {"hit reaction added", args{"", "", "", "hello", "", "hello", ""}, "hello", true}, + {"hit reaction removed", args{"", "", "", "", "hello", "", "hello"}, "hello", true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, got1 := getProccessedInputAndHitValue(tt.args.messageInput, tt.args.ruleRespondValue, tt.args.ruleHearValue, tt.args.messageReaction, tt.args.ruleReactionMatch) + rule := models.Rule{ + Hear: tt.args.ruleHearValue, + Respond: tt.args.ruleRespondValue, + ReactionsAdded: tt.args.ruleReactionAdded, + ReactionsRemoved: tt.args.ruleReactionRemoved, + } + + message := models.Message{ + Input: tt.args.messageInput, + ReactionAdded: tt.args.messageReactionAdded, + ReactionRemoved: tt.args.messageReactionRemoved, + } + + got, got1 := getProccessedInputAndHitValue(message, rule) + if got != tt.want { t.Errorf("getProccessedInputAndHitValue() got = %v, want %v", got, tt.want) } diff --git a/models/message.go b/models/message.go index 3065e0ee..254df4d0 100644 --- a/models/message.go +++ b/models/message.go @@ -17,8 +17,8 @@ type Message struct { ChannelName string Input string Output string - ReactionAction string - Reaction string + ReactionAdded string + ReactionRemoved string Error string Timestamp string ThreadID string diff --git a/models/rule.go b/models/rule.go index 45f39e61..2dfbcc25 100644 --- a/models/rule.go +++ b/models/rule.go @@ -7,7 +7,8 @@ type Rule struct { Name string `mapstructure:"name" binding:"required"` Respond string `mapstructure:"respond" binding:"omitempty"` Hear string `mapstructure:"hear" binding:"omitempty"` - ReactionMatch string `mapstructure:"reaction_match" binding:"omitempty"` + ReactionsAdded string `mapstructure:"reactions_added" binding:"omitempty"` + ReactionsRemoved string `mapstructure:"reactions_removed" binding:"omitempty"` Schedule string `mapstructure:"schedule"` Args []string `mapstructure:"args" binding:"required"` DirectMessageOnly bool `mapstructure:"direct_message_only" binding:"required"` diff --git a/remote/slack/helper.go b/remote/slack/helper.go index b70916a7..36198e6a 100644 --- a/remote/slack/helper.go +++ b/remote/slack/helper.go @@ -449,14 +449,15 @@ func populateMessage(message models.Message, msgType models.MessageType, channel func populateReaction(message models.Message, msgType models.MessageType, channel, action, reaction, timeStamp, link string, user *slack.User, bot *models.Bot) models.Message { switch msgType { case models.MsgTypeDirect, models.MsgTypeChannel, models.MsgTypePrivateChannel: - message.ReactionAction = action - message.Reaction = reaction - message.Input = "" - message.Output = "" - message.Timestamp = timeStamp - message.SourceLink = link - message.Vars["_reaction.action"] = action - message.Vars["_reaction"] = reaction + switch action { + case "added": + message.ReactionAdded = reaction + case "removed": + message.ReactionRemoved = reaction + } + + message.Vars["_reaction.added"] = message.ReactionAdded + message.Vars["_reaction.removed"] = message.ReactionRemoved message = populateMessage(message, msgType, channel, "", timeStamp, "", link, false, user, bot) default: From 49172c1f4ea8350f954fd9b7517ab06e4d6aa170 Mon Sep 17 00:00:00 2001 From: Steven Arnott Date: Fri, 25 Oct 2024 09:55:58 -0400 Subject: [PATCH 5/8] Resolve PR comments --- Makefile | 3 +-- core/matcher.go | 10 +--------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index aec14cda..9ac5ec28 100644 --- a/Makefile +++ b/Makefile @@ -63,8 +63,7 @@ test-coverage: clean: validate @echo "Running $@ tasks" -rm -v ./flottbot* - -## -rm -v ./debug #Not created + -rm -v ./debug #Not created # ┌┐ ┬ ┬┬┬ ┌┬┐ # ├┴┐│ │││ ││ diff --git a/core/matcher.go b/core/matcher.go index c96a9f81..0f2b505e 100644 --- a/core/matcher.go +++ b/core/matcher.go @@ -70,23 +70,15 @@ func getProccessedInputAndHitValue(message models.Message, rule models.Rule) (st ruleHearValue := rule.Hear if rule.Respond != "" { - log.Debug().Msgf("Respond Rule %s", rule.Name) - messageInput := message.Input processedInput, hit = utils.Match(ruleRespondValue, messageInput, true) } else if rule.Hear != "" { // Are we listening to everything? - log.Debug().Msgf("HearRule %s", rule.Name) - messageInput := message.Input _, hit = utils.Match(ruleHearValue, messageInput, false) } else if rule.ReactionsAdded != "" { - log.Debug().Msgf("ReactionAdded Rule %s", rule.Name) - messageReaction := message.ReactionAdded processedInput, hit = utils.Match(rule.ReactionsAdded, messageReaction, false) } else if rule.ReactionsRemoved != "" { - log.Debug().Msgf("ReactionRemoved Rule %s", rule.Name) - messageReaction := message.ReactionRemoved processedInput, hit = utils.Match(rule.ReactionsRemoved, messageReaction, false) } @@ -270,7 +262,7 @@ func isValidHitChatRule(message *models.Message, rule models.Rule, processedInpu } // If this wasn't a 'hear' rule, handle the args - if rule.Hear == "" && rule.ReactionsAdded == "" { + if rule.Hear == "" && rule.ReactionsAdded == "" && rule.ReactionsRemoved == "" { // Get all the args that the message sender supplied args := utils.RuleArgTokenizer(processedInput) From 422d8265bcd0ec7c04ca159d7bc4a08e93e62ead Mon Sep 17 00:00:00 2001 From: Steven Arnott Date: Fri, 25 Oct 2024 11:30:48 -0400 Subject: [PATCH 6/8] Switch logic to if "rule.Respond" type --- core/matcher.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/matcher.go b/core/matcher.go index 0f2b505e..1b10ed7e 100644 --- a/core/matcher.go +++ b/core/matcher.go @@ -261,8 +261,8 @@ func isValidHitChatRule(message *models.Message, rule models.Rule, processedInpu message.Vars[name] = value } - // If this wasn't a 'hear' rule, handle the args - if rule.Hear == "" && rule.ReactionsAdded == "" && rule.ReactionsRemoved == "" { + // If this is a "respond" type, handle args + if rule.Respond != "" { // Get all the args that the message sender supplied args := utils.RuleArgTokenizer(processedInput) From 63542e733452e64d6d6c3063a6d74960ea900868 Mon Sep 17 00:00:00 2001 From: Steven Arnott Date: Fri, 25 Oct 2024 15:33:04 -0400 Subject: [PATCH 7/8] Added check to isValidHitChatRule where a rule needs one of Hear, Respond, ReactionsAdded, or ReactionsRremoved Update matcher_tests --- core/matcher.go | 7 +++++++ core/matcher_test.go | 5 +++++ 2 files changed, 12 insertions(+) diff --git a/core/matcher.go b/core/matcher.go index 1b10ed7e..5d4ccbb6 100644 --- a/core/matcher.go +++ b/core/matcher.go @@ -241,6 +241,13 @@ func handleNoMatch(outputMsgs chan<- models.Message, message models.Message, hit // //nolint:gocyclo // refactor func isValidHitChatRule(message *models.Message, rule models.Rule, processedInput string, bot *models.Bot) bool { + + // Check rule has one of Hear, Respond, ReactionsAdded or ReactionsRemoved + if !isValidChatRule(rule) { + message.Output = "Rule does not have one of Hear, Respond, ReactionsAdded or ReactionsRemoved defined " + return false + } + // Check to honor allow_users or allow_usergroups canRunRule := utils.CanTrigger(message.Vars["_user.name"], message.Vars["_user.id"], rule, bot) if !canRunRule { diff --git a/core/matcher_test.go b/core/matcher_test.go index 062c518c..a35769e6 100644 --- a/core/matcher_test.go +++ b/core/matcher_test.go @@ -645,12 +645,14 @@ func Test_isValidHitChatRule(t *testing.T) { testBot := new(models.Bot) testRule := models.Rule{} + testRule.Hear = "stuff" testMessage := new(models.Message) happyVars := make(map[string]string) happyVars["_user.name"] = "fooUser" testMessage.Vars = happyVars testRuleFail := models.Rule{} + testRuleFail.Hear = "stuff" testRuleFail.AllowUsers = []string{"barUser"} testMessageFail := new(models.Message) failVars := make(map[string]string) @@ -658,6 +660,7 @@ func Test_isValidHitChatRule(t *testing.T) { testMessageFail.Vars = failVars testRuleUserAllowed := models.Rule{} + testRuleUserAllowed.Hear = "stuff" testRuleUserAllowed.AllowUsers = []string{"fooUser"} testMessageUserAllowed := new(models.Message) userAllowedVars := make(map[string]string) @@ -665,6 +668,7 @@ func Test_isValidHitChatRule(t *testing.T) { testMessageUserAllowed.Vars = userAllowedVars testRuleNeedArg := models.Rule{} + testRuleNeedArg.Respond = "stuff" testRuleNeedArg.AllowUsers = []string{"fooUser"} testRuleNeedArg.Args = []string{"arg1", "arg2"} testMessageNeedArg := new(models.Message) @@ -673,6 +677,7 @@ func Test_isValidHitChatRule(t *testing.T) { testMessageNeedArg.Vars = needArgVars testRuleArgs := models.Rule{} + testRuleArgs.Respond = "stuff" testRuleArgs.AllowUsers = []string{"fooUser"} testRuleArgs.Args = []string{"arg1", "arg2"} testMessageArgs := new(models.Message) From 727b67be1fdda19d7c969ef5004d8d91e44d641e Mon Sep 17 00:00:00 2001 From: Steven Arnott Date: Mon, 28 Oct 2024 09:45:24 -0400 Subject: [PATCH 8/8] Keep the linter happy --- core/matcher.go | 1 - 1 file changed, 1 deletion(-) diff --git a/core/matcher.go b/core/matcher.go index 5d4ccbb6..f94ebcde 100644 --- a/core/matcher.go +++ b/core/matcher.go @@ -241,7 +241,6 @@ func handleNoMatch(outputMsgs chan<- models.Message, message models.Message, hit // //nolint:gocyclo // refactor func isValidHitChatRule(message *models.Message, rule models.Rule, processedInput string, bot *models.Bot) bool { - // Check rule has one of Hear, Respond, ReactionsAdded or ReactionsRemoved if !isValidChatRule(rule) { message.Output = "Rule does not have one of Hear, Respond, ReactionsAdded or ReactionsRemoved defined "