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,13 +108,35 @@ 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.
 
+#### ngrok
+
+The following is an example of using <https://ngrok.com/> to provide webhook forwarding.
+You need to sign up for a free account, and also deal with configuring the GitHub webhook settings.
+
+1. Install ngrok.
+2. Run `ngrok http 8000`. This will forward webhook events to localhost on port 8000.
+3. Configure GitHub webhooks in the test repo you created.
+   In short:
+
+    1. Go to the settings page for your GitHub repo.
+    2. Go to the webhook section.
+    3. Click "Add webhook"
+    4. Include the settings:
+
+        * Payload URL: This is the URL to your Triagebot server, for example http://7e9ea9dc.ngrok.io/github-hook. This URL is displayed when you ran the `ngrok` command above.
+        * Content type: application/json
+        * Secret: Enter a shared secret (some longish random text)
+        * Events: "Send me everything"
+
 ### 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`).
+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 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:
+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" \
@@ -126,37 +148,17 @@ curl http://localhost:8000/zulip-hook \
             "recipient_id": <YOUR_ID>,
             "sender_full_name": "Randolph Carter",
             "sender_email": "[email protected]",
-            "type": "stream"
+            "type": "stream",
+            "stream_id": 1234567,
+            "subject": "Topic subject"
             }
         }'
 ```
 
 Where:
-- `CMD` is the exact command you would issue @triagebot on Zulip (ex. open a direct chat with the
-  bot and send "work show")
+- `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
-- `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.
-You need to sign up for a free account, and also deal with configuring the GitHub webhook settings.
-
-1. Install ngrok.
-2. Run `ngrok http 8000`. This will forward webhook events to localhost on port 8000.
-3. Configure GitHub webhooks in the test repo you created.
-   In short:
-
-    1. Go to the settings page for your GitHub repo.
-    2. Go to the webhook section.
-    3. Click "Add webhook"
-    4. Include the settings:
-
-        * Payload URL: This is the URL to your Triagebot server, for example http://7e9ea9dc.ngrok.io/github-hook. This URL is displayed when you ran the `ngrok` command above.
-        * Content type: application/json
-        * Secret: Enter a shared secret (some longish random text)
-        * Events: "Send me everything"
+- `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
 
 ### Cargo tests
 

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,
             })