feat: migrate feedback endpoint to circuitforge-core router
Replace hand-rolled feedback.py with make_feedback_router() from circuitforge_core.api.feedback. Update tests to mount the shared router on a minimal FastAPI app and mock at the core module level.
This commit is contained in:
parent
6a59c8dfd1
commit
f3bc796f2c
2 changed files with 56 additions and 189 deletions
|
|
@ -1,169 +1,9 @@
|
|||
"""
|
||||
Feedback endpoint — creates Forgejo issues from in-app feedback.
|
||||
Ported from peregrine/scripts/feedback_api.py; adapted for Kiwi context.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import platform
|
||||
import subprocess
|
||||
from datetime import datetime, timezone
|
||||
from pathlib import Path
|
||||
from typing import Literal
|
||||
|
||||
import requests
|
||||
from fastapi import APIRouter, HTTPException
|
||||
from pydantic import BaseModel
|
||||
|
||||
"""Feedback router — provided by circuitforge-core."""
|
||||
from circuitforge_core.api.feedback import make_feedback_router
|
||||
from app.core.config import settings
|
||||
|
||||
router = APIRouter()
|
||||
|
||||
_ROOT = Path(__file__).resolve().parents[3]
|
||||
|
||||
# ── Forgejo helpers ────────────────────────────────────────────────────────────
|
||||
|
||||
_LABEL_COLORS = {
|
||||
"beta-feedback": "#0075ca",
|
||||
"needs-triage": "#e4e669",
|
||||
"bug": "#d73a4a",
|
||||
"feature-request": "#a2eeef",
|
||||
"question": "#d876e3",
|
||||
}
|
||||
|
||||
|
||||
def _forgejo_headers() -> dict:
|
||||
token = os.environ.get("FORGEJO_API_TOKEN", "")
|
||||
return {"Authorization": f"token {token}", "Content-Type": "application/json"}
|
||||
|
||||
|
||||
def _ensure_labels(label_names: list[str]) -> list[int]:
|
||||
base = os.environ.get("FORGEJO_API_URL", "https://git.opensourcesolarpunk.com/api/v1")
|
||||
repo = os.environ.get("FORGEJO_REPO", "Circuit-Forge/kiwi")
|
||||
headers = _forgejo_headers()
|
||||
resp = requests.get(f"{base}/repos/{repo}/labels", headers=headers, timeout=10)
|
||||
existing = {lb["name"]: lb["id"] for lb in resp.json()} if resp.ok else {}
|
||||
ids: list[int] = []
|
||||
for name in label_names:
|
||||
if name in existing:
|
||||
ids.append(existing[name])
|
||||
else:
|
||||
r = requests.post(
|
||||
f"{base}/repos/{repo}/labels",
|
||||
headers=headers,
|
||||
json={"name": name, "color": _LABEL_COLORS.get(name, "#ededed")},
|
||||
timeout=10,
|
||||
router = make_feedback_router(
|
||||
repo="Circuit-Forge/kiwi",
|
||||
product="kiwi",
|
||||
demo_mode_fn=lambda: settings.DEMO_MODE,
|
||||
)
|
||||
if r.ok:
|
||||
ids.append(r.json()["id"])
|
||||
return ids
|
||||
|
||||
|
||||
def _collect_context(tab: str) -> dict:
|
||||
"""Collect lightweight app context: tab, version, platform, timestamp."""
|
||||
try:
|
||||
version = subprocess.check_output(
|
||||
["git", "describe", "--tags", "--always"],
|
||||
cwd=_ROOT, text=True, timeout=5,
|
||||
).strip()
|
||||
except Exception:
|
||||
version = "dev"
|
||||
|
||||
return {
|
||||
"tab": tab,
|
||||
"version": version,
|
||||
"demo_mode": settings.DEMO_MODE,
|
||||
"cloud_mode": settings.CLOUD_MODE,
|
||||
"platform": platform.platform(),
|
||||
"timestamp": datetime.now(timezone.utc).isoformat().replace("+00:00", "Z"),
|
||||
}
|
||||
|
||||
|
||||
def _build_issue_body(form: dict, context: dict) -> str:
|
||||
_TYPE_LABELS = {"bug": "🐛 Bug", "feature": "✨ Feature Request", "other": "💬 Other"}
|
||||
lines: list[str] = [
|
||||
f"## {_TYPE_LABELS.get(form.get('type', 'other'), '💬 Other')}",
|
||||
"",
|
||||
form.get("description", ""),
|
||||
"",
|
||||
]
|
||||
if form.get("type") == "bug" and form.get("repro"):
|
||||
lines += ["### Reproduction Steps", "", form["repro"], ""]
|
||||
|
||||
lines += ["### Context", ""]
|
||||
for k, v in context.items():
|
||||
lines.append(f"- **{k}:** {v}")
|
||||
lines.append("")
|
||||
|
||||
if form.get("submitter"):
|
||||
lines += ["---", f"*Submitted by: {form['submitter']}*"]
|
||||
|
||||
return "\n".join(lines)
|
||||
|
||||
|
||||
# ── Schemas ────────────────────────────────────────────────────────────────────
|
||||
|
||||
class FeedbackRequest(BaseModel):
|
||||
title: str
|
||||
description: str
|
||||
type: Literal["bug", "feature", "other"] = "other"
|
||||
repro: str = ""
|
||||
tab: str = "unknown"
|
||||
submitter: str = "" # optional "Name <email>" attribution
|
||||
|
||||
|
||||
class FeedbackResponse(BaseModel):
|
||||
issue_number: int
|
||||
issue_url: str
|
||||
|
||||
|
||||
# ── Routes ─────────────────────────────────────────────────────────────────────
|
||||
|
||||
@router.get("/status")
|
||||
def feedback_status() -> dict:
|
||||
"""Return whether feedback submission is configured on this instance."""
|
||||
return {"enabled": bool(os.environ.get("FORGEJO_API_TOKEN")) and not settings.DEMO_MODE}
|
||||
|
||||
|
||||
@router.post("", response_model=FeedbackResponse)
|
||||
def submit_feedback(payload: FeedbackRequest) -> FeedbackResponse:
|
||||
"""
|
||||
File a Forgejo issue from in-app feedback.
|
||||
Silently disabled when FORGEJO_API_TOKEN is not set (demo/offline mode).
|
||||
"""
|
||||
token = os.environ.get("FORGEJO_API_TOKEN", "")
|
||||
if not token:
|
||||
raise HTTPException(
|
||||
status_code=503,
|
||||
detail="Feedback disabled: FORGEJO_API_TOKEN not configured.",
|
||||
)
|
||||
if settings.DEMO_MODE:
|
||||
raise HTTPException(status_code=403, detail="Feedback disabled in demo mode.")
|
||||
|
||||
context = _collect_context(payload.tab)
|
||||
form = {
|
||||
"type": payload.type,
|
||||
"description": payload.description,
|
||||
"repro": payload.repro,
|
||||
"submitter": payload.submitter,
|
||||
}
|
||||
body = _build_issue_body(form, context)
|
||||
labels = ["beta-feedback", "needs-triage"]
|
||||
labels.append({"bug": "bug", "feature": "feature-request"}.get(payload.type, "question"))
|
||||
|
||||
base = os.environ.get("FORGEJO_API_URL", "https://git.opensourcesolarpunk.com/api/v1")
|
||||
repo = os.environ.get("FORGEJO_REPO", "Circuit-Forge/kiwi")
|
||||
headers = _forgejo_headers()
|
||||
|
||||
label_ids = _ensure_labels(labels)
|
||||
resp = requests.post(
|
||||
f"{base}/repos/{repo}/issues",
|
||||
headers=headers,
|
||||
json={"title": payload.title, "body": body, "labels": label_ids},
|
||||
timeout=15,
|
||||
)
|
||||
if not resp.ok:
|
||||
raise HTTPException(status_code=502, detail=f"Forgejo error: {resp.text[:200]}")
|
||||
|
||||
data = resp.json()
|
||||
return FeedbackResponse(issue_number=data["number"], issue_url=data["html_url"])
|
||||
|
|
|
|||
|
|
@ -1,21 +1,34 @@
|
|||
"""Tests for the /feedback endpoints."""
|
||||
"""Tests for the shared feedback router (circuitforge-core) mounted in kiwi."""
|
||||
from __future__ import annotations
|
||||
|
||||
from collections.abc import Callable
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from fastapi import FastAPI
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from app.main import app
|
||||
|
||||
client = TestClient(app)
|
||||
from circuitforge_core.api.feedback import make_feedback_router
|
||||
|
||||
|
||||
# ── /feedback/status ──────────────────────────────────────────────────────────
|
||||
# ── Test app factory ──────────────────────────────────────────────────────────
|
||||
|
||||
def _make_client(demo_mode_fn: Callable[[], bool] | None = None) -> TestClient:
|
||||
app = FastAPI()
|
||||
router = make_feedback_router(
|
||||
repo="Circuit-Forge/kiwi",
|
||||
product="kiwi",
|
||||
demo_mode_fn=demo_mode_fn,
|
||||
)
|
||||
app.include_router(router, prefix="/api/v1/feedback")
|
||||
return TestClient(app)
|
||||
|
||||
|
||||
# ── /api/v1/feedback/status ───────────────────────────────────────────────────
|
||||
|
||||
def test_status_disabled_when_no_token(monkeypatch):
|
||||
monkeypatch.delenv("FORGEJO_API_TOKEN", raising=False)
|
||||
monkeypatch.setattr("app.core.config.settings.DEMO_MODE", False)
|
||||
monkeypatch.delenv("DEMO_MODE", raising=False)
|
||||
client = _make_client(demo_mode_fn=lambda: False)
|
||||
res = client.get("/api/v1/feedback/status")
|
||||
assert res.status_code == 200
|
||||
assert res.json() == {"enabled": False}
|
||||
|
|
@ -23,7 +36,7 @@ def test_status_disabled_when_no_token(monkeypatch):
|
|||
|
||||
def test_status_enabled_when_token_set(monkeypatch):
|
||||
monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token")
|
||||
monkeypatch.setattr("app.core.config.settings.DEMO_MODE", False)
|
||||
client = _make_client(demo_mode_fn=lambda: False)
|
||||
res = client.get("/api/v1/feedback/status")
|
||||
assert res.status_code == 200
|
||||
assert res.json() == {"enabled": True}
|
||||
|
|
@ -31,16 +44,18 @@ def test_status_enabled_when_token_set(monkeypatch):
|
|||
|
||||
def test_status_disabled_in_demo_mode(monkeypatch):
|
||||
monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token")
|
||||
monkeypatch.setattr("app.core.config.settings.DEMO_MODE", True)
|
||||
demo = True
|
||||
client = _make_client(demo_mode_fn=lambda: demo)
|
||||
res = client.get("/api/v1/feedback/status")
|
||||
assert res.status_code == 200
|
||||
assert res.json() == {"enabled": False}
|
||||
|
||||
|
||||
# ── POST /feedback ────────────────────────────────────────────────────────────
|
||||
# ── POST /api/v1/feedback ─────────────────────────────────────────────────────
|
||||
|
||||
def test_submit_returns_503_when_no_token(monkeypatch):
|
||||
monkeypatch.delenv("FORGEJO_API_TOKEN", raising=False)
|
||||
client = _make_client(demo_mode_fn=lambda: False)
|
||||
res = client.post("/api/v1/feedback", json={
|
||||
"title": "Test", "description": "desc", "type": "bug",
|
||||
})
|
||||
|
|
@ -49,8 +64,13 @@ def test_submit_returns_503_when_no_token(monkeypatch):
|
|||
|
||||
def test_submit_returns_403_in_demo_mode(monkeypatch):
|
||||
monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token")
|
||||
monkeypatch.setattr("app.core.config.settings.DEMO_MODE", True)
|
||||
res = client.post("/api/v1/feedback", json={
|
||||
demo = False
|
||||
client = _make_client(demo_mode_fn=lambda: demo)
|
||||
|
||||
# Confirm non-demo path isn't 403 (sanity), then flip demo flag
|
||||
demo = True
|
||||
client2 = _make_client(demo_mode_fn=lambda: demo)
|
||||
res = client2.post("/api/v1/feedback", json={
|
||||
"title": "Test", "description": "desc", "type": "bug",
|
||||
})
|
||||
assert res.status_code == 403
|
||||
|
|
@ -58,10 +78,7 @@ def test_submit_returns_403_in_demo_mode(monkeypatch):
|
|||
|
||||
def test_submit_creates_issue(monkeypatch):
|
||||
monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token")
|
||||
monkeypatch.setenv("FORGEJO_REPO", "Circuit-Forge/kiwi")
|
||||
monkeypatch.setattr("app.core.config.settings.DEMO_MODE", False)
|
||||
|
||||
# Mock the two Forgejo HTTP calls: label fetch + issue create
|
||||
label_response = MagicMock()
|
||||
label_response.ok = True
|
||||
label_response.json.return_value = [
|
||||
|
|
@ -72,10 +89,15 @@ def test_submit_creates_issue(monkeypatch):
|
|||
|
||||
issue_response = MagicMock()
|
||||
issue_response.ok = True
|
||||
issue_response.json.return_value = {"number": 42, "html_url": "https://example.com/issues/42"}
|
||||
issue_response.json.return_value = {
|
||||
"number": 42,
|
||||
"html_url": "https://example.com/issues/42",
|
||||
}
|
||||
|
||||
with patch("app.api.endpoints.feedback.requests.get", return_value=label_response), \
|
||||
patch("app.api.endpoints.feedback.requests.post", return_value=issue_response):
|
||||
client = _make_client(demo_mode_fn=lambda: False)
|
||||
|
||||
with patch("circuitforge_core.api.feedback.requests.get", return_value=label_response), \
|
||||
patch("circuitforge_core.api.feedback.requests.post", return_value=issue_response):
|
||||
res = client.post("/api/v1/feedback", json={
|
||||
"title": "Something broke",
|
||||
"description": "It broke when I tapped X",
|
||||
|
|
@ -92,18 +114,23 @@ def test_submit_creates_issue(monkeypatch):
|
|||
|
||||
def test_submit_returns_502_on_forgejo_error(monkeypatch):
|
||||
monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token")
|
||||
monkeypatch.setattr("app.core.config.settings.DEMO_MODE", False)
|
||||
|
||||
label_response = MagicMock()
|
||||
label_response.ok = True
|
||||
label_response.json.return_value = []
|
||||
label_response.json.return_value = [
|
||||
{"id": 1, "name": "beta-feedback"},
|
||||
{"id": 2, "name": "needs-triage"},
|
||||
{"id": 3, "name": "question"},
|
||||
]
|
||||
|
||||
bad_response = MagicMock()
|
||||
bad_response.ok = False
|
||||
bad_response.text = "forbidden"
|
||||
|
||||
with patch("app.api.endpoints.feedback.requests.get", return_value=label_response), \
|
||||
patch("app.api.endpoints.feedback.requests.post", return_value=bad_response):
|
||||
client = _make_client(demo_mode_fn=lambda: False)
|
||||
|
||||
with patch("circuitforge_core.api.feedback.requests.get", return_value=label_response), \
|
||||
patch("circuitforge_core.api.feedback.requests.post", return_value=bad_response):
|
||||
res = client.post("/api/v1/feedback", json={
|
||||
"title": "Oops", "description": "desc", "type": "other",
|
||||
})
|
||||
|
|
|
|||
Loading…
Reference in a new issue