From 1cccdd8103c8d14856a316f849c3613b0d199c0a Mon Sep 17 00:00:00 2001 From: Lauryn Menard Date: Mon, 17 Jul 2023 13:25:24 +0200 Subject: [PATCH] realm-settings: Make default_code_block_language empty string as default. Updates the realm field `default_code_block_language` to have a default value of an empty string instead of None. Also updates the web-app to check for the empty string and not `null` to indicate no default is set. This means that both new realms and existing realms that have no default set will have the same value for this setting: an empty string. Previously, new realms would have None if no default was set, while realms that had set and then unset a value for this field would have an empty string when no default was set. --- api_docs/changelog.md | 10 +++++ version.py | 2 +- web/src/composebox_typeahead.js | 2 +- zerver/lib/event_schema.py | 2 - ...alter_realm_default_code_block_language.py | 39 +++++++++++++++++++ zerver/models.py | 4 +- zerver/openapi/zulip.yaml | 23 ++++++++--- zerver/tests/test_markdown.py | 6 +-- zerver/views/realm.py | 7 ---- 9 files changed, 73 insertions(+), 22 deletions(-) create mode 100644 zerver/migrations/0461_alter_realm_default_code_block_language.py diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 70d8c01337..989dd75b6a 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -20,6 +20,16 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 8.0 +**Feature level 195** + +* [`GET /events`](/api/get-events), [`POST /register`](/api/register-queue): + The `default_code_block_language` realm setting is now consistently an + empty string when no default pygments language code is set. Previously, + the server had a bug that meant it might represent no default for this + realm setting as either `null` or an empty string. Clients supporting + older server versions should treat either value (`null` or `""`) as no + default being set. + **Feature level 194** * [`GET /messages`](/api/get-messages), diff --git a/version.py b/version.py index 60bcda16d4..fde290044b 100644 --- a/version.py +++ b/version.py @@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.9.3" # Changes should be accompanied by documentation explaining what the # new level means in api_docs/changelog.md, as well as "**Changes**" # entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 194 +API_FEATURE_LEVEL = 195 # 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/web/src/composebox_typeahead.js b/web/src/composebox_typeahead.js index 891652b782..ead1aeff05 100644 --- a/web/src/composebox_typeahead.js +++ b/web/src/composebox_typeahead.js @@ -1034,7 +1034,7 @@ function get_header_html() { tip_text = $t({defaultMessage: "Silent mentions do not trigger notifications."}); break; case "syntax": - if (page_params.realm_default_code_block_language !== null) { + if (page_params.realm_default_code_block_language !== "") { tip_text = $t( {defaultMessage: "Default is {language}. Use 'text' to disable highlighting."}, {language: page_params.realm_default_code_block_language}, diff --git a/zerver/lib/event_schema.py b/zerver/lib/event_schema.py index c68877f5c0..0d0fe81fb2 100644 --- a/zerver/lib/event_schema.py +++ b/zerver/lib/event_schema.py @@ -887,8 +887,6 @@ def check_realm_update( assert isinstance(value, property_type) elif property_type == (int, type(None)): assert isinstance(value, int) - elif property_type == (str, type(None)): - assert isinstance(value, str) else: raise AssertionError(f"Unexpected property type {property_type}") diff --git a/zerver/migrations/0461_alter_realm_default_code_block_language.py b/zerver/migrations/0461_alter_realm_default_code_block_language.py new file mode 100644 index 0000000000..be9acb919a --- /dev/null +++ b/zerver/migrations/0461_alter_realm_default_code_block_language.py @@ -0,0 +1,39 @@ +# Generated by Django 4.2.2 on 2023-07-11 14:04 + +from django.db import migrations, models +from django.db.backends.base.schema import BaseDatabaseSchemaEditor +from django.db.migrations.state import StateApps + + +def set_default_code_block_language_to_empty_string( + apps: StateApps, schema_editor: BaseDatabaseSchemaEditor +) -> None: + Realm = apps.get_model("zerver", "Realm") + Realm.objects.filter(default_code_block_language=None).update(default_code_block_language="") + + +class Migration(migrations.Migration): + dependencies = [ + ("zerver", "0460_backfill_realmauditlog_extradata_to_json_field"), + ] + + operations = [ + # Change default value from None to empty string for new realms + migrations.AlterField( + model_name="realm", + name="default_code_block_language", + field=models.TextField(null=True, default=""), + ), + # Update existing realms with None to have empty string + migrations.RunPython( + set_default_code_block_language_to_empty_string, + reverse_code=migrations.RunPython.noop, + elidable=True, + ), + # Remove null=True for this realm field + migrations.AlterField( + model_name="realm", + name="default_code_block_language", + field=models.TextField(default=""), + ), + ] diff --git a/zerver/models.py b/zerver/models.py index fa3788e043..f84e432489 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -710,7 +710,7 @@ class Realm(models.Model): # type: ignore[django-manager-missing] # django-stub # maximum rating of the GIFs that will be retrieved from GIPHY giphy_rating = models.PositiveSmallIntegerField(default=GIPHY_RATING_OPTIONS["g"]["id"]) - default_code_block_language = models.TextField(null=True, default=None) + default_code_block_language = models.TextField(default="") # Whether read receipts are enabled in the organization. If disabled, # they will not be available regardless of users' personal settings. @@ -726,7 +726,7 @@ class Realm(models.Model): # type: ignore[django-manager-missing] # django-stub create_private_stream_policy=int, create_public_stream_policy=int, create_web_public_stream_policy=int, - default_code_block_language=(str, type(None)), + default_code_block_language=str, default_language=str, delete_own_message_policy=int, description=str, diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 7194456c94..b565c3ced2 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -3961,10 +3961,17 @@ paths: **Changes**: New in Zulip 5.0 (feature level 103). default_code_block_language: type: string - nullable: true description: | - The default pygments language code to be used for a code blocks - in this organization. If `null`, no default has been set. + The default pygments language code to be used for code blocks in this + organization. If an empty string, no default has been set. + + **Changes**: Prior to Zulip 8.0 (feature level 195), a server bug meant + that both `null` and an empty string could represent that no default was + set for this realm setting in the [`POST /register`](/api/register-queue) + response. The documentation for both that endpoint and this event + incorrectly stated that the only representation for no default language + was `null`. This event in fact uses the empty string to indicate that no + default has been set in all server versions. default_language: type: string description: | @@ -13339,12 +13346,16 @@ paths: [calc-full-member]: /api/roles-and-permissions#determining-if-a-user-is-a-full-member realm_default_code_block_language: type: string - nullable: true description: | Present if `realm` is present in `fetch_event_types`. - The default pygments language code to be used for a code blocks - in this organization. If `null`, no default has been set. + The default pygments language code to be used for code blocks in this + organization. If an empty string, no default has been set. + + **Changes**: Prior to Zulip 8.0 (feature level 195), a server bug meant + that both `null` and an empty string could represent that no default was + set for this realm setting. Clients supporting older server versions + should treat either value (`null` or `""`) as no default being set. realm_message_content_delete_limit_seconds: type: integer nullable: true diff --git a/zerver/tests/test_markdown.py b/zerver/tests/test_markdown.py index 2e17370ce7..1459e37876 100644 --- a/zerver/tests/test_markdown.py +++ b/zerver/tests/test_markdown.py @@ -1773,7 +1773,7 @@ class MarkdownTest(ZulipTestCase): def test_default_code_block_language(self) -> None: realm = get_realm("zulip") - self.assertEqual(realm.default_code_block_language, None) + self.assertEqual(realm.default_code_block_language, "") text = "```{}\nconsole.log('Hello World');\n```\n" # Render without default language @@ -1803,7 +1803,7 @@ class MarkdownTest(ZulipTestCase): msg_without_language_default_math = markdown_convert_wrapper(text.format("")) # Render without default language - do_set_realm_property(realm, "default_code_block_language", None, acting_user=None) + do_set_realm_property(realm, "default_code_block_language", "", acting_user=None) msg_without_language_final = markdown_convert_wrapper(text.format("")) self.assertTrue(msg_with_js == msg_without_language_default_js) @@ -1822,7 +1822,7 @@ class MarkdownTest(ZulipTestCase): with_language, without_language = re.findall(r"
(.*?)$", rendered, re.MULTILINE)
         self.assertTrue(with_language == without_language)
 
-        do_set_realm_property(realm, "default_code_block_language", None, acting_user=None)
+        do_set_realm_property(realm, "default_code_block_language", "", acting_user=None)
         rendered = markdown_convert_wrapper(nested_text)
         with_language, without_language = re.findall(r"
(.*?)$", rendered, re.MULTILINE)
         self.assertFalse(with_language == without_language)
diff --git a/zerver/views/realm.py b/zerver/views/realm.py
index 77619cfea1..f57e3680ac 100644
--- a/zerver/views/realm.py
+++ b/zerver/views/realm.py
@@ -327,13 +327,6 @@ def update_realm(
         )
         data["signup_notifications_stream_id"] = signup_notifications_stream_id
 
-    if default_code_block_language is not None:
-        # Migrate '', used in the API to encode the default/None behavior of this feature.
-        if default_code_block_language == "":
-            data["default_code_block_language"] = None
-        else:
-            data["default_code_block_language"] = default_code_block_language
-
     if string_id is not None:
         if not user_profile.is_realm_owner:
             raise OrganizationOwnerRequiredError