From bb5a8482f389c5ca5e3d01c5be16583debc06f1b Mon Sep 17 00:00:00 2001 From: Abhimanyu Saharan Date: Mon, 9 Feb 2026 00:26:49 +0530 Subject: [PATCH] feat: add endpoint to remove organization members with access cleanup --- backend/app/api/organizations.py | 58 ++++++ .../test_organizations_member_remove_api.py | 178 ++++++++++++++++++ frontend/src/app/organization/page.tsx | 78 +++++++- 3 files changed, 313 insertions(+), 1 deletion(-) create mode 100644 backend/tests/test_organizations_member_remove_api.py diff --git a/backend/app/api/organizations.py b/backend/app/api/organizations.py index b03dc895..5ff5ace1 100644 --- a/backend/app/api/organizations.py +++ b/backend/app/api/organizations.py @@ -359,6 +359,64 @@ async def update_member_access( return _member_to_read(member, user) +@router.delete("/me/members/{member_id}", response_model=OkResponse) +async def remove_org_member( + member_id: UUID, + session: AsyncSession = Depends(get_session), + ctx: OrganizationContext = Depends(require_org_admin), +) -> OkResponse: + member = await session.get(OrganizationMember, member_id) + if member is None or member.organization_id != ctx.organization.id: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND) + if member.user_id == ctx.member.user_id: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="You cannot remove yourself from the organization", + ) + if member.role == "owner" and ctx.member.role != "owner": + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Only owners can remove owners", + ) + if member.role == "owner": + owner_ids = list( + await session.exec( + select(OrganizationMember.id).where( + col(OrganizationMember.organization_id) == ctx.organization.id, + col(OrganizationMember.role) == "owner", + ) + ) + ) + if len(owner_ids) <= 1: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_CONTENT, + detail="Organization must have at least one owner", + ) + + await session.execute( + delete(OrganizationBoardAccess).where( + col(OrganizationBoardAccess.organization_member_id) == member.id + ) + ) + + user = await session.get(User, member.user_id) + if user is not None and user.active_organization_id == ctx.organization.id: + fallback_org_id = ( + await session.exec( + select(OrganizationMember.organization_id) + .where(col(OrganizationMember.user_id) == user.id) + .where(col(OrganizationMember.organization_id) != ctx.organization.id) + .order_by(col(OrganizationMember.created_at).asc()) + ) + ).first() + user.active_organization_id = fallback_org_id + session.add(user) + + await session.delete(member) + await session.commit() + return OkResponse() + + @router.get("/me/invites", response_model=DefaultLimitOffsetPage[OrganizationInviteRead]) async def list_org_invites( session: AsyncSession = Depends(get_session), diff --git a/backend/tests/test_organizations_member_remove_api.py b/backend/tests/test_organizations_member_remove_api.py new file mode 100644 index 00000000..f542f16c --- /dev/null +++ b/backend/tests/test_organizations_member_remove_api.py @@ -0,0 +1,178 @@ +from __future__ import annotations + +from dataclasses import dataclass, field +from types import SimpleNamespace +from typing import Any +from uuid import uuid4 + +import pytest +from fastapi import HTTPException, status + +from app.api import organizations +from app.models.organization_members import OrganizationMember +from app.models.users import User + + +@dataclass +class _FakeExecResult: + first_value: Any = None + all_values: list[Any] | None = None + + def first(self) -> Any: + return self.first_value + + def __iter__(self): + return iter(self.all_values or []) + + +@dataclass +class _FakeSession: + exec_results: list[Any] + get_results: dict[tuple[type[Any], Any], Any] = field(default_factory=dict) + + executed: list[Any] = field(default_factory=list) + deleted: list[Any] = field(default_factory=list) + added: list[Any] = field(default_factory=list) + committed: int = 0 + + async def exec(self, _statement: Any) -> Any: + if not self.exec_results: + raise AssertionError("No more exec_results left for session.exec") + return self.exec_results.pop(0) + + async def get(self, model: type[Any], key: Any) -> Any: + return self.get_results.get((model, key)) + + async def execute(self, statement: Any) -> None: + self.executed.append(statement) + + async def delete(self, value: Any) -> None: + self.deleted.append(value) + + def add(self, value: Any) -> None: + self.added.append(value) + + async def commit(self) -> None: + self.committed += 1 + + +@pytest.mark.asyncio +async def test_remove_org_member_deletes_member_access_and_member() -> None: + org_id = uuid4() + member_id = uuid4() + actor_user_id = uuid4() + target_user_id = uuid4() + fallback_org_id = uuid4() + member = OrganizationMember( + id=member_id, + organization_id=org_id, + user_id=target_user_id, + role="member", + ) + user = User( + id=target_user_id, + clerk_user_id="target", + active_organization_id=org_id, + ) + session = _FakeSession( + exec_results=[_FakeExecResult(first_value=fallback_org_id)], + get_results={ + (OrganizationMember, member_id): member, + (User, target_user_id): user, + }, + ) + ctx = SimpleNamespace( + organization=SimpleNamespace(id=org_id), + member=SimpleNamespace(user_id=actor_user_id, role="admin"), + ) + + await organizations.remove_org_member(member_id=member_id, session=session, ctx=ctx) + + executed_tables = [statement.table.name for statement in session.executed] + assert executed_tables == ["organization_board_access"] + assert session.deleted == [member] + assert session.committed == 1 + assert user.active_organization_id == fallback_org_id + assert session.added == [user] + + +@pytest.mark.asyncio +async def test_remove_org_member_disallows_self_removal() -> None: + org_id = uuid4() + user_id = uuid4() + member = OrganizationMember( + id=uuid4(), + organization_id=org_id, + user_id=user_id, + role="member", + ) + session = _FakeSession( + exec_results=[], + get_results={(OrganizationMember, member.id): member}, + ) + ctx = SimpleNamespace( + organization=SimpleNamespace(id=org_id), + member=SimpleNamespace(user_id=user_id, role="owner"), + ) + + with pytest.raises(HTTPException) as exc_info: + await organizations.remove_org_member(member_id=member.id, session=session, ctx=ctx) + + assert exc_info.value.status_code == status.HTTP_403_FORBIDDEN + assert session.executed == [] + assert session.deleted == [] + assert session.committed == 0 + + +@pytest.mark.asyncio +async def test_remove_org_member_requires_owner_to_remove_owner() -> None: + org_id = uuid4() + member = OrganizationMember( + id=uuid4(), + organization_id=org_id, + user_id=uuid4(), + role="owner", + ) + session = _FakeSession( + exec_results=[], + get_results={(OrganizationMember, member.id): member}, + ) + ctx = SimpleNamespace( + organization=SimpleNamespace(id=org_id), + member=SimpleNamespace(user_id=uuid4(), role="admin"), + ) + + with pytest.raises(HTTPException) as exc_info: + await organizations.remove_org_member(member_id=member.id, session=session, ctx=ctx) + + assert exc_info.value.status_code == status.HTTP_403_FORBIDDEN + assert session.executed == [] + assert session.deleted == [] + assert session.committed == 0 + + +@pytest.mark.asyncio +async def test_remove_org_member_rejects_removing_last_owner() -> None: + org_id = uuid4() + member = OrganizationMember( + id=uuid4(), + organization_id=org_id, + user_id=uuid4(), + role="owner", + ) + session = _FakeSession( + exec_results=[_FakeExecResult(all_values=[member.id])], + get_results={(OrganizationMember, member.id): member}, + ) + ctx = SimpleNamespace( + organization=SimpleNamespace(id=org_id), + member=SimpleNamespace(user_id=uuid4(), role="owner"), + ) + + with pytest.raises(HTTPException) as exc_info: + await organizations.remove_org_member(member_id=member.id, session=session, ctx=ctx) + + assert exc_info.value.status_code == status.HTTP_422_UNPROCESSABLE_CONTENT + assert session.executed == [] + assert session.deleted == [] + assert session.committed == 0 diff --git a/frontend/src/app/organization/page.tsx b/frontend/src/app/organization/page.tsx index e4274572..2a23c481 100644 --- a/frontend/src/app/organization/page.tsx +++ b/frontend/src/app/organization/page.tsx @@ -329,6 +329,7 @@ export default function OrganizationPage() { const [accessMap, setAccessMap] = useState(null); const [accessError, setAccessError] = useState(null); const [deleteOrgOpen, setDeleteOrgOpen] = useState(false); + const [removeMemberOpen, setRemoveMemberOpen] = useState(false); const orgQuery = useGetMyOrgApiV1OrganizationsMeGet< getMyOrgApiV1OrganizationsMeGetResponse, @@ -557,6 +558,28 @@ export default function OrganizationPage() { }, }); + const removeMemberMutation = useMutation< + { data: unknown; status: number; headers: Headers }, + ApiError, + { memberId: string } + >({ + mutationFn: async ({ memberId }) => + customFetch<{ data: unknown; status: number; headers: Headers }>( + `/api/v1/organizations/me/members/${memberId}`, + { method: "DELETE" }, + ), + onSuccess: async () => { + setRemoveMemberOpen(false); + setAccessDialogOpen(false); + setActiveMemberId(null); + await queryClient.invalidateQueries({ + queryKey: getListOrgMembersApiV1OrganizationsMeMembersGetQueryKey({ + limit: 200, + }), + }); + }, + }); + const resetAccessState = () => { setAccessRole(null); setAccessScope(null); @@ -720,6 +743,17 @@ export default function OrganizationPage() { deleteOrganizationMutation.mutate(); }; + const activeMemberCanBeRemoved = + isAdmin && + memberDetails !== null && + memberDetails.user_id !== membershipQuery.data?.data.user_id && + (isOwner || memberDetails.role !== "owner"); + + const handleRemoveMember = () => { + if (!activeMemberId || !activeMemberCanBeRemoved) return; + removeMemberMutation.mutate({ memberId: activeMemberId }); + }; + const memberAccessSummary = (member: OrganizationMemberRead) => summarizeAccess(member.all_boards_read, member.all_boards_write); @@ -1171,6 +1205,19 @@ export default function OrganizationPage() { )} + {activeMemberCanBeRemoved ? ( + + ) : null}