From 451ddf4c847e7638d328ad3783fa8a1b922ef575 Mon Sep 17 00:00:00 2001 From: Prakhar Pratyush Date: Fri, 24 Nov 2023 19:36:42 +0530 Subject: [PATCH] typeahead: Show @-topic typeahead whenever it might be possible to use. Earlier, when a topic had less than 15 participants, the @-topic typeahead was not visible. It should be visible irrespective of the 'realm_wildcard_mention_policy' setting when the participant count is not greater than 15. The participant count for a topic can't always be calculated accurately in the client as some of the messages might still be loading. We show @-topic in the typeahead whenever it might be possible to use it. We will give an error later if you aren't allowed to use it. Fixes #27852. --- web/src/compose_validate.js | 58 +++++++++++++++++++++++++- web/src/composebox_typeahead.js | 14 ++++--- web/src/recent_senders.js | 8 ++++ web/tests/compose_validate.test.js | 24 +++++------ web/tests/composebox_typeahead.test.js | 12 ++++-- web/tests/recent_senders.test.js | 5 +++ 6 files changed, 99 insertions(+), 22 deletions(-) diff --git a/web/src/compose_validate.js b/web/src/compose_validate.js index 110c5dbdf5..c3f87b119a 100644 --- a/web/src/compose_validate.js +++ b/web/src/compose_validate.js @@ -14,9 +14,12 @@ import * as compose_pm_pill from "./compose_pm_pill"; import * as compose_state from "./compose_state"; import * as compose_ui from "./compose_ui"; import {$t} from "./i18n"; +import * as message_store from "./message_store"; import {page_params} from "./page_params"; import * as peer_data from "./peer_data"; import * as people from "./people"; +import * as reactions from "./reactions"; +import * as recent_senders from "./recent_senders"; import * as settings_config from "./settings_config"; import * as settings_data from "./settings_data"; import * as stream_data from "./stream_data"; @@ -383,6 +386,55 @@ function is_recipient_large_stream() { ); } +export function topic_participant_count_more_than_threshold(stream_id, topic) { + // Topic participants: + // Users who either sent or reacted to the messages in the topic. + const participant_ids = new Set(); + + const sender_ids = recent_senders.get_topic_recent_senders(stream_id, topic); + for (const id of sender_ids) { + participant_ids.add(id); + } + + // If senders count is greater than threshold, no need to calculate reactors. + if (participant_ids.size > wildcard_mention_threshold) { + return true; + } + + for (const sender_id of sender_ids) { + const message_ids = recent_senders.get_topic_message_ids_for_sender( + stream_id, + topic, + sender_id, + ); + for (const message_id of message_ids) { + const message = message_store.get(message_id); + if (message) { + const message_reactions = reactions.get_message_reactions(message); + const reactor_ids = message_reactions.flatMap((obj) => obj.user_ids); + for (const id of reactor_ids) { + participant_ids.add(id); + } + if (participant_ids.size > wildcard_mention_threshold) { + return true; + } + } + } + } + + return false; +} + +function is_recipient_large_topic() { + return ( + compose_state.stream_id() && + topic_participant_count_more_than_threshold( + compose_state.stream_id(), + compose_state.topic(), + ) + ); +} + function wildcard_mention_policy_authorizes_user() { if ( page_params.realm_wildcard_mention_policy === @@ -427,10 +479,14 @@ function wildcard_mention_policy_authorizes_user() { return !page_params.is_guest; } -export function wildcard_mention_allowed() { +export function stream_wildcard_mention_allowed() { return !is_recipient_large_stream() || wildcard_mention_policy_authorizes_user(); } +export function topic_wildcard_mention_allowed() { + return !is_recipient_large_topic() || wildcard_mention_policy_authorizes_user(); +} + export function set_wildcard_mention_threshold(value) { wildcard_mention_threshold = value; } diff --git a/web/src/composebox_typeahead.js b/web/src/composebox_typeahead.js index 9c74207871..3836552ebc 100644 --- a/web/src/composebox_typeahead.js +++ b/web/src/composebox_typeahead.js @@ -387,13 +387,15 @@ function get_wildcard_string(mention) { } export function broadcast_mentions() { - if (!compose_validate.wildcard_mention_allowed()) { - return []; - } - const wildcard_mention_array = ["all", "everyone"]; - if (compose_state.get_message_type() === "stream") { - wildcard_mention_array.push("stream", "topic"); + let wildcard_mention_array = []; + if (compose_state.get_message_type() === "private") { + wildcard_mention_array = ["all", "everyone"]; + } else if (compose_validate.stream_wildcard_mention_allowed()) { + wildcard_mention_array = ["all", "everyone", "stream", "topic"]; + } else if (compose_validate.topic_wildcard_mention_allowed()) { + wildcard_mention_array = ["topic"]; } + return wildcard_mention_array.map((mention, idx) => ({ special_item_text: `${mention} (${get_wildcard_string(mention)})`, email: mention, diff --git a/web/src/recent_senders.js b/web/src/recent_senders.js index 2fb6c859dd..e21696e34c 100644 --- a/web/src/recent_senders.js +++ b/web/src/recent_senders.js @@ -248,3 +248,11 @@ export function get_pm_recent_senders(user_ids_string) { pm_senders_info.participants.sort(compare_pm_user_ids_by_recency); return pm_senders_info; } + +export function get_topic_message_ids_for_sender(stream_id, topic, sender_id) { + const id_tracker = topic_senders?.get(stream_id)?.get(topic)?.get(sender_id); + if (id_tracker === undefined) { + return new Set(); + } + return id_tracker.ids; +} diff --git a/web/tests/compose_validate.test.js b/web/tests/compose_validate.test.js index 563eaf4fec..c802c3f960 100644 --- a/web/tests/compose_validate.test.js +++ b/web/tests/compose_validate.test.js @@ -324,7 +324,7 @@ test_ui("get_invalid_recipient_emails", ({override_rewire}) => { assert.deepEqual(compose_validate.get_invalid_recipient_emails(), []); }); -test_ui("test_wildcard_mention_allowed", ({override_rewire}) => { +test_ui("test_stream_wildcard_mention_allowed", ({override_rewire}) => { page_params.user_id = me.user_id; // First, check for large streams (>15 subscribers) where the wildcard mention @@ -335,39 +335,39 @@ test_ui("test_wildcard_mention_allowed", ({override_rewire}) => { settings_config.wildcard_mention_policy_values.by_everyone.code; page_params.is_guest = true; page_params.is_admin = false; - assert.ok(compose_validate.wildcard_mention_allowed()); + assert.ok(compose_validate.stream_wildcard_mention_allowed()); page_params.realm_wildcard_mention_policy = settings_config.wildcard_mention_policy_values.nobody.code; page_params.is_admin = true; - assert.ok(!compose_validate.wildcard_mention_allowed()); + assert.ok(!compose_validate.stream_wildcard_mention_allowed()); page_params.realm_wildcard_mention_policy = settings_config.wildcard_mention_policy_values.by_members.code; page_params.is_guest = true; page_params.is_admin = false; - assert.ok(!compose_validate.wildcard_mention_allowed()); + assert.ok(!compose_validate.stream_wildcard_mention_allowed()); page_params.is_guest = false; - assert.ok(compose_validate.wildcard_mention_allowed()); + assert.ok(compose_validate.stream_wildcard_mention_allowed()); page_params.realm_wildcard_mention_policy = settings_config.wildcard_mention_policy_values.by_moderators_only.code; page_params.is_moderator = false; - assert.ok(!compose_validate.wildcard_mention_allowed()); + assert.ok(!compose_validate.stream_wildcard_mention_allowed()); page_params.is_moderator = true; - assert.ok(compose_validate.wildcard_mention_allowed()); + assert.ok(compose_validate.stream_wildcard_mention_allowed()); page_params.realm_wildcard_mention_policy = settings_config.wildcard_mention_policy_values.by_admins_only.code; page_params.is_admin = false; - assert.ok(!compose_validate.wildcard_mention_allowed()); + assert.ok(!compose_validate.stream_wildcard_mention_allowed()); // TODO: Add a by_admins_only case when we implement stream-level administrators. page_params.is_admin = true; - assert.ok(compose_validate.wildcard_mention_allowed()); + assert.ok(compose_validate.stream_wildcard_mention_allowed()); page_params.realm_wildcard_mention_policy = settings_config.wildcard_mention_policy_values.by_full_members.code; @@ -375,9 +375,9 @@ test_ui("test_wildcard_mention_allowed", ({override_rewire}) => { person.date_joined = new Date(Date.now()); page_params.realm_waiting_period_threshold = 10; - assert.ok(compose_validate.wildcard_mention_allowed()); + assert.ok(compose_validate.stream_wildcard_mention_allowed()); page_params.is_admin = false; - assert.ok(!compose_validate.wildcard_mention_allowed()); + assert.ok(!compose_validate.stream_wildcard_mention_allowed()); // Now, check for small streams (<=15 subscribers) where the wildcard mention // policy doesn't matter; everyone is allowed to use wildcard mentions. @@ -386,7 +386,7 @@ test_ui("test_wildcard_mention_allowed", ({override_rewire}) => { settings_config.wildcard_mention_policy_values.by_admins_only.code; page_params.is_admin = false; page_params.is_guest = true; - assert.ok(compose_validate.wildcard_mention_allowed()); + assert.ok(compose_validate.stream_wildcard_mention_allowed()); }); test_ui("validate_stream_message", ({override_rewire, mock_template}) => { diff --git a/web/tests/composebox_typeahead.test.js b/web/tests/composebox_typeahead.test.js index 3b1bd54b43..3fac8d8f4a 100644 --- a/web/tests/composebox_typeahead.test.js +++ b/web/tests/composebox_typeahead.test.js @@ -22,7 +22,7 @@ const compose_ui = mock_esm("../src/compose_ui", { const compose_validate = mock_esm("../src/compose_validate", { validate_message_length: () => true, warn_if_topic_resolved: noop, - wildcard_mention_allowed: () => true, + stream_wildcard_mention_allowed: () => true, }); const input_pill = mock_esm("../src/input_pill"); const message_user_ids = mock_esm("../src/message_user_ids", { @@ -82,10 +82,16 @@ run_test("verify wildcard mentions typeahead for stream message", () => { assert.equal(mention_stream.special_item_text, "stream (translated: Notify stream)"); assert.equal(mention_topic.special_item_text, "topic (translated: Notify topic)"); - compose_validate.wildcard_mention_allowed = () => false; + compose_validate.stream_wildcard_mention_allowed = () => false; + compose_validate.topic_wildcard_mention_allowed = () => true; + const mention_topic_only = ct.broadcast_mentions()[0]; + assert.equal(mention_topic_only.full_name, "topic"); + + compose_validate.stream_wildcard_mention_allowed = () => false; + compose_validate.topic_wildcard_mention_allowed = () => false; const mentionNobody = ct.broadcast_mentions(); assert.equal(mentionNobody.length, 0); - compose_validate.wildcard_mention_allowed = () => true; + compose_validate.stream_wildcard_mention_allowed = () => true; }); run_test("verify wildcard mentions typeahead for direct message", () => { diff --git a/web/tests/recent_senders.test.js b/web/tests/recent_senders.test.js index cd6fa1ed07..0e30193a35 100644 --- a/web/tests/recent_senders.test.js +++ b/web/tests/recent_senders.test.js @@ -184,6 +184,11 @@ test("process_stream_message", () => { true, ); + // Messages sent by sender1 in stream1 > topic1 + assert.equal(rs.get_topic_message_ids_for_sender(stream1, topic1, sender1).size, 2); + // Messages sent by sender1 in stream1 > topic2 + assert.equal(rs.get_topic_message_ids_for_sender(stream1, topic2, sender1).size, 0); + // Same stream, but different topics const message6 = make_stream_message({ stream_id: stream3,