Skip to content

Commit 6acb3ab

Browse files
paulnoirelpaulnoirel
committed
Fix _get_members_set to get role name
1 parent b71471a commit 6acb3ab

File tree

2 files changed

+109
-65
lines changed

2 files changed

+109
-65
lines changed

‎libs/labelbox/src/labelbox/schema/user_group.py‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -679,6 +679,12 @@ def _get_members_set(
679679
member_nodes = members_data.get("nodes", [])
680680
user_group_roles = members_data.get("userGroupRoles", [])
681681

682+
# Get all roles to map IDs to names
683+
from labelbox.schema.role import get_roles
684+
685+
all_roles = get_roles(self.client)
686+
role_id_to_role = {role.uid: role for role in all_roles.values()}
687+
682688
# Create a mapping from userId to roleId
683689
user_role_mapping = {
684690
role_data["userId"]: role_data["roleId"]
@@ -694,15 +700,9 @@ def _get_members_set(
694700

695701
# Get the role for this user from the mapping
696702
role_id = user_role_mapping.get(node["id"])
697-
if role_id:
698-
# We need to fetch the role details since we only have the roleId
699-
# For now, create a minimal Role object with just the ID
700-
role_values: defaultdict[str, Any] = defaultdict(lambda: None)
701-
role_values["id"] = role_id
702-
# We don't have the role name from this response, so we'll leave it as None
703-
# The Role object will fetch the name when needed
704-
role = Role(self.client, role_values)
705-
703+
if role_id and role_id in role_id_to_role:
704+
# Use the actual Role object with proper name resolution
705+
role = role_id_to_role[role_id]
706706
members.add(UserGroupMember(user=user, role=role))
707707

708708
return members

‎libs/labelbox/tests/unit/schema/test_user_group.py‎

Lines changed: 100 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,11 @@ def test_user_group_color_values(self):
100100

101101
class TestUserGroup:
102102
def setup_method(self):
103+
# Reset the global roles cache before each test
104+
import labelbox.schema.role as role_module
105+
106+
role_module._ROLES = None
107+
103108
self.client = MagicMock(Client)
104109
self.client.get_roles.return_value = {
105110
"LABELER": Role(self.client, {"id": "role_id", "name": "LABELER"}),
@@ -167,26 +172,36 @@ def test_get(self):
167172
"orgRole": {"id": "role_id_2", "name": "LABELER"},
168173
},
169174
]
170-
self.client.execute.return_value = {
171-
"userGroupV2": {
172-
"id": "group_id",
173-
"name": "Test Group",
174-
"color": "4ED2F9",
175-
"description": "",
176-
"projects": {
177-
"nodes": projects,
178-
"totalCount": 2,
179-
},
180-
"members": {
181-
"nodes": group_members,
182-
"totalCount": 2,
183-
"userGroupRoles": [
184-
{"userId": "user_id_1", "roleId": "role_id_1"},
185-
{"userId": "user_id_2", "roleId": "role_id_2"},
186-
],
187-
},
188-
}
189-
}
175+
self.client.execute.side_effect = [
176+
# Mock get_roles query response first
177+
{
178+
"roles": [
179+
{"id": "role_id_1", "name": "LABELER"},
180+
{"id": "role_id_2", "name": "LABELER"},
181+
]
182+
},
183+
# Mock userGroupV2 query response
184+
{
185+
"userGroupV2": {
186+
"id": "group_id",
187+
"name": "Test Group",
188+
"color": "4ED2F9",
189+
"description": "",
190+
"projects": {
191+
"nodes": projects,
192+
"totalCount": 2,
193+
},
194+
"members": {
195+
"nodes": group_members,
196+
"totalCount": 2,
197+
"userGroupRoles": [
198+
{"userId": "user_id_1", "roleId": "role_id_1"},
199+
{"userId": "user_id_2", "roleId": "role_id_2"},
200+
],
201+
},
202+
}
203+
},
204+
]
190205
group = UserGroup(self.client)
191206
assert group.id == ""
192207
assert group.name == ""
@@ -244,6 +259,12 @@ def test_update(self, group_user, group_project, mock_role):
244259
}
245260
}
246261
},
262+
# Mock get_roles query response
263+
{
264+
"roles": [
265+
{"id": "role_id", "name": "LABELER"},
266+
]
267+
},
247268
# Mock get query response after update
248269
{
249270
"userGroupV2": {
@@ -297,6 +318,8 @@ def test_update_without_members_should_work(self, group_project):
297318
}
298319
}
299320
},
321+
# Mock get_roles query response (even though no members, _get_members_set is still called)
322+
{"roles": []},
300323
# Mock get query response
301324
{
302325
"userGroupV2": {
@@ -356,42 +379,51 @@ def test_user_groups_empty(self):
356379
assert len(user_groups) == 0
357380

358381
def test_user_groups(self):
359-
self.client.execute.return_value = {
360-
"userGroupsV2": {
361-
"totalCount": 2,
362-
"nextCursor": None,
363-
"nodes": [
364-
{
365-
"id": "group_id_1",
366-
"name": "Group 1",
367-
"color": "9EC5FF",
368-
"description": "",
369-
"projects": {
370-
"nodes": [],
371-
"totalCount": 0,
372-
},
373-
"members": {
374-
"nodes": [],
375-
"totalCount": 0,
376-
},
377-
},
378-
{
379-
"id": "group_id_2",
380-
"name": "Group 2",
381-
"color": "CEB8FF",
382-
"description": "",
383-
"projects": {
384-
"nodes": [],
385-
"totalCount": 0,
382+
# Mock get_roles and get_user_groups responses
383+
# get_roles will be called once (cached for subsequent calls)
384+
self.client.execute.side_effect = [
385+
# get_roles query (called once, then cached)
386+
{"roles": []},
387+
# get_user_groups query
388+
{
389+
"userGroupsV2": {
390+
"totalCount": 2,
391+
"nextCursor": None,
392+
"nodes": [
393+
{
394+
"id": "group_id_1",
395+
"name": "Group 1",
396+
"color": "9EC5FF",
397+
"description": "",
398+
"projects": {
399+
"nodes": [],
400+
"totalCount": 0,
401+
},
402+
"members": {
403+
"nodes": [],
404+
"totalCount": 0,
405+
"userGroupRoles": [],
406+
},
386407
},
387-
"members": {
388-
"nodes": [],
389-
"totalCount": 0,
408+
{
409+
"id": "group_id_2",
410+
"name": "Group 2",
411+
"color": "CEB8FF",
412+
"description": "",
413+
"projects": {
414+
"nodes": [],
415+
"totalCount": 0,
416+
},
417+
"members": {
418+
"nodes": [],
419+
"totalCount": 0,
420+
"userGroupRoles": [],
421+
},
390422
},
391-
},
392-
],
393-
}
394-
}
423+
],
424+
}
425+
},
426+
]
395427
user_groups = list(UserGroup.get_user_groups(self.client))
396428
assert len(user_groups) == 2
397429
assert user_groups[0].name == "Group 1"
@@ -448,6 +480,12 @@ def test_create(self, group_user, group_project, mock_role):
448480
}
449481
}
450482
},
483+
# Mock get_roles query response
484+
{
485+
"roles": [
486+
{"id": "role_id", "name": "LABELER"},
487+
]
488+
},
451489
# Mock get query response after create
452490
{
453491
"userGroupV2": {
@@ -501,6 +539,8 @@ def test_create_without_members_should_work(self, group_project):
501539
}
502540
}
503541
},
542+
# Mock get_roles query response (even though no members, _get_members_set is still called)
543+
{"roles": []},
504544
# Mock get query response
505545
{
506546
"userGroupV2": {
@@ -588,7 +628,9 @@ def test_create_mutation():
588628
}
589629
}
590630
},
591-
# Second call: get query
631+
# Second call: get_roles query
632+
{"roles": []},
633+
# Third call: get query
592634
{
593635
"userGroupV2": {
594636
"id": "group_id",
@@ -646,7 +688,9 @@ def test_update_mutation():
646688
}
647689
}
648690
},
649-
# Second call: get query
691+
# Second call: get_roles query
692+
{"roles": []},
693+
# Third call: get query
650694
{
651695
"userGroupV2": {
652696
"id": "group_id",

0 commit comments

Comments
 (0)