From 808acc9e47d46defb64f41ce484caf751aea932a Mon Sep 17 00:00:00 2001 From: Prakhar Pratyush Date: Fri, 11 Oct 2024 10:08:17 +0530 Subject: [PATCH] events: Migrate plan_type & upload_quota to update_dict event format. 'realm_upload_quota_mib` is updated when `plan_type` changes. Earlier, we were including 'upload_quota' to update `realm_upload_quota_mib` in extra_data field of 'realm op: update' event format when property='plan_type'. This commit migrate those two parameters to `realm op: update_dict` event format. * None of the clients processes these fields, so no compatibility code required. * Renamed `upload_quota` to `upload_quota_mib` as it better aligns with our goal to encode units in the client-facing API names. Also, it helps to avoid extra code to update 'realm_upload_quota_mib` in web client, web client simply aligns with 'realm["realm_" + key] = value'. --- api_docs/changelog.md | 9 +++++++++ web/src/server_events_dispatch.js | 4 ++++ web/tests/dispatch.test.js | 4 ++++ web/tests/lib/events.js | 2 ++ zerver/actions/realm_settings.py | 17 ++++++++++------- zerver/lib/event_schema.py | 28 +++++++++++----------------- zerver/lib/events.py | 11 ++++------- zerver/openapi/zulip.yaml | 30 +++++++++++++++++------------- zerver/tests/test_events.py | 2 +- 9 files changed, 62 insertions(+), 45 deletions(-) diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 548274cadd..69a1098e60 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -20,6 +20,15 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 10.0 +**Feature level 306** + +* [`GET /events`](/api/get-events): Removed the `extra_data` optional + field from the `realm/update` event format, which was only used for + `plan_type` events, with a single `upload_quota` field. Now, we use + a standard `realm/update_dict` event to notify clients about changes + in `plan_type` and other fields that atomically change with a given + change in plan. + **Feature level 305** * [`POST /register`](/api/register-queue), [`GET /events`](/api/get-events), diff --git a/web/src/server_events_dispatch.js b/web/src/server_events_dispatch.js index cbd823c677..ef9951083c 100644 --- a/web/src/server_events_dispatch.js +++ b/web/src/server_events_dispatch.js @@ -317,6 +317,10 @@ export function dispatch_normal_event(event) { if (key === "edit_topic_policy") { message_live_update.rerender_messages_view(); } + + if (key === "plan_type") { + gear_menu.rerender(); + } } if (event.data.authentication_methods !== undefined) { settings_org.populate_auth_methods( diff --git a/web/tests/dispatch.test.js b/web/tests/dispatch.test.js index e364755cf3..c67206504f 100644 --- a/web/tests/dispatch.test.js +++ b/web/tests/dispatch.test.js @@ -584,6 +584,8 @@ run_test("realm settings", ({override}) => { override(realm, "realm_authentication_methods", {Google: {enabled: false, available: true}}); override(realm, "realm_can_create_public_channel_group", 1); override(realm, "realm_direct_message_permission_group", 1); + override(realm, "realm_plan_type", 2); + override(realm, "realm_upload_quota_mib", 5000); override(settings_org, "populate_auth_methods", noop); dispatch(event); assert_same(realm.realm_create_multiuse_invite_group, 3); @@ -595,6 +597,8 @@ run_test("realm settings", ({override}) => { }); assert_same(realm.realm_can_create_public_channel_group, 3); assert_same(realm.realm_direct_message_permission_group, 3); + assert_same(realm.realm_plan_type, 3); + assert_same(realm.realm_upload_quota_mib, 50000); assert_same(update_stream_privacy_choices_called, true); event = event_fixtures.realm__update_dict__icon; diff --git a/web/tests/lib/events.js b/web/tests/lib/events.js index 1a64127f92..92d40cefb8 100644 --- a/web/tests/lib/events.js +++ b/web/tests/lib/events.js @@ -370,6 +370,8 @@ exports.fixtures = { }, can_create_public_channel_group: 3, direct_message_permission_group: 3, + plan_type: 3, + upload_quota_mib: 50000, }, }, diff --git a/zerver/actions/realm_settings.py b/zerver/actions/realm_settings.py index 8541ec1463..0a10c204d1 100644 --- a/zerver/actions/realm_settings.py +++ b/zerver/actions/realm_settings.py @@ -28,6 +28,7 @@ from zerver.lib.user_groups import ( get_group_setting_value_for_api, get_group_setting_value_for_audit_log_data, ) +from zerver.lib.utils import optional_bytes_to_mib from zerver.models import ( ArchivedAttachment, Attachment, @@ -786,13 +787,15 @@ def do_change_realm_plan_type( realm.save(update_fields=["_max_invites", "message_visibility_limit"]) - event = { - "type": "realm", - "op": "update", - "property": "plan_type", - "value": plan_type, - "extra_data": {"upload_quota": realm.upload_quota_bytes()}, - } + event = dict( + type="realm", + op="update_dict", + property="default", + data={ + "plan_type": plan_type, + "upload_quota_mib": optional_bytes_to_mib(realm.upload_quota_bytes()), + }, + ) send_event_on_commit(realm, event, active_user_ids(realm.id)) diff --git a/zerver/lib/event_schema.py b/zerver/lib/event_schema.py index f6af8445e3..11dccafdf2 100644 --- a/zerver/lib/event_schema.py +++ b/zerver/lib/event_schema.py @@ -889,12 +889,6 @@ realm_linkifiers_event = event_dict_type( check_realm_linkifiers = make_checker(realm_linkifiers_event) -plan_type_extra_data_type = DictType( - required_keys=[ - ("upload_quota", int), - ] -) - """ realm/update events are flexible for values; we will use a more strict checker to check @@ -906,10 +900,7 @@ realm_update_event = event_dict_type( ("op", Equals("update")), ("property", str), ("value", value_type), - ], - optional_keys=[ - ("extra_data", plan_type_extra_data_type), - ], + ] ) _check_realm_update = make_checker(realm_update_event) @@ -935,13 +926,6 @@ def check_realm_update( assert prop == event["property"] value = event["value"] - if prop == "plan_type": - assert isinstance(value, int) - assert "extra_data" in event - return - - assert "extra_data" not in event - if prop in [ "new_stream_announcements_stream_id", "signup_announcements_stream_id", @@ -1087,6 +1071,13 @@ group_setting_update_data_type = DictType( ], ) +plan_type_data = DictType( + required_keys=[ + ("plan_type", int), + ("upload_quota_mib", OptionalType(int)), + ], +) + update_dict_data = UnionType( [ allow_message_editing_data, @@ -1097,6 +1088,7 @@ update_dict_data = UnionType( message_content_edit_limit_seconds_data, night_logo_data, group_setting_update_data_type, + plan_type_data, ] ) @@ -1133,6 +1125,8 @@ def check_realm_update_dict( setting_name in event["data"] for setting_name in Realm.REALM_PERMISSION_GROUP_SETTINGS ): sub_type = group_setting_update_data_type + elif "plan_type" in event["data"]: + sub_type = plan_type_data else: raise AssertionError("unhandled fields in data") diff --git a/zerver/lib/events.py b/zerver/lib/events.py index 6cfcde88d6..68cfd768b2 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -1279,13 +1279,6 @@ def apply_event( field = "realm_" + event["property"] state[field] = event["value"] - if event["property"] == "plan_type": - # Then there are some extra fields that also need to be set. - state["zulip_plan_is_not_limited"] = event["value"] != Realm.PLAN_TYPE_LIMITED - # upload_quota is in bytes, so we need to convert it to MiB. - upload_quota_bytes = event["extra_data"]["upload_quota"] - state["realm_upload_quota_mib"] = optional_bytes_to_mib(upload_quota_bytes) - if field == "realm_jitsi_server_url": state["jitsi_server_url"] = ( state["realm_jitsi_server_url"] @@ -1363,6 +1356,10 @@ def apply_event( or state["can_create_public_streams"] or state["can_create_web_public_streams"] ) + + if key == "plan_type": + # Then there are some extra fields that also need to be set. + state["zulip_plan_is_not_limited"] = value != Realm.PLAN_TYPE_LIMITED elif event["op"] == "deactivated": # The realm has just been deactivated. If our request had # arrived a moment later, we'd have rendered the diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 489a5190a8..9cd2cd3615 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -4064,6 +4064,12 @@ paths: A correct client implementation should convert these events into the corresponding [realm/update_dict](#realm-update_dict) event and then process that. + + **Changes**: Removed `extra_data` optional property in Zulip 10.0 + (feature level 306). The `extra_data` used to include an + `upload_quota` field when changed property was `plan_type`. The + server now sends a standard `realm/update_dict` event for plan + changes. properties: id: $ref: "#/components/schemas/EventIdSchema" @@ -4088,19 +4094,6 @@ paths: - type: string - type: boolean - type: integer - extra_data: - description: | - Object containing extra data related to the changed - property. - type: object - additionalProperties: false - properties: - upload_quota: - type: integer - description: | - Note: Only present if changed property is `plan_type`. - - The new upload quota for the Zulip organization. example: { "type": "realm", @@ -4907,6 +4900,17 @@ paths: **Changes**: In Zulip 9.0 (feature level 241), renamed `signup_notifications_stream_id` to `signup_announcements_stream_id`. + upload_quota_mib: + type: integer + nullable: true + description: | + The new upload quota for the Zulip organization. + + If `null`, there is no limit. + + **Changes**: New in Zulip 10.0 (feature level 306). Previously, + this was present changed via an `upload_quota` field in `extra_data` property + of [realm/update](#realm-update) event format for `plan_type` events. video_chat_provider: type: integer description: | diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index ff0246ba6b..5ab41b62b4 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -2772,7 +2772,7 @@ class NormalActionsTest(BaseAction): do_change_realm_plan_type(realm, Realm.PLAN_TYPE_LIMITED, acting_user=self.user_profile) check_realm_update("events[0]", events[0], "enable_spectator_access") check_realm_update_dict("events[1]", events[1]) - check_realm_update("events[2]", events[2], "plan_type") + check_realm_update_dict("events[2]", events[2]) state_data = fetch_initial_state_data(self.user_profile, realm=realm) self.assertEqual(state_data["realm_plan_type"], Realm.PLAN_TYPE_LIMITED)