diff --git a/web/shared/src/typing_status.js.flow b/web/shared/src/typing_status.js.flow index 82d1a11f7b..792362d4cd 100644 --- a/web/shared/src/typing_status.js.flow +++ b/web/shared/src/typing_status.js.flow @@ -5,14 +5,15 @@ "use strict"; type RecipientUserIds = $ReadOnlyArray; +type StreamTopicObject = {|stream_id: number, topic: string|}; type Worker = {| get_current_time: () => number, // as ms since epoch - notify_server_start: (RecipientUserIds) => void, - notify_server_stop: (RecipientUserIds) => void, + notify_server_start: (RecipientUserIds | StreamTopicObject) => void, + notify_server_stop: (RecipientUserIds | StreamTopicObject) => void, |}; declare export function update( worker: Worker, - new_recipient: RecipientUserIds | null, + new_recipient: RecipientUserIds | StreamTopicObject | null, ): void; diff --git a/web/shared/src/typing_status.ts b/web/shared/src/typing_status.ts index 19635a10aa..2492822fcd 100644 --- a/web/shared/src/typing_status.ts +++ b/web/shared/src/typing_status.ts @@ -1,18 +1,56 @@ import _ from "lodash"; import assert from "minimalistic-assert"; +type StreamTopic = { + stream_id: number; + topic: string; +}; +type Recipient = number[] | StreamTopic; type TypingStatusWorker = { get_current_time: () => number; - notify_server_start: (recipient_ids: number[]) => void; - notify_server_stop: (recipient_ids: number[]) => void; + notify_server_start: (recipient: Recipient) => void; + notify_server_stop: (recipient: Recipient) => void; }; type TypingStatusState = { - current_recipient_ids: number[]; + current_recipient: Recipient; next_send_start_time: number; idle_timer: ReturnType; }; +function message_type(recipient: Recipient): "direct" | "stream" { + if (Array.isArray(recipient)) { + return "direct"; + } + return "stream"; +} + +function lower_same(a: string, b: string): boolean { + return a.toLowerCase() === b.toLowerCase(); +} + +function same_stream_and_topic(a: StreamTopic, b: StreamTopic): boolean { + // Streams and topics are case-insensitive. + return a.stream_id === b.stream_id && lower_same(a.topic, b.topic); +} + +function same_recipient(a: Recipient | null, b: Recipient | null): boolean { + if (a === null || b === null) { + return false; + } + + if (message_type(a) === "direct" && message_type(b) === "direct") { + return _.isEqual(a, b); + } else if (message_type(a) === "stream" && message_type(b) === "stream") { + // type assertions to avoid linter error + const aStreamTopic = a as StreamTopic; + const bStreamTopic = b as StreamTopic; + return same_stream_and_topic(aStreamTopic, bStreamTopic); + } + + return false; +} + /** Exported only for tests. */ export let state: TypingStatusState | null = null; @@ -20,7 +58,7 @@ export let state: TypingStatusState | null = null; export function stop_last_notification(worker: TypingStatusWorker): void { assert(state !== null, "State object should not be null here."); clearTimeout(state.idle_timer); - worker.notify_server_stop(state.current_recipient_ids); + worker.notify_server_stop(state.current_recipient); state = null; } @@ -50,24 +88,24 @@ function set_next_start_time(current_time: number, typing_started_wait_period: n function actually_ping_server( worker: TypingStatusWorker, - recipient_ids: number[], + recipient: Recipient, current_time: number, typing_started_wait_period: number, ): void { - worker.notify_server_start(recipient_ids); + worker.notify_server_start(recipient); set_next_start_time(current_time, typing_started_wait_period); } /** Exported only for tests. */ export function maybe_ping_server( worker: TypingStatusWorker, - recipient_ids: number[], + recipient: Recipient, typing_started_wait_period: number, ): void { assert(state !== null, "State object should not be null here."); const current_time = worker.get_current_time(); if (current_time > state.next_send_start_time) { - actually_ping_server(worker, recipient_ids, current_time, typing_started_wait_period); + actually_ping_server(worker, recipient, current_time, typing_started_wait_period); } } @@ -79,11 +117,7 @@ export function maybe_ping_server( * rate, and keeps a timer to send a "stopped typing" notice when the user * hasn't typed for a few seconds. * - * Zulip supports typing notifications only for both 1:1 and group direct messages; - * so composing a stream message should be treated like composing no message at - * all. - * - * Call with `new_recipient_ids` of `null` when the user actively stops + * Call with `new_recipient` as `null` when the user actively stops * composing a message. If the user switches from one set of recipients to * another, there's no need to call with `null` in between; the * implementation tracks the change and behaves appropriately. @@ -93,23 +127,23 @@ export function maybe_ping_server( * * @param {*} worker Callbacks for reaching the real world. See typing.js * for implementations. - * @param {*} new_recipient_ids The users the direct message being composed is - * addressed to, as a sorted array of user IDs; or `null` if no direct message - * is being composed anymore. + * @param {*} new_recipient Depends on type of message being composed. If + * * Direct message: The users the DM being composed is addressed to, + * as a sorted array of user IDs + * * Stream message: An Object containing the stream_id and topic + * * No message is being composed: `null` */ export function update( worker: TypingStatusWorker, - new_recipient_ids: number[] | null, + new_recipient: Recipient | null, typing_started_wait_period: number, typing_stopped_wait_period: number, ): void { if (state !== null) { - // We need to use _.isEqual for comparisons; === doesn't work - // on arrays. - if (_.isEqual(new_recipient_ids, state.current_recipient_ids)) { + if (same_recipient(new_recipient, state.current_recipient)) { // Nothing has really changed, except we may need // to send a ping to the server. - maybe_ping_server(worker, new_recipient_ids!, typing_started_wait_period); + maybe_ping_server(worker, new_recipient!, typing_started_wait_period); // We can also extend out our idle time. state.idle_timer = start_or_extend_idle_timer(worker, typing_stopped_wait_period); @@ -123,7 +157,7 @@ export function update( stop_last_notification(worker); } - if (new_recipient_ids === null) { + if (new_recipient === null) { // If we are not talking to somebody we care about, // then there is no more action to take. return; @@ -132,10 +166,10 @@ export function update( // We just started talking to these recipients, so notify // the server. state = { - current_recipient_ids: new_recipient_ids, + current_recipient: new_recipient, next_send_start_time: 0, idle_timer: start_or_extend_idle_timer(worker, typing_stopped_wait_period), }; const current_time = worker.get_current_time(); - actually_ping_server(worker, new_recipient_ids, current_time, typing_started_wait_period); + actually_ping_server(worker, new_recipient, current_time, typing_started_wait_period); } diff --git a/web/src/typing.js b/web/src/typing.js index 5c44684649..4eee60a07f 100644 --- a/web/src/typing.js +++ b/web/src/typing.js @@ -8,6 +8,7 @@ import * as compose_pm_pill from "./compose_pm_pill"; import * as compose_state from "./compose_state"; import {page_params} from "./page_params"; import * as people from "./people"; +import * as stream_data from "./stream_data"; import {user_settings} from "./user_settings"; // This module handles the outbound side of typing indicators. @@ -22,13 +23,10 @@ const typing_started_wait_period = page_params.server_typing_started_wait_period // do we send a 'stop' notification. const typing_stopped_wait_period = page_params.server_typing_stopped_wait_period_milliseconds; -function send_typing_notification_ajax(user_ids_array, operation) { +function send_typing_notification_ajax(data) { channel.post({ url: "/json/typing", - data: { - to: JSON.stringify(user_ids_array), - op: operation, - }, + data, success() {}, error(xhr) { if (xhr.readyState !== 0) { @@ -38,6 +36,33 @@ function send_typing_notification_ajax(user_ids_array, operation) { }); } +function send_direct_message_typing_notification(user_ids_array, operation) { + const data = { + to: JSON.stringify(user_ids_array), + op: operation, + }; + send_typing_notification_ajax(data); +} + +function send_stream_typing_notification(stream_id, topic, operation) { + const data = { + type: "stream", + to: JSON.stringify(stream_id), + topic, + op: operation, + }; + send_typing_notification_ajax(data); +} + +function send_typing_notification_based_on_message_type(to, operation) { + const message_type = to.stream_id ? "stream" : "direct"; + if (message_type === "direct" && user_settings.send_private_typing_notifications) { + send_direct_message_typing_notification(to, operation); + } else if (message_type === "stream" && user_settings.send_stream_typing_notifications) { + send_stream_typing_notification(to.stream_id, to.topic, operation); + } +} + function get_user_ids_array() { const user_ids_string = compose_pm_pill.get_user_ids_string(); if (user_ids_string === "") { @@ -60,19 +85,27 @@ function get_current_time() { return Date.now(); } -function notify_server_start(user_ids_array) { - if (user_settings.send_private_typing_notifications) { - send_typing_notification_ajax(user_ids_array, "start"); - } +function notify_server_start(to) { + send_typing_notification_based_on_message_type(to, "start"); } -function notify_server_stop(user_ids_array) { - if (user_settings.send_private_typing_notifications) { - send_typing_notification_ajax(user_ids_array, "stop"); - } +function notify_server_stop(to) { + send_typing_notification_based_on_message_type(to, "stop"); } -export const get_recipient = get_user_ids_array; +export function get_recipient() { + const message_type = compose_state.get_message_type(); + if (message_type === "private") { + return get_user_ids_array(); + } + if (message_type === "stream") { + const stream_name = compose_state.stream_name(); + const stream_id = stream_data.get_stream_id(stream_name); + const topic = compose_state.topic(); + return {stream_id, topic}; + } + return null; +} export function initialize() { const worker = { diff --git a/web/tests/typing_status.test.js b/web/tests/typing_status.test.js index fb07a19b3c..0bd2bf051b 100644 --- a/web/tests/typing_status.test.js +++ b/web/tests/typing_status.test.js @@ -6,6 +6,8 @@ const {mock_esm, set_global, zrequire} = require("./lib/namespace"); const {run_test} = require("./lib/test"); const compose_pm_pill = mock_esm("../src/compose_pm_pill"); +const compose_state = mock_esm("../src/compose_state"); +const stream_data = mock_esm("../src/stream_data"); const typing = zrequire("typing"); const typing_status = zrequire("../shared/src/typing_status"); @@ -85,7 +87,7 @@ run_test("basics", ({override, override_rewire}) => { assert.deepEqual(typing_status.state, { next_send_start_time: make_time(5 + 10), idle_timer: "idle_timer_stub", - current_recipient_ids: [1, 2], + current_recipient: [1, 2], }); assert.deepEqual(events, { idle_callback: events.idle_callback, @@ -101,7 +103,7 @@ run_test("basics", ({override, override_rewire}) => { assert.deepEqual(typing_status.state, { next_send_start_time: make_time(5 + 10), idle_timer: "idle_timer_stub", - current_recipient_ids: [1, 2], + current_recipient: [1, 2], }); assert.deepEqual(events, { idle_callback: events.idle_callback, @@ -118,7 +120,7 @@ run_test("basics", ({override, override_rewire}) => { assert.deepEqual(typing_status.state, { next_send_start_time: make_time(18 + 10), idle_timer: "idle_timer_stub", - current_recipient_ids: [1, 2], + current_recipient: [1, 2], }); assert.deepEqual(events, { idle_callback: events.idle_callback, @@ -155,7 +157,7 @@ run_test("basics", ({override, override_rewire}) => { assert.deepEqual(typing_status.state, { next_send_start_time: make_time(50 + 10), idle_timer: "idle_timer_stub", - current_recipient_ids: [1, 2], + current_recipient: [1, 2], }); assert.deepEqual(events, { idle_callback: events.idle_callback, @@ -181,7 +183,7 @@ run_test("basics", ({override, override_rewire}) => { assert.deepEqual(typing_status.state, { next_send_start_time: make_time(80 + 10), idle_timer: "idle_timer_stub", - current_recipient_ids: [1, 2], + current_recipient: [1, 2], }); assert.deepEqual(events, { idle_callback: events.idle_callback, @@ -217,7 +219,7 @@ run_test("basics", ({override, override_rewire}) => { assert.deepEqual(typing_status.state, { next_send_start_time: make_time(170 + 10), idle_timer: "idle_timer_stub", - current_recipient_ids: [1, 2], + current_recipient: [1, 2], }); assert.deepEqual(events, { idle_callback: events.idle_callback, @@ -239,7 +241,7 @@ run_test("basics", ({override, override_rewire}) => { assert.deepEqual(typing_status.state, { next_send_start_time: make_time(171 + 10), idle_timer: "idle_timer_stub", - current_recipient_ids: [3, 4], + current_recipient: [3, 4], }); assert.deepEqual(events, { idle_callback: events.idle_callback, @@ -250,10 +252,11 @@ run_test("basics", ({override, override_rewire}) => { assert.ok(events.idle_callback); // test that we correctly detect if worker.get_recipient - // and typing_status.state.current_recipient_ids are the same + // and typing_status.state.current_recipient are the same override(compose_pm_pill, "get_user_ids_string", () => "1,2,3"); - typing_status.state.current_recipient_ids = typing.get_recipient(); + override(compose_state, "get_message_type", () => "private"); + typing_status.state.current_recipient = typing.get_recipient(); const call_count = { maybe_ping_server: 0, @@ -269,7 +272,7 @@ run_test("basics", ({override, override_rewire}) => { }); } - // User ids of people in compose narrow doesn't change and is same as stat.current_recipient_ids + // User ids of people in compose narrow doesn't change and is same as state.current_recipient // so counts of function should increase except stop_last_notification typing_status.update( worker, @@ -304,8 +307,11 @@ run_test("basics", ({override, override_rewire}) => { 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 - override(compose_pm_pill, "get_user_ids_string", () => ""); + // Stream messages + override(compose_state, "get_message_type", () => "stream"); + override(compose_state, "stream_name", () => "Verona"); + override(stream_data, "get_stream_id", () => "2"); + override(compose_state, "topic", () => "test topic"); typing_status.update( worker, typing.get_recipient(), @@ -313,6 +319,100 @@ run_test("basics", ({override, override_rewire}) => { TYPING_STOPPED_WAIT_PERIOD, ); 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, 4); assert.deepEqual(call_count.stop_last_notification, 2); }); + +run_test("stream_messages", ({override_rewire}) => { + override_rewire(typing_status, "state", null); + + let worker = {}; + const events = {}; + + function set_timeout(f, delay) { + assert.equal(delay, 5000); + events.idle_callback = f; + return "idle_timer_stub"; + } + + function clear_timeout() { + events.timer_cleared = true; + } + + set_global("setTimeout", set_timeout); + set_global("clearTimeout", clear_timeout); + + function notify_server_start(recipient) { + assert.deepStrictEqual(recipient, {stream_id: 3, topic: "test"}); + events.started = true; + } + + function notify_server_stop(recipient) { + assert.deepStrictEqual(recipient, {stream_id: 3, topic: "test"}); + events.stopped = true; + } + + function clear_events() { + events.idle_callback = undefined; + events.started = false; + events.stopped = false; + events.timer_cleared = false; + } + + function call_handler(new_recipient) { + clear_events(); + typing_status.update( + worker, + new_recipient, + TYPING_STARTED_WAIT_PERIOD, + TYPING_STOPPED_WAIT_PERIOD, + ); + } + + worker = { + get_current_time: returns_time(5), + notify_server_start, + notify_server_stop, + }; + + // Start typing stream message + call_handler({stream_id: 3, topic: "test"}); + assert.deepEqual(typing_status.state, { + next_send_start_time: make_time(5 + 10), + idle_timer: "idle_timer_stub", + current_recipient: {stream_id: 3, topic: "test"}, + }); + assert.deepEqual(events, { + idle_callback: events.idle_callback, + started: true, + stopped: false, + timer_cleared: false, + }); + assert.ok(events.idle_callback); + + // type again 3 seconds later. Covers 'same_stream_and_topic' codepath. + worker.get_current_time = returns_time(8); + call_handler({stream_id: 3, topic: "test"}); + assert.deepEqual(typing_status.state, { + next_send_start_time: make_time(5 + 10), + idle_timer: "idle_timer_stub", + current_recipient: {stream_id: 3, topic: "test"}, + }); + assert.deepEqual(events, { + idle_callback: events.idle_callback, + started: false, + stopped: false, + timer_cleared: true, + }); + assert.ok(events.idle_callback); + + // Explicitly stop. + call_handler(null); + assert.deepEqual(typing_status.state, null); + assert.deepEqual(events, { + idle_callback: undefined, + started: false, + stopped: true, + timer_cleared: true, + }); +});