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.
This commit is contained in:
Sahil Batra 2022-05-16 20:32:44 +05:30 committed by Tim Abbott
parent 04ccd8c6d8
commit dfd7902c77
13 changed files with 89 additions and 60 deletions

View File

@ -291,7 +291,7 @@ const hamletcharacters = {
description: "Characters of Hamlet", description: "Characters of Hamlet",
members: new Set([100, 104]), members: new Set([100, 104]),
is_system_group: false, is_system_group: false,
subgroups: new Set([10, 11]), direct_subgroup_ids: new Set([10, 11]),
}; };
const backend = { const backend = {
@ -300,7 +300,7 @@ const backend = {
description: "Backend team", description: "Backend team",
members: new Set([]), members: new Set([]),
is_system_group: false, is_system_group: false,
subgroups: new Set([]), direct_subgroup_ids: new Set([]),
}; };
const call_center = { const call_center = {
@ -309,7 +309,7 @@ const call_center = {
description: "folks working in support", description: "folks working in support",
members: new Set([]), members: new Set([]),
is_system_group: false, is_system_group: false,
subgroups: new Set([]), direct_subgroup_ids: new Set([]),
}; };
const make_emoji = (emoji_dict) => ({ const make_emoji = (emoji_dict) => ({

View File

@ -184,9 +184,9 @@ run_test("user groups", ({override}) => {
override(user_groups, "add_subgroups", stub.f); override(user_groups, "add_subgroups", stub.f);
dispatch(event); dispatch(event);
assert.equal(stub.num_calls, 1); 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.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; event = event_fixtures.user_group__remove_members;
@ -206,9 +206,9 @@ run_test("user groups", ({override}) => {
override(user_groups, "remove_subgroups", stub.f); override(user_groups, "remove_subgroups", stub.f);
dispatch(event); dispatch(event);
assert.equal(stub.num_calls, 1); 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.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; event = event_fixtures.user_group__update;

View File

@ -735,7 +735,7 @@ exports.fixtures = {
description: "mobile folks", description: "mobile folks",
members: [1], members: [1],
is_system_group: false, is_system_group: false,
subgroups: [2], direct_subgroup_ids: [2],
}, },
}, },
@ -750,7 +750,7 @@ exports.fixtures = {
type: "user_group", type: "user_group",
op: "add_subgroups", op: "add_subgroups",
group_id: 1, group_id: 1,
subgroup_ids: [3], direct_subgroup_ids: [3],
}, },
user_group__remove: { user_group__remove: {
@ -770,7 +770,7 @@ exports.fixtures = {
type: "user_group", type: "user_group",
op: "remove_subgroups", op: "remove_subgroups",
group_id: 1, group_id: 1,
subgroup_ids: [3], direct_subgroup_ids: [3],
}, },
user_group__update: { user_group__update: {

View File

@ -15,7 +15,7 @@ run_test("user_groups", () => {
id: 0, id: 0,
members: new Set([1, 2]), members: new Set([1, 2]),
is_system_group: false, is_system_group: false,
subgroups: new Set([4, 5]), direct_subgroup_ids: new Set([4, 5]),
}; };
const params = {}; const params = {};
@ -32,14 +32,14 @@ run_test("user_groups", () => {
id: 1, id: 1,
members: new Set([3]), members: new Set([3]),
is_system_group: false, is_system_group: false,
subgroups: new Set([]), direct_subgroup_ids: new Set([]),
}; };
const all = { const all = {
name: "Everyone", name: "Everyone",
id: 2, id: 2,
members: new Set([1, 2, 3]), members: new Set([1, 2, 3]),
is_system_group: false, is_system_group: false,
subgroups: new Set([4, 5, 6]), direct_subgroup_ids: new Set([4, 5, 6]),
}; };
user_groups.add(admins); user_groups.add(admins);
@ -101,12 +101,15 @@ run_test("user_groups", () => {
user_groups.add_subgroups(all.id, [2, 3]); user_groups.add_subgroups(all.id, [2, 3]);
assert.deepEqual( 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]), new Set([2, 3, 5, 4, 6]),
); );
user_groups.remove_subgroups(all.id, [2, 4]); 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)); assert.ok(user_groups.is_user_group(admins));
const object = { const object = {
@ -135,28 +138,28 @@ run_test("get_recursive_subgroups", () => {
id: 1, id: 1,
members: new Set([1]), members: new Set([1]),
is_system_group: false, is_system_group: false,
subgroups: new Set([4]), direct_subgroup_ids: new Set([4]),
}; };
const all = { const all = {
name: "Everyone", name: "Everyone",
id: 2, id: 2,
members: new Set([2, 3]), members: new Set([2, 3]),
is_system_group: false, is_system_group: false,
subgroups: new Set([1, 3]), direct_subgroup_ids: new Set([1, 3]),
}; };
const test = { const test = {
name: "Test", name: "Test",
id: 3, id: 3,
members: new Set([3, 4, 5]), members: new Set([3, 4, 5]),
is_system_group: false, is_system_group: false,
subgroups: new Set([2]), direct_subgroup_ids: new Set([2]),
}; };
const foo = { const foo = {
name: "Foo", name: "Foo",
id: 4, id: 4,
members: new Set([6, 7]), members: new Set([6, 7]),
is_system_group: false, is_system_group: false,
subgroups: new Set([]), direct_subgroup_ids: new Set([]),
}; };
user_groups.add(admins); user_groups.add(admins);
@ -190,28 +193,28 @@ run_test("is_user_in_group", () => {
id: 1, id: 1,
members: new Set([1]), members: new Set([1]),
is_system_group: false, is_system_group: false,
subgroups: new Set([4]), direct_subgroup_ids: new Set([4]),
}; };
const all = { const all = {
name: "Everyone", name: "Everyone",
id: 2, id: 2,
members: new Set([2, 3]), members: new Set([2, 3]),
is_system_group: false, is_system_group: false,
subgroups: new Set([1, 3]), direct_subgroup_ids: new Set([1, 3]),
}; };
const test = { const test = {
name: "Test", name: "Test",
id: 3, id: 3,
members: new Set([4, 5]), members: new Set([4, 5]),
is_system_group: false, is_system_group: false,
subgroups: new Set([1]), direct_subgroup_ids: new Set([1]),
}; };
const foo = { const foo = {
name: "Foo", name: "Foo",
id: 4, id: 4,
members: new Set([6, 7]), members: new Set([6, 7]),
is_system_group: false, is_system_group: false,
subgroups: new Set([]), direct_subgroup_ids: new Set([]),
}; };
user_groups.add(admins); user_groups.add(admins);
user_groups.add(all); user_groups.add(all);

View File

@ -755,10 +755,10 @@ export function dispatch_normal_event(event) {
user_groups.remove_members(event.group_id, event.user_ids); user_groups.remove_members(event.group_id, event.user_ids);
break; break;
case "add_subgroups": 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; break;
case "remove_subgroups": 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; break;
case "update": case "update":
user_groups.update(event); user_groups.update(event);

View File

@ -8,7 +8,7 @@ type UserGroup = {
name: string; name: string;
members: Set<number>; members: Set<number>;
is_system_group: boolean; is_system_group: boolean;
subgroups: Set<number>; direct_subgroup_ids: Set<number>;
}; };
// The members field is a number array which we convert // 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, name: user_group_raw.name,
members: new Set(user_group_raw.members), members: new Set(user_group_raw.members),
is_system_group: user_group_raw.is_system_group, 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); 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) { 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) { 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<number> |
// Correctness of this algorithm relying on the ES6 Set // Correctness of this algorithm relying on the ES6 Set
// implementation having the property that a `for of` loop will // implementation having the property that a `for of` loop will
// visit all items that are added to the set during the loop. // 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) { for (const subgroup_id of subgroup_ids) {
const subgroup = user_group_by_id_dict.get(subgroup_id); const subgroup = user_group_by_id_dict.get(subgroup_id);
if (subgroup === undefined) { if (subgroup === undefined) {
@ -172,7 +172,7 @@ export function get_recursive_subgroups(target_group_id: number): Set<number> |
return undefined; 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); subgroup_ids.add(direct_subgroup_id);
} }
} }

View File

@ -20,6 +20,14 @@ format used by the Zulip server that they are interacting with.
## Changes in Zulip 6.0 ## 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** **Feature level 130**
* `PATCH /bots/{bot_user_id}`: Added support for changing a bot's role * `PATCH /bots/{bot_user_id}`: Added support for changing a bot's role

View File

@ -100,7 +100,7 @@ def do_send_create_user_group_event(
description=user_group.description, description=user_group.description,
id=user_group.id, id=user_group.id,
is_system_group=user_group.is_system_group, 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)) 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] event_name: str, user_group: UserGroup, subgroup_ids: List[int]
) -> None: ) -> None:
event = dict( 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( transaction.on_commit(
lambda: send_event(user_group.realm, event, active_user_ids(user_group.realm_id)) lambda: send_event(user_group.realm, event, active_user_ids(user_group.realm_id))

View File

@ -1676,7 +1676,7 @@ group_type = DictType(
("id", int), ("id", int),
("name", str), ("name", str),
("members", ListType(int)), ("members", ListType(int)),
("subgroups", ListType(int)), ("direct_subgroup_ids", ListType(int)),
("description", str), ("description", str),
("is_system_group", bool), ("is_system_group", bool),
] ]
@ -1753,7 +1753,7 @@ user_group_add_subgroups_event = event_dict_type(
("type", Equals("user_group")), ("type", Equals("user_group")),
("op", Equals("add_subgroups")), ("op", Equals("add_subgroups")),
("group_id", int), ("group_id", int),
("subgroup_ids", ListType(int)), ("direct_subgroup_ids", ListType(int)),
] ]
) )
check_user_group_add_subgroups = make_checker(user_group_add_subgroups_event) 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")), ("type", Equals("user_group")),
("op", Equals("remove_subgroups")), ("op", Equals("remove_subgroups")),
("group_id", int), ("group_id", int),
("subgroup_ids", ListType(int)), ("direct_subgroup_ids", ListType(int)),
] ]
) )
check_user_group_remove_subgroups = make_checker(user_group_remove_subgroups_event) check_user_group_remove_subgroups = make_checker(user_group_remove_subgroups_event)

View File

@ -1249,14 +1249,16 @@ def apply_event(
elif event["op"] == "add_subgroups": elif event["op"] == "add_subgroups":
for user_group in state["realm_user_groups"]: for user_group in state["realm_user_groups"]:
if user_group["id"] == event["group_id"]: if user_group["id"] == event["group_id"]:
user_group["subgroups"].extend(event["subgroup_ids"]) user_group["direct_subgroup_ids"].extend(event["direct_subgroup_ids"])
user_group["subgroups"].sort() user_group["direct_subgroup_ids"].sort()
elif event["op"] == "remove_subgroups": elif event["op"] == "remove_subgroups":
for user_group in state["realm_user_groups"]: for user_group in state["realm_user_groups"]:
if user_group["id"] == event["group_id"]: if user_group["id"] == event["group_id"]:
subgroups = set(user_group["subgroups"]) subgroups = set(user_group["direct_subgroup_ids"])
user_group["subgroups"] = list(subgroups - set(event["subgroup_ids"])) user_group["direct_subgroup_ids"] = list(
user_group["subgroups"].sort() subgroups - set(event["direct_subgroup_ids"])
)
user_group["direct_subgroup_ids"].sort()
elif event["op"] == "remove": elif event["op"] == "remove":
state["realm_user_groups"] = [ state["realm_user_groups"] = [
ug for ug in state["realm_user_groups"] if ug["id"] != event["group_id"] ug for ug in state["realm_user_groups"] if ug["id"] != event["group_id"]

View File

@ -62,7 +62,7 @@ def user_groups_in_realm_serialized(realm: Realm) -> List[Dict[str, Any]]:
name=user_group.name, name=user_group.name,
description=user_group.description, description=user_group.description,
members=[], members=[],
subgroups=[], direct_subgroup_ids=[],
is_system_group=user_group.is_system_group, 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" "subgroup_id", "supergroup_id"
) )
for (subgroup_id, supergroup_id) in group_membership: 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(): for group_dict in group_dicts.values():
group_dict["members"] = sorted(group_dict["members"]) 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"]) return sorted(group_dicts.values(), key=lambda group_dict: group_dict["id"])

View File

@ -2853,19 +2853,24 @@ paths:
type: integer type: integer
description: | description: |
The ID of the user group whose details have changed. The ID of the user group whose details have changed.
subgroup_ids: direct_subgroup_ids:
type: array type: array
items: items:
type: integer type: integer
description: | description: |
Array containing the IDs of the subgroups that have been added Array containing the IDs of the subgroups that have been added
to the user group. 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: example:
{ {
"type": "user_group", "type": "user_group",
"op": "add_subgroups", "op": "add_subgroups",
"group_id": 2, "group_id": 2,
"subgroup_ids": [10], "direct_subgroup_ids": [10],
"id": 0, "id": 0,
} }
- type: object - type: object
@ -2891,19 +2896,24 @@ paths:
type: integer type: integer
description: | description: |
The ID of the user group whose details have changed. The ID of the user group whose details have changed.
subgroup_ids: direct_subgroup_ids:
type: array type: array
items: items:
type: integer type: integer
description: | description: |
Array containing the IDs of the subgroups that have been Array containing the IDs of the subgroups that have been
removed from the user group. 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: example:
{ {
"type": "user_group", "type": "user_group",
"op": "remove_subgroups", "op": "remove_subgroups",
"group_id": 2, "group_id": 2,
"subgroup_ids": [10], "direct_subgroup_ids": [10],
"id": 0, "id": 0,
} }
- type: object - type: object
@ -13998,12 +14008,15 @@ paths:
The integer user IDs of the user group members. The integer user IDs of the user group members.
items: items:
type: integer type: integer
subgroups: direct_subgroup_ids:
type: array type: array
description: | 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: items:
type: integer type: integer
name: name:
@ -14031,7 +14044,7 @@ paths:
"id": 1, "id": 1,
"name": "hamletcharacters", "name": "hamletcharacters",
"members": [3, 4], "members": [3, 4],
"subgroups": [], "direct_subgroup_ids": [],
"is_system_group": false, "is_system_group": false,
}, },
{ {
@ -14039,7 +14052,7 @@ paths:
"id": 2, "id": 2,
"name": "other users", "name": "other users",
"members": [1, 2], "members": [1, 2],
"subgroups": [1, 2], "direct_subgroup_ids": [1, 2],
"is_system_group": true, "is_system_group": true,
}, },
], ],
@ -14978,15 +14991,18 @@ components:
description: | description: |
Array containing the id of the users who are Array containing the id of the users who are
members of this user group. members of this user group.
subgroups: direct_subgroup_ids:
type: array type: array
items: items:
type: integer type: integer
description: | description: |
Array containing the id of the subgroups of this Array containing the id of the direct_subgroups of
user group. 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: id:
type: integer type: integer
description: | description: |

View File

@ -51,12 +51,12 @@ class UserGroupTestCase(ZulipTestCase):
self.assertEqual(user_groups[0]["name"], "@role:owners") self.assertEqual(user_groups[0]["name"], "@role:owners")
self.assertEqual(user_groups[0]["description"], "Owners of this organization") self.assertEqual(user_groups[0]["description"], "Owners of this organization")
self.assertEqual(set(user_groups[0]["members"]), set(membership)) 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) admins_system_group = UserGroup.objects.get(name="@role:administrators", realm=realm)
self.assertEqual(user_groups[1]["id"], admins_system_group.id) self.assertEqual(user_groups[1]["id"], admins_system_group.id)
# Check that owners system group is present in "subgroups" # Check that owners system group is present in "direct_subgroup_ids"
self.assertEqual(user_groups[1]["subgroups"], [user_group.id]) 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]["id"], empty_user_group.id)
self.assertEqual(user_groups[8]["name"], "newgroup") self.assertEqual(user_groups[8]["name"], "newgroup")