Zulip stream type check

Author: apiraino

README.md

@@ -82,7 +82,7 @@ To use Postgres, you will need to install it and configure it:
 I recommend at least skimming the [GitHub webhook documentation](https://docs.github.com/en/developers/webhooks-and-events/webhooks/about-webhooks) if you are not familiar with webhooks.
 In order for GitHub's webhooks to reach your triagebot server, you'll need to figure out some way to route them to your machine.
 There are various options on how to do this.
-You can poke holes into your firewall or use a proxy, but you shouldn't expose your machine to the the internet.
+You can poke holes into your firewall or use a proxy, but you shouldn't expose your machine to the internet.
 There are various services which help with this problem.
 These generally involve running a program on your machine that connects to an external server which relays the hooks into your machine.
 There are several to choose from:
@@ -108,36 +108,6 @@ gh webhook forward --repo=ehuss/triagebot-test --events=* \
 
 Where the value in `--secret` is the secret value you place in `GITHUB_WEBHOOK_SECRET` in the `.env` file, and `--repo` is the repo you want to test against.
 
-### Zulip testing
-
-If you are modifying code that sends message to Zulip and want to test your changes, you can register a [new free Zulip instance](https://zulip.com/new/). Before launching the triagebot locally, set the Zulip env vars to connect to your test instance (see example in `.env.sample`).
-
-You can also test Zulip webhooks locally with `curl`. For example, to test the Zulip hooks (commands sent to the
-Triagebot from the Rust lang Zulip), you start the triagebot on `localhost:8000` and then simulate a
-Zulip hook payload:
-``` sh
-curl http://localhost:8000/zulip-hook \
-    -H "Content-Type: application/json" \
-    -d '{
-        "data": "<CMD>",
-        "token": "<ZULIP_WEBHOOK_SECRET>",
-        "message": {
-            "sender_id": <YOUR_ID>,
-            "recipient_id": <YOUR_ID>,
-            "sender_full_name": "Randolph Carter",
-            "sender_email": "[email protected]",
-            "type": "stream"
-            }
-        }'
-```
-
-Where:
-- `CMD` is the exact command you would issue @triagebot on Zulip (ex. open a direct chat with the
-  bot and send "work show")
-- `ZULIP_WEBHOOK_SECRET`: can be anything. Must correspond to the env var `$ZULIP_WEBHOOK_SECRET` on your workstation
-- `YOUR_ID`: your GitHub user ID. Must be existing in your local triagebot database (table `users` and as
-  foreign key also in `review_prefs`)
-
 #### ngrok
 
 The following is an example of using <https://ngrok.com/> to provide webhook forwarding.
@@ -158,6 +128,39 @@ You need to sign up for a free account, and also deal with configuring the GitHu
         * Secret: Enter a shared secret (some longish random text)
         * Events: "Send me everything"
 
+### Zulip testing
+
+If you want a test Zulip instance, you can [register a free one](https://zulip.com/new/). To have your local triagebot talk to this Zulip instance you need to:
+- Configure a (webhook forwarding service)[#configure-webhook-forwarding]
+- Create in your Zulip instance a webhook bot, using the forwarding address created before.
+- Launch your local triagebot setting `ZULIP_WEBHOOK_SECRET` to the webhook bot `key` value (you get that as part of the Zulip webhook bot configuration)
+- Set the other Zulip env vars as needed (see example in `.env.sample`).
+
+You can also simulate a Zulip webhook payload with `cURL`. For example, this is the payload sent to the triagebot server when tagging a Zulip bot in a stream.
+``` sh
+curl http://localhost:8000/zulip-hook \
+    -H "Content-Type: application/json" \
+    -d '{
+        "data": "<CMD>",
+        "token": "<ZULIP_WEBHOOK_SECRET>",
+        "message": {
+            "sender_id": <YOUR_ZULIP_ID>,
+            "recipient_id": <BOT_ZULIP_ID>,
+            "sender_full_name": "Randolph Carter",
+            "sender_email": "[email protected]",
+            "type": "stream",
+            "stream_id": 1234567,
+            "subject": "Topic subject"
+            }
+        }'
+```
+
+Where:
+- `CMD` is the full command you would issue on Zulip (ex. `@**triagebot** work show`)
+- `ZULIP_WEBHOOK_SECRET`: can be anything. Must correspond to the env var `$ZULIP_WEBHOOK_SECRET` on your workstation
+- `sender_*`: the Zulip user data sending the message. `sender_id` must be mapped to a GitHub user in this mapping: https://team-api.infra.rust-lang.org/v1/zulip-map.json
+- `recipient_id`: Zulip ID of the recipient of the message (in this case the Zulip bot)
+
 ### Cargo tests
 
 You can run Cargo tests using `cargo test`. If you also want to run tests that access a Postgres database, you can specify an environment variables `TEST_DB_URL`, which should contain a connection string pointing to a running Postgres database instance:

src/zulip.rs

@@ -52,15 +52,14 @@ struct Message {
     sender_email: String,
     /// The ID of the stream.
     ///
-    /// `None` if it is a private message.
+    /// `None` if it is a direct message.
     stream_id: Option<u64>,
     /// The topic of the incoming message. Not the stream name.
     ///
-    /// Not currently set for private messages (though Zulip may change this in
-    /// the future if it adds topics to private messages).
+    /// Not currently set for direct messages (though Zulip may change this in
+    /// the future if it adds topics to direct messages).
     subject: Option<String>,
-    /// The type of the message: stream or private.
-    #[allow(unused)]
+    /// The type of the message: stream or direct.
     #[serde(rename = "type")]
     type_: String,
 }
@@ -76,7 +75,7 @@ impl Message {
                     .as_ref()
                     .expect("stream messages should have a topic"),
             },
-            None => Recipient::Private {
+            None => Recipient::Direct {
                 id: self.sender_id,
                 email: &self.sender_email,
             },
@@ -177,8 +176,9 @@ async fn handle_command<'a>(
     log::trace!("handling zulip command {:?}", command);
     let mut words: Vec<&str> = command.split_whitespace().collect();
 
-    // Missing stream means that this is a direct message
-    if message_data.stream_id.is_none() {
+    // Is this a direct or a stream message?
+    // See: https://zulip.com/api/send-message#parameter-type
+    if message_data.type_ == "direct" {
         // Handle impersonation
         let mut impersonated = false;
         if let Some(&"as") = words.get(0) {
@@ -253,7 +253,7 @@ async fn handle_command<'a>(
             );
 
             MessageApiRequest {
-                recipient: Recipient::Private {
+                recipient: Recipient::Direct {
                     id: user.user_id,
                     email: &user.email,
                 },
@@ -965,11 +965,11 @@ async fn post_waiter(
         recipient: Recipient::Stream {
             id: message
                 .stream_id
-                .ok_or_else(|| format_err!("private waiting not supported, missing stream id"))?,
+                .ok_or_else(|| format_err!("direct waiting not supported, missing stream id"))?,
             topic: message
                 .subject
                 .as_deref()
-                .ok_or_else(|| format_err!("private waiting not supported, missing topic"))?,
+                .ok_or_else(|| format_err!("direct waiting not supported, missing topic"))?,
         },
         content: waiting.primary,
     }

src/zulip/api.rs

@@ -47,7 +47,7 @@ pub(crate) enum Recipient<'a> {
         id: u64,
         topic: &'a str,
     },
-    Private {
+    Direct {
         #[serde(skip)]
         id: u64,
         #[serde(rename = "to")]
@@ -81,7 +81,7 @@ impl Recipient<'_> {
                 }
                 format!("stream/{}-xxx/topic/{}", id, encoded_topic)
             }
-            Recipient::Private { id, .. } => format!("pm-with/{}-xxx", id),
+            Recipient::Direct { id, .. } => format!("pm-with/{}-xxx", id),
         }
     }
 

src/zulip/client.rs

@@ -70,15 +70,15 @@ impl ZulipClient {
             .form(&SerializedApi {
                 type_: match recipient {
                     Recipient::Stream { .. } => "stream",
-                    Recipient::Private { .. } => "private",
+                    Recipient::Direct { .. } => "direct",
                 },
                 to: match recipient {
                     Recipient::Stream { id, .. } => id.to_string(),
-                    Recipient::Private { email, .. } => email.to_string(),
+                    Recipient::Direct { email, .. } => email.to_string(),
                 },
                 topic: match recipient {
                     Recipient::Stream { topic, .. } => Some(topic),
-                    Recipient::Private { .. } => None,
+                    Recipient::Direct { .. } => None,
                 },
                 content,
             })