Skip to content

Pno/fix test get slice #1995

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

Pno/fix test get slice #1995

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
98f135b
Fix _get_members_set to get role name
paulnoirel Jul 2, 2025
3866a97
Prevent workspace-wide users from being user group members
paulnoirel Jul 2, 2025
ef172e0
fix lint error
paulnoirel Jul 2, 2025
7c11782
Make tests for create-dataset more robust
paulnoirel Jul 2, 2025
092e889
Fix test_get_slice
paulnoirel Jul 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 62 additions & 64 deletions libs/labelbox/src/labelbox/schema/user_group.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ def update(self) -> UserGroup:
Raises:
ValueError: If group ID or name is not set, or if projects don't exist.
ResourceNotFoundError: If the group or projects are not found.
UnprocessableEntityError: If user validation fails.
UnprocessableEntityError: If user validation fails or users have workspace-level org roles.
"""
if not self.id:
raise ValueError("Group id is required")
Expand All @@ -247,8 +247,11 @@ def update(self) -> UserGroup:
)

# Filter eligible users and build user roles
eligible_users = self._filter_project_based_users()
user_roles = self._build_user_roles(eligible_users)
try:
eligible_users = self._filter_project_based_users()
user_roles = self._build_user_roles(eligible_users)
except ValueError as e:
raise UnprocessableEntityError(str(e)) from e

query = """
mutation UpdateUserGroupPyApi($id: ID!, $name: String!, $description: String, $color: String!, $projectIds: [ID!]!, $userRoles: [UserRoleInput!], $notifyMembers: Boolean) {
Expand Down Expand Up @@ -312,7 +315,7 @@ def create(self) -> UserGroup:

Raises:
ValueError: If group already has ID, name is invalid, or projects don't exist.
ResourceCreationError: If creation fails or user validation fails.
ResourceCreationError: If creation fails, user validation fails, or users have workspace-level org roles.
ResourceConflict: If a group with the same name already exists.
"""
if self.id:
Expand All @@ -330,8 +333,11 @@ def create(self) -> UserGroup:
)

# Filter eligible users and build user roles
eligible_users = self._filter_project_based_users()
user_roles = self._build_user_roles(eligible_users)
try:
eligible_users = self._filter_project_based_users()
user_roles = self._build_user_roles(eligible_users)
except ValueError as e:
raise ResourceCreationError(str(e)) from e

query = """
mutation CreateUserGroupPyApi($name: String!, $description: String, $color: String!, $projectIds: [ID!]!, $userRoles: [UserRoleInput!]!, $notifyMembers: Boolean, $roleId: String, $searchQuery: AlignerrSearchServiceQuery) {
Expand Down Expand Up @@ -496,11 +502,14 @@ def get_user_groups(
def _filter_project_based_users(self) -> Set[User]:
"""Filter users to only include users eligible for UserGroups.

Filters out users with specific admin organization roles that cannot be
added to UserGroups. Most users should be eligible.
Only project-based users (org role "NONE") can be added to UserGroups.
Users with any workspace-level organization role cannot be added.

Returns:
Set of users that are eligible to be added to the group.

Raises:
ValueError: If any user has a workspace-level organization role.
"""
all_users = set()
for member in self.members:
Expand All @@ -509,63 +518,52 @@ def _filter_project_based_users(self) -> Set[User]:
if not all_users:
return set()

user_ids = [user.uid for user in all_users]
query = """
query CheckUserOrgRolesPyApi($userIds: [ID!]!) {
users(where: {id_in: $userIds}) {
id
orgRole { id name }
}
}
"""
# Check each user's org role directly
invalid_users = []
eligible_users = set()

try:
result = self.client.execute(query, {"userIds": user_ids})
if not result or "users" not in result:
return all_users # Fallback: let server handle validation

# Check for users with org roles that cannot be used in UserGroups
# Only users with no org role (project-based users) can be assigned to UserGroups
eligible_user_ids = set()
invalid_users = []

for user_data in result["users"]:
org_role = user_data.get("orgRole")
user_id = user_data["id"]
user_email = user_data.get("email", "unknown")

if org_role is None:
# Users with no org role (project-based users) are eligible
eligible_user_ids.add(user_id)
for user in all_users:
try:
# Get the user's organization role directly
org_role = user.org_role()
if org_role is None or org_role.name.upper() == "NONE":
# Users with no org role or "NONE" role are project-based and eligible
eligible_users.add(user)
else:
# Users with ANY workspace org role cannot be assigned to UserGroups
# Users with any workspace org role cannot be assigned to UserGroups
invalid_users.append(
{
"id": user_id,
"email": user_email,
"org_role": org_role.get("name"),
"id": user.uid,
"email": getattr(user, "email", "unknown"),
"org_role": org_role.name,
}
)
except Exception as e:
# If we can't determine the user's role, treat as invalid for safety
invalid_users.append(
{
"id": user.uid,
"email": getattr(user, "email", "unknown"),
"org_role": f"unknown (error: {str(e)})",
}
)

# Raise error if any invalid users found
if invalid_users:
error_details = []
for user in invalid_users:
error_details.append(
f"User {user['id']} ({user['email']}) has org role '{user['org_role']}'"
)

raise ValueError(
f"Cannot create UserGroup with users who have organization roles. "
f"Only project-based users (no org role) can be assigned to UserGroups.\n"
f"Invalid users:\n"
+ "\n".join(f" • {detail}" for detail in error_details)
# Raise error if any invalid users found
if invalid_users:
error_details = []
for user_info in invalid_users:
error_details.append(
f"User {user_info['id']} ({user_info['email']}) has org role '{user_info['org_role']}'"
)

return {user for user in all_users if user.uid in eligible_user_ids}
raise ValueError(
f"Cannot create UserGroup with users who have organization roles. "
f"Only project-based users (no org role or role 'NONE') can be assigned to UserGroups.\n"
f"Invalid users:\n"
+ "\n".join(f" • {detail}" for detail in error_details)
)

except Exception:
return all_users # Fallback: let server handle validation
return eligible_users

def _build_user_roles(
self, eligible_users: Set[User]
Expand Down Expand Up @@ -679,6 +677,12 @@ def _get_members_set(
member_nodes = members_data.get("nodes", [])
user_group_roles = members_data.get("userGroupRoles", [])

# Get all roles to map IDs to names
from labelbox.schema.role import get_roles

all_roles = get_roles(self.client)
role_id_to_role = {role.uid: role for role in all_roles.values()}

# Create a mapping from userId to roleId
user_role_mapping = {
role_data["userId"]: role_data["roleId"]
Expand All @@ -694,15 +698,9 @@ def _get_members_set(

# Get the role for this user from the mapping
role_id = user_role_mapping.get(node["id"])
if role_id:
# We need to fetch the role details since we only have the roleId
# For now, create a minimal Role object with just the ID
role_values: defaultdict[str, Any] = defaultdict(lambda: None)
role_values["id"] = role_id
# We don't have the role name from this response, so we'll leave it as None
# The Role object will fetch the name when needed
role = Role(self.client, role_values)

if role_id and role_id in role_id_to_role:
# Use the actual Role object with proper name resolution
role = role_id_to_role[role_id]
members.add(UserGroupMember(user=user, role=role))

return members
60 changes: 55 additions & 5 deletions libs/labelbox/tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -501,14 +501,34 @@ def consensus_project_with_batch(

@pytest.fixture
def dataset(client, rand_gen):
dataset = client.create_dataset(name=rand_gen(str))
# Handle invalid default IAM integrations in test environments gracefully
try:
dataset = client.create_dataset(name=rand_gen(str))
except ValueError as e:
if "Integration is not valid" in str(e):
# Fallback to creating dataset without IAM integration for tests
dataset = client.create_dataset(
name=rand_gen(str), iam_integration=None
)
else:
raise e
yield dataset
dataset.delete()


@pytest.fixture(scope="function")
def unique_dataset(client, rand_gen):
dataset = client.create_dataset(name=rand_gen(str))
# Handle invalid default IAM integrations in test environments gracefully
try:
dataset = client.create_dataset(name=rand_gen(str))
except ValueError as e:
if "Integration is not valid" in str(e):
# Fallback to creating dataset without IAM integration for tests
dataset = client.create_dataset(
name=rand_gen(str), iam_integration=None
)
else:
raise e
yield dataset
dataset.delete()

Expand Down Expand Up @@ -857,15 +877,35 @@ def func(project):

@pytest.fixture
def initial_dataset(client, rand_gen):
dataset = client.create_dataset(name=rand_gen(str))
# Handle invalid default IAM integrations in test environments gracefully
try:
dataset = client.create_dataset(name=rand_gen(str))
except ValueError as e:
if "Integration is not valid" in str(e):
# Fallback to creating dataset without IAM integration for tests
dataset = client.create_dataset(
name=rand_gen(str), iam_integration=None
)
else:
raise e
yield dataset

dataset.delete()


@pytest.fixture
def video_data(client, rand_gen, video_data_row, wait_for_data_row_processing):
dataset = client.create_dataset(name=rand_gen(str))
# Handle invalid default IAM integrations in test environments gracefully
try:
dataset = client.create_dataset(name=rand_gen(str))
except ValueError as e:
if "Integration is not valid" in str(e):
# Fallback to creating dataset without IAM integration for tests
dataset = client.create_dataset(
name=rand_gen(str), iam_integration=None
)
else:
raise e
data_row_ids = []
data_row = dataset.create_data_row(video_data_row)
data_row = wait_for_data_row_processing(client, data_row)
Expand All @@ -884,7 +924,17 @@ def create_video_data_row(rand_gen):

@pytest.fixture
def video_data_100_rows(client, rand_gen, wait_for_data_row_processing):
dataset = client.create_dataset(name=rand_gen(str))
# Handle invalid default IAM integrations in test environments gracefully
try:
dataset = client.create_dataset(name=rand_gen(str))
except ValueError as e:
if "Integration is not valid" in str(e):
# Fallback to creating dataset without IAM integration for tests
dataset = client.create_dataset(
name=rand_gen(str), iam_integration=None
)
else:
raise e
data_row_ids = []
for _ in range(100):
data_row = dataset.create_data_row(create_video_data_row(rand_gen))
Expand Down
2 changes: 1 addition & 1 deletion libs/labelbox/tests/integration/test_slice.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def test_get_slice(client):
if s.name in ["Test Slice 1", "Test Slice 2"]
)
for slice in slices:
_delete_catalog_slice(client, slice.id)
_delete_catalog_slice(client, slice.uid)

# Create slices
slice_id_1 = _create_catalog_slice(
Expand Down
Loading
Loading