diff --git a/frontend_tests/node_tests/settings_org.js b/frontend_tests/node_tests/settings_org.js index 7695e9b754..c46e2a6858 100644 --- a/frontend_tests/node_tests/settings_org.js +++ b/frontend_tests/node_tests/settings_org.js @@ -247,11 +247,11 @@ function test_submit_settings_form(override, submit_form) { assert(patched); let expected_value = { - bot_creation_policy: "1", - invite_to_stream_policy: "1", - email_address_visibility: "1", + bot_creation_policy: 1, + invite_to_stream_policy: 1, + email_address_visibility: 1, add_emoji_by_admins_only: false, - create_stream_policy: "2", + create_stream_policy: 2, }; assert.deepEqual(data, expected_value); @@ -281,7 +281,7 @@ function test_submit_settings_form(override, submit_form) { assert(patched); expected_value = { - default_language: '"en"', + default_language: "en", default_twenty_four_hour_time: "true", }; assert.deepEqual(data, expected_value); diff --git a/static/js/settings_org.js b/static/js/settings_org.js index d7541eb51a..3ec49b995b 100644 --- a/static/js/settings_org.js +++ b/static/js/settings_org.js @@ -795,7 +795,8 @@ export function build_page() { } } else if (subsection === "other_settings") { const code_block_language_value = default_code_language_widget.value(); - data.default_code_block_language = JSON.stringify(code_block_language_value); + // No need to JSON-encode, since this value is already a string. + data.default_code_block_language = code_block_language_value; } else if (subsection === "other_permissions") { const add_emoji_permission = $("#id_realm_add_emoji_by_admins_only").val(); @@ -866,7 +867,7 @@ export function build_page() { const input_value = get_input_element_value(input_elem); if (input_value !== undefined) { const property_name = input_elem.attr("id").replace("id_realm_", ""); - data[property_name] = JSON.stringify(input_value); + data[property_name] = input_value; } } } diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index a6e4258a1e..358a0cab0e 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -10,6 +10,12 @@ below features are supported. ## Changes in Zulip 4.0 +**Feature level 52** + +* `PATCH /realm`: Removed unnecessary JSON-encoding of string + parameters `name`, `description`, `default_language`, and + `default_code_block_language`. + **Feature level 51** * [`POST /register`](/api/register-queue): Added a new boolean field diff --git a/version.py b/version.py index 83d4c70eca..e0d7adf7e5 100644 --- a/version.py +++ b/version.py @@ -30,7 +30,7 @@ DESKTOP_WARNING_VERSION = "5.2.0" # # Changes should be accompanied by documentation explaining what the # new level means in templates/zerver/api/changelog.md. -API_FEATURE_LEVEL = 51 +API_FEATURE_LEVEL = 52 # Bump the minor PROVISION_VERSION to indicate that folks should provision # only when going from an old version of the code to a newer version. Bump diff --git a/zerver/models.py b/zerver/models.py index 7229e72a59..db402e18e0 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -184,6 +184,7 @@ def clear_supported_auth_backends_cache() -> None: class Realm(models.Model): MAX_REALM_NAME_LENGTH = 40 + MAX_REALM_DESCRIPTION_LENGTH = 1000 MAX_REALM_SUBDOMAIN_LENGTH = 40 MAX_REALM_REDIRECT_URL_LENGTH = 128 diff --git a/zerver/tests/test_realm.py b/zerver/tests/test_realm.py index 8f6c89770d..0b9b6cfa3c 100644 --- a/zerver/tests/test_realm.py +++ b/zerver/tests/test_realm.py @@ -1,6 +1,6 @@ import datetime import re -from typing import Any, Dict, List, Mapping +from typing import Any, Dict, List, Mapping, Union from unittest import mock import orjson @@ -103,7 +103,7 @@ class RealmTest(ZulipTestCase): def test_update_realm_description(self) -> None: self.login("iago") new_description = "zulip dev group" - data = dict(description=orjson.dumps(new_description).decode()) + data = dict(description=new_description) events: List[Mapping[str, Any]] = [] with tornado_redirected_to_list(events): result = self.client_patch("/json/realm", data) @@ -124,25 +124,25 @@ class RealmTest(ZulipTestCase): def test_realm_description_length(self) -> None: new_description = "A" * 1001 - data = dict(description=orjson.dumps(new_description).decode()) + data = dict(description=new_description) # create an admin user self.login("iago") result = self.client_patch("/json/realm", data) - self.assert_json_error(result, "Organization description is too long.") + self.assert_json_error(result, "description is too long (limit: 1000 characters)") realm = get_realm("zulip") self.assertNotEqual(realm.description, new_description) def test_realm_name_length(self) -> None: new_name = "A" * (Realm.MAX_REALM_NAME_LENGTH + 1) - data = dict(name=orjson.dumps(new_name).decode()) + data = dict(name=new_name) # create an admin user self.login("iago") result = self.client_patch("/json/realm", data) - self.assert_json_error(result, "Organization name is too long.") + self.assert_json_error(result, "name is too long (limit: 40 characters)") realm = get_realm("zulip") self.assertNotEqual(realm.name, new_name) @@ -151,7 +151,7 @@ class RealmTest(ZulipTestCase): self.login("othello") - req = dict(name=orjson.dumps(new_name).decode()) + req = dict(name=new_name) result = self.client_patch("/json/realm", req) self.assert_json_error(result, "Must be an organization administrator") @@ -419,7 +419,7 @@ class RealmTest(ZulipTestCase): # we need an admin user. self.login("iago") - req = dict(default_language=orjson.dumps(new_lang).decode()) + req = dict(default_language=new_lang) result = self.client_patch("/json/realm", req) self.assert_json_success(result) realm = get_realm("zulip") @@ -429,7 +429,7 @@ class RealmTest(ZulipTestCase): # as the default realm language, correct validation error is # raised and the invalid language is not saved in db invalid_lang = "invalid_lang" - req = dict(default_language=orjson.dumps(invalid_lang).decode()) + req = dict(default_language=invalid_lang) result = self.client_patch("/json/realm", req) self.assert_json_error(result, f"Invalid language '{invalid_lang}'") realm = get_realm("zulip") @@ -820,8 +820,10 @@ class RealmAPITest(ZulipTestCase): setattr(realm, attr, value) realm.save(update_fields=[attr]) - def update_with_api(self, name: str, value: int) -> Realm: - result = self.client_patch("/json/realm", {name: orjson.dumps(value).decode()}) + def update_with_api(self, name: str, value: Union[int, str]) -> Realm: + if not isinstance(value, str): + value = orjson.dumps(value).decode() + result = self.client_patch("/json/realm", {name: value}) self.assert_json_success(result) return get_realm("zulip") # refresh data diff --git a/zerver/views/realm.py b/zerver/views/realm.py index 41fdf712cb..effa8239a4 100644 --- a/zerver/views/realm.py +++ b/zerver/views/realm.py @@ -26,10 +26,10 @@ from zerver.lib.retention import parse_message_retention_days from zerver.lib.streams import access_stream_by_id from zerver.lib.validator import ( check_bool, + check_capped_string, check_dict, check_int, check_int_in, - check_string, check_string_or_int, to_non_negative_int, ) @@ -41,8 +41,12 @@ from zerver.models import Realm, UserProfile def update_realm( request: HttpRequest, user_profile: UserProfile, - name: Optional[str] = REQ(json_validator=check_string, default=None), - description: Optional[str] = REQ(json_validator=check_string, default=None), + name: Optional[str] = REQ( + str_validator=check_capped_string(Realm.MAX_REALM_NAME_LENGTH), default=None + ), + description: Optional[str] = REQ( + str_validator=check_capped_string(Realm.MAX_REALM_DESCRIPTION_LENGTH), default=None + ), emails_restricted_to_domains: Optional[bool] = REQ(json_validator=check_bool, default=None), disallow_disposable_email_addresses: Optional[bool] = REQ( json_validator=check_bool, default=None @@ -68,7 +72,7 @@ def update_realm( converter=to_non_negative_int, default=None ), allow_edit_history: Optional[bool] = REQ(json_validator=check_bool, default=None), - default_language: Optional[str] = REQ(json_validator=check_string, default=None), + default_language: Optional[str] = REQ(default=None), waiting_period_threshold: Optional[int] = REQ(converter=to_non_negative_int, default=None), authentication_methods: Optional[Dict[str, Any]] = REQ( json_validator=check_dict([]), default=None @@ -106,7 +110,7 @@ def update_realm( ), default_twenty_four_hour_time: Optional[bool] = REQ(json_validator=check_bool, default=None), video_chat_provider: Optional[int] = REQ(json_validator=check_int, default=None), - default_code_block_language: Optional[str] = REQ(json_validator=check_string, default=None), + default_code_block_language: Optional[str] = REQ(default=None), digest_weekday: Optional[int] = REQ( json_validator=check_int_in(Realm.DIGEST_WEEKDAY_VALUES), default=None ), @@ -117,10 +121,6 @@ def update_realm( # the entire request can succeed or fail atomically. if default_language is not None and default_language not in get_available_language_codes(): raise JsonableError(_("Invalid language '{}'").format(default_language)) - if description is not None and len(description) > 1000: - return json_error(_("Organization description is too long.")) - if name is not None and len(name) > Realm.MAX_REALM_NAME_LENGTH: - return json_error(_("Organization name is too long.")) if authentication_methods is not None: if not user_profile.is_realm_owner: raise OrganizationOwnerRequired()