settings: Require clients to pass dense_mode value when needed.

Previously, if someone changed the font-size or line height
settings to some value other than the legacy values, we set
dense_mode to False if it was True. This commit changes the
code to require clients to pass dense_mode as False in such
cases and raise an error otherwise.

zerver/actions/realm_settings.py

@@ -479,21 +479,6 @@ def do_set_realm_user_default_setting(
             },
         )
 
-        if name in {"web_font_size_px", "web_line_height_percent"}:
-            if (
-                realm_user_default.web_font_size_px != RealmUserDefault.WEB_FONT_SIZE_PX_LEGACY
-                or realm_user_default.web_line_height_percent
-                != RealmUserDefault.WEB_LINE_HEIGHT_PERCENT_LEGACY
-            ):
-                expected_dense_mode = False
-            else:
-                expected_dense_mode = True
-
-            if realm_user_default.dense_mode != expected_dense_mode:
-                do_set_realm_user_default_setting(
-                    realm_user_default, "dense_mode", expected_dense_mode, acting_user=acting_user
-                )
-
     event = dict(
         type="realm_user_settings_defaults",
         op="update",

zerver/actions/user_settings.py

@@ -476,20 +476,6 @@ def do_change_user_setting(
 
     send_event_on_commit(user_profile.realm, event, [user_profile.id])
 
-    if setting_name in {"web_font_size_px", "web_line_height_percent"}:
-        if (
-            user_profile.web_font_size_px != UserProfile.WEB_FONT_SIZE_PX_LEGACY
-            or user_profile.web_line_height_percent != UserProfile.WEB_LINE_HEIGHT_PERCENT_LEGACY
-        ):
-            expected_dense_mode = False
-        else:
-            expected_dense_mode = True
-
-        if user_profile.dense_mode != expected_dense_mode:
-            do_change_user_setting(
-                user_profile, "dense_mode", expected_dense_mode, acting_user=acting_user
-            )
-
     if setting_name in UserProfile.notification_settings_legacy:
         # This legacy event format is for backwards-compatibility with
         # clients that don't support the new user_settings event type.

zerver/tests/test_realm.py

@@ -1996,41 +1996,101 @@ class RealmAPITest(ZulipTestCase):
         self.assertEqual(realm_user_default.dense_mode, True)
         self.login("iago")
 
-        data = {"web_font_size_px": 16}
+        data: dict[str, str | int] = {"web_font_size_px": 16}
+        result = self.client_patch("/json/realm/user_settings_defaults", data)
+        self.assert_json_error(
+            result,
+            "Incompatible values for 'dense_mode' and 'web_font_size_px' settings.",
+        )
+
+        data = {"web_font_size_px": 16, "dense_mode": orjson.dumps(False).decode()}
         result = self.client_patch("/json/realm/user_settings_defaults", data)
         self.assert_json_success(result)
         realm_user_default = RealmUserDefault.objects.get(realm=realm)
         self.assertEqual(realm_user_default.web_font_size_px, 16)
         self.assertEqual(realm_user_default.dense_mode, False)
 
+        data = {"web_font_size_px": 20}
+        result = self.client_patch("/json/realm/user_settings_defaults", data)
+        self.assert_json_success(result)
+        realm_user_default = RealmUserDefault.objects.get(realm=realm)
+        self.assertEqual(realm_user_default.web_font_size_px, 20)
+        self.assertEqual(realm_user_default.dense_mode, False)
+
+        # Check dense_mode is still false when both the
+        # settings are set to legacy values.
         data = {"web_font_size_px": 14}
         result = self.client_patch("/json/realm/user_settings_defaults", data)
         self.assert_json_success(result)
         realm_user_default = RealmUserDefault.objects.get(realm=realm)
         self.assertEqual(realm_user_default.web_font_size_px, 14)
+        self.assertEqual(realm_user_default.web_line_height_percent, 122)
+        self.assertEqual(realm_user_default.dense_mode, False)
+
+        data = {"dense_mode": orjson.dumps(True).decode()}
+        result = self.client_patch("/json/realm/user_settings_defaults", data)
+        self.assert_json_success(result)
+        realm_user_default = RealmUserDefault.objects.get(realm=realm)
+        self.assertEqual(realm_user_default.web_font_size_px, 14)
         self.assertEqual(realm_user_default.dense_mode, True)
 
         data = {"web_line_height_percent": 140}
         result = self.client_patch("/json/realm/user_settings_defaults", data)
+        self.assert_json_error(
+            result,
+            "Incompatible values for 'dense_mode' and 'web_line_height_percent' settings.",
+        )
+
+        data = {"web_line_height_percent": 140, "dense_mode": orjson.dumps(False).decode()}
+        result = self.client_patch("/json/realm/user_settings_defaults", data)
         self.assert_json_success(result)
         realm_user_default = RealmUserDefault.objects.get(realm=realm)
         self.assertEqual(realm_user_default.web_line_height_percent, 140)
         self.assertEqual(realm_user_default.dense_mode, False)
 
-        invalid_data = {"dense_mode": orjson.dumps(True).decode(), "web_font_size_px": 16}
+        data = {"web_line_height_percent": 130}
-        result = self.client_patch("/json/realm/user_settings_defaults", invalid_data)
+        result = self.client_patch("/json/realm/user_settings_defaults", data)
+        self.assert_json_success(result)
+        realm_user_default = RealmUserDefault.objects.get(realm=realm)
+        self.assertEqual(realm_user_default.web_line_height_percent, 130)
+        self.assertEqual(realm_user_default.dense_mode, False)
+
+        # Check dense_mode is still false when both the
+        # settings are set to legacy values.
+        data = {"web_line_height_percent": 122}
+        result = self.client_patch("/json/realm/user_settings_defaults", data)
+        self.assert_json_success(result)
+        realm_user_default = RealmUserDefault.objects.get(realm=realm)
+        self.assertEqual(realm_user_default.web_font_size_px, 14)
+        self.assertEqual(realm_user_default.web_line_height_percent, 122)
+        self.assertEqual(realm_user_default.dense_mode, False)
+
+        data = {"dense_mode": orjson.dumps(True).decode(), "web_font_size_px": 16}
+        result = self.client_patch("/json/realm/user_settings_defaults", data)
         self.assert_json_error(
             result,
             "Incompatible values for 'dense_mode' and 'web_font_size_px' settings.",
         )
 
-        invalid_data = {"dense_mode": orjson.dumps(True).decode(), "web_line_height_percent": 140}
+        data = {"dense_mode": orjson.dumps(True).decode(), "web_line_height_percent": 140}
-        result = self.client_patch("/json/realm/user_settings_defaults", invalid_data)
+        result = self.client_patch("/json/realm/user_settings_defaults", data)
         self.assert_json_error(
             result,
             "Incompatible values for 'dense_mode' and 'web_line_height_percent' settings.",
         )
 
+        data = {
+            "dense_mode": orjson.dumps(True).decode(),
+            "web_font_size_px": 14,
+            "web_line_height_percent": 122,
+        }
+        result = self.client_patch("/json/realm/user_settings_defaults", data)
+        self.assert_json_success(result)
+        realm_user_default = RealmUserDefault.objects.get(realm=realm)
+        self.assertEqual(realm_user_default.web_font_size_px, 14)
+        self.assertEqual(realm_user_default.web_line_height_percent, 122)
+        self.assertEqual(realm_user_default.dense_mode, True)
+
     def test_invalid_default_notification_sound_value(self) -> None:
         result = self.client_patch(
             "/json/realm/user_settings_defaults", {"notification_sound": "invalid"}

zerver/tests/test_settings.py

@@ -506,41 +506,101 @@ class ChangeSettingsTest(ZulipTestCase):
         self.assertEqual(hamlet.dense_mode, True)
         self.login("hamlet")
 
-        data = {"web_font_size_px": 16}
+        data: dict[str, str | int] = {"web_font_size_px": 16}
+        result = self.client_patch("/json/settings", data)
+        self.assert_json_error(
+            result,
+            "Incompatible values for 'dense_mode' and 'web_font_size_px' settings.",
+        )
+
+        data = {"web_font_size_px": 16, "dense_mode": orjson.dumps(False).decode()}
         result = self.client_patch("/json/settings", data)
         self.assert_json_success(result)
         hamlet = self.example_user("hamlet")
         self.assertEqual(hamlet.web_font_size_px, 16)
         self.assertEqual(hamlet.dense_mode, False)
 
+        data = {"web_font_size_px": 20}
+        result = self.client_patch("/json/settings", data)
+        self.assert_json_success(result)
+        hamlet = self.example_user("hamlet")
+        self.assertEqual(hamlet.web_font_size_px, 20)
+        self.assertEqual(hamlet.dense_mode, False)
+
+        # Check dense_mode is still false when both the
+        # settings are set to legacy values.
         data = {"web_font_size_px": 14}
         result = self.client_patch("/json/settings", data)
         self.assert_json_success(result)
         hamlet = self.example_user("hamlet")
         self.assertEqual(hamlet.web_font_size_px, 14)
+        self.assertEqual(hamlet.web_line_height_percent, 122)
+        self.assertEqual(hamlet.dense_mode, False)
+
+        data = {"dense_mode": orjson.dumps(True).decode()}
+        result = self.client_patch("/json/settings", data)
+        self.assert_json_success(result)
+        hamlet = self.example_user("hamlet")
+        self.assertEqual(hamlet.web_font_size_px, 14)
         self.assertEqual(hamlet.dense_mode, True)
 
         data = {"web_line_height_percent": 140}
         result = self.client_patch("/json/settings", data)
+        self.assert_json_error(
+            result,
+            "Incompatible values for 'dense_mode' and 'web_line_height_percent' settings.",
+        )
+
+        data = {"web_line_height_percent": 140, "dense_mode": orjson.dumps(False).decode()}
+        result = self.client_patch("/json/settings", data)
         self.assert_json_success(result)
         hamlet = self.example_user("hamlet")
         self.assertEqual(hamlet.web_line_height_percent, 140)
         self.assertEqual(hamlet.dense_mode, False)
 
-        invalid_data = {"dense_mode": orjson.dumps(True).decode(), "web_font_size_px": 16}
+        data = {"web_line_height_percent": 130}
-        result = self.client_patch("/json/settings", invalid_data)
+        result = self.client_patch("/json/settings", data)
+        self.assert_json_success(result)
+        hamlet = self.example_user("hamlet")
+        self.assertEqual(hamlet.web_line_height_percent, 130)
+        self.assertEqual(hamlet.dense_mode, False)
+
+        # Check dense_mode is still false when both the
+        # settings are set to legacy values.
+        data = {"web_line_height_percent": 122}
+        result = self.client_patch("/json/settings", data)
+        self.assert_json_success(result)
+        hamlet = self.example_user("hamlet")
+        self.assertEqual(hamlet.web_font_size_px, 14)
+        self.assertEqual(hamlet.web_line_height_percent, 122)
+        self.assertEqual(hamlet.dense_mode, False)
+
+        data = {"dense_mode": orjson.dumps(True).decode(), "web_font_size_px": 16}
+        result = self.client_patch("/json/settings", data)
         self.assert_json_error(
             result,
             "Incompatible values for 'dense_mode' and 'web_font_size_px' settings.",
         )
 
-        invalid_data = {"dense_mode": orjson.dumps(True).decode(), "web_line_height_percent": 140}
+        data = {"dense_mode": orjson.dumps(True).decode(), "web_line_height_percent": 140}
-        result = self.client_patch("/json/settings", invalid_data)
+        result = self.client_patch("/json/settings", data)
         self.assert_json_error(
             result,
             "Incompatible values for 'dense_mode' and 'web_line_height_percent' settings.",
         )
 
+        data = {
+            "dense_mode": orjson.dumps(True).decode(),
+            "web_font_size_px": 14,
+            "web_line_height_percent": 122,
+        }
+        result = self.client_patch("/json/settings", data)
+        self.assert_json_success(result)
+        hamlet = self.example_user("hamlet")
+        self.assertEqual(hamlet.web_font_size_px, 14)
+        self.assertEqual(hamlet.web_line_height_percent, 122)
+        self.assertEqual(hamlet.dense_mode, True)
+
 
 class UserChangesTest(ZulipTestCase):
     def test_update_api_key(self) -> None:

zerver/views/realm.py

@@ -61,7 +61,10 @@ from zerver.models.realms import (
     OrgTypeEnum,
     WildcardMentionPolicyEnum,
 )
-from zerver.views.user_settings import check_settings_values
+from zerver.views.user_settings import (
+    check_information_density_setting_values,
+    check_settings_values,
+)
 
 
 def parse_jitsi_server_url(value: str, special_values_map: Mapping[str, str | None]) -> str | None:
@@ -644,25 +647,17 @@ def update_realm_user_settings_defaults(
     if notification_sound is not None or email_notifications_batching_period_seconds is not None:
         check_settings_values(notification_sound, email_notifications_batching_period_seconds)
 
-    if (
-        dense_mode
-        and web_font_size_px is not None
-        and web_font_size_px != RealmUserDefault.WEB_FONT_SIZE_PX_LEGACY
-    ):
-        raise JsonableError(
-            _("Incompatible values for 'dense_mode' and 'web_font_size_px' settings.")
-        )
-
-    if (
-        dense_mode
-        and web_line_height_percent is not None
-        and web_line_height_percent != RealmUserDefault.WEB_LINE_HEIGHT_PERCENT_LEGACY
-    ):
-        raise JsonableError(
-            _("Incompatible values for 'dense_mode' and 'web_line_height_percent' settings.")
-        )
-
     realm_user_default = RealmUserDefault.objects.get(realm=user_profile.realm)
+
+    if (
+        dense_mode is not None
+        or web_font_size_px is not None
+        or web_line_height_percent is not None
+    ):
+        check_information_density_setting_values(
+            realm_user_default, dense_mode, web_font_size_px, web_line_height_percent
+        )
+
     request_settings = {k: v for k, v in locals().items() if k in RealmUserDefault.property_types}
     for k, v in request_settings.items():
         if v is not None and getattr(realm_user_default, k) != v:

zerver/views/user_settings.py

@@ -52,7 +52,7 @@ from zerver.lib.validator import (
     check_string_in,
     check_timezone,
 )
-from zerver.models import EmailChangeStatus, UserProfile
+from zerver.models import EmailChangeStatus, RealmUserDefault, UserBaseSettings, UserProfile
 from zerver.models.realms import avatar_changes_disabled, name_changes_disabled
 from zerver.views.auth import redirect_to_deactivation_notice
 from zproject.backends import check_password_strength, email_belongs_to_ldap
@@ -192,6 +192,34 @@ def check_settings_values(
         )
 
 
+def check_information_density_setting_values(
+    setting_object: UserProfile | RealmUserDefault,
+    dense_mode: bool | None,
+    web_font_size_px: int | None,
+    web_line_height_percent: int | None,
+) -> None:
+    dense_mode = dense_mode if dense_mode is not None else setting_object.dense_mode
+    web_font_size_px = (
+        web_font_size_px if web_font_size_px is not None else setting_object.web_font_size_px
+    )
+    web_line_height_percent = (
+        web_line_height_percent
+        if web_line_height_percent is not None
+        else setting_object.web_line_height_percent
+    )
+
+    if dense_mode:
+        if web_font_size_px != UserBaseSettings.WEB_FONT_SIZE_PX_LEGACY:
+            raise JsonableError(
+                _("Incompatible values for 'dense_mode' and 'web_font_size_px' settings.")
+            )
+
+        if web_line_height_percent != UserBaseSettings.WEB_LINE_HEIGHT_PERCENT_LEGACY:
+            raise JsonableError(
+                _("Incompatible values for 'dense_mode' and 'web_line_height_percent' settings.")
+            )
+
+
 @human_users_only
 @has_request_variables
 def json_change_settings(
@@ -389,21 +417,12 @@ def json_change_settings(
             check_change_full_name(user_profile, full_name, user_profile)
 
     if (
-        dense_mode
+        dense_mode is not None
-        and web_font_size_px is not None
+        or web_font_size_px is not None
-        and web_font_size_px != UserProfile.WEB_FONT_SIZE_PX_LEGACY
+        or web_line_height_percent is not None
     ):
-        raise JsonableError(
+        check_information_density_setting_values(
-            _("Incompatible values for 'dense_mode' and 'web_font_size_px' settings.")
+            user_profile, dense_mode, web_font_size_px, web_line_height_percent
-        )
-
-    if (
-        dense_mode
-        and web_line_height_percent is not None
-        and web_line_height_percent != UserProfile.WEB_LINE_HEIGHT_PERCENT_LEGACY
-    ):
-        raise JsonableError(
-            _("Incompatible values for 'dense_mode' and 'web_line_height_percent' settings.")
         )
 
     # Loop over user_profile.property_types