paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-03 21:43:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

typing_status: Refactor recipient family of variables.

Browse Source 
Refactored `recipient` family of variables to better names like
`recipient_ids` which also aligns with the type of these variables.

Also, refactored `typing_status.test.js` to use array of user ids
instead of string names like `alice` and `bob` to stay consistent with
the actual type of these parameters.
This commit is contained in:
Lalit
2023-06-11 22:24:04 +05:30
committed by Tim Abbott
parent 66c672ed36
commit 50dc8c87e9
2 changed files with 47 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -3,12 +3,12 @@ import assert from "minimalistic-assert";
type TypingStatusWorker = {
get_current_time: () => number;
notify_server_start: (recipient: number[]) => void;
notify_server_stop: (recipient: number[]) => void;
notify_server_start: (recipient_ids: number[]) => void;
notify_server_stop: (recipient_ids: number[]) => void;
};
type TypingStatusState = {
current_recipient: number[];
current_recipient_ids: number[];
next_send_start_time: number;
idle_timer: ReturnType<typeof setTimeout>;
};
@@ -32,7 +32,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);
worker.notify_server_stop(state.current_recipient_ids);
state = null;
}
@@ -43,7 +43,7 @@ export function start_or_extend_idle_timer(
function on_idle_timeout(): void {
// We don't do any real error checking here, because
// if we've been idle, we need to tell folks, and if
// our current recipient has changed, previous code will
// our current recipients has changed, previous code will
// have stopped the timer.
stop_last_notification(worker);
}
@@ -61,19 +61,19 @@ function set_next_start_time(current_time: number): void {
function actually_ping_server(
worker: TypingStatusWorker,
recipient: number[],
recipient_ids: number[],
current_time: number,
): void {
worker.notify_server_start(recipient);
worker.notify_server_start(recipient_ids);
set_next_start_time(current_time);
}
/** Exported only for tests. */
export function maybe_ping_server(worker: TypingStatusWorker, recipient: number[]): void {
export function maybe_ping_server(worker: TypingStatusWorker, recipient_ids: 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, current_time);
actually_ping_server(worker, recipient_ids, current_time);
}
}
@@ -89,7 +89,7 @@ export function maybe_ping_server(worker: TypingStatusWorker, recipient: number[
* composing a stream message should be treated like composing no message at
* all.
*
* Call with `new_recipient` of `null` when the user actively stops
* Call with `new_recipient_ids` of `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.
@@ -99,18 +99,18 @@ export function maybe_ping_server(worker: TypingStatusWorker, recipient: number[
*
* @param {*} worker Callbacks for reaching the real world. See typing.js
* for implementations.
* @param {*} new_recipient The users the PM being composed is addressed to,
* @param {*} new_recipient_ids The users the PM being composed is addressed to,
* as a sorted array of user IDs; or `null` if no PM is being composed
* anymore.
*/
export function update(worker: TypingStatusWorker, new_recipient: number[] | null): void {
export function update(worker: TypingStatusWorker, new_recipient_ids: number[] | null): void {
if (state !== null) {
// We need to use _.isEqual for comparisons; === doesn't work
// on arrays.
if (_.isEqual(new_recipient, state.current_recipient)) {
if (_.isEqual(new_recipient_ids, state.current_recipient_ids)) {
// Nothing has really changed, except we may need
// to send a ping to the server.
maybe_ping_server(worker, new_recipient!);
maybe_ping_server(worker, new_recipient_ids!);
// We can also extend out our idle time.
state.idle_timer = start_or_extend_idle_timer(worker);
@@ -118,25 +118,25 @@ export function update(worker: TypingStatusWorker, new_recipient: number[] | nul
return;
}
// We apparently stopped talking to our old recipient,
// We apparently stopped talking to our old recipients,
// so we must stop the old notification. Don't return
// yet, because we may have a new recipient.
// yet, because we may have new recipients.
stop_last_notification(worker);
}
if (new_recipient === null) {
if (new_recipient_ids === null) {
// If we are not talking to somebody we care about,
// then there is no more action to take.
return;
}
// We just started talking to this recipient, so notify
// We just started talking to these recipients, so notify
// the server.
state = {
current_recipient: new_recipient,
current_recipient_ids: new_recipient_ids,
next_send_start_time: 0,
idle_timer: start_or_extend_idle_timer(worker),
};
const current_time = worker.get_current_time();
actually_ping_server(worker, new_recipient, current_time);
actually_ping_server(worker, new_recipient_ids, current_time);
}

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

@@ -45,12 +45,12 @@ run_test("basics", ({override, override_rewire}) => {
set_global("clearTimeout", clear_timeout);
function notify_server_start(recipient) {
assert.equal(recipient, "alice");
assert.deepStrictEqual(recipient, [1, 2]);
events.started = true;
}
function notify_server_stop(recipient) {
assert.equal(recipient, "alice");
assert.deepStrictEqual(recipient, [1, 2]);
events.stopped = true;
}
@@ -72,12 +72,12 @@ run_test("basics", ({override, override_rewire}) => {
notify_server_stop,
};
// Start talking to alice.
call_handler("alice");
// Start talking to users having ids - 1, 2.
call_handler([1, 2]);
assert.deepEqual(typing_status.state, {
next_send_start_time: make_time(5 + 10),
idle_timer: "idle_timer_stub",
current_recipient: "alice",
current_recipient_ids: [1, 2],
});
assert.deepEqual(events, {
idle_callback: events.idle_callback,
@@ -89,11 +89,11 @@ run_test("basics", ({override, override_rewire}) => {
// type again 3 seconds later
worker.get_current_time = returns_time(8);
call_handler("alice");
call_handler([1, 2]);
assert.deepEqual(typing_status.state, {
next_send_start_time: make_time(5 + 10),
idle_timer: "idle_timer_stub",
current_recipient: "alice",
current_recipient_ids: [1, 2],
});
assert.deepEqual(events, {
idle_callback: events.idle_callback,
@@ -106,11 +106,11 @@ run_test("basics", ({override, override_rewire}) => {
// type after 15 secs, so that we can notify the server
// again
worker.get_current_time = returns_time(18);
call_handler("alice");
call_handler([1, 2]);
assert.deepEqual(typing_status.state, {
next_send_start_time: make_time(18 + 10),
idle_timer: "idle_timer_stub",
current_recipient: "alice",
current_recipient_ids: [1, 2],
});
assert.deepEqual(events, {
idle_callback: events.idle_callback,
@@ -119,7 +119,7 @@ run_test("basics", ({override, override_rewire}) => {
timer_cleared: true,
});
// Now call alice's idle callback that we captured earlier.
// Now call recipients idle callback that we captured earlier.
const callback = events.idle_callback;
clear_events();
callback();
@@ -141,13 +141,13 @@ run_test("basics", ({override, override_rewire}) => {
timer_cleared: false,
});
// Start talking to alice again.
// Start talking to users again.
worker.get_current_time = returns_time(50);
call_handler("alice");
call_handler([1, 2]);
assert.deepEqual(typing_status.state, {
next_send_start_time: make_time(50 + 10),
idle_timer: "idle_timer_stub",
current_recipient: "alice",
current_recipient_ids: [1, 2],
});
assert.deepEqual(events, {
idle_callback: events.idle_callback,
@@ -157,7 +157,7 @@ run_test("basics", ({override, override_rewire}) => {
});
assert.ok(events.idle_callback);
// Explicitly stop alice.
// Explicitly stop users.
call_handler(null);
assert.deepEqual(typing_status.state, null);
assert.deepEqual(events, {
@@ -167,13 +167,13 @@ run_test("basics", ({override, override_rewire}) => {
timer_cleared: true,
});
// Start talking to alice again.
// Start talking to users again.
worker.get_current_time = returns_time(80);
call_handler("alice");
call_handler([1, 2]);
assert.deepEqual(typing_status.state, {
next_send_start_time: make_time(80 + 10),
idle_timer: "idle_timer_stub",
current_recipient: "alice",
current_recipient_ids: [1, 2],
});
assert.deepEqual(events, {
idle_callback: events.idle_callback,
@@ -203,13 +203,13 @@ run_test("basics", ({override, override_rewire}) => {
timer_cleared: false,
});
// Start talking to alice again.
// Start talking to users again.
worker.get_current_time = returns_time(170);
call_handler("alice");
call_handler([1, 2]);
assert.deepEqual(typing_status.state, {
next_send_start_time: make_time(170 + 10),
idle_timer: "idle_timer_stub",
current_recipient: "alice",
current_recipient_ids: [1, 2],
});
assert.deepEqual(events, {
idle_callback: events.idle_callback,
@@ -219,19 +219,19 @@ run_test("basics", ({override, override_rewire}) => {
});
assert.ok(events.idle_callback);
// Switch to bob now.
// Switch to new users now.
worker.get_current_time = returns_time(171);
worker.notify_server_start = (recipient) => {
assert.equal(recipient, "bob");
assert.deepStrictEqual(recipient, [3, 4]);
events.started = true;
};
call_handler("bob");
call_handler([3, 4]);
assert.deepEqual(typing_status.state, {
next_send_start_time: make_time(171 + 10),
idle_timer: "idle_timer_stub",
current_recipient: "bob",
current_recipient_ids: [3, 4],
});
assert.deepEqual(events, {
idle_callback: events.idle_callback,
@@ -242,10 +242,10 @@ 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 are the same
// and typing_status.state.current_recipient_ids are the same
override(compose_pm_pill, "get_user_ids_string", () => "1,2,3");
typing_status.state.current_recipient = typing.get_recipient();
typing_status.state.current_recipient_ids = typing.get_recipient();
const call_count = {
maybe_ping_server: 0,
@@ -261,7 +261,7 @@ run_test("basics", ({override, override_rewire}) => {
});
}
// User ids of people in compose narrow doesn't change and is same as stat.current_recipient
// User ids of people in compose narrow doesn't change and is same as stat.current_recipient_ids
// so counts of function should increase except stop_last_notification
typing_status.update(worker, typing.get_recipient());
assert.deepEqual(call_count.maybe_ping_server, 1);