diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index ce77156f74..5d3a48c66e 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -5069,15 +5069,17 @@ def do_change_notification_settings( send_event(user_profile.realm, event, [user_profile.id]) - # This legacy event format is for backwards-compatiblity with - # clients that don't support the new user_settings event type. - legacy_event = { - "type": "update_global_notifications", - "user": user_profile.email, - "notification_name": name, - "setting": value, - } - send_event(user_profile.realm, legacy_event, [user_profile.id]) + if name in UserProfile.notification_settings_legacy: + # This legacy event format is for backwards-compatiblity with + # clients that don't support the new user_settings event type. + # We only send this for settings added before Feature level 89. + legacy_event = { + "type": "update_global_notifications", + "user": user_profile.email, + "notification_name": name, + "setting": value, + } + send_event(user_profile.realm, legacy_event, [user_profile.id]) def do_set_user_display_setting( @@ -5103,19 +5105,21 @@ def do_set_user_display_setting( send_event(user_profile.realm, event, [user_profile.id]) - # This legacy event format is for backwards-compatiblity with - # clients that don't support the new user_settings event type. - legacy_event = { - "type": "update_display_settings", - "user": user_profile.email, - "setting_name": setting_name, - "setting": setting_value, - } - if setting_name == "default_language": - assert isinstance(setting_value, str) - legacy_event["language_name"] = get_language_name(setting_value) + if setting_name in UserProfile.display_settings_legacy or setting_name == "timezone": + # This legacy event format is for backwards-compatiblity with + # clients that don't support the new user_settings event type. + # We only send this for settings added before Feature level 89. + legacy_event = { + "type": "update_display_settings", + "user": user_profile.email, + "setting_name": setting_name, + "setting": setting_value, + } + if setting_name == "default_language": + assert isinstance(setting_value, str) + legacy_event["language_name"] = get_language_name(setting_value) - send_event(user_profile.realm, legacy_event, [user_profile.id]) + send_event(user_profile.realm, legacy_event, [user_profile.id]) # Updates to the timezone display setting are sent to all users if setting_name == "timezone": diff --git a/zerver/lib/event_schema.py b/zerver/lib/event_schema.py index 1c8ed8c3fe..53f6d75bcf 100644 --- a/zerver/lib/event_schema.py +++ b/zerver/lib/event_schema.py @@ -1443,11 +1443,8 @@ def check_user_settings_update( assert isinstance(setting_name, str) if setting_name == "timezone": assert isinstance(value, str) - elif setting_name in UserProfile.property_types: - setting_type = UserProfile.property_types[setting_name] - assert isinstance(value, setting_type) else: - setting_type = UserProfile.notification_setting_types[setting_name] + setting_type = UserProfile.property_types[setting_name] assert isinstance(value, setting_type) if setting_name == "default_language": diff --git a/zerver/lib/events.py b/zerver/lib/events.py index 4cbf6a2326..eb91679bc4 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -515,13 +515,13 @@ def fetch_initial_state_data( state["stop_words"] = read_stop_words() if want("update_display_settings") and not user_settings_object: - for prop in UserProfile.property_types: + for prop in UserProfile.display_settings_legacy: state[prop] = getattr(settings_user, prop) state["emojiset_choices"] = UserProfile.emojiset_choices() state["timezone"] = settings_user.timezone if want("update_global_notifications") and not user_settings_object: - for notification in UserProfile.notification_setting_types: + for notification in UserProfile.notification_settings_legacy: state[notification] = getattr(settings_user, notification) state["available_notification_sounds"] = get_available_notification_sounds() @@ -530,8 +530,6 @@ def fetch_initial_state_data( for prop in UserProfile.property_types: state["user_settings"][prop] = getattr(settings_user, prop) - for notification in UserProfile.notification_setting_types: - state["user_settings"][notification] = getattr(settings_user, notification) state["user_settings"]["emojiset_choices"] = UserProfile.emojiset_choices() state["user_settings"]["timezone"] = settings_user.timezone @@ -1112,20 +1110,16 @@ def apply_event( state["realm_playgrounds"] = event["realm_playgrounds"] elif event["type"] == "update_display_settings": if event["setting_name"] != "timezone": - assert event["setting_name"] in UserProfile.property_types + assert event["setting_name"] in UserProfile.display_settings_legacy state[event["setting_name"]] = event["setting"] elif event["type"] == "update_global_notifications": - assert event["notification_name"] in UserProfile.notification_setting_types + assert event["notification_name"] in UserProfile.notification_settings_legacy state[event["notification_name"]] = event["setting"] elif event["type"] == "user_settings": - # timezone setting is not included in property_types or - # notification_setting_types dicts, because this setting - # is not a part of UserBaseSettings class. + # timezone setting is not included in property_types dict because + # this setting is not a part of UserBaseSettings class. if event["property"] != "timezone": - assert ( - event["property"] in UserProfile.property_types - or event["property"] in UserProfile.notification_setting_types - ) + assert event["property"] in UserProfile.property_types state[event["property"]] = event["value"] state["user_settings"][event["property"]] = event["value"] elif event["type"] == "invites_changed": diff --git a/zerver/models.py b/zerver/models.py index 22189981d6..498bda0a23 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -1362,8 +1362,7 @@ class UserBaseSettings(models.Model): # Whether or not the user wants to sync their drafts. enable_drafts_synchronization = models.BooleanField(default=True) - # Define the types of the various automatically managed properties - property_types = dict( + display_settings_legacy = dict( color_scheme=int, default_language=str, default_view=str, @@ -1380,7 +1379,7 @@ class UserBaseSettings(models.Model): twenty_four_hour_time=bool, ) - notification_setting_types = dict( + notification_settings_legacy = dict( enable_desktop_notifications=bool, enable_digest_emails=bool, enable_login_emails=bool, @@ -1403,6 +1402,13 @@ class UserBaseSettings(models.Model): presence_enabled=bool, ) + notification_setting_types = { + **notification_settings_legacy + } # Add new notifications settings here. + + # Define the types of the various automatically managed properties + property_types = {**display_settings_legacy, **notification_setting_types} + class Meta: abstract = True diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 517f692f05..19d87956b4 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -2204,7 +2204,10 @@ class UserDisplayActionTest(BaseAction): def test_set_user_display_settings(self) -> None: for prop in UserProfile.property_types: - self.do_set_user_display_settings_test(prop) + # Notification settings have a separate test suite, which + # handles their separate legacy event type. + if prop not in UserProfile.notification_settings_legacy: + self.do_set_user_display_settings_test(prop) def test_set_user_timezone(self) -> None: values = ["America/Denver", "Pacific/Pago_Pago", "Pacific/Galapagos", ""] diff --git a/zerver/tests/test_settings.py b/zerver/tests/test_settings.py index 2d4b3f0e8d..e1c3646705 100644 --- a/zerver/tests/test_settings.py +++ b/zerver/tests/test_settings.py @@ -319,7 +319,7 @@ class ChangeSettingsTest(ZulipTestCase): ) self.assert_json_error(result, "Your Zulip password is managed in LDAP") - def do_test_change_user_display_setting(self, setting_name: str) -> None: + def do_test_change_user_setting(self, setting_name: str) -> None: test_changes: Dict[str, Any] = dict( default_language="de", @@ -366,14 +366,19 @@ class ChangeSettingsTest(ZulipTestCase): user_profile = self.example_user("hamlet") self.assertNotEqual(getattr(user_profile, setting_name), invalid_value) - def test_change_user_display_setting(self) -> None: + def test_change_user_setting(self) -> None: """Test updating each non-boolean setting in UserProfile property_types""" user_settings = ( - s for s in UserProfile.property_types if UserProfile.property_types[s] is not bool + s + for s in UserProfile.property_types + if UserProfile.property_types[s] is not bool + # Legacy notification settings have a separate test suite, though + # we can likely merge that test suite with this one in the future. + and s not in UserProfile.notification_settings_legacy ) for setting in user_settings: - self.do_test_change_user_display_setting(setting) - self.do_test_change_user_display_setting("timezone") + self.do_test_change_user_setting(setting) + self.do_test_change_user_setting("timezone") def do_change_emojiset(self, emojiset: str) -> HttpResponse: self.login("hamlet") diff --git a/zerver/views/user_settings.py b/zerver/views/user_settings.py index b220dfd57d..07826d377b 100644 --- a/zerver/views/user_settings.py +++ b/zerver/views/user_settings.py @@ -259,7 +259,11 @@ def json_change_settings( check_change_full_name(user_profile, full_name, user_profile) # Loop over user_profile.property_types - request_settings = {k: v for k, v in list(locals().items()) if k in user_profile.property_types} + request_settings = { + k: v + for k, v in list(locals().items()) + if k in user_profile.property_types and k not in user_profile.notification_setting_types + } for k, v in list(request_settings.items()): if v is not None and getattr(user_profile, k) != v: do_set_user_display_setting(user_profile, k, v)