diff --git a/zerver/actions/realm_settings.py b/zerver/actions/realm_settings.py index b93a4709fc..6b58eee339 100644 --- a/zerver/actions/realm_settings.py +++ b/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", diff --git a/zerver/actions/user_settings.py b/zerver/actions/user_settings.py index 334806d1d2..b2ec7a92a3 100644 --- a/zerver/actions/user_settings.py +++ b/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. diff --git a/zerver/tests/test_realm.py b/zerver/tests/test_realm.py index 9625edf72c..d774ed1ae9 100644 --- a/zerver/tests/test_realm.py +++ b/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} - result = self.client_patch("/json/realm/user_settings_defaults", invalid_data) + data = {"web_line_height_percent": 130} + 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} - result = self.client_patch("/json/realm/user_settings_defaults", invalid_data) + data = {"dense_mode": orjson.dumps(True).decode(), "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 = { + "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"} diff --git a/zerver/tests/test_settings.py b/zerver/tests/test_settings.py index 286cd19e48..a825d246f2 100644 --- a/zerver/tests/test_settings.py +++ b/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} - result = self.client_patch("/json/settings", invalid_data) + data = {"web_line_height_percent": 130} + 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} - result = self.client_patch("/json/settings", invalid_data) + data = {"dense_mode": orjson.dumps(True).decode(), "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 = { + "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: diff --git a/zerver/views/realm.py b/zerver/views/realm.py index 595bad9c74..ab2dd7029d 100644 --- a/zerver/views/realm.py +++ b/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: diff --git a/zerver/views/user_settings.py b/zerver/views/user_settings.py index 508b7ce7eb..99fd8dfe76 100644 --- a/zerver/views/user_settings.py +++ b/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 - and web_font_size_px is not None - and web_font_size_px != UserProfile.WEB_FONT_SIZE_PX_LEGACY + dense_mode is not None + or web_font_size_px is not None + or web_line_height_percent is not None ): - 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 != UserProfile.WEB_LINE_HEIGHT_PERCENT_LEGACY - ): - raise JsonableError( - _("Incompatible values for 'dense_mode' and 'web_line_height_percent' settings.") + check_information_density_setting_values( + user_profile, dense_mode, web_font_size_px, web_line_height_percent ) # Loop over user_profile.property_types