wildcard_mentions_notify: Add per-stream override of global setting.

Adds required API and front-end changes to modify and read the
wildcard_mentions_notify field in the Subscription model.

It includes front-end code to add the setting to the user's "manage
streams" page. This setting will be greyed out when a stream is muted.
The PR also includes back-end code to add the setting the initial state of
a subscription.

New automated tests were added for the API, events system and front-end.
In manual testing, we checked that modifying the setting in the front end
persisted the change in the Subscription model. We noticed the notifications
were not behaving exactly as expected in manual testing; see
https://github.com/zulip/zulip/issues/13073#issuecomment-560263081 .

Tweaked by tabbott to fix real-time synchronization issues.

Fixes: #13429.
This commit is contained in:
Nat1405 2019-11-25 17:37:12 -08:00 committed by Tim Abbott
parent 792fbeea24
commit d5f005fd61
11 changed files with 88 additions and 4 deletions

View File

@ -94,6 +94,12 @@ run_test('update_property', () => {
checkbox = $("#email_notifications_1");
assert.equal(checkbox.prop('checked'), true);
// Tests wildcard_mentions_notify notifications
stream_events.update_property(1, 'wildcard_mentions_notify', true);
assert.equal(frontend.wildcard_mentions_notify, true);
checkbox = $("#wildcard_mentions_notify_1");
assert.equal(checkbox.prop('checked'), true);
// Test name change
with_overrides(function (override) {
global.with_stub(function (stub) {

View File

@ -293,6 +293,9 @@ exports.receives_notifications = function (stream_name, notification_name) {
if (sub[notification_name] !== null) {
return sub[notification_name];
}
if (notification_name === 'wildcard_mentions_notify') {
return page_params[notification_name];
}
return page_params["enable_stream_" + notification_name];
};
@ -301,6 +304,7 @@ const stream_notification_settings = [
"audible_notifications",
"push_notifications",
"email_notifications",
"wildcard_mentions_notify",
];
exports.update_calculated_fields = function (sub) {

View File

@ -227,6 +227,8 @@ function show_subscription_settings(sub_row) {
exports.is_notification_setting = function (setting_label) {
if (setting_label.indexOf("_notifications") > -1) {
return true;
} else if (setting_label.indexOf("_notify") > -1) {
return true;
}
return false;
};
@ -238,6 +240,7 @@ const settings_labels = {
push_notifications: i18n.t("Mobile notifications"),
email_notifications: i18n.t("Email notifications"),
pin_to_top: i18n.t("Pin stream to top of left sidebar"),
wildcard_mentions_notify: i18n.t("Notifications for @all/@everyone mentions"),
};
const check_realm_setting = {
@ -322,7 +325,11 @@ function stream_setting_clicked(e) {
return false;
}
if (exports.is_notification_setting(setting) && sub[setting] === null) {
sub[setting] = page_params["enable_stream_" + setting];
if (setting === 'wildcard_mentions_notify') {
sub[setting] = page_params[setting];
} else {
sub[setting] = page_params["enable_stream_" + setting];
}
}
exports.set_stream_property(sub, setting, !sub[setting]);

View File

@ -50,6 +50,9 @@ exports.update_property = function (stream_id, property, value, other_values) {
history_public_to_subscribers: other_values.history_public_to_subscribers,
});
break;
case 'wildcard_mentions_notify':
update_stream_setting(sub, value, property);
break;
case 'is_announcement_only':
subs.update_stream_announcement_only(sub, value);
break;

View File

@ -51,4 +51,6 @@ notification. Silent mentions start with `@_` instead of `@`.
You can mention everyone on a stream with the `@**all**` mention. Use
sparingly! Note that this will not notify anyone who has muted the
stream, and users can disable receiving email/push notifications for
these wildcard mentions.
these wildcard mentions, either
[globally](/help/pm-mention-alert-notifications) or for [individual
streams](/help/stream-notifications).

View File

@ -34,6 +34,9 @@ can toggle whether you receive notifications for wildcard mentions:
{end_tabs}
Additionally, you can override this configuration for individual
streams in your [Stream settings](/help/stream-notifications).
## Related articles
* [Add an alert word](/help/add-an-alert-word)

View File

@ -2781,6 +2781,7 @@ def notify_subscriptions_added(user_profile: UserProfile,
audible_notifications=subscription.audible_notifications,
push_notifications=subscription.push_notifications,
email_notifications=subscription.email_notifications,
wildcard_mentions_notify=subscription.wildcard_mentions_notify,
description=stream.description,
rendered_description=stream.rendered_description,
pin_to_top=subscription.pin_to_top,
@ -4734,7 +4735,7 @@ def gather_subscriptions_helper(user_profile: UserProfile,
sub_dicts = get_stream_subscriptions_for_user(user_profile).values(
"recipient_id", "is_muted", "color", "desktop_notifications",
"audible_notifications", "push_notifications", "email_notifications",
"active", "pin_to_top"
"wildcard_mentions_notify", "active", "pin_to_top"
).order_by("recipient_id")
sub_dicts = list(sub_dicts)
@ -4820,6 +4821,7 @@ def gather_subscriptions_helper(user_profile: UserProfile,
'audible_notifications': sub["audible_notifications"],
'push_notifications': sub["push_notifications"],
'email_notifications': sub["email_notifications"],
'wildcard_mentions_notify': sub["wildcard_mentions_notify"],
'pin_to_top': sub["pin_to_top"],
'stream_id': stream["id"],
'first_message_id': stream["first_message_id"],

View File

@ -365,6 +365,30 @@ class MissedMessageNotificationsTest(ZulipTestCase):
user_profile.wildcard_mentions_notify = True
user_profile.save()
# If wildcard_mentions_notify=True for a stream and False for a user, we treat the user
# as mentioned for that stream.
user_profile.wildcard_mentions_notify = False
sub.wildcard_mentions_notify = True
user_profile.save()
sub.save()
client_descriptor = allocate_event_queue()
self.assertTrue(client_descriptor.event_queue.empty())
msg_id = self.send_stream_message(self.example_email("iago"), "Denmark",
content="@**all** what's up?")
with mock.patch("zerver.tornado.event_queue.maybe_enqueue_notifications") as mock_enqueue:
missedmessage_hook(user_profile.id, client_descriptor, True)
mock_enqueue.assert_called_once()
args_list = mock_enqueue.call_args_list[0][0]
self.assertEqual(args_list, (user_profile.id, msg_id, False, False, True, False,
False, "Denmark", False, True,
{'email_notified': True, 'push_notified': True}))
destroy_event_queue(client_descriptor.event_queue.id)
user_profile.wildcard_mentions_notify = True
sub.wildcard_mentions_notify = None
user_profile.save()
sub.save()
# Test the hook with a stream message with stream_push_notify
change_subscription_properties(user_profile, stream, sub, {'push_notifications': True})
client_descriptor = allocate_event_queue()

View File

@ -2495,6 +2495,7 @@ class EventsRegisterTest(ZulipTestCase):
('pin_to_top', check_bool),
('stream_weekly_traffic', check_none_or(check_int)),
('is_old_stream', check_bool),
('wildcard_mentions_notify', check_none_or(check_bool)),
]
if include_subscribers:
subscription_fields.append(('subscribers', check_list(check_int)))

View File

@ -1611,6 +1611,28 @@ class SubscriptionPropertiesTest(ZulipTestCase):
self.assert_json_error(
result, "value key is missing from subscription_data[0]")
def test_set_stream_wildcard_mentions_notify(self) -> None:
"""
A POST request to /api/v1/users/me/subscriptions/properties with wildcard_mentions_notify
sets the property.
"""
test_user = self.example_user('hamlet')
test_email = test_user.email
self.login(test_email)
subs = gather_subscriptions(test_user)[0]
sub = subs[0]
result = self.api_post(test_email, "/api/v1/users/me/subscriptions/properties",
{"subscription_data": ujson.dumps([{"property": "wildcard_mentions_notify",
"stream_id": sub["stream_id"],
"value": True}])})
self.assert_json_success(result)
updated_sub = get_subscription(sub['name'], test_user)
self.assertIsNotNone(updated_sub)
self.assertEqual(updated_sub.wildcard_mentions_notify, True)
def test_set_pin_to_top(self) -> None:
"""
A POST request to /api/v1/users/me/subscriptions/properties with stream_id and
@ -1752,6 +1774,15 @@ class SubscriptionPropertiesTest(ZulipTestCase):
self.assert_json_error(result,
'%s is not a boolean' % (property_name,))
property_name = "wildcard_mentions_notify"
result = self.api_post(test_email, "/api/v1/users/me/subscriptions/properties",
{"subscription_data": ujson.dumps([{"property": property_name,
"value": "bad",
"stream_id": subs[0]["stream_id"]}])})
self.assert_json_error(result,
"%s is not a boolean" % (property_name,))
property_name = "color"
result = self.api_post(test_email, "/api/v1/users/me/subscriptions/properties",
{"subscription_data": ujson.dumps([{"property": property_name,

View File

@ -580,7 +580,8 @@ def update_subscription_properties_backend(
"audible_notifications": check_bool,
"push_notifications": check_bool,
"email_notifications": check_bool,
"pin_to_top": check_bool}
"pin_to_top": check_bool,
"wildcard_mentions_notify": check_bool}
response_data = []
for change in subscription_data: