Skip to content
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.

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

Allow deleting a UserGroup (except for the default one) #1885

Merged
62 changes: 48 additions & 14 deletions fractal_server/app/routes/auth/group.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from fastapi import APIRouter
from fastapi import Depends
from fastapi import HTTPException
from fastapi import Response
from fastapi import status
from sqlalchemy.exc import IntegrityError
from sqlalchemy.ext.asyncio import AsyncSession
Expand All @@ -12,14 +13,16 @@
from sqlmodel import select

from . import current_active_superuser
from ...db import get_async_db
from ...schemas.user_group import UserGroupCreate
from ...schemas.user_group import UserGroupRead
from ...schemas.user_group import UserGroupUpdate
from ._aux_auth import _get_single_group_with_user_ids
from fractal_server.app.db import get_async_db
from fractal_server.app.models import LinkUserGroup
from fractal_server.app.models import UserGroup
from fractal_server.app.models import UserOAuth
from fractal_server.app.models.v2 import TaskGroupV2
from fractal_server.app.schemas.user_group import UserGroupCreate
from fractal_server.app.schemas.user_group import UserGroupRead
from fractal_server.app.schemas.user_group import UserGroupUpdate
from fractal_server.app.security import FRACTAL_DEFAULT_GROUP_NAME
from fractal_server.logger import set_logger

logger = set_logger(__name__)
Expand Down Expand Up @@ -174,18 +177,49 @@ async def update_single_group(
return updated_group


@router_group.delete(
"/group/{group_id}/", status_code=status.HTTP_405_METHOD_NOT_ALLOWED
)
@router_group.delete("/group/{group_id}/", status_code=204)
async def delete_single_group(
group_id: int,
user: UserOAuth = Depends(current_active_superuser),
db: AsyncSession = Depends(get_async_db),
) -> UserGroupRead:
raise HTTPException(
status_code=status.HTTP_405_METHOD_NOT_ALLOWED,
detail=(
"Deleting a user group is not allowed, as it may restrict "
"previously-granted access.",
),
) -> Response:

group = await db.get(UserGroup, group_id)

# Preliminary checks

if group is None:
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail=f"UserGroup {group_id} not found.",
)
if group.name == FRACTAL_DEFAULT_GROUP_NAME:
raise HTTPException(
status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
detail=(
"Cannot delete default UserGroup "
f"'{FRACTAL_DEFAULT_GROUP_NAME}'."
),
)

# Cascade operations

res = await db.execute(
select(LinkUserGroup).where(LinkUserGroup.group_id == group_id)
)
for link in res.scalars().all():
await db.delete(link)

res = await db.execute(
select(TaskGroupV2).where(TaskGroupV2.user_group_id == group_id)
)
for task_group in res.scalars().all():
task_group.user_group_id = None
db.add(task_group)

# Delete

await db.delete(group)
await db.commit()

return Response(status_code=status.HTTP_204_NO_CONTENT)
50 changes: 46 additions & 4 deletions tests/no_version/test_api_user_groups.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ async def test_update_group(registered_superuser_client):
assert res.json()["viewer_paths"] == ["/new"]


async def test_user_group_crud(registered_superuser_client):
async def test_user_group_crud(
registered_superuser_client, db, default_user_group
):
"""
Test basic working of POST/GET/PATCH for user groups.
"""
Expand Down Expand Up @@ -127,7 +129,7 @@ async def test_user_group_crud(registered_superuser_client):
assert res.status_code == 201
group_2_id = res.json()["id"]

# Add user A to group 1
# Add user A and B to group 1
res = await registered_superuser_client.patch(
f"{PREFIX}/group/{group_1_id}/",
json=dict(new_user_ids=[user_A_id, user_B_id]),
Expand All @@ -145,20 +147,22 @@ async def test_user_group_crud(registered_superuser_client):
)
assert res.status_code == 200
groups_data = res.json()
assert len(groups_data) == 2
assert len(groups_data) == 3
for group in groups_data:
if group["name"] == "group 1":
assert set(group["user_ids"]) == {user_A_id, user_B_id}
elif group["name"] == "group 2":
assert group["user_ids"] == [user_B_id]
elif group["name"] == default_user_group.name:
assert set(group["user_ids"]) == {user_A_id, user_B_id}
else:
raise RuntimeError("Wrong branch.")

# Get all groups (group 1, group 2) without user_ids
res = await registered_superuser_client.get(f"{PREFIX}/group/")
assert res.status_code == 200
groups_data = res.json()
assert len(groups_data) == 2
assert len(groups_data) == 3
for group in groups_data:
assert group["user_ids"] is None

Expand Down Expand Up @@ -187,6 +191,44 @@ async def test_user_group_crud(registered_superuser_client):
assert res.status_code == 422
assert "`new_user_ids` list has repetitions'" in str(res.json())

# DELETE (and cascade operations)
from sqlmodel import select
from fractal_server.app.models import LinkUserGroup
from fractal_server.app.models.v2 import TaskGroupV2

task_group = TaskGroupV2(
user_id=user_A_id,
user_group_id=group_1_id,
origin="pypi",
pkg_name="fractal-tasks-core",
)
db.add(task_group)
await db.commit()
await db.refresh(task_group)
assert task_group.user_group_id == group_1_id

res = await registered_superuser_client.delete( # actual DELETE
f"{PREFIX}/group/{group_1_id}/"
)
assert res.status_code == 204
res = await registered_superuser_client.delete(
f"{PREFIX}/group/{group_1_id}/"
)
assert res.status_code == 404
res = await registered_superuser_client.delete(
f"{PREFIX}/group/{default_user_group.id}/"
)
assert res.status_code == 422

# test cascade operations
res = await db.execute(
select(LinkUserGroup).where(LinkUserGroup.group_id == group_1_id)
)
links = res.scalars().all()
assert links == []
await db.refresh(task_group)
assert task_group.user_group_id is None


async def test_create_user_group_same_name(registered_superuser_client):
"""
Expand Down