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

Merged
NotBigGhost merged 7 commits from issue-8-code-review into dev 2026-09-09 19:06:54 +03:00
7 Commits
Author SHA1 Message Date
NotBigGhostandClaude Opus 5 a9de68f27c Simplify: общий загрузчик файлов и единые ключи на фронте
- Четыре копии «ресайз → FormData → CSRF из cookie → fetch → разбор конверта
  ошибки» (аватар, фото партии, фото из админки, иконка ачивки) сведены в
  lib/upload.ts. Иконка по-прежнему уходит оригиналом: ресайз в JPEG убил бы
  прозрачность герба.
- Три копии readCsrfToken и вторая копия resizeImage удалены — берём readCookie
  из api/client.ts и resizeImage из lib/image.ts с параметром размера.
- Ключи, протухающие от партии, собраны в matchAffectedKeys: раньше они были
  написаны строками мимо реестра qk в двух местах, и переименование ключа
  сломало бы инвалидацию молча.
- Различение «нет сессии» и «нет связи» вынесено в authProbeRetry и применено к
  обеим пробам. Прошлый заход чинил только игроцкие гварды, и RequireAdmin
  по-прежнему выкидывал админа на страницу входа при обрыве связи.
- ToastContext больше не пересоздаёт значение контекста: провайдер обёрнут
  вокруг всего приложения, и каждый тост перерисовывал всех потребителей.
- PlaceEditor не трогает DOM, пока цель подсветки не изменилась (было
  querySelectorAll на каждый pointermove).

#8

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BoiJK9ux8peeyjLb8TYjFf
2026-09-09 18:52:06 +03:00
NotBigGhostandClaude Opus 5 94e2c09952 Simplify: общие хелперы в роутерах
- IP клиента для аудита брался инлайном в 19 местах шести модулей; теперь
  security.client_ip — за привратником адрес придётся читать из
  X-Forwarded-For, и одна точка правки для этого обязательна.
- Чтение загруженной картинки (лимит размера + sniff формата) было скопировано
  в четыре обработчика; вынесено в user_service.read_capped_image.
- Лимит размера вложения жил двумя одинаковыми константами в игроцком и
  админском роутере — перенесён к самим вложениям.
- update_nickname и set_active_group переиспользуют nickname_format_ok и
  group_service.get_membership вместо собственных копий проверки.
- Убраны осиротевшие импорты и комментарий-заготовка о вложениях, которые
  давно реализованы (MatchAttachment).

#8

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BoiJK9ux8peeyjLb8TYjFf
2026-09-09 18:48:29 +03:00
NotBigGhostandClaude Opus 5 8ac2757cd6 Simplify: единый источник времени, версии партии и запросов
Проход /simplify по backend/app:

- Три источника «текущего времени» (_utcnow в models.py и match_service.py при
  живом timeutil.utcnow) сведены к одному — это прямо инвариант из CLAUDE.md.
- Бамп версии партии из двух независимых мест собран в match_service.touch:
  следующая точка мутации, не трогающая строку matches, теперь имеет очевидный
  способ сделать правильно.
- membership_service переиспользует group_service.get_membership вместо трёх
  копий одного запроса; защита последнего владельца — один хелпер на удаление
  и смену роли вместо двух похожих блоков.
- Убраны N+1: участники страницы партий и ники пригласивших берутся одним
  запросом вместо запроса на строку (20 партий = 20 лишних запросов с двумя
  join каждый).
- Счётчики партий считает СУБД (COUNT/MAX) вместо выгрузки всех строк ради
  len() и max() в Python.

#8

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BoiJK9ux8peeyjLb8TYjFf
2026-09-09 18:45:50 +03:00
NotBigGhostandClaude Opus 5 1831a3e033 Ревью: закрыть дыры в собственном покрытии
Прогон /code-review по тестам показал, что проверка обхода каталога ачивок
ничего не проверяла: httpx нормализует «..» в URL до отправки, запрос уходил
на /api/admin/ и до обработчика не доходил — тест был бы зелёным и без
защиты. Теперь percent-кодированная форма плюс проверка конверта ошибки,
чтобы промах роутинга не выдавал себя за отказ.

Добавлено недостающее: передача владения группой (обратная сторона защиты
последнего владельца), сдвиг версии партии при загрузке вложения, совпадение
кэш-бастера аватара между профилем и лидербордом. Проверка выживания группы
после отказа в удалении теперь смотрит на саму группу и её партии, а не
только на код ответа.

#8

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186Fk74jkkszahEHSjBzTjD
2026-09-09 18:32:47 +03:00
NotBigGhostandClaude Opus 5 c4b72cca39 Ревью: корректность фронтенда
Находки прохода /code-review high по frontend/src:

- CreateMatchPage: барабан рандома, таймеры и строки адресовались по индексу,
  а удаление строки индексы сдвигает — фракция могла записаться соседу или
  потеряться. Строки получили стабильный id.
- AdminAccountsPage: переключение «активен» глотало ошибку и всё равно
  показывало «Сохранено».
- useServerEvents/useFinishMatch: завершение чужой партии не инвалидировало
  историю игр и публичные профили — открытый профиль показывал состав до
  завершения.
- guards/useMe: обрыв связи не отличался от «нет сессии», и мигание сети
  выкидывало авторизованного пользователя на /login. Транспортные ошибки
  повторяем, гварды показывают сообщение вместо редиректа.
- format: fallback на нераспознанную дату не работал (new Date не бросает
  исключение), и в интерфейс попадало «NaN.NaN NaN:NaN».
- PlaceEditor: без onPointerCancel прерванный перенос оставлял блок с классом
  dragging и сдвигом, которые React не снимает — они выставлены в обход него.
- AdminFactionsPage: refetch после сохранения одной фракции затирал
  несохранённый ввод в остальных.
- MatchDetailPage: nav(-1) после удаления уводил из приложения при открытии
  партии по прямой ссылке; id из URL мог быть NaN и уходил в запрос.

#8

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186Fk74jkkszahEHSjBzTjD
2026-09-09 15:30:56 +03:00
NotBigGhostandClaude Opus 5 62d75ea176 Ревью: тесты-регрессии на найденные дефекты
Четыре теста закрывают то, что чинил предыдущий коммит: обход каталога
ачивок через slug, неподвижная версия партии при правке участников,
удаление группы с партиями (409 вместо 500) и разжалование последнего
владельца.

#8

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186Fk74jkkszahEHSjBzTjD
2026-09-09 15:26:42 +03:00
NotBigGhostandClaude Opus 5 9d9e4a345a Ревью: безопасность и корректность в сервисах бэкенда
Находки прохода /code-review high по backend/app:

- achievement_service: slug из URL шёл в путь без проверки, из-за чего
  DELETE /api/admin/achievements/%2E%2E удалял rmtree'ом родительскую папку
  каталога ачивок (в проде это /data — БД, uploads, ачивки целиком).
- match_service/attachment_service: версия партии = updated_at, но onupdate
  срабатывает лишь при реальном UPDATE строки matches. Правка одних участников
  и работа с вложениями его не вызывали, и оптимистичная блокировка молча
  пропускала конкурентную запись — бампаем updated_at явно.
- admin_service: удаление группы с партиями упиралось в RESTRICT и уходило
  наружу голым 500; теперь понятная ошибка. Админское удаление партии не
  чистило файлы вложений с тома — они оставались навсегда.
- user_service: при повторной загрузке аватара с тем же расширением avatar_path
  не менялся, updated_at не двигался, и кэш-бастер оставлял старую картинку до
  часа. Плюс версия считалась из наивного времени как из локального и
  разъезжалась с лидербордом, где то же поле считает SQL.
- membership_service: единственный владелец мог разжаловать сам себя и группа
  оставалась без владельца навсегда.
- notification_service: mark_read не слал SSE-сигнал, и бейдж непрочитанных на
  других устройствах висел до перезагрузки.
- routers/admin: created_at после правки пользователя отдавался без смещения,
  и дата «создан» прыгала на часовой пояс до следующего обновления списка.

#8

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186Fk74jkkszahEHSjBzTjD
2026-09-09 15:24:52 +03:00