paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-04 22:13:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

typing: Fix invalid typing notifications for stream messages.

Browse Source 
In e42c3f7418, 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.
This commit is contained in:
Tim Abbott
2019-12-02 09:01:31 -08:00
parent ea7c6d395f
commit 8b55a310f1
2 changed files with 30 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
frontend_tests/node_tests/typing_status.js
View File

@@ -293,4 +293,11 @@ run_test('basics', () => {
assert.deepEqual(call_count.maybe_ping_server, 2); assert.deepEqual(call_count.maybe_ping_server, 2);
assert.deepEqual(call_count.start_or_extend_idle_timer, 3); assert.deepEqual(call_count.start_or_extend_idle_timer, 3);
assert.deepEqual(call_count.stop_last_notification, 1); 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);
}); });

35
static/js/compose_actions.js
View File

@@ -227,6 +227,13 @@ exports.start = function (msg_type, opts) {
clear_box(); 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.stream_name(opts.stream);
compose_state.topic(opts.topic); compose_state.topic(opts.topic);
@@ -313,15 +320,23 @@ exports.respond_to_message = function (opts) {
unread_ops.notify_server_message_read(message); unread_ops.notify_server_message_read(message);
} }
let stream = ''; // Important note: A reply_type of 'personal' is for the R hotkey
let topic = ''; // (replying to a message's sender with a private message). All
if (message.type === "stream") { // other replies can just copy message.type.
stream = message.stream; if (opts.reply_type === 'personal' || message.type === 'private') {
topic = util.get_message_topic(message); msg_type = 'private';
} else {
msg_type = message.type;
} }
let pm_recipient = message.reply_to; let stream = '';
if (message.type === "private") { 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") { if (opts.reply_type === "personal") {
// reply_to for private messages is everyone involved, so for // reply_to for private messages is everyone involved, so for
// personals replies we need to set the private message // 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); 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, exports.start(msg_type, {stream: stream, topic: topic,
private_message_recipient: pm_recipient, private_message_recipient: pm_recipient,
trigger: opts.trigger}); trigger: opts.trigger});