admin/enduro-trails
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity
Files
ff18afed8c51e568898a2845b8ee26b2e96b07f4
enduro-trails/docs/work-items/ET-011/12-review.md
claude-bot ff18afed8c
All checks were successful
CI / lint (push) Successful in 4s
Details
CI / lint (pull_request) Successful in 4s
Details
CI / test (push) Successful in 7s
Details
CI / build (push) Successful in 2s
Details
CI / test (pull_request) Successful in 6s
Details
CI / build (pull_request) Successful in 1s
Details
reviewer(ET): auto-commit from reviewer run_id=69
2026-06-03 23:04:25 +00:00

13 KiB
Raw Blame History

type, work_item_id, verdict, version
type work_item_id verdict version
review ET-011 APPROVED 2

Review ET-011 — GPX-download из popup публичного трека (round 2)

Branch: feature/ET-011-popup-enduro-trails HEAD: 721b33a (fix(gps-tracks): address ET-011 review — JS UI tests + flat 403 contract) Build commit (initial): eea6c84 (feat(gps-tracks): GPX download from public track popup) Fix commit: 721b33a (закрывает P1-01 и P2-01 из review v1) Reviewer: agent:reviewer Дата: 2026-06-03

Сводка

PR полностью реализует backend (/api/gps-tracks/{track_id}/download, export.py) и frontend (кнопка в popup, _downloadPublicTrack, обработчик ошибок), описанные в ADR-014 и ADR-015. Все findings P1/P2 из review v1 закрыты в commit'е 721b33a. Регрессий нет, все тесты зелёные.

Покрытие требований: REQ-F-01..F-07 и REQ-NF-01..NF-07 реализованы; AC-1..AC-15 покрыты автотестами или явно зафиксированы как manual smoke (AC-6, AC-12, AC-13, AC-14).

Что проверено в round 2

Срез Результат
02-trz.md REQ-F-01..F-07, REQ-NF-01..NF-07 см. таблицу ниже — все ✓
03-acceptance-criteria.md AC-1..AC-15 см. таблицу AC ниже — все авто или явный manual
06-adr/ADR-014 / ADR-015 соответствует A2/B1/C1/D1/E1/F/G/H/I/J и A2/B1/C1/D/F/G
Закрытие findings v1 (P1-01, P2-01) см. раздел «Закрытие findings»
Линт (ruff check) новые/изменённые файлы — clean
Тесты API (pytest tests/api) 93/93 PASS (89 v1 + 4 новых = регрессия + IT-05 упрощён)
JS-тесты download UI (node --test tests/web/track_download.test.js) 28/28 PASS
Существующие JS-тесты (node --test tests/web/gps_tracks.test.js) 24/24 PASS
Pytest-обёртка (tests/web/test_track_download.py) 4/4 PASS (статика + Node-раннер)

Закрытие findings v1

P1-01 — Отсутствие автоматических UI-тестов → CLOSED

Был: tests/web/test_track_download.spec.ts (Playwright) отсутствовал; AC-1, AC-2 (UI), AC-7 (UI), AC-13 — без авто-покрытия.

Сделано в 721b33a:

  • Новый файл tests/web/track_download.test.js (359 строк, 28 Node-тестов):
    • _parseFilenameFromCD — 9 кейсов (RFC 5987 приоритет, plain fallback, битый percent-encoding, null/empty) → закрывает REQ-F-05.2 и UI-сторону AC-2.
    • _handleDownloadError — 9 кейсов (400/403/404/413/500, защита при отсутствии showToast, поддержка и flat ADR-015 §G формы, и legacy wrapped) → закрывает REQ-F-05.4 и UI-сторону AC-7.
    • _renderTrackPopupHtml — 10 кейсов (наличие кнопки, aria-label, data-track-id, отсутствие при невалидном id, порядок actions/sources, регрессия прочих полей) → закрывает REQ-F-01 и AC-1.
  • Новый файл tests/web/test_track_download.py (4 pytest-кейса): статическая проверка наличия символов в gps_tracks.js + запуск Node-раннера; интегрирует JS-тесты в обычный pytest tests/.
  • 04b-ui-test-cases.md явно маркирует AC-13 (mobile bbox / 32×32 CSS px на 375×667) как manual release-smoke в TC-UI-02. Это альтернатива, согласованная reviewer'ом в P1-01 v1.

Это покрывает абсолютное большинство AC-1 / AC-2 / AC-7 на уровне поведения клиентского кода. AC-13 остаётся как manual — это сознательное и согласованное решение из round 1.

P2-01 — Контракт 403 не совпадал с ADR-015 §G → CLOSED

Был: HTTPException(detail={...}) давал двойную вложенность {"detail":{"detail":"source_forbidden","external_urls":[...]}}; расхождение «doc vs runtime».

Сделано в 721b33a:

  • src/api/gps_tracks/endpoint.py строки 389-396: замена HTTPException(detail={...}) на JSONResponse(status_code=403, content={"detail":"source_forbidden", "external_urls":[...]}). FastAPI больше не оборачивает в дополнительный слой detail.
  • tests/api/test_gps_tracks_download.py::test_it05_source_forbidden_403: упрощён, проверяет плоский body: body.get("detail") == "source_forbidden" и body.get("external_urls") == [...].
  • src/web/gps_tracks.js::_handleDownloadError: flat-форма стала приоритетной (body.external_urls), wrapped-форма (body.detail.external_urls) сохранена как defensive fallback с комментарием. Это снижает связанность с возможным регрессом в backend.

Контракт runtime теперь идентичен ADR-014 §6 и ADR-015 §G:

{ "detail": "source_forbidden", "external_urls": ["..."] }

P2-02 (defensive 400-toast), P3-01..03 — нет действий

Оставлены как есть (defensive / nice-to-have); не блокирует approve.

Findings round 2

P0

Нет.

P1

Нет.

P2

Нет новых; v1 P2-01 закрыт, v1 P2-02 (defensive) допустим.

P3 (carry-over, не блокеры)

P3-01. logging.getLogger("uvicorn.access") остаётся как в v1 — не блокер, согласовано ADR-014 §J.

P3-02. Связка external_urls[i] ↔ sources[i] по индексу в build_gpx сохраняется; edge-case при разной длине списков не покрыт тестом, но текущий fallback на sources[0] безопасен. Можно закрыть отдельным юнит-тестом в будущей итерации (out-of-scope).

P3-03. Pre-existing intercolation ${name}, ${user}, ${url} в _renderTrackPopupHtml — наследие ET-008, не введено в ET-011. Новый блок actionsHtml использует только data-track-id="${trackId}", и trackId — Number(props.id), прошедший Number.isFinite(...) && > 0 (см. unit-тесты «id = 0 / null / "abc" / -1 → кнопка не рендерится»). Это safety-итерация, не блокер ET-011.

REQ ↔ реализация (round 2)

REQ Реализация Статус
REQ-F-01 (кнопка в popup, aria-label, 32×32) gps_tracks.js:498-509, app.css:1311-1338, JS-тесты _renderTrackPopupHtml (10 кейсов) ✓
REQ-F-02 (endpoint, статусы 400/403/404/413/200) endpoint.py:332-441 — порядок проверок по ADR-014 §H ✓
REQ-F-03 (GPX 1.1) export.py::build_gpx + UT-01..03 (XSD-валидация в tests/fixtures/gpx-1.1/gpx.xsd) ✓
REQ-F-04 (имя файла, RFC 5987) export.py::safe_filename + UT-04 (10 кейсов) + IT-06 ✓
REQ-F-05 (UX клика, toasts, fetch+Blob) _downloadPublicTrack, _parseFilenameFromCD, _handleDownloadError + 28 JS unit-тестов ✓
REQ-F-06 (license 403) endpoint.py:389-396 (JSONResponse) + config.py::load_download_allowed_sources + IT-05 + IT-05 dual-source ✓
REQ-F-07 (логирование) endpoint.py:425-430 через uvicorn.access ✓
REQ-NF-01 (perf 300 ms p95) manual perf check (см. AC-12); IT-01 проходит за < 50 ms на 10 точек ✓ (manual)
REQ-NF-02 (cap 200k → 413) MAX_POINTS_FOR_DOWNLOAD = 200_000 + IT-04 ✓
REQ-NF-03 (XSD валидация) UT-03 + IT-07 (XSD 30 КБ в fixtures) ✓
REQ-NF-04 (mobile UX, 32×32) CSS 32×32, popup maxWidth:'300px'; AC-13 — manual smoke (TC-UI-02) ✓ (manual согласован)
REQ-NF-05 (Content-Disposition RFC 5987) IT-06 проверяет filename*=UTF-8'' и ASCII-fallback ✓
REQ-NF-06 (наблюдаемость) uvicorn access + logger.info(...) ✓
REQ-NF-07 (безопасность) Path(..., ge=1), safe_filename чистит ФС-символы, CORS не трогается ✓

AC ↔ покрытие (round 2)

AC Авто-тест Статус
AC-1 (кнопка в popup, aria-label) track_download.test.js: 10 кейсов на _renderTrackPopupHtml + test_popup_renders_download_button_markup ✓
AC-2 (клик → GPX-файл) IT-01 (HTTP) + JS-тесты _parseFilenameFromCD (9 кейсов) ✓
AC-3 (200 + headers) IT-01 ✓
AC-4 (имя файла, sanitization) UT-04 (10 кейсов) + IT-06 ✓
AC-5 (валидность GPX по XSD) UT-03 + IT-07 ✓
AC-6 (импорт в GPS-софт) manual smoke (test-plan допускает) вне scope авто
AC-7 (404 «не найден») IT-02 (HTTP) + JS-тесты _handleDownloadError 404 ✓
AC-8 (400 невалидный формат) IT-03 + JS-тест _handleDownloadError 400 ✓
AC-9 (413 patho) IT-04 + JS-тест _handleDownloadError 413 ✓
AC-10 (metadata: copyright/link) UT-01, UT-02, test_ut01_osm_copyright_present ✓
AC-11 (license 403) IT-05 (single + dual-source) + JS-тесты _handleDownloadError 403 (flat + legacy wrapped) ✓
AC-12 (perf 300 ms) manual perf (test-plan допускает) вне scope авто
AC-13 (mobile bbox / 32×32) TC-UI-02 — manual release-smoke (согласовано в P1-01 v1) ✓ (manual согласован)
AC-14 (a11y / aria-label) JS-тест: assert.match(html, /aria-label="Скачать GPX"/) ✓
AC-15 (регрессия) IT-08 + регрессионные API/JS-тесты (93/93 + 24/24) ✓

ADR conformance (round 2)

ADR-014 (GPX endpoint).

  • §A решение A2 (fetch+Blob+a.download) — ✓ _downloadPublicTrack.
  • §B решение B1 (xml.etree.ElementTree) — ✓ export.py:11.
  • §C решение C1 (Response(...), in-memory) — ✓ endpoint.py:432-441.
  • §D решение D1 (popup остаётся открытым) — ✓ (нет close-call).
  • §E решение E1 (export.py модуль) — ✓.
  • §F sanitization — ✓ (_sanitize_for_filesystem, _truncate_utf8, _ascii_fallback, UT-04).
  • §G GPX-структура — ✓ (порядок metadata-children name/desc/author/ copyright/link/time, 6 знаков precision, OSM copyright).
  • §H порядок проверок — ✓ (format → SELECT → points_count → license → coords → build).
  • §I регистрация route — ✓ (после /cache/clear, конфликта префиксов нет).
  • §J logging — ✓.

ADR-015 (Source redistribution).

  • §A решение A2 (поле в YAML, runtime-кэш) — ✓ load_download_allowed_sources.
  • §B решение B1 (ANY-правило) — ✓, IT-05 dual-source.
  • §C решение C1 (default-deny) — ✓ config.py.
  • §D финальный whitelist — ✓ config/gps_sources.yaml (osm=true, остальные=false).
  • §F валидация — ✓ в route-handler, кэш в closure router'а.
  • §G ответ 403 — ✓ (flat-JSON, исправлено в 721b33a).

Регрессия

  • pytest tests/api — 93/93 PASS (89 v1 + 4 в новом round: IT-05 dual-source + default-deny smoke + два других). Включая test_gps_tracks_endpoint.py (20 кейсов для существующих маршрутов).
  • node --test tests/web/gps_tracks.test.js — 24/24 PASS (ET-008/ ET-009 поведения не сломаны).
  • node --test tests/web/track_download.test.js — 28/28 PASS (новое, ET-011).
  • pytest tests/web/test_track_download.py — 4/4 PASS (статика + Node-раннер).
  • ruff check на новых/изменённых файлах — clean.

Существующих маршрутов / структуры popup-полей / sheet-route::downloadGPX — ничего не сломано (IT-08 + регрессионный JS-тест «Popup-регрессия: остаются прежние поля»).

Итог

Категория Round 1 Round 2
P0 0 0
P1 1 0 (P1-01 закрыт 721b33a)
P2 2 0 (P2-01 закрыт; P2-02 — defensive, допустим)
P3 3 3 (carry-over, не блокеры)

Verdict: APPROVED. Все P0/P1/P2 round 1 закрыты commit'ом 721b33a. Регрессий нет, тесты зелёные, линт чистый. Реализация полностью соответствует ADR-014 и ADR-015, AC-1..AC-15 покрыты (автоматически или согласованным manual smoke). PR готов к merge'у в main и переходу в стадию deploy/test.

Reference in New Issue View Git Blame Copy Permalink