From 3ce7b77092986adbc72a45ee3f7b3d47ccf1ab6e Mon Sep 17 00:00:00 2001 From: Samuel Date: Thu, 17 Aug 2023 13:42:41 +0100 Subject: [PATCH] typing: Add typing constants to the post register api response. Adds typing notification constants to the response given by `POST /register`. Until now, these were hardcoded by clients based on the documentation for implementing typing notifications in the main endpoint description for `api/set-typing-status`. This change also reflects updating the web-app frontend code to use the new constants from the register response. Co-authored-by: Samuel Kabuya Co-authored-by: Wilhelmina Asante --- api_docs/changelog.md | 9 +++++++ version.py | 2 +- web/shared/src/typing_status.ts | 45 ++++++++++++++++----------------- web/src/typing.js | 20 ++++++++++++--- web/src/typing_events.js | 8 +++--- web/tests/typing_status.test.js | 40 ++++++++++++++++++++++++----- zerver/lib/events.py | 11 +++++++- zerver/openapi/zulip.yaml | 35 ++++++++++++++++++++++++- zerver/tests/test_home.py | 3 +++ zproject/default_settings.py | 13 ++++++++++ 10 files changed, 146 insertions(+), 40 deletions(-) diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 676e62d7dd..8976a95356 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -20,6 +20,15 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 8.0 +**Feature level 204** + +* [`POST /register`](/api/register-queue): Added + `server_typing_started_wait_period_milliseconds`, + `server_typing_stopped_wait_period_milliseconds`, and + `server_typing_started_expiry_period_milliseconds` fields + for clients to use when implementing [typing + notifications](/api/set-typing-status) protocol. + **Feature level 203** * [`POST /register`](/api/register-queue): Add diff --git a/version.py b/version.py index 48a7a6d6e1..5ae83855b3 100644 --- a/version.py +++ b/version.py @@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.9.3" # Changes should be accompanied by documentation explaining what the # new level means in api_docs/changelog.md, as well as "**Changes**" # entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 203 +API_FEATURE_LEVEL = 204 # Bump the minor PROVISION_VERSION to indicate that folks should provision # only when going from an old version of the code to a newer version. Bump diff --git a/web/shared/src/typing_status.ts b/web/shared/src/typing_status.ts index 294aa38695..19635a10aa 100644 --- a/web/shared/src/typing_status.ts +++ b/web/shared/src/typing_status.ts @@ -13,18 +13,6 @@ type TypingStatusState = { idle_timer: ReturnType; }; -// The following constants are tuned to work with -// TYPING_STARTED_EXPIRY_PERIOD, which is what the other -// users will use to time out our messages. (Or us, -// depending on your perspective.) See typing_events.js. - -// How frequently 'still typing' notifications are sent -// to extend the expiry -const TYPING_STARTED_WAIT_PERIOD = 10000; // 10s -// How long after someone stops editing in the compose box -// do we send a 'stopped typing' notification -const TYPING_STOPPED_WAIT_PERIOD = 5000; // 5s - /** Exported only for tests. */ export let state: TypingStatusState | null = null; @@ -39,6 +27,7 @@ export function stop_last_notification(worker: TypingStatusWorker): void { /** Exported only for tests. */ export function start_or_extend_idle_timer( worker: TypingStatusWorker, + typing_stopped_wait_period: number, ): ReturnType { function on_idle_timeout(): void { // We don't do any real error checking here, because @@ -51,29 +40,34 @@ export function start_or_extend_idle_timer( if (state?.idle_timer) { clearTimeout(state.idle_timer); } - return setTimeout(on_idle_timeout, TYPING_STOPPED_WAIT_PERIOD); + return setTimeout(on_idle_timeout, typing_stopped_wait_period); } -function set_next_start_time(current_time: number): void { +function set_next_start_time(current_time: number, typing_started_wait_period: number): void { assert(state !== null, "State object should not be null here."); - state.next_send_start_time = current_time + TYPING_STARTED_WAIT_PERIOD; + state.next_send_start_time = current_time + typing_started_wait_period; } function actually_ping_server( worker: TypingStatusWorker, recipient_ids: number[], current_time: number, + typing_started_wait_period: number, ): void { worker.notify_server_start(recipient_ids); - set_next_start_time(current_time); + set_next_start_time(current_time, typing_started_wait_period); } /** Exported only for tests. */ -export function maybe_ping_server(worker: TypingStatusWorker, recipient_ids: number[]): void { +export function maybe_ping_server( + worker: TypingStatusWorker, + recipient_ids: number[], + 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); + actually_ping_server(worker, recipient_ids, current_time, typing_started_wait_period); } } @@ -103,17 +97,22 @@ export function maybe_ping_server(worker: TypingStatusWorker, recipient_ids: num * addressed to, as a sorted array of user IDs; or `null` if no direct message * is being composed anymore. */ -export function update(worker: TypingStatusWorker, new_recipient_ids: number[] | null): void { +export function update( + worker: TypingStatusWorker, + new_recipient_ids: number[] | 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)) { // Nothing has really changed, except we may need // to send a ping to the server. - maybe_ping_server(worker, new_recipient_ids!); + maybe_ping_server(worker, new_recipient_ids!, typing_started_wait_period); // We can also extend out our idle time. - state.idle_timer = start_or_extend_idle_timer(worker); + state.idle_timer = start_or_extend_idle_timer(worker, typing_stopped_wait_period); return; } @@ -135,8 +134,8 @@ export function update(worker: TypingStatusWorker, new_recipient_ids: number[] | state = { current_recipient_ids: new_recipient_ids, next_send_start_time: 0, - idle_timer: start_or_extend_idle_timer(worker), + 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); + actually_ping_server(worker, new_recipient_ids, current_time, typing_started_wait_period); } diff --git a/web/src/typing.js b/web/src/typing.js index c9f7a14ece..5c44684649 100644 --- a/web/src/typing.js +++ b/web/src/typing.js @@ -6,14 +6,21 @@ import * as blueslip from "./blueslip"; import * as channel from "./channel"; 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 {user_settings} from "./user_settings"; // This module handles the outbound side of typing indicators. // We detect changes in the compose box and notify the server // when we are typing. For the inbound side see typing_events.js. -// -// See docs/subsystems/typing-indicators.md for details on typing indicators. +// See docs/subsystems/typing-indicators.md for more details. + +// How frequently 'start' notifications are sent to extend +// the expiry of active typing indicators. +const typing_started_wait_period = page_params.server_typing_started_wait_period_milliseconds; +// How long after someone stops editing in the compose box +// 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) { channel.post({ @@ -78,12 +85,17 @@ export function initialize() { // If our previous state was no typing notification, send a // start-typing notice immediately. const new_recipient = is_valid_conversation() ? get_recipient() : null; - typing_status.update(worker, new_recipient); + typing_status.update( + worker, + new_recipient, + typing_started_wait_period, + typing_stopped_wait_period, + ); }); // We send a stop-typing notification immediately when compose is // closed/cancelled $(document).on("compose_canceled.zulip compose_finished.zulip", () => { - typing_status.update(worker, null); + typing_status.update(worker, null, typing_started_wait_period, typing_stopped_wait_period); }); } diff --git a/web/src/typing_events.js b/web/src/typing_events.js index 6af594ae98..e613fec409 100644 --- a/web/src/typing_events.js +++ b/web/src/typing_events.js @@ -14,10 +14,10 @@ import * as typing_data from "./typing_data"; // // We also handle the local event of re-narrowing. // (For the outbound code, see typing.js.) - +// // How long before we assume a client has gone away -// and expire its typing status -const TYPING_STARTED_EXPIRY_PERIOD = 15000; // 15s +// and expire the active typing indicator. +const typing_started_expiry_period = page_params.server_typing_started_expiry_period_milliseconds; // If number of users typing exceed this, // we render "Several people are typing..." @@ -93,7 +93,7 @@ export function display_notification(event) { render_notifications_for_narrow(); - typing_data.kickstart_inbound_timer(recipients, TYPING_STARTED_EXPIRY_PERIOD, () => { + typing_data.kickstart_inbound_timer(recipients, typing_started_expiry_period, () => { hide_notification(event); }); } diff --git a/web/tests/typing_status.test.js b/web/tests/typing_status.test.js index 6bf4f29879..fb07a19b3c 100644 --- a/web/tests/typing_status.test.js +++ b/web/tests/typing_status.test.js @@ -10,6 +10,9 @@ const compose_pm_pill = mock_esm("../src/compose_pm_pill"); const typing = zrequire("typing"); const typing_status = zrequire("../shared/src/typing_status"); +const TYPING_STARTED_WAIT_PERIOD = 10000; +const TYPING_STOPPED_WAIT_PERIOD = 5000; + function make_time(secs) { // make times semi-realistic return 1000000 + 1000 * secs; @@ -26,7 +29,7 @@ run_test("basics", ({override, override_rewire}) => { // invalid conversation basically does nothing let worker = {}; - typing_status.update(worker, null); + typing_status.update(worker, null, TYPING_STARTED_WAIT_PERIOD, TYPING_STOPPED_WAIT_PERIOD); // Start setting up more testing state. const events = {}; @@ -63,7 +66,12 @@ run_test("basics", ({override, override_rewire}) => { function call_handler(new_recipient) { clear_events(); - typing_status.update(worker, new_recipient); + typing_status.update( + worker, + new_recipient, + TYPING_STARTED_WAIT_PERIOD, + TYPING_STOPPED_WAIT_PERIOD, + ); } worker = { @@ -263,12 +271,22 @@ run_test("basics", ({override, override_rewire}) => { // 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()); + typing_status.update( + worker, + typing.get_recipient(), + TYPING_STARTED_WAIT_PERIOD, + TYPING_STOPPED_WAIT_PERIOD, + ); assert.deepEqual(call_count.maybe_ping_server, 1); assert.deepEqual(call_count.start_or_extend_idle_timer, 1); assert.deepEqual(call_count.stop_last_notification, 0); - typing_status.update(worker, typing.get_recipient()); + typing_status.update( + worker, + typing.get_recipient(), + TYPING_STARTED_WAIT_PERIOD, + TYPING_STOPPED_WAIT_PERIOD, + ); assert.deepEqual(call_count.maybe_ping_server, 2); assert.deepEqual(call_count.start_or_extend_idle_timer, 2); assert.deepEqual(call_count.stop_last_notification, 0); @@ -276,14 +294,24 @@ run_test("basics", ({override, override_rewire}) => { // change in recipient and new_recipient should make us // call typing_status.stop_last_notification override(compose_pm_pill, "get_user_ids_string", () => "2,3,4"); - typing_status.update(worker, typing.get_recipient()); + typing_status.update( + worker, + typing.get_recipient(), + TYPING_STARTED_WAIT_PERIOD, + 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.stop_last_notification, 1); // Stream messages are represented as get_user_ids_string being empty override(compose_pm_pill, "get_user_ids_string", () => ""); - typing_status.update(worker, typing.get_recipient()); + typing_status.update( + worker, + typing.get_recipient(), + TYPING_STARTED_WAIT_PERIOD, + 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.stop_last_notification, 2); diff --git a/zerver/lib/events.py b/zerver/lib/events.py index 27a5888744..8912e60c17 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -364,7 +364,16 @@ def fetch_initial_state_data( # Presence system parameters for client behavior. state["server_presence_ping_interval_seconds"] = settings.PRESENCE_PING_INTERVAL_SECS state["server_presence_offline_threshold_seconds"] = settings.OFFLINE_THRESHOLD_SECS - + # Typing notifications protocol parameters for client behavior. + state[ + "server_typing_started_expiry_period_milliseconds" + ] = settings.TYPING_STARTED_EXPIRY_PERIOD_MILLISECONDS + state[ + "server_typing_stopped_wait_period_milliseconds" + ] = settings.TYPING_STOPPED_WAIT_PERIOD_MILLISECONDS + state[ + "server_typing_started_wait_period_milliseconds" + ] = settings.TYPING_STARTED_WAIT_PERIOD_MILLISECONDS if want("realm_user_settings_defaults"): realm_user_default = RealmUserDefault.objects.get(realm=realm) state["realm_user_settings_defaults"] = {} diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index e475cd196b..e622f65f5e 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -11240,7 +11240,7 @@ paths: **Changes**: New in Zulip 7.0 (feature level 164). Clients should use 60 for older Zulip servers, since that's the value that was hardcoded in the - the Zulip mobile apps prior to this parameter being introduced. + Zulip mobile apps prior to this parameter being introduced. server_presence_offline_threshold_seconds: type: integer description: | @@ -11251,6 +11251,39 @@ paths: **Changes**: New in Zulip 7.0 (feature level 164). Clients should use 140 for older Zulip servers, since that's the value that was hardcoded in the Zulip client apps prior to this parameter being introduced. + server_typing_started_expiry_period_milliseconds: + type: integer + description: | + For clients implementing [typing notifications](/api/set-typing-status) + protocol, the time interval in milliseconds that the client should wait + for additional [typing start](/api/get-events#typing-start) events from + the server before removing an active typing indicator. + + **Changes**: New in Zulip 8.0 (feature level 204). Clients should use 15000 + for older Zulip servers, since that's the value that was hardcoded in the + Zulip apps prior to this parameter being introduced. + server_typing_stopped_wait_period_milliseconds: + type: integer + description: | + For clients implementing [typing notifications](/api/set-typing-status) + protocol, the time interval in milliseconds that the client should wait + when a user stops interacting with the compose UI before sending a stop + notification to the server. + + **Changes**: New in Zulip 8.0 (feature level 204). Clients should use 5000 + for older Zulip servers, since that's the value that was hardcoded in the + Zulip apps prior to this parameter being introduced. + server_typing_started_wait_period_milliseconds: + type: integer + description: | + For clients implementing [typing notifications](/api/set-typing-status) + protocol, the time interval in milliseconds that the client should use + to send regular start notifications to the server to indicate that the + user is still actively interacting with the compose UI. + + **Changes**: New in Zulip 8.0 (feature level 204). Clients should use 10000 + for older Zulip servers, since that's the value that was hardcoded in the + Zulip apps prior to this parameter being introduced. scheduled_messages: type: array description: | diff --git a/zerver/tests/test_home.py b/zerver/tests/test_home.py index b1521e194e..7a566043aa 100644 --- a/zerver/tests/test_home.py +++ b/zerver/tests/test_home.py @@ -199,6 +199,9 @@ class HomeTest(ZulipTestCase): "server_presence_ping_interval_seconds", "server_sentry_dsn", "server_timestamp", + "server_typing_started_expiry_period_milliseconds", + "server_typing_started_wait_period_milliseconds", + "server_typing_stopped_wait_period_milliseconds", "server_web_public_streams_enabled", "settings_send_digest_emails", "show_billing", diff --git a/zproject/default_settings.py b/zproject/default_settings.py index 15d5513b39..930153ae74 100644 --- a/zproject/default_settings.py +++ b/zproject/default_settings.py @@ -574,3 +574,16 @@ MAX_MESSAGE_LENGTH = 10000 # More drafts, should they exist for some crazy reason, could be # fetched in a separate request. MAX_DRAFTS_IN_REGISTER_RESPONSE = 1000 + +# How long before a client should assume that another client sending +# typing notifications has gone away and expire the active typing +# indicator. +TYPING_STARTED_EXPIRY_PERIOD_MILLISECONDS = 15000 + +# How long after a user has stopped interacting with the compose UI +# that a client should send a stop notification to the server. +TYPING_STOPPED_WAIT_PERIOD_MILLISECONDS = 5000 + +# How often a client should send start notifications to the server to +# indicate that the user is still interacting with the compose UI. +TYPING_STARTED_WAIT_PERIOD_MILLISECONDS = 10000