user_settings: Create _legacy dicts for existing settings.

Since 84742a0, all settings are sent in the `user_settings` dictionary
which were previously sent inline with other fields in /register
response.

In order to simplify the process of adding new personal settings, we
want to transition to a world where new settings only need to consider
the `property_types` object, and code that needs to reference the
legacy behavior interacts with an object with `legacy` in its name.

This way, contributors working on new settings don't need to think
about the legacy code paths at all.

See https://chat.zulip.org/#narrow/stream/378-api-design/topic/user.20settings.20response.20in.20.2Fregister
to understand this better.

zerver/lib/actions.py

@@ -5069,8 +5069,10 @@ def do_change_notification_settings(
 
     send_event(user_profile.realm, 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,
@@ -5103,8 +5105,10 @@ def do_set_user_display_setting(
 
     send_event(user_profile.realm, event, [user_profile.id])
 
+    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,

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":

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":

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
 

zerver/tests/test_events.py

@@ -2204,6 +2204,9 @@ class UserDisplayActionTest(BaseAction):
 
     def test_set_user_display_settings(self) -> None:
         for prop in UserProfile.property_types:
+            # 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:

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")

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)