diff --git a/app/api/catalog.py b/app/api/catalog.py index a8e82a1..5f54c09 100644 --- a/app/api/catalog.py +++ b/app/api/catalog.py @@ -1,11 +1,9 @@ """Catalog endpoints — table profiles, metrics.""" import json -import re from fastapi import APIRouter, Depends, HTTPException import duckdb -import yaml from app.auth.dependencies import get_current_user, _get_db from app.utils import get_data_dir as _get_data_dir @@ -70,30 +68,15 @@ async def list_catalog_tables( return {"tables": tables, "count": len(tables)} -@router.get("/metrics/{metric_path:path}") +@router.get("/metrics/{metric_path:path}", deprecated=True) async def get_metric( metric_path: str, user: dict = Depends(get_current_user), ): - """Get a metric YAML definition parsed as structured JSON.""" - if not re.match(r"^[a-z_]+/[a-z_]+\.yml$", metric_path): - raise HTTPException(status_code=400, detail="Invalid metric path") - - docs_dir = _get_data_dir() / "docs" / "metrics" - file_path = docs_dir / metric_path - - if not file_path.is_file(): - raise HTTPException(status_code=404, detail="Metric file not found") - - # Security: ensure path doesn't escape docs dir - if not file_path.resolve().is_relative_to(docs_dir.resolve()): - raise HTTPException(status_code=400, detail="Invalid path") - - try: - content = yaml.safe_load(file_path.read_text()) - return content - except Exception as e: - raise HTTPException(status_code=500, detail=f"Error parsing metric: {e}") + """Deprecated: use GET /api/metrics/{metric_id} instead.""" + from fastapi.responses import RedirectResponse + metric_id = metric_path.replace(".yml", "") + return RedirectResponse(url=f"/api/metrics/{metric_id}", status_code=301) @router.post("/profile/{table_name}/refresh") diff --git a/app/api/metrics.py b/app/api/metrics.py new file mode 100644 index 0000000..2d7b915 --- /dev/null +++ b/app/api/metrics.py @@ -0,0 +1,108 @@ +"""Metrics API endpoints — CRUD for metric definitions stored in DuckDB.""" + +from typing import List, Optional + +import duckdb +from fastapi import APIRouter, Depends, HTTPException +from pydantic import BaseModel + +from app.auth.dependencies import get_current_user, require_admin, _get_db +from src.repositories.metrics import MetricRepository + +router = APIRouter(tags=["metrics"]) + + +class MetricCreate(BaseModel): + id: str + name: str + display_name: str + category: str + sql: str + description: Optional[str] = None + type: str = "sum" + unit: Optional[str] = None + grain: str = "monthly" + table_name: Optional[str] = None + tables: Optional[List[str]] = None + expression: Optional[str] = None + time_column: Optional[str] = None + dimensions: Optional[List[str]] = None + filters: Optional[List[str]] = None + synonyms: Optional[List[str]] = None + notes: Optional[List[str]] = None + sql_variants: Optional[dict] = None + validation: Optional[dict] = None + source: str = "manual" + + +@router.get("/api/metrics") +async def list_metrics( + category: Optional[str] = None, + user: dict = Depends(get_current_user), + conn: duckdb.DuckDBPyConnection = Depends(_get_db), +): + """List all metric definitions, optionally filtered by category.""" + repo = MetricRepository(conn) + metrics = repo.list(category=category) + return {"metrics": metrics, "count": len(metrics)} + + +@router.get("/api/metrics/{metric_id:path}") +async def get_metric( + metric_id: str, + user: dict = Depends(get_current_user), + conn: duckdb.DuckDBPyConnection = Depends(_get_db), +): + """Get a single metric definition by ID.""" + repo = MetricRepository(conn) + metric = repo.get(metric_id) + if metric is None: + raise HTTPException(status_code=404, detail=f"Metric '{metric_id}' not found") + return metric + + +@router.post("/api/admin/metrics", status_code=201) +async def create_or_update_metric( + body: MetricCreate, + user: dict = Depends(require_admin), + conn: duckdb.DuckDBPyConnection = Depends(_get_db), +): + """Create or update a metric definition (admin only).""" + repo = MetricRepository(conn) + metric = repo.create( + id=body.id, + name=body.name, + display_name=body.display_name, + category=body.category, + sql=body.sql, + description=body.description, + type=body.type, + unit=body.unit, + grain=body.grain, + table_name=body.table_name, + tables=body.tables, + expression=body.expression, + time_column=body.time_column, + dimensions=body.dimensions, + filters=body.filters, + synonyms=body.synonyms, + notes=body.notes, + sql_variants=body.sql_variants, + validation=body.validation, + source=body.source, + ) + return metric + + +@router.delete("/api/admin/metrics/{metric_id:path}") +async def delete_metric( + metric_id: str, + user: dict = Depends(require_admin), + conn: duckdb.DuckDBPyConnection = Depends(_get_db), +): + """Delete a metric definition by ID (admin only).""" + repo = MetricRepository(conn) + deleted = repo.delete(metric_id) + if not deleted: + raise HTTPException(status_code=404, detail=f"Metric '{metric_id}' not found") + return {"status": "deleted", "id": metric_id} diff --git a/app/auth/dependencies.py b/app/auth/dependencies.py index 088b224..22101fc 100644 --- a/app/auth/dependencies.py +++ b/app/auth/dependencies.py @@ -80,3 +80,13 @@ def require_role(minimum_role: Role): ) return user return _check + + +async def require_admin(user: dict = Depends(get_current_user)) -> dict: + """Dependency: require user is an admin. Raises 403 otherwise.""" + if user.get("role") != "admin": + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Admin access required", + ) + return user diff --git a/app/main.py b/app/main.py index c2652b0..fc7eb04 100644 --- a/app/main.py +++ b/app/main.py @@ -27,6 +27,7 @@ from app.api.admin import router as admin_router from app.api.permissions import router as permissions_router from app.api.access_requests import router as access_requests_router from app.api.jira_webhooks import router as jira_webhooks_router +from app.api.metrics import router as metrics_router from app.web.router import router as web_router logger = logging.getLogger(__name__) @@ -133,6 +134,7 @@ def create_app() -> FastAPI: app.include_router(permissions_router) app.include_router(access_requests_router) app.include_router(jira_webhooks_router) + app.include_router(metrics_router) # Web UI router (must be last — has catch-all routes) app.include_router(web_router) diff --git a/tests/test_api.py b/tests/test_api.py index e1b407f..4a586d1 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -233,3 +233,99 @@ class TestUpload: ) assert resp.status_code == 200 assert resp.json()["user"] == "analyst@acme.com" + + +# ---- Metrics API ---- + +SAMPLE_METRIC = { + "id": "finance/mrr", + "name": "mrr", + "display_name": "Monthly Recurring Revenue", + "category": "finance", + "sql": "SELECT SUM(amount) FROM subscriptions WHERE active = true", +} + + +class TestMetricsAPI: + def test_list_metrics_empty(self, seeded_client): + client, admin_token, _ = seeded_client + headers = {"Authorization": f"Bearer {admin_token}"} + resp = client.get("/api/metrics", headers=headers) + assert resp.status_code == 200 + data = resp.json() + assert data["count"] == 0 + assert data["metrics"] == [] + + def test_create_and_list_metric(self, seeded_client): + client, admin_token, _ = seeded_client + headers = {"Authorization": f"Bearer {admin_token}"} + + resp = client.post("/api/admin/metrics", json=SAMPLE_METRIC, headers=headers) + assert resp.status_code == 201 + + resp = client.get("/api/metrics", headers=headers) + assert resp.status_code == 200 + data = resp.json() + assert data["count"] == 1 + assert data["metrics"][0]["id"] == "finance/mrr" + + def test_get_metric_detail(self, seeded_client): + client, admin_token, _ = seeded_client + headers = {"Authorization": f"Bearer {admin_token}"} + + client.post("/api/admin/metrics", json=SAMPLE_METRIC, headers=headers) + + resp = client.get("/api/metrics/finance/mrr", headers=headers) + assert resp.status_code == 200 + data = resp.json() + assert data["id"] == "finance/mrr" + assert data["display_name"] == "Monthly Recurring Revenue" + assert data["category"] == "finance" + + def test_get_metric_not_found(self, seeded_client): + client, admin_token, _ = seeded_client + headers = {"Authorization": f"Bearer {admin_token}"} + + resp = client.get("/api/metrics/nonexistent/metric", headers=headers) + assert resp.status_code == 404 + + def test_delete_metric(self, seeded_client): + client, admin_token, _ = seeded_client + headers = {"Authorization": f"Bearer {admin_token}"} + + client.post("/api/admin/metrics", json=SAMPLE_METRIC, headers=headers) + + resp = client.delete("/api/admin/metrics/finance/mrr", headers=headers) + assert resp.status_code == 200 + assert resp.json()["status"] == "deleted" + + resp = client.get("/api/metrics/finance/mrr", headers=headers) + assert resp.status_code == 404 + + def test_analyst_cannot_create_metric(self, seeded_client): + client, _, analyst_token = seeded_client + headers = {"Authorization": f"Bearer {analyst_token}"} + + resp = client.post("/api/admin/metrics", json=SAMPLE_METRIC, headers=headers) + assert resp.status_code == 403 + + def test_list_metrics_filter_by_category(self, seeded_client): + client, admin_token, _ = seeded_client + headers = {"Authorization": f"Bearer {admin_token}"} + + finance_metric = {**SAMPLE_METRIC} + support_metric = { + "id": "support/tickets", + "name": "tickets", + "display_name": "Open Tickets", + "category": "support", + "sql": "SELECT COUNT(*) FROM tickets WHERE status = 'open'", + } + client.post("/api/admin/metrics", json=finance_metric, headers=headers) + client.post("/api/admin/metrics", json=support_metric, headers=headers) + + resp = client.get("/api/metrics?category=finance", headers=headers) + assert resp.status_code == 200 + data = resp.json() + assert data["count"] == 1 + assert data["metrics"][0]["category"] == "finance"