Skip to content

Rewrite denied notes and postpone a week #23

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Drefvelin merged 1 commit into from
Sep 24, 2026
Merged

Rewrite denied notes and postpone a week #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 106 additions & 0 deletions backend/src/api/patchnotes_routes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,18 @@
BulletNotPending,
PatchnotesConfigError,
PatchnotesDBError,
WeekNotPostponed,
WeekPostponed,
approve_bullet,
approve_pending_week,
add_folder_rule,
current_week,
defer_postponed_week,
deny_bullet,
ensure_folder,
get_bullet,
get_folder,
get_week_status,
insert_bullet,
insert_sourced_bullet,
list_approved,
Expand All @@ -41,10 +47,13 @@
migrate,
observe_folder,
parse_week,
postpone_week,
queue_sync_task,
remove_added_folder,
replace_preview,
request_added_folder,
revise_denied_bullet,
undo_postpone,
)
from src.patchnotes.folders import catalog_entries, clean_folder_name, safe_rule
from src.patchnotes.safety import hidden_knowledge_warning
Expand Down Expand Up @@ -118,6 +127,9 @@ def _serialize(row: dict[str, Any], *, public: bool) -> dict[str, Any]:
warning = hidden_knowledge_warning(str(row.get("body") or ""))
if warning:
payload["warning"] = warning
note = str(row.get("revision_note") or "").strip()
if note:
payload["revision_note"] = note
return payload


Expand Down Expand Up @@ -649,4 +661,98 @@ def staff_deny_bullet(bullet_id: str, body: DenyBulletBody):
except (PatchnotesDBError, PatchnotesConfigError, psycopg2.Error) as e:
logger.exception("staff_deny_bullet failed")
raise HTTPException(status_code=502, detail=_client_detail(e)) from e
try:
revision = revise_denied_bullet(row, body.reason)
except (PatchnotesDBError, PatchnotesConfigError, psycopg2.Error, ValueError):
logger.exception("staff_deny_bullet rewrite failed")
revision = None
payload = _serialize(row, public=False)
payload["revision"] = _serialize(revision, public=False) if revision else None
return payload


@patchnotes_router.get("/staff/bullets/{bullet_id}", dependencies=[Depends(_staff_guard)])
def staff_get_bullet(bullet_id: str):
bullet_id = _bullet_id_or_404(bullet_id)
try:
migrate()
row = get_bullet(bullet_id)
except BulletNotFound as e:
raise _review_http(e) from e
except (PatchnotesDBError, PatchnotesConfigError, psycopg2.Error) as e:
logger.exception("staff_get_bullet failed")
raise HTTPException(status_code=502, detail=_client_detail(e)) from e
return _serialize(row, public=False)


def _week_payload(row: dict[str, Any]) -> dict[str, Any]:
return {
"week": row["week"],
"postponed": bool(row.get("postponed")),
"deferred_to": row.get("deferred_to"),
}


@patchnotes_router.get("/staff/weeks/{week}", dependencies=[Depends(_staff_guard)])
def staff_week_status(week: str):
week_key = _week_or_400(week)
try:
migrate()
row = get_week_status(week_key)
except (PatchnotesDBError, PatchnotesConfigError, psycopg2.Error) as e:
logger.exception("staff_week_status failed")
raise HTTPException(status_code=502, detail=_client_detail(e)) from e
return _week_payload(row)


@patchnotes_router.post("/staff/weeks/{week}/postpone", dependencies=[Depends(_staff_guard)])
def staff_postpone_week(week: str):
week_key = _week_or_400(week)
try:
migrate()
row = postpone_week(week_key)
except (PatchnotesDBError, PatchnotesConfigError, psycopg2.Error) as e:
logger.exception("staff_postpone_week failed")
raise HTTPException(status_code=502, detail=_client_detail(e)) from e
return _week_payload(row)


@patchnotes_router.post("/staff/weeks/{week}/undo-postpone", dependencies=[Depends(_staff_guard)])
def staff_undo_postpone(week: str):
week_key = _week_or_400(week)
try:
migrate()
row = undo_postpone(week_key)
except (PatchnotesDBError, PatchnotesConfigError, psycopg2.Error) as e:
logger.exception("staff_undo_postpone failed")
raise HTTPException(status_code=502, detail=_client_detail(e)) from e
return _week_payload(row)


@patchnotes_router.post("/staff/weeks/{week}/defer", dependencies=[Depends(_staff_guard)])
def staff_defer_week(week: str):
week_key = _week_or_400(week)
try:
migrate()
row = defer_postponed_week(week_key)
except WeekNotPostponed as e:
raise HTTPException(status_code=409, detail="Week is not postponed") from e
except (PatchnotesDBError, PatchnotesConfigError, psycopg2.Error) as e:
logger.exception("staff_defer_week failed")
raise HTTPException(status_code=502, detail=_client_detail(e)) from e
return _week_payload(row)


@patchnotes_router.post("/staff/weeks/{week}/auto-approve", dependencies=[Depends(_staff_guard)])
def staff_auto_approve_week(week: str):
"""Approve every line still waiting, except rewrites."""
week_key = _week_or_400(week)
try:
migrate()
approved = approve_pending_week(week_key)
except WeekPostponed as e:
raise HTTPException(status_code=409, detail="Week is postponed") from e
except (PatchnotesDBError, PatchnotesConfigError, psycopg2.Error) as e:
logger.exception("staff_auto_approve_week failed")
raise HTTPException(status_code=502, detail=_client_detail(e)) from e
return {"week": week_key, "approved": approved}
94 changes: 91 additions & 3 deletions backend/src/api/test_patchnotes_routes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,13 @@
from fastapi.testclient import TestClient # noqa: E402

from server import app # noqa: E402
from src.patchnotes.db import BulletNotFound, BulletNotPending, PatchnotesDBError # noqa: E402
from src.patchnotes.db import ( # noqa: E402
BulletNotFound,
BulletNotPending,
PatchnotesDBError,
WeekNotPostponed,
WeekPostponed,
)

_HEADERS = {"X-Staff-Key": "dev-staff-key"}
_CREATED = datetime(2026, 9, 23, 12, 0, tzinfo=timezone.utc)
Expand Down Expand Up @@ -76,6 +82,16 @@ def test_staff_routes_require_auth(self) -> None:
).status_code,
401,
)
self.assertEqual(self.client.get(f"/patchnotes/staff/bullets/{_BULLET_ID}").status_code, 401)
self.assertEqual(self.client.get("/patchnotes/staff/weeks/2026-W39").status_code, 401)
self.assertEqual(self.client.post("/patchnotes/staff/weeks/2026-W39/postpone").status_code, 401)
self.assertEqual(
self.client.post("/patchnotes/staff/weeks/2026-W39/undo-postpone").status_code, 401
)
self.assertEqual(self.client.post("/patchnotes/staff/weeks/2026-W39/defer").status_code, 401)
self.assertEqual(
self.client.post("/patchnotes/staff/weeks/2026-W39/auto-approve").status_code, 401
)

def test_bad_staff_key_does_not_fall_through_to_session(self) -> None:
res = self.client.get(
Expand Down Expand Up @@ -177,20 +193,49 @@ def test_approve(self, mock_approve) -> None:
self.assertEqual(res.json()["status"], "approved")
mock_approve.assert_called_once_with(_BULLET_ID)

@mock.patch("src.api.patchnotes_routes.revise_denied_bullet", return_value=None)
@mock.patch(
"src.api.patchnotes_routes.deny_bullet",
return_value=_row(status="denied", deny_reason="Internal only", reviewed_at=_CREATED),
)
def test_deny_stores_the_reason(self, mock_deny) -> None:
def test_deny_stores_the_reason(self, mock_deny, _mock_revise) -> None:
res = self.client.post(
f"/patchnotes/staff/bullets/{_BULLET_ID}/deny",
json={"reason": "Internal only"},
headers=_HEADERS,
)
self.assertEqual(res.status_code, 200)
self.assertEqual(res.json()["deny_reason"], "Internal only")
body = res.json()
self.assertEqual(body["deny_reason"], "Internal only")
self.assertIsNone(body["revision"])
mock_deny.assert_called_once_with(_BULLET_ID, "Internal only")

@mock.patch(
"src.api.patchnotes_routes.revise_denied_bullet",
return_value=_row(
id="22222222-2222-2222-2222-222222222222",
body="Added a station",
revision_note="Don't mention the vault",
),
)
@mock.patch(
"src.api.patchnotes_routes.deny_bullet",
return_value=_row(status="denied", deny_reason="Don't mention the vault", reviewed_at=_CREATED),
)
def test_deny_returns_the_rewrite_without_putting_the_reason_in_the_body(
self, _mock_deny, _mock_revise
) -> None:
res = self.client.post(
f"/patchnotes/staff/bullets/{_BULLET_ID}/deny",
json={"reason": "Don't mention the vault"},
headers=_HEADERS,
)
self.assertEqual(res.status_code, 200)
revision = res.json()["revision"]
self.assertEqual(revision["body"], "Added a station")
self.assertEqual(revision["revision_note"], "Don't mention the vault")
self.assertNotIn("vault", revision["body"])

def test_deny_requires_a_reason(self) -> None:
res = self.client.post(
f"/patchnotes/staff/bullets/{_BULLET_ID}/deny",
Expand Down Expand Up @@ -286,6 +331,7 @@ def test_published_weeks_need_no_auth(self, _mock_weeks) -> None:
status="approved",
deny_reason="should not leak",
reviewed_at=_CREATED,
revision_note="staff only",
)
],
)
Expand All @@ -298,6 +344,7 @@ def test_published_week_hides_review_fields(self, mock_list) -> None:
self.assertNotIn("status", bullet)
self.assertNotIn("reviewed_at", bullet)
self.assertNotIn("warning", bullet)
self.assertNotIn("revision_note", bullet)
mock_list.assert_called_once_with("2026-W39")

def test_published_week_rejects_a_bad_week(self) -> None:
Expand Down Expand Up @@ -492,3 +539,44 @@ def test_a_watch_note_is_once_per_plugin_per_week(self, mock_insert, _week) -> N
mock_insert.call_args.kwargs["source_key"],
"watch:MythicDungeons:2026-W39",
)


class WeekActionRouteTest(unittest.TestCase):
def setUp(self) -> None:
self.client = TestClient(app)
patcher = mock.patch("src.api.patchnotes_routes.migrate")
self.addCleanup(patcher.stop)
patcher.start()

def tearDown(self) -> None:
self.client.close()

@mock.patch(
"src.api.patchnotes_routes.postpone_week",
return_value={"week": "2026-W39", "postponed": True, "deferred_to": None},
)
def test_postpone(self, mock_postpone) -> None:
res = self.client.post("/patchnotes/staff/weeks/2026-W39/postpone", headers=_HEADERS)
self.assertEqual(res.status_code, 200)
self.assertTrue(res.json()["postponed"])
mock_postpone.assert_called_once_with("2026-W39")

@mock.patch(
"src.api.patchnotes_routes.defer_postponed_week",
side_effect=WeekNotPostponed("2026-W39"),
)
def test_defer_requires_a_hold(self, _mock_defer) -> None:
res = self.client.post("/patchnotes/staff/weeks/2026-W39/defer", headers=_HEADERS)
self.assertEqual(res.status_code, 409)

@mock.patch("src.api.patchnotes_routes.approve_pending_week", side_effect=WeekPostponed("2026-W39"))
def test_auto_approve_refuses_a_postponed_week(self, _mock_approve) -> None:
res = self.client.post("/patchnotes/staff/weeks/2026-W39/auto-approve", headers=_HEADERS)
self.assertEqual(res.status_code, 409)

@mock.patch("src.api.patchnotes_routes.approve_pending_week", return_value=3)
def test_auto_approve_counts_lines(self, mock_approve) -> None:
res = self.client.post("/patchnotes/staff/weeks/2026-W39/auto-approve", headers=_HEADERS)
self.assertEqual(res.status_code, 200)
self.assertEqual(res.json()["approved"], 3)
mock_approve.assert_called_once_with("2026-W39")
Loading