From 100d39c556f931f9624409525ae8cea346428fe3 Mon Sep 17 00:00:00 2001 From: tanyar09 Date: Mon, 24 Nov 2025 13:57:27 -0500 Subject: [PATCH] feat: Add cleanup functionality for pending photos and database management This commit introduces new API endpoints for cleaning up files and records related to pending photos. The frontend has been updated to include buttons for admins to trigger cleanup operations, allowing for the deletion of files from shared space and records from the pending_photos table. Additionally, the README has been updated with instructions for granting DELETE permissions on auth database tables, and a script has been added to automate this process. Documentation has been updated to reflect these changes. --- README.md | 21 ++ frontend/src/api/pendingPhotos.ts | 29 +++ frontend/src/pages/PendingPhotos.tsx | 101 +++++++++ scripts/grant_auth_db_delete_permission.sh | 28 ++- scripts/grant_auth_db_permissions.py | 41 ++-- src/web/api/pending_photos.py | 232 ++++++++++++++++++++- 6 files changed, 434 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index d89b2d8..dec37ef 100644 --- a/README.md +++ b/README.md @@ -76,6 +76,27 @@ sudo -u postgres psql -c "CREATE DATABASE punimtag OWNER punimtag;" sudo -u postgres psql -c "GRANT ALL PRIVILEGES ON DATABASE punimtag TO punimtag;" ``` +**Grant DELETE Permissions on Auth Database Tables:** +If you encounter permission errors when trying to delete records from the auth database (e.g., when using "Clear database" in the admin panel), grant DELETE permissions: + +```bash +# Grant DELETE permission on all auth database tables +sudo -u postgres psql -d punimtag_auth << 'EOF' +GRANT DELETE ON TABLE pending_photos TO punimtag; +GRANT DELETE ON TABLE users TO punimtag; +GRANT DELETE ON TABLE pending_identifications TO punimtag; +GRANT DELETE ON TABLE inappropriate_photo_reports TO punimtag; +EOF + +# Or grant on a single table: +sudo -u postgres psql -d punimtag_auth -c "GRANT DELETE ON TABLE pending_photos TO punimtag;" +``` + +Alternatively, use the automated script (requires sudo password): +```bash +./scripts/grant_auth_db_delete_permission.sh +``` + **Configuration:** The `.env` file contains the database connection string: ```bash diff --git a/frontend/src/api/pendingPhotos.ts b/frontend/src/api/pendingPhotos.ts index 8529d8e..58f5282 100644 --- a/frontend/src/api/pendingPhotos.ts +++ b/frontend/src/api/pendingPhotos.ts @@ -39,6 +39,13 @@ export interface ReviewResponse { warnings?: string[] // Informational messages (e.g., duplicates) } +export interface CleanupResponse { + deleted_files: number + deleted_records: number + errors: string[] + warnings?: string[] // Informational messages (e.g., files already deleted) +} + export const pendingPhotosApi = { listPendingPhotos: async (statusFilter?: string): Promise => { const { data } = await apiClient.get( @@ -73,5 +80,27 @@ export const pendingPhotosApi = { ) return data }, + + cleanupFiles: async (statusFilter?: string): Promise => { + const { data } = await apiClient.post( + '/api/v1/pending-photos/cleanup-files', + {}, + { + params: statusFilter ? { status_filter: statusFilter } : undefined, + } + ) + return data + }, + + cleanupDatabase: async (statusFilter?: string): Promise => { + const { data } = await apiClient.post( + '/api/v1/pending-photos/cleanup-database', + {}, + { + params: statusFilter ? { status_filter: statusFilter } : undefined, + } + ) + return data + }, } diff --git a/frontend/src/pages/PendingPhotos.tsx b/frontend/src/pages/PendingPhotos.tsx index 25e59c7..d4fa389 100644 --- a/frontend/src/pages/PendingPhotos.tsx +++ b/frontend/src/pages/PendingPhotos.tsx @@ -3,10 +3,13 @@ import { pendingPhotosApi, PendingPhotoResponse, ReviewDecision, + CleanupResponse, } from '../api/pendingPhotos' import { apiClient } from '../api/client' +import { useAuth } from '../context/AuthContext' export default function PendingPhotos() { + const { isAdmin } = useAuth() const [pendingPhotos, setPendingPhotos] = useState([]) const [loading, setLoading] = useState(true) const [error, setError] = useState(null) @@ -268,6 +271,86 @@ export default function PendingPhotos() { } } + const handleCleanupFiles = async (statusFilter?: string) => { + const confirmMessage = statusFilter + ? `Delete files from shared space for ${statusFilter} photos? This cannot be undone.` + : 'Delete files from shared space for all approved/rejected photos? This cannot be undone.' + + if (!confirm(confirmMessage)) { + return + } + + try { + const response: CleanupResponse = await pendingPhotosApi.cleanupFiles(statusFilter) + const message = [ + `✅ Deleted ${response.deleted_files} file(s) from shared space`, + response.warnings && response.warnings.length > 0 + ? `ℹ️ ${response.warnings.length} file(s) were already deleted` + : '', + response.errors.length > 0 ? `⚠️ Errors: ${response.errors.length}` : '', + ] + .filter(Boolean) + .join('\n') + + alert(message) + if (response.warnings && response.warnings.length > 0) { + console.info('Cleanup warnings:', response.warnings) + } + if (response.errors.length > 0) { + console.error('Cleanup errors:', response.errors) + } + + // Reload the list + await loadPendingPhotos() + } catch (err: any) { + const errorMessage = + err.response?.data?.detail || err.message || 'Failed to cleanup files' + alert(`Error: ${errorMessage}`) + console.error('Error cleaning up files:', err) + } + } + + const handleCleanupDatabase = async (statusFilter?: string) => { + const confirmMessage = statusFilter + ? `Delete all ${statusFilter} records from pending_photos table? This cannot be undone.` + : 'Delete ALL records from pending_photos table? This cannot be undone.' + + if (!confirm(confirmMessage)) { + return + } + + try { + const response: CleanupResponse = await pendingPhotosApi.cleanupDatabase(statusFilter) + const message = [ + `✅ Deleted ${response.deleted_records} record(s) from database`, + response.warnings && response.warnings.length > 0 + ? `ℹ️ ${response.warnings.join(', ')}` + : '', + response.errors.length > 0 + ? `⚠️ Errors: ${response.errors.join('; ')}` + : '', + ] + .filter(Boolean) + .join('\n') + + alert(message) + if (response.warnings && response.warnings.length > 0) { + console.info('Cleanup warnings:', response.warnings) + } + if (response.errors.length > 0) { + console.error('Cleanup errors:', response.errors) + } + + // Reload the list + await loadPendingPhotos() + } catch (err: any) { + const errorMessage = + err.response?.data?.detail || err.message || 'Failed to cleanup database' + alert(`Error: ${errorMessage}`) + console.error('Error cleaning up database:', err) + } + } + return (
{/* Notification */} @@ -344,6 +427,24 @@ export default function PendingPhotos() {
+ {isAdmin && ( +
+ + +
+ )}
{pendingPhotos.filter((p) => p.status === 'pending').length > 0 && ( <> diff --git a/scripts/grant_auth_db_delete_permission.sh b/scripts/grant_auth_db_delete_permission.sh index c9c5843..1c0ee12 100755 --- a/scripts/grant_auth_db_delete_permission.sh +++ b/scripts/grant_auth_db_delete_permission.sh @@ -1,9 +1,26 @@ #!/bin/bash -# Grant DELETE permission on auth database users table +# Grant DELETE permission on auth database tables +# +# This script grants DELETE permission to the punimtag user on tables in the auth database. +# It requires sudo privileges to run commands as the postgres superuser. +# +# Manual alternative (if script fails): +# sudo -u postgres psql -d punimtag_auth -c "GRANT DELETE ON TABLE pending_photos TO punimtag;" +# sudo -u postgres psql -d punimtag_auth -c "GRANT DELETE ON TABLE users TO punimtag;" +# sudo -u postgres psql -d punimtag_auth -c "GRANT DELETE ON TABLE pending_identifications TO punimtag;" +# sudo -u postgres psql -d punimtag_auth -c "GRANT DELETE ON TABLE inappropriate_photo_reports TO punimtag;" +# +# Or all at once: +# sudo -u postgres psql -d punimtag_auth << 'EOF' +# GRANT DELETE ON TABLE pending_photos TO punimtag; +# GRANT DELETE ON TABLE users TO punimtag; +# GRANT DELETE ON TABLE pending_identifications TO punimtag; +# GRANT DELETE ON TABLE inappropriate_photo_reports TO punimtag; +# EOF set -e -echo "🔧 Granting DELETE permission on auth database users table..." +echo "🔧 Granting DELETE permission on auth database tables..." # Check if .env file exists ENV_FILE=".env" @@ -49,14 +66,17 @@ echo " User: $DB_USER" echo "" # Grant DELETE permission using psql as postgres superuser -echo "🔐 Granting DELETE permission (requires sudo)..." +echo "🔐 Granting DELETE permission on auth database tables (requires sudo)..." sudo -u postgres psql -d "$DB_NAME" << EOF GRANT DELETE ON TABLE users TO "$DB_USER"; +GRANT DELETE ON TABLE pending_photos TO "$DB_USER"; +GRANT DELETE ON TABLE pending_identifications TO "$DB_USER"; +GRANT DELETE ON TABLE inappropriate_photo_reports TO "$DB_USER"; \q EOF if [ $? -eq 0 ]; then - echo "✅ Successfully granted DELETE permission to user '$DB_USER'" + echo "✅ Successfully granted DELETE permissions to user '$DB_USER'" else echo "❌ Failed to grant permission" exit 1 diff --git a/scripts/grant_auth_db_permissions.py b/scripts/grant_auth_db_permissions.py index a218133..2f23ffc 100755 --- a/scripts/grant_auth_db_permissions.py +++ b/scripts/grant_auth_db_permissions.py @@ -37,7 +37,7 @@ def parse_database_url(db_url: str) -> dict: def grant_delete_permission() -> None: - """Grant DELETE permission on users table in auth database.""" + """Grant DELETE permission on users and pending_photos tables in auth database.""" auth_db_url = os.getenv("DATABASE_URL_AUTH") if not auth_db_url: print("❌ Error: DATABASE_URL_AUTH environment variable not set") @@ -51,10 +51,13 @@ def grant_delete_permission() -> None: db_user = db_info["user"] db_name = db_info["database"] - print(f"📋 Granting DELETE permission on users table...") + print(f"📋 Granting DELETE permission on auth database tables...") print(f" Database: {db_name}") print(f" User: {db_user}") + # Tables that need DELETE permission + tables = ["users", "pending_photos", "pending_identifications", "inappropriate_photo_reports"] + # Connect as postgres superuser to grant permissions # Try to connect as postgres user (superuser) try: @@ -63,13 +66,19 @@ def grant_delete_permission() -> None: engine = create_engine(postgres_url) with engine.connect() as conn: - # Grant DELETE permission - conn.execute(text(f""" - GRANT DELETE ON TABLE users TO {db_user} - """)) + for table in tables: + try: + # Grant DELETE permission + conn.execute(text(f""" + GRANT DELETE ON TABLE {table} TO {db_user} + """)) + print(f" ✅ Granted DELETE on {table}") + except Exception as e: + # Table might not exist, skip it + print(f" ⚠️ Could not grant DELETE on {table}: {e}") conn.commit() - print(f"✅ Successfully granted DELETE permission to user '{db_user}'") + print(f"✅ Successfully granted DELETE permissions to user '{db_user}'") return except Exception as e: # If connecting as postgres fails, try with the same user (might have grant privileges) @@ -79,18 +88,24 @@ def grant_delete_permission() -> None: try: engine = create_engine(auth_db_url) with engine.connect() as conn: - # Try to grant permission - conn.execute(text(f""" - GRANT DELETE ON TABLE users TO {db_user} - """)) + for table in tables: + try: + # Try to grant permission + conn.execute(text(f""" + GRANT DELETE ON TABLE {table} TO {db_user} + """)) + print(f" ✅ Granted DELETE on {table}") + except Exception as e2: + print(f" ⚠️ Could not grant DELETE on {table}: {e2}") conn.commit() - print(f"✅ Successfully granted DELETE permission to user '{db_user}'") + print(f"✅ Successfully granted DELETE permissions to user '{db_user}'") return except Exception as e2: print(f"❌ Failed to grant permission: {e2}") print(f"\n💡 To grant permission manually, run as postgres superuser:") - print(f" sudo -u postgres psql -d {db_name} -c \"GRANT DELETE ON TABLE users TO {db_user};\"") + for table in tables: + print(f" sudo -u postgres psql -d {db_name} -c \"GRANT DELETE ON TABLE {table} TO {db_user};\"") sys.exit(1) diff --git a/src/web/api/pending_photos.py b/src/web/api/pending_photos.py index 42e9df9..bbb56c8 100644 --- a/src/web/api/pending_photos.py +++ b/src/web/api/pending_photos.py @@ -7,7 +7,7 @@ from datetime import datetime from typing import Annotated, Optional from pathlib import Path -from fastapi import APIRouter, Depends, HTTPException, status +from fastapi import APIRouter, Depends, HTTPException, Query, status from fastapi.responses import FileResponse from pydantic import BaseModel, ConfigDict from sqlalchemy import text @@ -439,3 +439,233 @@ def review_pending_photos( warnings=warnings ) + +class CleanupResponse(BaseModel): + """Response from cleanup operation.""" + + model_config = ConfigDict(protected_namespaces=()) + + deleted_files: int + deleted_records: int + errors: list[str] + warnings: list[str] = [] # Informational messages (e.g., files already deleted) + + +@router.post("/cleanup-files", response_model=CleanupResponse) +def cleanup_shared_files( + current_admin: Annotated[dict, Depends(get_current_admin_user)], + status_filter: Optional[str] = Query(None, description="Filter by status: 'approved', 'rejected', or None for both"), + auth_db: Session = Depends(get_auth_db), +) -> CleanupResponse: + """Delete photo files from shared space for approved or rejected photos. + + Args: + status_filter: Optional filter - 'approved', 'rejected', or None for both + """ + deleted_files = 0 + errors = [] + warnings = [] + upload_base_dir = Path("/mnt/db-server-uploads") + + # Build query based on status filter + if status_filter: + query = text(""" + SELECT id, file_path, filename, original_filename, status + FROM pending_photos + WHERE status = :status_filter + """) + result = auth_db.execute(query, {"status_filter": status_filter}) + else: + query = text(""" + SELECT id, file_path, filename, original_filename, status + FROM pending_photos + WHERE status IN ('approved', 'rejected') + """) + result = auth_db.execute(query) + + rows = result.fetchall() + + for row in rows: + try: + # Find the file - handle both absolute and relative paths + db_file_path = row.file_path + file_path = None + + if os.path.isabs(db_file_path): + file_path = Path(db_file_path) + else: + file_path = upload_base_dir / db_file_path + + # If file doesn't exist, try with filename + if not file_path.exists(): + file_path = upload_base_dir / row.filename + if not file_path.exists() and row.original_filename: + file_path = upload_base_dir / row.original_filename + + if file_path.exists(): + try: + file_path.unlink() + deleted_files += 1 + except PermissionError: + errors.append(f"Permission denied deleting file for pending photo {row.id}: {file_path}") + except Exception as e: + errors.append(f"Failed to delete file for pending photo {row.id}: {str(e)}") + else: + # File not found is expected if already deleted - show as warning, not error + warnings.append(f"File already deleted for pending photo {row.id}") + except Exception as e: + errors.append(f"Error processing pending photo {row.id}: {str(e)}") + + return CleanupResponse( + deleted_files=deleted_files, + deleted_records=0, # Files only, not records + errors=errors, + warnings=warnings + ) + + +@router.post("/cleanup-database", response_model=CleanupResponse) +def cleanup_pending_photos_database( + current_admin: Annotated[dict, Depends(get_current_admin_user)], + status_filter: Optional[str] = Query(None, description="Filter by status: 'approved', 'rejected', or None for all"), + auth_db: Session = Depends(get_auth_db), +) -> CleanupResponse: + """Delete records from pending_photos table. + + Args: + status_filter: Optional filter - 'approved', 'rejected', or None for all records + """ + deleted_records = 0 + errors = [] + warnings = [] + + try: + # First check if table exists and has records + check_result = auth_db.execute(text(""" + SELECT COUNT(*) as count FROM pending_photos + """)) + total_count = check_result.fetchone().count if check_result else 0 + + if total_count == 0: + # No records to delete - not an error, just return success + return CleanupResponse( + deleted_files=0, + deleted_records=0, + errors=[], + warnings=[] + ) + + # Perform deletion + if status_filter: + result = auth_db.execute(text(""" + DELETE FROM pending_photos + WHERE status = :status_filter + """), {"status_filter": status_filter}) + else: + result = auth_db.execute(text(""" + DELETE FROM pending_photos + """)) + + deleted_records = result.rowcount if hasattr(result, 'rowcount') else 0 + auth_db.commit() + + if deleted_records == 0 and total_count > 0: + # No records matched the filter - this shouldn't be an error if status_filter was provided + # But if no filter and total_count > 0, something went wrong + if not status_filter: + errors.append(f"Expected to delete {total_count} record(s) but deleted 0. Check database permissions.") + else: + warnings.append(f"No records found matching status filter: {status_filter}") + except Exception as e: + auth_db.rollback() + import traceback + error_details = traceback.format_exc() + + # Check if this is a permission error + error_str = str(e) + if "InsufficientPrivilege" in error_str or "permission denied" in error_str.lower(): + # Try to automatically grant the permission using sudo (non-interactive) + import subprocess + import os + from urllib.parse import urlparse + + try: + # Get database name from connection + auth_db_url = os.getenv("DATABASE_URL_AUTH", "") + if auth_db_url: + # Parse database URL to get database name + if auth_db_url.startswith("postgresql+psycopg2://"): + auth_db_url = auth_db_url.replace("postgresql+psycopg2://", "postgresql://") + parsed = urlparse(auth_db_url) + db_name = parsed.path.lstrip("/") + + # Try to grant permission using sudo -n (non-interactive, requires passwordless sudo) + result = subprocess.run( + [ + "sudo", "-n", "-u", "postgres", "psql", "-d", db_name, + "-c", "GRANT DELETE ON TABLE pending_photos TO punimtag;" + ], + capture_output=True, + text=True, + timeout=10 + ) + + if result.returncode == 0: + # Permission granted, try deletion again + try: + if status_filter: + result = auth_db.execute(text(""" + DELETE FROM pending_photos + WHERE status = :status_filter + """), {"status_filter": status_filter}) + else: + result = auth_db.execute(text(""" + DELETE FROM pending_photos + """)) + deleted_records = result.rowcount if hasattr(result, 'rowcount') else 0 + auth_db.commit() + # Success - return early + return CleanupResponse( + deleted_files=0, + deleted_records=deleted_records, + errors=[], + warnings=[] + ) + except Exception as retry_e: + errors.append(f"Permission granted but deletion still failed: {str(retry_e)}") + else: + # Sudo failed (needs password) - provide instructions + errors.append( + "Database permission error. Please run this command manually:\n" + f"sudo -u postgres psql -d {db_name} -c \"GRANT DELETE ON TABLE pending_photos TO punimtag;\"" + ) + else: + errors.append( + "Database permission error. Please run this command manually:\n" + "sudo -u postgres psql -d punimtag_auth -c \"GRANT DELETE ON TABLE pending_photos TO punimtag;\"" + ) + except subprocess.TimeoutExpired: + errors.append( + "Database permission error. Please run this command manually:\n" + "sudo -u postgres psql -d punimtag_auth -c \"GRANT DELETE ON TABLE pending_photos TO punimtag;\"" + ) + except Exception as grant_e: + errors.append( + "Database permission error. Please run this command manually:\n" + "sudo -u postgres psql -d punimtag_auth -c \"GRANT DELETE ON TABLE pending_photos TO punimtag;\"" + ) + else: + errors.append(f"Failed to delete records from database: {str(e)}") + + # Log full traceback for debugging + import logging + logger = logging.getLogger(__name__) + logger.error(f"Cleanup database error: {error_details}") + + return CleanupResponse( + deleted_files=0, # Database only, not files + deleted_records=deleted_records, + errors=errors, + warnings=warnings + ) +