Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

typing_notifications: Send ajax requests for stream typing too. 

For the timing part of sending requests, it will be the same
as DMs, as the code in 'typing_status.ts' is being reused
for that purpose.

As a note, 'state.current_recipient_ids' and 'new_recipient_ids'
of update() in 'typing_status.ts' used to be an array of recipient ids.

Renamed them to 'state.current_recipient' and 'new_recipient' as they
can now be either of:
1) An object of format {stream_id: 2, topic:'hello'}
2) an array of recipient user IDs like previously

Also, made required changes in 'typing_status.ts' and
'typing_status.js.flow', i.e., documenting the new format of
new_recipient.

Co-authored-by: Prakhar Pratyush <prakhar841301@gmail.com>
This commit is contained in:
Dinesh 2020-12-29 01:04:17 +05:30 committed by Tim Abbott
parent 820dcc50a0
commit 91f03e0d38
4 changed files with 222 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
web/shared/src/typing_status.js.flow
View File

@ -5,14 +5,15 @@
"use strict";
type RecipientUserIds<UserId: number> = $ReadOnlyArray<UserId>;
type StreamTopicObject = {|stream_id: number, topic: string|};
type Worker<UserId> = {|
get_current_time: () => number, // as ms since epoch
notify_server_start: (RecipientUserIds<UserId>) => void,
notify_server_stop: (RecipientUserIds<UserId>) => void,
notify_server_start: (RecipientUserIds<UserId> | StreamTopicObject) => void,
notify_server_stop: (RecipientUserIds<UserId> | StreamTopicObject) => void,
|};
declare export function update<UserId>(
worker: Worker<UserId>,
new_recipient: RecipientUserIds<UserId> | null,
new_recipient: RecipientUserIds<UserId> | StreamTopicObject | null,
): void;

82
web/shared/src/typing_status.ts
View File

@ -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<typeof setTimeout>;
};
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);
}

61
web/src/typing.js
View File

@ -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 = {

126
web/tests/typing_status.test.js
View File

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