From f5be62f60d41ebf3f7ee718476b2f7ad45b926a7 Mon Sep 17 00:00:00 2001 From: evykassirer Date: Tue, 24 Sep 2024 15:55:31 -0700 Subject: [PATCH] buddy_list: Include offline users in participants list for large orgs. --- web/src/buddy_data.ts | 39 ++++++++++++++++++++++++++++++------ web/src/buddy_list.ts | 13 +----------- web/tests/buddy_data.test.js | 36 +++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 18 deletions(-) diff --git a/web/src/buddy_data.ts b/web/src/buddy_data.ts index 34ade2aa89..f95abe80e5 100644 --- a/web/src/buddy_data.ts +++ b/web/src/buddy_data.ts @@ -2,6 +2,7 @@ import assert from "minimalistic-assert"; import * as hash_util from "./hash_util"; import {$t} from "./i18n"; +import * as message_lists from "./message_lists"; import * as muted_users from "./muted_users"; import * as narrow_state from "./narrow_state"; import {page_params} from "./page_params"; @@ -285,7 +286,11 @@ function user_is_recently_active(user_id: number): boolean { return level(user_id) <= 2; } -function maybe_shrink_list(user_ids: number[], user_filter_text: string): number[] { +function maybe_shrink_list( + user_ids: number[], + user_filter_text: string, + conversation_participants: Set, +): number[] { if (user_ids.length <= max_size_before_shrinking) { return user_ids; } @@ -310,7 +315,8 @@ function maybe_shrink_list(user_ids: number[], user_filter_text: string): number user_ids = user_ids.filter( (user_id) => user_is_recently_active(user_id) || - user_matches_narrow(user_id, pm_ids_set, filter_by_stream_id ? stream_id : undefined), + user_matches_narrow(user_id, pm_ids_set, filter_by_stream_id ? stream_id : undefined) || + conversation_participants.has(user_id), ); return user_ids; @@ -357,8 +363,12 @@ function filter_user_ids(user_filter_text: string, user_ids: number[]): number[] return [...people.filter_people_by_search_terms(persons, user_filter_text)]; } -function get_filtered_user_id_list(user_filter_text: string): number[] { - let base_user_id_list; +function get_filtered_user_id_list( + user_filter_text: string, + conversation_participants: Set, +): number[] { + // We always want to show conversation participants even if they're inactive. + let base_user_id_list = [...conversation_participants]; if (user_filter_text) { // If there's a filter, select from all users, not just those @@ -399,10 +409,27 @@ function get_filtered_user_id_list(user_filter_text: string): number[] { return user_ids; } +export function get_conversation_participants(): Set { + const participant_ids_set = new Set(); + if (!narrow_state.stream_id() || !narrow_state.topic() || !message_lists.current) { + return participant_ids_set; + } + for (const message of message_lists.current.all_messages()) { + if ( + !people.is_valid_bot_user(message.sender_id) && + people.is_person_active(message.sender_id) + ) { + participant_ids_set.add(message.sender_id); + } + } + return participant_ids_set; +} + export function get_filtered_and_sorted_user_ids(user_filter_text: string): number[] { let user_ids; - user_ids = get_filtered_user_id_list(user_filter_text); - user_ids = maybe_shrink_list(user_ids, user_filter_text); + const conversation_participants = get_conversation_participants(); + user_ids = get_filtered_user_id_list(user_filter_text, conversation_participants); + user_ids = maybe_shrink_list(user_ids, user_filter_text, conversation_participants); return sort_users(user_ids); } diff --git a/web/src/buddy_list.ts b/web/src/buddy_list.ts index f85e035c1a..926d03d15b 100644 --- a/web/src/buddy_list.ts +++ b/web/src/buddy_list.ts @@ -15,7 +15,6 @@ import type {BuddyUserInfo} from "./buddy_data"; import {media_breakpoints_num} from "./css_variables"; import * as hash_util from "./hash_util"; import {$t} from "./i18n"; -import * as message_lists from "./message_lists"; import * as message_viewport from "./message_viewport"; import * as narrow_state from "./narrow_state"; import * as padded_widget from "./padded_widget"; @@ -90,17 +89,7 @@ function get_render_data(): BuddyListRenderData { const total_human_users = people.get_active_human_count(); const other_users_count = total_human_users - total_human_subscribers_count; const hide_headers = should_hide_headers(current_sub, pm_ids_set); - const participant_ids_set = new Set(); - if (current_sub && narrow_state.topic() && message_lists.current) { - for (const message of message_lists.current.all_messages()) { - if ( - !people.is_valid_bot_user(message.sender_id) && - people.is_person_active(message.sender_id) - ) { - participant_ids_set.add(message.sender_id); - } - } - } + const participant_ids_set = buddy_data.get_conversation_participants(); return { current_sub, diff --git a/web/tests/buddy_data.test.js b/web/tests/buddy_data.test.js index 2d9c3b2645..f9d8d50b06 100644 --- a/web/tests/buddy_data.test.js +++ b/web/tests/buddy_data.test.js @@ -22,6 +22,8 @@ const presence = zrequire("presence"); const stream_data = zrequire("stream_data"); const user_status = zrequire("user_status"); const buddy_data = zrequire("buddy_data"); +const {Filter} = zrequire("filter"); +const message_lists = zrequire("message_lists"); // The buddy_data module is mostly tested indirectly through // activity.test.js, but we should feel free to add direct tests @@ -109,6 +111,7 @@ function test(label, f) { people.add_active_user(me); people.initialize_current_user(me.user_id); muted_users.set_muted_users([]); + message_lists.set_current(undefined); f(helpers); @@ -376,6 +379,39 @@ test("show offline channel subscribers for small channels", ({override_rewire}) assert.deepEqual(buddy_data.get_filtered_and_sorted_user_ids(""), [me.user_id, alice.user_id]); }); +test("get_conversation_participants", ({override_rewire}) => { + people.add_active_user(selma); + + const rome_sub = {name: "Rome", subscribed: true, stream_id: 1001}; + stream_data.add_sub(rome_sub); + peer_data.set_subscribers(rome_sub.stream_id, [selma.user_id]); + + const filter = new Filter([ + {operator: "channel", operand: rome_sub.channel_id}, + {operator: "topic", operand: "Foo"}, + ]); + message_lists.set_current({ + data: { + filter, + }, + all_messages() { + return [ + { + sender_id: selma.user_id, + id: rome_sub.stream_id, + content: "example content", + topic: "Foo", + type: "stream", + }, + ]; + }, + }); + override_rewire(narrow_state, "stream_id", () => rome_sub.stream_id); + override_rewire(narrow_state, "topic", () => "Foo"); + + assert.deepEqual(buddy_data.get_conversation_participants(), new Set([selma.user_id])); +}); + test("level", () => { realm.server_presence_offline_threshold_seconds = 200;