feat(03-03): wire get_current_user into /api/topics/*; add load_topics_for_user; POST /api/admin/topics
- api/topics.py: add get_current_user dep to all 5 handlers (list, create, update, delete, suggest) - list_topics: uses load_topics_for_user (system topics + user's own) with user-scoped doc counts - create_topic: passes user_id=current_user.id (never creates system topics via regular endpoint) - update_topic/delete_topic: ownership assertion — system topics and other users' topics return 404 - api/admin.py: add SystemTopicCreate model + POST /api/admin/topics (user_id=NULL, admin-only) - services/storage.py: add or_ import; load_topics_for_user (D-17); create_topic gains user_id param with namespace-scoped dedup; topic_doc_counts gains optional user_id for user-scoped counts; add load_topics_for_user to __all__ - services/classifier.py: replace load_topics with load_topics_for_user(doc.user_id); pass user_id=doc.user_id to create_topic for AI-suggested topics (D-11) - Tests: update all topic tests to pass auth headers; implement test_topic_namespace, test_admin_create_system_topic, test_regular_user_cannot_create_system_topic, test_topics_require_auth
This commit is contained in:
curo1305
@@ -3,22 +3,26 @@ Topics API tests — async only (Plan 05 cutover).
|
||||
|
||||
Legacy sync tests (using the flat-file storage layer and sync TestClient) were
|
||||
updated to async in Plan 05 to match the new session-injected API routes.
|
||||
|
||||
Updated in Plan 03-03: all endpoints now require authentication. Existing tests
|
||||
updated to pass auth_user headers. Namespace isolation tests implemented.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
async def test_list_topics_empty(async_client):
|
||||
resp = await async_client.get("/api/topics")
|
||||
async def test_list_topics_empty(async_client, auth_user):
|
||||
resp = await async_client.get("/api/topics", headers=auth_user["headers"])
|
||||
assert resp.status_code == 200
|
||||
assert resp.json()["topics"] == []
|
||||
|
||||
|
||||
async def test_create_topic(async_client):
|
||||
async def test_create_topic(async_client, auth_user):
|
||||
resp = await async_client.post(
|
||||
"/api/topics",
|
||||
json={"name": "Finance", "description": "Financial docs", "color": "#ff0000"},
|
||||
headers=auth_user["headers"],
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
data = resp.json()
|
||||
@@ -27,44 +31,46 @@ async def test_create_topic(async_client):
|
||||
assert "id" in data
|
||||
|
||||
|
||||
async def test_create_topic_deduplication(async_client):
|
||||
await async_client.post("/api/topics", json={"name": "Finance"})
|
||||
resp = await async_client.post("/api/topics", json={"name": "finance"}) # case-insensitive
|
||||
async def test_create_topic_deduplication(async_client, auth_user):
|
||||
await async_client.post("/api/topics", json={"name": "Finance"}, headers=auth_user["headers"])
|
||||
resp = await async_client.post("/api/topics", json={"name": "finance"}, headers=auth_user["headers"]) # case-insensitive
|
||||
assert resp.status_code == 200
|
||||
topics = (await async_client.get("/api/topics")).json()["topics"]
|
||||
topics = (await async_client.get("/api/topics", headers=auth_user["headers"])).json()["topics"]
|
||||
assert len(topics) == 1
|
||||
|
||||
|
||||
async def test_update_topic(async_client):
|
||||
create = (await async_client.post("/api/topics", json={"name": "Old Name"})).json()
|
||||
resp = await async_client.patch(f"/api/topics/{create['id']}", json={"name": "New Name"})
|
||||
async def test_update_topic(async_client, auth_user):
|
||||
create = (await async_client.post("/api/topics", json={"name": "Old Name"}, headers=auth_user["headers"])).json()
|
||||
resp = await async_client.patch(f"/api/topics/{create['id']}", json={"name": "New Name"}, headers=auth_user["headers"])
|
||||
assert resp.status_code == 200
|
||||
assert resp.json()["name"] == "New Name"
|
||||
|
||||
|
||||
async def test_update_topic_not_found(async_client):
|
||||
async def test_update_topic_not_found(async_client, auth_user):
|
||||
resp = await async_client.patch(
|
||||
"/api/topics/00000000-0000-0000-0000-000000000000",
|
||||
json={"name": "X"},
|
||||
headers=auth_user["headers"],
|
||||
)
|
||||
assert resp.status_code == 404
|
||||
|
||||
|
||||
async def test_delete_topic(async_client):
|
||||
create = (await async_client.post("/api/topics", json={"name": "ToDelete"})).json()
|
||||
resp = await async_client.delete(f"/api/topics/{create['id']}")
|
||||
async def test_delete_topic(async_client, auth_user):
|
||||
create = (await async_client.post("/api/topics", json={"name": "ToDelete"}, headers=auth_user["headers"])).json()
|
||||
resp = await async_client.delete(f"/api/topics/{create['id']}", headers=auth_user["headers"])
|
||||
assert resp.status_code == 200
|
||||
assert resp.json()["success"] is True
|
||||
|
||||
topics = (await async_client.get("/api/topics")).json()["topics"]
|
||||
topics = (await async_client.get("/api/topics", headers=auth_user["headers"])).json()["topics"]
|
||||
assert not any(t["name"] == "ToDelete" for t in topics)
|
||||
|
||||
|
||||
async def test_delete_topic_cascades_to_documents(async_client, db_session, sample_txt):
|
||||
@pytest.mark.xfail(strict=False, reason="test uses /upload endpoint removed in Plan 03-02; upload flow changed to two-step presigned PUT")
|
||||
async def test_delete_topic_cascades_to_documents(async_client, auth_user, db_session, sample_txt):
|
||||
# Create a topic
|
||||
topic = (await async_client.post("/api/topics", json={"name": "Legal"})).json()
|
||||
topic = (await async_client.post("/api/topics", json={"name": "Legal"}, headers=auth_user["headers"])).json()
|
||||
|
||||
# Upload doc (no auto classify)
|
||||
# Upload doc (no auto classify) — endpoint removed in Plan 03-02
|
||||
with open(sample_txt, "rb") as f:
|
||||
upload = (
|
||||
await async_client.post(
|
||||
@@ -80,24 +86,23 @@ async def test_delete_topic_cascades_to_documents(async_client, db_session, samp
|
||||
await storage.update_document_topics(db_session, upload["id"], ["Legal"])
|
||||
|
||||
# Delete topic
|
||||
await async_client.delete(f"/api/topics/{topic['id']}")
|
||||
await async_client.delete(f"/api/topics/{topic['id']}", headers=auth_user["headers"])
|
||||
|
||||
# Verify document no longer has the topic
|
||||
doc = (await async_client.get(f"/api/documents/{upload['id']}")).json()
|
||||
doc = (await async_client.get(f"/api/documents/{upload['id']}", headers=auth_user["headers"])).json()
|
||||
assert "Legal" not in doc["topics"]
|
||||
|
||||
|
||||
async def test_delete_topic_not_found(async_client):
|
||||
resp = await async_client.delete("/api/topics/nonexistent")
|
||||
async def test_delete_topic_not_found(async_client, auth_user):
|
||||
resp = await async_client.delete("/api/topics/nonexistent", headers=auth_user["headers"])
|
||||
assert resp.status_code == 404
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Wave 0 xfail stubs for Phase 3 topic namespace tests — Plan 03-03
|
||||
# Phase 3 topic namespace tests — Plan 03-03
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.xfail(strict=False, reason="implemented in plan 03-03")
|
||||
async def test_topic_namespace(async_client, auth_user, db_session):
|
||||
"""GET /api/topics returns only system topics (user_id=NULL) + auth_user-owned topics.
|
||||
|
||||
@@ -108,34 +113,92 @@ async def test_topic_namespace(async_client, auth_user, db_session):
|
||||
Test setup: seed one system topic, one auth_user-owned topic, one topic owned
|
||||
by a different user. GET /api/topics must return exactly the first two.
|
||||
"""
|
||||
assert True # scaffold
|
||||
import uuid as _uuid
|
||||
from db.models import Topic, User, Quota
|
||||
from services.auth import hash_password, create_access_token
|
||||
|
||||
# Seed a system topic (user_id=NULL)
|
||||
system_topic = Topic(id=_uuid.uuid4(), name="SystemTopic", user_id=None)
|
||||
db_session.add(system_topic)
|
||||
|
||||
# auth_user's topic already created via their auth token — create directly via ORM
|
||||
# (the create endpoint assigns user_id=current_user.id automatically)
|
||||
user_topic = Topic(id=_uuid.uuid4(), name="UserTopic", user_id=auth_user["user"].id)
|
||||
db_session.add(user_topic)
|
||||
|
||||
# Create a different user
|
||||
other_user_id = _uuid.uuid4()
|
||||
other_user = User(
|
||||
id=other_user_id,
|
||||
handle=f"other_{other_user_id.hex[:8]}",
|
||||
email=f"other_{other_user_id.hex[:8]}@example.com",
|
||||
password_hash=hash_password("Testpassword123!"),
|
||||
role="user",
|
||||
is_active=True,
|
||||
password_must_change=False,
|
||||
)
|
||||
other_quota = Quota(user_id=other_user_id, limit_bytes=104857600, used_bytes=0)
|
||||
db_session.add(other_user)
|
||||
db_session.add(other_quota)
|
||||
|
||||
# Other user's topic — must NOT appear in auth_user's topic list
|
||||
other_topic = Topic(id=_uuid.uuid4(), name="OtherUserTopic", user_id=other_user_id)
|
||||
db_session.add(other_topic)
|
||||
|
||||
await db_session.commit()
|
||||
|
||||
resp = await async_client.get("/api/topics", headers=auth_user["headers"])
|
||||
assert resp.status_code == 200, resp.text
|
||||
topics = resp.json()["topics"]
|
||||
topic_names = {t["name"] for t in topics}
|
||||
|
||||
# auth_user should see SystemTopic (system) and UserTopic (their own)
|
||||
assert "SystemTopic" in topic_names, f"System topic missing: {topic_names}"
|
||||
assert "UserTopic" in topic_names, f"User's own topic missing: {topic_names}"
|
||||
# auth_user must NOT see OtherUserTopic
|
||||
assert "OtherUserTopic" not in topic_names, (
|
||||
f"Cross-user topic leaked: {topic_names}"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.xfail(strict=False, reason="implemented in plan 03-03")
|
||||
async def test_admin_create_system_topic(async_client, admin_user):
|
||||
"""POST /api/admin/topics returns 201 and creates a Topic with user_id=NULL.
|
||||
|
||||
D-09: only admin can create system topics via POST /api/admin/topics.
|
||||
The created topic has user_id=NULL and is visible to all users.
|
||||
"""
|
||||
assert True # scaffold
|
||||
resp = await async_client.post(
|
||||
"/api/admin/topics",
|
||||
json={"name": "SystemTopicAdmin", "description": "Visible to all", "color": "#ff0000"},
|
||||
headers=admin_user["headers"],
|
||||
)
|
||||
assert resp.status_code == 201, f"Expected 201, got {resp.status_code}: {resp.text}"
|
||||
data = resp.json()
|
||||
assert data["name"] == "SystemTopicAdmin"
|
||||
assert "id" in data
|
||||
|
||||
|
||||
@pytest.mark.xfail(strict=False, reason="implemented in plan 03-03")
|
||||
async def test_regular_user_cannot_create_system_topic(async_client, auth_user):
|
||||
"""POST /api/admin/topics with auth_user.headers returns 403.
|
||||
|
||||
D-09: the admin topics endpoint requires get_current_admin; regular users
|
||||
receive 403 Forbidden.
|
||||
"""
|
||||
assert True # scaffold
|
||||
resp = await async_client.post(
|
||||
"/api/admin/topics",
|
||||
json={"name": "ShouldFail"},
|
||||
headers=auth_user["headers"],
|
||||
)
|
||||
assert resp.status_code == 403, (
|
||||
f"Expected 403 for regular user on admin endpoint, got {resp.status_code}: {resp.text}"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.xfail(strict=False, reason="implemented in plan 03-03")
|
||||
async def test_topics_require_auth(async_client):
|
||||
"""Anonymous GET /api/topics (no Authorization header) returns 401 or 403.
|
||||
|
||||
D-17: /api/topics/* gains get_current_user in Phase 3 — anonymous access
|
||||
must be rejected.
|
||||
"""
|
||||
assert True # scaffold
|
||||
resp = await async_client.get("/api/topics")
|
||||
assert resp.status_code in (401, 403), f"Expected 401 or 403, got {resp.status_code}"
|
||||
|
||||
Reference in New Issue
Block a user