fix: admin delete bypass + update merge checklist for new features
- Fix doc-service delete endpoint: admins could not delete non-owned, non-shared documents — they hit 404 because the initial query filtered by owner/watch/group even before the is_admin bypass was checked. Admins now get an unconditional fetch, consistent with intent. - Add 18 new checklist tests covering: group admin role (4.9–4.10), delete permission variants (12.16b–12.16e), can_delete sharing (13.11–13.14), category scopes / PascalCase naming (14.7–14.17), and three-dots portal fix (19.11). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
curo1305
@@ -0,0 +1,12 @@
|
||||
# 2026-04-19 — Merge checklist update + admin delete bug fix
|
||||
|
||||
**Timestamp:** 2026-04-19T00:15:00Z
|
||||
|
||||
## Summary
|
||||
|
||||
Updated `tests/MERGE_CHECKLIST.md` with all new tests for the two recently merged features (document delete permissions and category scopes / group-admin role). While running the new test 12.16b, discovered and fixed a bug where the doc-service delete endpoint returned 404 for admins deleting non-owned documents.
|
||||
|
||||
## Files Added / Modified / Deleted
|
||||
|
||||
- **Modified** `tests/MERGE_CHECKLIST.md` — added 18 new tests: 4.9–4.10 (group admin role), 12.16b–12.16e (delete permissions), 13.11–13.14 (can_delete sharing), 14.7–14.17 (category scopes, PascalCase naming), 19.11 (three-dots portal fix); updated 12.16 and 14.5 descriptions
|
||||
- **Modified** `features/doc-service/app/routers/documents.py` — fixed `delete_document` to bypass group-membership filter for admins; previously admins got 404 on any document they didn't own or that wasn't a watch doc
|
||||
@@ -526,21 +526,25 @@ async def delete_document(
|
||||
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,
|
||||
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,
|
||||
),
|
||||
if is_admin:
|
||||
# Admins can delete any document — fetch unconditionally.
|
||||
result = await db.execute(select(Document).where(Document.id == doc_id))
|
||||
else:
|
||||
# Fetch the document (owner, watch, or shared with user's groups)
|
||||
result = await db.execute(
|
||||
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")
|
||||
|
||||
@@ -83,6 +83,8 @@ Mark each row before opening the PR.
|
||||
| 4.6 | Remove member | Click remove on a member | User removed from group |
|
||||
| 4.7 | Duplicate group name | Create group with name that already exists | 400 / validation error shown |
|
||||
| 4.8 | Non-admin access | Regular user calls `GET /api/admin/groups` | 404 |
|
||||
| 4.9 | Set group admin role | Admin → group detail → tick "Group admin" checkbox on a member → save | `PATCH /api/admin/groups/{id}/members/{user_id}/admin` with `{"is_group_admin": true}` returns 200; badge shown in member list |
|
||||
| 4.10 | Unset group admin role | Admin unticks "Group admin" on an existing group admin member | Returns 200; badge removed; user loses group-admin privileges |
|
||||
|
||||
---
|
||||
|
||||
@@ -209,8 +211,12 @@ Mark each row before opening the PR.
|
||||
| 12.13 | Raw text section | Expand raw text collapse | First ~500k chars of extracted PDF text shown |
|
||||
| 12.14 | Download | Click "Download" | Browser downloads the original PDF file |
|
||||
| 12.15 | View in new tab | Click "View" | PDF opens in new browser tab; URL auto-revokes after 60s |
|
||||
| 12.16 | Delete | Click "Delete" → confirm dialog | Document and file removed; table row gone |
|
||||
| 12.17 | Non-owner cannot edit | Recipient of shared doc opens slide-over | Edit controls (type, tags, title, delete) absent; download available |
|
||||
| 12.16 | Delete — owner | Click "Delete" → confirm dialog | Document and file removed; table row gone |
|
||||
| 12.16b | Delete — admin | Admin opens any doc (not their own) → Delete → confirm | Document deleted; 204 returned |
|
||||
| 12.16c | Delete — can_delete share | Group member whose share has `can_delete=true` → Delete | 204; document removed; `viewer_can_delete` was `true` in `DocumentOut` |
|
||||
| 12.16d | Delete — group admin | User is group admin for a group the doc is shared with; no explicit `can_delete` flag → Delete | 204; group admin always has delete rights for docs shared with their group |
|
||||
| 12.16e | Delete — watch document, admin only | Watch-ingested doc (`source=watch`); regular user → Delete | 403 (not owner); admin can delete it |
|
||||
| 12.17 | Non-owner cannot edit | Recipient of shared doc opens slide-over (no can_delete, not group admin) | Edit controls (type, tags, title, delete) absent; download available |
|
||||
|
||||
---
|
||||
|
||||
@@ -229,6 +235,10 @@ Mark each row before opening the PR.
|
||||
| 13.8 | Bulk share | Select multiple rows → bulk share → pick group | All selected docs shared with that group |
|
||||
| 13.9 | Share count indicator | Document shared with 2 groups | Users icon in table row shows "2" |
|
||||
| 13.10 | Share with non-member group | `POST /api/documents/{id}/shares` with group not in X-User-Groups | 403 / validation error |
|
||||
| 13.11 | Share with can_delete enabled | Owner opens sharing section → tick "Allow group members to delete" → share | `can_delete=true` stored; trash icon badge appears next to group name in shares list |
|
||||
| 13.12 | Share without can_delete (default) | Owner shares without ticking delete checkbox | `can_delete=false`; recipient sees the doc but Delete button absent in slide-over |
|
||||
| 13.13 | can_delete shown in shares list | Share with can_delete=true → inspect shares list in slide-over | Trash2 icon rendered beside the group name; tooltip "Group members can delete this document" |
|
||||
| 13.14 | viewer_can_delete in document list | Share with can_delete=true; log in as group member → `GET /api/documents` | `viewer_can_delete=true` in the recipient's list response for that doc |
|
||||
|
||||
---
|
||||
|
||||
@@ -240,8 +250,19 @@ Mark each row before opening the PR.
|
||||
| 14.2 | Rename category | Manage categories dialog → edit → save | New name reflected everywhere |
|
||||
| 14.3 | Delete category | Delete category with documents assigned | 204; documents remain; category assignment removed |
|
||||
| 14.4 | Category search | More than 4 categories → type in search field | Tree filtered in real time |
|
||||
| 14.5 | Manage categories dialog | Click "Manage categories" | Modal shows all categories with rename/delete actions |
|
||||
| 14.5 | Manage categories dialog | Click "Manage categories" | Modal shows categories grouped by scope (Personal / Group / System) with lock icons on group and system categories |
|
||||
| 14.6 | New category triggers re-analysis | Create category with name similar to AI suggestion | Background re-analysis triggered (check backend logs) |
|
||||
| 14.7 | Create personal category | SourcePanel → "New category" → no group selected → submit valid PascalCase name | Created with `scope=personal`; visible only to owner |
|
||||
| 14.8 | Create group-scoped category | SourcePanel → "New category" → select a group → submit | Created with `scope=group`; visible to all members of that group |
|
||||
| 14.9 | Group category visible to group members | Log in as another group member | Group category appears in their category list and SourcePanel |
|
||||
| 14.10 | Non-member cannot see group category | Log in as user not in the group | Group category absent from list |
|
||||
| 14.11 | Only group admin can rename group category | Regular group member → rename group category | 403; group admin can rename it successfully |
|
||||
| 14.12 | Only group admin can delete group category | Regular group member → delete group category | 403; group admin can delete it |
|
||||
| 14.13 | System categories read-only for non-admin | Regular user → Manage categories → rename/delete a system category | 403; lock icon shown; action blocked in UI |
|
||||
| 14.14 | Admin can manage system categories | Superuser → rename or delete a system category | Succeeds; ManageCategoriesDialog shows edit/delete controls for system rows |
|
||||
| 14.15 | PascalCase naming enforced — invalid | Create category named `my-invoices` or `Invoice Reports` | 422 with message explaining PascalCase-with-dashes format |
|
||||
| 14.16 | PascalCase naming enforced — valid | Create category named `Vendor-Invoices` | 201; category created successfully |
|
||||
| 14.17 | SourcePanel scope sections | Categories exist for all three scopes | SourcePanel tree shows "Mine", per-group, and "System" sections separately |
|
||||
|
||||
---
|
||||
|
||||
@@ -323,3 +344,4 @@ Mark each row before opening the PR.
|
||||
| 19.8 | Unknown route | Navigate to `/does-not-exist` | Redirected to `/` |
|
||||
| 19.9 | TanStack Query cache | Navigate away from docs → back | List loads from cache instantly; background refetch runs |
|
||||
| 19.10 | 30s service poll | Leave `/apps` open for 30s | `GET /api/services` fires again in network tab |
|
||||
| 19.11 | Three-dots menu not clipped | Scroll document table → open three-dot actions on any row | Dropdown renders above the table's overflow-hidden container; not cut off |
|
||||
|
||||
Reference in New Issue
Block a user