feat: add circuitforge_core.api.feedback — shared feedback router factory (closes #23)
Adds make_feedback_router(repo, product, demo_mode_fn) which returns a FastAPI APIRouter with GET /status and POST / endpoints. Handles Forgejo label creation/reuse, issue body assembly (including repro steps for bugs), demo mode gating, and FORGEJO_API_TOKEN presence checks. 12 tests covering all status/submit paths, mock Forgejo interaction, and body content assertions. Also adds fastapi>=0.110 and httpx>=0.27 to [dev] optional deps.
This commit is contained in:
parent
c244260d1c
commit
0a15ad9522
5 changed files with 474 additions and 0 deletions
0
circuitforge_core/api/__init__.py
Normal file
0
circuitforge_core/api/__init__.py
Normal file
174
circuitforge_core/api/feedback.py
Normal file
174
circuitforge_core/api/feedback.py
Normal file
|
|
@ -0,0 +1,174 @@
|
|||
"""
|
||||
Shared feedback router — creates Forgejo issues from in-app beta feedback.
|
||||
Products include this with make_feedback_router(repo=..., product=...).
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import platform
|
||||
import subprocess
|
||||
from collections.abc import Callable
|
||||
from datetime import datetime, timezone
|
||||
from pathlib import Path
|
||||
from typing import Literal
|
||||
|
||||
import requests
|
||||
from fastapi import APIRouter, HTTPException
|
||||
from pydantic import BaseModel
|
||||
|
||||
_LABEL_COLORS: dict[str, str] = {
|
||||
"beta-feedback": "#0075ca",
|
||||
"needs-triage": "#e4e669",
|
||||
"bug": "#d73a4a",
|
||||
"feature-request": "#a2eeef",
|
||||
"question": "#d876e3",
|
||||
}
|
||||
|
||||
_TYPE_LABEL_MAP: dict[str, str] = {"bug": "bug", "feature": "feature-request"}
|
||||
_TYPE_DISPLAY: dict[str, str] = {
|
||||
"bug": "🐛 Bug",
|
||||
"feature": "✨ Feature Request",
|
||||
"other": "💬 Other",
|
||||
}
|
||||
|
||||
|
||||
class FeedbackRequest(BaseModel):
|
||||
title: str
|
||||
description: str
|
||||
type: Literal["bug", "feature", "other"] = "other"
|
||||
repro: str = ""
|
||||
tab: str = "unknown"
|
||||
submitter: str = ""
|
||||
|
||||
|
||||
class FeedbackResponse(BaseModel):
|
||||
issue_number: int
|
||||
issue_url: str
|
||||
|
||||
|
||||
def _forgejo_headers() -> dict[str, str]:
|
||||
token = os.environ.get("FORGEJO_API_TOKEN", "")
|
||||
return {"Authorization": f"token {token}", "Content-Type": "application/json"}
|
||||
|
||||
|
||||
def _ensure_labels(label_names: list[str], base: str, repo: str) -> list[int]:
|
||||
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,
|
||||
)
|
||||
if r.ok:
|
||||
ids.append(r.json()["id"])
|
||||
return ids
|
||||
|
||||
|
||||
def _collect_context(tab: str, product: str) -> dict[str, str]:
|
||||
try:
|
||||
version = subprocess.check_output(
|
||||
["git", "describe", "--tags", "--always"],
|
||||
cwd=Path.cwd(),
|
||||
text=True,
|
||||
timeout=5,
|
||||
).strip()
|
||||
except Exception:
|
||||
version = "dev"
|
||||
return {
|
||||
"product": product,
|
||||
"tab": tab,
|
||||
"version": version,
|
||||
"platform": platform.platform(),
|
||||
"timestamp": datetime.now(timezone.utc).isoformat().replace("+00:00", "Z"),
|
||||
}
|
||||
|
||||
|
||||
def _build_issue_body(payload: FeedbackRequest, context: dict[str, str]) -> str:
|
||||
lines: list[str] = [
|
||||
f"## {_TYPE_DISPLAY.get(payload.type, '💬 Other')}",
|
||||
"",
|
||||
payload.description,
|
||||
"",
|
||||
]
|
||||
if payload.type == "bug" and payload.repro:
|
||||
lines += ["### Reproduction Steps", "", payload.repro, ""]
|
||||
lines += ["### Context", ""]
|
||||
for k, v in context.items():
|
||||
lines.append(f"- **{k}:** {v}")
|
||||
lines.append("")
|
||||
if payload.submitter:
|
||||
lines += ["---", f"*Submitted by: {payload.submitter}*"]
|
||||
return "\n".join(lines)
|
||||
|
||||
|
||||
def make_feedback_router(
|
||||
repo: str,
|
||||
product: str,
|
||||
demo_mode_fn: Callable[[], bool] | None = None,
|
||||
) -> APIRouter:
|
||||
"""Return a configured feedback APIRouter for the given Forgejo repo and product.
|
||||
|
||||
Args:
|
||||
repo: Forgejo repo slug, e.g. "Circuit-Forge/kiwi".
|
||||
product: Product name injected into issue context, e.g. "kiwi".
|
||||
demo_mode_fn: Optional callable returning True when in demo mode.
|
||||
If None, reads the DEMO_MODE environment variable.
|
||||
"""
|
||||
|
||||
def _is_demo() -> bool:
|
||||
if demo_mode_fn is not None:
|
||||
return demo_mode_fn()
|
||||
return os.environ.get("DEMO_MODE", "").lower() in ("1", "true", "yes")
|
||||
|
||||
router = APIRouter()
|
||||
|
||||
@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 _is_demo()}
|
||||
|
||||
@router.post("", response_model=FeedbackResponse)
|
||||
def submit_feedback(payload: FeedbackRequest) -> FeedbackResponse:
|
||||
"""File a Forgejo issue from in-app feedback."""
|
||||
token = os.environ.get("FORGEJO_API_TOKEN", "")
|
||||
if not token:
|
||||
raise HTTPException(
|
||||
status_code=503,
|
||||
detail="Feedback disabled: FORGEJO_API_TOKEN not configured.",
|
||||
)
|
||||
if _is_demo():
|
||||
raise HTTPException(status_code=403, detail="Feedback disabled in demo mode.")
|
||||
|
||||
base = os.environ.get(
|
||||
"FORGEJO_API_URL", "https://git.opensourcesolarpunk.com/api/v1"
|
||||
)
|
||||
context = _collect_context(payload.tab, product)
|
||||
body = _build_issue_body(payload, context)
|
||||
labels = [
|
||||
"beta-feedback",
|
||||
"needs-triage",
|
||||
_TYPE_LABEL_MAP.get(payload.type, "question"),
|
||||
]
|
||||
label_ids = _ensure_labels(labels, base, repo)
|
||||
|
||||
resp = requests.post(
|
||||
f"{base}/repos/{repo}/issues",
|
||||
headers=_forgejo_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"])
|
||||
|
||||
return router
|
||||
|
|
@ -22,6 +22,8 @@ dev = [
|
|||
"circuitforge-core[manage]",
|
||||
"pytest>=8.0",
|
||||
"pytest-asyncio>=0.23",
|
||||
"fastapi>=0.110",
|
||||
"httpx>=0.27",
|
||||
]
|
||||
|
||||
[project.scripts]
|
||||
|
|
|
|||
0
tests/test_api/__init__.py
Normal file
0
tests/test_api/__init__.py
Normal file
298
tests/test_api/test_feedback.py
Normal file
298
tests/test_api/test_feedback.py
Normal file
|
|
@ -0,0 +1,298 @@
|
|||
"""Tests for circuitforge_core.api.feedback — shared feedback router factory."""
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from fastapi import FastAPI
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from circuitforge_core.api.feedback import make_feedback_router
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _make_client(
|
||||
demo_mode_fn=None,
|
||||
repo: str = "Circuit-Forge/test",
|
||||
product: str = "test",
|
||||
) -> TestClient:
|
||||
app = FastAPI()
|
||||
router = make_feedback_router(repo=repo, product=product, demo_mode_fn=demo_mode_fn)
|
||||
app.include_router(router, prefix="/feedback")
|
||||
return TestClient(app)
|
||||
|
||||
|
||||
def _mock_forgejo_get(label_list: list[dict] | None = None):
|
||||
"""Return a mock for requests.get that returns an empty label list."""
|
||||
mock = MagicMock()
|
||||
mock.ok = True
|
||||
mock.json.return_value = label_list or []
|
||||
return mock
|
||||
|
||||
|
||||
def _mock_forgejo_post_issue(number: int = 42, url: str = "https://example.com/issues/42"):
|
||||
"""Return a mock for requests.post that simulates a successful issue creation."""
|
||||
mock = MagicMock()
|
||||
mock.ok = True
|
||||
mock.json.return_value = {"number": number, "html_url": url}
|
||||
return mock
|
||||
|
||||
|
||||
_VALID_PAYLOAD = {
|
||||
"title": "Something broke",
|
||||
"description": "It stopped working after the update.",
|
||||
"type": "bug",
|
||||
"repro": "1. Open app\n2. Click submit\n3. See error",
|
||||
"tab": "dashboard",
|
||||
"submitter": "alan@example.com",
|
||||
}
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# GET /feedback/status
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_status_no_token_returns_disabled(monkeypatch):
|
||||
"""GET /status returns enabled=False when FORGEJO_API_TOKEN is not set."""
|
||||
monkeypatch.delenv("FORGEJO_API_TOKEN", raising=False)
|
||||
monkeypatch.delenv("DEMO_MODE", raising=False)
|
||||
client = _make_client()
|
||||
resp = client.get("/feedback/status")
|
||||
assert resp.status_code == 200
|
||||
assert resp.json() == {"enabled": False}
|
||||
|
||||
|
||||
def test_status_with_token_returns_enabled(monkeypatch):
|
||||
"""GET /status returns enabled=True when token is set and not in demo mode."""
|
||||
monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token-abc")
|
||||
monkeypatch.delenv("DEMO_MODE", raising=False)
|
||||
client = _make_client()
|
||||
resp = client.get("/feedback/status")
|
||||
assert resp.status_code == 200
|
||||
assert resp.json() == {"enabled": True}
|
||||
|
||||
|
||||
def test_status_demo_mode_env_returns_disabled(monkeypatch):
|
||||
"""GET /status returns enabled=False when DEMO_MODE=1 even with a token."""
|
||||
monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token-abc")
|
||||
monkeypatch.setenv("DEMO_MODE", "1")
|
||||
client = _make_client()
|
||||
resp = client.get("/feedback/status")
|
||||
assert resp.status_code == 200
|
||||
assert resp.json() == {"enabled": False}
|
||||
|
||||
|
||||
def test_status_demo_mode_fn_returns_disabled(monkeypatch):
|
||||
"""GET /status returns enabled=False when demo_mode_fn() returns True."""
|
||||
monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token-abc")
|
||||
monkeypatch.delenv("DEMO_MODE", raising=False)
|
||||
client = _make_client(demo_mode_fn=lambda: True)
|
||||
resp = client.get("/feedback/status")
|
||||
assert resp.status_code == 200
|
||||
assert resp.json() == {"enabled": False}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# POST /feedback
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_post_no_token_returns_503(monkeypatch):
|
||||
"""POST / returns 503 when FORGEJO_API_TOKEN is not configured."""
|
||||
monkeypatch.delenv("FORGEJO_API_TOKEN", raising=False)
|
||||
monkeypatch.delenv("DEMO_MODE", raising=False)
|
||||
client = _make_client()
|
||||
resp = client.post("/feedback", json=_VALID_PAYLOAD)
|
||||
assert resp.status_code == 503
|
||||
assert "FORGEJO_API_TOKEN" in resp.json()["detail"]
|
||||
|
||||
|
||||
def test_post_demo_mode_fn_returns_403(monkeypatch):
|
||||
"""POST / returns 403 when demo_mode_fn returns True."""
|
||||
monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token-abc")
|
||||
monkeypatch.delenv("DEMO_MODE", raising=False)
|
||||
client = _make_client(demo_mode_fn=lambda: True)
|
||||
resp = client.post("/feedback", json=_VALID_PAYLOAD)
|
||||
assert resp.status_code == 403
|
||||
assert "demo" in resp.json()["detail"].lower()
|
||||
|
||||
|
||||
def test_post_success_returns_issue_number_and_url(monkeypatch):
|
||||
"""POST / returns issue_number and issue_url on success."""
|
||||
monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token-abc")
|
||||
monkeypatch.delenv("DEMO_MODE", raising=False)
|
||||
monkeypatch.setenv("FORGEJO_API_URL", "https://forgejo.test/api/v1")
|
||||
|
||||
mock_get = _mock_forgejo_get()
|
||||
mock_post_label = MagicMock(ok=True)
|
||||
mock_post_label.json.return_value = {"id": 99, "name": "beta-feedback"}
|
||||
mock_post_issue = _mock_forgejo_post_issue(number=7, url="https://forgejo.test/Circuit-Forge/test/issues/7")
|
||||
|
||||
# requests.post is called multiple times: once per new label, then once for the issue.
|
||||
# We use side_effect to distinguish label creation calls from the issue creation call.
|
||||
post_calls = []
|
||||
|
||||
def post_side_effect(url, **kwargs):
|
||||
post_calls.append(url)
|
||||
if "/labels" in url:
|
||||
return mock_post_label
|
||||
return mock_post_issue
|
||||
|
||||
client = _make_client()
|
||||
with patch("circuitforge_core.api.feedback.requests.get", return_value=mock_get), \
|
||||
patch("circuitforge_core.api.feedback.requests.post", side_effect=post_side_effect):
|
||||
resp = client.post("/feedback", json=_VALID_PAYLOAD)
|
||||
|
||||
assert resp.status_code == 200
|
||||
data = resp.json()
|
||||
assert data["issue_number"] == 7
|
||||
assert data["issue_url"] == "https://forgejo.test/Circuit-Forge/test/issues/7"
|
||||
|
||||
|
||||
def test_post_forgejo_error_returns_502(monkeypatch):
|
||||
"""POST / returns 502 when Forgejo returns a non-ok response for issue creation."""
|
||||
monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token-abc")
|
||||
monkeypatch.delenv("DEMO_MODE", raising=False)
|
||||
monkeypatch.setenv("FORGEJO_API_URL", "https://forgejo.test/api/v1")
|
||||
|
||||
mock_get = _mock_forgejo_get()
|
||||
mock_issue_error = MagicMock(ok=False)
|
||||
mock_issue_error.text = "Internal Server Error"
|
||||
|
||||
def post_side_effect(url, **kwargs):
|
||||
if "/labels" in url:
|
||||
m = MagicMock(ok=True)
|
||||
m.json.return_value = {"id": 1, "name": "beta-feedback"}
|
||||
return m
|
||||
return mock_issue_error
|
||||
|
||||
client = _make_client()
|
||||
with patch("circuitforge_core.api.feedback.requests.get", return_value=mock_get), \
|
||||
patch("circuitforge_core.api.feedback.requests.post", side_effect=post_side_effect):
|
||||
resp = client.post("/feedback", json=_VALID_PAYLOAD)
|
||||
|
||||
assert resp.status_code == 502
|
||||
assert "Forgejo error" in resp.json()["detail"]
|
||||
|
||||
|
||||
def test_post_product_name_appears_in_issue_body(monkeypatch):
|
||||
"""The product name passed to make_feedback_router appears in the issue body context."""
|
||||
monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token-abc")
|
||||
monkeypatch.delenv("DEMO_MODE", raising=False)
|
||||
monkeypatch.setenv("FORGEJO_API_URL", "https://forgejo.test/api/v1")
|
||||
|
||||
captured_body: list[str] = []
|
||||
mock_get = _mock_forgejo_get()
|
||||
|
||||
def post_side_effect(url, **kwargs):
|
||||
if "/labels" in url:
|
||||
m = MagicMock(ok=True)
|
||||
m.json.return_value = {"id": 1, "name": "beta-feedback"}
|
||||
return m
|
||||
# Capture the body sent for the issue creation call
|
||||
captured_body.append(kwargs.get("json", {}).get("body", ""))
|
||||
m = MagicMock(ok=True)
|
||||
m.json.return_value = {"number": 1, "html_url": "https://forgejo.test/issues/1"}
|
||||
return m
|
||||
|
||||
client = _make_client(product="kiwi")
|
||||
with patch("circuitforge_core.api.feedback.requests.get", return_value=mock_get), \
|
||||
patch("circuitforge_core.api.feedback.requests.post", side_effect=post_side_effect):
|
||||
resp = client.post(
|
||||
"/feedback",
|
||||
json={
|
||||
"title": "Pantry bug",
|
||||
"description": "Items disappear.",
|
||||
"type": "bug",
|
||||
"tab": "pantry",
|
||||
},
|
||||
)
|
||||
|
||||
assert resp.status_code == 200
|
||||
assert captured_body, "No issue body was captured"
|
||||
assert "kiwi" in captured_body[0], f"Product name not found in body: {captured_body[0]}"
|
||||
|
||||
|
||||
def test_post_bug_with_repro_includes_repro_section(monkeypatch):
|
||||
"""A bug report with a repro string includes the Reproduction Steps section in the body."""
|
||||
monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token-abc")
|
||||
monkeypatch.delenv("DEMO_MODE", raising=False)
|
||||
monkeypatch.setenv("FORGEJO_API_URL", "https://forgejo.test/api/v1")
|
||||
|
||||
captured_body: list[str] = []
|
||||
mock_get = _mock_forgejo_get()
|
||||
|
||||
def post_side_effect(url, **kwargs):
|
||||
if "/labels" in url:
|
||||
m = MagicMock(ok=True)
|
||||
m.json.return_value = {"id": 1, "name": "bug"}
|
||||
return m
|
||||
captured_body.append(kwargs.get("json", {}).get("body", ""))
|
||||
m = MagicMock(ok=True)
|
||||
m.json.return_value = {"number": 2, "html_url": "https://forgejo.test/issues/2"}
|
||||
return m
|
||||
|
||||
repro_text = "1. Open the app\n2. Click the button\n3. Observe crash"
|
||||
client = _make_client()
|
||||
with patch("circuitforge_core.api.feedback.requests.get", return_value=mock_get), \
|
||||
patch("circuitforge_core.api.feedback.requests.post", side_effect=post_side_effect):
|
||||
resp = client.post(
|
||||
"/feedback",
|
||||
json={
|
||||
"title": "App crashes",
|
||||
"description": "The app crashes on button click.",
|
||||
"type": "bug",
|
||||
"repro": repro_text,
|
||||
"tab": "home",
|
||||
},
|
||||
)
|
||||
|
||||
assert resp.status_code == 200
|
||||
assert captured_body, "No issue body was captured"
|
||||
body = captured_body[0]
|
||||
assert "Reproduction Steps" in body, f"'Reproduction Steps' not found in body: {body}"
|
||||
assert repro_text in body, f"Repro text not found in body: {body}"
|
||||
|
||||
|
||||
def test_status_demo_mode_env_true_string(monkeypatch):
|
||||
"""GET /status treats DEMO_MODE=true as demo mode."""
|
||||
monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token-abc")
|
||||
monkeypatch.setenv("DEMO_MODE", "true")
|
||||
client = _make_client()
|
||||
resp = client.get("/feedback/status")
|
||||
assert resp.status_code == 200
|
||||
assert resp.json() == {"enabled": False}
|
||||
|
||||
|
||||
def test_post_existing_labels_reused(monkeypatch):
|
||||
"""When labels already exist on Forgejo, their IDs are reused (no POST to /labels)."""
|
||||
monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token-abc")
|
||||
monkeypatch.delenv("DEMO_MODE", raising=False)
|
||||
monkeypatch.setenv("FORGEJO_API_URL", "https://forgejo.test/api/v1")
|
||||
|
||||
existing_labels = [
|
||||
{"name": "beta-feedback", "id": 10},
|
||||
{"name": "needs-triage", "id": 11},
|
||||
{"name": "bug", "id": 12},
|
||||
]
|
||||
mock_get = _mock_forgejo_get(existing_labels)
|
||||
label_post_calls: list[str] = []
|
||||
|
||||
def post_side_effect(url, **kwargs):
|
||||
if "/labels" in url:
|
||||
label_post_calls.append(url)
|
||||
m = MagicMock(ok=True)
|
||||
m.json.return_value = {"id": 99, "name": "new-label"}
|
||||
return m
|
||||
m = MagicMock(ok=True)
|
||||
m.json.return_value = {"number": 5, "html_url": "https://forgejo.test/issues/5"}
|
||||
return m
|
||||
|
||||
client = _make_client()
|
||||
with patch("circuitforge_core.api.feedback.requests.get", return_value=mock_get), \
|
||||
patch("circuitforge_core.api.feedback.requests.post", side_effect=post_side_effect):
|
||||
resp = client.post("/feedback", json=_VALID_PAYLOAD)
|
||||
|
||||
assert resp.status_code == 200
|
||||
assert label_post_calls == [], "Should not POST to /labels when all labels already exist"
|
||||
Loading…
Reference in a new issue