Small joys: Codex audit #2 fixes (route resolution, noindex, sense/tone, exclude-current re-pick)
- Admin joy item route moved to /api/admin/joys/{kind}/items/{item_id} so the
/add and /repick verbs resolve to their own routes instead of 422-ing as a
non-int item id (the launch blocker). Frontend mutate URL updated to match.
- Re-pick now excludes the currently-shown item: the endpoint reads today's
daily pool_id and passes it as `avoid`, so "Re-pick today" yields a different
item. Added `avoid` to pick_daily/_candidates across wotd/quote/onthisday.
- WOTD sense selection: the LLM now proposes word + intended part of speech, and
_lookup prefers that sense (fixes "serene" returning the archaic noun).
- On This Day tone prompt tightened to favor genuinely uplifting events and
exclude merely procedural/political-administrative ones.
- Caddy @hidden now also noindexes /word /quote /onthisday /admin (+ .html).
- Regression tests: add/repick resolve (401 not 422), add/feature/block/delete,
re-pick excludes current; WOTD pos-preference + proposal parsing units.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,95 @@
|
||||
"""Small-joys admin API — route resolution (add/repick must NOT parse as an item id),
|
||||
auth gating, add/feature/block/delete with numeric ids, and re-pick excluding the
|
||||
currently-shown item."""
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from goodnews.db import connect, init_db
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def api_app(tmp_path, monkeypatch):
|
||||
db = tmp_path / "t.sqlite3"
|
||||
monkeypatch.setenv("GOODNEWS_DB", str(db))
|
||||
monkeypatch.setenv("GOODNEWS_PUBLIC_BASE_URL", "http://testserver")
|
||||
monkeypatch.setenv("GOODNEWS_ADMIN_EMAILS", "admin@b.com")
|
||||
monkeypatch.setenv("GOODNEWS_LLM_BASE_URL", "http://127.0.0.1:9") # dead → no LLM, fast
|
||||
import importlib
|
||||
import goodnews.api as api
|
||||
importlib.reload(api)
|
||||
c = connect(str(db)); init_db(c)
|
||||
c.close()
|
||||
return api.create_app()
|
||||
|
||||
|
||||
def _admin(app):
|
||||
tc = TestClient(app)
|
||||
sent = {}
|
||||
import goodnews.email_send as es
|
||||
orig = es.send_magic_link
|
||||
es.send_magic_link = lambda to, link: sent.update(link=link)
|
||||
try:
|
||||
tc.post("/api/auth/email/start", json={"email": "admin@b.com"})
|
||||
tc.post("/api/auth/email/verify", json={"token": sent["link"].split("token=")[1]})
|
||||
finally:
|
||||
es.send_magic_link = orig
|
||||
return tc
|
||||
|
||||
|
||||
def test_add_and_repick_require_admin_not_422(api_app):
|
||||
"""The blocker: /add and /repick must resolve to their own routes (401 unauth),
|
||||
not be swallowed by /{item_id} and 422 on int parsing."""
|
||||
anon = TestClient(api_app)
|
||||
for kind in ("quote", "word", "onthisday"):
|
||||
assert anon.post(f"/api/admin/joys/{kind}/add", json={}).status_code == 401
|
||||
assert anon.post(f"/api/admin/joys/{kind}/repick").status_code == 401
|
||||
assert anon.get(f"/api/admin/joys/{kind}").status_code == 401
|
||||
assert anon.post(f"/api/admin/joys/{kind}/items/1",
|
||||
json={"action": "feature"}).status_code == 401
|
||||
|
||||
|
||||
def test_quote_add_feature_block_delete(api_app):
|
||||
tc = _admin(api_app)
|
||||
r = tc.post("/api/admin/joys/quote/add",
|
||||
json={"text": "A small kindness echoes far.", "author": "Anon"})
|
||||
assert r.status_code == 200 and r.json()["ok"] is True
|
||||
items = tc.get("/api/admin/joys/quote").json() # endpoint returns a bare list
|
||||
qid = next(it["id"] for it in items if "kindness echoes" in (it.get("text") or ""))
|
||||
assert tc.post(f"/api/admin/joys/quote/items/{qid}", json={"action": "feature"}).json()["ok"]
|
||||
assert tc.post(f"/api/admin/joys/quote/items/{qid}", json={"action": "block"}).json()["ok"]
|
||||
assert tc.post(f"/api/admin/joys/quote/items/{qid}", json={"action": "delete"}).json()["ok"]
|
||||
remaining = tc.get("/api/admin/joys/quote").json()
|
||||
assert all(it["id"] != qid for it in remaining)
|
||||
|
||||
|
||||
def test_repick_excludes_current(api_app):
|
||||
tc = _admin(api_app)
|
||||
tc.post("/api/admin/joys/quote/add", json={"text": "First light is a fresh start.", "author": "A"})
|
||||
tc.post("/api/admin/joys/quote/add", json={"text": "Every step forward counts.", "author": "B"})
|
||||
assert tc.post("/api/admin/joys/quote/repick").json()["picked"] is True # establish today's pick
|
||||
first = tc.get("/api/quote/today").json()
|
||||
assert tc.post("/api/admin/joys/quote/repick").json()["picked"] is True # force a fresh one
|
||||
second = tc.get("/api/quote/today").json()
|
||||
assert second["text"] != first["text"] # a genuinely different item
|
||||
|
||||
|
||||
def test_word_add_and_repick(api_app, monkeypatch):
|
||||
import goodnews.wotd as wotd
|
||||
# admin word-add validates against the dictionary + caches audio — stub both (no network)
|
||||
fake = {"serene": {"word": "serene", "part_of_speech": "adjective", "phonetic": "/sɪˈriːn/",
|
||||
"audio_url": None, "definition": "Calm, peaceful, untroubled.", "examples": []},
|
||||
"luminous": {"word": "luminous", "part_of_speech": "adjective", "phonetic": "/ˈluːmɪnəs/",
|
||||
"audio_url": None, "definition": "Giving off light; radiant.", "examples": []}}
|
||||
monkeypatch.setattr(wotd, "_lookup", lambda w, prefer_pos=None: fake.get(w))
|
||||
monkeypatch.setattr(wotd, "_cache_audio", lambda url, word: None)
|
||||
|
||||
tc = _admin(api_app)
|
||||
assert tc.post("/api/admin/joys/word/add", json={"word": "serene"}).json()["ok"]
|
||||
assert tc.post("/api/admin/joys/word/add", json={"word": "luminous"}).json()["ok"]
|
||||
items = tc.get("/api/admin/joys/word").json()
|
||||
assert {"serene", "luminous"} <= {it.get("word") for it in items}
|
||||
assert tc.post("/api/admin/joys/word/repick").json()["picked"] is True
|
||||
first = tc.get("/api/word/today").json()
|
||||
assert tc.post("/api/admin/joys/word/repick").json()["picked"] is True
|
||||
second = tc.get("/api/word/today").json()
|
||||
assert second["word"] != first["word"]
|
||||
+22
-1
@@ -24,7 +24,7 @@ class FakeClient:
|
||||
@pytest.fixture
|
||||
def conn(tmp_path, monkeypatch):
|
||||
monkeypatch.setenv("GOODNEWS_WOTD_AUDIO", str(tmp_path / "audio"))
|
||||
monkeypatch.setattr(wotd, "_lookup", lambda w: FAKE_DICT.get(w)) # no dictionary network
|
||||
monkeypatch.setattr(wotd, "_lookup", lambda w, prefer_pos=None: FAKE_DICT.get(w)) # no dictionary network
|
||||
monkeypatch.setattr(wotd, "_cache_audio", lambda url, word: f"{word}.mp3" if url else None)
|
||||
c = connect(":memory:"); init_db(c)
|
||||
yield c
|
||||
@@ -64,3 +64,24 @@ def test_get_today_never_empty(conn):
|
||||
def test_run_daily_bootstraps(conn):
|
||||
r = wotd.run_daily(conn, client=FakeClient())
|
||||
assert r["pool"] == 2 and r["picked"] in ("serene", "dawn")
|
||||
|
||||
|
||||
def test_lookup_prefers_intended_pos(monkeypatch):
|
||||
"""When the LLM says 'serene' as an adjective, _lookup must pick the adjective sense,
|
||||
not an earlier archaic noun sense the dictionary lists first."""
|
||||
entry = {"word": "serene", "phonetics": [], "meanings": [
|
||||
{"partOfSpeech": "noun", "definitions": [{"definition": "Serenity; clearness; calmness."}]},
|
||||
{"partOfSpeech": "adjective", "definitions": [{"definition": "Calm, peaceful, untroubled."}]},
|
||||
]}
|
||||
monkeypatch.setattr(wotd.daily, "http_json", lambda url, timeout=20: [entry])
|
||||
assert wotd._lookup("serene", "adjective")["part_of_speech"] == "adjective"
|
||||
assert wotd._lookup("serene", "adjective")["definition"] == "Calm, peaceful, untroubled."
|
||||
assert wotd._lookup("serene")["part_of_speech"] == "noun" # no preference → first usable sense
|
||||
|
||||
|
||||
def test_propose_words_accepts_dicts_and_strings():
|
||||
class C:
|
||||
def chat_text(self, m):
|
||||
return '{"words": [{"word": "Serene", "pos": "Adjective"}, "dawn", {"word": ""}]}'
|
||||
out = wotd._propose_words(C(), 3)
|
||||
assert out == [{"word": "serene", "pos": "adjective"}, {"word": "dawn", "pos": None}]
|
||||
|
||||
Reference in New Issue
Block a user