diff --git a/frontend_tests/node_tests/notifications.js b/frontend_tests/node_tests/notifications.js index 9ce83e0e4a..cadec04891 100644 --- a/frontend_tests/node_tests/notifications.js +++ b/frontend_tests/node_tests/notifications.js @@ -59,6 +59,7 @@ function test(label, f) { page_params.enable_desktop_notifications = true; page_params.enable_sounds = true; page_params.wildcard_mentions_notify = true; + page_params.notification_sound = "ding"; f(override); }); } @@ -239,13 +240,35 @@ test("message_is_notifiable", () => { assert.equal(notifications.should_send_audible_notification(message), true); assert.equal(notifications.message_is_notifiable(message), false); + // Case 9: If `None` is selected as the notification sound, send no + // audible notification, no matter what other user configurations are. + message = { + id: 50, + content: "message number 7", + sent_by_me: false, + notification_sent: false, + mentioned: true, + mentioned_me_directly: true, + type: "stream", + stream: "general", + stream_id: general.stream_id, + topic: "whatever", + }; + page_params.notification_sound = "none"; + assert.equal(notifications.should_send_desktop_notification(message), true); + assert.equal(notifications.should_send_audible_notification(message), false); + assert.equal(notifications.message_is_notifiable(message), true); + + // Reset state + page_params.notification_sound = "ding"; + // If none of the above cases apply // (ie: topic is not muted, message does not mention user, // no notification sent before, message not sent by user), // return true to pass it to notifications settings, which will return false. message = { id: 60, - content: "message number 7", + content: "message number 8", sent_by_me: false, notification_sent: false, mentioned: false, diff --git a/static/js/notifications.js b/static/js/notifications.js index 664472f487..f3685a4d1f 100644 --- a/static/js/notifications.js +++ b/static/js/notifications.js @@ -91,14 +91,16 @@ export function initialize() { } function update_notification_sound_source() { - const audio_file_without_extension = - "/static/audio/notification_sounds/" + page_params.notification_sound; + const notification_sound = page_params.notification_sound; + const audio_file_without_extension = "/static/audio/notification_sounds/" + notification_sound; $("#notification-sound-source-ogg").attr("src", `${audio_file_without_extension}.ogg`); $("#notification-sound-source-mp3").attr("src", `${audio_file_without_extension}.mp3`); - // Load it so that it is ready to be played; without this the old sound - // is played. - $("#notification-sound-audio")[0].load(); + if (notification_sound !== "none") { + // Load it so that it is ready to be played; without this the old sound + // is played. + $("#notification-sound-audio")[0].load(); + } } export function permission_state() { @@ -415,6 +417,12 @@ export function should_send_desktop_notification(message) { } export function should_send_audible_notification(message) { + // If `None` is selected as the notification sound, never send + // audible notifications regardless of other configuration. + if (page_params.notification_sound === "none") { + return false; + } + // For streams, ding if sounds are enabled for all messages on // this stream. if ( diff --git a/static/js/settings_notifications.js b/static/js/settings_notifications.js index 91972f40a5..be66afa338 100644 --- a/static/js/settings_notifications.js +++ b/static/js/settings_notifications.js @@ -92,7 +92,9 @@ export function set_up() { }); $("#play_notification_sound").on("click", () => { - $("#notification-sound-audio")[0].play(); + if (page_params.notification_sound !== "none") { + $("#notification-sound-audio")[0].play(); + } }); const notification_sound_dropdown = $("#notification_sound"); diff --git a/static/templates/settings/notification_settings.hbs b/static/templates/settings/notification_settings.hbs index 6d8b87dd16..c06f7549d6 100644 --- a/static/templates/settings/notification_settings.hbs +++ b/static/templates/settings/notification_settings.hbs @@ -69,6 +69,7 @@ {{#unless enable_sound_select}} disabled {{/unless}}> + {{#each page_params.available_notification_sounds}} {{/each}} diff --git a/zerver/views/user_settings.py b/zerver/views/user_settings.py index 551a6b8450..c52b610acb 100644 --- a/zerver/views/user_settings.py +++ b/zerver/views/user_settings.py @@ -272,6 +272,7 @@ def json_change_notify_settings( if ( notification_sound is not None and notification_sound not in get_available_notification_sounds() + and notification_sound != "none" ): raise JsonableError(_("Invalid notification sound '{}'").format(notification_sound))