Merge: resolve conflicts between feat/document-delete-permissions and feat/category-scopes-group-admin

- Keep HEAD's get_user_admin_groups dep and richer delete permission logic (can_delete via share OR group admin path)
- Use sa.text("false") for migration server_default (correct SQLAlchemy form)
- Preserve 0006/0007 migration entries in doc-service CLAUDE.md

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

backend/CLAUDE.md

@@ -218,7 +218,7 @@ Auth: is_superuser OR member of group listed in manifest `required_groups`. Retu
 
 ### Documents and Categories — proxied
 
-`/api/documents/*` and `/api/documents/categories/*` are transparently proxied to `doc-service:8001`. The backend injects `x-user-id` and `x-user-groups` headers. See `features/doc-service/CLAUDE.md` for the internal endpoint list.
+`/api/documents/*` and `/api/documents/categories/*` are transparently proxied to `doc-service:8001`. The backend injects `x-user-id`, `x-user-groups`, and `x-user-is-admin` headers. See `features/doc-service/CLAUDE.md` for the internal endpoint list.
 
 ---
 

changelog/2026-04-18_document-delete-permissions.md

@@ -0,0 +1,25 @@
+# 2026-04-18 — Document delete permissions + three-dots menu fix
+
+**Timestamp:** 2026-04-18T00:00:00Z
+
+## Summary
+
+Added a proper permission model for document deletion: owners and admins can always delete; group members can delete only when the share was explicitly granted `can_delete=true`. Fixed silent delete failures (watch docs returning 404 with no user feedback) and fixed the three-dots context menu being clipped by `overflow-hidden` on the table container.
+
+## Files Added / Modified / Deleted
+
+### Added
+- `features/doc-service/alembic/versions/0005_add_share_can_delete.py` — migration: adds `can_delete BOOLEAN NOT NULL DEFAULT false` to `document_shares`
+
+### Modified
+- `features/doc-service/app/models/document_share.py` — added `can_delete: Mapped[bool]` column
+- `features/doc-service/app/schemas/share.py` — added `can_delete` to `DocumentShareOut` and `DocumentShareCreate`; added `viewer_can_delete` to `SharedDocumentOut`
+- `features/doc-service/app/schemas/document.py` — added `viewer_can_delete: bool = False` to `DocumentOut`
+- `features/doc-service/app/deps.py` — added `get_user_is_admin()` dep reading `x-user-is-admin` header
+- `features/doc-service/app/routers/documents.py` — added `_get_deletable_doc_ids()` helper; updated list/get/delete endpoints with permission logic; updated `add_share` to store `can_delete`; updated shared-with-me to include `viewer_can_delete`
+- `backend/app/routers/documents_proxy.py` — `_forward_headers()` now injects `x-user-is-admin` header
+- `frontend/src/api/client.ts` — `DocumentOut`: added `viewer_can_delete`; `DocumentShareOut`: added `can_delete`; `addDocumentShare`: accepts `canDelete` param
+- `frontend/src/pages/DocumentsPage.tsx` — `RowActionsMenu`: replaced absolute dropdown with `createPortal` to fix clipping; delete button now uses `doc.viewer_can_delete`; added `onError` handler for silent failures
+- `frontend/src/components/DocumentSlideOver.tsx` — sharing section: shows trash icon badge on shares with `can_delete=true`; added "Allow group members to delete" checkbox before group picker; delete button uses `doc.viewer_can_delete`
+- `features/doc-service/CLAUDE.md` — updated `document_shares` table docs + migration chain
+- `backend/CLAUDE.md` — noted `x-user-is-admin` header injection

features/doc-service/alembic/versions/0005_add_share_can_delete.py

@@ -23,7 +23,7 @@ def upgrade() -> None:
             "can_delete",
             sa.Boolean(),
             nullable=False,
-            server_default="false",
+            server_default=sa.text("false"),
         ),
     )
 

features/doc-service/app/models/document_share.py

@@ -1,7 +1,7 @@
 import uuid
 from datetime import datetime
 
-from sqlalchemy import DateTime, String, UniqueConstraint, func
+from sqlalchemy import Boolean, DateTime, String, UniqueConstraint, func
 from sqlalchemy.orm import Mapped, mapped_column
 
 from app.database import Base
@@ -14,6 +14,7 @@ class DocumentShare(Base):
     document_id: Mapped[str] = mapped_column(String, nullable=False, index=True)
     group_id: Mapped[str] = mapped_column(String, nullable=False, index=True)
     shared_by_user_id: Mapped[str] = mapped_column(String, nullable=False)
+    can_delete: Mapped[bool] = mapped_column(Boolean, nullable=False, default=False, server_default="false")
     created_at: Mapped[datetime] = mapped_column(
         DateTime(timezone=True), server_default=func.now(), nullable=False
     )

features/doc-service/app/routers/documents.py

@@ -73,7 +73,26 @@ async def _get_share_counts(doc_ids: list[str], db: AsyncSession) -> dict[str, i
     return {row[0]: row[1] for row in rows.all()}
 
 
-def _doc_with_categories(doc: Document, share_count: int = 0) -> DocumentOut:
+async def _get_deletable_doc_ids(
+    doc_ids: list[str], user_groups: list[str], db: AsyncSession
+) -> set[str]:
+    """Return doc IDs for which the user has delete permission via a group share."""
+    if not doc_ids or not user_groups:
+        return set()
+    rows = await db.execute(
+        select(DocumentShare.document_id)
+        .where(
+            DocumentShare.document_id.in_(doc_ids),
+            DocumentShare.group_id.in_(user_groups),
+            DocumentShare.can_delete.is_(True),
+        )
+    )
+    return {row[0] for row in rows.all()}
+
+
+def _doc_with_categories(
+    doc: Document, share_count: int = 0, viewer_can_delete: bool = False
+) -> DocumentOut:
     from app.schemas.document import CategoryOut
     cats = [CategoryOut(id=a.category.id, name=a.category.name) for a in doc.category_assignments]
     return DocumentOut(
@@ -95,6 +114,7 @@ def _doc_with_categories(doc: Document, share_count: int = 0) -> DocumentOut:
         suggested_folder=doc.suggested_folder,
         suggested_filename=doc.suggested_filename,
         share_count=share_count,
+        viewer_can_delete=viewer_can_delete,
     )
 
 
@@ -209,6 +229,8 @@ async def list_documents(
     search: str | None = Query(default=None),
     category_id: str | None = Query(default=None),
     user_id: str = Depends(get_user_id),
+    user_groups: list[str] = Depends(get_user_groups),
+    is_admin: bool = Depends(get_user_is_admin),
     db: AsyncSession = Depends(get_db),
 ) -> DocumentPage:
     sort_col = _SORT_COLUMNS.get(sort, Document.created_at)
@@ -254,8 +276,19 @@ async def list_documents(
     )
     docs = items_result.scalars().all()
 
-    share_counts = await _get_share_counts([d.id for d in docs], db)
-    items = [_doc_with_categories(d, share_counts.get(d.id, 0)) for d in docs]
+    doc_ids = [d.id for d in docs]
+    share_counts = await _get_share_counts(doc_ids, db)
+
+    if is_admin:
+        deletable_ids = set(doc_ids)
+    else:
+        deletable_ids = {d.id for d in docs if d.user_id == user_id}
+        deletable_ids |= await _get_deletable_doc_ids(doc_ids, user_groups, db)
+
+    items = [
+        _doc_with_categories(d, share_counts.get(d.id, 0), viewer_can_delete=d.id in deletable_ids)
+        for d in docs
+    ]
 
     return DocumentPage(
         items=items,
@@ -367,6 +400,7 @@ async def list_shared_with_me(
                 source=doc.source,
                 shared_by_user_id=share.shared_by_user_id if share else "",
                 shared_via_group_id=share.group_id if share else "",
+                viewer_can_delete=bool(share and share.can_delete),
             )
         )
 
@@ -382,11 +416,19 @@ async def list_shared_with_me(
 async def get_document(
     doc_id: str,
     user_id: str = Depends(get_user_id),
+    user_groups: list[str] = Depends(get_user_groups),
+    is_admin: bool = Depends(get_user_is_admin),
     db: AsyncSession = Depends(get_db),
 ) -> DocumentOut:
     doc = await _get_user_doc(doc_id, user_id, db)
     counts = await _get_share_counts([doc.id], db)
-    return _doc_with_categories(doc, counts.get(doc.id, 0))
+    if is_admin:
+        viewer_can_delete = True
+    elif doc.user_id == user_id:
+        viewer_can_delete = True
+    else:
+        viewer_can_delete = bool(await _get_deletable_doc_ids([doc.id], user_groups, db))
+    return _doc_with_categories(doc, counts.get(doc.id, 0), viewer_can_delete=viewer_can_delete)
 
 
 @router.get("/{doc_id}/status", response_model=DocumentStatusOut)
@@ -777,6 +819,7 @@ async def add_document_share(
         document_id=doc_id,
         group_id=body.group_id,
         shared_by_user_id=user_id,
+        can_delete=body.can_delete,
     )
     db.add(share)
     await db.commit()

features/doc-service/app/schemas/document.py

@@ -28,6 +28,7 @@ class DocumentOut(BaseModel):
     suggested_folder: str | None = None
     suggested_filename: str | None = None
     share_count: int = 0
+    viewer_can_delete: bool = False
 
     model_config = {"from_attributes": True}
 

features/doc-service/app/schemas/share.py

@@ -8,6 +8,7 @@ class DocumentShareOut(BaseModel):
     document_id: str
     group_id: str
     shared_by_user_id: str
+    can_delete: bool
     created_at: datetime
 
     model_config = {"from_attributes": True}
@@ -15,6 +16,7 @@ class DocumentShareOut(BaseModel):
 
 class DocumentShareCreate(BaseModel):
     group_id: str
+    can_delete: bool = False
 
 
 class SharedDocumentOut(BaseModel):
@@ -34,6 +36,7 @@ class SharedDocumentOut(BaseModel):
     categories: list = []
     source: str = "upload"
     share_count: int = 0
+    viewer_can_delete: bool = False
     # Sharing context
     shared_by_user_id: str
     shared_via_group_id: str

frontend/src/api/client.ts

@@ -222,6 +222,7 @@ export interface DocumentOut {
   suggested_folder: string | null;
   suggested_filename: string | null;
   share_count: number;
+  viewer_can_delete: boolean;
 }
 
 export interface SharedDocumentOut extends DocumentOut {
@@ -234,6 +235,7 @@ export interface DocumentShareOut {
   document_id: string;
   group_id: string;
   shared_by_user_id: string;
+  can_delete: boolean;
   created_at: string;
 }
 
@@ -272,8 +274,8 @@ export const listSharedWithMe = (params: DocumentListParams = {}) =>
 export const getDocumentShares = (docId: string) =>
   api.get<DocumentShareOut[]>(`/documents/${docId}/shares`);
 
-export const addDocumentShare = (docId: string, groupId: string) =>
-  api.post<DocumentShareOut>(`/documents/${docId}/shares`, { group_id: groupId });
+export const addDocumentShare = (docId: string, groupId: string, canDelete = false) =>
+  api.post<DocumentShareOut>(`/documents/${docId}/shares`, { group_id: groupId, can_delete: canDelete });
 
 export const removeDocumentShare = (docId: string, groupId: string) =>
   api.delete(`/documents/${docId}/shares/${groupId}`);

frontend/src/components/DocumentSlideOver.tsx

@@ -186,6 +186,7 @@ export default function DocumentSlideOver({ doc, isOwner, onClose, onDeleted }:
   const [titleValue, setTitleValue] = useState("");
   const [editingType, setEditingType] = useState(false);
   const [tagInput, setTagInput] = useState("");
+  const [canDeleteNew, setCanDeleteNew] = useState(false);
   const titleInputRef = useRef<HTMLInputElement>(null);
 
   useEffect(() => {
@@ -279,8 +280,12 @@ export default function DocumentSlideOver({ doc, isOwner, onClose, onDeleted }:
   });
 
   const addShareMut = useMutation({
-    mutationFn: (groupId: string) => addDocumentShare(doc!.id, groupId),
-    onSuccess: () => queryClient.invalidateQueries({ queryKey: ["document-shares", doc!.id] }),
+    mutationFn: ({ groupId, canDelete }: { groupId: string; canDelete: boolean }) =>
+      addDocumentShare(doc!.id, groupId, canDelete),
+    onSuccess: () => {
+      queryClient.invalidateQueries({ queryKey: ["document-shares", doc!.id] });
+      setCanDeleteNew(false);
+    },
   });
 
   const removeShareMut = useMutation({
@@ -694,6 +699,11 @@ export default function DocumentSlideOver({ doc, isOwner, onClose, onDeleted }:
                     <div key={share.id} className="flex items-center gap-2 text-sm">
                       <Users className="h-3.5 w-3.5 text-muted shrink-0" />
                       <span className="flex-1 text-sm">{group?.name ?? share.group_id}</span>
+                      {share.can_delete && (
+                        <span title="Group members can delete this document">
+                          <Trash2 className="h-3 w-3 text-muted shrink-0" />
+                        </span>
+                      )}
                       <button
                         onClick={() => removeShareMut.mutate(share.group_id)}
                         disabled={removeShareMut.isPending}
@@ -706,41 +716,54 @@ export default function DocumentSlideOver({ doc, isOwner, onClose, onDeleted }:
                   );
                 })}
               </div>
+              <label className="flex items-center gap-1.5 text-xs text-muted cursor-pointer mt-1 mb-0.5 select-none">
+                <input
+                  type="checkbox"
+                  checked={canDeleteNew}
+                  onChange={(e) => setCanDeleteNew(e.target.checked)}
+                  className="h-3 w-3 accent-primary"
+                />
+                Allow group members to delete
+              </label>
               <GroupCombobox
                 groups={myGroups}
                 sharedGroupIds={sharedGroupIds}
-                onShare={(id) => addShareMut.mutate(id)}
+                onShare={(id) => addShareMut.mutate({ groupId: id, canDelete: canDeleteNew })}
               />
             </div>
           )}
 
-          {/* Owner actions */}
-          {isOwner && (
+          {/* Owner/permitted actions */}
+          {(isOwner || doc.viewer_can_delete) && (
             <div className="flex gap-2 pt-1">
-              <Button
-                variant="outline"
-                size="sm"
-                onClick={() => reprocessMut.mutate()}
-                disabled={reprocessMut.isPending || doc.status === "pending" || doc.status === "processing"}
-                className="gap-1.5"
-              >
-                <RefreshCw className={cn("h-3.5 w-3.5", reprocessMut.isPending && "animate-spin")} />
-                Re-analyse
-              </Button>
-              <Button
-                variant="outline"
-                size="sm"
-                onClick={() => {
-                  if (confirm(`Delete "${doc.title ?? doc.filename}"? This cannot be undone.`)) {
-                    deleteMut.mutate();
-                  }
-                }}
-                disabled={deleteMut.isPending}
-                className="gap-1.5 text-red-500 border-red-200 hover:bg-red-50 dark:hover:bg-red-900/20"
-              >
-                <Trash2 className="h-3.5 w-3.5" />
-                Delete
-              </Button>
+              {isOwner && (
+                <Button
+                  variant="outline"
+                  size="sm"
+                  onClick={() => reprocessMut.mutate()}
+                  disabled={reprocessMut.isPending || doc.status === "pending" || doc.status === "processing"}
+                  className="gap-1.5"
+                >
+                  <RefreshCw className={cn("h-3.5 w-3.5", reprocessMut.isPending && "animate-spin")} />
+                  Re-analyse
+                </Button>
+              )}
+              {doc.viewer_can_delete && (
+                <Button
+                  variant="outline"
+                  size="sm"
+                  onClick={() => {
+                    if (confirm(`Delete "${doc.title ?? doc.filename}"? This cannot be undone.`)) {
+                      deleteMut.mutate();
+                    }
+                  }}
+                  disabled={deleteMut.isPending}
+                  className="gap-1.5 text-red-500 border-red-200 hover:bg-red-50 dark:hover:bg-red-900/20"
+                >
+                  <Trash2 className="h-3.5 w-3.5" />
+                  Delete
+                </Button>
+              )}
             </div>
           )}
 

frontend/src/pages/DocumentsPage.tsx

@@ -1,4 +1,5 @@
 import { useState, useRef, useCallback, useEffect } from "react";
+import { createPortal } from "react-dom";
 import { useSearchParams } from "react-router-dom";
 import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query";
 import {
@@ -47,43 +48,70 @@ function formatDate(iso: string) {
 // ── Row actions dropdown ──────────────────────────────────────────────────────
 
 function RowActionsMenu({
-  doc, isOwner, onSelect,
-}: { doc: DocumentOut; isOwner: boolean; onSelect: () => void }) {
+  doc, onSelect,
+}: { doc: DocumentOut; onSelect: () => void }) {
   const [open, setOpen] = useState(false);
-  const ref = useRef<HTMLDivElement>(null);
+  const [menuPos, setMenuPos] = useState({ top: 0, right: 0 });
+  const triggerRef = useRef<HTMLButtonElement>(null);
+  const menuRef = useRef<HTMLDivElement>(null);
   const queryClient = useQueryClient();
 
   useEffect(() => {
-    const handler = (e: MouseEvent) => {
-      if (ref.current && !ref.current.contains(e.target as Node)) setOpen(false);
+    const close = (e: MouseEvent) => {
+      if (
+        triggerRef.current && !triggerRef.current.contains(e.target as Node) &&
+        menuRef.current && !menuRef.current.contains(e.target as Node)
+      ) setOpen(false);
+    };
+    const closeOnScroll = () => setOpen(false);
+    document.addEventListener("mousedown", close);
+    window.addEventListener("scroll", closeOnScroll, true);
+    window.addEventListener("resize", closeOnScroll);
+    return () => {
+      document.removeEventListener("mousedown", close);
+      window.removeEventListener("scroll", closeOnScroll, true);
+      window.removeEventListener("resize", closeOnScroll);
     };
-    document.addEventListener("mousedown", handler);
-    return () => document.removeEventListener("mousedown", handler);
   }, []);
 
   const deleteMut = useMutation({
     mutationFn: () => deleteDocument(doc.id),
     onSuccess: () => queryClient.invalidateQueries({ queryKey: ["documents"] }),
+    onError: () => alert("Failed to delete document. Please try again."),
   });
 
+  const handleToggle = (e: React.MouseEvent) => {
+    e.stopPropagation();
+    if (!open && triggerRef.current) {
+      const rect = triggerRef.current.getBoundingClientRect();
+      setMenuPos({ top: rect.bottom + 4, right: window.innerWidth - rect.right });
+    }
+    setOpen((o) => !o);
+  };
+
   return (
-    <div className="relative" ref={ref} onClick={(e) => e.stopPropagation()}>
+    <div onClick={(e) => e.stopPropagation()}>
       <button
-        onClick={() => setOpen((o) => !o)}
+        ref={triggerRef}
+        onClick={handleToggle}
         className="p-1 rounded text-muted hover:text-foreground hover:bg-muted/20 transition-colors opacity-0 group-hover:opacity-100"
         title="Actions"
       >
         <MoreHorizontal className="h-4 w-4" />
       </button>
-      {open && (
-        <div className="absolute right-0 top-full mt-1 z-20 bg-surface border border-border rounded-lg shadow-lg w-36 py-1">
+      {open && createPortal(
+        <div
+          ref={menuRef}
+          style={{ position: "fixed", top: menuPos.top, right: menuPos.right, zIndex: 9999 }}
+          className="bg-surface border border-border rounded-lg shadow-lg w-36 py-1"
+        >
           <button
             className="w-full text-left px-3 py-1.5 text-sm hover:bg-muted/20 transition-colors"
             onClick={() => { onSelect(); setOpen(false); }}
           >
             Open details
           </button>
-          {isOwner && (
+          {doc.viewer_can_delete && (
             <button
               className="w-full text-left px-3 py-1.5 text-sm hover:bg-muted/20 transition-colors text-red-500"
               onClick={() => {
@@ -94,7 +122,8 @@ function RowActionsMenu({
               Delete
             </button>
           )}
-        </div>
+        </div>,
+        document.body
       )}
     </div>
   );
@@ -470,7 +499,7 @@ function DocumentRow({
       </td>
 
       <td className="px-3 py-2.5 w-8 text-right">
-        <RowActionsMenu doc={doc} isOwner={isOwner} onSelect={onOpen} />
+        <RowActionsMenu doc={doc} onSelect={onOpen} />
       </td>
     </tr>
   );