refacto(clean-architecture): refacto DELETE /v1/admin/users/{user_id} endpoint toward clean architecture (#898)

* refacto(clean-architecture): delete_user

* refacto delete_user

* Update unit coverage badge

* refacto after review

* Update unit coverage badge

---------

Co-authored-by: Benjamin PILIA <benjamin.pilia@protonmail.com>
Co-authored-by: benjaminpilia <benjaminpilia@users.noreply.github.com>
Co-authored-by: Léo Guillaume <62661249+leoguillaume@users.noreply.github.com>
Co-authored-by: leoguillaume <leoguillaume@users.noreply.github.com>

Author: 3 people

.github/badges/coverage.json

@@ -1 +1 @@
-{"schemaVersion":1,"label":"coverage","message":"56.11%","color":"red"}
+{"schemaVersion":1,"label":"coverage","message":"56.14%","color":"red"}

api/dependencies.py

@@ -34,7 +34,7 @@
 )
 from api.use_cases.admin.roles import CreateRoleUseCase, DeleteRoleUseCase, GetRolesUseCase, GetRoleUseCase, UpdateRoleUseCase
 from api.use_cases.admin.routers import CreateRouterUseCase, DeleteRouterUseCase, GetOneRouterUseCase, GetRoutersUseCase, UpdateRouterUseCase
-from api.use_cases.admin.users import CreateUserUseCase, GetOneUserUseCase, GetUsersUseCase
+from api.use_cases.admin.users import CreateUserUseCase, DeleteUserUseCase, GetOneUserUseCase, GetUsersUseCase
 from api.use_cases.health import GetHealthModelsUseCase
 from api.use_cases.models import GetModelsUseCase, GetModelUseCase
 from api.utils.configuration import configuration
@@ -166,6 +166,15 @@ def get_users_use_case_factory(postgres_session: AsyncSession = Depends(get_post
     return GetUsersUseCase(user_repository=_user_repository(postgres_session), user_with_role_query=_user_with_role_query(postgres_session))
 
 
+def delete_user_use_case_factory(postgres_session: AsyncSession = Depends(get_postgres_session)) -> DeleteUserUseCase:
+    return DeleteUserUseCase(
+        user_repository=_user_repository(postgres_session),
+        user_with_role_query=_user_with_role_query(postgres_session),
+        router_repository=_router_repository(postgres_session),
+        provider_repository=_provider_repository(postgres_session),
+    )
+
+
 # roles use cases
 def create_role_use_case_factory(postgres_session: AsyncSession = Depends(get_postgres_session)) -> CreateRoleUseCase:
     return CreateRoleUseCase(

api/domain/provider/_providerrepository.py

@@ -52,3 +52,7 @@ async def update_provider(self, provider: Provider) -> Provider | ProviderAlread
     @abstractmethod
     async def get_all_providers(self) -> list[Provider]:
         pass
+
+    @abstractmethod
+    async def get_provider_ids_by_user_id(self, user_id: int) -> list[int]:
+        pass

api/domain/router/_routerrepository.py

@@ -61,3 +61,7 @@ async def get_aliases(self, filtered_aliases: list[str] | None = None) -> list[s
     @abstractmethod
     async def update_router(self, router: Router) -> Router | RouterNameAlreadyExistsError:
         pass
+
+    @abstractmethod
+    async def get_router_ids_by_user_id(self, user_id: int) -> list[int]:
+        pass

api/domain/user/_userrepository.py

@@ -4,7 +4,7 @@
 from api.domain.organization.errors import OrganizationNotFoundError
 from api.domain.role.errors import RoleNotFoundError
 from api.domain.user.entities import User, UserPage, UserSortField
-from api.domain.user.errors import UserAlreadyExistsError, UserNotFoundError
+from api.domain.user.errors import DeleteUserWithProvidersError, DeleteUserWithRoutersError, UserAlreadyExistsError, UserNotFoundError
 
 
 class UserRepository(ABC):
@@ -49,7 +49,7 @@ async def update_user(self, user: User) -> User | UserNotFoundError | UserAlread
         pass
 
     @abstractmethod
-    async def delete_user(self, user_id: int) -> User | UserNotFoundError:
+    async def delete_user(self, user_id: int) -> User | UserNotFoundError | DeleteUserWithRoutersError | DeleteUserWithProvidersError:
         pass
 
     @abstractmethod

api/domain/user/errors.py

@@ -20,3 +20,15 @@ class UserIsNotAdminError:
 @dataclass
 class UserExpiredError:
     pass
+
+
+@dataclass
+class DeleteUserWithProvidersError:
+    user_id: int
+    provider_ids: list[int] | None
+
+
+@dataclass
+class DeleteUserWithRoutersError:
+    user_id: int
+    router_ids: list[int] | None

api/endpoints/admin/users.py

@@ -11,24 +11,6 @@
 from api.utils.variables import EndpointRoute
 
 
-@router.delete(
-    path=EndpointRoute.ADMIN_USERS + "/{user_id}",
-    dependencies=[Security(dependency=AccessController(permissions=[PermissionType.ADMIN]))],
-    status_code=204,
-)
-async def delete_user(
-    request: Request,
-    user_id: int = Path(description="The ID of the user to delete."),
-    postgres_session: AsyncSession = Depends(get_postgres_session),
-) -> Response:
-    """
-    Delete a user.
-    """
-    await global_context.identity_access_manager.delete_user(postgres_session=postgres_session, user_id=user_id)
-
-    return Response(status_code=204)
-
-
 @router.patch(
     path=EndpointRoute.ADMIN_USERS + "/{user}",
     dependencies=[Security(dependency=AccessController(permissions=[PermissionType.ADMIN]))],

api/helpers/_identityaccessmanager.py

@@ -25,8 +25,6 @@
 from api.utils.exceptions import (
     DeleteOrganizationWithUsersException,
     DeleteRoleWithUsersException,
-    DeleteUserWithProvidersException,
-    DeleteUserWithRoutersException,
     InvalidCurrentPasswordException,
     InvalidTokenExpirationException,
     OrganizationAlreadyExistsException,
@@ -268,23 +266,6 @@ async def create_user(
 
         return user_id
 
-    @staticmethod
-    async def delete_user(postgres_session: AsyncSession, user_id: int) -> None:
-        result = await postgres_session.execute(statement=select(UserTable.id).where(UserTable.id == user_id))
-        try:
-            result.scalar_one()
-        except NoResultFound:
-            raise UserNotFoundException()
-
-        try:
-            await postgres_session.execute(statement=delete(table=UserTable).where(UserTable.id == user_id))
-        except IntegrityError as e:
-            if "provider_user_id_fkey" in str(e.orig):
-                raise DeleteUserWithProvidersException()
-            if "router_user_id_fkey" in str(e.orig):
-                raise DeleteUserWithRoutersException()
-        await postgres_session.commit()
-
     async def update_user(
         self,
         postgres_session: AsyncSession,

api/infrastructure/fastapi/endpoints/admin/users.py

@@ -4,18 +4,33 @@
 
 from fastapi import Body, Depends, Path, Query, Security
 
-from api.dependencies import create_user_use_case_factory, get_one_user_use_case_factory, get_request_context, get_users_use_case_factory
+from api.dependencies import (
+    create_user_use_case_factory,
+    delete_user_use_case_factory,
+    get_one_user_use_case_factory,
+    get_request_context,
+    get_users_use_case_factory,
+)
 from api.domain import SortOrder
 from api.domain.organization.errors import OrganizationNotFoundError
 from api.domain.role.errors import RoleNotFoundError
 from api.domain.user.entities import UserSortField
-from api.domain.user.errors import UserAlreadyExistsError, UserExpiredError, UserIsNotAdminError, UserNotFoundError
+from api.domain.user.errors import (
+    DeleteUserWithProvidersError,
+    DeleteUserWithRoutersError,
+    UserAlreadyExistsError,
+    UserExpiredError,
+    UserIsNotAdminError,
+    UserNotFoundError,
+)
 from api.infrastructure.fastapi.access import get_current_key
 from api.infrastructure.fastapi.context import RequestContext
 from api.infrastructure.fastapi.documentation import get_documentation_responses
 from api.infrastructure.fastapi.endpoints.admin import router
 from api.infrastructure.fastapi.endpoints.exceptions import (
     AccountExpiredHTTPException,
+    DeleteUserWithProvidersHTTPException,
+    DeleteUserWithRoutersHTTPException,
     InternalServerHTTPException,
     NotAdminUserHTTPException,
     OrganizationNotFoundHTTPException,
@@ -28,6 +43,9 @@
     CreateUserCommand,
     CreateUserUseCase,
     CreateUserUseCaseSuccess,
+    DeleteUserCommand,
+    DeleteUserUseCase,
+    DeleteUserUseCaseSuccess,
     GetOneUserCommand,
     GetOneUserUseCase,
     GetOneUserUseCaseSuccess,
@@ -184,3 +202,53 @@ async def get_users(
             raise AccountExpiredHTTPException()
         case _ as unreachable:
             assert_never(unreachable)
+
+
+@router.delete(
+    path=EndpointRoute.ADMIN_USERS + "/{user_id}",
+    dependencies=[Security(dependency=get_current_key)],
+    status_code=200,
+    responses=get_documentation_responses(
+        [
+            UserNotFoundHTTPException,
+            DeleteUserWithRoutersHTTPException,
+            DeleteUserWithProvidersHTTPException,
+        ]
+    ),
+)
+async def delete_user(
+    user_id: int = Path(description="The ID of the user to delete."),
+    delete_user_use_case: DeleteUserUseCase = Depends(delete_user_use_case_factory),
+    request_context: ContextVar[RequestContext] = Depends(get_request_context),
+) -> UserResponse:
+    command = DeleteUserCommand(
+        authenticated_user_id=request_context.get().user_id,
+        user_id=user_id,
+    )
+    try:
+        result = await delete_user_use_case.execute(command)
+    except Exception as e:
+        logger.exception(
+            "Unexpected error while executing delete_user use case",
+            extra={
+                "authenticated_user_id": command.authenticated_user_id,
+                "user_id": command.user_id,
+                "error_type": type(e).__name__,
+            },
+        )
+        raise InternalServerHTTPException()
+    match result:
+        case DeleteUserUseCaseSuccess(user=user):
+            return UserResponse.model_validate(user, from_attributes=True)
+        case UserNotFoundError(id=not_found_id):
+            raise UserNotFoundHTTPException(user_id=not_found_id)
+        case DeleteUserWithRoutersError(router_ids=router_ids):
+            raise DeleteUserWithRoutersHTTPException(router_ids=router_ids)
+        case DeleteUserWithProvidersError(provider_ids=provider_ids):
+            raise DeleteUserWithProvidersHTTPException(provider_ids=provider_ids)
+        case UserIsNotAdminError():
+            raise NotAdminUserHTTPException()
+        case UserExpiredError():
+            raise AccountExpiredHTTPException()
+        case _ as unreachable:
+            assert_never(unreachable)

api/infrastructure/fastapi/endpoints/exceptions.py

@@ -164,6 +164,22 @@ def __init__(self, role_id: int, number_of_users: int) -> None:
         super().__init__(status_code=409, detail=f"Role {role_id} has {number_of_users} users and cannot be removed.")
 
 
+class DeleteUserWithRoutersHTTPException(HTTPException):
+    status_code = 409
+    detail = "User cannot be deleted because the user owns routers: {router_ids}."
+
+    def __init__(self, router_ids: list[int] | None) -> None:
+        super().__init__(status_code=409, detail=f"User cannot be deleted because the user owns routers: {router_ids}.")
+
+
+class DeleteUserWithProvidersHTTPException(HTTPException):
+    status_code = 409
+    detail = "User cannot be deleted because the user owns providers: {provider_ids}."
+
+    def __init__(self, provider_ids: list[int] | None) -> None:
+        super().__init__(status_code=409, detail=f"User cannot be deleted because the user owns providers: {provider_ids}.")
+
+
 # 413