diff --git a/tradein-mvp/backend/app/services/exporters/trade_in_pdf.py b/tradein-mvp/backend/app/services/exporters/trade_in_pdf.py index 341fea48..7c5125ae 100644 --- a/tradein-mvp/backend/app/services/exporters/trade_in_pdf.py +++ b/tradein-mvp/backend/app/services/exporters/trade_in_pdf.py @@ -290,8 +290,9 @@ matplotlib.rcParams["font.family"] = "Manrope" # ── Source pseudo-logos (текстовые pill-badges с брендовыми цветами источников) ─ # Неизвестный/снятый source (напр. историческое inactive 'n1', #2204) рендерится -# через безопасный fallback в _source_pill: серый фон + source.title() — код на -# исторических строках НЕ падает. Цвета источников — реальные бренд-цвета +# через безопасный fallback в _source_pill: серый фон + «Другой источник» (#3341 — +# сырой id тоже мог бы утечь именем площадки, source.title() больше не используется) +# — код на исторических строках НЕ падает. Цвета источников — реальные бренд-цвета # сторонних площадок (Avito/Циан/...), НЕ часть внутренней design-системы — # намеренно не конвертируются в наши OKLCH-токены. _SOURCE_LOGO_COLORS: dict[str, tuple[str, str]] = { @@ -303,15 +304,59 @@ _SOURCE_LOGO_COLORS: dict[str, tuple[str, str]] = { "etazhi": ("#e30613", "#fff"), # Этажи красный } +# Публичные лейблы — канон `frontend/src/lib/source-registry.ts::SOURCES` (#3341, +# класс #3264): клиентский PDF не должен называть площадки-источники так же, как +# веб-отчёт и лендинг перестали делать это с 31.08. Ключи — ТОЛЬКО базовые id +# (алиасы валюации/написания резолвятся через _SOURCE_CANONICAL/_canonical_source +# ДО обращения сюда — иначе два источника правды для одной площадки). _SOURCE_DISPLAY_NAMES: dict[str, str] = { - "avito": "Avito", - "cian": "Циан", - "domklik": "Домклик · Сбер", - "yandex": "Я.Недвижимость", + "avito": "Источник 1", + "cian": "Источник 2", + "yandex": "Источник 3", + "domklik": "Источник 4", + "etazhi": "Источник 5", "rosreestr": "Росреестр", - "etazhi": "Этажи", } +# Алиас → базовый id. Зеркалит группировку publicLabel/dot в +# frontend/src/lib/source-registry.ts (одна площадка = один номер/цвет там же); +# правишь один реестр — проверь другой (#3341 review: без канонизации +# estimate.sources_used = [avito, avito_imv, cian, cian_valuation, ...] +# (estimator.py `_canonical_sources`, listing+valuation union) рендерил +# ДВЕ одинаковые пилюли «Источник 1, Источник 1» и серую точку у алиаса — +# ни avito_imv/cian_valuation/yandex_valuation/domclick/etagi не было ключом +# ни в _SOURCE_LOGO_COLORS, ни (после дедупа лейблов) в _SOURCE_DISPLAY_NAMES). +_SOURCE_CANONICAL: dict[str, str] = { + "avito_imv": "avito", + "cian_valuation": "cian", + "yandex_valuation": "yandex", + "domclick": "domklik", + "etagi": "etazhi", +} + + +def _canonical_source(source: str) -> str: + """Алиас (valuation-вариант / альтернативное написание) → базовый id площадки.""" + return _SOURCE_CANONICAL.get(source, source) + + +def _public_sources(ids: list[str]) -> list[str]: + """Канонизирует id и дедуплицирует с сохранением порядка первого появления. + + `estimate.sources_used` — отсортированное объединение listing-id и их + valuation-алиасов (estimator.py `_canonical_sources`), напр. + `[avito, avito_imv, cian, cian_valuation, domklik, yandex, yandex_valuation]`. + Без канонизации+дедупа ДО среза `[:5]` рендерились дубли лейблов и «Источник 3» + (yandex) мог быть вытеснен алиасом другой площадки (#3341 review).""" + seen: set[str] = set() + result: list[str] = [] + for raw in ids: + canon = _canonical_source(raw) + if canon not in seen: + seen.add(canon) + result.append(canon) + return result + def _source_logo_pill(source: str) -> str: """Source pill — мягкий HUD-чип (.source-pill, ObjectSummary.tsx dot-идиома): @@ -319,10 +364,12 @@ def _source_logo_pill(source: str) -> str: непрозрачный цветной чип; теперь единая с остальным документом мягкая палитра (Mera v2), бренд-цвет остаётся только акцентной точкой. Название — из _SOURCE_DISPLAY_NAMES (короткий bounded набор, max ~18 символов) — - overflow-wrap на всякий случай, если source незнаком и попадёт .title() - произвольной длины (см. fallback ниже).""" - dot = _SOURCE_LOGO_COLORS.get(source, (_MUTED, "#fff"))[0] - name = _SOURCE_DISPLAY_NAMES.get(source, source.title()) + overflow-wrap на всякий случай, если source незнаком (см. fallback ниже). + Канонизирует алиасы (avito_imv → avito и т.п.) ДО lookup, чтобы цвет/лейбл + совпадали с базовой площадкой (#3341 review).""" + canon = _canonical_source(source) + dot = _SOURCE_LOGO_COLORS.get(canon, (_MUTED, "#fff"))[0] + name = _SOURCE_DISPLAY_NAMES.get(canon, "Другой источник") return ( "—" - bg, fg = _SOURCE_LOGO_COLORS.get(source, (_MUTED, "#fff")) - name = _SOURCE_DISPLAY_NAMES.get(source, source.title()) + canon = _canonical_source(source) + bg, fg = _SOURCE_LOGO_COLORS.get(canon, (_MUTED, "#fff")) + name = _SOURCE_DISPLAY_NAMES.get(canon, "Другой источник") return ( f"{_html.escape(name)}" @@ -1167,12 +1216,12 @@ def _build_cover(estimate: AggregatedEstimate, input_snapshot: dict, brand) -> s advice_discount_text = ( f"Фактические сделки проходят ниже цен в объявлениях — по этому объекту " f"на {discount_pct}% (см. «Ожидаемая цена продажи»); подтверждают Росреестр, " - f"ДомКлик и продажи агентств недвижимости" + f"сделки площадок и продажи агентств недвижимости" ) else: advice_discount_text = ( "Фактические сделки проходят ниже цен в объявлениях, что подтверждают " - "Росреестр, ДомКлик и продажи агентств недвижимости" + "Росреестр, сделки площадок и продажи агентств недвижимости" ) disclaimer_html = "" @@ -1357,7 +1406,11 @@ def _build_listings_page(estimate: AggregatedEstimate, input_snapshot: dict, bra # count «с учётом ремонта» не существует, второе число было идентично n_total. # Source logos (pseudo) — берём из estimate.sources_used (не захардкоженный список). - sources_to_show = estimate.sources_used or [] + # #3341 review: sources_used — union listing+valuation алиасов (estimator.py + # `_canonical_sources`), напр. [avito, avito_imv, cian, cian_valuation, ...] — + # _public_sources канонизирует+дедуплицирует ДО среза [:5], иначе дубли пилюль + # и «Источник 3» мог быть вытеснен алиасом другой площадки. + sources_to_show = _public_sources(estimate.sources_used or []) sources_html = "".join(_source_logo_pill(s) for s in sources_to_show[:5]) # Params правой колонки — параметры поиска (НЕ конкретной квартиры) @@ -1598,10 +1651,13 @@ def _build_deals_page(estimate: AggregatedEstimate, input_snapshot: dict, brand) # Источники для сделок — берём из estimate.sources_used (не захардкоженный список). # Фильтруем по известным источникам сделок; fallback к пустому (не fabricate). + # #3341 review: канонизация+дедуп (_public_sources) ДО фильтра/среза [:5] — те же + # причины, что и на странице листингов (см. коммент там). _deal_source_keys = {"etazhi", "domklik", "rosreestr"} - deal_sources = [s for s in (estimate.sources_used or []) if s in _deal_source_keys] + canonical_sources = _public_sources(estimate.sources_used or []) + deal_sources = [s for s in canonical_sources if s in _deal_source_keys] if not deal_sources: - deal_sources = [s for s in (estimate.sources_used or [])] + deal_sources = canonical_sources sources_html = "".join(_source_logo_pill(s) for s in deal_sources[:5]) area = float(input_snapshot.get("area_m2", 0) or 0) @@ -1847,7 +1903,7 @@ def _build_offer_page(estimate: AggregatedEstimate, input_snapshot: dict, brand)
Расходы на рекламу
Ежемесячное базовое продвижение объекта - на Циан, Авито, Я.Недвижимости
+ на основных площадках объявлений бесплатно diff --git a/tradein-mvp/backend/tests/test_pdf_public_source_labels.py b/tradein-mvp/backend/tests/test_pdf_public_source_labels.py new file mode 100644 index 00000000..2be05837 --- /dev/null +++ b/tradein-mvp/backend/tests/test_pdf_public_source_labels.py @@ -0,0 +1,267 @@ +"""Gate test (#3341): клиентский PDF не должен называть реальные площадки-источники. + +Канон — `frontend/src/lib/source-registry.ts::SOURCES` (публичные лейблы, закреплены +для веб-отчёта/лендинга с 31.08, #3264-scope): avito/avito_imv → «Источник 1», +cian/cian_valuation → «Источник 2», yandex/yandex_valuation → «Источник 3», +domklik/domclick → «Источник 4», etagi/etazhi → «Источник 5», rosreestr → «Росреестр», +неизвестный id → «Другой источник» (НЕ сырой id — им тоже может утечь имя площадки). + +WeasyPrint стаблен (как в test_pdf_security.py) — тесты дергают только HTML-билдеры. +""" + +from __future__ import annotations + +import os +import re +import sys +from datetime import UTC, datetime, timedelta +from uuid import uuid4 + +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") + +from unittest.mock import MagicMock + +_wp_mock = MagicMock() +sys.modules.setdefault("weasyprint", _wp_mock) + +import pytest # noqa: E402 + +from app.schemas.trade_in import AggregatedEstimate, AnalogLot # noqa: E402 +from app.services.brand import Brand # noqa: E402 +from app.services.exporters import trade_in_pdf as mod # noqa: E402 + +_GENERIC = Brand( + slug="generic", + name="Trade-In", + logo_url=None, + primary_color="#1d4ed8", + accent_color="#f59e0b", + footer_text=None, + pdf_disclaimer=None, +) + +_SNAPSHOT = { + "address": "Екатеринбург, ул. Ленина, 1", + "area_m2": 50.0, + "rooms": 2, + "floor": 3, + "total_floors": 9, + "year_built": 2010, + "house_type": "panel", + "repair_state": "standard", + "has_balcony": True, +} + +# Все известные id источников (листинги + valuation-варианты + сделки) + неизвестный. +_ALL_SOURCE_IDS = [ + "avito", + "avito_imv", + "cian", + "cian_valuation", + "yandex", + "yandex_valuation", + "domklik", + "domclick", + "etazhi", + "rosreestr", +] + +# Запрещённые в видимом тексте PDF реальные имена площадок (#pdf-honesty/#3264). +_FORBIDDEN_NAMES = [ + "Avito", + "AVITO", + "Авито", + "Циан", + "ЦИАН", + "Cian", + "ДомКлик", + "Домклик", + "ДОМКЛИК", + "Domclick", + "Я.Недвижимость", + "Яндекс", + "Yandex", + "Этажи", + "Avito_Imv", + "Rosreestr", +] + + +def _analog(**overrides) -> AnalogLot: + base = dict( + address="ул. Тест, 1", + area_m2=50.0, + rooms=2, + floor=3, + total_floors=9, + price_rub=5_000_000, + price_per_m2=100_000, + listing_date=None, + days_on_market=None, + source="avito", + source_url="https://avito.ru/1", + distance_m=200, + ) + base.update(overrides) + return AnalogLot(**base) + + +def _estimate(**overrides) -> AggregatedEstimate: + base = dict( + estimate_id=uuid4(), + median_price_rub=10_000_000, + range_low_rub=9_000_000, + range_high_rub=11_000_000, + median_price_per_m2=200_000, + confidence="high", + n_analogs=15, + period_months=24, + analogs=[], + actual_deals=[], + expires_at=datetime.now(UTC) + timedelta(days=30), + ) + base.update(overrides) + return AggregatedEstimate(**base) + + +def _strip_tags(html: str) -> str: + """Видимый текст без разметки — href/src (легальные URL объявлений) не проверяем.""" + return re.sub(r"<[^>]+>", " ", html) + + +# ── Part A: helper-функции напрямую (source-pill / badge fallback) ─────────── + + +@pytest.mark.parametrize("forbidden", _FORBIDDEN_NAMES) +def test_source_logo_pill_never_leaks_forbidden_names(forbidden: str) -> None: + """Ни один известный id, ни неизвестный не рендерят реальное имя площадки.""" + html = "".join(mod._source_logo_pill(sid) for sid in [*_ALL_SOURCE_IDS, "n1"]) + assert forbidden not in html + + +@pytest.mark.parametrize("forbidden", _FORBIDDEN_NAMES) +def test_source_badge_inline_never_leaks_forbidden_names(forbidden: str) -> None: + html = "".join(mod._source_badge_inline(sid) for sid in [*_ALL_SOURCE_IDS, "n1"]) + assert forbidden not in html + + +def test_source_logo_pill_known_ids_map_to_public_labels() -> None: + assert "Источник 1" in mod._source_logo_pill("avito") + assert "Источник 1" in mod._source_logo_pill("avito_imv") + assert "Источник 2" in mod._source_logo_pill("cian") + assert "Источник 2" in mod._source_logo_pill("cian_valuation") + assert "Источник 3" in mod._source_logo_pill("yandex") + assert "Источник 3" in mod._source_logo_pill("yandex_valuation") + assert "Источник 4" in mod._source_logo_pill("domklik") + assert "Источник 4" in mod._source_logo_pill("domclick") + assert "Источник 5" in mod._source_logo_pill("etazhi") + assert "Источник 5" in mod._source_logo_pill("etagi") + assert "Росреестр" in mod._source_logo_pill("rosreestr") + + +def test_source_logo_pill_unknown_id_falls_back_to_other_source() -> None: + """Неизвестный source → «Другой источник», НЕ сырой id (сам тоже мог бы назвать площадку).""" + html = mod._source_logo_pill("something_new") + assert "Другой источник" in html + assert "Something_New" not in html + assert "something_new" not in html + + +def test_source_badge_inline_unknown_id_falls_back_to_other_source() -> None: + html = mod._source_badge_inline("something_new") + assert "Другой источник" in html + assert "Something_New" not in html + assert "something_new" not in html + + +# ── Part B: билдеры страниц (listings/deals/cover/offer) end-to-end ────────── + + +def test_listings_page_all_sources_no_forbidden_names() -> None: + analogs = [ + _analog(source=sid, source_url=f"https://example.org/{i}", distance_m=i * 10) + for i, sid in enumerate([*_ALL_SOURCE_IDS, "n1"]) + ] + est = _estimate(sources_used=_ALL_SOURCE_IDS, analogs=analogs, n_analogs=len(analogs)) + html = mod._build_listings_page(est, _SNAPSHOT, _GENERIC) + text = _strip_tags(html) + for forbidden in _FORBIDDEN_NAMES: + assert forbidden not in text, f"{forbidden!r} leaked into listings page" + # Listings page shows only listing sources (first 5 of sources_used/analogs by + # distance) — rosreestr (deals) legitimately doesn't reach it here, see deals-page + # test below for that assertion. + assert "Источник 1" in text + + +def test_listings_page_dedupes_alias_sources_into_single_pill() -> None: + """estimator.py `_canonical_sources` — sources_used — отсортированное union + listing-id + их valuation-алиасов, напр. + [avito, avito_imv, cian, cian_valuation, domklik, yandex, yandex_valuation] + (#3341 review). Листинги должны рендерить РОВНО одну пилюлю на канонический + источник (не по одной на алиас), и алиас обязан нести цвет точки базовой + площадки, не серый fallback.""" + sources_used = [ + "avito", + "avito_imv", + "cian", + "cian_valuation", + "domklik", + "yandex", + "yandex_valuation", + ] + est = _estimate(sources_used=sources_used) + html = mod._build_listings_page(est, _SNAPSHOT, _GENERIC) + for label in ("Источник 1", "Источник 2", "Источник 3", "Источник 4"): + count = html.count(label) + assert count == 1, f"{label!r} should render exactly once, got {count}" + # avito_imv (valuation-алиас) обязан нести тот же бренд-цвет точки, что и avito. + assert "#00aaff" in mod._source_logo_pill("avito") + assert "#00aaff" in mod._source_logo_pill("avito_imv") + + +def test_deals_page_all_sources_no_forbidden_names() -> None: + deals = [ + _analog(source=sid, source_url=f"https://example.org/{i}", distance_m=i * 10) + for i, sid in enumerate(["etazhi", "domklik", "rosreestr", "n1"]) + ] + est = _estimate( + sources_used=["etazhi", "domklik", "rosreestr"], + actual_deals=deals, + ) + html = mod._build_deals_page(est, _SNAPSHOT, _GENERIC) + text = _strip_tags(html) + for forbidden in _FORBIDDEN_NAMES: + assert forbidden not in text, f"{forbidden!r} leaked into deals page" + assert "Росреестр" in text + + +def test_cover_advice_discount_text_no_forbidden_names_without_pct() -> None: + """Ветка без expected_sold_price_rub (_discount_pct → None) — только else-текст.""" + est = _estimate() + html = mod._build_cover(est, _SNAPSHOT, _GENERIC) + text = _strip_tags(html) + assert "ДомКлик" not in text + assert "сделки площадок" in text + assert "Росреестр" in text + + +def test_cover_advice_discount_text_no_forbidden_names_with_pct() -> None: + """С expected_sold_price_rub _discount_pct возвращает %, покрываем именно + эту f-строку (review #3341: без sold-цены тест не доходил до неё).""" + est = _estimate(expected_sold_price_rub=9_000_000) + html = mod._build_cover(est, _SNAPSHOT, _GENERIC) + text = _strip_tags(html) + assert "на 10%" in text + assert "сделки площадок" in text + assert "ДомКлик" not in text + assert "Росреестр" in text + + +def test_offer_page_advertising_row_no_brand_names() -> None: + est = _estimate() + html = mod._build_offer_page(est, _SNAPSHOT, _GENERIC) + text = _strip_tags(html) + assert "на основных площадках объявлений" in text + assert "Циан" not in text + assert "Авито" not in text + assert "Я.Недвижимости" not in text diff --git a/tradein-mvp/backend/tests/test_pdf_security.py b/tradein-mvp/backend/tests/test_pdf_security.py index 7789b8e0..633133c7 100644 --- a/tradein-mvp/backend/tests/test_pdf_security.py +++ b/tradein-mvp/backend/tests/test_pdf_security.py @@ -381,13 +381,14 @@ def test_listings_page_no_crash_with_no_sources() -> None: def test_listings_sources_from_estimate_not_hardcoded() -> None: - """sources_used=['yandex'] → только yandex pill, не avito/cian/domklik/...""" + """sources_used=['yandex'] → только источник 3 (yandex) pill, не 1/2/4/... (#3341: + публичные лейблы, не реальные названия площадок).""" est = _estimate(sources_used=["yandex"]) html = mod._build_listings_page(est, _SNAPSHOT, _GENERIC) - assert "Я.Недвижимость" in html + assert "Источник 3" in html # avito and cian should NOT appear if not in sources_used - assert "Avito" not in html - assert "Циан" not in html + assert "Источник 1" not in html + assert "Источник 2" not in html def test_deals_sources_from_estimate_not_hardcoded() -> None: @@ -395,9 +396,9 @@ def test_deals_sources_from_estimate_not_hardcoded() -> None: est = _estimate(sources_used=["rosreestr"]) html = mod._build_deals_page(est, _SNAPSHOT, _GENERIC) assert "Росреестр" in html - # Этажи/Домклик — не должны появиться - assert "Этажи" not in html - assert "Домклик" not in html + # источник 5 (этажи) / источник 4 (домклик) — не должны появиться (#3341) + assert "Источник 5" not in html + assert "Источник 4" not in html def test_examples_rows_none_price_per_m2_no_crash() -> None: