From 6ef08873d813ae4f86ce95756d11c23719e80f33 Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Fri, 6 Oct 2023 13:36:26 -0700 Subject: [PATCH] notifications: Split out compose_notifications module. Signed-off-by: Anders Kaseorg --- tools/test-js-with-node | 1 + web/src/compose_notifications.js | 267 ++++++++++++++++++++++++++++++ web/src/echo.js | 4 +- web/src/message_events.js | 6 +- web/src/notifications.js | 270 +------------------------------ web/src/ui_init.js | 9 +- web/tests/echo.test.js | 4 +- 7 files changed, 284 insertions(+), 277 deletions(-) create mode 100644 web/src/compose_notifications.js diff --git a/tools/test-js-with-node b/tools/test-js-with-node index 623a51c662..d959e2a579 100755 --- a/tools/test-js-with-node +++ b/tools/test-js-with-node @@ -67,6 +67,7 @@ EXEMPT_FILES = make_set( "web/src/compose_call_ui.js", "web/src/compose_closed_ui.js", "web/src/compose_fade.js", + "web/src/compose_notifications.js", "web/src/compose_popovers.js", "web/src/compose_recipient.js", "web/src/compose_reply.js", diff --git a/web/src/compose_notifications.js b/web/src/compose_notifications.js new file mode 100644 index 0000000000..7bc282ab1f --- /dev/null +++ b/web/src/compose_notifications.js @@ -0,0 +1,267 @@ +import $ from "jquery"; + +import render_message_sent_banner from "../templates/compose_banner/message_sent_banner.hbs"; +import render_unmute_topic_banner from "../templates/compose_banner/unmute_topic_banner.hbs"; + +import * as blueslip from "./blueslip"; +import * as compose_banner from "./compose_banner"; +import * as hash_util from "./hash_util"; +import {$t} from "./i18n"; +import * as message_lists from "./message_lists"; +import * as narrow_state from "./narrow_state"; +import * as people from "./people"; +import * as stream_data from "./stream_data"; +import * as user_topics from "./user_topics"; + +function notify_unmute(muted_narrow, stream_id, topic_name) { + const $unmute_notification = $( + render_unmute_topic_banner({ + muted_narrow, + stream_id, + topic_name, + classname: compose_banner.CLASSNAMES.unmute_topic_notification, + banner_type: "", + button_text: $t({defaultMessage: "Unmute topic"}), + }), + ); + compose_banner.clear_unmute_topic_notifications(); + compose_banner.append_compose_banner_to_banner_list( + $unmute_notification, + $("#compose_banners"), + ); +} + +export function notify_above_composebox( + banner_text, + classname, + above_composebox_narrow_url, + link_msg_id, + link_text, +) { + const $notification = $( + render_message_sent_banner({ + banner_text, + classname, + above_composebox_narrow_url, + link_msg_id, + link_text, + }), + ); + // We pass in include_unmute_banner as false because we don't want to + // clear any unmute_banner associated with this same message. + compose_banner.clear_message_sent_banners(false); + compose_banner.append_compose_banner_to_banner_list($notification, $("#compose_banners")); +} + +// Note that this returns values that are not HTML-escaped, for use in +// Handlebars templates that will do further escaping. +function get_message_header(message) { + if (message.type === "stream") { + const stream_name = stream_data.get_stream_name_from_id(message.stream_id); + return `#${stream_name} > ${message.topic}`; + } + if (message.display_recipient.length > 2) { + return $t( + {defaultMessage: "group direct messages with {recipient}"}, + {recipient: message.display_reply_to}, + ); + } + if (people.is_current_user(message.reply_to)) { + return $t({defaultMessage: "direct messages with yourself"}); + } + return $t( + {defaultMessage: "direct messages with {recipient}"}, + {recipient: message.display_reply_to}, + ); +} + +export function get_muted_narrow(message) { + if ( + message.type === "stream" && + stream_data.is_muted(message.stream_id) && + !user_topics.is_topic_unmuted_or_followed(message.stream_id, message.topic) + ) { + return "stream"; + } + if (message.type === "stream" && user_topics.is_topic_muted(message.stream_id, message.topic)) { + return "topic"; + } + return undefined; +} + +export function get_local_notify_mix_reason(message) { + const $row = message_lists.current.get_row(message.id); + if ($row.length > 0) { + // If our message is in the current message list, we do + // not have a mix, so we are happy. + return undefined; + } + + // offscreen because it is outside narrow + // we can only look for these on non-search (can_apply_locally) messages + // see also: exports.notify_messages_outside_current_search + const current_filter = narrow_state.filter(); + if ( + current_filter && + current_filter.can_apply_locally() && + !current_filter.predicate()(message) + ) { + return $t({defaultMessage: "Sent! Your message is outside your current narrow."}); + } + + return undefined; +} + +export function notify_local_mixes(messages, need_user_to_scroll) { + /* + This code should only be called when we are displaying + messages sent by current client. It notifies users that + their messages aren't actually in the view that they + composed to. + + This code is called after we insert messages into our + message list widgets. All of the conditions here are + checkable locally, so we may want to execute this code + earlier in the codepath at some point and possibly punt + on local rendering. + + Possible cleanup: Arguably, we should call this function + unconditionally and just check if message.local_id is in + sent_messages.messages here. + */ + + for (const message of messages) { + if (!people.is_my_user_id(message.sender_id)) { + // This can happen if the client is offline for a while + // around the time this client sends a message; see the + // caller of message_events.insert_new_messages. + blueslip.info( + "Slightly unexpected: A message not sent by us batches with those that were.", + ); + continue; + } + + const muted_narrow = get_muted_narrow(message); + if (muted_narrow) { + notify_unmute(muted_narrow, message.stream_id, message.topic); + // We don't `continue` after showing the unmute banner, allowing multiple + // banners (at max 2 including the unmute banner) to be shown at once, + // as it's common for the unmute case to occur simultaneously with + // another banner's case, like sending a message to another narrow. + } + + let banner_text = get_local_notify_mix_reason(message); + + const link_msg_id = message.id; + + if (!banner_text) { + if (need_user_to_scroll) { + banner_text = $t({defaultMessage: "Sent!"}); + const link_text = $t({defaultMessage: "Scroll down to view your message."}); + notify_above_composebox( + banner_text, + compose_banner.CLASSNAMES.sent_scroll_to_view, + // Don't display a URL on hover for the "Scroll to bottom" link. + null, + link_msg_id, + link_text, + ); + compose_banner.set_scroll_to_message_banner_message_id(link_msg_id); + } + + // This is the HAPPY PATH--for most messages we do nothing + // other than maybe sending the above message. + continue; + } + + const link_text = $t( + {defaultMessage: "Narrow to {message_recipient}"}, + {message_recipient: get_message_header(message)}, + ); + + notify_above_composebox( + banner_text, + compose_banner.CLASSNAMES.narrow_to_recipient, + get_above_composebox_narrow_url(message), + link_msg_id, + link_text, + ); + } +} + +function get_above_composebox_narrow_url(message) { + let above_composebox_narrow_url; + if (message.type === "stream") { + above_composebox_narrow_url = hash_util.by_stream_topic_url( + message.stream_id, + message.topic, + ); + } else { + above_composebox_narrow_url = message.pm_with_url; + } + return above_composebox_narrow_url; +} + +// for callback when we have to check with the server if a message should be in +// the message_lists.current (!can_apply_locally; a.k.a. "a search"). +export function notify_messages_outside_current_search(messages) { + for (const message of messages) { + if (!people.is_current_user(message.sender_email)) { + continue; + } + const above_composebox_narrow_url = get_above_composebox_narrow_url(message); + const link_text = $t( + {defaultMessage: "Narrow to {message_recipient}"}, + {message_recipient: get_message_header(message)}, + ); + notify_above_composebox( + $t({defaultMessage: "Sent! Your recent message is outside the current search."}), + compose_banner.CLASSNAMES.narrow_to_recipient, + above_composebox_narrow_url, + message.id, + link_text, + ); + } +} + +export function reify_message_id(opts) { + const old_id = opts.old_id; + const new_id = opts.new_id; + + // If a message ID that we're currently storing (as a link) has changed, + // update that link as well + for (const e of $("#compose_banners a")) { + const $elem = $(e); + const message_id = $elem.data("message-id"); + + if (message_id === old_id) { + $elem.data("message-id", new_id); + compose_banner.set_scroll_to_message_banner_message_id(new_id); + } + } +} + +export function initialize({on_click_scroll_to_selected, on_narrow_to_recipient}) { + $("#compose_banners").on( + "click", + ".narrow_to_recipient .above_compose_banner_action_link", + (e) => { + const message_id = $(e.currentTarget).data("message-id"); + on_narrow_to_recipient(message_id); + e.stopPropagation(); + e.preventDefault(); + }, + ); + $("#compose_banners").on( + "click", + ".sent_scroll_to_view .above_compose_banner_action_link", + (e) => { + const message_id = $(e.currentTarget).data("message-id"); + message_lists.current.select_id(message_id); + on_click_scroll_to_selected(); + compose_banner.clear_message_sent_banners(false); + e.stopPropagation(); + e.preventDefault(); + }, + ); +} diff --git a/web/src/echo.js b/web/src/echo.js index 8d903b805e..ea2f9316b7 100644 --- a/web/src/echo.js +++ b/web/src/echo.js @@ -3,6 +3,7 @@ import $ from "jquery"; import * as alert_words from "./alert_words"; import {all_messages_data} from "./all_messages_data"; import * as blueslip from "./blueslip"; +import * as compose_notifications from "./compose_notifications"; import * as compose_ui from "./compose_ui"; import * as drafts from "./drafts"; import * as local_message from "./local_message"; @@ -11,7 +12,6 @@ import * as message_lists from "./message_lists"; import * as message_live_update from "./message_live_update"; import * as message_store from "./message_store"; import * as narrow_state from "./narrow_state"; -import * as notifications from "./notifications"; import {page_params} from "./page_params"; import * as people from "./people"; import * as pm_list from "./pm_list"; @@ -363,7 +363,7 @@ export function reify_message_id(local_id, server_id) { message_store.reify_message_id(opts); update_message_lists(opts); - notifications.reify_message_id(opts); + compose_notifications.reify_message_id(opts); recent_view_data.reify_message_id_if_available(opts); } diff --git a/web/src/message_events.js b/web/src/message_events.js index fb805b30b3..5567f1659e 100644 --- a/web/src/message_events.js +++ b/web/src/message_events.js @@ -5,6 +5,7 @@ import {all_messages_data} from "./all_messages_data"; import * as blueslip from "./blueslip"; import * as channel from "./channel"; import * as compose_fade from "./compose_fade"; +import * as compose_notifications from "./compose_notifications"; import * as compose_state from "./compose_state"; import * as compose_validate from "./compose_validate"; import * as drafts from "./drafts"; @@ -18,7 +19,6 @@ import * as message_store from "./message_store"; import * as message_util from "./message_util"; import * as narrow from "./narrow"; import * as narrow_state from "./narrow_state"; -import * as notifications from "./notifications"; import {page_params} from "./page_params"; import * as pm_list from "./pm_list"; import * as recent_senders from "./recent_senders"; @@ -80,7 +80,7 @@ function maybe_add_narrowed_messages(messages, msg_list, callback, attempt = 1) callback(new_messages, msg_list); unread_ops.process_visible(); - notifications.notify_messages_outside_current_search(elsewhere_messages); + compose_notifications.notify_messages_outside_current_search(elsewhere_messages); }, error(xhr) { if (!narrow_state.is_message_feed_visible() || msg_list !== message_lists.current) { @@ -154,7 +154,7 @@ export function insert_new_messages(messages, sent_by_this_client) { // were sent by this client; notifications.notify_local_mixes // will filter out any not sent by us. if (sent_by_this_client) { - notifications.notify_local_mixes(messages, need_user_to_scroll); + compose_notifications.notify_local_mixes(messages, need_user_to_scroll); } if (any_untracked_unread_messages) { diff --git a/web/src/notifications.js b/web/src/notifications.js index cf5a7753e4..154bc98769 100644 --- a/web/src/notifications.js +++ b/web/src/notifications.js @@ -1,19 +1,6 @@ import $ from "jquery"; -import render_message_sent_banner from "../templates/compose_banner/message_sent_banner.hbs"; -import render_unmute_topic_banner from "../templates/compose_banner/unmute_topic_banner.hbs"; - -import * as blueslip from "./blueslip"; -import * as compose_banner from "./compose_banner"; -import * as hash_util from "./hash_util"; -import {$t} from "./i18n"; -import * as message_lists from "./message_lists"; -import * as narrow from "./narrow"; -import * as narrow_state from "./narrow_state"; -import * as people from "./people"; -import * as stream_data from "./stream_data"; import {user_settings} from "./user_settings"; -import * as user_topics from "./user_topics"; export const notice_memory = new Map(); @@ -56,7 +43,7 @@ export function get_notifications() { return notice_memory; } -export function initialize({on_click_scroll_to_selected}) { +export function initialize() { $(window).on("focus", () => { for (const notice_mem_entry of notice_memory.values()) { notice_mem_entry.obj.close(); @@ -65,8 +52,6 @@ export function initialize({on_click_scroll_to_selected}) { }); update_notification_sound_source($("#user-notification-sound-audio"), user_settings); - - register_click_handlers({on_click_scroll_to_selected}); } export function update_notification_sound_source(container_elem, settings_object) { @@ -95,46 +80,6 @@ export function permission_state() { return NotificationAPI.permission; } -function notify_unmute(muted_narrow, stream_id, topic_name) { - const $unmute_notification = $( - render_unmute_topic_banner({ - muted_narrow, - stream_id, - topic_name, - classname: compose_banner.CLASSNAMES.unmute_topic_notification, - banner_type: "", - button_text: $t({defaultMessage: "Unmute topic"}), - }), - ); - compose_banner.clear_unmute_topic_notifications(); - compose_banner.append_compose_banner_to_banner_list( - $unmute_notification, - $("#compose_banners"), - ); -} - -export function notify_above_composebox( - banner_text, - classname, - above_composebox_narrow_url, - link_msg_id, - link_text, -) { - const $notification = $( - render_message_sent_banner({ - banner_text, - classname, - above_composebox_narrow_url, - link_msg_id, - link_text, - }), - ); - // We pass in include_unmute_banner as false because we don't want to - // clear any unmute_banner associated with this same message. - compose_banner.clear_message_sent_banners(false); - compose_banner.append_compose_banner_to_banner_list($notification, $("#compose_banners")); -} - export function close_notification(message) { for (const [key, notice_mem_entry] of notice_memory) { if (notice_mem_entry.message_id === message.id) { @@ -153,216 +98,3 @@ export function request_desktop_notifications_permission() { NotificationAPI.requestPermission(); } } - -// Note that this returns values that are not HTML-escaped, for use in -// Handlebars templates that will do further escaping. -function get_message_header(message) { - if (message.type === "stream") { - const stream_name = stream_data.get_stream_name_from_id(message.stream_id); - return `#${stream_name} > ${message.topic}`; - } - if (message.display_recipient.length > 2) { - return $t( - {defaultMessage: "group direct messages with {recipient}"}, - {recipient: message.display_reply_to}, - ); - } - if (people.is_current_user(message.reply_to)) { - return $t({defaultMessage: "direct messages with yourself"}); - } - return $t( - {defaultMessage: "direct messages with {recipient}"}, - {recipient: message.display_reply_to}, - ); -} - -export function get_muted_narrow(message) { - if ( - message.type === "stream" && - stream_data.is_muted(message.stream_id) && - !user_topics.is_topic_unmuted_or_followed(message.stream_id, message.topic) - ) { - return "stream"; - } - if (message.type === "stream" && user_topics.is_topic_muted(message.stream_id, message.topic)) { - return "topic"; - } - return undefined; -} - -export function get_local_notify_mix_reason(message) { - const $row = message_lists.current.get_row(message.id); - if ($row.length > 0) { - // If our message is in the current message list, we do - // not have a mix, so we are happy. - return undefined; - } - - // offscreen because it is outside narrow - // we can only look for these on non-search (can_apply_locally) messages - // see also: exports.notify_messages_outside_current_search - const current_filter = narrow_state.filter(); - if ( - current_filter && - current_filter.can_apply_locally() && - !current_filter.predicate()(message) - ) { - return $t({defaultMessage: "Sent! Your message is outside your current narrow."}); - } - - return undefined; -} - -export function notify_local_mixes(messages, need_user_to_scroll) { - /* - This code should only be called when we are displaying - messages sent by current client. It notifies users that - their messages aren't actually in the view that they - composed to. - - This code is called after we insert messages into our - message list widgets. All of the conditions here are - checkable locally, so we may want to execute this code - earlier in the codepath at some point and possibly punt - on local rendering. - - Possible cleanup: Arguably, we should call this function - unconditionally and just check if message.local_id is in - sent_messages.messages here. - */ - - for (const message of messages) { - if (!people.is_my_user_id(message.sender_id)) { - // This can happen if the client is offline for a while - // around the time this client sends a message; see the - // caller of message_events.insert_new_messages. - blueslip.info( - "Slightly unexpected: A message not sent by us batches with those that were.", - ); - continue; - } - - const muted_narrow = get_muted_narrow(message); - if (muted_narrow) { - notify_unmute(muted_narrow, message.stream_id, message.topic); - // We don't `continue` after showing the unmute banner, allowing multiple - // banners (at max 2 including the unmute banner) to be shown at once, - // as it's common for the unmute case to occur simultaneously with - // another banner's case, like sending a message to another narrow. - } - - let banner_text = get_local_notify_mix_reason(message); - - const link_msg_id = message.id; - - if (!banner_text) { - if (need_user_to_scroll) { - banner_text = $t({defaultMessage: "Sent!"}); - const link_text = $t({defaultMessage: "Scroll down to view your message."}); - notify_above_composebox( - banner_text, - compose_banner.CLASSNAMES.sent_scroll_to_view, - // Don't display a URL on hover for the "Scroll to bottom" link. - null, - link_msg_id, - link_text, - ); - compose_banner.set_scroll_to_message_banner_message_id(link_msg_id); - } - - // This is the HAPPY PATH--for most messages we do nothing - // other than maybe sending the above message. - continue; - } - - const link_text = $t( - {defaultMessage: "Narrow to {message_recipient}"}, - {message_recipient: get_message_header(message)}, - ); - - notify_above_composebox( - banner_text, - compose_banner.CLASSNAMES.narrow_to_recipient, - get_above_composebox_narrow_url(message), - link_msg_id, - link_text, - ); - } -} - -function get_above_composebox_narrow_url(message) { - let above_composebox_narrow_url; - if (message.type === "stream") { - above_composebox_narrow_url = hash_util.by_stream_topic_url( - message.stream_id, - message.topic, - ); - } else { - above_composebox_narrow_url = message.pm_with_url; - } - return above_composebox_narrow_url; -} - -// for callback when we have to check with the server if a message should be in -// the message_lists.current (!can_apply_locally; a.k.a. "a search"). -export function notify_messages_outside_current_search(messages) { - for (const message of messages) { - if (!people.is_current_user(message.sender_email)) { - continue; - } - const above_composebox_narrow_url = get_above_composebox_narrow_url(message); - const link_text = $t( - {defaultMessage: "Narrow to {message_recipient}"}, - {message_recipient: get_message_header(message)}, - ); - notify_above_composebox( - $t({defaultMessage: "Sent! Your recent message is outside the current search."}), - compose_banner.CLASSNAMES.narrow_to_recipient, - above_composebox_narrow_url, - message.id, - link_text, - ); - } -} - -export function reify_message_id(opts) { - const old_id = opts.old_id; - const new_id = opts.new_id; - - // If a message ID that we're currently storing (as a link) has changed, - // update that link as well - for (const e of $("#compose_banners a")) { - const $elem = $(e); - const message_id = $elem.data("message-id"); - - if (message_id === old_id) { - $elem.data("message-id", new_id); - compose_banner.set_scroll_to_message_banner_message_id(new_id); - } - } -} - -export function register_click_handlers({on_click_scroll_to_selected}) { - $("#compose_banners").on( - "click", - ".narrow_to_recipient .above_compose_banner_action_link", - (e) => { - const message_id = $(e.currentTarget).data("message-id"); - narrow.by_topic(message_id, {trigger: "notification"}); - e.stopPropagation(); - e.preventDefault(); - }, - ); - $("#compose_banners").on( - "click", - ".sent_scroll_to_view .above_compose_banner_action_link", - (e) => { - const message_id = $(e.currentTarget).data("message-id"); - message_lists.current.select_id(message_id); - on_click_scroll_to_selected(); - compose_banner.clear_message_sent_banners(false); - e.stopPropagation(); - e.preventDefault(); - }, - ); -} diff --git a/web/src/ui_init.js b/web/src/ui_init.js index 50507a6554..52170c8501 100644 --- a/web/src/ui_init.js +++ b/web/src/ui_init.js @@ -23,6 +23,7 @@ import * as click_handlers from "./click_handlers"; import * as common from "./common"; import * as compose from "./compose"; import * as compose_closed_ui from "./compose_closed_ui"; +import * as compose_notifications from "./compose_notifications"; import * as compose_pm_pill from "./compose_pm_pill"; import * as compose_popovers from "./compose_popovers"; import * as compose_recipient from "./compose_recipient"; @@ -668,7 +669,13 @@ export function initialize_everything() { on_narrow_search: narrow.activate, }); tutorial.initialize(); - notifications.initialize({on_click_scroll_to_selected: navigate.scroll_to_selected}); + notifications.initialize(); + compose_notifications.initialize({ + on_click_scroll_to_selected: navigate.scroll_to_selected, + on_narrow_to_recipient(message_id) { + narrow.by_topic(message_id, {trigger: "compose_notification"}); + }, + }); unread_ops.initialize(); gear_menu.initialize(); giphy.initialize(); diff --git a/web/tests/echo.test.js b/web/tests/echo.test.js index fa4f3144d8..a89bfa1701 100644 --- a/web/tests/echo.test.js +++ b/web/tests/echo.test.js @@ -9,9 +9,9 @@ const {make_stub} = require("./lib/stub"); const {run_test} = require("./lib/test"); const {page_params} = require("./lib/zpage_params"); +const compose_notifications = mock_esm("../src/compose_notifications"); const markdown = mock_esm("../src/markdown"); const message_lists = mock_esm("../src/message_lists"); -const notifications = mock_esm("../src/notifications"); let disparities = []; @@ -328,7 +328,7 @@ run_test("test reify_message_id", ({override}) => { message_store_reify_called = true; }); - override(notifications, "reify_message_id", () => { + override(compose_notifications, "reify_message_id", () => { notifications_reify_called = true; });