From fec39530095990d2f0ddedb8cb047fccb4a4b618 Mon Sep 17 00:00:00 2001 From: curo1305 Date: Sat, 18 Apr 2026 22:16:49 +0200 Subject: [PATCH 1/4] feat: category scopes, group-admin role, and permission model - Three category scopes: personal / group / system (watch) - PascalCase-with-dashes naming convention enforced at backend + frontend - is_group_admin flag on GroupMembership; PATCH endpoint for admins to toggle it - Categories router: scope-based list/create/rename/delete with _check_can_manage_cat - Documents router: delete uses is_admin + can_delete share flag + group-admin check; remove_category requires doc ownership; assign_category accepts group/system categories - Proxy layers inject x-user-is-admin and x-user-admin-groups headers - Frontend: ManageCategoriesDialog grouped by scope with lock icons; SourcePanel scope picker + client-side name validation; AdminGroupsPage group-admin checkbox Co-Authored-By: Claude Sonnet 4.6 --- backend/CLAUDE.md | 3 + .../e1f2a3b4c5d6_add_group_member_is_admin.py | 32 +++ backend/app/models/group.py | 5 +- backend/app/routers/categories_proxy.py | 18 +- backend/app/routers/documents_proxy.py | 21 +- backend/app/routers/groups.py | 23 +- backend/app/routers/users.py | 9 +- backend/app/schemas/group.py | 5 + backend/app/schemas/user.py | 1 + .../2026-04-18_category-scopes-group-admin.md | 34 +++ features/doc-service/CLAUDE.md | 19 +- .../versions/0005_add_share_can_delete.py | 32 +++ .../versions/0006_add_category_scope.py | 42 ++++ features/doc-service/app/deps.py | 19 ++ features/doc-service/app/models/category.py | 2 + .../doc-service/app/routers/categories.py | 96 ++++++-- features/doc-service/app/routers/documents.py | 65 +++++- features/doc-service/app/schemas/category.py | 3 + frontend/src/api/client.ts | 13 +- .../src/components/ManageCategoriesDialog.tsx | 212 ++++++++++++------ frontend/src/components/SourcePanel.tsx | 173 ++++++++++---- frontend/src/pages/AdminGroupsPage.tsx | 19 ++ 22 files changed, 691 insertions(+), 155 deletions(-) create mode 100644 backend/alembic/versions/e1f2a3b4c5d6_add_group_member_is_admin.py create mode 100644 changelog/2026-04-18_category-scopes-group-admin.md create mode 100644 features/doc-service/alembic/versions/0005_add_share_can_delete.py create mode 100644 features/doc-service/alembic/versions/0006_add_category_scope.py diff --git a/backend/CLAUDE.md b/backend/CLAUDE.md index 4c85907..2eced14 100644 --- a/backend/CLAUDE.md +++ b/backend/CLAUDE.md @@ -115,6 +115,7 @@ Relationship: `profile` (one-to-one, cascade all+delete-orphan) | `id` | String | PK, UUID | | `group_id` | String | FK→groups.id, indexed, CASCADE | | `user_id` | String | FK→users.id, indexed, CASCADE | +| `is_group_admin` | Boolean | NOT NULL, default=false | grants group-admin rights (manage group categories, delete shared docs) | | `joined_at` | DateTime(tz) | server_default=now() | Unique constraint: `(group_id, user_id)` @@ -128,6 +129,7 @@ Unique constraint: `(group_id, user_id)` | `a3f9c2d14e87` | `add_groups_and_group_memberships` | | `c7e8f9a0b1d2` | `add_dashboard_app_ids_to_users` | | `dd6ad2f2c211` | `add_color_mode_to_users` | +| `e1f2a3b4c5d6` | `add_group_member_is_admin` | --- @@ -177,6 +179,7 @@ Unique constraint: `(group_id, user_id)` | DELETE | `/api/admin/groups/{id}` | Delete (cascades memberships) | | POST | `/api/admin/groups/{id}/members/{user_id}` | Add member | | DELETE | `/api/admin/groups/{id}/members/{user_id}` | Remove member | +| PATCH | `/api/admin/groups/{id}/members/{user_id}/admin` | Set/unset group admin role (body: `{ is_group_admin: bool }`) | ### Settings (`/api/settings`) — admin-only diff --git a/backend/alembic/versions/e1f2a3b4c5d6_add_group_member_is_admin.py b/backend/alembic/versions/e1f2a3b4c5d6_add_group_member_is_admin.py new file mode 100644 index 0000000..b0e0773 --- /dev/null +++ b/backend/alembic/versions/e1f2a3b4c5d6_add_group_member_is_admin.py @@ -0,0 +1,32 @@ +"""add is_group_admin to group_memberships + +Revision ID: e1f2a3b4c5d6 +Revises: dd6ad2f2c211 +Create Date: 2026-04-18 + +""" +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +revision: str = "e1f2a3b4c5d6" +down_revision: Union[str, None] = "dd6ad2f2c211" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.add_column( + "group_memberships", + sa.Column( + "is_group_admin", + sa.Boolean(), + nullable=False, + server_default=sa.text("false"), + ), + ) + + +def downgrade() -> None: + op.drop_column("group_memberships", "is_group_admin") diff --git a/backend/app/models/group.py b/backend/app/models/group.py index c208b2a..7a1c647 100644 --- a/backend/app/models/group.py +++ b/backend/app/models/group.py @@ -1,7 +1,7 @@ import uuid from datetime import datetime, timezone -from sqlalchemy import DateTime, ForeignKey, String, UniqueConstraint +from sqlalchemy import Boolean, DateTime, ForeignKey, String, UniqueConstraint from sqlalchemy.orm import Mapped, mapped_column, relationship from app.database import Base @@ -35,6 +35,9 @@ class GroupMembership(Base): user_id: Mapped[str] = mapped_column( String, ForeignKey("users.id", ondelete="CASCADE"), nullable=False, index=True ) + is_group_admin: Mapped[bool] = mapped_column( + Boolean, nullable=False, default=False, server_default="false" + ) joined_at: Mapped[datetime] = mapped_column( DateTime(timezone=True), default=lambda: datetime.now(timezone.utc), diff --git a/backend/app/routers/categories_proxy.py b/backend/app/routers/categories_proxy.py index 7d6936b..91ea15b 100644 --- a/backend/app/routers/categories_proxy.py +++ b/backend/app/routers/categories_proxy.py @@ -39,18 +39,26 @@ _HOP_BY_HOP = frozenset([ _STRIP_RESPONSE = frozenset([*_HOP_BY_HOP, "content-length", "content-type"]) -async def _forward_headers(request: Request, user_id: str, db: AsyncSession) -> dict: +async def _forward_headers( + request: Request, user_id: str, is_admin: bool, db: AsyncSession +) -> dict: headers = { k: v for k, v in request.headers.items() if k.lower() not in _HOP_BY_HOP } headers["x-user-id"] = user_id - result = await db.execute( - select(GroupMembership.group_id).where(GroupMembership.user_id == user_id) + headers["x-user-is-admin"] = "true" if is_admin else "false" + + mem_result = await db.execute( + select(GroupMembership.group_id, GroupMembership.is_group_admin) + .where(GroupMembership.user_id == user_id) ) - group_ids = [row[0] for row in result.all()] + rows = mem_result.all() + group_ids = [row[0] for row in rows] + admin_group_ids = [row[0] for row in rows if row[1]] headers["x-user-groups"] = ",".join(group_ids) + headers["x-user-admin-groups"] = ",".join(admin_group_ids) return headers @@ -63,7 +71,7 @@ async def proxy_categories( path: str = "", ) -> Response: url = f"/categories/{path}" if path else "/categories" - headers = await _forward_headers(request, str(current_user.id), db) + headers = await _forward_headers(request, str(current_user.id), current_user.is_superuser, db) body = await request.body() try: diff --git a/backend/app/routers/documents_proxy.py b/backend/app/routers/documents_proxy.py index 55bcc61..1d55203 100644 --- a/backend/app/routers/documents_proxy.py +++ b/backend/app/routers/documents_proxy.py @@ -50,21 +50,28 @@ _HOP_BY_HOP = frozenset([ _STRIP_RESPONSE = frozenset([*_HOP_BY_HOP, "content-length", "content-type"]) -async def _forward_headers(request: Request, user_id: str, db: AsyncSession) -> dict: +async def _forward_headers( + request: Request, user_id: str, is_admin: bool, db: AsyncSession +) -> dict: headers = { k: v for k, v in request.headers.items() if k.lower() not in _HOP_BY_HOP } headers["x-user-id"] = user_id + headers["x-user-is-admin"] = "true" if is_admin else "false" - # Inject the user's group memberships so the doc-service can evaluate - # group-shared document access without querying the backend DB. - result = await db.execute( - select(GroupMembership.group_id).where(GroupMembership.user_id == user_id) + # Inject group memberships and group-admin status so the doc-service can + # evaluate ownership, sharing access, and group-admin permissions. + mem_result = await db.execute( + select(GroupMembership.group_id, GroupMembership.is_group_admin) + .where(GroupMembership.user_id == user_id) ) - group_ids = [row[0] for row in result.all()] + rows = mem_result.all() + group_ids = [row[0] for row in rows] + admin_group_ids = [row[0] for row in rows if row[1]] headers["x-user-groups"] = ",".join(group_ids) + headers["x-user-admin-groups"] = ",".join(admin_group_ids) return headers @@ -78,7 +85,7 @@ async def proxy_documents( path: str = "", ) -> Response: url = f"/documents/{path}" if path else "/documents" - headers = await _forward_headers(request, str(current_user.id), db) + headers = await _forward_headers(request, str(current_user.id), current_user.is_superuser, db) body = await request.body() try: diff --git a/backend/app/routers/groups.py b/backend/app/routers/groups.py index a1cd774..0fc3f3e 100644 --- a/backend/app/routers/groups.py +++ b/backend/app/routers/groups.py @@ -7,7 +7,7 @@ from app.database import get_db from app.deps import get_current_admin from app.models.group import Group, GroupMembership from app.models.user import User -from app.schemas.group import GroupCreate, GroupDetailOut, GroupOut, GroupUpdate, GroupMemberOut +from app.schemas.group import GroupCreate, GroupDetailOut, GroupMemberAdminUpdate, GroupMemberOut, GroupOut, GroupUpdate router = APIRouter() @@ -111,6 +111,7 @@ async def get_group( email=user.email, full_name=user.full_name, is_active=user.is_active, + is_group_admin=membership.is_group_admin, joined_at=membership.joined_at, ) for membership, user in rows @@ -197,6 +198,26 @@ async def add_member( await db.commit() +@router.patch("/{group_id}/members/{user_id}/admin", status_code=status.HTTP_204_NO_CONTENT) +async def set_member_admin( + group_id: str, + user_id: str, + body: GroupMemberAdminUpdate, + _admin: User = Depends(get_current_admin), + db: AsyncSession = Depends(get_db), +) -> None: + result = await db.execute( + select(GroupMembership).where( + GroupMembership.group_id == group_id, GroupMembership.user_id == user_id + ) + ) + membership = result.scalar_one_or_none() + if not membership: + raise HTTPException(status_code=404, detail="User is not a member of this group") + membership.is_group_admin = body.is_group_admin + await db.commit() + + @router.delete("/{group_id}/members/{user_id}", status_code=status.HTTP_204_NO_CONTENT) async def remove_member( group_id: str, diff --git a/backend/app/routers/users.py b/backend/app/routers/users.py index 9974f7a..50d41c8 100644 --- a/backend/app/routers/users.py +++ b/backend/app/routers/users.py @@ -39,14 +39,17 @@ async def get_my_groups( current_user: User = Depends(get_current_user), db: AsyncSession = Depends(get_db), ): - """Return all groups the current user belongs to.""" + """Return all groups the current user belongs to, including their admin status.""" result = await db.execute( - select(Group) + select(Group, GroupMembership.is_group_admin) .join(GroupMembership, GroupMembership.group_id == Group.id) .where(GroupMembership.user_id == current_user.id) .order_by(Group.name) ) - return result.scalars().all() + return [ + UserGroupOut(id=g.id, name=g.name, description=g.description, is_group_admin=is_admin) + for g, is_admin in result.all() + ] @router.patch("/me/color-mode", response_model=UserOut) diff --git a/backend/app/schemas/group.py b/backend/app/schemas/group.py index bbb06d5..20ecb79 100644 --- a/backend/app/schemas/group.py +++ b/backend/app/schemas/group.py @@ -18,11 +18,16 @@ class GroupMemberOut(BaseModel): email: str full_name: str | None is_active: bool + is_group_admin: bool = False joined_at: datetime model_config = {"from_attributes": True} +class GroupMemberAdminUpdate(BaseModel): + is_group_admin: bool + + class GroupOut(BaseModel): id: str name: str diff --git a/backend/app/schemas/user.py b/backend/app/schemas/user.py index adddec7..9031f1f 100644 --- a/backend/app/schemas/user.py +++ b/backend/app/schemas/user.py @@ -122,6 +122,7 @@ class UserGroupOut(BaseModel): id: str name: str description: str | None + is_group_admin: bool = False model_config = {"from_attributes": True} diff --git a/changelog/2026-04-18_category-scopes-group-admin.md b/changelog/2026-04-18_category-scopes-group-admin.md new file mode 100644 index 0000000..dab76f0 --- /dev/null +++ b/changelog/2026-04-18_category-scopes-group-admin.md @@ -0,0 +1,34 @@ +# 2026-04-18 — Category scopes, group admin role, and permission model + +**Timestamp:** 2026-04-18T00:00:00Z + +## Summary + +Introduces three category scopes (personal / group / system), a PascalCase-with-dashes naming convention, a group-admin role on group memberships, and a full permission model for who can create, rename, and delete categories and documents. + +## Files Added + +- `backend/alembic/versions/e1f2a3b4c5d6_add_group_member_is_admin.py` — adds `is_group_admin BOOLEAN` to `group_memberships` +- `features/doc-service/alembic/versions/0005_add_share_can_delete.py` — adds `can_delete BOOLEAN` to `document_shares` (backfill from feat/document-delete-permissions) +- `features/doc-service/alembic/versions/0006_add_category_scope.py` — adds `scope VARCHAR(16)` and `group_id VARCHAR` to `document_categories`; data-migrates watch categories to scope='system' + +## Files Modified + +- `backend/app/models/group.py` — added `is_group_admin` to `GroupMembership` +- `backend/app/schemas/group.py` — added `is_group_admin` to `GroupMemberOut`; new `GroupMemberAdminUpdate` +- `backend/app/schemas/user.py` — added `is_group_admin` to `UserGroupOut` +- `backend/app/routers/users.py` — `get_my_groups` now joins `GroupMembership` to include `is_group_admin` +- `backend/app/routers/groups.py` — `get_group` includes `is_group_admin`; new `PATCH /{id}/members/{user_id}/admin` endpoint +- `backend/app/routers/categories_proxy.py` — injects `x-user-is-admin` and `x-user-admin-groups` headers +- `backend/app/routers/documents_proxy.py` — injects `x-user-admin-groups` header (was already injecting `x-user-is-admin`) +- `features/doc-service/app/models/category.py` — added `scope`, `group_id` columns +- `features/doc-service/app/schemas/category.py` — `CategoryOut` includes `scope`/`group_id`; `CategoryCreate` accepts `group_id` +- `features/doc-service/app/deps.py` — added `get_user_is_admin`, `get_user_admin_groups` +- `features/doc-service/app/routers/categories.py` — full rewrite: name validation regex, scope-based list/create, `_check_can_manage_cat` permission helper, scope-aware rename/delete +- `features/doc-service/app/routers/documents.py` — `delete_document` enforces is_admin/can_delete/group-admin hierarchy; `remove_category` requires doc ownership; `assign_category` accepts group/system categories +- `frontend/src/api/client.ts` — `CategoryOut` gains `scope`/`group_id`; `createCategory` accepts optional `groupId`; `UserGroupOut`/`GroupMemberOut` gain `is_group_admin`; new `adminSetGroupMemberAdmin()`; `ApiError` exported +- `frontend/src/components/ManageCategoriesDialog.tsx` — categories grouped by scope; lock icons for unmanageable categories; rename/delete gated by scope permissions; inline rename error display +- `frontend/src/components/SourcePanel.tsx` — categories shown in sections (Mine / Group name / System); scope picker on new category form; client-side name validation +- `frontend/src/pages/AdminGroupsPage.tsx` — group admin checkbox column in members table +- `backend/CLAUDE.md` — updated `group_memberships` model, migration chain, endpoints +- `features/doc-service/CLAUDE.md` — updated `document_categories` model, `document_shares` model, migration chain, deps note diff --git a/features/doc-service/CLAUDE.md b/features/doc-service/CLAUDE.md index 3068298..03bd28d 100644 --- a/features/doc-service/CLAUDE.md +++ b/features/doc-service/CLAUDE.md @@ -23,7 +23,7 @@ features/doc-service/ ├── app/ │ ├── main.py ← FastAPI, lifespan (file watcher start/stop) │ ├── database.py ← Same PostgreSQL instance as backend -│ ├── deps.py ← get_user_id (x-user-id), get_user_groups (x-user-groups) +│ ├── deps.py ← get_user_id, get_user_groups, get_user_is_admin, get_user_admin_groups (injected headers) │ ├── models/ │ │ ├── document.py ← Document model │ │ ├── category.py ← DocumentCategory model @@ -78,12 +78,14 @@ features/doc-service/ ### `document_categories` -| Column | Type | Constraints | -|--------|------|-------------| -| `id` | String | PK, UUID | -| `user_id` | String | indexed | -| `name` | String(128) | NOT NULL | -| `created_at` | DateTime(tz) | server_default=now() | +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `id` | String | PK, UUID | | +| `user_id` | String | indexed | owner; "watch" for system categories | +| `name` | String(128) | NOT NULL | PascalCase-with-dashes convention enforced on create/rename | +| `scope` | String(16) | NOT NULL, default="personal" | "personal" / "group" / "system" | +| `group_id` | String | nullable, indexed | set when scope="group" | +| `created_at` | DateTime(tz) | server_default=now() | | ### `document_category_assignments` (composite PK) @@ -100,6 +102,7 @@ features/doc-service/ | `document_id` | String | indexed, NOT NULL | not FK — trusts proxy | | `group_id` | String | indexed, NOT NULL | group from backend | | `shared_by_user_id` | String | NOT NULL | owner who shared | +| `can_delete` | Boolean | NOT NULL, default=false | allows group members to delete the doc | | `created_at` | DateTime(tz) | server_default=now() | | Unique constraint: `(document_id, group_id)` @@ -112,6 +115,8 @@ Unique constraint: `(document_id, group_id)` | `0002` | `add_document_title` | | `0003` | `add_watch_columns` | | `0004` | `add_document_shares` | +| `0005` | `add_share_can_delete` | +| `0006` | `add_category_scope` | --- diff --git a/features/doc-service/alembic/versions/0005_add_share_can_delete.py b/features/doc-service/alembic/versions/0005_add_share_can_delete.py new file mode 100644 index 0000000..b7bbd0e --- /dev/null +++ b/features/doc-service/alembic/versions/0005_add_share_can_delete.py @@ -0,0 +1,32 @@ +"""add can_delete to document_shares + +Revision ID: 0005 +Revises: 0004 +Create Date: 2026-04-18 + +""" +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +revision: str = "0005" +down_revision: Union[str, None] = "0004" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.add_column( + "document_shares", + sa.Column( + "can_delete", + sa.Boolean(), + nullable=False, + server_default="false", + ), + ) + + +def downgrade() -> None: + op.drop_column("document_shares", "can_delete") diff --git a/features/doc-service/alembic/versions/0006_add_category_scope.py b/features/doc-service/alembic/versions/0006_add_category_scope.py new file mode 100644 index 0000000..9dcd8e3 --- /dev/null +++ b/features/doc-service/alembic/versions/0006_add_category_scope.py @@ -0,0 +1,42 @@ +"""add scope and group_id to document_categories + +Revision ID: 0006 +Revises: 0005 +Create Date: 2026-04-18 + +""" +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +revision: str = "0006" +down_revision: Union[str, None] = "0005" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.add_column( + "document_categories", + sa.Column( + "scope", + sa.String(16), + nullable=False, + server_default="personal", + ), + ) + op.add_column( + "document_categories", + sa.Column("group_id", sa.String(), nullable=True), + ) + op.create_index("ix_document_categories_group_id", "document_categories", ["group_id"]) + + # Migrate existing watch-owned categories to system scope + op.execute("UPDATE document_categories SET scope = 'system' WHERE user_id = 'watch'") + + +def downgrade() -> None: + op.drop_index("ix_document_categories_group_id", table_name="document_categories") + op.drop_column("document_categories", "group_id") + op.drop_column("document_categories", "scope") diff --git a/features/doc-service/app/deps.py b/features/doc-service/app/deps.py index 21adc31..558dd03 100644 --- a/features/doc-service/app/deps.py +++ b/features/doc-service/app/deps.py @@ -21,3 +21,22 @@ async def get_user_groups(x_user_groups: str = Header(default="")) -> list[str]: if not x_user_groups: return [] return [g.strip() for g in x_user_groups.split(",") if g.strip()] + + +async def get_user_is_admin(x_user_is_admin: str = Header(default="false")) -> bool: + """ + Extract the superuser flag injected by the main backend proxy. + Returns True only when the header value is exactly "true". + """ + return x_user_is_admin.lower() == "true" + + +async def get_user_admin_groups(x_user_admin_groups: str = Header(default="")) -> list[str]: + """ + Extract the group IDs for which the current user is a group admin. + Injected by the main backend proxy from GroupMembership.is_group_admin. + Returns an empty list if absent or empty. + """ + if not x_user_admin_groups: + return [] + return [g.strip() for g in x_user_admin_groups.split(",") if g.strip()] diff --git a/features/doc-service/app/models/category.py b/features/doc-service/app/models/category.py index e1b7fd4..935d136 100644 --- a/features/doc-service/app/models/category.py +++ b/features/doc-service/app/models/category.py @@ -13,6 +13,8 @@ class DocumentCategory(Base): id: Mapped[str] = mapped_column(String, primary_key=True, default=lambda: str(uuid.uuid4())) user_id: Mapped[str] = mapped_column(String, nullable=False, index=True) name: Mapped[str] = mapped_column(String(128), nullable=False) + scope: Mapped[str] = mapped_column(String(16), nullable=False, default="personal", server_default="personal") + group_id: Mapped[str | None] = mapped_column(String, nullable=True, index=True) created_at: Mapped[datetime] = mapped_column( DateTime(timezone=True), server_default=func.now(), nullable=False ) diff --git a/features/doc-service/app/routers/categories.py b/features/doc-service/app/routers/categories.py index 1a60508..79ebebd 100644 --- a/features/doc-service/app/routers/categories.py +++ b/features/doc-service/app/routers/categories.py @@ -1,5 +1,6 @@ import difflib import json +import re from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException from sqlalchemy import select @@ -8,7 +9,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy import or_ from app.database import AsyncSessionLocal, get_db -from app.deps import get_user_id +from app.deps import get_user_admin_groups, get_user_groups, get_user_id, get_user_is_admin from app.models.category import DocumentCategory from app.models.category_assignment import CategoryAssignment from app.models.document import Document @@ -22,14 +23,31 @@ _WATCH_USER_ID = "watch" _SIMILARITY_THRESHOLD = 0.4 +# PascalCase-with-dashes: each word starts with a capital, words joined by '-' +# Valid: Invoices, Vendor-Invoices, Q1-Reports +# Invalid: invoices, Invoice Reports, Invoice-reports +_NAME_RE = re.compile(r'^[A-Z][a-zA-Z0-9]*(-[A-Z][a-zA-Z0-9]*)*$') + + +def _validate_name(name: str) -> None: + if not _NAME_RE.match(name): + raise HTTPException( + status_code=422, + detail=( + "Category name must start with a capital letter. " + "Multiple words are joined with dashes, each starting with a capital " + "(e.g. Invoices, Vendor-Invoices, Q1-Reports)." + ), + ) + def _name_similarity(a: str, b: str) -> float: """Return similarity score (0–1) between two category names.""" a_low = a.lower() b_low = b.lower() # Word overlap is a strong signal - a_words = set(a_low.split()) - b_words = set(b_low.split()) + a_words = set(a_low.split("-")) + b_words = set(b_low.split("-")) if a_words & b_words: return 0.9 # Fallback: character sequence ratio @@ -81,15 +99,44 @@ async def _reanalyze_documents_for_new_category( pass +async def _check_can_manage_cat( + cat: DocumentCategory, + user_id: str, + is_admin: bool, + user_admin_groups: list[str], +) -> None: + """Raise 403/404 if the current user may not rename or delete this category.""" + if is_admin: + return # superuser can manage anything + if cat.scope == "system": + raise HTTPException(status_code=403, detail="Only admins can modify system categories") + if cat.scope == "personal" and cat.user_id != user_id: + raise HTTPException(status_code=404, detail="Category not found") + if cat.scope == "group" and cat.group_id not in user_admin_groups: + raise HTTPException(status_code=403, detail="Only group admins can modify group categories") + + @router.get("", response_model=list[CategoryOut]) async def list_categories( user_id: str = Depends(get_user_id), + user_groups: list[str] = Depends(get_user_groups), db: AsyncSession = Depends(get_db), ) -> list[DocumentCategory]: - # Include watch-ingested categories so they appear in the sidebar/filter + """ + Return all categories visible to the current user: + - personal (owned by the user) + - group (any group the user belongs to) + - system (watch-ingested, scope='system') + """ result = await db.execute( select(DocumentCategory) - .where(or_(DocumentCategory.user_id == user_id, DocumentCategory.user_id == _WATCH_USER_ID)) + .where( + or_( + DocumentCategory.user_id == user_id, # personal + DocumentCategory.group_id.in_(user_groups), # group + DocumentCategory.scope == "system", # system + ) + ) .order_by(DocumentCategory.name) ) return result.scalars().all() @@ -100,18 +147,32 @@ async def create_category( body: CategoryCreate, background_tasks: BackgroundTasks, user_id: str = Depends(get_user_id), + user_groups: list[str] = Depends(get_user_groups), db: AsyncSession = Depends(get_db), ) -> DocumentCategory: name = body.name.strip() if not name: raise HTTPException(status_code=422, detail="Category name cannot be empty") + _validate_name(name) + + if body.group_id: + # User must be a member of the target group + if body.group_id not in user_groups: + raise HTTPException(status_code=403, detail="You are not a member of that group") + cat = DocumentCategory( + user_id=user_id, + name=name[:128], + scope="group", + group_id=body.group_id, + ) + else: + cat = DocumentCategory(user_id=user_id, name=name[:128], scope="personal") - cat = DocumentCategory(user_id=user_id, name=name[:128]) db.add(cat) await db.commit() await db.refresh(cat) - # Find existing categories with similar names + # Find existing personal categories with similar names for background AI reanalysis result = await db.execute( select(DocumentCategory) .where(DocumentCategory.user_id == user_id) @@ -140,12 +201,18 @@ async def rename_category( cat_id: str, body: CategoryUpdate, user_id: str = Depends(get_user_id), + is_admin: bool = Depends(get_user_is_admin), + user_admin_groups: list[str] = Depends(get_user_admin_groups), db: AsyncSession = Depends(get_db), ) -> DocumentCategory: - cat = await _get_user_cat(cat_id, user_id, db) + cat = await _fetch_visible_cat(cat_id, user_id, db) + await _check_can_manage_cat(cat, user_id, is_admin, user_admin_groups) + name = body.name.strip() if not name: raise HTTPException(status_code=422, detail="Category name cannot be empty") + _validate_name(name) + cat.name = name[:128] await db.commit() await db.refresh(cat) @@ -156,19 +223,20 @@ async def rename_category( async def delete_category( cat_id: str, user_id: str = Depends(get_user_id), + is_admin: bool = Depends(get_user_is_admin), + user_admin_groups: list[str] = Depends(get_user_admin_groups), db: AsyncSession = Depends(get_db), ) -> None: - cat = await _get_user_cat(cat_id, user_id, db) + cat = await _fetch_visible_cat(cat_id, user_id, db) + await _check_can_manage_cat(cat, user_id, is_admin, user_admin_groups) await db.delete(cat) await db.commit() -async def _get_user_cat(cat_id: str, user_id: str, db: AsyncSession) -> DocumentCategory: +async def _fetch_visible_cat(cat_id: str, user_id: str, db: AsyncSession) -> DocumentCategory: + """Fetch a category that the user can see (personal/group/system). 404 if absent.""" result = await db.execute( - select(DocumentCategory).where( - DocumentCategory.id == cat_id, - DocumentCategory.user_id == user_id, - ) + select(DocumentCategory).where(DocumentCategory.id == cat_id) ) cat = result.scalar_one_or_none() if cat is None: diff --git a/features/doc-service/app/routers/documents.py b/features/doc-service/app/routers/documents.py index b68a736..066e2d1 100644 --- a/features/doc-service/app/routers/documents.py +++ b/features/doc-service/app/routers/documents.py @@ -13,7 +13,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import selectinload from app.database import AsyncSessionLocal, get_db -from app.deps import get_user_groups, get_user_id +from app.deps import get_user_admin_groups, get_user_groups, get_user_id, get_user_is_admin from app.models.category import DocumentCategory from app.models.category_assignment import CategoryAssignment from app.models.document import Document @@ -479,14 +479,59 @@ async def reprocess_document( async def delete_document( doc_id: str, user_id: str = Depends(get_user_id), + is_admin: bool = Depends(get_user_is_admin), + user_groups: list[str] = Depends(get_user_groups), + user_admin_groups: list[str] = Depends(get_user_admin_groups), db: AsyncSession = Depends(get_db), ) -> None: + # Fetch the document (owner, watch, or shared with user's groups) result = await db.execute( - select(Document).where(Document.id == doc_id, Document.user_id == user_id) + select(Document).where( + Document.id == doc_id, + or_( + Document.user_id == user_id, + Document.user_id == _WATCH_USER_ID, + Document.id.in_( + select(DocumentShare.document_id).where( + DocumentShare.group_id.in_(user_groups) + ) + ) if user_groups else False, + ), + ) ) doc = result.scalar_one_or_none() if doc is None: raise HTTPException(status_code=404, detail="Document not found") + + is_owner = doc.user_id == user_id + + if not is_owner and not is_admin: + # Check: shared with a group where the user has can_delete=True + can_delete_via_share = False + if user_groups: + share_result = await db.execute( + select(DocumentShare).where( + DocumentShare.document_id == doc_id, + DocumentShare.group_id.in_(user_groups), + DocumentShare.can_delete.is_(True), + ) + ) + can_delete_via_share = share_result.scalar_one_or_none() is not None + + # Check: user is a group admin for a group the doc is shared with + can_delete_as_group_admin = False + if user_admin_groups: + admin_share_result = await db.execute( + select(DocumentShare).where( + DocumentShare.document_id == doc_id, + DocumentShare.group_id.in_(user_admin_groups), + ) + ) + can_delete_as_group_admin = admin_share_result.scalar_one_or_none() is not None + + if not can_delete_via_share and not can_delete_as_group_admin: + raise HTTPException(status_code=403, detail="Not allowed to delete this document") + delete_file(doc.file_path) await db.delete(doc) await db.commit() @@ -537,6 +582,7 @@ async def assign_category( doc_id: str, cat_id: str, user_id: str = Depends(get_user_id), + user_groups: list[str] = Depends(get_user_groups), db: AsyncSession = Depends(get_db), ) -> None: doc_result = await db.execute( @@ -548,9 +594,15 @@ async def assign_category( if doc_result.scalar_one_or_none() is None: raise HTTPException(status_code=404, detail="Document not found") + # Accept personal, group (user is a member), or system categories cat_result = await db.execute( select(DocumentCategory).where( - DocumentCategory.id == cat_id, DocumentCategory.user_id == user_id + DocumentCategory.id == cat_id, + or_( + DocumentCategory.user_id == user_id, + DocumentCategory.group_id.in_(user_groups) if user_groups else False, + DocumentCategory.scope == "system", + ), ) ) if cat_result.scalar_one_or_none() is None: @@ -574,6 +626,13 @@ async def remove_category( user_id: str = Depends(get_user_id), db: AsyncSession = Depends(get_db), ) -> None: + # Only the document owner may remove a category assignment + doc_result = await db.execute( + select(Document).where(Document.id == doc_id, Document.user_id == user_id) + ) + if doc_result.scalar_one_or_none() is None: + raise HTTPException(status_code=403, detail="Only the document owner can remove category assignments") + result = await db.execute( select(CategoryAssignment).where( CategoryAssignment.document_id == doc_id, diff --git a/features/doc-service/app/schemas/category.py b/features/doc-service/app/schemas/category.py index 9d498e5..cad9232 100644 --- a/features/doc-service/app/schemas/category.py +++ b/features/doc-service/app/schemas/category.py @@ -7,6 +7,8 @@ class CategoryOut(BaseModel): id: str user_id: str name: str + scope: str = "personal" + group_id: str | None = None created_at: datetime model_config = {"from_attributes": True} @@ -14,6 +16,7 @@ class CategoryOut(BaseModel): class CategoryCreate(BaseModel): name: str + group_id: str | None = None class CategoryUpdate(BaseModel): diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 1cf1144..90389a6 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -4,7 +4,7 @@ const BASE = "/api"; // Core fetch wrapper // --------------------------------------------------------------------------- -class ApiError extends Error { +export class ApiError extends Error { status: number; detail: string; @@ -199,6 +199,8 @@ export type DocumentStatus = "pending" | "processing" | "done" | "failed"; export interface CategoryOut { id: string; name: string; + scope: "personal" | "group" | "system"; + group_id: string | null; } export interface DocumentOut { @@ -332,8 +334,8 @@ export const removeCategory = (docId: string, catId: string) => export const listCategories = () => api.get("/documents/categories"); -export const createCategory = (name: string) => - api.post("/documents/categories", { name }); +export const createCategory = (name: string, groupId?: string) => + api.post("/documents/categories", { name, group_id: groupId ?? null }); export const renameCategory = (id: string, name: string) => api.patch(`/documents/categories/${id}`, { name }); @@ -431,6 +433,7 @@ export interface UserGroupOut { id: string; name: string; description: string | null; + is_group_admin: boolean; } export const getMyGroups = () => api.get("/users/me/groups"); @@ -453,6 +456,7 @@ export interface GroupMemberOut { full_name: string | null; is_active: boolean; joined_at: string; + is_group_admin: boolean; } export interface GroupDetailOut extends GroupOut { @@ -489,6 +493,9 @@ export const adminAddGroupMember = (groupId: string, userId: string) => export const adminRemoveGroupMember = (groupId: string, userId: string) => api.delete(`/admin/groups/${groupId}/members/${userId}`); +export const adminSetGroupMemberAdmin = (groupId: string, userId: string, isAdmin: boolean) => + api.patch(`/admin/groups/${groupId}/members/${userId}/admin`, { is_group_admin: isAdmin }); + // --------------------------------------------------------------------------- // Services // --------------------------------------------------------------------------- diff --git a/frontend/src/components/ManageCategoriesDialog.tsx b/frontend/src/components/ManageCategoriesDialog.tsx index c832ed8..216e5da 100644 --- a/frontend/src/components/ManageCategoriesDialog.tsx +++ b/frontend/src/components/ManageCategoriesDialog.tsx @@ -1,7 +1,14 @@ import { useState, useRef, useEffect } from "react"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; -import { X, Pencil, Trash2, Check } from "lucide-react"; -import { listCategories, renameCategory, deleteCategory } from "@/api/client"; +import { X, Pencil, Trash2, Check, Lock } from "lucide-react"; +import { + listCategories, + renameCategory, + deleteCategory, + getMyGroups, + type CategoryOut, + ApiError, +} from "@/api/client"; import { Input } from "@/components/ui/input"; import { Button } from "@/components/ui/button"; @@ -11,13 +18,23 @@ interface Props { export default function ManageCategoriesDialog({ onClose }: Props) { const queryClient = useQueryClient(); + const { data: categories = [] } = useQuery({ queryKey: ["categories"], queryFn: listCategories, }); + const { data: myGroups = [] } = useQuery({ + queryKey: ["my-groups"], + queryFn: getMyGroups, + }); + + // Set of group IDs for which the current user is a group admin + const adminGroupIds = new Set(myGroups.filter((g) => g.is_group_admin).map((g) => g.id)); + const [editingId, setEditingId] = useState(null); const [editValue, setEditValue] = useState(""); + const [editError, setEditError] = useState(null); const [search, setSearch] = useState(""); const editInputRef = useRef(null); @@ -30,6 +47,10 @@ export default function ManageCategoriesDialog({ onClose }: Props) { onSuccess: () => { queryClient.invalidateQueries({ queryKey: ["categories"] }); setEditingId(null); + setEditError(null); + }, + onError: (err) => { + setEditError(err instanceof ApiError ? err.message : "Rename failed"); }, }); @@ -38,24 +59,135 @@ export default function ManageCategoriesDialog({ onClose }: Props) { onSuccess: () => queryClient.invalidateQueries({ queryKey: ["categories"] }), }); + function canManage(cat: CategoryOut): boolean { + if (cat.scope === "personal") return true; + if (cat.scope === "group") return cat.group_id != null && adminGroupIds.has(cat.group_id); + return false; // system — managed only by superuser via other means + } + const filtered = search ? categories.filter((c) => c.name.toLowerCase().includes(search.toLowerCase())) : categories; + // Group by scope + const personal = filtered.filter((c) => c.scope === "personal"); + const system = filtered.filter((c) => c.scope === "system"); + + // Group-scoped categories grouped by group_id + const groupCats = filtered.filter((c) => c.scope === "group"); + const groupMap = new Map(); + for (const cat of groupCats) { + if (!cat.group_id) continue; + if (!groupMap.has(cat.group_id)) { + const grp = myGroups.find((g) => g.id === cat.group_id); + groupMap.set(cat.group_id, { name: grp?.name ?? cat.group_id, cats: [] }); + } + groupMap.get(cat.group_id)!.cats.push(cat); + } + function startEdit(id: string, name: string) { setEditingId(id); setEditValue(name); + setEditError(null); } function submitEdit(id: string) { const name = editValue.trim(); - if (name && name !== categories.find((c) => c.id === id)?.name) { - renameMut.mutate({ id, name }); - } else { + if (!name) return; + if (name === categories.find((c) => c.id === id)?.name) { setEditingId(null); + return; } + renameMut.mutate({ id, name }); } + function renderCat(cat: CategoryOut) { + const manageable = canManage(cat); + return ( +
+ {editingId === cat.id ? ( +
+
{ e.preventDefault(); submitEdit(cat.id); }} + > + { setEditValue(e.target.value); setEditError(null); }} + className="h-7 text-sm flex-1" + disabled={renameMut.isPending} + onKeyDown={(e) => { if (e.key === "Escape") { setEditingId(null); setEditError(null); } }} + /> + + +
+ {editError && ( +

{editError}

+ )} +
+ ) : ( + <> + {!manageable && ( + + )} + {cat.name} + {manageable && ( +
+ + +
+ )} + + )} +
+ ); + } + + function renderSection(title: string, cats: CategoryOut[]) { + if (cats.length === 0) return null; + return ( +
+

{title}

+ {cats.map(renderCat)} +
+ ); + } + + const hasAny = filtered.length > 0; + return (
- {filtered.length === 0 && ( +
+ {!hasAny && (

{search ? "No categories match" : "No categories yet"}

)} - {filtered.map((cat) => ( -
- {editingId === cat.id ? ( -
{ e.preventDefault(); submitEdit(cat.id); }} - > - setEditValue(e.target.value)} - className="h-7 text-sm flex-1" - disabled={renameMut.isPending} - onKeyDown={(e) => { if (e.key === "Escape") setEditingId(null); }} - /> - - -
- ) : ( - <> - {cat.name} -
- - -
- - )} -
- ))} + {renderSection("My Categories", personal)} + {Array.from(groupMap.entries()).map(([, { name, cats }]) => + renderSection(`Group: ${name}`, cats) + )} + {renderSection("System", system)}
{/* Footer */} diff --git a/frontend/src/components/SourcePanel.tsx b/frontend/src/components/SourcePanel.tsx index d0fa7e7..d93a538 100644 --- a/frontend/src/components/SourcePanel.tsx +++ b/frontend/src/components/SourcePanel.tsx @@ -2,11 +2,20 @@ import { useState, useRef, useEffect } from "react"; import { useSearchParams } from "react-router-dom"; import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; import { Files, User, Users, Folder, Plus, Settings2, Check, X } from "lucide-react"; -import { listCategories, createCategory, CategoryOut } from "@/api/client"; +import { + listCategories, + createCategory, + getMyGroups, + type CategoryOut, + ApiError, +} from "@/api/client"; import { Input } from "@/components/ui/input"; import { cn } from "@/lib/utils"; import ManageCategoriesDialog from "@/components/ManageCategoriesDialog"; +// PascalCase-with-dashes naming convention +const NAME_RE = /^[A-Z][a-zA-Z0-9]*(-[A-Z][a-zA-Z0-9]*)*$/; + export default function SourcePanel() { const [searchParams, setSearchParams] = useSearchParams(); const queryClient = useQueryClient(); @@ -16,6 +25,8 @@ export default function SourcePanel() { const [catSearch, setCatSearch] = useState(""); const [addingCat, setAddingCat] = useState(false); const [newCatName, setNewCatName] = useState(""); + const [newCatGroupId, setNewCatGroupId] = useState(""); + const [nameError, setNameError] = useState(null); const [manageOpen, setManageOpen] = useState(false); const addInputRef = useRef(null); @@ -24,12 +35,23 @@ export default function SourcePanel() { queryFn: listCategories, }); + const { data: myGroups = [] } = useQuery({ + queryKey: ["my-groups"], + queryFn: getMyGroups, + }); + const createMut = useMutation({ - mutationFn: createCategory, + mutationFn: ({ name, groupId }: { name: string; groupId?: string }) => + createCategory(name, groupId), onSuccess: () => { queryClient.invalidateQueries({ queryKey: ["categories"] }); setNewCatName(""); + setNewCatGroupId(""); setAddingCat(false); + setNameError(null); + }, + onError: (err) => { + setNameError(err instanceof ApiError ? err.message : "Failed to create category"); }, }); @@ -41,6 +63,19 @@ export default function SourcePanel() { ? categories.filter((c) => c.name.toLowerCase().includes(catSearch.toLowerCase())) : categories; + // Split by scope for display + const personalCats = filteredCats.filter((c) => c.scope === "personal"); + const systemCats = filteredCats.filter((c) => c.scope === "system"); + const groupCatMap = new Map(); + for (const cat of filteredCats.filter((c) => c.scope === "group")) { + if (!cat.group_id) continue; + if (!groupCatMap.has(cat.group_id)) { + const grp = myGroups.find((g) => g.id === cat.group_id); + groupCatMap.set(cat.group_id, { name: grp?.name ?? cat.group_id, cats: [] }); + } + groupCatMap.get(cat.group_id)!.cats.push(cat); + } + function setView(view: string) { setSearchParams((prev) => { const next = new URLSearchParams(prev); @@ -61,6 +96,20 @@ export default function SourcePanel() { }); } + function handleAddSubmit(e: React.FormEvent) { + e.preventDefault(); + const name = newCatName.trim(); + if (!name) return; + if (!NAME_RE.test(name)) { + setNameError( + "Must start with a capital letter. Join multiple words with dashes, each capitalised (e.g. Vendor-Invoices)." + ); + return; + } + setNameError(null); + createMut.mutate({ name, groupId: newCatGroupId || undefined }); + } + const viewItemClass = (active: boolean) => cn( "flex items-center gap-2 w-full px-2 py-1.5 rounded-md text-sm transition-colors", @@ -77,6 +126,32 @@ export default function SourcePanel() { : "text-muted hover:bg-muted/20 hover:text-foreground" ); + function renderCatSection(label: string, cats: CategoryOut[]) { + if (cats.length === 0) return null; + return ( +
+

+ {label} +

+ {cats.map((cat) => ( + + ))} +
+ ); + } + + const hasSections = + personalCats.length > 0 || + systemCats.length > 0 || + groupCatMap.size > 0; + return ( <>