feat: add endpoint to remove organization members with access cleanup
This commit is contained in:
@@ -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),
|
||||
|
||||
178
backend/tests/test_organizations_member_remove_api.py
Normal file
178
backend/tests/test_organizations_member_remove_api.py
Normal file
@@ -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
|
||||
@@ -329,6 +329,7 @@ export default function OrganizationPage() {
|
||||
const [accessMap, setAccessMap] = useState<BoardAccessState | null>(null);
|
||||
const [accessError, setAccessError] = useState<string | null>(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() {
|
||||
)}
|
||||
|
||||
<DialogFooter className="pt-2">
|
||||
{activeMemberCanBeRemoved ? (
|
||||
<Button
|
||||
type="button"
|
||||
variant="outline"
|
||||
className="mr-auto border-rose-200 text-rose-600 hover:border-rose-300 hover:text-rose-700"
|
||||
onClick={() => {
|
||||
removeMemberMutation.reset();
|
||||
setRemoveMemberOpen(true);
|
||||
}}
|
||||
>
|
||||
Remove member
|
||||
</Button>
|
||||
) : null}
|
||||
<Button
|
||||
type="button"
|
||||
variant="outline"
|
||||
@@ -1183,7 +1230,8 @@ export default function OrganizationPage() {
|
||||
onClick={handleSaveAccess}
|
||||
disabled={
|
||||
updateMemberAccessMutation.isPending ||
|
||||
updateMemberRoleMutation.isPending
|
||||
updateMemberRoleMutation.isPending ||
|
||||
removeMemberMutation.isPending
|
||||
}
|
||||
>
|
||||
{updateMemberAccessMutation.isPending ||
|
||||
@@ -1217,6 +1265,34 @@ export default function OrganizationPage() {
|
||||
confirmLabel="Delete organization"
|
||||
confirmingLabel="Deleting…"
|
||||
/>
|
||||
<ConfirmActionDialog
|
||||
open={removeMemberOpen}
|
||||
onOpenChange={(open) => {
|
||||
setRemoveMemberOpen(open);
|
||||
if (!open) {
|
||||
removeMemberMutation.reset();
|
||||
}
|
||||
}}
|
||||
ariaLabel="Remove organization member"
|
||||
title="Remove member"
|
||||
description={
|
||||
<>
|
||||
Remove{" "}
|
||||
<strong>
|
||||
{memberDetails?.user?.name ||
|
||||
memberDetails?.user?.preferred_name ||
|
||||
memberDetails?.user?.email ||
|
||||
"this member"}
|
||||
</strong>{" "}
|
||||
from <strong>{orgName}</strong>? They will lose access immediately.
|
||||
</>
|
||||
}
|
||||
errorMessage={removeMemberMutation.error?.message}
|
||||
onConfirm={handleRemoveMember}
|
||||
isConfirming={removeMemberMutation.isPending}
|
||||
confirmLabel="Remove member"
|
||||
confirmingLabel="Removing…"
|
||||
/>
|
||||
</DashboardShell>
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user