diff --git a/README.md b/README.md index dec37ef..c7071ff 100644 --- a/README.md +++ b/README.md @@ -321,6 +321,7 @@ punimtag/ - ✅ Alembic migrations configured and applied - ✅ Database initialized (SQLite default, PostgreSQL supported) - ✅ RQ worker auto-start (starts automatically with API server) +- ✅ Pending linkage moderation API for user tag suggestions **Frontend:** - ✅ React + Vite + TypeScript setup @@ -329,6 +330,7 @@ punimtag/ - ✅ Protected routes with auth context - ✅ Navigation layout (left sidebar + top bar) - ✅ All page routes (Dashboard, Scan, Process, Search, Identify, Auto-Match, Tags, Settings) +- ✅ User Tagged Photos moderation tab for approving/denying pending tag linkages **Database:** - ✅ All tables created automatically on startup: `photos`, `faces`, `people`, `person_encodings`, `tags`, `phototaglinkage` diff --git a/TAG_TO_IDENTIFY_ANALYSIS.md b/TAG_TO_IDENTIFY_ANALYSIS.md new file mode 100644 index 0000000..d6dd486 --- /dev/null +++ b/TAG_TO_IDENTIFY_ANALYSIS.md @@ -0,0 +1,380 @@ +# Analysis: Extract Faces from Tag UI and Navigate to Identify Page + +## User Request +In Tag UI, when selecting a photo, extract faces from it (if processed) and jump to Identify page with only those faces as reference faces (for left panel), possibly in a new tab. + +## Current State Analysis + +### Tag UI (`frontend/src/pages/Tags.tsx`) +- **Photo Selection**: Photos can be selected via checkboxes (lines 585-600) +- **Photo Data Available**: + - `photo.id` - Photo ID + - `photo.face_count` - Number of faces detected (line 651) + - `photo.processed` - Whether photo has been processed (line 641) +- **Current Actions**: + - Tag management (add/remove tags) + - Bulk tagging operations + - No navigation to Identify page currently + +### Identify Page (`frontend/src/pages/Identify.tsx`) +- **Face Loading**: Uses `facesApi.getUnidentified()` (line 86) +- **API Endpoint**: `/api/v1/faces/unidentified` +- **Current Filters Supported**: + - `page`, `page_size` + - `min_quality` + - `date_taken_from`, `date_taken_to` + - `sort_by`, `sort_dir` + - `tag_names`, `match_all` + - **❌ NO `photo_id` filter currently supported** + +### Backend API (`src/web/api/faces.py`) +- **Endpoint**: `GET /api/v1/faces/unidentified` (lines 104-171) +- **Service Function**: `list_unidentified_faces()` in `face_service.py` (lines 1194-1300) +- **Current Filters**: Quality, dates, tags +- **❌ NO `photo_id` parameter in service function** + +### Routing (`frontend/src/App.tsx`) +- Uses React Router v6 +- Identify route: `/identify` (line 42) +- Can use `useNavigate()` hook for navigation +- Can pass state via `navigate('/identify', { state: {...} })` +- Can use URL search params: `/identify?photo_ids=1,2,3` +- Can open in new tab: `window.open('/identify?photo_ids=1,2,3', '_blank')` + +## What's Needed + +1. **Get faces for selected photo(s)** + - Need API endpoint or modify existing to filter by `photo_id` + - Only get faces if photo is processed (`photo.processed === true`) + - Only get unidentified faces (no `person_id`) + +2. **Navigate to Identify page** + - Pass face IDs or photo IDs to Identify page + - Load only those faces in the left panel (reference faces) + - Optionally open in new tab + +3. **Identify page modifications** + - Check for photo_ids or face_ids in URL params or state + - If provided, load only those faces instead of all unidentified faces + - Display them in the left panel as reference faces + +## Possible Approaches + +### Approach A: Add `photo_id` filter to existing `/unidentified` endpoint +**Pros:** +- Minimal changes to existing API +- Reuses existing filtering logic +- Consistent with other filters + +**Cons:** +- Only works for unidentified faces +- Need to support multiple photo_ids (array) + +**Implementation:** +1. Add `photo_ids: Optional[List[int]]` parameter to `list_unidentified_faces()` service +2. Add `photo_ids: Optional[str]` query param to API endpoint (comma-separated) +3. Filter query: `query.filter(Face.photo_id.in_(photo_ids))` +4. Frontend: Pass `photo_ids` in `getUnidentified()` call +5. Identify page: Check URL params for `photo_ids`, parse and pass to API + +### Approach B: Create new endpoint `/api/v1/faces/by-photo/{photo_id}` +**Pros:** +- Clean separation of concerns +- Can return all faces (identified + unidentified) if needed later +- More explicit purpose + +**Cons:** +- New endpoint to maintain +- Need to handle multiple photos (could use POST with array) + +**Implementation:** +1. Create `GET /api/v1/faces/by-photo/{photo_id}` endpoint +2. Or `POST /api/v1/faces/by-photos` with `{photo_ids: [1,2,3]}` +3. Return `UnidentifiedFacesResponse` format +4. Frontend: Call new endpoint from Tags page +5. Navigate with face IDs in state/URL params + +### Approach C: Use URL params to pass photo_ids, filter on frontend +**Pros:** +- No backend changes needed +- Quick to implement + +**Cons:** +- Need to load ALL unidentified faces first, then filter +- Inefficient for large databases +- Not scalable + +**Implementation:** +1. Tags page: Navigate to `/identify?photo_ids=1,2,3` +2. Identify page: Load all unidentified faces +3. Filter faces array: `faces.filter(f => photoIds.includes(f.photo_id))` +4. ❌ **Not recommended** - inefficient + +## Recommended Solution: Approach A (Extend Existing Endpoint) + +### Why Approach A? +- Minimal backend changes +- Efficient (database-level filtering) +- Consistent with existing API patterns +- Supports multiple photos easily + +### Implementation Plan + +#### 1. Backend Changes + +**File: `src/web/services/face_service.py`** +```python +def list_unidentified_faces( + db: Session, + page: int = 1, + page_size: int = 50, + min_quality: float = 0.0, + date_from: Optional[date] = None, + date_to: Optional[date] = None, + date_taken_from: Optional[date] = None, + date_taken_to: Optional[date] = None, + date_processed_from: Optional[date] = None, + date_processed_to: Optional[date] = None, + sort_by: str = "quality", + sort_dir: str = "desc", + tag_names: Optional[List[str]] = None, + match_all: bool = False, + photo_ids: Optional[List[int]] = None, # NEW PARAMETER +) -> Tuple[List[Face], int]: + # ... existing code ... + + # Photo ID filtering (NEW) + if photo_ids: + query = query.filter(Face.photo_id.in_(photo_ids)) + + # ... rest of existing code ... +``` + +**File: `src/web/api/faces.py`** +```python +@router.get("/unidentified", response_model=UnidentifiedFacesResponse) +def get_unidentified_faces( + # ... existing params ... + photo_ids: str | None = Query(None, description="Comma-separated photo IDs"), + db: Session = Depends(get_db), +) -> UnidentifiedFacesResponse: + # ... existing code ... + + # Parse photo_ids + photo_ids_list = None + if photo_ids: + try: + photo_ids_list = [int(pid.strip()) for pid in photo_ids.split(',') if pid.strip()] + except ValueError: + raise HTTPException(status_code=400, detail="Invalid photo_ids format") + + faces, total = list_unidentified_faces( + # ... existing params ... + photo_ids=photo_ids_list, # NEW PARAMETER + ) + # ... rest of existing code ... +``` + +**File: `frontend/src/api/faces.ts`** +```typescript +getUnidentified: async (params: { + // ... existing params ... + photo_ids?: string, // NEW: comma-separated photo IDs +}): Promise => { + // ... existing code ... +} +``` + +#### 2. Frontend Changes + +**File: `frontend/src/pages/Tags.tsx`** +Add button/action to selected photos: +```typescript +// Add state for "Identify Faces" action +const handleIdentifyFaces = (photoIds: number[]) => { + // Filter to only processed photos with faces + const processedPhotos = photos.filter(p => + photoIds.includes(p.id) && p.processed && p.face_count > 0 + ) + + if (processedPhotos.length === 0) { + alert('No processed photos with faces selected') + return + } + + // Navigate to Identify page with photo IDs + const photoIdsStr = processedPhotos.map(p => p.id).join(',') + + // Option 1: Same tab + navigate(`/identify?photo_ids=${photoIdsStr}`) + + // Option 2: New tab + // window.open(`/identify?photo_ids=${photoIdsStr}`, '_blank') +} +``` + +**File: `frontend/src/pages/Identify.tsx`** +Modify to check for `photo_ids` URL param: +```typescript +import { useSearchParams } from 'react-router-dom' + +export default function Identify() { + const [searchParams] = useSearchParams() + const photoIdsParam = searchParams.get('photo_ids') + + // Parse photo IDs from URL + const photoIds = useMemo(() => { + if (!photoIdsParam) return null + return photoIdsParam.split(',').map(id => parseInt(id.trim())).filter(id => !isNaN(id)) + }, [photoIdsParam]) + + const loadFaces = async (clearState: boolean = false) => { + setLoadingFaces(true) + + try { + const res = await facesApi.getUnidentified({ + page: 1, + page_size: pageSize, + min_quality: minQuality, + date_taken_from: dateFrom || undefined, + date_taken_to: dateTo || undefined, + sort_by: sortBy, + sort_dir: sortDir, + tag_names: selectedTags.length > 0 ? selectedTags.join(', ') : undefined, + match_all: false, + photo_ids: photoIds ? photoIds.join(',') : undefined, // NEW + }) + + // ... rest of existing code ... + } finally { + setLoadingFaces(false) + } + } + + // ... rest of component ... +} +``` + +#### 3. UI Enhancement in Tags Page + +Add a button/action when photos are selected: +```tsx +{selectedPhotoIds.size > 0 && ( + +)} +``` + +Or add a context menu/button on individual photos: +```tsx +{photo.processed && photo.face_count > 0 && ( + +)} +``` + +## Implementation Considerations + +### 1. **Photo Processing Status** +- Only show action for processed photos (`photo.processed === true`) +- Only show if `photo.face_count > 0` +- Show appropriate message if no processed photos selected + +### 2. **New Tab vs Same Tab** +- **Same Tab**: User loses Tag page context, but simpler navigation +- **New Tab**: Preserves Tag page, better UX for comparison +- **Recommendation**: Start with same tab, add option for new tab later + +### 3. **Multiple Photos** +- Support multiple photo selection +- Combine all faces from selected photos +- Show count: "X faces from Y photos" + +### 4. **Empty Results** +- If no faces found for selected photos, show message +- Could be because: + - Photos not processed yet + - All faces already identified + - No faces detected + +### 5. **URL Parameter Length** +- For many photos, URL could get long +- Consider using POST with state instead of URL params +- Or use sessionStorage to pass photo IDs + +### 6. **State Management** +- Identify page uses sessionStorage for state persistence +- Need to handle case where photo_ids override normal loading +- Clear photo_ids filter when user clicks "Refresh" or "Apply Filters" + +## Alternative: Using State Instead of URL Params + +If URL params become too long or we want to avoid exposing photo IDs: + +```typescript +// Tags page +navigate('/identify', { + state: { + photoIds: processedPhotos.map(p => p.id), + source: 'tags' + } +}) + +// Identify page +const location = useLocation() +const photoIds = location.state?.photoIds + +// But this doesn't work for new tabs - would need sessionStorage +``` + +## Testing Checklist + +- [ ] Select single processed photo with faces → Navigate to Identify +- [ ] Select multiple processed photos → Navigate to Identify +- [ ] Select unprocessed photo → Show appropriate message +- [ ] Select photo with no faces → Show appropriate message +- [ ] Select mix of processed/unprocessed → Only process processed ones +- [ ] Navigate with photo_ids → Only those faces shown +- [ ] Clear filters in Identify → Should clear photo_ids filter +- [ ] Refresh in Identify → Should maintain photo_ids filter (or clear?) +- [ ] Open in new tab → Works correctly +- [ ] URL with many photo_ids → Handles correctly + +## Summary + +**Feasibility**: ✅ **YES, this is possible** + +**Recommended Approach**: Extend existing `/unidentified` endpoint with `photo_ids` filter + +**Key Changes Needed**: +1. Backend: Add `photo_ids` parameter to service and API +2. Frontend: Add navigation from Tags to Identify with photo_ids +3. Frontend: Modify Identify page to handle photo_ids URL param +4. UI: Add button/action in Tags page for selected photos + +**Complexity**: Low-Medium +- Backend: Simple filter addition +- Frontend: URL param handling + navigation +- UI: Button/action addition + +**Estimated Effort**: 2-4 hours + + + + + diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 3862a95..81c1ab9 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -16,6 +16,7 @@ import ApproveIdentified from './pages/ApproveIdentified' import ManageUsers from './pages/ManageUsers' import ReportedPhotos from './pages/ReportedPhotos' import PendingPhotos from './pages/PendingPhotos' +import UserTaggedPhotos from './pages/UserTaggedPhotos' import ManagePhotos from './pages/ManagePhotos' import Settings from './pages/Settings' import Help from './pages/Help' @@ -101,6 +102,14 @@ function AppRoutes() { } /> + + + + } + /> { + const { data } = await apiClient.get('/api/v1/pending-linkages', { + params: statusFilter ? { status_filter: statusFilter } : undefined, + }) + return data + }, + + async reviewPendingLinkages(request: ReviewRequest): Promise { + const { data } = await apiClient.post('/api/v1/pending-linkages/review', request) + return data + }, + + async cleanupPendingLinkages(): Promise { + const { data } = await apiClient.post('/api/v1/pending-linkages/cleanup', {}) + return data + }, +} + +export default pendingLinkagesApi + + diff --git a/frontend/src/components/Layout.tsx b/frontend/src/components/Layout.tsx index 518ca39..c89e85f 100644 --- a/frontend/src/components/Layout.tsx +++ b/frontend/src/components/Layout.tsx @@ -41,6 +41,7 @@ export default function Layout() { { path: '/faces-maintenance', label: 'Faces', icon: '🔧', featureKey: 'faces_maintenance' }, { path: '/approve-identified', label: 'User Identified Faces', icon: '✅', featureKey: 'user_identified' }, { path: '/reported-photos', label: 'User Reported Photos', icon: '🚩', featureKey: 'user_reported' }, + { path: '/pending-linkages', label: 'User Tagged Photos', icon: '🔖', featureKey: 'user_tagged' }, { path: '/pending-photos', label: 'User Uploaded Photos', icon: '📤', featureKey: 'user_uploaded' }, { path: '/manage-users', label: 'Users', icon: '👥', featureKey: 'manage_users' }, ] diff --git a/frontend/src/pages/UserTaggedPhotos.tsx b/frontend/src/pages/UserTaggedPhotos.tsx new file mode 100644 index 0000000..2049a47 --- /dev/null +++ b/frontend/src/pages/UserTaggedPhotos.tsx @@ -0,0 +1,447 @@ +import { useCallback, useEffect, useMemo, useState } from 'react' +import { + pendingLinkagesApi, + PendingLinkageResponse, + ReviewDecision, +} from '../api/pendingLinkages' +import { apiClient } from '../api/client' +import { useAuth } from '../context/AuthContext' + +type DecisionValue = 'approve' | 'deny' + +function formatDate(value: string | null | undefined): string { + if (!value) { + return '-' + } + try { + return new Date(value).toLocaleString() + } catch (error) { + console.error('Failed to format date', error) + return value + } +} + +export default function UserTaggedPhotos() { + const { isAdmin } = useAuth() + const [linkages, setLinkages] = useState([]) + const [loading, setLoading] = useState(true) + const [error, setError] = useState(null) + const [statusFilter, setStatusFilter] = useState('pending') + const [decisions, setDecisions] = useState>({}) + const [submitting, setSubmitting] = useState(false) + const [clearing, setClearing] = useState(false) + + const loadLinkages = useCallback(async () => { + setLoading(true) + setError(null) + try { + const response = await pendingLinkagesApi.listPendingLinkages( + statusFilter || undefined + ) + setLinkages(response.items) + setDecisions({}) + } catch (err: any) { + setError(err.response?.data?.detail || err.message || 'Failed to load user tagged photos') + console.error('Error loading pending linkages:', err) + } finally { + setLoading(false) + } + }, [statusFilter]) + + useEffect(() => { + loadLinkages() + }, [loadLinkages]) + + const pendingCount = useMemo( + () => linkages.filter((item) => item.status === 'pending').length, + [linkages] + ) + + const hasPendingDecision = useMemo( + () => + Object.entries(decisions).some(([id, value]) => { + const linkage = linkages.find((item) => item.id === Number(id)) + return value !== null && linkage?.status === 'pending' + }), + [decisions, linkages] + ) + + const handleDecisionChange = (id: number, nextDecision: DecisionValue) => { + setDecisions((prev) => { + const current = prev[id] ?? null + const toggled = current === nextDecision ? null : nextDecision + return { + ...prev, + [id]: toggled, + } + }) + } + + const handleSubmit = async () => { + const decisionsList: ReviewDecision[] = Object.entries(decisions) + .filter(([id, decision]) => { + const linkage = linkages.find((item) => item.id === Number(id)) + return decision !== null && linkage?.status === 'pending' + }) + .map(([id, decision]) => ({ + id: Number(id), + decision: decision as DecisionValue, + })) + + if (decisionsList.length === 0) { + alert('Select Approve or Deny for at least one pending tag.') + return + } + + const approveCount = decisionsList.filter((item) => item.decision === 'approve').length + const denyCount = decisionsList.length - approveCount + + const confirmMessage = [ + `Submit ${decisionsList.length} decision(s)?`, + approveCount ? `✅ Approve: ${approveCount}` : null, + denyCount ? `❌ Deny: ${denyCount}` : null, + ] + .filter(Boolean) + .join('\n') + + if (!confirm(confirmMessage)) { + return + } + + setSubmitting(true) + try { + const response = await pendingLinkagesApi.reviewPendingLinkages({ + decisions: decisionsList, + }) + + const summary = [ + `Approved: ${response.approved}`, + `Denied: ${response.denied}`, + response.tags_created ? `New tags: ${response.tags_created}` : null, + response.linkages_created ? `New linkages: ${response.linkages_created}` : null, + response.errors.length ? `Errors: ${response.errors.join('; ')}` : null, + ] + .filter(Boolean) + .join('\n') + + alert(summary || 'Review complete.') + await loadLinkages() + setDecisions({}) + } catch (err: any) { + const message = err.response?.data?.detail || err.message || 'Failed to submit decisions' + alert(message) + console.error('Error submitting pending linkage decisions:', err) + } finally { + setSubmitting(false) + } + } + + const handleClearDatabase = async () => { + const confirmMessage = [ + 'Delete all approved and denied records?', + '', + 'Only records with Pending status will remain.', + 'This action cannot be undone.', + ].join('\n') + + if (!confirm(confirmMessage)) { + return + } + + setClearing(true) + try { + const response = await pendingLinkagesApi.cleanupPendingLinkages() + const summary = [ + `✅ Deleted ${response.deleted_records} record(s)`, + response.warnings && response.warnings.length > 0 + ? `ℹ️ ${response.warnings.join('; ')}` + : '', + response.errors.length > 0 ? `⚠️ ${response.errors.join('; ')}` : '', + ] + .filter(Boolean) + .join('\n') + + alert(summary || 'Cleanup complete.') + + if (response.errors.length > 0) { + console.error('Cleanup errors:', response.errors) + } + if (response.warnings && response.warnings.length > 0) { + console.info('Cleanup warnings:', response.warnings) + } + + await loadLinkages() + } catch (err: any) { + const errorMessage = + err.response?.data?.detail || err.message || 'Failed to cleanup pending linkages' + alert(`Error: ${errorMessage}`) + console.error('Error clearing pending linkages:', err) + } finally { + setClearing(false) + } + } + + return ( +
+
+
+

User Tagged Photos

+

+ Review tags suggested by users. Approving creates/links the tag to the selected photo. +

+
+ +
+ + + + +
+ Pending items: {pendingCount} +
+ +
+ + +
+ +
+ + + Clear approved/denied records + +
+
+ + {error && ( +
+ {error} +
+ )} + + {loading ? ( +
Loading...
+ ) : linkages.length === 0 ? ( +
+ No user tagged photos found for this filter. +
+ ) : ( +
+
+ + + + + + + + + + + + + + + {linkages.map((linkage) => { + const canReview = linkage.status === 'pending' + const decision = decisions[linkage.id] ?? null + return ( + + + + + + + + + + + ) + })} + +
+ Photo + + Proposed Tag + + Current Tags + + Submitted By + + Submitted At + + Notes + + Status + + Decision +
+ {linkage.photo_id ? ( +
{ + const url = `${apiClient.defaults.baseURL}/api/v1/photos/${linkage.photo_id}/image` + window.open(url, '_blank') + }} + title="Open photo in new tab" + > + {`Photo { + const target = event.target as HTMLImageElement + target.style.display = 'none' + const parent = target.parentElement + if (parent && !parent.querySelector('.fallback-text')) { + const fallback = document.createElement('div') + fallback.className = 'fallback-text text-gray-400 text-xs text-center' + fallback.textContent = `#${linkage.photo_id}` + parent.appendChild(fallback) + } + }} + /> +
+ ) : ( +
Photo not found
+ )} +
+ {linkage.photo_filename || `Photo #${linkage.photo_id}`} +
+ 		+
+ + {linkage.resolved_tag_name || linkage.proposed_tag_name || '-'} + + {linkage.tag_id === null && linkage.proposed_tag_name && ( + + New tag + + )} + {linkage.tag_id && ( + + Existing tag + #{linkage.tag_id} + + )} +
+ 		+ {linkage.photo_tags.length === 0 ? ( + No tags + ) : ( +
+ {linkage.photo_tags.map((tag) => ( + + {tag} + + ))} +
+ )} + 		+
{linkage.user_name || 'Unknown'}
+
{linkage.user_email || '-'}
+ 		+ {formatDate(linkage.created_at)} + + {linkage.notes ? ( +
+ {linkage.notes} +
+ ) : ( + - + )} + 		+ + {linkage.status} + + + {canReview ? ( +
+ + +
+ ) : ( + - + )} +
+
+
+ )} +
+ ) +} + + diff --git a/src/web/api/pending_linkages.py b/src/web/api/pending_linkages.py new file mode 100644 index 0000000..bb99492 --- /dev/null +++ b/src/web/api/pending_linkages.py @@ -0,0 +1,448 @@ +"""Pending linkage review endpoints - admin only.""" + +from __future__ import annotations + +from datetime import datetime +from typing import Annotated, Optional, Union + +from fastapi import APIRouter, Depends, HTTPException, Query, status +from pydantic import BaseModel, ConfigDict, Field +from sqlalchemy import text +from sqlalchemy.orm import Session + +from src.web.api.users import require_feature_permission +from src.web.db.models import Photo, PhotoTagLinkage, Tag +from src.web.db.session import get_auth_db, get_db + +router = APIRouter(prefix="/pending-linkages", tags=["pending-linkages"]) + + +def _get_or_create_tag_by_name(db: Session, tag_name: str) -> tuple[Tag, bool]: + """Return a tag for the provided name, creating it if necessary.""" + normalized = (tag_name or "").strip() + if not normalized: + raise ValueError("Tag name cannot be empty") + + existing = ( + db.query(Tag) + .filter(Tag.tag_name.ilike(normalized)) + .first() + ) + if existing: + return existing, False + + tag = Tag(tag_name=normalized) + db.add(tag) + db.flush() + return tag, True + + +def _format_datetime(value: Union[str, datetime, None]) -> Optional[str]: + """Safely serialize datetime values returned from different drivers.""" + if value is None: + return None + if isinstance(value, str): + return value + if isinstance(value, datetime): + return value.isoformat() + return str(value) + + +class PendingLinkageResponse(BaseModel): + """Pending linkage DTO returned from API.""" + + model_config = ConfigDict(from_attributes=True, protected_namespaces=()) + + id: int + photo_id: int + tag_id: Optional[int] = None + proposed_tag_name: Optional[str] = None + resolved_tag_name: Optional[str] = None + user_id: int + user_name: Optional[str] = None + user_email: Optional[str] = None + status: str + notes: Optional[str] = None + created_at: str + updated_at: Optional[str] = None + photo_filename: Optional[str] = None + photo_path: Optional[str] = None + photo_tags: list[str] = Field(default_factory=list) + + +class PendingLinkagesListResponse(BaseModel): + """List of pending linkage rows.""" + + model_config = ConfigDict(protected_namespaces=()) + + items: list[PendingLinkageResponse] + total: int + + +class ReviewDecision(BaseModel): + """Decision payload for a pending linkage row.""" + + model_config = ConfigDict(protected_namespaces=()) + + id: int + decision: str # 'approve' or 'deny' + + +class ReviewRequest(BaseModel): + """Request payload for reviewing pending linkages.""" + + model_config = ConfigDict(protected_namespaces=()) + + decisions: list[ReviewDecision] + + +class ReviewResponse(BaseModel): + """Review summary returned after processing decisions.""" + + model_config = ConfigDict(protected_namespaces=()) + + approved: int + denied: int + tags_created: int + linkages_created: int + errors: list[str] + + +@router.get("", response_model=PendingLinkagesListResponse) +def list_pending_linkages( + current_user: Annotated[ + dict, Depends(require_feature_permission("user_tagged")) + ], + status_filter: Annotated[ + Optional[str], + Query( + description="Optional status filter: pending, approved, or denied." + ), + ] = None, + auth_db: Session = Depends(get_auth_db), + main_db: Session = Depends(get_db), +) -> PendingLinkagesListResponse: + """List all pending linkages stored in the auth database.""" + valid_statuses = {"pending", "approved", "denied"} + if status_filter and status_filter not in valid_statuses: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Invalid status_filter. Use pending, approved, or denied.", + ) + + try: + params = {} + status_clause = "" + if status_filter: + status_clause = "WHERE pl.status = :status_filter" + params["status_filter"] = status_filter + + result = auth_db.execute( + text( + f""" + SELECT + pl.id, + pl.photo_id, + pl.tag_id, + pl.tag_name, + pl.user_id, + pl.status, + pl.notes, + pl.created_at, + pl.updated_at, + u.name AS user_name, + u.email AS user_email + FROM pending_linkages pl + LEFT JOIN users u ON pl.user_id = u.id + {status_clause} + ORDER BY pl.created_at DESC + """ + ), + params, + ) + rows = result.fetchall() + + photo_ids = {row.photo_id for row in rows if row.photo_id} + tag_ids = {row.tag_id for row in rows if row.tag_id} + + photo_map: dict[int, Photo] = {} + if photo_ids: + photos = ( + main_db.query(Photo) + .filter(Photo.id.in_(photo_ids)) + .all() + ) + photo_map = {photo.id: photo for photo in photos} + + tag_map: dict[int, str] = {} + if tag_ids: + tags = ( + main_db.query(Tag) + .filter(Tag.id.in_(tag_ids)) + .all() + ) + tag_map = {tag.id: tag.tag_name for tag in tags} + + photo_tags_map: dict[int, list[str]] = { + photo_id: [] for photo_id in photo_ids + } + if photo_ids: + tag_rows = ( + main_db.query(PhotoTagLinkage.photo_id, Tag.tag_name) + .join(Tag, Tag.id == PhotoTagLinkage.tag_id) + .filter(PhotoTagLinkage.photo_id.in_(photo_ids)) + .all() + ) + for photo_id, tag_name in tag_rows: + photo_tags_map.setdefault(photo_id, []).append(tag_name) + + items: list[PendingLinkageResponse] = [] + for row in rows: + created_at = _format_datetime(getattr(row, "created_at", None)) or "" + updated_at = _format_datetime(getattr(row, "updated_at", None)) + photo = photo_map.get(row.photo_id) + resolved_tag_name = None + if row.tag_id: + resolved_tag_name = tag_map.get(row.tag_id) + proposal_name = row.tag_name + items.append( + PendingLinkageResponse( + id=row.id, + photo_id=row.photo_id, + tag_id=row.tag_id, + proposed_tag_name=proposal_name, + resolved_tag_name=resolved_tag_name or proposal_name, + user_id=row.user_id, + user_name=row.user_name, + user_email=row.user_email, + status=row.status, + notes=row.notes, + created_at=created_at, + updated_at=updated_at, + photo_filename=photo.filename if photo else None, + photo_path=photo.path if photo else None, + photo_tags=photo_tags_map.get(row.photo_id, []), + ) + ) + + return PendingLinkagesListResponse(items=items, total=len(items)) + except HTTPException: + raise + except Exception as exc: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"Error reading pending linkages: {exc}", + ) + + +@router.post("/review", response_model=ReviewResponse) +def review_pending_linkages( + current_user: Annotated[ + dict, Depends(require_feature_permission("user_tagged")) + ], + request: ReviewRequest, + auth_db: Session = Depends(get_auth_db), + main_db: Session = Depends(get_db), +) -> ReviewResponse: + """Approve or deny pending user-proposed tag linkages.""" + approved = 0 + denied = 0 + tags_created = 0 + linkages_created = 0 + errors: list[str] = [] + now = datetime.utcnow() + + for decision in request.decisions: + try: + row = auth_db.execute( + text( + """ + SELECT id, photo_id, tag_id, tag_name, status + FROM pending_linkages + WHERE id = :id + """ + ), + {"id": decision.id}, + ).fetchone() + + if not row: + errors.append( + f"Pending linkage {decision.id} not found or already deleted" + ) + continue + + if row.status != "pending": + errors.append( + f"Pending linkage {decision.id} cannot be reviewed (status={row.status})" + ) + continue + + if decision.decision == "deny": + auth_db.execute( + text( + """ + UPDATE pending_linkages + SET status = 'denied', + updated_at = :updated_at + WHERE id = :id + """ + ), + {"id": decision.id, "updated_at": now}, + ) + auth_db.commit() + denied += 1 + continue + + if decision.decision != "approve": + errors.append( + f"Invalid decision '{decision.decision}' for linkage {decision.id}" + ) + continue + + photo = ( + main_db.query(Photo) + .filter(Photo.id == row.photo_id) + .first() + ) + if not photo: + errors.append( + f"Photo {row.photo_id} not found for linkage {decision.id}" + ) + continue + + tag_obj: Optional[Tag] = None + created_tag = False + + if row.tag_id: + tag_obj = ( + main_db.query(Tag) + .filter(Tag.id == row.tag_id) + .first() + ) + if not tag_obj and row.tag_name: + tag_obj, created_tag = _get_or_create_tag_by_name( + main_db, row.tag_name + ) + elif not tag_obj: + errors.append( + f"Tag {row.tag_id} missing for linkage {decision.id}" + ) + continue + else: + if not row.tag_name: + errors.append( + f"No tag information provided for linkage {decision.id}" + ) + continue + tag_obj, created_tag = _get_or_create_tag_by_name( + main_db, row.tag_name + ) + + if created_tag: + tags_created += 1 + + resolved_tag_id = tag_obj.id # type: ignore[union-attr] + + existing_linkage = ( + main_db.query(PhotoTagLinkage) + .filter( + PhotoTagLinkage.photo_id == row.photo_id, + PhotoTagLinkage.tag_id == resolved_tag_id, + ) + .first() + ) + + if not existing_linkage: + linkage = PhotoTagLinkage( + photo_id=row.photo_id, + tag_id=resolved_tag_id, + ) + main_db.add(linkage) + linkages_created += 1 + + main_db.commit() + + auth_db.execute( + text( + """ + UPDATE pending_linkages + SET status = 'approved', + tag_id = :tag_id, + updated_at = :updated_at + WHERE id = :id + """ + ), + { + "id": decision.id, + "tag_id": resolved_tag_id, + "updated_at": now, + }, + ) + auth_db.commit() + approved += 1 + except ValueError as exc: + main_db.rollback() + auth_db.rollback() + errors.append(f"Validation error for linkage {decision.id}: {exc}") + except Exception as exc: + main_db.rollback() + auth_db.rollback() + errors.append(f"Error processing linkage {decision.id}: {exc}") + + return ReviewResponse( + approved=approved, + denied=denied, + tags_created=tags_created, + linkages_created=linkages_created, + errors=errors, + ) + + +class CleanupResponse(BaseModel): + """Response payload for cleanup operations.""" + + model_config = ConfigDict(protected_namespaces=()) + + deleted_records: int + errors: list[str] + warnings: list[str] = [] + + +@router.post("/cleanup", response_model=CleanupResponse) +def cleanup_pending_linkages( + current_user: Annotated[ + dict, Depends(require_feature_permission("user_tagged")) + ], + auth_db: Session = Depends(get_auth_db), +) -> CleanupResponse: + """Delete all approved or denied records from pending_linkages table.""" + warnings: list[str] = [] + + try: + result = auth_db.execute( + text( + """ + DELETE FROM pending_linkages + WHERE status IN ('approved', 'denied') + """ + ) + ) + + deleted_records = result.rowcount if hasattr(result, "rowcount") else 0 + auth_db.commit() + + if deleted_records == 0: + warnings.append("No approved or denied pending linkages to delete.") + + return CleanupResponse( + deleted_records=deleted_records, + errors=[], + warnings=warnings, + ) + except Exception as exc: + auth_db.rollback() + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"Failed to cleanup pending linkages: {exc}", + ) + diff --git a/src/web/app.py b/src/web/app.py index e4d2f1a..28f7273 100644 --- a/src/web/app.py +++ b/src/web/app.py @@ -17,6 +17,7 @@ from src.web.api.jobs import router as jobs_router from src.web.api.metrics import router as metrics_router from src.web.api.people import router as people_router from src.web.api.pending_identifications import router as pending_identifications_router +from src.web.api.pending_linkages import router as pending_linkages_router from src.web.api.photos import router as photos_router from src.web.api.reported_photos import router as reported_photos_router from src.web.api.pending_photos import router as pending_photos_router @@ -408,6 +409,7 @@ def create_app() -> FastAPI: app.include_router(faces_router, prefix="/api/v1") app.include_router(people_router, prefix="/api/v1") app.include_router(pending_identifications_router, prefix="/api/v1") + app.include_router(pending_linkages_router, prefix="/api/v1") app.include_router(reported_photos_router, prefix="/api/v1") app.include_router(pending_photos_router, prefix="/api/v1") app.include_router(tags_router, prefix="/api/v1") diff --git a/src/web/constants/role_features.py b/src/web/constants/role_features.py index 5b6f937..8e12f78 100644 --- a/src/web/constants/role_features.py +++ b/src/web/constants/role_features.py @@ -17,6 +17,7 @@ ROLE_FEATURES: Final[List[dict[str, str]]] = [ {"key": "faces_maintenance", "label": "Faces Maintenance"}, {"key": "user_identified", "label": "User Identified"}, {"key": "user_reported", "label": "User Reported"}, + {"key": "user_tagged", "label": "User Tagged Photos"}, {"key": "user_uploaded", "label": "User Uploaded"}, {"key": "manage_users", "label": "Manage Users"}, {"key": "manage_roles", "label": "Manage Roles"}, @@ -28,10 +29,10 @@ DEFAULT_ROLE_FEATURE_MATRIX: Final[Dict[str, Set[str]]] = { UserRole.ADMIN.value: set(ROLE_FEATURE_KEYS), UserRole.MANAGER.value: set(ROLE_FEATURE_KEYS), UserRole.MODERATOR.value: {"scan", "process", "manage_users"}, - UserRole.REVIEWER.value: {"user_identified", "user_reported", "user_uploaded"}, - UserRole.EDITOR.value: {"user_identified", "user_uploaded", "manage_users"}, + UserRole.REVIEWER.value: {"user_identified", "user_reported", "user_uploaded", "user_tagged"}, + UserRole.EDITOR.value: {"user_identified", "user_uploaded", "manage_users", "user_tagged"}, UserRole.IMPORTER.value: {"user_uploaded"}, - UserRole.VIEWER.value: {"user_identified", "user_reported"}, + UserRole.VIEWER.value: {"user_identified", "user_reported", "user_tagged"}, } diff --git a/tests/test_pending_linkages_api.py b/tests/test_pending_linkages_api.py new file mode 100644 index 0000000..db4da7e --- /dev/null +++ b/tests/test_pending_linkages_api.py @@ -0,0 +1,264 @@ +from __future__ import annotations + +from typing import Generator + +import pytest +from fastapi.testclient import TestClient +from sqlalchemy import create_engine, text +from sqlalchemy.orm import Session, sessionmaker +from sqlalchemy.pool import StaticPool + +from src.web.app import app +from src.web.db import models +from src.web.db.models import Photo, PhotoTagLinkage, Tag, User +from src.web.db.session import get_auth_db, get_db +from src.web.constants.roles import DEFAULT_ADMIN_ROLE +from src.web.api.auth import get_current_user + + +# Create isolated in-memory databases for main and auth stores. +main_engine = create_engine( + "sqlite://", + connect_args={"check_same_thread": False}, + poolclass=StaticPool, +) +auth_engine = create_engine( + "sqlite://", + connect_args={"check_same_thread": False}, + poolclass=StaticPool, +) + +MainSessionLocal = sessionmaker( + bind=main_engine, autoflush=False, autocommit=False, future=True +) +AuthSessionLocal = sessionmaker( + bind=auth_engine, autoflush=False, autocommit=False, future=True +) + +models.Base.metadata.create_all(bind=main_engine) + +with auth_engine.begin() as connection: + connection.execute( + text( + """ + CREATE TABLE users ( + id INTEGER PRIMARY KEY, + name TEXT, + email TEXT + ) + """ + ) + ) + connection.execute( + text( + """ + CREATE TABLE pending_linkages ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + photo_id INTEGER NOT NULL, + tag_id INTEGER, + tag_name VARCHAR(255), + user_id INTEGER NOT NULL, + status VARCHAR(50) DEFAULT 'pending', + notes TEXT, + created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP + ) + """ + ) + ) + + +def override_get_db() -> Generator[Session, None, None]: + db = MainSessionLocal() + try: + yield db + finally: + db.close() + + +def override_get_auth_db() -> Generator[Session, None, None]: + db = AuthSessionLocal() + try: + yield db + finally: + db.close() + + +def override_get_current_user() -> dict[str, str]: + return {"username": "admin"} + + +app.dependency_overrides[get_db] = override_get_db +app.dependency_overrides[get_auth_db] = override_get_auth_db +app.dependency_overrides[get_current_user] = override_get_current_user + +client = TestClient(app) + + +def _ensure_admin_user() -> None: + with MainSessionLocal() as session: + existing = session.query(User).filter(User.username == "admin").first() + if existing: + existing.is_admin = True + existing.role = DEFAULT_ADMIN_ROLE + session.commit() + return + + admin_user = User( + username="admin", + password_hash="test", + email="admin@example.com", + full_name="Admin", + is_active=True, + is_admin=True, + role=DEFAULT_ADMIN_ROLE, + ) + session.add(admin_user) + session.commit() + + +@pytest.fixture(autouse=True) +def clean_databases() -> Generator[None, None, None]: + with MainSessionLocal() as session: + session.query(PhotoTagLinkage).delete() + session.query(Tag).delete() + session.query(Photo).delete() + session.query(User).filter(User.username != "admin").delete() + session.commit() + + with AuthSessionLocal() as session: + session.execute(text("DELETE FROM pending_linkages")) + session.execute(text("DELETE FROM users")) + session.commit() + + _ensure_admin_user() + yield + + +def _insert_auth_user(user_id: int = 1) -> None: + with auth_engine.begin() as connection: + connection.execute( + text( + """ + INSERT INTO users (id, name, email) + VALUES (:id, :name, :email) + """ + ), + {"id": user_id, "name": "Tester", "email": "tester@example.com"}, + ) + + +def _insert_pending_linkage( + photo_id: int, + *, + tag_id: int | None = None, + tag_name: str | None = None, + status: str = "pending", + user_id: int = 1, +) -> int: + with auth_engine.begin() as connection: + result = connection.execute( + text( + """ + INSERT INTO pending_linkages ( + photo_id, tag_id, tag_name, user_id, status, notes + ) + VALUES (:photo_id, :tag_id, :tag_name, :user_id, :status, 'note') + """ + ), + { + "photo_id": photo_id, + "tag_id": tag_id, + "tag_name": tag_name, + "user_id": user_id, + "status": status, + }, + ) + return int(result.lastrowid) + + +def _create_photo(path: str, filename: str, file_hash: str) -> int: + with MainSessionLocal() as session: + photo = Photo(path=path, filename=filename, file_hash=file_hash) + session.add(photo) + session.commit() + session.refresh(photo) + return photo.id + + +def test_list_pending_linkages_returns_existing_rows(): + _ensure_admin_user() + photo_id = _create_photo("/tmp/photo1.jpg", "photo1.jpg", "hash1") + _insert_auth_user() + linkage_id = _insert_pending_linkage(photo_id, tag_name="Beach Day") + + response = client.get("/api/v1/pending-linkages") + assert response.status_code == 200 + + payload = response.json() + assert payload["total"] == 1 + item = payload["items"][0] + assert item["photo_id"] == photo_id + assert item["proposed_tag_name"] == "Beach Day" + assert item["status"] == "pending" + + +def test_review_pending_linkages_creates_tag_and_linkage(): + _ensure_admin_user() + photo_id = _create_photo("/tmp/photo2.jpg", "photo2.jpg", "hash2") + _insert_auth_user() + linkage_id = _insert_pending_linkage(photo_id, tag_name="Sunset Crew") + + response = client.post( + "/api/v1/pending-linkages/review", + json={"decisions": [{"id": linkage_id, "decision": "approve"}]}, + ) + assert response.status_code == 200 + + payload = response.json() + assert payload["approved"] == 1 + assert payload["denied"] == 0 + assert payload["tags_created"] == 1 + assert payload["linkages_created"] == 1 + + with MainSessionLocal() as session: + tags = session.query(Tag).all() + assert len(tags) == 1 + assert tags[0].tag_name == "Sunset Crew" + linkage = session.query(PhotoTagLinkage).first() + assert linkage is not None + assert linkage.photo_id == photo_id + assert linkage.tag_id == tags[0].id + + with AuthSessionLocal() as session: + statuses = session.execute( + text("SELECT status FROM pending_linkages WHERE id = :id"), + {"id": linkage_id}, + ).fetchone() + assert statuses is not None + assert statuses[0] == "approved" + + +def test_cleanup_pending_linkages_deletes_approved_and_denied(): + _ensure_admin_user() + photo_id = _create_photo("/tmp/photo3.jpg", "photo3.jpg", "hash3") + _insert_auth_user() + + approved_id = _insert_pending_linkage(photo_id, tag_name="Approved Tag", status="approved") + denied_id = _insert_pending_linkage(photo_id, tag_name="Denied Tag", status="denied") + pending_id = _insert_pending_linkage(photo_id, tag_name="Pending Tag", status="pending") + + response = client.post("/api/v1/pending-linkages/cleanup") + assert response.status_code == 200 + + payload = response.json() + assert payload["deleted_records"] == 2 + + with AuthSessionLocal() as session: + remaining = session.execute( + text("SELECT id, status FROM pending_linkages ORDER BY id") + ).fetchall() + assert len(remaining) == 1 + assert remaining[0][0] == pending_id + assert remaining[0][1] == "pending" +