From 50dc8c87e9b26cd2b1cae58e4f4ea971735ec963 Mon Sep 17 00:00:00 2001 From: Lalit Date: Sun, 11 Jun 2023 22:24:04 +0530 Subject: [PATCH] typing_status: Refactor `recipient` family of variables. 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. --- web/shared/src/typing_status.ts | 40 ++++++++++++------------ web/tests/typing_status.test.js | 54 ++++++++++++++++----------------- 2 files changed, 47 insertions(+), 47 deletions(-) diff --git a/web/shared/src/typing_status.ts b/web/shared/src/typing_status.ts index c50e0fc924..99e42da929 100644 --- a/web/shared/src/typing_status.ts +++ b/web/shared/src/typing_status.ts @@ -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; }; @@ -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); } diff --git a/web/tests/typing_status.test.js b/web/tests/typing_status.test.js index 97a42b351d..dceb6d4874 100644 --- a/web/tests/typing_status.test.js +++ b/web/tests/typing_status.test.js @@ -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);