Round 5 taught `notifyActionError` to discard a plain-string `detail` in favour
of the caller's translated fallback, because `req` leaves 403/404/429 to the
caller and those routes hand-write English. Three places did not get the
message.
**The demo account lost its explanation.** `require_human` refused with a plain
string, so from that commit on the shared demo account got its caller's generic
"couldn't do that" and no hint that the demo account was the reason — and the
gate is not backed by a UI gate everywhere: the comfort shelf has no `is_demo`
check, so the pin button is live and simply fails. A structured `{code: "demo"}`
and an `errors.demo` string in both locales. The two auth routes that hand-roll
the same check were swept with it.
**`ChatThread` carried a second copy of the contract.** It filtered 404/429
itself and then did `e.detail || t(…)`, the pre-fix order, so the central fix
could not reach it and "That member hasn't set up messaging yet." stayed English
under a Hungarian UI. It goes through the shared helper now, and a contract test
pins the remaining raw-`detail` consumers as an explicit SET — never a count —
so a new copy fails the gate instead of shipping. The five on that list are
inline surfaces (a form field's error line, a setup banner) and are deferred
with the ~60 backend routes whose details they print.
**The Plex "Add to playlist" dialog was still silent.** Same shape as the
collection editor round 5 fixed and the YouTube twin that already had the fix:
no catch, so a playlist deleted on another device answered a structured 404 that
nothing reported — the tick never flipped, the gone row stayed, and the floating
promise became an unhandled rejection.
Also: the `ABORTED` sentinel's VALUE was asserted nowhere. The consumer test
hand-built the error it expected, so replacing `xhr.onabort`'s rejection with a
friendlier message left the lane green — and every upload the user cancels would
then raise a red "the upload failed" toast, which is what the sentinel prevents.
Both producer tests assert through `isAborted` now.
And two backend tests were pinning fewer sites than their own docstrings claimed:
the channel-404 loop said "all five" and issued three requests (explore and
subscribe unexercised, the excuse for subscribe naming a different call site
with a different reason slug), and the recipient/job test named three refusals
and requested two — `share_download`'s `not_found("recipient")` had no test in
the suite at all, nor did the locale keys added beside it. Every site named now
has a request behind it.
6 tests, mutation-checked against 7 mutations: the demo detail at both the gate
and an auth route, a re-hand-rolled ChatThread, the silent Plex dialog, the
abort sentinel, and each unpinned 404 — all red on the test that names them.
149 lines
7.3 KiB
Python
149 lines
7.3 KiB
Python
"""A row that is gone answers with a reason the bilingual UI can translate (deep-review round 3).
|
|
|
|
The 404 a user actually meets is the one for something deleted in another tab or on another device.
|
|
`owned_or_404` / `get_or_404` sent it as a bare English string ("Unknown view", "Not found"), and the
|
|
Hungarian UI toasted it verbatim. The structured-404 fix had gone into one ad-hoc helper instead of
|
|
the shared one, so every other owner check kept the English. Needs the DB.
|
|
"""
|
|
|
|
from app.auth import require_write_scope
|
|
from app.models import DownloadJob, MediaAsset
|
|
|
|
|
|
def test_a_gone_view_tag_or_pin_says_what_is_gone(db, make_user, as_user):
|
|
client = as_user(make_user())
|
|
for method, url, what in [
|
|
("patch", "/api/saved-views/999999", "view"),
|
|
("delete", "/api/saved-views/999999", "view"),
|
|
("delete", "/api/tags/999999", "tag"),
|
|
("delete", "/api/discover/pins/999999", "pin"),
|
|
]:
|
|
r = getattr(client, method)(url, **({"json": {"name": "x"}} if method == "patch" else {}))
|
|
assert r.status_code == 404, (url, r.text)
|
|
assert r.json()["detail"] == {"code": "not_found", "reason": what}, url
|
|
|
|
|
|
def test_a_gone_channel_video_or_user_says_what_is_gone(db, make_user, as_user):
|
|
"""The siblings that still hand-wrote `db.get(...) is None → 404 "Unknown …"` (deep-review
|
|
round 4): the same gone row, but the Hungarian UI showed the English."""
|
|
admin = make_user(role="admin")
|
|
client = as_user(admin)
|
|
for method, url, body, what in [
|
|
("post", "/api/channels/UCgone000000000000000000/block", None, "channel"),
|
|
("post", "/api/videos/gone0000000/state", {"status": "watched"}, "video"),
|
|
(
|
|
"post",
|
|
"/api/videos/gone0000000/progress",
|
|
{"position_seconds": 60, "duration_seconds": 600},
|
|
"video",
|
|
),
|
|
("put", "/api/admin/downloads/quota/999999", {"max_bytes": 1}, "user"),
|
|
]:
|
|
r = getattr(client, method)(url, **({"json": body} if body is not None else {}))
|
|
assert r.status_code == 404, (url, r.text)
|
|
assert r.json()["detail"] == {"code": "not_found", "reason": what}, url
|
|
|
|
|
|
def test_every_gone_channel_route_says_it_the_same_way(db, make_user, as_user, override_dep):
|
|
"""The round-4 sweep converted ONE of the five `db.get(Channel, …) is None -> 404 "Unknown
|
|
channel"` sites (block), and shipped the HU/EN string with it — so the Hungarian UI said it in
|
|
Hungarian on the Block button and in English on the four beside it (deep-review round 5). The
|
|
asymmetric fix is the smell this repo bans, so all five are pinned together.
|
|
|
|
Literally all five: round 6 found this loop issuing three requests under a docstring claiming
|
|
five, with explore silently absent and subscribe excused as "covered by the tags route" — which
|
|
reaches `_user_subscription`, a different site answering `reason: subscription`. Reverting
|
|
either of those two to its bare English string left the whole lane green."""
|
|
admin = make_user(role="admin")
|
|
client = as_user(admin)
|
|
# Subscribe's own gate (the optional YouTube write scope) sits in FRONT of the lookup, so a
|
|
# plain account 403s before reaching it. The gate is not what is under test here.
|
|
override_dep(require_write_scope, lambda: admin)
|
|
gone = "UCgone000000000000000000"
|
|
for method, url in [
|
|
("get", f"/api/channels/{gone}"),
|
|
("post", f"/api/channels/{gone}/explore"),
|
|
("post", f"/api/channels/{gone}/reset-backfill"),
|
|
("post", f"/api/channels/{gone}/subscribe"),
|
|
("post", f"/api/channels/{gone}/block"),
|
|
]:
|
|
r = getattr(client, method)(url)
|
|
assert r.status_code == 404, (url, r.text)
|
|
assert r.json()["detail"] == {"code": "not_found", "reason": "channel"}, url
|
|
|
|
|
|
def test_a_gone_subscription_recipient_or_job_says_what_is_gone(db, make_user, as_user):
|
|
"""The three bare-English 404s the same sweep left behind, which `notifyActionError` printed
|
|
verbatim into an otherwise Hungarian UI: "Not subscribed to this channel", "No user with that
|
|
email.", "Unknown job".
|
|
|
|
All three, with a request each. Round 6 found this test naming three and issuing two: the
|
|
recipient site had no test in the whole suite (nor did the `errors.notFound.recipient` keys
|
|
added beside it), and `run_job_now` rode on its PATCH sibling's coverage."""
|
|
admin = make_user(role="admin")
|
|
client = as_user(admin)
|
|
r = client.post("/api/channels/UCgone000000000000000000/tags", json={"tag_id": 1})
|
|
assert r.status_code == 404, r.text
|
|
assert r.json()["detail"] == {"code": "not_found", "reason": "subscription"}
|
|
for method, url in [
|
|
("patch", "/api/admin/scheduler/jobs/no_such_job"),
|
|
("post", "/api/admin/scheduler/jobs/no_such_job/run"),
|
|
]:
|
|
kw = {"json": {"interval_minutes": 30}} if method == "patch" else {}
|
|
r = getattr(client, method)(url, **kw)
|
|
assert r.status_code == 404, (url, r.text)
|
|
assert r.json()["detail"] == {"code": "not_found", "reason": "job"}, url
|
|
# Sharing needs a finished download of one's own to share; the recipient lookup is the next check.
|
|
asset = MediaAsset(
|
|
source_kind="url", source_ref="share-404", format_sig="sig", status="ready", ref_count=1
|
|
)
|
|
db.add(asset)
|
|
db.flush()
|
|
job = DownloadJob(
|
|
user_id=admin.id,
|
|
asset_id=asset.id,
|
|
source_kind="url",
|
|
source_ref="share-404",
|
|
status="done",
|
|
queue_pos=1,
|
|
profile_snapshot={},
|
|
)
|
|
db.add(job)
|
|
db.commit()
|
|
r = client.post(f"/api/downloads/{job.id}/share", json={"email": "nobody@example.com"})
|
|
assert r.status_code == 404, r.text
|
|
assert r.json()["detail"] == {"code": "not_found", "reason": "recipient"}
|
|
|
|
|
|
def test_a_nul_in_a_request_string_is_a_422_not_a_500(db, make_user, as_user):
|
|
"""Postgres cannot hold NUL in text, so the driver refuses the query outright: a DataError, i.e.
|
|
a 500 with a traceback, wherever a `%00` or an escaped NUL reached a comparison (deep-review
|
|
round 4 — round 3 had guarded only the by-rating_key lookup). One app-wide handler now answers
|
|
all of them."""
|
|
client = as_user(make_user())
|
|
r = client.get("/api/plex/playlists?contains=%00")
|
|
assert r.status_code == 422, r.text
|
|
assert r.json()["detail"] == {"code": "invalid_input", "reason": "nul"}
|
|
assert client.get("/api/plex/playlists?contains_group=1,%00").status_code == 422
|
|
|
|
|
|
def test_any_other_data_error_is_still_a_500():
|
|
"""The other half of the app-wide handler, and the half a refactor can delete in silence: it is
|
|
registered for EVERY DataError on every route and decides on a substring of the driver's message,
|
|
so only a NUL may become a 422. Without this, simplifying it to an unconditional 422 — it reads
|
|
like dead defensive code — would answer a numeric overflow or an over-long ingested title with
|
|
"your input was bad", losing the traceback that names the real fault (deep-review round 5)."""
|
|
import asyncio
|
|
|
|
import pytest
|
|
from sqlalchemy.exc import DataError
|
|
|
|
from app.main import _nul_in_text
|
|
|
|
overflow = DataError("SELECT 1", {}, Exception("numeric field overflow"))
|
|
with pytest.raises(DataError):
|
|
asyncio.run(_nul_in_text(None, overflow)) # re-raised: it stays the 500 it always was
|
|
|
|
nul = DataError("SELECT 1", {}, Exception("invalid byte sequence: NUL (0x00)"))
|
|
assert asyncio.run(_nul_in_text(None, nul)).status_code == 422
|