From e7cf1112c8f716712a38db6008e20c88de8fe6cd Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Tue, 10 Dec 2019 16:41:20 -0800 Subject: [PATCH] notifications: Enable online push notifications by default. For new user onboarding, it's important for it to be easy to verify that Zulip's mobile push notifications work without jumping through hoops or potentially making mistakes. For that reason, it makes sense to toggle the notification defaults for new users to the more aggressive mode (ignoring whether the user is currently actively online); they can set the more subtle mode if they find that the notifications are annoying. --- .../zerver/help/test-mobile-notifications.md | 9 +++++---- zerver/lib/test_fixtures.py | 1 + ...nable_online_push_notifications_default.py | 20 +++++++++++++++++++ zerver/models.py | 2 +- zerver/tests/test_event_queue.py | 4 ++++ .../tests/test_message_edit_notifications.py | 16 ++++++++------- zerver/tests/test_users.py | 10 ++++++++++ 7 files changed, 50 insertions(+), 12 deletions(-) create mode 100644 zerver/migrations/0258_enable_online_push_notifications_default.py diff --git a/templates/zerver/help/test-mobile-notifications.md b/templates/zerver/help/test-mobile-notifications.md index 461f1a9e4d..6f4d015d0c 100644 --- a/templates/zerver/help/test-mobile-notifications.md +++ b/templates/zerver/help/test-mobile-notifications.md @@ -1,10 +1,11 @@ # Test mobile notifications -Normally, mobile notifications are only sent after you've been idle for a -few minutes, which isn't ideal for testing. +Zulip supports configuring mobile notifications to skip sending mobile +push notifications when you are currently actively using one of the +Zulip apps. -You can instead have mobile notifications sent regardless of how recently -you've been online. +When testing mobile notifications, you should make sure Zulip is +configured to send mobile notifications even when you're online. {start_tabs} diff --git a/zerver/lib/test_fixtures.py b/zerver/lib/test_fixtures.py index 4b3b89b65f..ffbb2b5153 100644 --- a/zerver/lib/test_fixtures.py +++ b/zerver/lib/test_fixtures.py @@ -199,6 +199,7 @@ def template_database_status( 'zerver/lib/generate_test_data.py', 'tools/setup/postgres-init-test-db', 'tools/setup/postgres-init-dev-db', + 'zerver/migrations/0258_enable_online_push_notifications_default.py', ] if check_settings is None: check_settings = [ diff --git a/zerver/migrations/0258_enable_online_push_notifications_default.py b/zerver/migrations/0258_enable_online_push_notifications_default.py new file mode 100644 index 0000000000..e8b9a58053 --- /dev/null +++ b/zerver/migrations/0258_enable_online_push_notifications_default.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.26 on 2019-12-11 00:41 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('zerver', '0257_fix_has_link_attribute'), + ] + + operations = [ + migrations.AlterField( + model_name='userprofile', + name='enable_online_push_notifications', + field=models.BooleanField(default=True), + ), + ] diff --git a/zerver/models.py b/zerver/models.py index 17a67e75c1..e19ebaf2d7 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -901,7 +901,7 @@ class UserProfile(AbstractBaseUser, PermissionsMixin): enable_offline_email_notifications = models.BooleanField(default=True) # type: bool message_content_in_email_notifications = models.BooleanField(default=True) # type: bool enable_offline_push_notifications = models.BooleanField(default=True) # type: bool - enable_online_push_notifications = models.BooleanField(default=False) # type: bool + enable_online_push_notifications = models.BooleanField(default=True) # type: bool DESKTOP_ICON_COUNT_DISPLAY_MESSAGES = 1 DESKTOP_ICON_COUNT_DISPLAY_NOTIFIABLE = 2 diff --git a/zerver/tests/test_event_queue.py b/zerver/tests/test_event_queue.py index 96a8ac7a6f..2927c4a8d0 100644 --- a/zerver/tests/test_event_queue.py +++ b/zerver/tests/test_event_queue.py @@ -217,6 +217,10 @@ class MissedMessageNotificationsTest(ZulipTestCase): """Tests what arguments missedmessage_hook passes into maybe_enqueue_notifications. Combined with the previous test, this ensures that the missedmessage_hook is correct""" user_profile = self.example_user('hamlet') + + user_profile.enable_online_push_notifications = False + user_profile.save() + email = user_profile.email # Fetch the Denmark stream for testing stream = get_stream("Denmark", user_profile.realm) diff --git a/zerver/tests/test_message_edit_notifications.py b/zerver/tests/test_message_edit_notifications.py index ac5016e292..780925f5e5 100644 --- a/zerver/tests/test_message_edit_notifications.py +++ b/zerver/tests/test_message_edit_notifications.py @@ -56,7 +56,8 @@ class EditMessageSideEffectsTest(ZulipTestCase): content='now we mention @**Cordelia Lear**', ) - def _login_and_send_original_stream_message(self, content: str) -> int: + def _login_and_send_original_stream_message(self, content: str, + enable_online_push_notifications: bool=False) -> int: ''' Note our conventions here: @@ -67,6 +68,9 @@ class EditMessageSideEffectsTest(ZulipTestCase): hamlet = self.example_user('hamlet') cordelia = self.example_user('cordelia') + cordelia.enable_online_push_notifications = enable_online_push_notifications + cordelia.save() + self.login(hamlet.email) self.subscribe(hamlet, 'Scotland') self.subscribe(cordelia, 'Scotland') @@ -292,11 +296,10 @@ class EditMessageSideEffectsTest(ZulipTestCase): def test_always_push_notify_for_fully_present_mentioned_user(self) -> None: cordelia = self.example_user('cordelia') - cordelia.enable_online_push_notifications = True - cordelia.save() message_id = self._login_and_send_original_stream_message( - content='no mention' + content='no mention', + enable_online_push_notifications=True, ) self._make_cordelia_present_on_web() @@ -331,11 +334,10 @@ class EditMessageSideEffectsTest(ZulipTestCase): def test_always_push_notify_for_fully_present_boring_user(self) -> None: cordelia = self.example_user('cordelia') - cordelia.enable_online_push_notifications = True - cordelia.save() message_id = self._login_and_send_original_stream_message( - content='no mention' + content='no mention', + enable_online_push_notifications=True, ) self._make_cordelia_present_on_web() diff --git a/zerver/tests/test_users.py b/zerver/tests/test_users.py index b60cd4e0b1..49d0640e6e 100644 --- a/zerver/tests/test_users.py +++ b/zerver/tests/test_users.py @@ -1030,6 +1030,16 @@ class RecipientInfoTest(ZulipTestCase): cordelia = self.example_user('cordelia') othello = self.example_user('othello') + # These tests were written with the old default for + # enable_online_push_notifications; that default is better for + # testing the full code path anyway. + hamlet.enable_online_push_notifications = False + cordelia.enable_online_push_notifications = False + othello.enable_online_push_notifications = False + hamlet.save() + cordelia.save() + othello.save() + realm = hamlet.realm stream_name = 'Test Stream'