From 91e97b18a32fc0ca78c52e99d594c7997f41e5f5 Mon Sep 17 00:00:00 2001 From: David May <1301201+wass3r@users.noreply.github.com> Date: Wed, 21 Jul 2021 21:34:31 +0000 Subject: [PATCH] feat(rule): add limit_to_rooms (#184) --- core/matcher.go | 45 ++++++++++++++++++++++++++++++++++++++++---- core/matcher_test.go | 32 +++++++++++++++++++++++++++++++ models/action.go | 3 ++- models/rule.go | 1 + 4 files changed, 76 insertions(+), 5 deletions(-) diff --git a/core/matcher.go b/core/matcher.go index 54d906cd..acc049a4 100644 --- a/core/matcher.go +++ b/core/matcher.go @@ -70,6 +70,7 @@ func getProccessedInputAndHitValue(messageInput, ruleRespondValue, ruleHearValue } // handleChatServiceRule handles the processing logic for a rule that came from either the chat application or CLI remote +// nolint:gocyclo // mark for refactor 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 != "" { @@ -87,6 +88,35 @@ func handleChatServiceRule(outputMsgs chan<- models.Message, message models.Mess return true, true } + // check if limit_to_rooms is set on the rule + if hit && len(rule.LimitToRooms) > 0 { + bot.Log.Debug().Msgf("rule '%s' has 'limit_to_rooms' set - checking whether message should be processed further", rule.Name) + // do we have a channel name to work with? + if message.ChannelName != "" { + // keep track of whether room is in list + isInLimitToRooms := false + + for _, room := range rule.LimitToRooms { + if strings.EqualFold(room, message.ChannelName) { + // found the room in the list, + // stop looking + isInLimitToRooms = true + break + } + } + + // if the room the message came from is not + // in the list of rooms the rule is limited to + // suppress the response + if !isInLimitToRooms { + bot.Log.Debug().Msgf("rule '%s' was matched but skipped due to message not coming from a room defined in 'limit_to_rooms'", rule.Name) + return true, false + } + } + + bot.Log.Debug().Msgf("rule '%s' has 'limit_to_rooms' set, but the message didn't include the channel name to compare against", rule.Name) + } + // if it's a 'respond' rule, make sure the bot was mentioned if hit && rule.Respond != "" && !message.BotMentioned && message.Type != models.MsgTypeDirect { return match, stopSearch @@ -449,16 +479,23 @@ func handleMessage(action models.Action, outputMsgs chan<- models.Message, msg * } msg.Output = output + + // bridge for deprecation of LimitToRooms on action + if len(action.LimitToRooms) > 0 && len(action.OutputToRooms) == 0 { + bot.Log.Warn().Msgf("'limit_to_rooms' on actions is deprecated and will be removed in the next version - update action '%s' to use `output_to_rooms' instead", action.Name) + action.OutputToRooms = action.LimitToRooms[:] + } + // Send to desired room(s) - if direct && len(action.LimitToRooms) > 0 { // direct=true and limit_to_rooms is specified + if direct && len(action.OutputToRooms) > 0 { // direct=true and limit_to_rooms is specified bot.Log.Debug().Msgf("'direct_message_only' is set - 'limit_to_rooms' field on the '%s' action will be ignored", action.Name) - } else if !direct && len(action.LimitToRooms) > 0 { // direct=false and limit_to_rooms is specified - msg.OutputToRooms = utils.GetRoomIDs(action.LimitToRooms, bot) + } else if !direct && len(action.OutputToRooms) > 0 { // direct=false and limit_to_rooms is specified + msg.OutputToRooms = utils.GetRoomIDs(action.OutputToRooms, bot) if len(msg.OutputToRooms) == 0 { return errors.New("the rooms defined in 'limit_to_rooms' do not exist") } - } else if !direct && len(action.LimitToRooms) == 0 { // direct=false and no limit_to_rooms is specified + } else if !direct && len(action.OutputToRooms) == 0 { // direct=false and no limit_to_rooms is specified msg.OutputToRooms = []string{msg.ChannelID} } // Else: direct=true and no limit_to_rooms is specified diff --git a/core/matcher_test.go b/core/matcher_test.go index d5df5fc6..e9d3e7bd 100644 --- a/core/matcher_test.go +++ b/core/matcher_test.go @@ -699,6 +699,18 @@ func Test_handleChatServiceRule(t *testing.T) { IgnoreThreads: true, } + ruleLimitToRooms := models.Rule{ + Name: "Test Rule with limit_to_rooms set", + Respond: "hello", + LimitToRooms: []string{"foo"}, + } + + ruleSkipDueToLimitToRooms := models.Rule{ + Name: "Test Rule with limit_to_rooms set", + Respond: "hello", + LimitToRooms: []string{"foo"}, + } + testBot := new(models.Bot) testBot.Name = "Testbot" @@ -740,6 +752,24 @@ func Test_handleChatServiceRule(t *testing.T) { ThreadTimestamp: "x", } + testMessageLimitToRooms := models.Message{ + Input: "hello", + Vars: map[string]string{}, + Timestamp: "x", + ThreadTimestamp: "x", + BotMentioned: true, + ChannelName: "foo", + } + + testMessageSkipDueToLimitToRooms := models.Message{ + Input: "hello", + Vars: map[string]string{}, + Timestamp: "x", + ThreadTimestamp: "x", + BotMentioned: true, + ChannelName: "bar", + } + tests := []struct { name string args args @@ -759,6 +789,8 @@ func Test_handleChatServiceRule(t *testing.T) { {"respond rule - hit true - valid vargs", args{rule: ruleVarg, hit: true, bot: testBot, message: testMessageVargs, processedInput: "arg1 arg2 arg3 arg4"}, true, true, "", map[string]string{"arg1": "arg1", "argv": "arg2 arg3 arg4"}}, {"respond rule - hit true - invalid", args{rule: rule, hit: true, bot: testBot, message: testMessage}, true, true, "you might be missing an argument or two - this is what i'm looking for\n```foo ```", map[string]string{}}, {"hear rule - ignore thread", args{rule: ruleIgnoreThread, hit: true, bot: testBot, message: testMessageIgnoreThread}, true, true, "", map[string]string{}}, + {"respond rule - limit_to_rooms set", args{rule: ruleLimitToRooms, hit: true, bot: testBot, message: testMessageLimitToRooms}, true, true, "", map[string]string{}}, + {"respond rule - skip due to limit_to_rooms", args{rule: ruleSkipDueToLimitToRooms, hit: true, bot: testBot, message: testMessageSkipDueToLimitToRooms}, true, false, "", map[string]string{}}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/models/action.go b/models/action.go index d81cdc34..b80b7db4 100644 --- a/models/action.go +++ b/models/action.go @@ -12,7 +12,8 @@ type Action struct { Auth []Auth `mapstructure:"auth"` ExposeJSONFields map[string]string `mapstructure:"expose_json_fields"` Response string `mapstructure:"response"` - LimitToRooms []string `mapstructure:"limit_to_rooms"` + LimitToRooms []string `mapstructure:"limit_to_rooms"` // deprecated + OutputToRooms []string `mapstructure:"output_to_rooms"` Message string `mapstructure:"message"` Reaction string `mapstructure:"update_reaction" binding:"omitempty"` } diff --git a/models/rule.go b/models/rule.go index dc556e99..6757950d 100644 --- a/models/rule.go +++ b/models/rule.go @@ -25,6 +25,7 @@ type Rule struct { Actions []Action `mapstructure:"actions" binding:"required"` Remotes Remotes `mapstructure:"remotes" binding:"omitempty"` Reaction string `mapstructure:"reaction" binding:"omitempty"` + LimitToRooms []string `mapstructure:"limit_to_rooms" binding:"omitempty"` // The following fields are not included in rule file RemoveReaction string }