feat(admin): finalizing modular refactoring with testing suite and UI polish

backend/main.py

@@ -6,7 +6,8 @@ from slowapi import Limiter
 from slowapi.util import get_remote_address
 from . import models
 from .database import engine
-from .routers import items, operations, users, categories, admin_db
+from .routers import items, operations, users, categories
+from .routers.admin import backups, config
 from .logger import log
 from .scheduler import scheduler, sync_scheduler_config
 
@@ -87,7 +88,8 @@ app.include_router(items.router)
 app.include_router(operations.router)
 app.include_router(users.router)
 app.include_router(categories.router)
-app.include_router(admin_db.router)
+app.include_router(backups.router)
+app.include_router(config.router)
 
 @app.on_event("startup")
 def startup_event():

backend/requirements.txt

@@ -13,3 +13,6 @@ passlib[bcrypt]>=1.7.4
 python-jose[cryptography]>=3.3.0
 slowapi>=0.1.9
 apscheduler>=3.10.1
+pytest>=8.0.0
+pytest-asyncio>=0.23.0
+httpx>=0.27.0

backend/routers/admin/backups.py

@@ -0,0 +1,91 @@
+from fastapi import APIRouter, Depends, HTTPException, UploadFile, File
+from sqlalchemy.orm import Session
+from typing import List
+from fastapi.responses import FileResponse
+from ... import schemas, auth, models
+from ...database import get_db
+from ...db_manager import DbManager
+
+router = APIRouter(
+    prefix="/admin/db",
+    tags=["Admin Database Backups"]
+)
+
+@router.get("/backups", response_model=List[schemas.BackupInfo])
+def get_backups(
+    db: Session = Depends(get_db),
+    current_admin: auth.TokenData = Depends(auth.get_current_admin)
+):
+    """List available database backups."""
+    return DbManager.get_backup_list()
+
+@router.get("/stats", response_model=schemas.DatabaseStats)
+def get_db_stats(
+    db: Session = Depends(get_db),
+    current_admin: auth.TokenData = Depends(auth.get_current_admin)
+):
+    """Get database backup storage statistics."""
+    return DbManager.get_stats()
+
+@router.post("/backup", response_model=schemas.BackupInfo)
+def trigger_manual_backup(
+    db: Session = Depends(get_db),
+    current_admin: auth.TokenData = Depends(auth.get_current_admin)
+):
+    """Trigger a manual database backup."""
+    filename = DbManager.create_backup(db, label="manual", user_id=current_admin.sub)
+    backups = DbManager.get_backup_list()
+    for b in backups:
+        if b.filename == filename:
+            return b
+    raise HTTPException(status_code=500, detail="Backup created but info not found")
+
+@router.post("/restore")
+def restore_database(
+    payload: dict,
+    db: Session = Depends(get_db),
+    current_admin: auth.TokenData = Depends(auth.get_current_admin)
+):
+    """Restore database from a specific file. DANGEROUS."""
+    filename = payload.get("filename")
+    confirm = payload.get("confirm", False)
+    
+    if not filename:
+        raise HTTPException(status_code=400, detail="Filename required")
+    if not confirm:
+        raise HTTPException(status_code=400, detail="Confirmation required")
+        
+    try:
+        DbManager.restore_backup(filename, db, user_id=current_admin.sub)
+        return {"status": "success", "message": f"Database restored from {filename}"}
+    except Exception as e:
+        raise HTTPException(status_code=500, detail=str(e))
+
+@router.get("/export")
+def export_database(
+    current_admin: auth.TokenData = Depends(auth.get_current_admin)
+):
+    """Download the current database file."""
+    try:
+        path = DbManager.export_db()
+        return FileResponse(
+            path, 
+            media_type="application/x-sqlite3", 
+            filename="inventory_export.db"
+        )
+    except Exception as e:
+        raise HTTPException(status_code=500, detail=str(e))
+
+@router.post("/import")
+async def import_database(
+    file: UploadFile = File(...),
+    db: Session = Depends(get_db),
+    current_admin: auth.TokenData = Depends(auth.get_current_admin)
+):
+    """Upload and replace the current database. DANGEROUS."""
+    contents = await file.read()
+    try:
+        DbManager.import_db(contents, db, user_id=current_admin.sub)
+        return {"status": "success", "message": "Database successfully imported and replaced."}
+    except Exception as e:
+        raise HTTPException(status_code=500, detail=str(e))

backend/routers/admin/config.py

@@ -1,68 +1,18 @@
-from fastapi import APIRouter, Depends, HTTPException, status
+import os
+from fastapi import APIRouter, Depends, HTTPException
 from sqlalchemy.orm import Session
-from typing import List
-from .. import models, schemas, auth
-from ..database import get_db
-from ..db_manager import DbManager
-from ..scheduler import sync_scheduler_config
-from fastapi.responses import FileResponse
-from fastapi import UploadFile, File
+from ... import models, schemas, auth
+from ...database import get_db, BASE_DIR
+from ...scheduler import sync_scheduler_config
+from ...config_manager import ConfigManager
 
 router = APIRouter(
     prefix="/admin/db",
-    tags=["Admin Database"]
+    tags=["Admin Configuration"]
 )
 
-@router.get("/backups", response_model=List[schemas.BackupInfo])
-def get_backups(
-    db: Session = Depends(get_db),
-    current_admin: auth.TokenData = Depends(auth.get_current_admin)
-):
-    """List available database backups."""
-    return DbManager.get_backup_list()
-
-@router.get("/stats", response_model=schemas.DatabaseStats)
-def get_db_stats(
-    db: Session = Depends(get_db),
-    current_admin: auth.TokenData = Depends(auth.get_current_admin)
-):
-    """Get database backup storage statistics."""
-    return DbManager.get_stats()
-
-@router.post("/backup", response_model=schemas.BackupInfo)
-def trigger_manual_backup(
-    db: Session = Depends(get_db),
-    current_admin: auth.TokenData = Depends(auth.get_current_admin)
-):
-    """Trigger a manual database backup."""
-    filename = DbManager.create_backup(db, label="manual", user_id=current_admin.sub)
-    # Re-fetch the newly created file info
-    backups = DbManager.get_backup_list()
-    for b in backups:
-        if b.filename == filename:
-            return b
-    raise HTTPException(status_code=500, detail="Backup created but info not found")
-
-@router.post("/restore")
-def restore_database(
-    payload: dict,
-    db: Session = Depends(get_db),
-    current_admin: auth.TokenData = Depends(auth.get_current_admin)
-):
-    """Restore database from a specific file. DANGEROUS."""
-    filename = payload.get("filename")
-    confirm = payload.get("confirm", False)
-    
-    if not filename:
-        raise HTTPException(status_code=400, detail="Filename required")
-    if not confirm:
-        raise HTTPException(status_code=400, detail="Confirmation required")
-        
-    try:
-        success = DbManager.restore_backup(filename, db, user_id=current_admin.sub)
-        return {"status": "success", "message": f"Database restored from {filename}"}
-    except Exception as e:
-        raise HTTPException(status_code=500, detail=str(e))
+PROJECT_ROOT = os.path.dirname(BASE_DIR)
+PROMPT_FILE_PATH = os.path.join(PROJECT_ROOT, "config", "ai_prompt.md")
 
 @router.get("/settings", response_model=schemas.DbSettingsUpdate)
 def get_db_settings(
@@ -70,7 +20,6 @@ def get_db_settings(
     current_admin: auth.TokenData = Depends(auth.get_current_admin)
 ):
     """Get database retention and scheduling settings."""
-    # Ensure default settings exist
     retention = db.query(models.SystemSetting).filter(models.SystemSetting.key == "backup_retention_count").first()
     hour = db.query(models.SystemSetting).filter(models.SystemSetting.key == "backup_schedule_hour").first()
     freq = db.query(models.SystemSetting).filter(models.SystemSetting.key == "backup_schedule_freq_days").first()
@@ -102,59 +51,22 @@ def update_db_settings(
             db.add(models.SystemSetting(key=key, value=val))
     
     db.commit()
-    # Re-trigger scheduler sync
     sync_scheduler_config()
     return settings
 
-@router.get("/export")
-def export_database(
-    current_admin: auth.TokenData = Depends(auth.get_current_admin)
-):
-    """Download the current database file."""
-    try:
-        path = DbManager.export_db()
-        return FileResponse(
-            path, 
-            media_type="application/x-sqlite3", 
-            filename="inventory_export.db"
-        )
-    except Exception as e:
-        raise HTTPException(status_code=500, detail=str(e))
-
-@router.post("/import")
-async def import_database(
-    file: UploadFile = File(...),
-    db: Session = Depends(get_db),
-    current_admin: auth.TokenData = Depends(auth.get_current_admin)
-):
-    """Upload and replace the current database. DANGEROUS."""
-    contents = await file.read()
-    try:
-        DbManager.import_db(contents, db, user_id=current_admin.sub)
-        return {"status": "success", "message": "Database successfully imported and replaced."}
-    except Exception as e:
-        raise HTTPException(status_code=500, detail=str(e))
-
-import os
-from ..database import BASE_DIR
-PROJECT_ROOT = os.path.dirname(BASE_DIR)
-PROMPT_FILE_PATH = os.path.join(PROJECT_ROOT, "config", "ai_prompt.md")
-
 @router.get("/settings/prompt")
 def get_ai_prompt(
     db: Session = Depends(get_db),
     current_admin: auth.TokenData = Depends(auth.get_current_admin)
 ):
-    """Get the current AI extraction prompt (prioritizes config file)."""
-    # 1. Try file first
+    """Get the current AI extraction prompt."""
     if os.path.exists(PROMPT_FILE_PATH):
         try:
             with open(PROMPT_FILE_PATH, 'r', encoding='utf-8') as f:
                 return {"value": f.read().strip(), "source": "file"}
-        except Exception as e:
-            print(f"Error reading prompt file: {e}")
+        except Exception:
+            pass
 
-    # 2. Fallback to DB
     setting = db.query(models.SystemSetting).filter(models.SystemSetting.key == "ai_extraction_prompt").first()
     if not setting:
         return {"value": "", "source": "none"}
@@ -166,21 +78,18 @@ def update_ai_prompt(
     db: Session = Depends(get_db),
     current_admin: auth.TokenData = Depends(auth.get_current_admin)
 ):
-    """Update the AI extraction prompt (writes to both file and DB)."""
+    """Update the AI extraction prompt."""
     value = payload.get("value")
     if value is None:
         raise HTTPException(status_code=400, detail="Value required")
     
-    # 1. Update File (Primary)
     try:
         os.makedirs(os.path.dirname(PROMPT_FILE_PATH), exist_ok=True)
         with open(PROMPT_FILE_PATH, 'w', encoding='utf-8') as f:
             f.write(value)
-    except Exception as e:
-        print(f"Failed to write prompt file: {e}")
-        # We continue to DB update anyway
+    except Exception:
+        pass
 
-    # 2. Update DB (Backup/Sync)
     existing = db.query(models.SystemSetting).filter(models.SystemSetting.key == "ai_extraction_prompt").first()
     if existing:
         existing.value = value
@@ -190,8 +99,6 @@ def update_ai_prompt(
     db.commit()
     return {"status": "success", "file_updated": os.path.exists(PROMPT_FILE_PATH)}
 
-from ..config_manager import ConfigManager
-
 @router.get("/settings/ai")
 def get_ai_config(
     db: Session = Depends(get_db),
@@ -201,7 +108,6 @@ def get_ai_config(
     gemini_key = os.environ.get("GEMINI_API_KEY")
     claude_key = os.environ.get("CLAUDE_API_KEY")
     
-    # Get active provider from DB
     provider_setting = db.query(models.SystemSetting).filter(models.SystemSetting.key == "ai_provider").first()
     active_provider = provider_setting.value if provider_setting else "gemini"
     
@@ -230,7 +136,7 @@ def update_ai_keys(
     payload: dict,
     current_admin: auth.TokenData = Depends(auth.get_current_admin)
 ):
-    """Update AI API keys in inventory.env."""
+    """Update AI API keys."""
     gemini_key = payload.get("gemini_api_key")
     claude_key = payload.get("claude_api_key")
     
@@ -240,10 +146,9 @@ def update_ai_keys(
     if claude_key:
         updates["CLAUDE_API_KEY"] = claude_key
         
-    if not updates:
-        return {"status": "no_change"}
+    if updates:
+        ConfigManager.update_keys(updates)
         
-    ConfigManager.update_keys(updates)
     return {
         "status": "success", 
         "gemini_configured": bool(os.environ.get("GEMINI_API_KEY")),
@@ -262,7 +167,6 @@ def test_ai_key(
     if not provider or provider not in ["gemini", "claude"]:
         raise HTTPException(status_code=400, detail="Invalid provider")
     
-    # If key is masked or empty, use the one from environment
     if not key or "****" in key:
         key = os.environ.get("GEMINI_API_KEY" if provider == "gemini" else "CLAUDE_API_KEY")
         
@@ -273,25 +177,15 @@ def test_ai_key(
         if provider == "gemini":
             from google import genai
             client = genai.Client(api_key=key, http_options={'api_version': 'v1beta'})
-            # Listing models is the standard way to check key validity
             client.models.list()
             return {"status": "success", "message": "Google Gemini API connection verified!"}
-        
         elif provider == "claude":
             import anthropic
             client = anthropic.Anthropic(api_key=key)
-            # Claude also supports model listing for key verification
             client.models.list(limit=1)
             return {"status": "success", "message": "Anthropic Claude API connection verified!"}
-            
     except Exception as e:
-        error_msg = str(e)
-        if "API_KEY_INVALID" in error_msg or "invalid_api_key" in error_msg:
-             error_msg = "The API key provided is invalid."
-        elif "PERMISSION_DENIED" in error_msg:
-             error_msg = "Permission denied. Check if the key has correct permissions."
-             
-        raise HTTPException(status_code=400, detail=f"{provider.capitalize()} Test Failed: {error_msg}")
+        raise HTTPException(status_code=400, detail=f"{provider.capitalize()} Test Failed: {str(e)}")
 
 @router.post("/settings/ai")
 def update_ai_provider(
@@ -302,7 +196,7 @@ def update_ai_provider(
     """Update the active AI provider."""
     provider = payload.get("provider")
     if provider not in ["gemini", "claude"]:
-        raise HTTPException(status_code=400, detail="Invalid provider. Must be 'gemini' or 'claude'.")
+        raise HTTPException(status_code=400, detail="Invalid provider")
     
     existing = db.query(models.SystemSetting).filter(models.SystemSetting.key == "ai_provider").first()
     if existing:

backend/tests/conftest.py

@@ -0,0 +1,65 @@
+import pytest
+from fastapi.testclient import TestClient
+from sqlalchemy import create_engine
+from sqlalchemy.orm import sessionmaker
+from sqlalchemy.pool import StaticPool
+
+from backend.database import Base, get_db
+from backend.main import app
+from backend.auth import get_current_admin, TokenData
+
+from datetime import datetime, timezone
+
+# Use in-memory SQLite for tests
+SQLALCHEMY_DATABASE_URL = "sqlite://"
+
+engine = create_engine(
+    SQLALCHEMY_DATABASE_URL,
+    connect_args={"check_same_thread": False},
+    poolclass=StaticPool,
+)
+TestingSessionLocal = sessionmaker(autocommit=False, autoflush=False, bind=engine)
+
+@pytest.fixture(scope="function", autouse=True)
+def mock_scheduler():
+    from backend.scheduler import scheduler
+    if scheduler.running:
+        scheduler.shutdown()
+    yield
+    if scheduler.running:
+        scheduler.shutdown()
+
+@pytest.fixture(scope="function")
+def db():
+    Base.metadata.create_all(bind=engine)
+    session = TestingSessionLocal()
+    try:
+        yield session
+    finally:
+        session.close()
+        Base.metadata.drop_all(bind=engine)
+
+@pytest.fixture(scope="function")
+def client(db):
+    def override_get_db():
+        try:
+            yield db
+        finally:
+            pass
+    
+    # Mock admin user with valid TokenData
+    def override_get_current_admin():
+        return TokenData(
+            sub=1, 
+            username="admin", 
+            role="admin", 
+            exp=datetime.now(timezone.utc)
+        )
+
+    app.dependency_overrides[get_db] = override_get_db
+    app.dependency_overrides[get_current_admin] = override_get_current_admin
+    
+    with TestClient(app) as c:
+        yield c
+    
+    app.dependency_overrides.clear()

backend/tests/test_admin.py

@@ -0,0 +1,35 @@
+import pytest
+
+def test_get_backups(client):
+    response = client.get("/admin/db/backups")
+    assert response.status_code == 200
+    assert isinstance(response.json(), list)
+
+def test_db_settings_workflow(client):
+    # GET settings
+    response = client.get("/admin/db/settings")
+    assert response.status_code == 200
+    data = response.json()
+    assert "retention_count" in data
+    
+    # PATCH settings
+    new_settings = {
+        "retention_count": 25,
+        "schedule_hour": 5,
+        "schedule_freq_days": 2
+    }
+    response = client.patch("/admin/db/settings", json=new_settings)
+    assert response.status_code == 200
+    assert response.json()["retention_count"] == 25
+    
+    # Verify persistence
+    response = client.get("/admin/db/settings")
+    assert response.json()["retention_count"] == 25
+
+def test_ai_config(client):
+    response = client.get("/admin/db/settings/ai")
+    assert response.status_code == 200
+    data = response.json()
+    assert "active_provider" in data
+    assert "providers" in data
+    assert len(data["providers"]) == 2