From 8b55a310f1753fb8a217ace9ca5a33cfff35d45d Mon Sep 17 00:00:00 2001
From: Tim Abbott <tabbott@zulipchat.com>
Date: Mon, 2 Dec 2019 09:01:31 -0800
Subject: [PATCH] typing: Fix invalid typing notifications for stream messages.

In e42c3f7418dfd67a092fb4c5f5473098e448388e, we made the assumption
that compose_pm_pill.get_recipient() would return no users for stream
messages.  It turns out, due to the confusing name of
compose_state.recipient (which we just renamed to
compose_state.private_message_recipient), this assumption was wrong.

As a result, when composing a stream message using the reply hotkeys,
we'd end up sending typing notiifcations to the person who sent the
message we're replying to as though a PM was being composed.

We fix this by avoiding passing an (expected to be unused) value for
private_message_recipient to compose_state.start.
---
 frontend_tests/node_tests/typing_status.js |  7 +++++
 static/js/compose_actions.js               | 35 ++++++++++++++--------
 2 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/frontend_tests/node_tests/typing_status.js b/frontend_tests/node_tests/typing_status.js
index 4166edcb99..6543a47e49 100644
--- a/frontend_tests/node_tests/typing_status.js
+++ b/frontend_tests/node_tests/typing_status.js
@@ -293,4 +293,11 @@ run_test('basics', () => {
     assert.deepEqual(call_count.maybe_ping_server, 2);
     assert.deepEqual(call_count.start_or_extend_idle_timer, 3);
     assert.deepEqual(call_count.stop_last_notification, 1);
+
+    // Stream messages are represented as get_user_ids_string being empty
+    compose_pm_pill.get_user_ids_string = () => '';
+    typing_status.update(worker, typing.get_recipient());
+    assert.deepEqual(call_count.maybe_ping_server, 2);
+    assert.deepEqual(call_count.start_or_extend_idle_timer, 3);
+    assert.deepEqual(call_count.stop_last_notification, 2);
 });
diff --git a/static/js/compose_actions.js b/static/js/compose_actions.js
index 1b0850cb58..caed5b3003 100644
--- a/static/js/compose_actions.js
+++ b/static/js/compose_actions.js
@@ -227,6 +227,13 @@ exports.start = function (msg_type, opts) {
         clear_box();
     }
 
+    // We set the stream/topic/private_message_recipient
+    // unconditionally here, which assumes the caller will have passed
+    // '' or undefined for these values if they are not appropriate
+    // for this message.
+    //
+    // TODO: Move these into a conditional on message_type, using an
+    // explicit "clear" function for compose_state.
     compose_state.stream_name(opts.stream);
     compose_state.topic(opts.topic);
 
@@ -313,15 +320,23 @@ exports.respond_to_message = function (opts) {
         unread_ops.notify_server_message_read(message);
     }
 
-    let stream = '';
-    let topic = '';
-    if (message.type === "stream") {
-        stream = message.stream;
-        topic = util.get_message_topic(message);
+    // Important note: A reply_type of 'personal' is for the R hotkey
+    // (replying to a message's sender with a private message).  All
+    // other replies can just copy message.type.
+    if (opts.reply_type === 'personal' || message.type === 'private') {
+        msg_type = 'private';
+    } else {
+        msg_type = message.type;
     }
 
-    let pm_recipient = message.reply_to;
-    if (message.type === "private") {
+    let stream = '';
+    let topic = '';
+    let pm_recipient = '';
+    if (msg_type === "stream") {
+        stream = message.stream;
+        topic = util.get_message_topic(message);
+    } else {
+        pm_recipient = message.reply_to;
         if (opts.reply_type === "personal") {
             // reply_to for private messages is everyone involved, so for
             // personals replies we need to set the private message
@@ -331,11 +346,7 @@ exports.respond_to_message = function (opts) {
             pm_recipient = people.pm_reply_to(message);
         }
     }
-    if (opts.reply_type === 'personal' || message.type === 'private') {
-        msg_type = 'private';
-    } else {
-        msg_type = message.type;
-    }
+
     exports.start(msg_type, {stream: stream, topic: topic,
                              private_message_recipient: pm_recipient,
                              trigger: opts.trigger});