From be653dd5b4231e01fd811475ad1acf33816c7170 Mon Sep 17 00:00:00 2001 From: Hemant Umre Date: Tue, 19 Sep 2023 22:33:08 +0530 Subject: [PATCH] org_settings: Add backend for `realm_jitsi_server_url` setting. This commit adds a `jitsi_server_url` field to the Realm model, which will be used to save the URL of the custom Jitsi Meet server. In the database, `None` will encode the server-level default. We can't readily use `None` in the API, as it could be confused with "field not sent". Therefore, we will use the string "default" for this purpose. We have also introduced `server_jitsi_server_url` in the `/register` API. This will be used to display the server's default Jitsi server URL in the settings UI. The existing `jitsi_server_url` will now be calculated as `realm_jitsi_server_url || server_jitsi_server_url`. Fixes a part of #17914. Co-authored-by: Gaurav Pandey --- api_docs/changelog.md | 11 ++++ version.py | 2 +- zerver/lib/event_schema.py | 2 + zerver/lib/events.py | 20 ++++++-- .../migrations/0475_realm_jitsi_server_url.py | 17 +++++++ zerver/models.py | 4 ++ zerver/openapi/zulip.yaml | 51 ++++++++++++++++++- zerver/tests/test_events.py | 1 + zerver/tests/test_home.py | 2 + zerver/tests/test_realm.py | 37 +++++++++++++- zerver/views/realm.py | 42 ++++++++++++++- 11 files changed, 181 insertions(+), 8 deletions(-) create mode 100644 zerver/migrations/0475_realm_jitsi_server_url.py diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 2944eb7e96..c8b0555df1 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -20,6 +20,17 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 8.0 +**Feature level 212** + +* [`GET /events`](/api/get-events), [`POST /register`](/api/register-queue), + `PATCH /realm`: Added the `jitsi_server_url` field to the `realm` object, + allowing organizations to set a custom Jitsi Meet server. Previously, this + was only available as a server-level configuration. + +* [`POST /register`](/api/register-queue): Added `server_jitsi_server_url` + fields to the `realm` object. The existing `jitsi_server_url` will now be + calculated as `realm_jitsi_server_url || server_jitsi_server_url`. + **Feature level 211** * [`POST /streams/{stream_id}/delete_topic`](/api/delete-topic), diff --git a/version.py b/version.py index 916688ec3a..5bd5801de6 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 = 211 +API_FEATURE_LEVEL = 212 # 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/lib/event_schema.py b/zerver/lib/event_schema.py index 8c5cd858ed..2be2ab1bba 100644 --- a/zerver/lib/event_schema.py +++ b/zerver/lib/event_schema.py @@ -866,6 +866,8 @@ 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/lib/events.py b/zerver/lib/events.py index 7e5c2ff00f..fe8e3a8bee 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -337,10 +337,15 @@ def fetch_initial_state_data( state["realm_push_notifications_enabled"] = push_notifications_enabled() state["realm_default_external_accounts"] = get_default_external_accounts() - if settings.JITSI_SERVER_URL is not None: - state["jitsi_server_url"] = settings.JITSI_SERVER_URL.rstrip("/") - else: # nocoverage - state["jitsi_server_url"] = None + server_default_jitsi_server_url = ( + settings.JITSI_SERVER_URL.rstrip("/") if settings.JITSI_SERVER_URL is not None else None + ) + state["server_jitsi_server_url"] = server_default_jitsi_server_url + state["jitsi_server_url"] = ( + realm.jitsi_server_url + if realm.jitsi_server_url is not None + else server_default_jitsi_server_url + ) if realm.notifications_stream and not realm.notifications_stream.deactivated: notifications_stream = realm.notifications_stream @@ -1068,6 +1073,13 @@ def apply_event( state["zulip_plan_is_not_limited"] = event["value"] != Realm.PLAN_TYPE_LIMITED state["realm_upload_quota_mib"] = event["extra_data"]["upload_quota"] + if field == "realm_jitsi_server_url": + state["jitsi_server_url"] = ( + state["realm_jitsi_server_url"] + if state["realm_jitsi_server_url"] is not None + else state["server_jitsi_server_url"] + ) + policy_permission_dict = { "create_public_stream_policy": "can_create_public_streams", "create_private_stream_policy": "can_create_private_streams", diff --git a/zerver/migrations/0475_realm_jitsi_server_url.py b/zerver/migrations/0475_realm_jitsi_server_url.py new file mode 100644 index 0000000000..6a396c04c4 --- /dev/null +++ b/zerver/migrations/0475_realm_jitsi_server_url.py @@ -0,0 +1,17 @@ +# Generated by Django 4.2.5 on 2023-09-19 17:02 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("zerver", "0474_realmuserdefault_web_stream_unreads_count_display_policy_and_more"), + ] + + operations = [ + migrations.AddField( + model_name="realm", + name="jitsi_server_url", + field=models.URLField(default=None, null=True), + ), + ] diff --git a/zerver/models.py b/zerver/models.py index 8f12dc3bfe..813554008c 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -671,6 +671,9 @@ class Realm(models.Model): # type: ignore[django-manager-missing] # django-stub default=VIDEO_CHAT_PROVIDERS["jitsi_meet"]["id"] ) + JITSI_SERVER_SPECIAL_VALUES_MAP = {"default": None} + jitsi_server_url = models.URLField(null=True, default=None) + # Please access this via get_giphy_rating_options. GIPHY_RATING_OPTIONS = { "disabled": { @@ -740,6 +743,7 @@ class Realm(models.Model): # type: ignore[django-manager-missing] # django-stub invite_required=bool, invite_to_realm_policy=int, invite_to_stream_policy=int, + jitsi_server_url=(str, type(None)), mandatory_topics=bool, message_content_allowed_in_email_notifications=bool, message_content_edit_limit_seconds=(int, type(None)), diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 58b8bac926..21cd04bd74 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -4365,6 +4365,19 @@ paths: **Changes**: None added as an option in Zulip 3.0 (feature level 1) to disable video call UI. + jitsi_server_url: + type: string + nullable: true + description: | + The URL of the custom Jitsi Meet server configured in this organization's + settings. + + `null`, the default, means that the organization is using the should use the + server-level configuration, `server_jitsi_server_url`. + + **Changes**: New in Zulip 8.0 (feature level 212). Previously, this was only + available as a server-level configuration, and required a server restart to + change. waiting_period_threshold: type: integer description: | @@ -13553,6 +13566,21 @@ paths: **Changes**: None added as an option in Zulip 3.0 (feature level 1) to disable video call UI. + realm_jitsi_server_url: + type: string + nullable: true + description: | + The URL of the custom Jitsi Meet server configured in this organization's + settings. + + `null`, the default, means that the organization is using the should use the + server-level configuration, `server_jitsi_server_url`. A correct client + supporting only the modern API should use `realm_jitsi_server_url || + server_jitsi_server_url` to create calls. + + **Changes**: New in Zulip 8.0 (feature level 212). Previously, this was only + available as a server-level configuration, which was available via the + `jitsi_server_url` field. realm_giphy_rating: type: integer description: | @@ -14009,10 +14037,21 @@ paths: on the external site. jitsi_server_url: type: string + deprecated: true description: | Present if `realm` is present in `fetch_event_types`. - The base URL the organization uses to create Jitsi video calls. + The base URL to be used to create Jitsi video calls. Equals + `realm_jitsi_server_url || server_jitsi_server_url`. + + **Changes**: Deprecated in Zulip 8.0 (feature level 212) and will + eventually be removed. Previously, the Jitsi server to use was not + configurable on a per-realm basis, and this field contained the server's + configured Jitsi server. (Which is now provided as + `server_jitsi_server_url`). Clients supporting older versions should fall + back to this field when creating calls: using `realm_jitsi_server_url || + server_jitsi_server_url` with newer servers and using `jitsi_server_url` + with servers below feature level 212. development_environment: type: boolean description: | @@ -14164,6 +14203,16 @@ paths: since the last request. **Changes**: New in Zulip 6.0 (feature level 140). + server_jitsi_server_url: + type: string + nullable: true + description: | + The URL of the Jitsi server that the Zulip server is configured to use by + default; the organization-level setting `realm_jitsi_server_url` takes + precedence over this setting when both are set. + + **Changes**: New in Zulip 8.0 (feature level 212). Previously, this value + was available as the now-deprecated `jitsi_server_url`. event_queue_longpoll_timeout_seconds: type: integer description: | diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index a9e2f3553d..b2f88b2062 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -2933,6 +2933,7 @@ class RealmPropertyActionTest(BaseAction): video_chat_provider=[ Realm.VIDEO_CHAT_PROVIDERS["jitsi_meet"]["id"], ], + jitsi_server_url=["https://jitsi1.example.com", "https://jitsi2.example.com"], giphy_rating=[ Realm.GIPHY_RATING_OPTIONS["disabled"]["id"], ], diff --git a/zerver/tests/test_home.py b/zerver/tests/test_home.py index db6f7211e7..a666d0ad9f 100644 --- a/zerver/tests/test_home.py +++ b/zerver/tests/test_home.py @@ -150,6 +150,7 @@ class HomeTest(ZulipTestCase): "realm_invite_to_realm_policy", "realm_invite_to_stream_policy", "realm_is_zephyr_mirror_realm", + "realm_jitsi_server_url", "realm_linkifiers", "realm_logo_source", "realm_logo_url", @@ -194,6 +195,7 @@ class HomeTest(ZulipTestCase): "server_generation", "server_inline_image_preview", "server_inline_url_embed_preview", + "server_jitsi_server_url", "server_name_changes_disabled", "server_needs_upgrade", "server_presence_offline_threshold_seconds", diff --git a/zerver/tests/test_realm.py b/zerver/tests/test_realm.py index 46e84afba9..13a93dd530 100644 --- a/zerver/tests/test_realm.py +++ b/zerver/tests/test_realm.py @@ -864,6 +864,36 @@ class RealmTest(ZulipTestCase): result = self.client_patch("/json/realm", req) self.assert_json_success(result) + def test_jitsi_server_url(self) -> None: + self.login("iago") + realm = get_realm("zulip") + self.assertEqual(realm.video_chat_provider, Realm.VIDEO_CHAT_PROVIDERS["jitsi_meet"]["id"]) + + req = dict(jitsi_server_url=orjson.dumps("").decode()) + result = self.client_patch("/json/realm", req) + self.assert_json_error(result, "jitsi_server_url is not an allowed_type") + + req = dict(jitsi_server_url=orjson.dumps("invalidURL").decode()) + result = self.client_patch("/json/realm", req) + self.assert_json_error(result, "jitsi_server_url is not an allowed_type") + + req = dict(jitsi_server_url=orjson.dumps(12).decode()) + result = self.client_patch("/json/realm", req) + self.assert_json_error(result, "jitsi_server_url is not an allowed_type") + + valid_url = "https://jitsi.example.com" + req = dict(jitsi_server_url=orjson.dumps(valid_url).decode()) + result = self.client_patch("/json/realm", req) + self.assert_json_success(result) + realm = get_realm("zulip") + self.assertEqual(realm.jitsi_server_url, valid_url) + + req = dict(jitsi_server_url=orjson.dumps("default").decode()) + result = self.client_patch("/json/realm", req) + self.assert_json_success(result) + realm = get_realm("zulip") + self.assertEqual(realm.jitsi_server_url, None) + def test_do_create_realm(self) -> None: realm = do_create_realm("realm_string_id", "realm name") @@ -1179,6 +1209,11 @@ class RealmAPITest(ZulipTestCase): ).decode(), ), ], + jitsi_server_url=[ + dict( + jitsi_server_url=orjson.dumps("https://example.jit.si").decode(), + ), + ], giphy_rating=[ Realm.GIPHY_RATING_OPTIONS["y"]["id"], Realm.GIPHY_RATING_OPTIONS["r"]["id"], @@ -1200,7 +1235,7 @@ class RealmAPITest(ZulipTestCase): if vals is None: raise AssertionError(f"No test created for {name}") - if name == "video_chat_provider": + if name in ("video_chat_provider", "jitsi_server_url"): self.set_up_db(name, vals[0][name]) realm = self.update_with_api_multiple_value(vals[0]) self.assertEqual(getattr(realm, name), orjson.loads(vals[0][name])) diff --git a/zerver/views/realm.py b/zerver/views/realm.py index ab020f0659..02deb51e54 100644 --- a/zerver/views/realm.py +++ b/zerver/views/realm.py @@ -1,4 +1,4 @@ -from typing import Any, Dict, Optional, Union +from typing import Any, Dict, Mapping, Optional, Union from django.core.exceptions import ValidationError from django.http import HttpRequest, HttpResponse @@ -37,12 +37,23 @@ from zerver.lib.validator import ( check_int_in, check_string_in, check_string_or_int, + check_union, + check_url, to_non_negative_int, ) from zerver.models import Realm, RealmReactivationStatus, RealmUserDefault, UserProfile from zerver.views.user_settings import check_settings_values +def parse_jitsi_server_url( + value: str, special_values_map: Mapping[str, Optional[str]] +) -> Optional[str]: + if value in special_values_map: + return special_values_map[value] + + return value + + @require_realm_admin @has_request_variables def update_realm( @@ -131,6 +142,13 @@ def update_realm( json_validator=check_int_in(Realm.WILDCARD_MENTION_POLICY_TYPES), default=None ), video_chat_provider: Optional[int] = REQ(json_validator=check_int, default=None), + jitsi_server_url_raw: Optional[str] = REQ( + "jitsi_server_url", + json_validator=check_union( + [check_string_in(list(Realm.JITSI_SERVER_SPECIAL_VALUES_MAP.keys())), check_url] + ), + default=None, + ), giphy_rating: Optional[int] = REQ(json_validator=check_int, default=None), default_code_block_language: Optional[str] = REQ(default=None), digest_weekday: Optional[int] = REQ( @@ -276,6 +294,28 @@ def update_realm( "move_messages_between_streams_limit_seconds" ] = move_messages_between_streams_limit_seconds + jitsi_server_url: Optional[str] = None + if jitsi_server_url_raw is not None: + jitsi_server_url = parse_jitsi_server_url( + jitsi_server_url_raw, + Realm.JITSI_SERVER_SPECIAL_VALUES_MAP, + ) + + # We handle the "None" case separately here because + # in the loop below, do_set_realm_property is called only when + # the setting value is not "None". For values other than "None", + # the loop itself sets the value of 'jitsi_server_url' by + # calling do_set_realm_property. + if jitsi_server_url is None and realm.jitsi_server_url is not None: + do_set_realm_property( + realm, + "jitsi_server_url", + jitsi_server_url, + acting_user=user_profile, + ) + + data["jitsi_server_url"] = jitsi_server_url + # The user of `locals()` here is a bit of a code smell, but it's # restricted to the elements present in realm.property_types. #