Ревью кода (#8) #34

Merged
NotBigGhost merged 7 commits from issue-8-code-review into dev 2026-09-09 19:06:54 +03:00
4 changed files with 101 additions and 2 deletions
Showing only changes of commit 1831a3e033 - Show all commits
+6 -1
View File
@@ -124,6 +124,11 @@ def test_delete_rejects_traversal_slug(client: TestClient, make_admin, monkeypat
sibling.write_bytes(b"data") sibling.write_bytes(b"data")
_admin(client, make_admin) _admin(client, make_admin)
r = client.delete("/api/admin/achievements/..", headers=csrf_headers(client)) # Именно percent-кодированная форма: обычные точки httpx нормализует ещё до
# отправки, запрос уходит на /api/admin/ и до обработчика вовсе не доходит.
r = client.request(
"DELETE", "/api/admin/achievements/%2E%2E", headers=csrf_headers(client)
)
assert r.status_code == 404, r.text assert r.status_code == 404, r.text
assert r.json()["error"]["code"] == "NOT_FOUND" # ответ обработчика, а не промах роутинга
assert sibling.exists() and root.is_dir() assert sibling.exists() and root.is_dir()
+27 -1
View File
@@ -227,7 +227,9 @@ def test_admin_delete_group_with_matches_is_conflict(client: TestClient, make_ad
_admin_login(client, make_admin) _admin_login(client, make_admin)
r = client.delete(f"/api/admin/groups/{gid}", headers=csrf_headers(client)) r = client.delete(f"/api/admin/groups/{gid}", headers=csrf_headers(client))
assert r.status_code == 409, r.text assert r.status_code == 409, r.text
assert client.get("/api/admin/groups").status_code == 200 # Важен не только код ответа: группа и её партии должны пережить отказ.
assert any(g["id"] == gid for g in client.get("/api/admin/groups").json())
assert any(m["group_id"] == gid for m in client.get("/api/admin/matches").json())
def test_last_owner_cannot_demote_self(client: TestClient, engine): def test_last_owner_cannot_demote_self(client: TestClient, engine):
@@ -249,3 +251,27 @@ def test_last_owner_cannot_demote_self(client: TestClient, engine):
assert r.status_code == 403, r.text assert r.status_code == 403, r.text
members = client.get(f"/api/groups/{gid}/members").json() members = client.get(f"/api/groups/{gid}/members").json()
assert any(m["user_id"] == me["id"] and m["role"] == "owner" for m in members) assert any(m["user_id"] == me["id"] and m["role"] == "owner" for m in members)
def test_ownership_transfer_still_works(client: TestClient, engine):
"""Обратная сторона защиты последнего владельца: передать роль по-прежнему можно."""
me = login(client, "Owner")
gid = client.post(
"/api/groups", json={"name": "Группа", "expansion_ids": []}, headers=csrf_headers(client)
).json()["id"]
p2 = add_group_member(engine, gid, "Игрок2")
promote = client.patch(
f"/api/groups/{gid}/members/{p2}", json={"role": "owner"}, headers=csrf_headers(client)
)
assert promote.status_code == 200, promote.text
# Владельцев теперь двое — прежний может сложить полномочия.
demote = client.patch(
f"/api/groups/{gid}/members/{me['id']}",
json={"role": "member"},
headers=csrf_headers(client),
)
assert demote.status_code == 200, demote.text
members = client.get(f"/api/groups/{gid}/members").json()
assert [m["user_id"] for m in members if m["role"] == "owner"] == [p2]
+20
View File
@@ -138,3 +138,23 @@ def test_non_member_cannot_view(client: TestClient, engine, monkeypatch, tmp_pat
login(client, "Чужак") # не состоит в группе login(client, "Чужак") # не состоит в группе
g = client.get(f"/api/matches/{mid}/attachments/{aid}") g = client.get(f"/api/matches/{mid}/attachments/{aid}")
assert g.status_code in (401, 403) assert g.status_code in (401, 403)
def test_attachment_upload_moves_version(client: TestClient, engine, monkeypatch, tmp_path):
"""Вложения видны в MatchRead, но строку matches не трогают.
Регрессия: из-за этого версия партии не двигалась, и правка со старой версией
проходила мимо оптимистичной блокировки."""
_use_tmp_uploads(monkeypatch, tmp_path)
me, gid, p2, mid = _start(client, engine)
v1 = client.get(f"/api/matches/{mid}").json()["version"]
assert _upload(client, mid).status_code == 200
v2 = client.get(f"/api/matches/{mid}").json()["version"]
assert v2 != v1
stale = client.delete(
f"/api/matches/{mid}", params={"expected_version": v1}, headers=csrf_headers(client)
)
assert stale.status_code == 409, stale.text
assert stale.json()["error"]["code"] == "STALE_WRITE"
+48
View File
@@ -3,7 +3,9 @@
from __future__ import annotations from __future__ import annotations
from fastapi.testclient import TestClient from fastapi.testclient import TestClient
from sqlmodel import Session
from app.models import User
from tests.conftest import ( from tests.conftest import (
add_group_member, add_group_member,
create_finished_match, create_finished_match,
@@ -399,3 +401,49 @@ def test_history_uses_owner_mode_for_guests(client: TestClient, engine):
assert data["mode"] == "best" assert data["mode"] == "best"
assert data["detail"] == "full" assert data["detail"] == "full"
assert data["total"] == 1 assert data["total"] == 1
def test_avatar_version_is_stable_across_surfaces(client: TestClient, engine, monkeypatch, tmp_path):
"""Кэш-бастер аватара одинаков в профиле и в лидерборде, и меняется при перезаливке.
Регрессия: версию профиля считал Python из наивного времени как из локального,
а лидерборд — SQL как из UTC, и браузер тянул одну картинку дважды. Плюс при
том же расширении файла updated_at не двигался и ссылка оставалась прежней."""
_use_tmp_uploads(monkeypatch, tmp_path)
me = login(client, "Версия")
_finished_match_for(client, engine, me)
def version_in(url: str) -> str:
return url.split("?v=")[1]
def leaderboard_url() -> str:
board = client.get("/api/stats/leaderboard").json()
entry = next(
e for e in board["entries"] + board["provisional"] if e["user_id"] == me["id"]
)
return entry["avatar_url"]
first = client.put(
"/api/users/me/avatar",
files={"file": ("a.png", PNG, "image/png")},
headers=csrf_headers(client),
)
assert first.status_code == 200, first.text
v_profile = version_in(first.json()["avatar_url"])
assert version_in(leaderboard_url()) == v_profile
# Повторная загрузка с тем же расширением: avatar_path не меняется, поэтому UPDATE
# строки сам собой не эмитится — updated_at должен двигаться явно, иначе кэш-бастер
# замирает и браузер час показывает прежнюю картинку. Версия в ссылке считается с
# точностью до секунды, поэтому сдвиг проверяем по времени в БД.
with Session(engine) as s:
before = s.get(User, me["id"]).updated_at
second = client.put(
"/api/users/me/avatar",
files={"file": ("a.png", PNG + b"\x00", "image/png")},
headers=csrf_headers(client),
)
assert second.status_code == 200, second.text
with Session(engine) as s:
assert s.get(User, me["id"]).updated_at > before
assert version_in(leaderboard_url()) == version_in(second.json()["avatar_url"])