Allow outgoing webhook to add reaction when sending a response without additional calls to the REST API.

[PIG208]

Fixes: #12811

Testing plan:
It has been tested both manually and with automated tests.

GIFs or screenshots:
This introduces a new format for response messages for outgoing webhooks:

{
  "reactions": [
    {"emoji_name": "wave"}
  ]
}

It also works with content:

{
  "content": "test",
  "reactions": [
    {"emoji_name": "wave"}
  ]
}

Responding with two emoji reactions along with a message:

When the emoji_name field is missing in a specified emoji reaction:

Updated documentation for the API:

Error messages;

[zulipbot]

Hello @zulip/server-bots members, this pull request was labeled with the "area: bots" label, so you may want to check it out!

[PIG208]

While working on the PR, I found that if the response message is not JSON, it will send an error message after the message that triggers it, but it doesn't notify the user. This might be inconsistent with our error handling logic for outgoing webhooks. Prove me wrong if it's intended this way.

The following is the code responsible for this behavior. We might just want to raise a JsonableError here.

try:
    response_json = json.loads(response.text)
except json.JSONDecodeError:
    fail_with_message(event, "Invalid JSON in response")
    return

[timabbott]

Good find; I think we ended up doing just that in b7b1ec0.

[timabbott]

A small comment on commit messages: You should only put Fixes: in one commit. Also, documentation changes generally belong in the commit that changes reality to match that documentation; so we should squash the documentation commit into the previous one.

[PIG208]

Thank you for the feedback!

[timabbott]

I merged the first commit as 9c0ce19, after moving check_add_reaction after do_add_reaction, since generally it's clearer to have functions call functions above them in the file.

I pushed the second commit back here after a few small documentation/comment changes. It looks correct, but I'd like tests that actually verify the full set of parameters passed to check_add_reaction -- one can imagine some future refactoring bug where we mess up the plumbing of parameters, or something, and so it's worth having a test. (Probably in test_outgoing_webhook_system.py?)

[PIG208]

It looks like that I have broken something with the rebase.

[PIG208]

Regarding the access_message issue that I mentioned in #17927, I have added a commit here to fix that. The service bots will be able to access private messages that they have sent or received.

[PIG208]

I've rebased this due to a recent change. I'm not sure if a service bot is the only case when a user doesn't have a UserMessage row. It is possible that we can remove the check below?

if (
   user_profile.bot_type == UserProfile.EMBEDDED_BOT
   or user_profile.bot_type == UserProfile.OUTGOING_WEBHOOK_BOT
):

[eeshangarg]

Hey @PIG208! This looks great, thanks for working on this! I just left a couple of minor comments.
As always, please feel free to reach out if you have any questions. Thanks! :)

[eeshangarg]

@PIG208 I wonder why the build is failing here. Could you please investigate?

[PIG208]

@PIG208 I wonder why the build is failing here. Could you please investigate?

I have rerun the tests several times and I think this might be a nondeterministic error? Will rerun it again.

[zulipbot]

Heads up @PIG208, 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/main branch and resolve your pull request's merge conflicts accordingly.