mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-03-22 05:37:20 +08:00
refactor(backend): update the patch organization api to support organization name updates (#12834)
This commit is contained in:
@@ -210,6 +210,10 @@ class OrgUpdate(BaseModel):
|
||||
"""Request model for updating an organization."""
|
||||
|
||||
# Basic organization information (any authenticated user can update)
|
||||
name: Annotated[
|
||||
str | None,
|
||||
StringConstraints(strip_whitespace=True, min_length=1, max_length=255),
|
||||
] = None
|
||||
contact_name: str | None = None
|
||||
contact_email: EmailStr | None = None
|
||||
conversation_expiration: int | None = None
|
||||
|
||||
@@ -449,6 +449,11 @@ async def update_org(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail=str(e),
|
||||
)
|
||||
except OrgNameExistsError as e:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_409_CONFLICT,
|
||||
detail=str(e),
|
||||
)
|
||||
except PermissionError as e:
|
||||
# User lacks permission for LLM settings
|
||||
raise HTTPException(
|
||||
|
||||
@@ -521,6 +521,7 @@ class OrgService:
|
||||
Raises:
|
||||
ValueError: If organization not found
|
||||
PermissionError: If user is not a member, or lacks admin/owner role for LLM settings
|
||||
OrgNameExistsError: If new name already exists for another organization
|
||||
OrgDatabaseError: If database update fails
|
||||
"""
|
||||
logger.info(
|
||||
@@ -550,6 +551,24 @@ class OrgService:
|
||||
'User must be a member of the organization to update it'
|
||||
)
|
||||
|
||||
# Check if name is being updated and validate uniqueness
|
||||
if update_data.name is not None:
|
||||
# Check if new name conflicts with another org
|
||||
existing_org_with_name = OrgStore.get_org_by_name(update_data.name)
|
||||
if (
|
||||
existing_org_with_name is not None
|
||||
and existing_org_with_name.id != org_id
|
||||
):
|
||||
logger.warning(
|
||||
'Attempted to update organization with duplicate name',
|
||||
extra={
|
||||
'user_id': user_id,
|
||||
'org_id': str(org_id),
|
||||
'attempted_name': update_data.name,
|
||||
},
|
||||
)
|
||||
raise OrgNameExistsError(update_data.name)
|
||||
|
||||
# Check if update contains any LLM settings
|
||||
llm_fields_being_updated = OrgService._has_llm_settings_updates(update_data)
|
||||
if llm_fields_being_updated:
|
||||
|
||||
@@ -1651,6 +1651,34 @@ async def test_update_org_permission_denied_llm_settings(mock_update_app):
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_org_duplicate_name_returns_409(mock_update_app):
|
||||
"""
|
||||
GIVEN: User updates organization name to one already used by another org
|
||||
WHEN: PATCH /api/organizations/{org_id} is called with that name
|
||||
THEN: 409 Conflict is returned with message about name already existing
|
||||
"""
|
||||
# Arrange
|
||||
org_id = uuid.uuid4()
|
||||
update_data = {'name': 'Existing Organization'}
|
||||
|
||||
with patch(
|
||||
'server.routes.orgs.OrgService.update_org_with_permissions',
|
||||
AsyncMock(side_effect=OrgNameExistsError('Existing Organization')),
|
||||
):
|
||||
async with httpx.AsyncClient(
|
||||
transport=httpx.ASGITransport(app=mock_update_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_409_CONFLICT
|
||||
assert 'already exists' in response.json()['detail'].lower()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_org_database_error(mock_update_app):
|
||||
"""
|
||||
@@ -1751,6 +1779,27 @@ async def test_update_org_invalid_field_values(mock_update_app):
|
||||
assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_org_empty_name_returns_422(mock_update_app):
|
||||
"""
|
||||
GIVEN: Update request with empty organization name (after strip)
|
||||
WHEN: PATCH /api/organizations/{org_id} is called
|
||||
THEN: 422 validation error is returned (OrgUpdate name min_length=1)
|
||||
"""
|
||||
# Arrange
|
||||
org_id = uuid.uuid4()
|
||||
update_data = {'name': ' '}
|
||||
|
||||
async with httpx.AsyncClient(
|
||||
transport=httpx.ASGITransport(app=mock_update_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_422_UNPROCESSABLE_ENTITY
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_org_invalid_email_format(mock_update_app):
|
||||
"""
|
||||
|
||||
@@ -1535,6 +1535,118 @@ async def test_update_org_with_permissions_database_error(session_maker):
|
||||
assert 'Failed to update organization' in str(exc_info.value)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_org_with_permissions_duplicate_name_raises_org_name_exists_error(
|
||||
session_maker,
|
||||
):
|
||||
"""
|
||||
GIVEN: User updates org name to a name already used by another organization
|
||||
WHEN: update_org_with_permissions is called
|
||||
THEN: OrgNameExistsError is raised with the conflicting name
|
||||
"""
|
||||
# Arrange
|
||||
org_id = uuid.uuid4()
|
||||
other_org_id = uuid.uuid4()
|
||||
user_id = str(uuid.uuid4())
|
||||
duplicate_name = 'Existing Org Name'
|
||||
|
||||
mock_current_org = Org(
|
||||
id=org_id,
|
||||
name='My Org',
|
||||
contact_name='John Doe',
|
||||
contact_email='john@example.com',
|
||||
org_version=5,
|
||||
)
|
||||
mock_org_with_name = Org(
|
||||
id=other_org_id,
|
||||
name=duplicate_name,
|
||||
contact_name='Jane Doe',
|
||||
contact_email='jane@example.com',
|
||||
)
|
||||
|
||||
from server.routes.org_models import OrgUpdate
|
||||
|
||||
update_data = OrgUpdate(name=duplicate_name)
|
||||
|
||||
with (
|
||||
patch('storage.org_store.session_maker', session_maker),
|
||||
patch('storage.org_member_store.session_maker', session_maker),
|
||||
patch('storage.role_store.session_maker', session_maker),
|
||||
patch(
|
||||
'storage.org_service.OrgStore.get_org_by_id',
|
||||
return_value=mock_current_org,
|
||||
),
|
||||
patch('storage.org_service.OrgService.is_org_member', return_value=True),
|
||||
patch(
|
||||
'storage.org_service.OrgStore.get_org_by_name',
|
||||
return_value=mock_org_with_name,
|
||||
),
|
||||
):
|
||||
# Act & Assert
|
||||
with pytest.raises(OrgNameExistsError) as exc_info:
|
||||
await OrgService.update_org_with_permissions(
|
||||
org_id=org_id,
|
||||
update_data=update_data,
|
||||
user_id=user_id,
|
||||
)
|
||||
|
||||
assert duplicate_name in str(exc_info.value)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_org_with_permissions_same_name_allowed(session_maker):
|
||||
"""
|
||||
GIVEN: User updates org with name unchanged (same as current org name)
|
||||
WHEN: update_org_with_permissions is called
|
||||
THEN: No OrgNameExistsError; update proceeds (name uniqueness allows same org)
|
||||
"""
|
||||
# Arrange
|
||||
org_id = uuid.uuid4()
|
||||
user_id = str(uuid.uuid4())
|
||||
current_name = 'My Org'
|
||||
|
||||
mock_org = Org(
|
||||
id=org_id,
|
||||
name=current_name,
|
||||
contact_name='John Doe',
|
||||
contact_email='john@example.com',
|
||||
org_version=5,
|
||||
)
|
||||
|
||||
from server.routes.org_models import OrgUpdate
|
||||
|
||||
update_data = OrgUpdate(name=current_name)
|
||||
|
||||
with (
|
||||
patch('storage.org_store.session_maker', session_maker),
|
||||
patch('storage.org_member_store.session_maker', session_maker),
|
||||
patch('storage.role_store.session_maker', session_maker),
|
||||
patch(
|
||||
'storage.org_service.OrgStore.get_org_by_id',
|
||||
return_value=mock_org,
|
||||
),
|
||||
patch('storage.org_service.OrgService.is_org_member', return_value=True),
|
||||
patch(
|
||||
'storage.org_service.OrgStore.get_org_by_name',
|
||||
return_value=mock_org,
|
||||
),
|
||||
patch(
|
||||
'storage.org_service.OrgStore.update_org',
|
||||
return_value=mock_org,
|
||||
),
|
||||
):
|
||||
# Act
|
||||
result = await OrgService.update_org_with_permissions(
|
||||
org_id=org_id,
|
||||
update_data=update_data,
|
||||
user_id=user_id,
|
||||
)
|
||||
|
||||
# Assert
|
||||
assert result is not None
|
||||
assert result.name == current_name
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_org_with_permissions_only_llm_fields(session_maker):
|
||||
"""
|
||||
|
||||
Reference in New Issue
Block a user