From 3ce7a9dd0844d0d959c0ed1ea511a6f5c56b6d84 Mon Sep 17 00:00:00 2001 From: Priyam Seth Date: Wed, 28 Jul 2021 21:17:40 +0530 Subject: [PATCH] compose: Move warn functions from compose.js to compose_validate.js. This commit moves the warn_if_private_stream_is_linked, needs_subscribe_warning, and warn_if_mentioning_unsubscribed_user to compose_validate.js from compose.js. These warning functions are very naturally part of the compose box validation system, though they're a bit different in being called from the typeahead codebase. Part of splitting compose.js into more natural modules. --- frontend_tests/node_tests/compose.js | 232 ------------------ frontend_tests/node_tests/compose_validate.js | 232 ++++++++++++++++++ .../node_tests/composebox_typeahead.js | 13 +- static/js/compose.js | 140 ----------- static/js/compose_validate.js | 139 +++++++++++ static/js/composebox_typeahead.js | 4 +- 6 files changed, 380 insertions(+), 380 deletions(-) diff --git a/frontend_tests/node_tests/compose.js b/frontend_tests/node_tests/compose.js index 8fcafd1246..019ebc63dc 100644 --- a/frontend_tests/node_tests/compose.js +++ b/frontend_tests/node_tests/compose.js @@ -37,7 +37,6 @@ const rendered_markdown = mock_esm("../../static/js/rendered_markdown"); const resize = mock_esm("../../static/js/resize"); const sent_messages = mock_esm("../../static/js/sent_messages"); const server_events = mock_esm("../../static/js/server_events"); -const settings_data = mock_esm("../../static/js/settings_data"); const stream_edit = mock_esm("../../static/js/stream_edit"); const stream_settings_ui = mock_esm("../../static/js/stream_settings_ui"); const transmit = mock_esm("../../static/js/transmit"); @@ -48,7 +47,6 @@ const compose_pm_pill = zrequire("compose_pm_pill"); const compose_state = zrequire("compose_state"); const compose = zrequire("compose"); const echo = zrequire("echo"); -const peer_data = zrequire("peer_data"); const people = zrequire("people"); const stream_data = zrequire("stream_data"); const upload = zrequire("upload"); @@ -415,78 +413,6 @@ test_ui("finish", ({override}) => { })(); }); -test_ui("warn_if_private_stream_is_linked", ({mock_template}) => { - const test_sub = { - name: compose_state.stream_name(), - stream_id: 99, - }; - - stream_data.add_sub(test_sub); - peer_data.set_subscribers(test_sub.stream_id, [1, 2]); - - let denmark = { - stream_id: 100, - name: "Denmark", - }; - stream_data.add_sub(denmark); - - peer_data.set_subscribers(denmark.stream_id, [1, 2, 3]); - - function test_noop_case(invite_only) { - compose_state.set_message_type("stream"); - denmark.invite_only = invite_only; - compose.warn_if_private_stream_is_linked(denmark); - assert.equal($("#compose_private_stream_alert").visible(), false); - } - - test_noop_case(false); - // invite_only=true and current compose stream subscribers are a subset - // of mentioned_stream subscribers. - test_noop_case(true); - - $("#compose_private").hide(); - compose_state.set_message_type("stream"); - - const checks = [ - (function () { - let called; - mock_template("compose_private_stream_alert.hbs", false, (context) => { - called = true; - assert.equal(context.stream_name, "Denmark"); - return "fake-compose_private_stream_alert-template"; - }); - return function () { - assert.ok(called); - }; - })(), - - (function () { - let called; - $("#compose_private_stream_alert").append = (html) => { - called = true; - assert.equal(html, "fake-compose_private_stream_alert-template"); - }; - return function () { - assert.ok(called); - }; - })(), - ]; - - denmark = { - invite_only: true, - name: "Denmark", - stream_id: 22, - }; - stream_data.add_sub(denmark); - - compose.warn_if_private_stream_is_linked(denmark); - assert.equal($("#compose_private_stream_alert").visible(), true); - - for (const f of checks) { - f(); - } -}); - test_ui("initialize", ({override, mock_template}) => { override(giphy, "is_giphy_enabled", () => true); @@ -638,164 +564,6 @@ test_ui("trigger_submit_compose_form", ({override}) => { assert.ok(compose_finish_checked); }); -test_ui("needs_subscribe_warning", () => { - const invalid_user_id = 999; - - const test_bot = { - full_name: "Test Bot", - email: "test-bot@example.com", - user_id: 135, - is_bot: true, - }; - people.add_active_user(test_bot); - - const sub = { - stream_id: 110, - name: "stream", - }; - - stream_data.add_sub(sub); - peer_data.set_subscribers(sub.stream_id, [bob.user_id, me.user_id]); - - blueslip.expect("error", "Unknown user_id in get_by_user_id: 999"); - // Test with an invalid user id. - assert.equal(compose.needs_subscribe_warning(invalid_user_id, sub.stream_id), false); - - // Test with bot user. - assert.equal(compose.needs_subscribe_warning(test_bot.user_id, sub.stream_id), false); - - // Test when user is subscribed to the stream. - assert.equal(compose.needs_subscribe_warning(bob.user_id, sub.stream_id), false); - - peer_data.remove_subscriber(sub.stream_id, bob.user_id); - // Test when the user is not subscribed. - assert.equal(compose.needs_subscribe_warning(bob.user_id, sub.stream_id), true); -}); - -test_ui("warn_if_mentioning_unsubscribed_user", ({override, mock_template}) => { - override(settings_data, "user_can_subscribe_other_users", () => true); - - let mentioned = { - email: "foo@bar.com", - }; - - $("#compose_invite_users .compose_invite_user").length = 0; - - function test_noop_case(is_private, is_zephyr_mirror, is_broadcast) { - const msg_type = is_private ? "private" : "stream"; - compose_state.set_message_type(msg_type); - page_params.realm_is_zephyr_mirror_realm = is_zephyr_mirror; - mentioned.is_broadcast = is_broadcast; - compose.warn_if_mentioning_unsubscribed_user(mentioned); - assert.equal($("#compose_invite_users").visible(), false); - } - - test_noop_case(true, false, false); - test_noop_case(false, true, false); - test_noop_case(false, false, true); - - $("#compose_invite_users").hide(); - compose_state.set_message_type("stream"); - page_params.realm_is_zephyr_mirror_realm = false; - - // Test with empty stream name in compose box. It should return noop. - assert.equal(compose_state.stream_name(), ""); - compose.warn_if_mentioning_unsubscribed_user(mentioned); - assert.equal($("#compose_invite_users").visible(), false); - - compose_state.stream_name("random"); - const sub = { - stream_id: 111, - name: "random", - }; - - // Test with invalid stream in compose box. It should return noop. - compose.warn_if_mentioning_unsubscribed_user(mentioned); - assert.equal($("#compose_invite_users").visible(), false); - - // Test mentioning a user that should gets a warning. - - const checks = [ - (function () { - let called; - override(compose, "needs_subscribe_warning", (user_id, stream_id) => { - called = true; - assert.equal(user_id, 34); - assert.equal(stream_id, 111); - return true; - }); - return function () { - assert.ok(called); - }; - })(), - - (function () { - let called; - mock_template("compose_invite_users.hbs", false, (context) => { - called = true; - assert.equal(context.user_id, 34); - assert.equal(context.stream_id, 111); - assert.equal(context.name, "Foo Barson"); - return "fake-compose-invite-user-template"; - }); - return function () { - assert.ok(called); - }; - })(), - - (function () { - let called; - $("#compose_invite_users").append = (html) => { - called = true; - assert.equal(html, "fake-compose-invite-user-template"); - }; - return function () { - assert.ok(called); - }; - })(), - ]; - - mentioned = { - email: "foo@bar.com", - user_id: 34, - full_name: "Foo Barson", - }; - - stream_data.add_sub(sub); - compose.warn_if_mentioning_unsubscribed_user(mentioned); - assert.equal($("#compose_invite_users").visible(), true); - - for (const f of checks) { - f(); - } - - // Simulate that the row was added to the DOM. - const warning_row = $(""); - - let looked_for_existing; - warning_row.data = (field) => { - if (field === "user-id") { - looked_for_existing = true; - return "34"; - } - if (field === "stream-id") { - return "111"; - } - throw new Error(`Unknown field ${field}`); - }; - - const previous_users = $("#compose_invite_users .compose_invite_user"); - previous_users.length = 1; - previous_users[0] = warning_row; - $("#compose_invite_users").hide(); - - // Now try to mention the same person again. The template should - // not render. - compose.warn_if_mentioning_unsubscribed_user(mentioned); - assert.equal($("#compose_invite_users").visible(), true); - assert.ok(looked_for_existing); -}); - test_ui("on_events", ({override}) => { initialize_handlers({override}); diff --git a/frontend_tests/node_tests/compose_validate.js b/frontend_tests/node_tests/compose_validate.js index 8a8dbb5b1b..0bd91de7a2 100644 --- a/frontend_tests/node_tests/compose_validate.js +++ b/frontend_tests/node_tests/compose_validate.js @@ -5,6 +5,7 @@ const {strict: assert} = require("assert"); const {$t_html} = require("../zjsunit/i18n"); const {mock_esm, set_global, zrequire} = require("../zjsunit/namespace"); const {run_test} = require("../zjsunit/test"); +const blueslip = require("../zjsunit/zblueslip"); const $ = require("../zjsunit/zjquery"); const {page_params} = require("../zjsunit/zpage_params"); @@ -20,6 +21,7 @@ const compose_validate = zrequire("compose_validate"); const peer_data = zrequire("peer_data"); const people = zrequire("people"); const settings_config = zrequire("settings_config"); +const settings_data = mock_esm("../../static/js/settings_data"); const stream_data = zrequire("stream_data"); const me = { @@ -523,3 +525,233 @@ test_ui("test_message_overflow", () => { $("#compose-textarea").val("a"); assert.ok(compose_validate.validate()); }); + +test_ui("needs_subscribe_warning", () => { + const invalid_user_id = 999; + + const test_bot = { + full_name: "Test Bot", + email: "test-bot@example.com", + user_id: 135, + is_bot: true, + }; + people.add_active_user(test_bot); + + const sub = { + stream_id: 110, + name: "stream", + }; + + stream_data.add_sub(sub); + peer_data.set_subscribers(sub.stream_id, [bob.user_id, me.user_id]); + + blueslip.expect("error", "Unknown user_id in get_by_user_id: 999"); + // Test with an invalid user id. + assert.equal(compose_validate.needs_subscribe_warning(invalid_user_id, sub.stream_id), false); + + // Test with bot user. + assert.equal(compose_validate.needs_subscribe_warning(test_bot.user_id, sub.stream_id), false); + + // Test when user is subscribed to the stream. + assert.equal(compose_validate.needs_subscribe_warning(bob.user_id, sub.stream_id), false); + + peer_data.remove_subscriber(sub.stream_id, bob.user_id); + // Test when the user is not subscribed. + assert.equal(compose_validate.needs_subscribe_warning(bob.user_id, sub.stream_id), true); +}); + +test_ui("warn_if_private_stream_is_linked", ({mock_template}) => { + const test_sub = { + name: compose_state.stream_name(), + stream_id: 99, + }; + + stream_data.add_sub(test_sub); + peer_data.set_subscribers(test_sub.stream_id, [1, 2]); + + let denmark = { + stream_id: 100, + name: "Denmark", + }; + stream_data.add_sub(denmark); + + peer_data.set_subscribers(denmark.stream_id, [1, 2, 3]); + + function test_noop_case(invite_only) { + compose_state.set_message_type("stream"); + denmark.invite_only = invite_only; + compose_validate.warn_if_private_stream_is_linked(denmark); + assert.equal($("#compose_private_stream_alert").visible(), false); + } + + test_noop_case(false); + // invite_only=true and current compose stream subscribers are a subset + // of mentioned_stream subscribers. + test_noop_case(true); + + $("#compose_private").hide(); + compose_state.set_message_type("stream"); + + const checks = [ + (function () { + let called; + mock_template("compose_private_stream_alert.hbs", false, (context) => { + called = true; + assert.equal(context.stream_name, "Denmark"); + return "fake-compose_private_stream_alert-template"; + }); + return function () { + assert.ok(called); + }; + })(), + + (function () { + let called; + $("#compose_private_stream_alert").append = (html) => { + called = true; + assert.equal(html, "fake-compose_private_stream_alert-template"); + }; + return function () { + assert.ok(called); + }; + })(), + ]; + + denmark = { + invite_only: true, + name: "Denmark", + stream_id: 22, + }; + stream_data.add_sub(denmark); + + compose_validate.warn_if_private_stream_is_linked(denmark); + assert.equal($("#compose_private_stream_alert").visible(), true); + + for (const f of checks) { + f(); + } +}); + +test_ui("warn_if_mentioning_unsubscribed_user", ({override, mock_template}) => { + override(settings_data, "user_can_subscribe_other_users", () => true); + + let mentioned = { + email: "foo@bar.com", + }; + + $("#compose_invite_users .compose_invite_user").length = 0; + + function test_noop_case(is_private, is_zephyr_mirror, is_broadcast) { + const msg_type = is_private ? "private" : "stream"; + compose_state.set_message_type(msg_type); + page_params.realm_is_zephyr_mirror_realm = is_zephyr_mirror; + mentioned.is_broadcast = is_broadcast; + compose_validate.warn_if_mentioning_unsubscribed_user(mentioned); + assert.equal($("#compose_invite_users").visible(), false); + } + + test_noop_case(true, false, false); + test_noop_case(false, true, false); + test_noop_case(false, false, true); + + $("#compose_invite_users").hide(); + compose_state.set_message_type("stream"); + page_params.realm_is_zephyr_mirror_realm = false; + + // Test with empty stream name in compose box. It should return noop. + assert.equal(compose_state.stream_name(), ""); + compose_validate.warn_if_mentioning_unsubscribed_user(mentioned); + assert.equal($("#compose_invite_users").visible(), false); + + compose_state.stream_name("random"); + const sub = { + stream_id: 111, + name: "random", + }; + + // Test with invalid stream in compose box. It should return noop. + compose_validate.warn_if_mentioning_unsubscribed_user(mentioned); + assert.equal($("#compose_invite_users").visible(), false); + + // Test mentioning a user that should gets a warning. + + const checks = [ + (function () { + let called; + override(compose_validate, "needs_subscribe_warning", (user_id, stream_id) => { + called = true; + assert.equal(user_id, 34); + assert.equal(stream_id, 111); + return true; + }); + return function () { + assert.ok(called); + }; + })(), + + (function () { + let called; + mock_template("compose_invite_users.hbs", false, (context) => { + called = true; + assert.equal(context.user_id, 34); + assert.equal(context.stream_id, 111); + assert.equal(context.name, "Foo Barson"); + return "fake-compose-invite-user-template"; + }); + return function () { + assert.ok(called); + }; + })(), + + (function () { + let called; + $("#compose_invite_users").append = (html) => { + called = true; + assert.equal(html, "fake-compose-invite-user-template"); + }; + return function () { + assert.ok(called); + }; + })(), + ]; + + mentioned = { + email: "foo@bar.com", + user_id: 34, + full_name: "Foo Barson", + }; + + stream_data.add_sub(sub); + compose_validate.warn_if_mentioning_unsubscribed_user(mentioned); + assert.equal($("#compose_invite_users").visible(), true); + + for (const f of checks) { + f(); + } + + // Simulate that the row was added to the DOM. + const warning_row = $(""); + + let looked_for_existing; + warning_row.data = (field) => { + if (field === "user-id") { + looked_for_existing = true; + return "34"; + } + if (field === "stream-id") { + return "111"; + } + throw new Error(`Unknown field ${field}`); + }; + + const previous_users = $("#compose_invite_users .compose_invite_user"); + previous_users.length = 1; + previous_users[0] = warning_row; + $("#compose_invite_users").hide(); + + // Now try to mention the same person again. The template should + // not render. + compose_validate.warn_if_mentioning_unsubscribed_user(mentioned); + assert.equal($("#compose_invite_users").visible(), true); + assert.ok(looked_for_existing); +}); diff --git a/frontend_tests/node_tests/composebox_typeahead.js b/frontend_tests/node_tests/composebox_typeahead.js index 62b98dd6b1..46c7429ec7 100644 --- a/frontend_tests/node_tests/composebox_typeahead.js +++ b/frontend_tests/node_tests/composebox_typeahead.js @@ -37,6 +37,7 @@ set_global("document", "document-stub"); const emoji = zrequire("../shared/js/emoji"); const typeahead = zrequire("../shared/js/typeahead"); const compose_state = zrequire("compose_state"); +const compose_validate = zrequire("compose_validate"); const typeahead_helper = zrequire("typeahead_helper"); const muted_users = zrequire("muted_users"); const people = zrequire("people"); @@ -375,7 +376,7 @@ test("content_typeahead_selected", ({override}) => { // mention fake_this.completing = "mention"; - override(compose, "warn_if_mentioning_unsubscribed_user", () => {}); + override(compose_validate, "warn_if_mentioning_unsubscribed_user", () => {}); fake_this.query = "@**Mark Tw"; fake_this.token = "Mark Tw"; @@ -384,7 +385,7 @@ test("content_typeahead_selected", ({override}) => { assert.equal(actual_value, expected_value); let warned_for_mention = false; - override(compose, "warn_if_mentioning_unsubscribed_user", (mentioned) => { + override(compose_validate, "warn_if_mentioning_unsubscribed_user", (mentioned) => { assert.equal(mentioned, othello); warned_for_mention = true; }); @@ -416,7 +417,7 @@ test("content_typeahead_selected", ({override}) => { fake_this.query = "@back"; fake_this.token = "back"; - with_field(compose, "warn_if_mentioning_unsubscribed_user", unexpected_warn, () => { + with_field(compose_validate, "warn_if_mentioning_unsubscribed_user", unexpected_warn, () => { actual_value = ct.content_typeahead_selected.call(fake_this, backend); }); expected_value = "@*Backend* "; @@ -436,7 +437,7 @@ test("content_typeahead_selected", ({override}) => { fake_this.query = "@_kin"; fake_this.token = "kin"; - with_field(compose, "warn_if_mentioning_unsubscribed_user", unexpected_warn, () => { + with_field(compose_validate, "warn_if_mentioning_unsubscribed_user", unexpected_warn, () => { actual_value = ct.content_typeahead_selected.call(fake_this, hamlet); }); @@ -463,7 +464,7 @@ test("content_typeahead_selected", ({override}) => { fake_this.query = "@_back"; fake_this.token = "back"; - with_field(compose, "warn_if_mentioning_unsubscribed_user", unexpected_warn, () => { + with_field(compose_validate, "warn_if_mentioning_unsubscribed_user", unexpected_warn, () => { actual_value = ct.content_typeahead_selected.call(fake_this, backend); }); expected_value = "@_*Backend* "; @@ -484,7 +485,7 @@ test("content_typeahead_selected", ({override}) => { // stream fake_this.completing = "stream"; let warned_for_stream_link = false; - override(compose, "warn_if_private_stream_is_linked", (linked_stream) => { + override(compose_validate, "warn_if_private_stream_is_linked", (linked_stream) => { assert.equal(linked_stream, sweden_stream); warned_for_stream_link = true; }); diff --git a/static/js/compose.js b/static/js/compose.js index e2ad60fb14..4256dd8fe6 100644 --- a/static/js/compose.js +++ b/static/js/compose.js @@ -2,8 +2,6 @@ import $ from "jquery"; import _ from "lodash"; import render_compose from "../templates/compose.hbs"; -import render_compose_invite_users from "../templates/compose_invite_users.hbs"; -import render_compose_private_stream_alert from "../templates/compose_private_stream_alert.hbs"; import * as blueslip from "./blueslip"; import * as channel from "./channel"; @@ -21,7 +19,6 @@ import * as loading from "./loading"; import * as markdown from "./markdown"; import * as notifications from "./notifications"; import {page_params} from "./page_params"; -import * as peer_data from "./peer_data"; import * as people from "./people"; import * as reminder from "./reminder"; import * as rendered_markdown from "./rendered_markdown"; @@ -29,7 +26,6 @@ import * as resize from "./resize"; import * as rows from "./rows"; import * as sent_messages from "./sent_messages"; import * as server_events from "./server_events"; -import * as settings_data from "./settings_data"; import * as stream_data from "./stream_data"; import * as stream_edit from "./stream_edit"; import * as stream_settings_ui from "./stream_settings_ui"; @@ -335,40 +331,6 @@ export function update_email(user_id, new_email) { compose_state.private_message_recipient(reply_to); } -export function needs_subscribe_warning(user_id, stream_id) { - // This returns true if all of these conditions are met: - // * the user is valid - // * the user is not already subscribed to the stream - // * the user has no back-door way to see stream messages - // (i.e. bots on public/private streams) - // - // You can think of this as roughly answering "is there an - // actionable way to subscribe the user and do they actually - // need it?". - // - // We expect the caller to already have verified that we're - // sending to a valid stream and trying to mention the user. - - const user = people.get_by_user_id(user_id); - - if (!user) { - return false; - } - - if (user.is_bot) { - // Bots may receive messages on public/private streams even if they are - // not subscribed. - return false; - } - - if (stream_data.is_user_subscribed(stream_id, user_id)) { - // If our user is already subscribed - return false; - } - - return true; -} - function insert_video_call_url(url, target_textarea) { const link_text = $t({defaultMessage: "Click to join video call"}); compose_ui.insert_syntax_and_focus(`[${link_text}](${url})`, target_textarea); @@ -434,108 +396,6 @@ export function render_and_show_preview(preview_spinner, preview_content_box, co } } -export function warn_if_private_stream_is_linked(linked_stream) { - // For PMs, we currently don't warn about links to private - // streams, since you are specifically sharing the existence of - // the private stream with someone. One could imagine changing - // this policy if user feedback suggested it was useful. - if (compose_state.get_message_type() !== "stream") { - return; - } - - const compose_stream = stream_data.get_sub(compose_state.stream_name()); - if (compose_stream === undefined) { - // We have an invalid stream name, don't warn about this here as - // we show an error to the user when they try to send the message. - return; - } - - // If the stream we're linking to is not invite-only, then it's - // public, and there is no need to warn about it, since all - // members can already see all the public streams. - // - // Theoretically, we could still do a warning if there are any - // guest users subscribed to the stream we're posting to; we may - // change this policy if user feedback suggests it'd be an - // improvement. - if (!linked_stream.invite_only) { - return; - } - - // Don't warn if subscribers list of current compose_stream is - // a subset of linked_stream's subscribers list, because - // everyone will be subscribed to the linked stream and so - // knows it exists. (But always warn Zephyr users, since - // we may not know their stream's subscribers.) - if ( - peer_data.is_subscriber_subset(compose_stream.stream_id, linked_stream.stream_id) && - !page_params.realm_is_zephyr_mirror_realm - ) { - return; - } - - const stream_name = linked_stream.name; - - const warning_area = $("#compose_private_stream_alert"); - const context = {stream_name}; - const new_row = render_compose_private_stream_alert(context); - - warning_area.append(new_row); - warning_area.show(); -} - -export function warn_if_mentioning_unsubscribed_user(mentioned) { - if (compose_state.get_message_type() !== "stream") { - return; - } - - // Disable for Zephyr mirroring realms, since we never have subscriber lists there - if (page_params.realm_is_zephyr_mirror_realm) { - return; - } - - const user_id = mentioned.user_id; - - if (mentioned.is_broadcast) { - return; // don't check if @all/@everyone/@stream - } - - const stream_name = compose_state.stream_name(); - - if (!stream_name) { - return; - } - - const sub = stream_data.get_sub(stream_name); - - if (!sub) { - return; - } - - if (needs_subscribe_warning(user_id, sub.stream_id)) { - const error_area = $("#compose_invite_users"); - const existing_invites_area = $("#compose_invite_users .compose_invite_user"); - - const existing_invites = Array.from($(existing_invites_area), (user_row) => - Number.parseInt($(user_row).data("user-id"), 10), - ); - - if (!existing_invites.includes(user_id)) { - const context = { - user_id, - stream_id: sub.stream_id, - name: mentioned.full_name, - can_subscribe_other_users: settings_data.user_can_subscribe_other_users(), - }; - - const new_row = render_compose_invite_users(context); - error_area.append(new_row); - } - - error_area.show(); - } -} - export function render_compose_box() { $("#compose-container").append( render_compose({ diff --git a/static/js/compose_validate.js b/static/js/compose_validate.js index 3ef6f2844b..aa06c51fd9 100644 --- a/static/js/compose_validate.js +++ b/static/js/compose_validate.js @@ -2,7 +2,9 @@ import $ from "jquery"; import render_compose_all_everyone from "../templates/compose_all_everyone.hbs"; import render_compose_announce from "../templates/compose_announce.hbs"; +import render_compose_invite_users from "../templates/compose_invite_users.hbs"; import render_compose_not_subscribed from "../templates/compose_not_subscribed.hbs"; +import render_compose_private_stream_alert from "../templates/compose_private_stream_alert.hbs"; import * as channel from "./channel"; import * as compose_error from "./compose_error"; @@ -13,6 +15,7 @@ import {page_params} from "./page_params"; import * as peer_data from "./peer_data"; import * as people from "./people"; import * as settings_config from "./settings_config"; +import * as settings_data from "./settings_data"; import * as stream_data from "./stream_data"; import * as util from "./util"; @@ -23,6 +26,142 @@ let wildcard_mention; export const announce_warn_threshold = 60; export let wildcard_mention_large_stream_threshold = 15; +export function needs_subscribe_warning(user_id, stream_id) { + // This returns true if all of these conditions are met: + // * the user is valid + // * the user is not already subscribed to the stream + // * the user has no back-door way to see stream messages + // (i.e. bots on public/private streams) + // + // You can think of this as roughly answering "is there an + // actionable way to subscribe the user and do they actually + // need it?". + // + // We expect the caller to already have verified that we're + // sending to a valid stream and trying to mention the user. + + const user = people.get_by_user_id(user_id); + + if (!user) { + return false; + } + + if (user.is_bot) { + // Bots may receive messages on public/private streams even if they are + // not subscribed. + return false; + } + + if (stream_data.is_user_subscribed(stream_id, user_id)) { + // If our user is already subscribed + return false; + } + + return true; +} + +export function warn_if_private_stream_is_linked(linked_stream) { + // For PMs, we currently don't warn about links to private + // streams, since you are specifically sharing the existence of + // the private stream with someone. One could imagine changing + // this policy if user feedback suggested it was useful. + if (compose_state.get_message_type() !== "stream") { + return; + } + + const compose_stream = stream_data.get_sub(compose_state.stream_name()); + if (compose_stream === undefined) { + // We have an invalid stream name, don't warn about this here as + // we show an error to the user when they try to send the message. + return; + } + + // If the stream we're linking to is not invite-only, then it's + // public, and there is no need to warn about it, since all + // members can already see all the public streams. + // + // Theoretically, we could still do a warning if there are any + // guest users subscribed to the stream we're posting to; we may + // change this policy if user feedback suggests it'd be an + // improvement. + if (!linked_stream.invite_only) { + return; + } + + // Don't warn if subscribers list of current compose_stream is + // a subset of linked_stream's subscribers list, because + // everyone will be subscribed to the linked stream and so + // knows it exists. (But always warn Zephyr users, since + // we may not know their stream's subscribers.) + if ( + peer_data.is_subscriber_subset(compose_stream.stream_id, linked_stream.stream_id) && + !page_params.realm_is_zephyr_mirror_realm + ) { + return; + } + + const stream_name = linked_stream.name; + + const warning_area = $("#compose_private_stream_alert"); + const context = {stream_name}; + const new_row = render_compose_private_stream_alert(context); + + warning_area.append(new_row); + warning_area.show(); +} + +export function warn_if_mentioning_unsubscribed_user(mentioned) { + if (compose_state.get_message_type() !== "stream") { + return; + } + + // Disable for Zephyr mirroring realms, since we never have subscriber lists there + if (page_params.realm_is_zephyr_mirror_realm) { + return; + } + + const user_id = mentioned.user_id; + + if (mentioned.is_broadcast) { + return; // don't check if @all/@everyone/@stream + } + + const stream_name = compose_state.stream_name(); + + if (!stream_name) { + return; + } + + const sub = stream_data.get_sub(stream_name); + + if (!sub) { + return; + } + + if (needs_subscribe_warning(user_id, sub.stream_id)) { + const error_area = $("#compose_invite_users"); + const existing_invites_area = $("#compose_invite_users .compose_invite_user"); + + const existing_invites = Array.from($(existing_invites_area), (user_row) => + Number.parseInt($(user_row).data("user-id"), 10), + ); + + if (!existing_invites.includes(user_id)) { + const context = { + user_id, + stream_id: sub.stream_id, + name: mentioned.full_name, + can_subscribe_other_users: settings_data.user_can_subscribe_other_users(), + }; + + const new_row = render_compose_invite_users(context); + error_area.append(new_row); + } + + error_area.show(); + } +} + function show_all_everyone_warnings(stream_id) { const stream_count = peer_data.get_subscriber_count(stream_id) || 0; diff --git a/static/js/composebox_typeahead.js b/static/js/composebox_typeahead.js index f649f02c28..7aae1fda27 100644 --- a/static/js/composebox_typeahead.js +++ b/static/js/composebox_typeahead.js @@ -844,7 +844,7 @@ export function content_typeahead_selected(item, event) { ); beginning += mention_text + " "; if (!is_silent) { - compose.warn_if_mentioning_unsubscribed_user(item); + compose_validate.warn_if_mentioning_unsubscribed_user(item); } } break; @@ -866,7 +866,7 @@ export function content_typeahead_selected(item, event) { } else { beginning += "** "; } - compose.warn_if_private_stream_is_linked(item); + compose_validate.warn_if_private_stream_is_linked(item); break; case "syntax": { // Isolate the end index of the triple backticks/tildes, including