Fix #145: tree membership management (list / add / role / remove)

TreeMembership was enforced on every read/write but had no API or UI to manage members — trees were effectively single-user, breaking full-CRUD (NN#8). Backend (/trees/{id}/members): list (members only — the list exposes emails, so non-members never see it, even on public trees); add an existing user by email (owner only, 404 if no such account, 409 if already a member); PATCH role; DELETE. A tree must always keep ≥1 owner (demote/remove of the sole owner → 409). All changes audited. Frontend: a Members page (owner gets add-by-email + per-member role select + remove; others see a read-only list) and a sidebar entry. Test covers the full lifecycle + every guard. Suite 77 passed. Closes #145 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Justin Paul <justin@jpaul.me>
2026-06-09 12:43:30 -04:00
parent 6d3147e86d
commit eb0350733b
9 changed files with 991 additions and 0 deletions
@@ -9,6 +9,7 @@ from app.api.v1 import (
    events,
    gedcom,
    media,
+    members,
    names,
    persons,
    public,
@@ -32,3 +33,4 @@ api_router.include_router(media.router)
 api_router.include_router(gedcom.router)
 api_router.include_router(cleanup.router)
 api_router.include_router(public.router)
+api_router.include_router(members.router)
@@ -0,0 +1,61 @@
+"""Tree membership management endpoints (owner-managed; members can list)."""
+
+import uuid
+
+from fastapi import APIRouter, status
+
+from app.api.deps import CurrentUser, SessionDep
+from app.schemas.membership import MemberAdd, MemberRoleUpdate, MembershipRead
+from app.services import membership_service, tree_service
+
+router = APIRouter(prefix="/trees", tags=["members"])
+
+
+@router.get("/{tree_id}/members", response_model=list[MembershipRead])
+async def list_members(
+    tree_id: uuid.UUID, session: SessionDep, current: CurrentUser
+) -> list[MembershipRead]:
+    tree = await tree_service.get_tree(session, viewer_id=current.id, tree_id=tree_id)
+    rows = await membership_service.list_members(session, viewer_id=current.id, tree=tree)
+    return [MembershipRead(**r) for r in rows]
+
+
+@router.post(
+    "/{tree_id}/members", response_model=MembershipRead, status_code=status.HTTP_201_CREATED
+)
+async def add_member(
+    tree_id: uuid.UUID, data: MemberAdd, session: SessionDep, current: CurrentUser
+) -> MembershipRead:
+    tree = await tree_service.get_tree(session, viewer_id=current.id, tree_id=tree_id)
+    row = await membership_service.add_member(
+        session, actor=current, tree=tree, email=data.email, role=data.role
+    )
+    return MembershipRead(**row)
+
+
+@router.patch("/{tree_id}/members/{membership_id}", response_model=MembershipRead)
+async def update_member(
+    tree_id: uuid.UUID,
+    membership_id: uuid.UUID,
+    data: MemberRoleUpdate,
+    session: SessionDep,
+    current: CurrentUser,
+) -> MembershipRead:
+    tree = await tree_service.get_tree(session, viewer_id=current.id, tree_id=tree_id)
+    row = await membership_service.update_member_role(
+        session, actor=current, tree=tree, membership_id=membership_id, role=data.role
+    )
+    return MembershipRead(**row)
+
+
+@router.delete("/{tree_id}/members/{membership_id}", status_code=status.HTTP_204_NO_CONTENT)
+async def remove_member(
+    tree_id: uuid.UUID,
+    membership_id: uuid.UUID,
+    session: SessionDep,
+    current: CurrentUser,
+) -> None:
+    tree = await tree_service.get_tree(session, viewer_id=current.id, tree_id=tree_id)
+    await membership_service.remove_member(
+        session, actor=current, tree=tree, membership_id=membership_id
+    )
@@ -0,0 +1,26 @@
+import uuid
+from datetime import datetime
+
+from pydantic import BaseModel, ConfigDict
+
+from app.models.enums import MembershipRole
+
+
+class MembershipRead(BaseModel):
+    model_config = ConfigDict(from_attributes=True)
+
+    id: uuid.UUID
+    user_id: uuid.UUID
+    email: str
+    display_name: str | None
+    role: MembershipRole
+    created_at: datetime
+
+
+class MemberAdd(BaseModel):
+    email: str
+    role: MembershipRole = MembershipRole.viewer
+
+
+class MemberRoleUpdate(BaseModel):
+    role: MembershipRole
@@ -0,0 +1,156 @@
+"""Tree membership management: list / add / change-role / remove.
+
+Only an owner may change membership. A tree must always keep at least one owner.
+The member list (which exposes user emails) is visible only to members — never
+to a non-member viewing a public/unlisted tree.
+"""
+
+import uuid
+
+from sqlalchemy import func, select
+from sqlalchemy.ext.asyncio import AsyncSession
+
+from app.models.enums import MembershipRole
+from app.models.tree import Tree, TreeMembership
+from app.models.user import User
+from app.services import privacy
+from app.services.audit import record_audit
+from app.services.exceptions import Conflict, Forbidden, NotFound
+
+
+async def _require_owner(session: AsyncSession, *, actor_id: uuid.UUID, tree: Tree) -> None:
+    if await privacy.get_membership_role(session, actor_id, tree.id) is not MembershipRole.owner:
+        raise Forbidden("only the owner can manage members")
+
+
+async def _owner_count(session: AsyncSession, tree_id: uuid.UUID) -> int:
+    return (
+        await session.execute(
+            select(func.count())
+            .select_from(TreeMembership)
+            .where(TreeMembership.tree_id == tree_id, TreeMembership.role == MembershipRole.owner)
+        )
+    ).scalar_one()
+
+
+def _row(m: TreeMembership, u: User) -> dict:
+    return {
+        "id": m.id,
+        "user_id": u.id,
+        "email": u.email,
+        "display_name": u.display_name,
+        "role": m.role,
+        "created_at": m.created_at,
+    }
+
+
+async def list_members(session: AsyncSession, *, viewer_id: uuid.UUID, tree: Tree) -> list[dict]:
+    # Member-only: the list exposes emails, so a non-member (even on a public
+    # tree) must not see it.
+    if await privacy.get_membership_role(session, viewer_id, tree.id) is None:
+        raise Forbidden("only members can see the member list")
+    rows = (
+        await session.execute(
+            select(TreeMembership, User)
+            .join(User, User.id == TreeMembership.user_id)
+            .where(TreeMembership.tree_id == tree.id)
+            .order_by(TreeMembership.created_at)
+        )
+    ).all()
+    return [_row(m, u) for m, u in rows]
+
+
+async def add_member(
+    session: AsyncSession, *, actor: User, tree: Tree, email: str, role: MembershipRole
+) -> dict:
+    await _require_owner(session, actor_id=actor.id, tree=tree)
+    user = (
+        await session.execute(
+            select(User).where(User.email == email, User.deleted_at.is_(None))
+        )
+    ).scalar_one_or_none()
+    if user is None:
+        raise NotFound("no user with that email on this instance")
+    if await privacy.get_membership_role(session, user.id, tree.id) is not None:
+        raise Conflict("that user is already a member")
+    m = TreeMembership(tree_id=tree.id, user_id=user.id, role=role)
+    session.add(m)
+    record_audit(
+        session,
+        action="add_member",
+        entity_type="Tree",
+        entity_id=tree.id,
+        tree_id=tree.id,
+        actor_user_id=actor.id,
+        after={"user_id": str(user.id), "role": role.value},
+    )
+    await session.commit()
+    await session.refresh(m)
+    return _row(m, user)
+
+
+async def _get_membership(
+    session: AsyncSession, tree: Tree, membership_id: uuid.UUID
+) -> TreeMembership:
+    m = (
+        await session.execute(
+            select(TreeMembership).where(
+                TreeMembership.id == membership_id, TreeMembership.tree_id == tree.id
+            )
+        )
+    ).scalar_one_or_none()
+    if m is None:
+        raise NotFound("member not found")
+    return m
+
+
+async def update_member_role(
+    session: AsyncSession,
+    *,
+    actor: User,
+    tree: Tree,
+    membership_id: uuid.UUID,
+    role: MembershipRole,
+) -> dict:
+    await _require_owner(session, actor_id=actor.id, tree=tree)
+    m = await _get_membership(session, tree, membership_id)
+    if (
+        m.role == MembershipRole.owner
+        and role != MembershipRole.owner
+        and await _owner_count(session, tree.id) <= 1
+    ):
+        raise Conflict("a tree must keep at least one owner")
+    m.role = role
+    record_audit(
+        session,
+        action="update_member",
+        entity_type="Tree",
+        entity_id=tree.id,
+        tree_id=tree.id,
+        actor_user_id=actor.id,
+        after={"membership_id": str(m.id), "role": role.value},
+    )
+    await session.commit()
+    await session.refresh(m)
+    u = (await session.execute(select(User).where(User.id == m.user_id))).scalar_one()
+    return _row(m, u)
+
+
+async def remove_member(
+    session: AsyncSession, *, actor: User, tree: Tree, membership_id: uuid.UUID
+) -> None:
+    await _require_owner(session, actor_id=actor.id, tree=tree)
+    m = await _get_membership(session, tree, membership_id)
+    if m.role == MembershipRole.owner and await _owner_count(session, tree.id) <= 1:
+        raise Conflict("a tree must keep at least one owner")
+    await session.delete(m)
+    record_audit(
+        session,
+        action="remove_member",
+        entity_type="Tree",
+        entity_id=tree.id,
+        tree_id=tree.id,
+        actor_user_id=actor.id,
+        after={"membership_id": str(membership_id)},
+    )
+    await session.commit()