mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-03-22 13:47:19 +08:00
feat(backend): return is_personal field in OrgResponse (#12777)
This commit is contained in:
@@ -91,14 +91,18 @@ class OrgResponse(BaseModel):
|
||||
enable_solvability_analysis: bool | None = None
|
||||
v1_enabled: bool | None = None
|
||||
credits: float | None = None
|
||||
is_personal: bool = False
|
||||
|
||||
@classmethod
|
||||
def from_org(cls, org: Org, credits: float | None = None) -> 'OrgResponse':
|
||||
def from_org(
|
||||
cls, org: Org, credits: float | None = None, user_id: str | None = None
|
||||
) -> 'OrgResponse':
|
||||
"""Create an OrgResponse from an Org entity.
|
||||
|
||||
Args:
|
||||
org: The organization entity to convert
|
||||
credits: Optional credits value (defaults to None)
|
||||
user_id: Optional user ID to determine if org is personal (defaults to None)
|
||||
|
||||
Returns:
|
||||
OrgResponse: The response model instance
|
||||
@@ -134,6 +138,7 @@ class OrgResponse(BaseModel):
|
||||
enable_solvability_analysis=org.enable_solvability_analysis,
|
||||
v1_enabled=org.v1_enabled,
|
||||
credits=credits,
|
||||
is_personal=str(org.id) == user_id if user_id else False,
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -69,7 +69,9 @@ async def list_user_orgs(
|
||||
)
|
||||
|
||||
# Convert Org entities to OrgResponse objects
|
||||
org_responses = [OrgResponse.from_org(org, credits=None) for org in orgs]
|
||||
org_responses = [
|
||||
OrgResponse.from_org(org, credits=None, user_id=user_id) for org in orgs
|
||||
]
|
||||
|
||||
logger.info(
|
||||
'Successfully retrieved organizations',
|
||||
@@ -136,7 +138,7 @@ async def create_org(
|
||||
# Retrieve credits from LiteLLM
|
||||
credits = await OrgService.get_org_credits(user_id, org.id)
|
||||
|
||||
return OrgResponse.from_org(org, credits=credits)
|
||||
return OrgResponse.from_org(org, credits=credits, user_id=user_id)
|
||||
except OrgNameExistsError as e:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_409_CONFLICT,
|
||||
@@ -211,7 +213,7 @@ async def get_org(
|
||||
# Retrieve credits from LiteLLM
|
||||
credits = await OrgService.get_org_credits(user_id, org.id)
|
||||
|
||||
return OrgResponse.from_org(org, credits=credits)
|
||||
return OrgResponse.from_org(org, credits=credits, user_id=user_id)
|
||||
except OrgNotFoundError as e:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
@@ -368,7 +370,7 @@ async def update_org(
|
||||
# Retrieve credits from LiteLLM (following same pattern as create endpoint)
|
||||
credits = await OrgService.get_org_credits(user_id, updated_org.id)
|
||||
|
||||
return OrgResponse.from_org(updated_org, credits=credits)
|
||||
return OrgResponse.from_org(updated_org, credits=credits, user_id=user_id)
|
||||
|
||||
except ValueError as e:
|
||||
# Organization not found
|
||||
|
||||
@@ -322,6 +322,50 @@ async def test_create_org_forbidden_non_openhands_email():
|
||||
assert 'openhands.dev' in response.json()['detail'].lower()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_create_org_is_not_personal(mock_app):
|
||||
"""
|
||||
GIVEN: Admin creates a new team organization
|
||||
WHEN: POST /api/organizations is called
|
||||
THEN: is_personal field is False (team orgs have different ID than creator)
|
||||
"""
|
||||
# Arrange
|
||||
org_id = uuid.uuid4() # Different from user_id
|
||||
mock_org = Org(
|
||||
id=org_id,
|
||||
name='New Team Organization',
|
||||
contact_name='John Doe',
|
||||
contact_email='john@example.com',
|
||||
org_version=5,
|
||||
)
|
||||
|
||||
request_data = {
|
||||
'name': 'New Team Organization',
|
||||
'contact_name': 'John Doe',
|
||||
'contact_email': 'john@example.com',
|
||||
}
|
||||
|
||||
with (
|
||||
patch(
|
||||
'server.routes.orgs.OrgService.create_org_with_owner',
|
||||
AsyncMock(return_value=mock_org),
|
||||
),
|
||||
patch(
|
||||
'server.routes.orgs.OrgService.get_org_credits',
|
||||
AsyncMock(return_value=100.0),
|
||||
),
|
||||
):
|
||||
client = TestClient(mock_app)
|
||||
|
||||
# Act
|
||||
response = client.post('/api/organizations', json=request_data)
|
||||
|
||||
# Assert
|
||||
assert response.status_code == status.HTTP_201_CREATED
|
||||
response_data = response.json()
|
||||
assert response_data['is_personal'] is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_create_org_sensitive_fields_not_exposed(mock_app):
|
||||
"""
|
||||
@@ -396,6 +440,9 @@ def mock_app_list():
|
||||
|
||||
app.dependency_overrides[get_user_id] = mock_get_user_id
|
||||
|
||||
# Store test_user_id for test access
|
||||
app.state.test_user_id = test_user_id
|
||||
|
||||
return app
|
||||
|
||||
|
||||
@@ -584,6 +631,126 @@ async def test_list_user_orgs_unauthorized():
|
||||
assert response.status_code == status.HTTP_401_UNAUTHORIZED
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_list_user_orgs_personal_org_identified(mock_app_list):
|
||||
"""
|
||||
GIVEN: User has a personal organization (org.id == user_id)
|
||||
WHEN: GET /api/organizations is called
|
||||
THEN: is_personal field is True for personal org
|
||||
"""
|
||||
# Arrange
|
||||
user_id = mock_app_list.state.test_user_id
|
||||
personal_org_id = uuid.UUID(user_id)
|
||||
|
||||
personal_org = Org(
|
||||
id=personal_org_id,
|
||||
name=f'user_{user_id}_org',
|
||||
contact_name='John Doe',
|
||||
contact_email='john@example.com',
|
||||
)
|
||||
|
||||
with patch(
|
||||
'server.routes.orgs.OrgService.get_user_orgs_paginated',
|
||||
return_value=([personal_org], None),
|
||||
):
|
||||
client = TestClient(mock_app_list)
|
||||
|
||||
# Act
|
||||
response = client.get('/api/organizations')
|
||||
|
||||
# Assert
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
response_data = response.json()
|
||||
assert len(response_data['items']) == 1
|
||||
assert response_data['items'][0]['is_personal'] is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_list_user_orgs_team_org_identified(mock_app_list):
|
||||
"""
|
||||
GIVEN: User has a team organization (org.id != user_id)
|
||||
WHEN: GET /api/organizations is called
|
||||
THEN: is_personal field is False for team org
|
||||
"""
|
||||
# Arrange
|
||||
team_org = Org(
|
||||
id=uuid.uuid4(), # Different from user_id
|
||||
name='Team Organization',
|
||||
contact_name='John Doe',
|
||||
contact_email='john@example.com',
|
||||
)
|
||||
|
||||
with patch(
|
||||
'server.routes.orgs.OrgService.get_user_orgs_paginated',
|
||||
return_value=([team_org], None),
|
||||
):
|
||||
client = TestClient(mock_app_list)
|
||||
|
||||
# Act
|
||||
response = client.get('/api/organizations')
|
||||
|
||||
# Assert
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
response_data = response.json()
|
||||
assert len(response_data['items']) == 1
|
||||
assert response_data['items'][0]['is_personal'] is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_list_user_orgs_mixed_personal_and_team(mock_app_list):
|
||||
"""
|
||||
GIVEN: User has both personal and team organizations
|
||||
WHEN: GET /api/organizations is called
|
||||
THEN: is_personal field correctly identifies each org type
|
||||
"""
|
||||
# Arrange
|
||||
user_id = mock_app_list.state.test_user_id
|
||||
personal_org_id = uuid.UUID(user_id)
|
||||
|
||||
personal_org = Org(
|
||||
id=personal_org_id,
|
||||
name=f'user_{user_id}_org',
|
||||
contact_name='John Doe',
|
||||
contact_email='john@example.com',
|
||||
)
|
||||
|
||||
team_org = Org(
|
||||
id=uuid.uuid4(),
|
||||
name='Team Organization',
|
||||
contact_name='Jane Doe',
|
||||
contact_email='jane@example.com',
|
||||
)
|
||||
|
||||
with patch(
|
||||
'server.routes.orgs.OrgService.get_user_orgs_paginated',
|
||||
return_value=([personal_org, team_org], None),
|
||||
):
|
||||
client = TestClient(mock_app_list)
|
||||
|
||||
# Act
|
||||
response = client.get('/api/organizations')
|
||||
|
||||
# Assert
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
response_data = response.json()
|
||||
assert len(response_data['items']) == 2
|
||||
|
||||
# Find personal and team orgs in response
|
||||
personal_org_response = next(
|
||||
item
|
||||
for item in response_data['items']
|
||||
if item['id'] == str(personal_org_id)
|
||||
)
|
||||
team_org_response = next(
|
||||
item
|
||||
for item in response_data['items']
|
||||
if item['id'] != str(personal_org_id)
|
||||
)
|
||||
|
||||
assert personal_org_response['is_personal'] is True
|
||||
assert team_org_response['is_personal'] is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_list_user_orgs_all_fields_present(mock_app_list):
|
||||
"""
|
||||
@@ -833,6 +1000,93 @@ async def test_get_org_unexpected_error(mock_app_with_get_user_id):
|
||||
assert 'unexpected error' in response.json()['detail'].lower()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_org_personal_workspace():
|
||||
"""
|
||||
GIVEN: User retrieves their personal organization (org.id == user_id)
|
||||
WHEN: GET /api/organizations/{org_id} is called
|
||||
THEN: is_personal field is True
|
||||
"""
|
||||
# Arrange
|
||||
app = FastAPI()
|
||||
app.include_router(org_router)
|
||||
|
||||
# Use a valid UUID for user_id
|
||||
user_id = str(uuid.uuid4())
|
||||
org_id = uuid.UUID(user_id) # Personal org has same ID as user
|
||||
|
||||
def mock_get_user_id():
|
||||
return user_id
|
||||
|
||||
app.dependency_overrides[get_user_id] = mock_get_user_id
|
||||
|
||||
mock_org = Org(
|
||||
id=org_id,
|
||||
name=f'user_{user_id}_org',
|
||||
contact_name='John Doe',
|
||||
contact_email='john@example.com',
|
||||
org_version=5,
|
||||
)
|
||||
|
||||
with (
|
||||
patch(
|
||||
'server.routes.orgs.OrgService.get_org_by_id',
|
||||
AsyncMock(return_value=mock_org),
|
||||
),
|
||||
patch(
|
||||
'server.routes.orgs.OrgService.get_org_credits',
|
||||
AsyncMock(return_value=50.0),
|
||||
),
|
||||
):
|
||||
client = TestClient(app)
|
||||
|
||||
# Act
|
||||
response = client.get(f'/api/organizations/{org_id}')
|
||||
|
||||
# Assert
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
response_data = response.json()
|
||||
assert response_data['is_personal'] is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_org_team_workspace(mock_app_with_get_user_id):
|
||||
"""
|
||||
GIVEN: User retrieves a team organization (org.id != user_id)
|
||||
WHEN: GET /api/organizations/{org_id} is called
|
||||
THEN: is_personal field is False
|
||||
"""
|
||||
# Arrange
|
||||
org_id = uuid.uuid4() # Different from user_id
|
||||
mock_org = Org(
|
||||
id=org_id,
|
||||
name='Team Organization',
|
||||
contact_name='John Doe',
|
||||
contact_email='john@example.com',
|
||||
org_version=5,
|
||||
)
|
||||
|
||||
with (
|
||||
patch(
|
||||
'server.routes.orgs.OrgService.get_org_by_id',
|
||||
AsyncMock(return_value=mock_org),
|
||||
),
|
||||
patch(
|
||||
'server.routes.orgs.OrgService.get_org_credits',
|
||||
AsyncMock(return_value=100.0),
|
||||
),
|
||||
):
|
||||
client = TestClient(mock_app_with_get_user_id)
|
||||
|
||||
# Act
|
||||
response = client.get(f'/api/organizations/{org_id}')
|
||||
|
||||
# Assert
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
response_data = response.json()
|
||||
assert response_data['is_personal'] is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_org_with_credits_none(mock_app_with_get_user_id):
|
||||
"""
|
||||
@@ -1153,6 +1407,114 @@ def mock_update_app():
|
||||
# Route handler tests focus on error handling and validation
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_org_personal_workspace_preserved():
|
||||
"""
|
||||
GIVEN: User updates their personal organization
|
||||
WHEN: PATCH /api/organizations/{org_id} is called
|
||||
THEN: is_personal field remains True in response
|
||||
"""
|
||||
# Arrange
|
||||
app = FastAPI()
|
||||
app.include_router(org_router)
|
||||
|
||||
user_id = str(uuid.uuid4())
|
||||
org_id = uuid.UUID(user_id) # Personal org
|
||||
|
||||
async def mock_user_id():
|
||||
return user_id
|
||||
|
||||
app.dependency_overrides[get_user_id] = mock_user_id
|
||||
|
||||
updated_org = Org(
|
||||
id=org_id,
|
||||
name=f'user_{user_id}_org',
|
||||
contact_name='Updated Name',
|
||||
contact_email='john@example.com',
|
||||
org_version=5,
|
||||
)
|
||||
|
||||
update_data = {'contact_name': 'Updated Name'}
|
||||
|
||||
with (
|
||||
patch(
|
||||
'server.routes.orgs.OrgService.update_org_with_permissions',
|
||||
AsyncMock(return_value=updated_org),
|
||||
),
|
||||
patch(
|
||||
'server.routes.orgs.OrgService.get_org_credits',
|
||||
AsyncMock(return_value=75.0),
|
||||
),
|
||||
):
|
||||
async with httpx.AsyncClient(
|
||||
transport=httpx.ASGITransport(app=app), base_url='http://test'
|
||||
) as client:
|
||||
# Act
|
||||
response = await client.patch(
|
||||
f'/api/organizations/{org_id}', json=update_data
|
||||
)
|
||||
|
||||
# Assert
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
response_data = response.json()
|
||||
assert response_data['is_personal'] is True
|
||||
assert response_data['contact_name'] == 'Updated Name'
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_org_team_workspace_preserved():
|
||||
"""
|
||||
GIVEN: User updates a team organization
|
||||
WHEN: PATCH /api/organizations/{org_id} is called
|
||||
THEN: is_personal field remains False in response
|
||||
"""
|
||||
# Arrange
|
||||
app = FastAPI()
|
||||
app.include_router(org_router)
|
||||
|
||||
user_id = str(uuid.uuid4())
|
||||
org_id = uuid.uuid4() # Team org (different from user_id)
|
||||
|
||||
async def mock_user_id():
|
||||
return user_id
|
||||
|
||||
app.dependency_overrides[get_user_id] = mock_user_id
|
||||
|
||||
updated_org = Org(
|
||||
id=org_id,
|
||||
name='Updated Team Org',
|
||||
contact_name='Jane Doe',
|
||||
contact_email='jane@example.com',
|
||||
org_version=5,
|
||||
)
|
||||
|
||||
update_data = {'name': 'Updated Team Org'}
|
||||
|
||||
with (
|
||||
patch(
|
||||
'server.routes.orgs.OrgService.update_org_with_permissions',
|
||||
AsyncMock(return_value=updated_org),
|
||||
),
|
||||
patch(
|
||||
'server.routes.orgs.OrgService.get_org_credits',
|
||||
AsyncMock(return_value=150.0),
|
||||
),
|
||||
):
|
||||
async with httpx.AsyncClient(
|
||||
transport=httpx.ASGITransport(app=app), base_url='http://test'
|
||||
) as client:
|
||||
# Act
|
||||
response = await client.patch(
|
||||
f'/api/organizations/{org_id}', json=update_data
|
||||
)
|
||||
|
||||
# Assert
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
response_data = response.json()
|
||||
assert response_data['is_personal'] is False
|
||||
assert response_data['name'] == 'Updated Team Org'
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_org_not_found(mock_update_app):
|
||||
"""
|
||||
|
||||
Reference in New Issue
Block a user