Add ability to respond to a message reaction being added or remove

[ArcticSnowman]

Proposed change

I worked on a slack chat bot that was based on https://github.com/ArcticSnowman/go-slackbot which I added the ability to respond to emojis (reaction).

So this is my first attempt to add reaction support to this bot framework. Hope this helps with #45

Types of changes

What types of changes is this pull request introducing to flottbot? Put an x in the boxes that apply

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

You can fill this out after creating your PR. Put an x in the boxes that apply

• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)

[coveralls]

Pull Request Test Coverage Report for Build 11524319912

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 0.0%

---

Totals
Change from base Build 10426782526:	0.0%
Covered Lines:	0
Relevant Lines:	0

---

💛 - Coveralls

[wass3r]

thanks for the contribution! with a couple of caveats it works as advertised (see comments).

some further tweaks that would be interesting to explore are (and this definitely doesn't have to be in this PR):

1. separate the action (added/removed) so you can target either
2. check the target thread to see whether we already replied and, at least by default, don't post again

[wass3r]

did this exact rule work for you?

asking because, 1) i can only make it work without the surrounding : and 2) only if the value doesn't contain +, like heavy_plus_sign works without problem.

the issue is that we send the the string to regex in the matcher where + has special meaning, and the :s aren't part of the reaction value returned by the Slack API - at least in my test workspace.

if you throw :+1: as instead of :hello: in the test you added you will notice that it fails or maybe you saw that too.

Suggested change

	reaction_match: ":+1:"
	reaction_match: "heavy_plus_sign"

i think to make +1 work we will need to make a few more tweaks and maybe bypass regex to do an exact match. we can do it in a follow-up PR.

[ArcticSnowman]

I've updated the example and fixed some issue I found. I decided that we need to treat addition and removals of reactions are separate in Rule and Message model so action can be triggered for one or other or both.

[wass3r]

works as advertised. thanks for the tweaks. couple of small things and we're good i think.

i noticed you used plural reactions_added vs reaction_added. was the thought to be able to support matching multiple reactions in one rule at some point? if so, i think i like that. some workspaces have multiple emojis that are similar, eg. :question: and :grey_question:. nothing that has to be fleshed out in this PR though. just curious.

thank you!

[ArcticSnowman]

Should

	if rule.Hear == "" && rule.ReactionsAdded == "" && rule.ReactionsRemoved == "" {

not be better as

	if rule.Respond != "" {

[wass3r]

Should

	if rule.Hear == "" && rule.ReactionsAdded == "" && rule.ReactionsRemoved == "" {

not be better as

	if rule.Respond != "" {

makes a lot of sense, i like it! let's do that. if you don't mind changing the comment above it as well 🥺 thank you!

[ArcticSnowman]

@wass3r - I went with plurals as the use case in the other bot library I use, does take multiple emoji triggers.

For this framework, the user would need to craft a regexp string to match multiple.

For example:

(plus_one|plusone|plus1)

[ArcticSnowman]

PR for docs target/flottbot-docs#60

[wass3r]

looks like this addition is making the build and validate steps fail

[wass3r]

for the failing test (https://github.com/target/flottbot/actions/runs/11521113211/job/32081108474?pr=520#step:5:252), it looks like the improvement you made exposed a flaw in a testcase. in this block

flottbot/core/matcher_test.go

Lines 647 to 653 in dc4cfeb

	testRuleNeedArg := models.Rule{}
	testRuleNeedArg.AllowUsers = []string{"fooUser"}
	testRuleNeedArg.Args = []string{"arg1", "arg2"}
	testMessageNeedArg := new(models.Message)
	needArgVars := make(map[string]string)
	needArgVars["_user.name"] = "fooUser"
	testMessageNeedArg.Vars = needArgVars

we will need to add testRuleNeedArg.Respond = "foo" (for example)

[ArcticSnowman]

@wass3r - Yeah those unit tests need an explicit Hear/Respond added. I've clean up the test and added a check to the function that requires one of Hear, Respond, ReactionsAdded, or ReactionsRemoved. to have a value to be valid.

[wass3r]

i think we're one commit away.. linter is (harshly) complaining about this line

flottbot/core/matcher.go

Line 244 in 63542e7

. happy to merge after that tiny edit. thanks for all the work!