Skip to content

Commit 08cef21

Browse files
committed
Make default_role mandatory and reorganise GraphQL fields
1 parent 751cc74 commit 08cef21

File tree

3 files changed

+211
-51
lines changed

3 files changed

+211
-51
lines changed

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

Lines changed: 30 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -162,18 +162,19 @@ def __init__(
162162
)
163163

164164
def model_post_init(self, __context: Any) -> None:
165-
"""Set default_role to LABELER if not specified.
165+
"""Validate that default_role is set when users field is used.
166166
167167
Args:
168168
__context: Pydantic context (unused).
169+
170+
Raises:
171+
ValueError: If users is set but default_role is not provided.
169172
"""
170-
if self.default_role is None:
171-
try:
172-
roles = self.client.get_roles()
173-
self.default_role = roles.get("LABELER")
174-
except Exception:
175-
# Silently fail if roles cannot be retrieved
176-
pass
173+
# Validate that default_role is set when legacy users field is used
174+
if self.users and self.default_role is None:
175+
raise ValueError(
176+
"default_role must be set when using the 'users' field."
177+
)
177178

178179
def get(self) -> UserGroup:
179180
"""Reload the user group information from the server.
@@ -245,35 +246,34 @@ def update(self) -> UserGroup:
245246
f"Project {project.uid} not found or inaccessible"
246247
)
247248

248-
# Get default role if not set
249-
if not self.default_role:
250-
roles = self.client.get_roles()
251-
self.default_role = roles.get("LABELER")
252-
if not self.default_role:
253-
raise ValueError("Unable to get default role for users")
249+
# Validate default_role is set when legacy users field is used
250+
if self.users and self.default_role is None:
251+
raise ValueError(
252+
"default_role must be set when using the 'users' field."
253+
)
254254

255255
# Filter eligible users and build user roles
256256
eligible_users = self._filter_project_based_users()
257257
user_roles = self._build_user_roles(eligible_users)
258258

259259
query = """
260-
mutation UpdateUserGroupPyApi($id: ID!, $name: String!, $color: String!, $projectIds: [ID!]!, $userRoles: [UserRoleInput!], $description: String, $notifyMembers: Boolean) {
260+
mutation UpdateUserGroupPyApi($id: ID!, $name: String!, $description: String, $color: String!, $projectIds: [ID!]!, $userRoles: [UserRoleInput!], $notifyMembers: Boolean) {
261261
updateUserGroupV3(
262262
where: { id: $id }
263263
data: {
264264
name: $name
265+
description: $description
265266
color: $color
266267
projectIds: $projectIds
267268
userRoles: $userRoles
268-
description: $description
269269
notifyMembers: $notifyMembers
270270
}
271271
) {
272272
group {
273273
id
274274
name
275-
color
276275
description
276+
color
277277
projects { nodes { id name } totalCount }
278278
members {
279279
nodes { id email orgRole { id name } }
@@ -287,10 +287,10 @@ def update(self) -> UserGroup:
287287
params = {
288288
"id": self.id,
289289
"name": self.name,
290+
"description": self.description,
290291
"color": self.color.value,
291292
"projectIds": [project.uid for project in self.projects],
292293
"userRoles": user_roles,
293-
"description": self.description,
294294
"notifyMembers": self.notify_members,
295295
}
296296

@@ -334,56 +334,54 @@ def create(self) -> UserGroup:
334334
f"Project {project.uid} not found or inaccessible"
335335
)
336336

337-
# Get default role if not set
338-
if not self.default_role:
339-
roles = self.client.get_roles()
340-
self.default_role = roles.get("LABELER")
341-
if not self.default_role:
342-
raise ValueError("Unable to get default role for users")
337+
# Validate default_role is set when legacy users field is used
338+
if self.users and self.default_role is None:
339+
raise ValueError(
340+
"default_role must be explicitly set when using the 'users' field. "
341+
"This ensures you are aware of what role will be assigned to legacy users."
342+
)
343343

344344
# Filter eligible users and build user roles
345345
eligible_users = self._filter_project_based_users()
346346
user_roles = self._build_user_roles(eligible_users)
347347

348348
query = """
349-
mutation CreateUserGroupPyApi($description: String, $color: String!, $name: String!, $projectIds: [ID!], $userRoles: [UserRoleInput!], $roleId: String, $searchQuery: AlignerrSearchServiceQuery, $notifyMembers: Boolean) {
349+
mutation CreateUserGroupPyApi($name: String!, $description: String, $color: String!, $projectIds: [ID!], $userRoles: [UserRoleInput!], $notifyMembers: Boolean, $roleId: String, $searchQuery: AlignerrSearchServiceQuery) {
350350
createUserGroupV3(
351351
data: {
352352
name: $name
353353
description: $description
354354
color: $color
355355
projectIds: $projectIds
356356
userRoles: $userRoles
357-
searchQuery: $searchQuery
358-
roleId: $roleId
359357
notifyMembers: $notifyMembers
358+
roleId: $roleId
359+
searchQuery: $searchQuery
360360
}
361361
) {
362362
group {
363363
id
364364
name
365+
description
365366
color
366367
updatedAt
367368
createdByUserName
368-
description
369-
__typename
370369
projects { nodes { id name } totalCount }
371370
members {
372371
nodes { id email orgRole { id name } }
373372
totalCount
374373
}
375374
}
376-
__typename
377375
}
378376
}
379377
"""
380378

381379
params = {
382380
"name": self.name,
381+
"description": self.description,
383382
"color": self.color.value,
384383
"projectIds": [project.uid for project in self.projects],
385384
"userRoles": user_roles,
386-
"description": self.description,
387385
"notifyMembers": self.notify_members,
388386
"roleId": None,
389387
"searchQuery": None,
@@ -505,7 +503,7 @@ def _filter_project_based_users(self) -> Set[User]:
505503

506504
user_ids = [user.uid for user in all_users]
507505
query = """
508-
query CheckUserOrgRoles($userIds: [ID!]!) {
506+
query CheckUserOrgRolesPyApi($userIds: [ID!]!) {
509507
users(where: {id_in: $userIds}) {
510508
id
511509
orgRole { id name }

libs/labelbox/tests/integration/schema/test_user_group.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,10 @@ def test_create_user_group_advanced(client, project_pack):
145145
projects = project_pack
146146
user = users[0]
147147
project = projects[0]
148+
149+
# Must set default_role when using users field
150+
roles = client.get_roles()
151+
user_group.default_role = roles["LABELER"]
148152
user_group.users.add(user)
149153
user_group.projects.add(project)
150154

@@ -182,6 +186,8 @@ def test_create_user_group_advanced(client, project_pack):
182186
or "internal server error" in creation_error.lower()
183187
or "workspace wide role" in creation_error.lower()
184188
or "conflicts with the group role" in creation_error.lower()
189+
or "default_role must be"
190+
in creation_error.lower() # New validation error
185191
)
186192

187193

@@ -498,6 +504,31 @@ def test_usergroup_functionality_demonstration(client, project_pack):
498504
pass
499505

500506

507+
def test_validation_users_without_default_role(client, project_pack):
508+
"""Test that using users field without default_role raises ValidationError."""
509+
if not list(client.get_users()):
510+
pytest.skip("No users available for testing")
511+
512+
group_name = f"{data.name()}_{int(time.time())}"
513+
user_group = UserGroup(client)
514+
user_group.name = group_name
515+
user_group.color = (
516+
UserGroupColor.RED
517+
if hasattr(UserGroupColor, "RED")
518+
else UserGroupColor.PINK
519+
)
520+
user_group.projects.add(project_pack[0])
521+
522+
users = list(client.get_users())
523+
user_group.users.add(users[0])
524+
# Deliberately NOT setting default_role
525+
526+
with pytest.raises(
527+
ValueError, match="default_role must be.*when using the 'users' field"
528+
):
529+
user_group.create()
530+
531+
501532
if __name__ == "__main__":
502533
import subprocess
503534

0 commit comments

Comments
 (0)