api: Allow setting email_notifications_batching_period_seconds.

We allow a maximum value of one week to make sure there aren't a huge
number of rows in the table for any user (this could happen if stream
notifications are enabled).

This commit also fixes a small error in the user_settings test.
This commit is contained in:
Abhijeet Prasad Bodas 2021-07-22 13:35:04 +05:30 committed by Tim Abbott
parent dd5e12d112
commit 5db4fe8652
5 changed files with 66 additions and 2 deletions

View File

@ -11,6 +11,13 @@ below features are supported.
## Changes in Zulip 5.0 ## Changes in Zulip 5.0
**Feature level 82**
* [`PATCH /settings`](/api/update-settings) now accepts a new
`email_notifications_batching_period_seconds` field for setting the
time duration for which the server will collect email notifications
before sending them.
**Feature level 81** **Feature level 81**
* `POST /users/me/enter-sends` has been removed. The `enter_sends` * `POST /users/me/enter-sends` has been removed. The `enter_sends`

View File

@ -8446,6 +8446,14 @@ paths:
description: | description: |
Present if `update_global_notifications` is present in `fetch_event_types`. Present if `update_global_notifications` is present in `fetch_event_types`.
The current value of this global notification setting for the user.
See [PATCH /settings](/api/update-settings) for details on
the meaning of this setting.
email_notifications_batching_period_seconds:
type: integer
description: |
Present if `update_global_notifications` is present in `fetch_event_types`.
The current value of this global notification setting for the user. The current value of this global notification setting for the user.
See [PATCH /settings](/api/update-settings) for details on See [PATCH /settings](/api/update-settings) for details on
the meaning of this setting. the meaning of this setting.
@ -10253,6 +10261,16 @@ paths:
schema: schema:
type: boolean type: boolean
example: true example: true
- name: email_notifications_batching_period_seconds
in: query
description: |
The duration (in seconds) for which the server should wait to batch
email notifications before sending them.
**Changes**: New in Zulip 5.0 (feature level 82)
schema:
type: integer
example: 120
- name: enable_offline_email_notifications - name: enable_offline_email_notifications
in: query in: query
description: | description: |

View File

@ -65,6 +65,7 @@ class HomeTest(ZulipTestCase):
"desktop_icon_count_display", "desktop_icon_count_display",
"development_environment", "development_environment",
"email", "email",
"email_notifications_batching_period_seconds",
"emojiset", "emojiset",
"emojiset_choices", "emojiset_choices",
"enable_desktop_notifications", "enable_desktop_notifications",

View File

@ -135,12 +135,12 @@ 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.keys():
# `notification_sound` is a string not a boolean, so this test # `notification_sound` is a string not a boolean, so this test
# doesn't work for it. # doesn't work for it.
# #
# TODO: Make this work more like do_test_realm_update_api # TODO: Make this work more like do_test_realm_update_api
if notification_setting != "notification_sound": if UserProfile.notification_setting_types[notification_setting] is bool:
self.check_for_toggle_param_patch("/json/settings", notification_setting) self.check_for_toggle_param_patch("/json/settings", notification_setting)
def test_change_notification_sound(self) -> None: def test_change_notification_sound(self) -> None:
@ -166,6 +166,30 @@ class ChangeSettingsTest(ZulipTestCase):
user_profile = self.example_user("hamlet") user_profile = self.example_user("hamlet")
self.assertEqual(getattr(user_profile, param), "zulip") self.assertEqual(getattr(user_profile, param), "zulip")
def test_change_email_batching_period(self) -> None:
hamlet = self.example_user("hamlet")
self.login_user(hamlet)
# Default is two minutes
self.assertEqual(hamlet.email_notifications_batching_period_seconds, 120)
result = self.client_patch(
"/json/settings", {"email_notifications_batching_period_seconds": -1}
)
self.assert_json_error(result, "Invalid email batching period: -1 seconds")
result = self.client_patch(
"/json/settings", {"email_notifications_batching_period_seconds": 7 * 24 * 60 * 60 + 10}
)
self.assert_json_error(result, "Invalid email batching period: 604810 seconds")
result = self.client_patch(
"/json/settings", {"email_notifications_batching_period_seconds": 5 * 60}
)
self.assert_json_success(result)
hamlet = self.example_user("hamlet")
self.assertEqual(hamlet.email_notifications_batching_period_seconds, 300)
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"""
boolean_settings = ( boolean_settings = (

View File

@ -121,6 +121,9 @@ def json_change_settings(
timezone: Optional[str] = REQ( timezone: Optional[str] = REQ(
str_validator=check_string_in(pytz.all_timezones_set), default=None str_validator=check_string_in(pytz.all_timezones_set), default=None
), ),
email_notifications_batching_period_seconds: Optional[int] = REQ(
json_validator=check_int, default=None
),
enable_stream_desktop_notifications: Optional[bool] = REQ( enable_stream_desktop_notifications: Optional[bool] = REQ(
json_validator=check_bool, default=None json_validator=check_bool, default=None
), ),
@ -169,6 +172,17 @@ def json_change_settings(
): ):
raise JsonableError(_("Invalid notification sound '{}'").format(notification_sound)) raise JsonableError(_("Invalid notification sound '{}'").format(notification_sound))
if email_notifications_batching_period_seconds is not None and (
email_notifications_batching_period_seconds <= 0
or email_notifications_batching_period_seconds > 7 * 24 * 60 * 60
):
# We set a limit of one week for the batching period
raise JsonableError(
_("Invalid email batching period: {} seconds").format(
email_notifications_batching_period_seconds
)
)
if new_password != "": if new_password != "":
return_data: Dict[str, Any] = {} return_data: Dict[str, Any] = {}
if email_belongs_to_ldap(user_profile.realm, user_profile.delivery_email): if email_belongs_to_ldap(user_profile.realm, user_profile.delivery_email):