notifications: Add a setting for changing the notification sound.

Also, add a new notification sound, "ding". It comes from
https://freesound.org, where the original Zulip notification sound comes
from as well. In the future, new sounds can be added by adding audio
files to the `static/audio/notification_sounds` directory.

Tweaked significantly by tabbott:
* Avoided removing static/audio/zulip.ogg, because that file is
  checked for by old versions of the desktop app.
* Added a views check for the sound being valid + tests.
* Added additional tests.
* Restructured the test_events test to be cleaner.
* Removed check_bool_or_string.
* Increased max length of notification_sound.
* Provide available_notification_sounds in events data set if global
  notifications settings are requested.

Fixes #8051.
This commit is contained in:
Marco Burstein 2018-01-11 12:36:11 -08:00 committed by Tim Abbott
parent 7026a8c574
commit ba46dc83c6
20 changed files with 220 additions and 15 deletions

View File

@ -77,7 +77,12 @@ Copyright: 2003-2006 Thom May
2018 Kandra Labs, Inc., and contributors 2018 Kandra Labs, Inc., and contributors
License: BSD-3-Clause License: BSD-3-Clause
Files: static/audio/zulip.* Files: static/audio/notification_sounds/ding.*
Copyright: 2017 InspectorJ
License: CC-BY-3.0
Comment: From https://freesound.org/s/411089/.
Files: static/audio/notification_sounds/zulip.*
Copyright: 2011 Vidsyn Copyright: 2011 Vidsyn
License: CC-0-1.0 License: CC-0-1.0

View File

@ -338,6 +338,28 @@ casper.waitUntilVisible('#language-settings-status a', function () {
casper.test.assertSelectorHasText('#language-settings-status', 'Gespeichert. Bitte lade die Seite neu um die Änderungen zu aktivieren.'); casper.test.assertSelectorHasText('#language-settings-status', 'Gespeichert. Bitte lade die Seite neu um die Änderungen zu aktivieren.');
}); });
casper.then(function () {
casper.waitUntilVisible('[data-section="notifications"]', function () {
casper.test.info('Testing disabled/enabled behavior for Notification sound');
casper.click('[data-section="notifications"]');
});
});
casper.then(function () {
// At the beginning, `#enable_sounds` will be on and `#enable_stream_sounds`
// will be off by default.
casper.test.assertVisible("#notification_sound:enabled", "Notification sound selector is enabled");
casper.click('#enable_stream_sounds');
casper.test.assertVisible("#notification_sound:enabled", "Notification sound selector is enabled");
casper.click('#enable_sounds');
casper.test.assertVisible("#notification_sound:enabled", "Notification sound selector is enabled");
casper.click('#enable_stream_sounds');
casper.test.assertVisible("#notification_sound:disabled", "Notification sound selector is disabled");
});
casper.thenOpen("http://zulip.zulipdev.com:9981/"); casper.thenOpen("http://zulip.zulipdev.com:9981/");
// TODO: test the "Declare Zulip Bankruptcy option" // TODO: test the "Declare Zulip Bankruptcy option"

Binary file not shown.

Binary file not shown.

Binary file not shown.

Binary file not shown.

Binary file not shown.

View File

@ -64,6 +64,14 @@ exports.get_notifications = function () {
return notice_memory; return notice_memory;
}; };
function get_audio_file_path(audio_element, audio_file_without_extension) {
if (audio_element.canPlayType('audio/ogg; codecs="vorbis"')) {
return audio_file_without_extension + ".ogg";
}
return audio_file_without_extension + ".mp3";
}
exports.initialize = function () { exports.initialize = function () {
$(window).focus(function () { $(window).focus(function () {
window_has_focus = true; window_has_focus = true;
@ -86,21 +94,37 @@ exports.initialize = function () {
supports_sound = false; supports_sound = false;
} else { } else {
supports_sound = true; supports_sound = true;
$("#notifications-area").append(audio); $("#notifications-area").append(audio);
audio.append($("<source>").attr("loop", "yes"));
var source = $("#notifications-area audio source");
if (audio[0].canPlayType('audio/ogg; codecs="vorbis"')) { if (audio[0].canPlayType('audio/ogg; codecs="vorbis"')) {
audio.append( source.attr("type", "audio/ogg");
$("<source>").attr("type", "audio/ogg")
.attr("loop", "yes")
.attr("src", "/static/audio/zulip.ogg"));
} else { } else {
audio.append( source.attr("type", "audio/mpeg");
$("<source>").attr("type", "audio/mpeg")
.attr("loop", "yes")
.attr("src", "/static/audio/zulip.mp3"));
} }
var audio_file_without_extension
= "/static/audio/notification_sounds/" + page_params.notification_sound;
source.attr("src", get_audio_file_path(audio[0], audio_file_without_extension));
} }
}; };
function update_notification_sound_source() {
// Simplified version of the source creation in `exports.initialize`, for
// updating the source instead of creating it for the first time.
var audio = $("#notifications-area audio");
var source = $("#notifications-area audio source");
var audio_file_without_extension
= "/static/audio/notification_sounds/" + page_params.notification_sound;
source.attr("src", get_audio_file_path(audio[0], audio_file_without_extension));
// Load it so that it is ready to be played; without this the old sound
// is played.
$("#notifications-area").find("audio")[0].load();
}
exports.permission_state = function () { exports.permission_state = function () {
if (window.Notification === undefined) { if (window.Notification === undefined) {
// act like notifications are blocked if they do not have access to // act like notifications are blocked if they do not have access to
@ -633,6 +657,11 @@ exports.handle_global_notification_updates = function (notification_name, settin
if (settings_notifications.notification_settings.indexOf(notification_name) !== -1) { if (settings_notifications.notification_settings.indexOf(notification_name) !== -1) {
page_params[notification_name] = setting; page_params[notification_name] = setting;
} }
if (notification_name === "notification_sound") {
// Change the sound source with the new page `notification_sound`.
update_notification_sound_source();
}
}; };
return exports; return exports;

View File

@ -126,6 +126,7 @@ exports.build_page = function () {
var rendered_settings_tab = templates.render('settings_tab', { var rendered_settings_tab = templates.render('settings_tab', {
full_name: people.my_full_name(), full_name: people.my_full_name(),
page_params: page_params, page_params: page_params,
enable_sound_select: page_params.enable_sounds || page_params.enable_stream_sounds,
zuliprc: 'zuliprc', zuliprc: 'zuliprc',
botserverrc: 'botserverrc', botserverrc: 'botserverrc',
timezones: moment.tz.names(), timezones: moment.tz.names(),

View File

@ -19,6 +19,7 @@ var pm_mention_notification_settings = [
]; ];
var other_notification_settings = [ var other_notification_settings = [
"notification_sound",
"enable_digest_emails", "enable_digest_emails",
"enable_login_emails", "enable_login_emails",
"realm_name_in_notifications", "realm_name_in_notifications",
@ -66,7 +67,16 @@ exports.set_up = function () {
_.each(other_notification_settings, function (setting) { _.each(other_notification_settings, function (setting) {
$("#" + setting).change(function () { $("#" + setting).change(function () {
change_notification_setting(setting, $(this).prop('checked'), var value;
if (setting === "notification_sound") {
// `notification_sound` is not a boolean.
value = $(this).val();
} else {
value = $(this).prop('checked');
}
change_notification_setting(setting, value,
"#other-notify-settings-status"); "#other-notify-settings-status");
}); });
}); });
@ -83,6 +93,23 @@ exports.set_up = function () {
}); });
}); });
$("#play_notification_sound").click(function () {
$("#notifications-area").find("audio")[0].play();
});
var notification_sound_dropdown = $("#notification_sound");
notification_sound_dropdown.val(page_params.notification_sound);
$("#enable_sounds, #enable_stream_sounds").change(function () {
if ($("#enable_stream_sounds").prop("checked") || $("#enable_sounds").prop("checked")) {
notification_sound_dropdown.prop("disabled", false);
notification_sound_dropdown.parent().removeClass("control-label-disabled");
} else {
notification_sound_dropdown.prop("disabled", true);
notification_sound_dropdown.parent().addClass("control-label-disabled");
}
});
$("#enable_desktop_notifications").change(function () { $("#enable_desktop_notifications").change(function () {
settings_ui.disable_sub_setting_onchange(this.checked, "pm_content_in_desktop_notifications", true); settings_ui.disable_sub_setting_onchange(this.checked, "pm_content_in_desktop_notifications", true);
}); });

View File

@ -129,6 +129,17 @@ label {
padding: 0px 20px; padding: 0px 20px;
} }
#notification_sound,
#play_notification_sound {
display: inline;
margin-right: 8px;
margin-bottom: 0px;
}
.attributions_title {
margin-top: 24px;
}
.table.table-condensed.table-striped { .table.table-condensed.table-striped {
margin: 0px; margin: 0px;
} }

View File

@ -105,6 +105,23 @@
"is_checked" page_params.realm_name_in_notifications "is_checked" page_params.realm_name_in_notifications
"label" settings_label.realm_name_in_notifications}} "label" settings_label.realm_name_in_notifications}}
</div> </div>
</form>
<label for="notification_sound">
{{t "Notification sound:" }}
</label>
<div class="input-group {{#unless enable_sound_select}}control-label-disabled{{/unless}}">
<select name="notification_sound" id="notification_sound"
{{#unless enable_sound_select}}
disabled
{{/unless}}>
{{#each page_params.available_notification_sounds}}
<option value="{{ this }}">{{ this }}</option>
{{/each}}
</select>
<a id="play_notification_sound">
{{t "Play sound" }}
</a>
</div>
</form>
</div> </div>

View File

@ -2715,6 +2715,17 @@ def bulk_add_subscriptions(streams: Iterable[Stream],
[(sub.user_profile, stream) for (sub, stream) in subs_to_activate], [(sub.user_profile, stream) for (sub, stream) in subs_to_activate],
already_subscribed) already_subscribed)
def get_available_notification_sounds() -> List[str]:
notification_sounds_path = os.path.join(settings.STATIC_ROOT, 'audio/notification_sounds')
available_notification_sounds = []
for file_name in os.listdir(notification_sounds_path):
root, ext = os.path.splitext(file_name)
if ext == '.ogg':
available_notification_sounds.append(root)
return available_notification_sounds
def notify_subscriptions_removed(user_profile: UserProfile, streams: Iterable[Stream], def notify_subscriptions_removed(user_profile: UserProfile, streams: Iterable[Stream],
no_log: bool=False) -> None: no_log: bool=False) -> None:
if not no_log: if not no_log:
@ -3311,7 +3322,7 @@ def do_create_realm(string_id: str, name: str,
"signups", realm.display_subdomain, signup_message) "signups", realm.display_subdomain, signup_message)
return realm return realm
def do_change_notification_settings(user_profile: UserProfile, name: str, value: bool, def do_change_notification_settings(user_profile: UserProfile, name: str, value: Union[bool, str],
log: bool=True) -> None: log: bool=True) -> None:
"""Takes in a UserProfile object, the name of a global notification """Takes in a UserProfile object, the name of a global notification
preference to update, and the value to update to preference to update, and the value to update to

View File

@ -40,6 +40,7 @@ from zerver.lib.actions import (
get_status_dict, streams_to_dicts_sorted, get_status_dict, streams_to_dicts_sorted,
default_stream_groups_to_dicts_sorted, default_stream_groups_to_dicts_sorted,
get_owned_bot_dicts, get_owned_bot_dicts,
get_available_notification_sounds,
) )
from zerver.lib.user_groups import user_groups_in_realm_serialized from zerver.lib.user_groups import user_groups_in_realm_serialized
from zerver.tornado.event_queue import request_event_queue, get_user_events from zerver.tornado.event_queue import request_event_queue, get_user_events
@ -285,6 +286,7 @@ def fetch_initial_state_data(user_profile: UserProfile,
if want('update_global_notifications'): if want('update_global_notifications'):
for notification in UserProfile.notification_setting_types: for notification in UserProfile.notification_setting_types:
state[notification] = getattr(user_profile, notification) state[notification] = getattr(user_profile, notification)
state['available_notification_sounds'] = get_available_notification_sounds()
if want('zulip_version'): if want('zulip_version'):
state['zulip_version'] = ZULIP_VERSION state['zulip_version'] = ZULIP_VERSION

View File

@ -0,0 +1,20 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.11.6 on 2018-03-12 03:18
from __future__ import unicode_literals
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('zerver', '0193_realm_email_address_visibility'),
]
operations = [
migrations.AddField(
model_name='userprofile',
name='notification_sound',
field=models.CharField(default='zulip', max_length=20),
),
]

View File

@ -744,6 +744,7 @@ class UserProfile(AbstractBaseUser, PermissionsMixin):
enable_stream_email_notifications = models.BooleanField(default=False) # type: bool enable_stream_email_notifications = models.BooleanField(default=False) # type: bool
enable_stream_push_notifications = models.BooleanField(default=False) # type: bool enable_stream_push_notifications = models.BooleanField(default=False) # type: bool
enable_stream_sounds = models.BooleanField(default=False) # type: bool enable_stream_sounds = models.BooleanField(default=False) # type: bool
notification_sound = models.CharField(max_length=20, default='zulip') # type: str
# PM + @-mention notifications. # PM + @-mention notifications.
enable_desktop_notifications = models.BooleanField(default=True) # type: bool enable_desktop_notifications = models.BooleanField(default=True) # type: bool
@ -865,6 +866,7 @@ class UserProfile(AbstractBaseUser, PermissionsMixin):
enable_stream_push_notifications=bool, enable_stream_push_notifications=bool,
enable_stream_sounds=bool, enable_stream_sounds=bool,
message_content_in_email_notifications=bool, message_content_in_email_notifications=bool,
notification_sound=str,
pm_content_in_desktop_notifications=bool, pm_content_in_desktop_notifications=bool,
realm_name_in_notifications=bool, realm_name_in_notifications=bool,
) )

View File

@ -1672,6 +1672,10 @@ class EventsRegisterTest(ZulipTestCase):
@slow("Actually runs several full-stack fetching tests") @slow("Actually runs several full-stack fetching tests")
def test_change_notification_settings(self) -> None: def test_change_notification_settings(self) -> None:
for notification_setting, v in self.user_profile.notification_setting_types.items(): for notification_setting, v in self.user_profile.notification_setting_types.items():
if notification_setting == "notification_sound":
# notification_sound is tested in its own test
continue
schema_checker = self.check_events_dict([ schema_checker = self.check_events_dict([
('type', equals('update_global_notifications')), ('type', equals('update_global_notifications')),
('notification_name', equals(notification_setting)), ('notification_name', equals(notification_setting)),
@ -1679,12 +1683,27 @@ class EventsRegisterTest(ZulipTestCase):
('setting', check_bool), ('setting', check_bool),
]) ])
do_change_notification_settings(self.user_profile, notification_setting, False) do_change_notification_settings(self.user_profile, notification_setting, False)
for setting_value in [True, False]: for setting_value in [True, False]:
events = self.do_test(lambda: do_change_notification_settings( events = self.do_test(lambda: do_change_notification_settings(
self.user_profile, notification_setting, setting_value, log=False)) self.user_profile, notification_setting, setting_value, log=False))
error = schema_checker('events[0]', events[0]) error = schema_checker('events[0]', events[0])
self.assert_on_error(error) self.assert_on_error(error)
def test_change_notification_sound(self) -> None:
notification_setting = "notification_sound"
schema_checker = self.check_events_dict([
('type', equals('update_global_notifications')),
('notification_name', equals(notification_setting)),
('user', check_string),
('setting', equals("ding")),
])
events = self.do_test(lambda: do_change_notification_settings(
self.user_profile, notification_setting, 'ding', log=False))
error = schema_checker('events[0]', events[0])
self.assert_on_error(error)
def test_realm_emoji_events(self) -> None: def test_realm_emoji_events(self) -> None:
schema_checker = self.check_events_dict([ schema_checker = self.check_events_dict([
('type', equals('realm_emoji')), ('type', equals('realm_emoji')),

View File

@ -47,6 +47,7 @@ class HomeTest(ZulipTestCase):
# Keep this list sorted!!! # Keep this list sorted!!!
expected_keys = [ expected_keys = [
"alert_words", "alert_words",
"available_notification_sounds",
"avatar_source", "avatar_source",
"avatar_url", "avatar_url",
"avatar_url_medium", "avatar_url_medium",
@ -104,6 +105,7 @@ class HomeTest(ZulipTestCase):
"needs_tutorial", "needs_tutorial",
"never_subscribed", "never_subscribed",
"night_mode", "night_mode",
"notification_sound",
"password_min_guesses", "password_min_guesses",
"password_min_length", "password_min_length",
"pm_content_in_desktop_notifications", "pm_content_in_desktop_notifications",

View File

@ -131,8 +131,39 @@ class ChangeSettingsTest(ZulipTestCase):
# This is basically a don't-explode test. # This is basically a don't-explode test.
def test_notify_settings(self) -> None: def test_notify_settings(self) -> None:
for notification_setting in UserProfile.notification_setting_types: for notification_setting in UserProfile.notification_setting_types:
self.check_for_toggle_param_patch("/json/settings/notifications", # `notification_sound` is a string not a boolean, so this test
notification_setting) # doesn't work for it.
#
# TODO: Make this work more like do_test_realm_update_api
if notification_setting is not 'notification_sound':
self.check_for_toggle_param_patch("/json/settings/notifications",
notification_setting)
def test_change_notification_sound(self) -> None:
pattern = "/json/settings/notifications"
param = "notification_sound"
user_profile = self.example_user('hamlet')
self.login(user_profile.email)
json_result = self.client_patch(pattern,
{param: ujson.dumps("invalid")})
self.assert_json_error(json_result, "Invalid notification sound 'invalid'")
json_result = self.client_patch(pattern,
{param: ujson.dumps("ding")})
self.assert_json_success(json_result)
# refetch user_profile object to correctly handle caching
user_profile = self.example_user('hamlet')
self.assertEqual(getattr(user_profile, param), "ding")
json_result = self.client_patch(pattern,
{param: ujson.dumps('zulip')})
self.assert_json_success(json_result)
# refetch user_profile object to correctly handle caching
user_profile = self.example_user('hamlet')
self.assertEqual(getattr(user_profile, param), 'zulip')
def test_toggling_boolean_user_display_settings(self) -> None: def test_toggling_boolean_user_display_settings(self) -> None:
"""Test updating each boolean setting in UserProfile property_types""" """Test updating each boolean setting in UserProfile property_types"""

View File

@ -12,7 +12,8 @@ from zerver.decorator import has_request_variables, \
from zerver.lib.actions import do_change_password, do_change_notification_settings, \ from zerver.lib.actions import do_change_password, do_change_notification_settings, \
do_change_enter_sends, do_regenerate_api_key, do_change_avatar_fields, \ do_change_enter_sends, do_regenerate_api_key, do_change_avatar_fields, \
do_set_user_display_setting, validate_email, do_change_user_delivery_email, \ do_set_user_display_setting, validate_email, do_change_user_delivery_email, \
do_start_email_change_process, check_change_full_name, do_change_user_delivery_email do_start_email_change_process, check_change_full_name, do_change_user_delivery_email, \
get_available_notification_sounds
from zerver.lib.avatar import avatar_url from zerver.lib.avatar import avatar_url
from zerver.lib.send_email import send_email, FromAddress from zerver.lib.send_email import send_email, FromAddress
from zerver.lib.i18n import get_available_language_codes from zerver.lib.i18n import get_available_language_codes
@ -155,6 +156,7 @@ def json_change_notify_settings(
enable_stream_email_notifications: Optional[bool]=REQ(validator=check_bool, default=None), enable_stream_email_notifications: Optional[bool]=REQ(validator=check_bool, default=None),
enable_stream_push_notifications: Optional[bool]=REQ(validator=check_bool, default=None), enable_stream_push_notifications: Optional[bool]=REQ(validator=check_bool, default=None),
enable_stream_sounds: Optional[bool]=REQ(validator=check_bool, default=None), enable_stream_sounds: Optional[bool]=REQ(validator=check_bool, default=None),
notification_sound: Optional[str]=REQ(validator=check_string, default=None),
enable_desktop_notifications: Optional[bool]=REQ(validator=check_bool, default=None), enable_desktop_notifications: Optional[bool]=REQ(validator=check_bool, default=None),
enable_sounds: Optional[bool]=REQ(validator=check_bool, default=None), enable_sounds: Optional[bool]=REQ(validator=check_bool, default=None),
enable_offline_email_notifications: Optional[bool]=REQ(validator=check_bool, default=None), enable_offline_email_notifications: Optional[bool]=REQ(validator=check_bool, default=None),
@ -170,6 +172,10 @@ def json_change_notify_settings(
# Stream notification settings. # Stream notification settings.
if (notification_sound is not None and
notification_sound not in get_available_notification_sounds()):
raise JsonableError(_("Invalid notification sound '%s'") % (notification_sound,))
req_vars = {k: v for k, v in list(locals().items()) if k in user_profile.notification_setting_types} req_vars = {k: v for k, v in list(locals().items()) if k in user_profile.notification_setting_types}
for k, v in list(req_vars.items()): for k, v in list(req_vars.items()):