zulip
zulip copied to clipboard
zulip
EmbeddedBotHandler: Add react function to bots.
This PR complements the PR #593 in python-zulip-api repository which adds react function to bots.
Welcome @iishiishii! You'll want to clean up your commit history to squash the commits here, and also look into the CI errors.
Check out our GitHub guide and commit guidelines for more details.
@timabbott Yes, I'm rerunning the tests for the errors before committing and will squash them all.
@iishiishii Thanks for working on this and cleaning up your commit message.
We generally frown on directly calling view functions. You can see this in the current EmbettedBotHandler code--for example, look at how send_message calls our internal functions like internal_send_private_message.
You will want to call do_add_reaction from zerver.lib.actions here.
You'll also want to make sure we have automated test coverage for this.
Finally, it's really important that you test this out on a real dev server. We haven't given much love to the embedded-bots system recently, so you may discover issues trying to set that up, but hopefully small ones we can fix.
I guess one of the tricky things here is that we do a lot of stuff in the view function that really belongs more in a library function, such as looking for already-existing reactions, updating the user_message, etc. We probably want a prep commit that extracts most of the view (except for the REQ validation stuff) into something like maybe_do_add_reaction. I'm curious what Tim thinks we should do here.
Chat discussion: https://chat.zulip.org/#narrow/stream/9-issues/topic/.23P566.20ExternalBotHandler.3A.20support.20reactions/near/871627
Heads up @iishiishii, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/master branch and resolve your pull request's merge conflicts accordingly.
