From dfd7902c77a5a293df15bc2306f8987f642d5a1d Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Mon, 16 May 2022 20:32:44 +0530 Subject: [PATCH] user_groups: Rename subgroups fields to direct_subgroup_ids. This commit renames subgroups and subgroup_ids field sent in user group objects to direct_subgroup_ids for better readability. --- .../node_tests/composebox_typeahead.js | 6 +-- frontend_tests/node_tests/dispatch.js | 8 ++-- frontend_tests/node_tests/lib/events.js | 6 +-- frontend_tests/node_tests/user_groups.js | 29 +++++++------ static/js/server_events_dispatch.js | 4 +- static/js/user_groups.ts | 12 +++--- templates/zerver/api/changelog.md | 8 ++++ zerver/actions/user_groups.py | 4 +- zerver/lib/event_schema.py | 6 +-- zerver/lib/events.py | 12 +++--- zerver/lib/user_groups.py | 6 +-- zerver/openapi/zulip.yaml | 42 +++++++++++++------ zerver/tests/test_user_groups.py | 6 +-- 13 files changed, 89 insertions(+), 60 deletions(-) diff --git a/frontend_tests/node_tests/composebox_typeahead.js b/frontend_tests/node_tests/composebox_typeahead.js index 718dbd4178..d6d6419693 100644 --- a/frontend_tests/node_tests/composebox_typeahead.js +++ b/frontend_tests/node_tests/composebox_typeahead.js @@ -291,7 +291,7 @@ const hamletcharacters = { description: "Characters of Hamlet", members: new Set([100, 104]), is_system_group: false, - subgroups: new Set([10, 11]), + direct_subgroup_ids: new Set([10, 11]), }; const backend = { @@ -300,7 +300,7 @@ const backend = { description: "Backend team", members: new Set([]), is_system_group: false, - subgroups: new Set([]), + direct_subgroup_ids: new Set([]), }; const call_center = { @@ -309,7 +309,7 @@ const call_center = { description: "folks working in support", members: new Set([]), is_system_group: false, - subgroups: new Set([]), + direct_subgroup_ids: new Set([]), }; const make_emoji = (emoji_dict) => ({ diff --git a/frontend_tests/node_tests/dispatch.js b/frontend_tests/node_tests/dispatch.js index 0e602208fe..6c97de99ef 100644 --- a/frontend_tests/node_tests/dispatch.js +++ b/frontend_tests/node_tests/dispatch.js @@ -184,9 +184,9 @@ run_test("user groups", ({override}) => { override(user_groups, "add_subgroups", stub.f); dispatch(event); assert.equal(stub.num_calls, 1); - const args = stub.get_args("group_id", "subgroup_ids"); + const args = stub.get_args("group_id", "direct_subgroup_ids"); assert_same(args.group_id, event.group_id); - assert_same(args.subgroup_ids, event.subgroup_ids); + assert_same(args.direct_subgroup_ids, event.direct_subgroup_ids); } event = event_fixtures.user_group__remove_members; @@ -206,9 +206,9 @@ run_test("user groups", ({override}) => { override(user_groups, "remove_subgroups", stub.f); dispatch(event); assert.equal(stub.num_calls, 1); - const args = stub.get_args("group_id", "subgroup_ids"); + const args = stub.get_args("group_id", "direct_subgroup_ids"); assert_same(args.group_id, event.group_id); - assert_same(args.subgroup_ids, event.subgroup_ids); + assert_same(args.direct_subgroup_ids, event.direct_subgroup_ids); } event = event_fixtures.user_group__update; diff --git a/frontend_tests/node_tests/lib/events.js b/frontend_tests/node_tests/lib/events.js index 083cb6a554..e76ff0b656 100644 --- a/frontend_tests/node_tests/lib/events.js +++ b/frontend_tests/node_tests/lib/events.js @@ -735,7 +735,7 @@ exports.fixtures = { description: "mobile folks", members: [1], is_system_group: false, - subgroups: [2], + direct_subgroup_ids: [2], }, }, @@ -750,7 +750,7 @@ exports.fixtures = { type: "user_group", op: "add_subgroups", group_id: 1, - subgroup_ids: [3], + direct_subgroup_ids: [3], }, user_group__remove: { @@ -770,7 +770,7 @@ exports.fixtures = { type: "user_group", op: "remove_subgroups", group_id: 1, - subgroup_ids: [3], + direct_subgroup_ids: [3], }, user_group__update: { diff --git a/frontend_tests/node_tests/user_groups.js b/frontend_tests/node_tests/user_groups.js index 26df424310..6af61c33c9 100644 --- a/frontend_tests/node_tests/user_groups.js +++ b/frontend_tests/node_tests/user_groups.js @@ -15,7 +15,7 @@ run_test("user_groups", () => { id: 0, members: new Set([1, 2]), is_system_group: false, - subgroups: new Set([4, 5]), + direct_subgroup_ids: new Set([4, 5]), }; const params = {}; @@ -32,14 +32,14 @@ run_test("user_groups", () => { id: 1, members: new Set([3]), is_system_group: false, - subgroups: new Set([]), + direct_subgroup_ids: new Set([]), }; const all = { name: "Everyone", id: 2, members: new Set([1, 2, 3]), is_system_group: false, - subgroups: new Set([4, 5, 6]), + direct_subgroup_ids: new Set([4, 5, 6]), }; user_groups.add(admins); @@ -101,12 +101,15 @@ run_test("user_groups", () => { user_groups.add_subgroups(all.id, [2, 3]); assert.deepEqual( - user_groups.get_user_group_from_id(all.id).subgroups, + user_groups.get_user_group_from_id(all.id).direct_subgroup_ids, new Set([2, 3, 5, 4, 6]), ); user_groups.remove_subgroups(all.id, [2, 4]); - assert.deepEqual(user_groups.get_user_group_from_id(all.id).subgroups, new Set([3, 5, 6])); + assert.deepEqual( + user_groups.get_user_group_from_id(all.id).direct_subgroup_ids, + new Set([3, 5, 6]), + ); assert.ok(user_groups.is_user_group(admins)); const object = { @@ -135,28 +138,28 @@ run_test("get_recursive_subgroups", () => { id: 1, members: new Set([1]), is_system_group: false, - subgroups: new Set([4]), + direct_subgroup_ids: new Set([4]), }; const all = { name: "Everyone", id: 2, members: new Set([2, 3]), is_system_group: false, - subgroups: new Set([1, 3]), + direct_subgroup_ids: new Set([1, 3]), }; const test = { name: "Test", id: 3, members: new Set([3, 4, 5]), is_system_group: false, - subgroups: new Set([2]), + direct_subgroup_ids: new Set([2]), }; const foo = { name: "Foo", id: 4, members: new Set([6, 7]), is_system_group: false, - subgroups: new Set([]), + direct_subgroup_ids: new Set([]), }; user_groups.add(admins); @@ -190,28 +193,28 @@ run_test("is_user_in_group", () => { id: 1, members: new Set([1]), is_system_group: false, - subgroups: new Set([4]), + direct_subgroup_ids: new Set([4]), }; const all = { name: "Everyone", id: 2, members: new Set([2, 3]), is_system_group: false, - subgroups: new Set([1, 3]), + direct_subgroup_ids: new Set([1, 3]), }; const test = { name: "Test", id: 3, members: new Set([4, 5]), is_system_group: false, - subgroups: new Set([1]), + direct_subgroup_ids: new Set([1]), }; const foo = { name: "Foo", id: 4, members: new Set([6, 7]), is_system_group: false, - subgroups: new Set([]), + direct_subgroup_ids: new Set([]), }; user_groups.add(admins); user_groups.add(all); diff --git a/static/js/server_events_dispatch.js b/static/js/server_events_dispatch.js index 4acb6a255e..8bdd81c8a6 100644 --- a/static/js/server_events_dispatch.js +++ b/static/js/server_events_dispatch.js @@ -755,10 +755,10 @@ export function dispatch_normal_event(event) { user_groups.remove_members(event.group_id, event.user_ids); break; case "add_subgroups": - user_groups.add_subgroups(event.group_id, event.subgroup_ids); + user_groups.add_subgroups(event.group_id, event.direct_subgroup_ids); break; case "remove_subgroups": - user_groups.remove_subgroups(event.group_id, event.subgroup_ids); + user_groups.remove_subgroups(event.group_id, event.direct_subgroup_ids); break; case "update": user_groups.update(event); diff --git a/static/js/user_groups.ts b/static/js/user_groups.ts index 3c6146106f..048e35f1eb 100644 --- a/static/js/user_groups.ts +++ b/static/js/user_groups.ts @@ -8,7 +8,7 @@ type UserGroup = { name: string; members: Set; is_system_group: boolean; - subgroups: Set; + direct_subgroup_ids: Set; }; // The members field is a number array which we convert @@ -36,7 +36,7 @@ export function add(user_group_raw: UserGroupRaw): void { name: user_group_raw.name, members: new Set(user_group_raw.members), is_system_group: user_group_raw.is_system_group, - subgroups: new Set(user_group_raw.subgroups), + direct_subgroup_ids: new Set(user_group_raw.direct_subgroup_ids), }; user_group_name_dict.set(user_group.name, user_group); @@ -120,7 +120,7 @@ export function add_subgroups(user_group_id: number, subgroup_ids: number[]): vo } for (const subgroup_id of subgroup_ids) { - user_group.subgroups.add(subgroup_id); + user_group.direct_subgroup_ids.add(subgroup_id); } } @@ -132,7 +132,7 @@ export function remove_subgroups(user_group_id: number, subgroup_ids: number[]): } for (const subgroup_id of subgroup_ids) { - user_group.subgroups.delete(subgroup_id); + user_group.direct_subgroup_ids.delete(subgroup_id); } } @@ -164,7 +164,7 @@ export function get_recursive_subgroups(target_group_id: number): Set | // Correctness of this algorithm relying on the ES6 Set // implementation having the property that a `for of` loop will // visit all items that are added to the set during the loop. - const subgroup_ids = new Set(target_user_group.subgroups); + const subgroup_ids = new Set(target_user_group.direct_subgroup_ids); for (const subgroup_id of subgroup_ids) { const subgroup = user_group_by_id_dict.get(subgroup_id); if (subgroup === undefined) { @@ -172,7 +172,7 @@ export function get_recursive_subgroups(target_group_id: number): Set | return undefined; } - for (const direct_subgroup_id of subgroup.subgroups) { + for (const direct_subgroup_id of subgroup.direct_subgroup_ids) { subgroup_ids.add(direct_subgroup_id); } } diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index a134297360..8bd6ac389c 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -20,6 +20,14 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 6.0 +**Feature level 131** + +* [`GET /user_groups`](/api/get-user-groups),[`POST + /register`](/api/register-queue): Renamed `subgroups` field in + the user group objects to `direct_subgroup_ids`. +* [`GET /events`](/api/get-events): Renamed `subgroup_ids` field + in the group object to `direct_subgroup_ids`. + **Feature level 130** * `PATCH /bots/{bot_user_id}`: Added support for changing a bot's role diff --git a/zerver/actions/user_groups.py b/zerver/actions/user_groups.py index c417856603..6ea23a06db 100644 --- a/zerver/actions/user_groups.py +++ b/zerver/actions/user_groups.py @@ -100,7 +100,7 @@ def do_send_create_user_group_event( description=user_group.description, id=user_group.id, is_system_group=user_group.is_system_group, - subgroups=[subgroup.id for subgroup in subgroups], + direct_subgroup_ids=[subgroup.id for subgroup in subgroups], ), ) send_event(user_group.realm, event, active_user_ids(user_group.realm_id)) @@ -169,7 +169,7 @@ def do_send_subgroups_update_event( event_name: str, user_group: UserGroup, subgroup_ids: List[int] ) -> None: event = dict( - type="user_group", op=event_name, group_id=user_group.id, subgroup_ids=subgroup_ids + type="user_group", op=event_name, group_id=user_group.id, direct_subgroup_ids=subgroup_ids ) transaction.on_commit( lambda: send_event(user_group.realm, event, active_user_ids(user_group.realm_id)) diff --git a/zerver/lib/event_schema.py b/zerver/lib/event_schema.py index 4ebea066ee..579d51bb9d 100644 --- a/zerver/lib/event_schema.py +++ b/zerver/lib/event_schema.py @@ -1676,7 +1676,7 @@ group_type = DictType( ("id", int), ("name", str), ("members", ListType(int)), - ("subgroups", ListType(int)), + ("direct_subgroup_ids", ListType(int)), ("description", str), ("is_system_group", bool), ] @@ -1753,7 +1753,7 @@ user_group_add_subgroups_event = event_dict_type( ("type", Equals("user_group")), ("op", Equals("add_subgroups")), ("group_id", int), - ("subgroup_ids", ListType(int)), + ("direct_subgroup_ids", ListType(int)), ] ) check_user_group_add_subgroups = make_checker(user_group_add_subgroups_event) @@ -1764,7 +1764,7 @@ user_group_remove_subgroups_event = event_dict_type( ("type", Equals("user_group")), ("op", Equals("remove_subgroups")), ("group_id", int), - ("subgroup_ids", ListType(int)), + ("direct_subgroup_ids", ListType(int)), ] ) check_user_group_remove_subgroups = make_checker(user_group_remove_subgroups_event) diff --git a/zerver/lib/events.py b/zerver/lib/events.py index 1ee558749e..2afe7c2592 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -1249,14 +1249,16 @@ def apply_event( elif event["op"] == "add_subgroups": for user_group in state["realm_user_groups"]: if user_group["id"] == event["group_id"]: - user_group["subgroups"].extend(event["subgroup_ids"]) - user_group["subgroups"].sort() + user_group["direct_subgroup_ids"].extend(event["direct_subgroup_ids"]) + user_group["direct_subgroup_ids"].sort() elif event["op"] == "remove_subgroups": for user_group in state["realm_user_groups"]: if user_group["id"] == event["group_id"]: - subgroups = set(user_group["subgroups"]) - user_group["subgroups"] = list(subgroups - set(event["subgroup_ids"])) - user_group["subgroups"].sort() + subgroups = set(user_group["direct_subgroup_ids"]) + user_group["direct_subgroup_ids"] = list( + subgroups - set(event["direct_subgroup_ids"]) + ) + user_group["direct_subgroup_ids"].sort() elif event["op"] == "remove": state["realm_user_groups"] = [ ug for ug in state["realm_user_groups"] if ug["id"] != event["group_id"] diff --git a/zerver/lib/user_groups.py b/zerver/lib/user_groups.py index 1164aa4dd9..7bd2dafdf2 100644 --- a/zerver/lib/user_groups.py +++ b/zerver/lib/user_groups.py @@ -62,7 +62,7 @@ def user_groups_in_realm_serialized(realm: Realm) -> List[Dict[str, Any]]: name=user_group.name, description=user_group.description, members=[], - subgroups=[], + direct_subgroup_ids=[], is_system_group=user_group.is_system_group, ) @@ -76,11 +76,11 @@ def user_groups_in_realm_serialized(realm: Realm) -> List[Dict[str, Any]]: "subgroup_id", "supergroup_id" ) for (subgroup_id, supergroup_id) in group_membership: - group_dicts[supergroup_id]["subgroups"].append(subgroup_id) + group_dicts[supergroup_id]["direct_subgroup_ids"].append(subgroup_id) for group_dict in group_dicts.values(): group_dict["members"] = sorted(group_dict["members"]) - group_dict["subgroups"] = sorted(group_dict["subgroups"]) + group_dict["direct_subgroup_ids"] = sorted(group_dict["direct_subgroup_ids"]) return sorted(group_dicts.values(), key=lambda group_dict: group_dict["id"]) diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 7e3906f6c4..037e2efb4e 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -2853,19 +2853,24 @@ paths: type: integer description: | The ID of the user group whose details have changed. - subgroup_ids: + direct_subgroup_ids: type: array items: type: integer description: | Array containing the IDs of the subgroups that have been added to the user group. + + **Changes**: New in Zulip 6.0 (feature level 131). + Previously, this was called `subgroup_ids`, but + clients can ignore older events as this feature level + predates subgroups being fully implemented. example: { "type": "user_group", "op": "add_subgroups", "group_id": 2, - "subgroup_ids": [10], + "direct_subgroup_ids": [10], "id": 0, } - type: object @@ -2891,19 +2896,24 @@ paths: type: integer description: | The ID of the user group whose details have changed. - subgroup_ids: + direct_subgroup_ids: type: array items: type: integer description: | Array containing the IDs of the subgroups that have been removed from the user group. + + **Changes**: New in Zulip 6.0 (feature level 131). + Previously, this was called `subgroup_ids`, but + clients can ignore older events as this feature level + predates subgroups being fully implemented. example: { "type": "user_group", "op": "remove_subgroups", "group_id": 2, - "subgroup_ids": [10], + "direct_subgroup_ids": [10], "id": 0, } - type: object @@ -13998,12 +14008,15 @@ paths: The integer user IDs of the user group members. items: type: integer - subgroups: + direct_subgroup_ids: type: array description: | - The integer user group IDs of the subgroups. + The integer user group IDs of the direct subgroups. - **Changes**: New in Zulip 6.0 (feature level 127). + **Changes**: New in Zulip 6.0 (feature level 131). + Introduced in feature level 127 as `subgroups`, but + clients can ignore older events as this feature level + predates subgroups being fully implemented. items: type: integer name: @@ -14031,7 +14044,7 @@ paths: "id": 1, "name": "hamletcharacters", "members": [3, 4], - "subgroups": [], + "direct_subgroup_ids": [], "is_system_group": false, }, { @@ -14039,7 +14052,7 @@ paths: "id": 2, "name": "other users", "members": [1, 2], - "subgroups": [1, 2], + "direct_subgroup_ids": [1, 2], "is_system_group": true, }, ], @@ -14978,15 +14991,18 @@ components: description: | Array containing the id of the users who are members of this user group. - subgroups: + direct_subgroup_ids: type: array items: type: integer description: | - Array containing the id of the subgroups of this - user group. + Array containing the id of the direct_subgroups of + this user group. - **Changes**: New in Zulip 6.0 (feature level 127). + **Changes**: New in Zulip 6.0 (feature level 131). + Introduced in feature level 127 as `subgroups`, but + clients can ignore older events as this feature level + predates subgroups being fully implemented. id: type: integer description: | diff --git a/zerver/tests/test_user_groups.py b/zerver/tests/test_user_groups.py index b273d9f272..798e088e0e 100644 --- a/zerver/tests/test_user_groups.py +++ b/zerver/tests/test_user_groups.py @@ -51,12 +51,12 @@ class UserGroupTestCase(ZulipTestCase): self.assertEqual(user_groups[0]["name"], "@role:owners") self.assertEqual(user_groups[0]["description"], "Owners of this organization") self.assertEqual(set(user_groups[0]["members"]), set(membership)) - self.assertEqual(user_groups[0]["subgroups"], []) + self.assertEqual(user_groups[0]["direct_subgroup_ids"], []) admins_system_group = UserGroup.objects.get(name="@role:administrators", realm=realm) self.assertEqual(user_groups[1]["id"], admins_system_group.id) - # Check that owners system group is present in "subgroups" - self.assertEqual(user_groups[1]["subgroups"], [user_group.id]) + # Check that owners system group is present in "direct_subgroup_ids" + self.assertEqual(user_groups[1]["direct_subgroup_ids"], [user_group.id]) self.assertEqual(user_groups[8]["id"], empty_user_group.id) self.assertEqual(user_groups[8]["name"], "newgroup")