user_groups: Add client capability to handle deactivated groups.

This commit adds a client capability to not receive data about
deactivated groups.
This commit is contained in:
Sahil Batra 2024-09-12 15:16:48 +05:30 committed by Tim Abbott
parent aa123b38b4
commit fb63c47ea6
8 changed files with 131 additions and 1 deletions

View File

@ -20,6 +20,23 @@ format used by the Zulip server that they are interacting with.
## Changes in Zulip 10.0 ## Changes in Zulip 10.0
**Feature level 294**
* [`POST /register`](/api/register-queue): Clients that do not
support the `include_deactivated_groups`
[client capability](/api/register-queue#parameter-client_capabilities)
do not receive deactivated user groups in the response.
* [`GET /events`](/api/get-events): Clients that do not support the
`include_deactivated_groups`
[client capability](/api/register-queue#parameter-client_capabilities)
receive `remove` event on user group deactivation instead of `update`
event.
* [`GET /events`](/api/get-events): Clients that do not support the
`include_deactivated_groups`
[client capability](/api/register-queue#parameter-client_capabilities)
do not receive `update` event when name of a deactivated user group
is updated.
**Feature level 293** **Feature level 293**
* [`POST /register`](/api/register-queue), [`PATCH /settings`](/api/update-settings): * [`POST /register`](/api/register-queue), [`PATCH /settings`](/api/update-settings):

View File

@ -224,6 +224,12 @@ def do_send_user_group_update_event(
user_group: NamedUserGroup, data: dict[str, str | int | AnonymousSettingGroupDict] user_group: NamedUserGroup, data: dict[str, str | int | AnonymousSettingGroupDict]
) -> None: ) -> None:
event = dict(type="user_group", op="update", group_id=user_group.id, data=data) event = dict(type="user_group", op="update", group_id=user_group.id, data=data)
if "name" in data:
# This field will be popped eventually before sending the event
# to client, but is needed to make sure we do not send the
# name update event for deactivated groups to client with
# 'include_deactivated_groups' client capability set to false.
event["deactivated"] = user_group.deactivated
send_event_on_commit(user_group.realm, event, active_user_ids(user_group.realm_id)) send_event_on_commit(user_group.realm, event, active_user_ids(user_group.realm_id))
@ -463,6 +469,9 @@ def do_deactivate_user_group(
do_send_user_group_update_event(user_group, dict(deactivated=True)) do_send_user_group_update_event(user_group, dict(deactivated=True))
event = dict(type="user_group", op="remove", group_id=user_group.id)
send_event_on_commit(user_group.realm, event, active_user_ids(user_group.realm_id))
@transaction.atomic(savepoint=False) @transaction.atomic(savepoint=False)
def do_change_user_group_permission_setting( def do_change_user_group_permission_setting(

View File

@ -141,6 +141,7 @@ def fetch_initial_state_data(
pronouns_field_type_supported: bool = True, pronouns_field_type_supported: bool = True,
linkifier_url_template: bool = False, linkifier_url_template: bool = False,
user_list_incomplete: bool = False, user_list_incomplete: bool = False,
include_deactivated_groups: bool = False,
) -> dict[str, Any]: ) -> dict[str, Any]:
"""When `event_types` is None, fetches the core data powering the """When `event_types` is None, fetches the core data powering the
web app's `page_params` and `/api/v1/register` (for mobile/terminal web app's `page_params` and `/api/v1/register` (for mobile/terminal
@ -507,7 +508,9 @@ def fetch_initial_state_data(
state["realm_playgrounds"] = get_realm_playgrounds(realm) state["realm_playgrounds"] = get_realm_playgrounds(realm)
if want("realm_user_groups"): if want("realm_user_groups"):
state["realm_user_groups"] = user_groups_in_realm_serialized(realm, allow_deactivated=True) state["realm_user_groups"] = user_groups_in_realm_serialized(
realm, allow_deactivated=include_deactivated_groups
)
if user_profile is not None: if user_profile is not None:
settings_user = user_profile settings_user = user_profile
@ -782,6 +785,7 @@ def apply_events(
include_subscribers: bool, include_subscribers: bool,
linkifier_url_template: bool, linkifier_url_template: bool,
user_list_incomplete: bool, user_list_incomplete: bool,
include_deactivated_groups: bool,
) -> None: ) -> None:
for event in events: for event in events:
if fetch_event_types is not None and event["type"] not in fetch_event_types: if fetch_event_types is not None and event["type"] not in fetch_event_types:
@ -803,6 +807,7 @@ def apply_events(
include_subscribers=include_subscribers, include_subscribers=include_subscribers,
linkifier_url_template=linkifier_url_template, linkifier_url_template=linkifier_url_template,
user_list_incomplete=user_list_incomplete, user_list_incomplete=user_list_incomplete,
include_deactivated_groups=include_deactivated_groups,
) )
@ -816,6 +821,7 @@ def apply_event(
include_subscribers: bool, include_subscribers: bool,
linkifier_url_template: bool, linkifier_url_template: bool,
user_list_incomplete: bool, user_list_incomplete: bool,
include_deactivated_groups: bool,
) -> None: ) -> None:
if event["type"] == "message": if event["type"] == "message":
state["max_message_id"] = max(state["max_message_id"], event["message"]["id"]) state["max_message_id"] = max(state["max_message_id"], event["message"]["id"])
@ -1662,6 +1668,7 @@ class ClientCapabilities(TypedDict):
user_settings_object: NotRequired[bool] user_settings_object: NotRequired[bool]
linkifier_url_template: NotRequired[bool] linkifier_url_template: NotRequired[bool]
user_list_incomplete: NotRequired[bool] user_list_incomplete: NotRequired[bool]
include_deactivated_groups: NotRequired[bool]
def do_events_register( def do_events_register(
@ -1698,6 +1705,7 @@ def do_events_register(
user_settings_object = client_capabilities.get("user_settings_object", False) user_settings_object = client_capabilities.get("user_settings_object", False)
linkifier_url_template = client_capabilities.get("linkifier_url_template", False) linkifier_url_template = client_capabilities.get("linkifier_url_template", False)
user_list_incomplete = client_capabilities.get("user_list_incomplete", False) user_list_incomplete = client_capabilities.get("user_list_incomplete", False)
include_deactivated_groups = client_capabilities.get("include_deactivated_groups", False)
if fetch_event_types is not None: if fetch_event_types is not None:
event_types_set: set[str] | None = set(fetch_event_types) event_types_set: set[str] | None = set(fetch_event_types)
@ -1739,6 +1747,7 @@ def do_events_register(
# Force include_streams=False for security reasons. # Force include_streams=False for security reasons.
include_streams=include_streams, include_streams=include_streams,
spectator_requested_language=spectator_requested_language, spectator_requested_language=spectator_requested_language,
include_deactivated_groups=include_deactivated_groups,
) )
post_process_state(user_profile, ret, notification_settings_null=False) post_process_state(user_profile, ret, notification_settings_null=False)
@ -1767,6 +1776,7 @@ def do_events_register(
pronouns_field_type_supported=pronouns_field_type_supported, pronouns_field_type_supported=pronouns_field_type_supported,
linkifier_url_template=linkifier_url_template, linkifier_url_template=linkifier_url_template,
user_list_incomplete=user_list_incomplete, user_list_incomplete=user_list_incomplete,
include_deactivated_groups=include_deactivated_groups,
) )
if queue_id is None: if queue_id is None:
@ -1788,6 +1798,7 @@ def do_events_register(
pronouns_field_type_supported=pronouns_field_type_supported, pronouns_field_type_supported=pronouns_field_type_supported,
linkifier_url_template=linkifier_url_template, linkifier_url_template=linkifier_url_template,
user_list_incomplete=user_list_incomplete, user_list_incomplete=user_list_incomplete,
include_deactivated_groups=include_deactivated_groups,
) )
# Apply events that came in while we were fetching initial data # Apply events that came in while we were fetching initial data
@ -1802,6 +1813,7 @@ def do_events_register(
include_subscribers=include_subscribers, include_subscribers=include_subscribers,
linkifier_url_template=linkifier_url_template, linkifier_url_template=linkifier_url_template,
user_list_incomplete=user_list_incomplete, user_list_incomplete=user_list_incomplete,
include_deactivated_groups=include_deactivated_groups,
) )
post_process_state(user_profile, ret, notification_settings_null) post_process_state(user_profile, ret, notification_settings_null)

View File

@ -3163,6 +3163,14 @@ paths:
description: | description: |
Event sent to all users in a Zulip organization Event sent to all users in a Zulip organization
when a property of a user group is changed. when a property of a user group is changed.
For group deactivation, this event is only sent
if `include_deactivated_groups` client capability
is set to `true`.
**Changes**: Prior to Zulip 10.0 (feature level 294),
this event was sent to all clients when a user group
was deactivated.
properties: properties:
id: id:
$ref: "#/components/schemas/EventIdSchema" $ref: "#/components/schemas/EventIdSchema"
@ -3410,6 +3418,13 @@ paths:
additionalProperties: false additionalProperties: false
description: | description: |
Event sent to all users when a user group has been deleted. Event sent to all users when a user group has been deleted.
This event is also sent when a user group is deactivated
but only to clients with `include_deactivated_groups` client
capability set to `false`.
**Changes**: Prior to Zulip 10.0 (feature level 294), this
event was only sent when a user group was deleted.
properties: properties:
id: id:
$ref: "#/components/schemas/EventIdSchema" $ref: "#/components/schemas/EventIdSchema"
@ -13485,6 +13500,16 @@ paths:
**Changes**: New in Zulip 8.0 (feature level 232). This **Changes**: New in Zulip 8.0 (feature level 232). This
capability is for backwards-compatibility. capability is for backwards-compatibility.
- `include_deactivated_groups`: Boolean for whether the client can handle
deactivated user groups by themselves. If false, then the `realm_user_groups`
array in the `/register` response will only include active groups, clients
will receive a `remove` event instead of `update` event when a group is
deactivated and no `update` event will be sent to the client if a deactivated
user group is renamed.
<br />
**Changes**: New in Zulip 10.0 (feature level 294). This
capability is for backwards-compatibility.
[help-linkifiers]: /help/add-a-custom-linkifier [help-linkifiers]: /help/add-a-custom-linkifier
[rfc6570]: https://www.rfc-editor.org/rfc/rfc6570.html [rfc6570]: https://www.rfc-editor.org/rfc/rfc6570.html
[events-linkifiers]: /api/get-events#realm_linkifiers [events-linkifiers]: /api/get-events#realm_linkifiers
@ -14021,6 +14046,12 @@ paths:
An array of dictionaries where each dictionary describes a An array of dictionaries where each dictionary describes a
[user group](/help/user-groups) in the Zulip organization. [user group](/help/user-groups) in the Zulip organization.
Deactivated groups will only be included if `include_deactivated_groups`
client capability is set to `true`.
**Changes**: Prior to Zulip 10.0 (feature level 294), deactivated
groups were included for all the clients.
realm_bots: realm_bots:
type: array type: array
items: items:

View File

@ -299,6 +299,7 @@ class BaseAction(ZulipTestCase):
linkifier_url_template: bool = True, linkifier_url_template: bool = True,
user_list_incomplete: bool = False, user_list_incomplete: bool = False,
client_is_old: bool = False, client_is_old: bool = False,
include_deactivated_groups: bool = False,
) -> Iterator[list[dict[str, Any]]]: ) -> Iterator[list[dict[str, Any]]]:
""" """
Make sure we have a clean slate of client descriptors for these tests. Make sure we have a clean slate of client descriptors for these tests.
@ -329,6 +330,7 @@ class BaseAction(ZulipTestCase):
pronouns_field_type_supported=pronouns_field_type_supported, pronouns_field_type_supported=pronouns_field_type_supported,
linkifier_url_template=linkifier_url_template, linkifier_url_template=linkifier_url_template,
user_list_incomplete=user_list_incomplete, user_list_incomplete=user_list_incomplete,
include_deactivated_groups=include_deactivated_groups,
) )
) )
@ -346,6 +348,7 @@ class BaseAction(ZulipTestCase):
pronouns_field_type_supported=pronouns_field_type_supported, pronouns_field_type_supported=pronouns_field_type_supported,
linkifier_url_template=linkifier_url_template, linkifier_url_template=linkifier_url_template,
user_list_incomplete=user_list_incomplete, user_list_incomplete=user_list_incomplete,
include_deactivated_groups=include_deactivated_groups,
) )
if client_is_old: if client_is_old:
@ -386,6 +389,7 @@ class BaseAction(ZulipTestCase):
include_subscribers=include_subscribers, include_subscribers=include_subscribers,
linkifier_url_template=linkifier_url_template, linkifier_url_template=linkifier_url_template,
user_list_incomplete=user_list_incomplete, user_list_incomplete=user_list_incomplete,
include_deactivated_groups=include_deactivated_groups,
) )
post_process_state(self.user_profile, hybrid_state, notification_settings_null) post_process_state(self.user_profile, hybrid_state, notification_settings_null)
after = orjson.dumps(hybrid_state) after = orjson.dumps(hybrid_state)
@ -415,6 +419,7 @@ class BaseAction(ZulipTestCase):
pronouns_field_type_supported=pronouns_field_type_supported, pronouns_field_type_supported=pronouns_field_type_supported,
linkifier_url_template=linkifier_url_template, linkifier_url_template=linkifier_url_template,
user_list_incomplete=user_list_incomplete, user_list_incomplete=user_list_incomplete,
include_deactivated_groups=include_deactivated_groups,
) )
post_process_state(self.user_profile, normal_state, notification_settings_null) post_process_state(self.user_profile, normal_state, notification_settings_null)
self.match_states(hybrid_state, normal_state, events) self.match_states(hybrid_state, normal_state, events)
@ -1924,8 +1929,23 @@ class NormalActionsTest(BaseAction):
# Test deactivate event # Test deactivate event
with self.verify_action() as events: with self.verify_action() as events:
do_deactivate_user_group(backend, acting_user=None) do_deactivate_user_group(backend, acting_user=None)
check_user_group_remove("events[0]", events[0])
with self.verify_action(include_deactivated_groups=True) as events:
do_deactivate_user_group(api_design, acting_user=None)
check_user_group_update("events[0]", events[0], "deactivated") check_user_group_update("events[0]", events[0], "deactivated")
with self.verify_action(num_events=0, state_change_expected=False):
do_update_user_group_name(api_design, "api-deisgn-team", acting_user=None)
with self.verify_action(include_deactivated_groups=True) as events:
do_update_user_group_name(api_design, "api-deisgn", acting_user=None)
check_user_group_update("events[0]", events[0], "name")
# Reactivate the group to test "remove" event.
backend.deactivated = False
backend.save()
# Test remove event # Test remove event
with self.verify_action() as events: with self.verify_action() as events:
check_delete_user_group(backend, acting_user=othello) check_delete_user_group(backend, acting_user=othello)

View File

@ -90,6 +90,7 @@ def request_event_queue(
pronouns_field_type_supported: bool = True, pronouns_field_type_supported: bool = True,
linkifier_url_template: bool = False, linkifier_url_template: bool = False,
user_list_incomplete: bool = False, user_list_incomplete: bool = False,
include_deactivated_groups: bool = False,
) -> str | None: ) -> str | None:
if not settings.USING_TORNADO: if not settings.USING_TORNADO:
return None return None
@ -113,6 +114,7 @@ def request_event_queue(
"pronouns_field_type_supported": orjson.dumps(pronouns_field_type_supported), "pronouns_field_type_supported": orjson.dumps(pronouns_field_type_supported),
"linkifier_url_template": orjson.dumps(linkifier_url_template), "linkifier_url_template": orjson.dumps(linkifier_url_template),
"user_list_incomplete": orjson.dumps(user_list_incomplete), "user_list_incomplete": orjson.dumps(user_list_incomplete),
"include_deactivated_groups": orjson.dumps(include_deactivated_groups),
} }
if event_types is not None: if event_types is not None:

View File

@ -78,6 +78,7 @@ class ClientDescriptor:
pronouns_field_type_supported: bool = True, pronouns_field_type_supported: bool = True,
linkifier_url_template: bool = False, linkifier_url_template: bool = False,
user_list_incomplete: bool = False, user_list_incomplete: bool = False,
include_deactivated_groups: bool = False,
) -> None: ) -> None:
# TODO: We eventually want to upstream this code to the caller, but # TODO: We eventually want to upstream this code to the caller, but
# serialization concerns make it a bit difficult. # serialization concerns make it a bit difficult.
@ -108,6 +109,7 @@ class ClientDescriptor:
self.pronouns_field_type_supported = pronouns_field_type_supported self.pronouns_field_type_supported = pronouns_field_type_supported
self.linkifier_url_template = linkifier_url_template self.linkifier_url_template = linkifier_url_template
self.user_list_incomplete = user_list_incomplete self.user_list_incomplete = user_list_incomplete
self.include_deactivated_groups = include_deactivated_groups
# Default for lifespan_secs is DEFAULT_EVENT_QUEUE_TIMEOUT_SECS; # Default for lifespan_secs is DEFAULT_EVENT_QUEUE_TIMEOUT_SECS;
# but users can set it as high as MAX_QUEUE_TIMEOUT_SECS. # but users can set it as high as MAX_QUEUE_TIMEOUT_SECS.
@ -138,6 +140,7 @@ class ClientDescriptor:
pronouns_field_type_supported=self.pronouns_field_type_supported, pronouns_field_type_supported=self.pronouns_field_type_supported,
linkifier_url_template=self.linkifier_url_template, linkifier_url_template=self.linkifier_url_template,
user_list_incomplete=self.user_list_incomplete, user_list_incomplete=self.user_list_incomplete,
include_deactivated_groups=self.include_deactivated_groups,
) )
@override @override
@ -174,6 +177,7 @@ class ClientDescriptor:
d.get("pronouns_field_type_supported", True), d.get("pronouns_field_type_supported", True),
d.get("linkifier_url_template", False), d.get("linkifier_url_template", False),
d.get("user_list_incomplete", False), d.get("user_list_incomplete", False),
d.get("include_deactivated_groups", False),
) )
ret.last_connection_time = d["last_connection_time"] ret.last_connection_time = d["last_connection_time"]
return ret return ret
@ -231,6 +235,16 @@ class ClientDescriptor:
# events are sent only if user_settings_object is False, # events are sent only if user_settings_object is False,
# otherwise only 'user_settings' event is sent. # otherwise only 'user_settings' event is sent.
return False return False
if event["type"] == "user_group":
if event["op"] == "remove":
# 'user_group/remove' events are only sent if the client
# cannot filter out deactivated groups by themselves.
return not self.include_deactivated_groups
if event["op"] == "update" and "deactivated" in event["data"]:
# 'update' events for group deactivation are only sent to
# clients who can filter out deactivated groups by themselves.
# Other clients receive 'remove' event.
return self.include_deactivated_groups
return True return True
# TODO: Refactor so we don't need this function # TODO: Refactor so we don't need this function
@ -1556,6 +1570,21 @@ def reformat_legacy_send_message_event(
return (modern_event, user_dicts) return (modern_event, user_dicts)
def process_user_group_name_update_event(event: Mapping[str, Any], users: Iterable[int]) -> None:
user_group_event = dict(event)
# 'deactivated' field is no longer needed and can be popped, as we now
# know whether the group that was renamed is deactivated or not and can
# avoid sending the event to client with 'include_deactivated_groups'
# client capability set to false.
event_for_deactivated_group = user_group_event.pop("deactivated", False)
for user_profile_id in users:
for client in get_client_descriptors_for_user(user_profile_id):
if client.accepts_event(user_group_event):
if event_for_deactivated_group and not client.include_deactivated_groups:
continue
client.add_event(user_group_event)
def process_notification(notice: Mapping[str, Any]) -> None: def process_notification(notice: Mapping[str, Any]) -> None:
event: Mapping[str, Any] = notice["event"] event: Mapping[str, Any] = notice["event"]
users: list[int] | list[Mapping[str, Any]] = notice["users"] users: list[int] | list[Mapping[str, Any]] = notice["users"]
@ -1579,6 +1608,11 @@ def process_notification(notice: Mapping[str, Any]) -> None:
process_custom_profile_fields_event(event, cast(list[int], users)) process_custom_profile_fields_event(event, cast(list[int], users))
elif event["type"] == "realm_user" and event["op"] == "add": elif event["type"] == "realm_user" and event["op"] == "add":
process_realm_user_add_event(event, cast(list[int], users)) process_realm_user_add_event(event, cast(list[int], users))
elif event["type"] == "user_group" and event["op"] == "update" and "name" in event["data"]:
# Only name can be changed for deactivated groups, so we handle the
# event sent for updating name separately for clients with different
# capabilities.
process_user_group_name_update_event(event, cast(list[int], users))
elif event["type"] == "cleanup_queue": elif event["type"] == "cleanup_queue":
# cleanup_event_queue may generate this event to forward cleanup # cleanup_event_queue may generate this event to forward cleanup
# requests to the right shard. # requests to the right shard.

View File

@ -206,6 +206,10 @@ def get_events_backend(
Json[bool], Json[bool],
ApiParamConfig(documentation_status=DocumentationStatus.INTENTIONALLY_UNDOCUMENTED), ApiParamConfig(documentation_status=DocumentationStatus.INTENTIONALLY_UNDOCUMENTED),
] = False, ] = False,
include_deactivated_groups: Annotated[
Json[bool],
ApiParamConfig(documentation_status=DocumentationStatus.INTENTIONALLY_UNDOCUMENTED),
] = False,
) -> HttpResponse: ) -> HttpResponse:
if narrow is None: if narrow is None:
narrow = [] narrow = []
@ -243,6 +247,7 @@ def get_events_backend(
pronouns_field_type_supported=pronouns_field_type_supported, pronouns_field_type_supported=pronouns_field_type_supported,
linkifier_url_template=linkifier_url_template, linkifier_url_template=linkifier_url_template,
user_list_incomplete=user_list_incomplete, user_list_incomplete=user_list_incomplete,
include_deactivated_groups=include_deactivated_groups,
) )
result = in_tornado_thread(fetch_events)( result = in_tornado_thread(fetch_events)(