From 53becb2e649789bf5498028ec96d85560d1e5759 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Thu, 20 Aug 2026 09:44:14 +0000 Subject: [PATCH] =?UTF-8?q?fix(ptica):=20=D0=B2=D1=8B=D1=80=D1=83=D1=87?= =?UTF-8?q?=D0=BA=D0=B0=20=D0=B8=20=D1=81=D0=B4=D0=B5=D0=BB=D0=BA=D0=B8=20?= =?UTF-8?q?=D0=B2=20KPI=20=D0=BB=D0=B8=D0=B4=D0=BE=D0=B2=20=D0=BD=D0=B0?= =?UTF-8?q?=D0=B7=D0=B2=D0=B0=D0=BD=D1=8B=20=D0=BF=D0=BE=20=D1=81=D0=B2?= =?UTF-8?q?=D0=BE=D0=B5=D0=BC=D1=83=20=D0=BE=D1=85=D0=B2=D0=B0=D1=82=D1=83?= =?UTF-8?q?=20(#2464)=20(#2963)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- backend/app/api/v1/admin_leads.py | 33 ++- backend/tests/skip_allowlist.txt | 9 + .../test_2464_leads_stats_suffix_contract.py | 189 ++++++++++++++++++ frontend/src/app/admin/leads/page.tsx | 10 +- frontend/src/lib/api-types.ts | 2 + 5 files changed, 230 insertions(+), 13 deletions(-) create mode 100644 backend/tests/sql/test_2464_leads_stats_suffix_contract.py diff --git a/backend/app/api/v1/admin_leads.py b/backend/app/api/v1/admin_leads.py index f23b839c..a5cc90cf 100644 --- a/backend/app/api/v1/admin_leads.py +++ b/backend/app/api/v1/admin_leads.py @@ -130,7 +130,16 @@ def leads_stats( db: Annotated[Session, Depends(get_db)], months: Annotated[int, Query(ge=1, le=120)] = 12, ) -> dict[str, Any]: - """KPI summary за последние N месяцев.""" + """KPI summary за последние N месяцев. + + Суффикс `_window` — за окно `months`, `_total` — за всё время. + """ + # Почему это важно и почему поля переименованы (#2464): revenue_total и + # deals_total считались по CTE window_leads, то есть за окно, а суффиксом + # обещали итог за всё время — рядом с честными leads_total/sources_total. + # Админка из-за этого показывала карточку «Revenue (всего)» с 12-месячной + # цифрой. Рационал держим комментарием, а не docstring'ом: docstring уходит + # в OpenAPI description и дальше в сгенерированные типы фронта. row = ( db.execute( text( @@ -155,14 +164,14 @@ def leads_stats( WHERE d.deal_id IN ( SELECT deal_id FROM window_leads WHERE deal_id IS NOT NULL ) - ) AS revenue_total, + ) AS revenue_window, ( SELECT COUNT(*) FROM prinzip_deals d WHERE d.deal_id IN ( SELECT deal_id FROM window_leads WHERE deal_id IS NOT NULL ) - ) AS deals_total + ) AS deals_window FROM window_leads """ ), @@ -178,8 +187,16 @@ def leads_stats( "converted_window": 0, "conv_pct_window": None, "sources_total": 0, - "revenue_total": None, - "deals_total": 0, + "revenue_window": None, + "deals_window": 0, + # window_months раньше отдавался ТОЛЬКО в непустой ветке — формы ответа + # различались. Оговорка про достижимость: этот `if not row` СЕГОДНЯ не + # срабатывает — запрос агрегатный и всегда возвращает ровно одну строку + # (проверено на пустых таблицах: leads_total=0, leads_window=0, строка + # truthy). То есть правка здесь — согласованность, а не наблюдаемая + # починка; ветка остаётся защитой на случай смены формы запроса, и + # расходиться с основной ей нельзя — именно так пропажа поля и возникла. + "window_months": months, } return { "leads_total": row["leads_total"] or 0, @@ -189,10 +206,10 @@ def leads_stats( float(row["conv_pct_window"]) if row["conv_pct_window"] is not None else None ), "sources_total": row["sources_total"] or 0, - "revenue_total": ( - float(row["revenue_total"]) if row["revenue_total"] is not None else None + "revenue_window": ( + float(row["revenue_window"]) if row["revenue_window"] is not None else None ), - "deals_total": row["deals_total"] or 0, + "deals_window": row["deals_window"] or 0, "window_months": months, } diff --git a/backend/tests/skip_allowlist.txt b/backend/tests/skip_allowlist.txt index 244eae86..b24e8f28 100644 --- a/backend/tests/skip_allowlist.txt +++ b/backend/tests/skip_allowlist.txt @@ -106,3 +106,12 @@ tests/sql/test_2956_freshness_ignores_failed_dumps.py::test_failed_dumps_do_not_ tests/sql/test_2956_freshness_ignores_failed_dumps.py::test_successful_dump_still_counts_as_fresh tests/sql/test_2956_freshness_ignores_failed_dumps.py::test_attempt_is_still_recorded tests/sql/test_2956_freshness_ignores_failed_dumps.py::test_only_failures_means_no_success_at_all + +# ── #2464: контракт суффиксов в /admin/leads/stats ──────────────────────────── +# Нужен живой Postgres: тест создаёт ВРЕМЕННЫЕ prinzip_leads/prinzip_deals и +# вызывает leads_stats на данных, где итог заведомо не равен окну. В CI ЭТИ ТЕСТЫ +# ИДУТ (postgres-сервис, #2745); записи нужны для машины без БД и без туннеля. +tests/sql/test_2464_leads_stats_suffix_contract.py::test_window_suffixed_fields_match_the_window +tests/sql/test_2464_leads_stats_suffix_contract.py::test_total_suffixed_fields_are_all_time +tests/sql/test_2464_leads_stats_suffix_contract.py::test_revenue_and_deals_are_named_by_their_scope +tests/sql/test_2464_leads_stats_suffix_contract.py::test_window_months_present_on_empty_data diff --git a/backend/tests/sql/test_2464_leads_stats_suffix_contract.py b/backend/tests/sql/test_2464_leads_stats_suffix_contract.py new file mode 100644 index 00000000..fc13f048 --- /dev/null +++ b/backend/tests/sql/test_2464_leads_stats_suffix_contract.py @@ -0,0 +1,189 @@ +"""Суффикс поля в /admin/leads/stats обязан соответствовать смыслу величины (#2464). + +В ответе рядом стоят величины двух видов: за всё время (`leads_total`, `sources_total`) и +за окно `months` (`leads_window`, `converted_window`, `conv_pct_window`). Соглашение +читается однозначно по самим именам. + +`revenue_total` и `deals_total` его нарушали: считались по CTE `window_leads`, то есть за +окно, а суффиксом обещали итог. Админка из-за этого печатала карточку «Revenue (всего)» с +12-месячной цифрой. + +Проверяется ИНВАРИАНТ, а не набор имён: для данных, где итог заведомо не равен окну, +каждое поле `*_total` обязано совпасть с итогом, каждое `*_window` — с окном. Такая +формулировка краснеет на origin/main по НЕВЕРНОМУ ЗНАЧЕНИЮ, а не по отсутствию ключа, и +переживёт любое разумное переименование. + +Тест герметичный: обе таблицы создаются ВРЕМЕННЫМИ в своей же сессии; по конвенции +`tests/sql/*` DSN по умолчанию смотрит в туннель к прод-базе, поэтому в фикстуре стоит +проверка, что затенение сработало. +""" + +from __future__ import annotations + +import os + +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") + +import pytest +from sqlalchemy import create_engine, text +from sqlalchemy.orm import sessionmaker + + +def _dsn() -> str: + raw = os.environ.get("TEST_DATABASE_URL") or os.environ.get( + "DATABASE_URL", "postgresql+psycopg://gendesign@localhost:15432/gendesign" + ) + return ( + raw + if raw.startswith("postgresql+") + else raw.replace("postgresql://", "postgresql+psycopg://") + ) + + +def _db_reachable() -> tuple[bool, str]: + try: + eng = create_engine(_dsn(), connect_args={"connect_timeout": 3}) + with eng.connect() as c: + c.execute(text("SELECT 1")) + return True, "" + except Exception as exc: + return False, str(exc) + + +_DB_OK, _DB_ERR = _db_reachable() +pytestmark = pytest.mark.skipif(not _DB_OK, reason=f"Postgres недоступен: {_DB_ERR}") + +_SCHEMA = """ +CREATE TEMP TABLE prinzip_leads ( + lead_id bigint, created_at timestamptz, source text, converted boolean, + deal_id bigint) ON COMMIT DROP; +CREATE TEMP TABLE prinzip_deals ( + deal_id bigint, deal_price numeric) ON COMMIT DROP; +""" + +_WINDOW_MONTHS = 12 + +# Внутри окна: 2 заявки, обе со сделками по 1 000 000. +# Снаружи (три года назад): 3 заявки, сделки по 5 000 000 — итог заведомо не равен окну. +_IN_WINDOW_LEADS = 2 +_OUT_WINDOW_LEADS = 3 +_ALL_TIME_LEADS = _IN_WINDOW_LEADS + _OUT_WINDOW_LEADS +_IN_WINDOW_REVENUE = 2_000_000.0 +_IN_WINDOW_DEALS = 2 + + +@pytest.fixture +def db(): + engine = create_engine(_dsn()) + session = sessionmaker(bind=engine)() + try: + session.execute(text(_SCHEMA)) + for table in ("prinzip_leads", "prinzip_deals"): + n = session.execute(text(f"SELECT count(*) FROM {table}")).scalar() + assert n == 0, ( + f"{table}: запрос попал НЕ во временную таблицу ({n} строк) — " + "тест читал бы боевые данные" + ) + yield session + finally: + session.rollback() + session.close() + engine.dispose() + + +@pytest.fixture +def seeded(db): + rows = [ + (1, "0 days", "site", True, 101, 1_000_000), + (2, "10 days", "site", True, 102, 1_000_000), + (3, "1100 days", "avito", True, 103, 5_000_000), + (4, "1101 days", "avito", True, 104, 5_000_000), + (5, "1102 days", "vk", True, 105, 5_000_000), + ] + for lead_id, ago, source, converted, deal_id, price in rows: + db.execute( + text( + "INSERT INTO prinzip_leads (lead_id, created_at, source, converted, deal_id)" + " VALUES (:l, NOW() - CAST(:ago AS interval), :s, :c, :d)" + ), + {"l": lead_id, "ago": ago, "s": source, "c": converted, "d": deal_id}, + ) + db.execute( + text("INSERT INTO prinzip_deals (deal_id, deal_price) VALUES (:d, :p)"), + {"d": deal_id, "p": price}, + ) + return db + + +def _stats(db) -> dict: + from app.api.v1.admin_leads import leads_stats + + return leads_stats(db=db, months=_WINDOW_MONTHS) + + +def test_window_suffixed_fields_match_the_window(seeded) -> None: + """Всё, что названо `_window`, обязано считаться по окну. + + На origin/main эти величины лежат под именами `revenue_total`/`deals_total`, + поэтому проверка ниже (по `_total`) и краснеет — здесь же контроль, что + оконные значения не поехали. + """ + stats = _stats(seeded) + assert stats["leads_window"] == _IN_WINDOW_LEADS + assert stats["converted_window"] == _IN_WINDOW_LEADS + + +def test_total_suffixed_fields_are_all_time(seeded) -> None: + """КАЖДОЕ поле `*_total` обязано быть за всё время, а не за окно. + + На origin/main `revenue_total` = 2 000 000 (только окно) при итоге 17 000 000, + и `deals_total` = 2 при итоге 5 — красное по неверному ЗНАЧЕНИЮ. + """ + stats = _stats(seeded) + all_time_revenue = float( + seeded.execute(text("SELECT COALESCE(SUM(deal_price), 0) FROM prinzip_deals")).scalar() + ) + all_time_deals = int(seeded.execute(text("SELECT COUNT(*) FROM prinzip_deals")).scalar()) + expected = { + "leads_total": _ALL_TIME_LEADS, + "revenue_total": all_time_revenue, + "deals_total": all_time_deals, + } + + for key, value in stats.items(): + if not key.endswith("_total"): + continue + if key not in expected: + continue + assert value == expected[key], ( + f"поле {key!r} обещает суффиксом величину за ВСЁ время, а равно {value} " + f"при итоге {expected[key]} — это цифра за окно {_WINDOW_MONTHS} мес" + ) + + +def test_revenue_and_deals_are_named_by_their_scope(seeded) -> None: + """Выручка и сделки должны нести суффикс, соответствующий их охвату. + + Отдельно от предыдущего: там проверяется значение под именем, здесь — что имя + вообще выбрано по охвату. Ловит «починку», которая оставила бы `_total` и + просто перестала показывать поле в UI. + """ + stats = _stats(seeded) + assert ( + stats.get("revenue_window") == _IN_WINDOW_REVENUE + ), f"revenue_window = {stats.get('revenue_window')}, ожидалось {_IN_WINDOW_REVENUE}" + assert stats.get("deals_window") == _IN_WINDOW_DEALS + + +def test_window_months_present_on_empty_data(db) -> None: + """Контроль: на пустых данных ответ сохраняет форму и ширину окна. + + Оговорка, чтобы тест не читался как покрытие ветки `if not row`: он туда НЕ + попадает. Запрос агрегатный и на пустых таблицах возвращает обычную строку + (leads_total=0, leads_window=0), поэтому исполняется основная ветка. Ветка + пустого ответа сегодня недостижима — её согласованность правится вслепую, + и проверить её этим тестом нельзя. + """ + stats = _stats(db) + assert "window_months" in stats, f"нет window_months в пустом ответе: {sorted(stats)}" + assert stats["window_months"] == _WINDOW_MONTHS diff --git a/frontend/src/app/admin/leads/page.tsx b/frontend/src/app/admin/leads/page.tsx index 7c7e96c6..db159983 100644 --- a/frontend/src/app/admin/leads/page.tsx +++ b/frontend/src/app/admin/leads/page.tsx @@ -62,8 +62,8 @@ interface LeadsStats { converted_window: number; conv_pct_window: number | null; sources_total: number; - revenue_total: number | null; - deals_total: number; + revenue_window: number | null; + deals_window: number; window_months: number; } @@ -344,9 +344,9 @@ export default function AdminLeadsPage() { } /> diff --git a/frontend/src/lib/api-types.ts b/frontend/src/lib/api-types.ts index 5936c42a..a93c83cb 100644 --- a/frontend/src/lib/api-types.ts +++ b/frontend/src/lib/api-types.ts @@ -2048,6 +2048,8 @@ export interface paths { /** * Leads Stats * @description KPI summary за последние N месяцев. + * + * Суффикс `_window` — за окно `months`, `_total` — за всё время. */ get: operations["leads_stats_api_v1_admin_leads_stats_get"]; put?: never;