From 48664dfe0e7929be804260b4775b6c915cc0d1bb Mon Sep 17 00:00:00 2001 From: bot-backend Date: Thu, 6 Aug 2026 22:49:09 +0300 Subject: [PATCH] =?UTF-8?q?fix(tradein/payments):=20pre-flight=20=D0=B4?= =?UTF-8?q?=D0=BE=D0=BB=D0=B6=D0=B5=D0=BD=20=D0=BB=D0=BE=D0=B2=D0=B8=D1=82?= =?UTF-8?q?=D1=8C=20=D0=B0=D0=BD=D0=BE=D0=BC=D0=B0=D0=BB=D0=B8=D1=8E,=20?= =?UTF-8?q?=D0=BD=D0=B5=20=D1=88=D1=82=D0=B0=D1=82=D0=BD=D0=BE=D0=B5=20?= =?UTF-8?q?=D1=81=D0=BE=D1=81=D1=82=D0=BE=D1=8F=D0=BD=D0=B8=D0=B5=20(revie?= =?UTF-8?q?w=20PR=20#2754)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deep-review MEDIUM: предполётная проверка purge_expired_trade_in_data считала по базовому предикату без retain_until — здоровая оплаченная строка (retain_until проставлен, платёж есть) через сутки после продажи тоже попадала под счётчик, и джоба аварийно останавливалась на первой же честной продаже навсегда (вместе с ней — и 180-дневное удаление лидов, вызываемое из той же функции после этой проверки). - _PREFLIGHT_PAID_CANDIDATES_SQL: добавлен терм `retain_until IS NULL` — теперь считает только реальную аномалию (retain_until не проставлен, а платёж есть), а не штатное состояние. Докстринги функции/модуля поправлены под фактическое поведение. - Тест на неверный инвариант (`"retain_until" not in sql`) заменён на позитивный (`"retain_until IS NULL" in sql`) + добавлены live-DB тесты на оба случая из ревью (здоровая оплаченная строка не поднимает тревогу, джоба не блокируется). - privacy/page.tsx: константа "12 месяцев" вынесена в content.ts (PAID_REPORT_RETENTION_MONTHS) вместо литерала + расходящегося комментария; добавлен сверяющий тест (test_paid_retention_text_consistency.py) по образцу _CONSENT_TEXT_SNAPSHOT. Смягчена формулировка про автоматическое удаление — задача на проде выключена и ни разу не запускалась, текст теперь описывает установленный порядок, а не наблюдаемый факт. - Все 10 висячих ссылок на untracked `mera-pr-d-spec.md` (7 файлов) заменены на краткое изложение сути в комментарии + ссылку на PR #2754. --- tradein-mvp/backend/app/api/v1/trade_in.py | 2 +- tradein-mvp/backend/app/core/config.py | 19 +- .../app/tasks/purge_expired_trade_in_data.py | 91 +++++--- .../234_trade_in_estimates_retain_until.sql | 5 +- .../backend/tests/test_estimate_idor.py | 2 +- .../test_paid_retention_text_consistency.py | 103 +++++++++ .../tests/test_purge_expired_trade_in_data.py | 206 ++++++++++++++++-- .../frontend/src/app/mera-public/content.ts | 19 ++ .../src/app/mera-public/privacy/page.tsx | 29 ++- 9 files changed, 401 insertions(+), 75 deletions(-) create mode 100644 tradein-mvp/backend/tests/test_paid_retention_text_consistency.py diff --git a/tradein-mvp/backend/app/api/v1/trade_in.py b/tradein-mvp/backend/app/api/v1/trade_in.py index 05950b0c..3bc7c08d 100644 --- a/tradein-mvp/backend/app/api/v1/trade_in.py +++ b/tradein-mvp/backend/app/api/v1/trade_in.py @@ -58,7 +58,7 @@ router = APIRouter() # неизбежно без унификации. `retain_until > NOW()` при NULL даёт NULL → false # в SQL — для всех существующих строк (retain_until IS NULL) поведение не # меняется вообще. Не копировать это выражение по месту — только через -# константу/хелпер ниже. См. `mera-pr-d-spec.md` §1.3 в корне репо. +# константу/хелпер ниже. Payments retention, PR #2754. ESTIMATE_READABLE_SQL = "(expires_at > NOW() OR retain_until > NOW())" diff --git a/tradein-mvp/backend/app/core/config.py b/tradein-mvp/backend/app/core/config.py index 9a9b4ebf..eb70c787 100644 --- a/tradein-mvp/backend/app/core/config.py +++ b/tradein-mvp/backend/app/core/config.py @@ -836,15 +836,20 @@ class Settings(BaseSettings): # срок — решение DPO/юриста, не инженера). ENV: TRADE_IN_LEAD_RETENTION_DAYS. trade_in_lead_retention_days: int = 180 - # ── PR-D1: платный отчёт живёт год (retain_until, migration 234) ──────── + # ── Платный отчёт живёт год (retain_until, migration 234, PR #2754) ───── # trade_in_estimates.retain_until TTL (дни ОТ ОПЛАТЫ) — срок жизни ССЫЛКИ/ # СТРОКИ для оплаченной оценки, независимый от expires_at (актуальность - # расчёта, 24ч). НЕ трогает expires_at — см. migration 234 докстринг и - # `mera-pr-d-spec.md` §1.1/§1.2 в корне репо. Единственный источник числа - # «12 месяцев»: текст оферты (content.ts), текст экрана S4 и SQL продления - # retain_until при оплате (платёжный код, отдельный PR) обязаны читать его - # отсюда, а не хардкодить — иначе классический исход "в оферте 12 месяцев, - # в конфиге 365 дней, на экране «год»". ENV: TRADE_IN_PAID_RETENTION_DAYS. + # расчёта, 24ч, глобальный для ВСЕХ строк). НЕ трогает expires_at — см. + # migration 234 докстринг. Отдельная колонка, а не подъём expires_at: + # expires_at печатается в PDF/UI как «актуальность расчёта» и одинаков + # для всех строк, поднять его до года = соврать в документе клиента про + # свежесть цифры + нарушить минимизацию ПДн для неоплаченных B2C-адресов. + # Единственный источник числа «12 месяцев» на фронте — + # `mera-public/content.ts::PAID_REPORT_RETENTION_MONTHS`; текст оферты, + # экран после оплаты и SQL продления retain_until при оплате (платёжный + # код, отдельный PR) обязаны читать его оттуда, а не хардкодить — иначе + # классический исход "в оферте 12 месяцев, в конфиге 365 дней, на экране + # «год»". ENV: TRADE_IN_PAID_RETENTION_DAYS. trade_in_paid_retention_days: int = 365 # Батч-размер физического DELETE в purge_expired_trade_in_data (нельзя одним diff --git a/tradein-mvp/backend/app/tasks/purge_expired_trade_in_data.py b/tradein-mvp/backend/app/tasks/purge_expired_trade_in_data.py index 5a96989c..c48e8dba 100644 --- a/tradein-mvp/backend/app/tasks/purge_expired_trade_in_data.py +++ b/tradein-mvp/backend/app/tasks/purge_expired_trade_in_data.py @@ -55,16 +55,20 @@ BATCHING (не единый DELETE по всей таблице): committed batches deleted (correct, not rolled back) and mark_failed records the partial counters reached so far. -PR-D1 (2026-08-06, payments retention -- see `mera-pr-d-spec.md` §1 at repo root): - the `created_by IS NULL` population above is EXACTLY the future paying-customer - population -- the owner sells this report to individuals for money, and a paid - row must outlive the 24h `expires_at` link TTL. Two independent safeguards were - added to `_DELETE_EXPIRED_ESTIMATES_SQL` (retain_until IS NULL + NOT EXISTS - payments) plus a pre-flight count in `purge_expired_trade_in_data` that refuses - to run at all if it finds a paid candidate -- see the SQL constants and - `_preflight_paid_candidates` below for the mechanics. No payment code lives in - this file; `retain_until` is set by the (separate, not-yet-existing) payment - fulfillment code. +Payments retention (PR #2754): the `created_by IS NULL` population above is EXACTLY + the future paying-customer population -- the owner sells this report to + individuals for money, and a paid row must outlive the 24h `expires_at` link TTL + (a separate column, `retain_until`, set by the -- separate, not-yet-existing -- + payment fulfillment code to now() + settings.trade_in_paid_retention_days, NOT + a change to `expires_at` itself). Two independent safeguards were added to + `_DELETE_EXPIRED_ESTIMATES_SQL` (retain_until IS NULL + NOT EXISTS payments) + plus a pre-flight count in `purge_expired_trade_in_data` that refuses to run at + all if it finds an ANOMALOUS paid candidate -- see the SQL constants and + `_preflight_paid_candidates` below for the mechanics (deep-review finding + 2026-08-06 MEDIUM on PR #2754: the pre-flight predicate itself must ALSO carry + `retain_until IS NULL`, otherwise a perfectly healthy paid row trips it and + wedges the job permanently -- see that function's docstring). No payment code + lives in this file. """ from __future__ import annotations @@ -86,9 +90,14 @@ logger = logging.getLogger(__name__) _DEFAULT_MAX_BATCHES = 20 # -# PR-D1 (2026-08-06): два независимые предохранителя добавлены к тому же -# предикату, ПЕРЕД тем как платёжный код появился в проекте (мина уже была -# заряжена, см. `mera-pr-d-spec.md` §1 в корне репо): +# Payments retention (2026-08-06, PR #2754): два независимых предохранителя +# добавлены к тому же предикату ПЕРЕД тем, как платёжный код появился в +# проекте (мина уже была заряжена: без них джоба удаляла бы будущих платящих +# клиентов). Отдельная колонка retain_until (не подъём expires_at) — потому +# что expires_at глобальный TTL расчёта на ВСЕ строки (включая неоплаченные) +# и печатается в PDF/UI как «актуальность расчёта»; поднять его до года +# означало бы одновременно нарушить минимизацию ПДн по 152-ФЗ и соврать в +# документе клиента про срок актуальности цифры: # 1. `retain_until IS NULL` — именно IS NULL, НЕ `< NOW()`. Оплаченная # строка (retain_until IS NOT NULL, migration 234) не удаляется джобой # В ПРИНЦИПЕ, пока не поднято ослабление отдельным PR не раньше чем @@ -117,17 +126,25 @@ _DELETE_EXPIRED_ESTIMATES_SQL = text( """ ) -# PR-D1 pre-flight (см. _preflight_paid_candidates): считает по БАЗОВОМУ -# (пред-PR-D1) предикату purge -- `expires_at < NOW() AND created_by IS NULL`, -# БЕЗ retain_until/NOT EXISTS -- специально ШИРЕ итогового DELETE-предиката -# выше, чтобы поймать именно случай "retain_until не проставлен, а деньги -# были" (а не только штатно защищённые retain_until IS NOT NULL строки, -# которые и так не попали бы под DELETE). +# Pre-flight (см. _preflight_paid_candidates). deep-review finding 2026-08-06 +# MEDIUM (PR #2754): первая редакция считала по БАЗОВОМУ предикату БЕЗ +# retain_until вообще -- а это ловит и штатно-здоровые оплаченные строки +# (retain_until проставлен, есть payments) точно так же, как настоящую +# аномалию (retain_until НЕ проставлен, но payments есть) -- джоба вставала +# на первой же честной продаже и больше никогда не запускалась (вместе с ней +# вставало и удаление лидов, вызываемое из той же функции ПОСЛЕ этой +# проверки -- 180-дневный purge по 152-ФЗ тоже переставал бы работать). +# Правильная форма: базовый предикат AND "новый предохранитель НЕ сработал +# бы" (retain_until IS NULL) AND "признак аномалии" (payments всё же есть). +# Здоровая оплаченная строка (retain_until IS NOT NULL) исключается ЭТИМ +# термом -- она и так под DELETE не попадает (см. safeguard 1 выше), тревогу +# поднимать не должна. _PREFLIGHT_PAID_CANDIDATES_SQL = text( """ SELECT count(*) FROM trade_in_estimates e WHERE e.expires_at < NOW() AND e.created_by IS NULL + AND e.retain_until IS NULL AND EXISTS (SELECT 1 FROM payments p WHERE p.estimate_id = e.id) """ ) @@ -181,14 +198,20 @@ def _drain_expired( def _preflight_paid_candidates(db: Session) -> int: - """PR-D1 safety gate: count purge-candidates (base predicate) that have a payments row. + """Safety gate: count ANOMALOUS purge-candidates -- base predicate, retain_until + IS NULL (safeguard 1 did NOT protect the row), AND a payments row exists anyway. - Runs BEFORE any DELETE batch. A non-zero result means at least one estimate that - would have matched the OLD (pre-PR-D1) purge predicate was actually touched by - money -- either `retain_until` failed to be set (fulfillment bug/race/manual - INSERT) or something inconsistent happened. Either way this run must not delete - anything; see `purge_expired_trade_in_data` below, which aborts before the first - batch when this returns non-zero. + Runs BEFORE any DELETE batch. A non-zero result means fulfillment failed to set + `retain_until` on a row money actually touched (bug/race/manual INSERT) -- this + run must not delete anything; see `purge_expired_trade_in_data` below, which + aborts before the first batch when this returns non-zero. + + MUST include `retain_until IS NULL` (deep-review finding 2026-08-06 MEDIUM, PR + #2754): a healthy paid row (retain_until set, has a payments row) is the EXPECTED + steady state one day after every sale -- without this term it counts as a "paid + candidate" too, so the very first successful sale permanently wedges this job + (mark_failed, zero deletions, forever -- and since leads purge runs from the same + function AFTER this check, the unrelated 180-day lead retention would also stop). """ return db.execute(_PREFLIGHT_PAID_CANDIDATES_SQL).scalar_one() @@ -207,11 +230,15 @@ def purge_expired_trade_in_data( Returns {"estimates_deleted": N, "leads_deleted": M}. - PR-D1 pre-flight (see `_preflight_paid_candidates`): if any purge-candidate - estimate has a `payments` row, the run aborts BEFORE the first DELETE batch -- - zero rows deleted, `mark_failed` records why. This is deliberately checked - outside the `try` below so it can never be caught and silently re-reported as a - generic mid-run failure -- it is a distinct, actionable pre-condition failure. + Payments retention pre-flight (see `_preflight_paid_candidates`): if any + purge-candidate estimate has `retain_until IS NULL` AND a `payments` row (the + ANOMALY -- fulfillment failed to set retain_until on a row money touched), the + run aborts BEFORE the first DELETE batch (estimates OR leads) -- zero rows + deleted, `mark_failed` records why. Healthy paid rows (retain_until set) do NOT + trip this -- they never matched the check to begin with. This is deliberately + checked outside the `try` below so it can never be caught and silently + re-reported as a generic mid-run failure -- it is a distinct, actionable + pre-condition failure. """ batch_size = batch_size or settings.trade_in_purge_batch_size max_batches = max_batches or _DEFAULT_MAX_BATCHES @@ -221,7 +248,7 @@ def purge_expired_trade_in_data( if paid_candidates: error = ( f"pre-flight abort: {paid_candidates} purge-candidate trade_in_estimates " - "row(s) have a matching payments row (retain_until may be unset) -- " + "row(s) have retain_until IS NULL but a matching payments row -- " "refusing to run, zero rows deleted" ) logger.error("purge_expired_trade_in_data run_id=%d %s", run_id, error) diff --git a/tradein-mvp/backend/data/sql/234_trade_in_estimates_retain_until.sql b/tradein-mvp/backend/data/sql/234_trade_in_estimates_retain_until.sql index 2d2e3d4f..912eb915 100644 --- a/tradein-mvp/backend/data/sql/234_trade_in_estimates_retain_until.sql +++ b/tradein-mvp/backend/data/sql/234_trade_in_estimates_retain_until.sql @@ -1,6 +1,7 @@ -- 234_trade_in_estimates_retain_until.sql --- PR-D1 «Ретеншен: оплаченное живёт год, purge его не трогает» — см. --- `mera-pr-d-spec.md` §1 в корне репо (обоснования там, здесь только SQL). +-- Платёжный контур МЕРЫ, ретеншен (PR #2754): «оплаченное живёт год, purge +-- его не трогает». Владелец продаёт отчёт физлицу за 150 ₽ — отчёт должен +-- жить год на нашей стороне, а не 24ч (см. WHY ниже). -- Номер сверен и по `forgejo/main`, и по всем открытым PR-веткам на момент -- написания (последняя занятая — 233_payments.sql) — см. урок в шапке того -- же файла про то, как коллизия 228/229/231/232 обнаруживается поздно. diff --git a/tradein-mvp/backend/tests/test_estimate_idor.py b/tradein-mvp/backend/tests/test_estimate_idor.py index 1dc6aaf9..7cb0e735 100644 --- a/tradein-mvp/backend/tests/test_estimate_idor.py +++ b/tradein-mvp/backend/tests/test_estimate_idor.py @@ -605,7 +605,7 @@ def test_get_estimate_imv_benchmark_other_pilot_gets_404(trade_in_app: FastAPI) assert resp.status_code == 404 -# ── PR-D1: retention gate unification (retain_until, mera-pr-d-spec.md §1.3) ── +# ── Payments retention: retention gate unification (retain_until, PR #2754) ── def test_estimate_readable_sql_uses_disjunction() -> None: diff --git a/tradein-mvp/backend/tests/test_paid_retention_text_consistency.py b/tradein-mvp/backend/tests/test_paid_retention_text_consistency.py new file mode 100644 index 00000000..434d419f --- /dev/null +++ b/tradein-mvp/backend/tests/test_paid_retention_text_consistency.py @@ -0,0 +1,103 @@ +"""Payments retention (PR #2754) — "12 месяцев" text sync guard. + +WHY: + mera-public/content.ts declares itself as the ONE place product promises + live (docstring at the top of that file: "ни одного утверждения, которого + не делает код"). The public retention promise ("оплаченный отчёт хранится + N месяцев") has THREE places it could quietly drift: the backend setting + (`settings.trade_in_paid_retention_days`), the frontend constant + (`PAID_REPORT_RETENTION_MONTHS` in content.ts), and any page that renders + it (today: privacy/page.tsx). Deep-review finding 2026-08-06 MEDIUM on + PR #2754 caught exactly this: a comment claimed the number "reads from the + same setting" while the page actually hardcoded a `12 месяцев` literal -- + a comment cannot fail CI, same lesson as + test_consent_text_frontend_sync.py's _CONSENT_TEXT_SNAPSHOT guard (which + this file mirrors). + +WHAT: + 1. privacy/page.tsx imports PAID_REPORT_RETENTION_MONTHS from content.ts + and does NOT hardcode a "N месяцев" literal of its own. + 2. The frontend months constant and the backend days setting stay within + a sane calendar tolerance of each other (28-31 days per month) -- this + does NOT enforce byte-identity (days and months are different units by + design, see content.ts docstring), only that nobody silently changes + one without the other drifting out of "still honestly ~12 months". +""" + +from __future__ import annotations + +import os +import re +from pathlib import Path + +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") + +_FRONTEND_CONTENT = ( + Path(__file__).resolve().parents[2] / "frontend" / "src" / "app" / "mera-public" / "content.ts" +) +_FRONTEND_PRIVACY_PAGE = ( + Path(__file__).resolve().parents[2] + / "frontend" + / "src" + / "app" + / "mera-public" + / "privacy" + / "page.tsx" +) + +_MONTHS_CONST_RE = re.compile(r"PAID_REPORT_RETENTION_MONTHS\s*=\s*(\d+)\s*;") +_LITERAL_MONTHS_RE = re.compile(r'"?\d+ месяцев"?') + + +def _extract_months_constant(content_ts_source: str) -> int: + match = _MONTHS_CONST_RE.search(content_ts_source) + assert match is not None, ( + "PAID_REPORT_RETENTION_MONTHS not found in content.ts -- " + "constant renamed/removed without updating this test" + ) + return int(match.group(1)) + + +def test_frontend_files_exist() -> None: + assert _FRONTEND_CONTENT.is_file(), f"missing frontend file: {_FRONTEND_CONTENT}" + assert _FRONTEND_PRIVACY_PAGE.is_file(), f"missing frontend file: {_FRONTEND_PRIVACY_PAGE}" + + +def test_privacy_page_imports_retention_constant_not_hardcoded() -> None: + """The whole point: FAILS if privacy/page.tsx stops importing the shared + constant and goes back to a hardcoded '12 месяцев' literal (exactly the + drift the deep-review finding caught -- comment said 'reads from content.ts', + code said otherwise).""" + src = _FRONTEND_PRIVACY_PAGE.read_text(encoding="utf-8") + assert "PAID_REPORT_RETENTION_MONTHS" in src, ( + "privacy/page.tsx no longer references PAID_REPORT_RETENTION_MONTHS -- " + "the retention promise must be rendered from the shared content.ts " + "constant, not written out again by hand" + ) + assert re.search(r'from\s+"\.\./content"', src), ( + "privacy/page.tsx must import from '../content' (content.ts), where " + "PAID_REPORT_RETENTION_MONTHS is declared" + ) + literal_hits = _LITERAL_MONTHS_RE.findall(src) + assert not literal_hits, ( + "privacy/page.tsx contains a hardcoded 'N месяцев' literal -- render the " + "PAID_REPORT_RETENTION_MONTHS constant instead: " + f"{literal_hits!r}" + ) + + +def test_backend_days_setting_matches_frontend_months_within_calendar_tolerance() -> None: + """Not byte-identity (days vs months are different units, deliberately -- + see content.ts docstring on PAID_REPORT_RETENTION_MONTHS): just a sanity + bound that `trade_in_paid_retention_days` still honestly rounds to the + number of months the public page promises (28-31 days/month, generous).""" + from app.core.config import settings + + days = settings.trade_in_paid_retention_days + months = _extract_months_constant(_FRONTEND_CONTENT.read_text(encoding="utf-8")) + + assert 28 * months <= days <= 31 * months, ( + f"settings.trade_in_paid_retention_days={days} no longer honestly rounds to " + f"content.ts PAID_REPORT_RETENTION_MONTHS={months} -- update both together " + "(and the offer text, when it exists) so the public promise stays true" + ) diff --git a/tradein-mvp/backend/tests/test_purge_expired_trade_in_data.py b/tradein-mvp/backend/tests/test_purge_expired_trade_in_data.py index caa1b207..a49f1a96 100644 --- a/tradein-mvp/backend/tests/test_purge_expired_trade_in_data.py +++ b/tradein-mvp/backend/tests/test_purge_expired_trade_in_data.py @@ -7,20 +7,22 @@ Covers app/tasks/purge_expired_trade_in_data.py: - both tables (trade_in_estimates, trade_in_leads) get drained - failure path: rollback + mark_failed with partial counters, exception re-raised - SQL shape: DELETE (not UPDATE/deactivate), no psycopg `::` cast trap - - PR-D1 (payments retention, mera-pr-d-spec.md §1.4): retain_until IS NULL + - NOT EXISTS(payments) safeguards on the estimates DELETE, plus a pre-flight - that refuses to run at all if it finds a paid purge-candidate. + - Payments retention (PR #2754): retain_until IS NULL + NOT EXISTS(payments) + safeguards on the estimates DELETE, plus a pre-flight that refuses to run + at all if it finds an ANOMALOUS paid purge-candidate (retain_until unset + despite a payments row) -- a healthy paid row (retain_until set) must NOT + trip it, see test_preflight_ignores_healthy_paid_row below. Style mirrors tests/test_deactivate_stale_listings.py (_FakeDB, monkeypatched runs_mod.mark_done/mark_failed). -PR-D1 note on _FakeDB: purge_expired_trade_in_data now issues ONE extra -db.execute() call BEFORE any DELETE batch — the pre-flight paid-candidates -count (_PREFLIGHT_PAID_CANDIDATES_SQL). _FakeDB special-cases that statement -by identity and answers it from `preflight_count` (default 0 == "no paid -candidates, proceed exactly as before this PR"). Every pre-existing test's -`db.executed` index shifted by +1 to account for this; `db.commits` is -unaffected (the pre-flight is a read, never committed). +Payments retention note on _FakeDB: purge_expired_trade_in_data now issues ONE +extra db.execute() call BEFORE any DELETE batch — the pre-flight paid- +candidates count (_PREFLIGHT_PAID_CANDIDATES_SQL). _FakeDB special-cases that +statement by identity and answers it from `preflight_count` (default 0 == +"no anomalous candidates, proceed exactly as before this PR"). Every +pre-existing test's `db.executed` index shifted by +1 to account for this; +`db.commits` is unaffected (the pre-flight is a read, never committed). """ from __future__ import annotations @@ -169,7 +171,7 @@ def test_estimates_sql_is_delete_not_update() -> None: assert not re.search(r":\w+::", sql) -# ── PR-D1 (mera-pr-d-spec.md §1.4): two independent purge safeguards ──────── +# ── Payments retention (PR #2754): two independent purge safeguards ───────── def test_estimates_sql_excludes_retain_until_not_null() -> None: @@ -190,15 +192,20 @@ def test_estimates_sql_has_not_exists_payments_safeguard() -> None: assert "p.estimate_id = trade_in_estimates.id" in sql -def test_preflight_sql_is_wider_than_delete_predicate() -> None: - """Pre-flight counts by the BASE (pre-PR-D1) predicate -- expires_at < NOW() - AND created_by IS NULL, WITHOUT retain_until/NOT EXISTS -- so it also catches - the case those two terms exist specifically to guard against (retain_until - unset despite a payments row existing).""" +def test_preflight_sql_requires_retain_until_is_null() -> None: + """Deep-review finding 2026-08-06 MEDIUM (PR #2754): the pre-flight predicate + MUST carry `retain_until IS NULL` -- without it, a perfectly healthy paid row + (retain_until set, has a payments row -- the ORDINARY steady state one day + after every sale) trips the alarm exactly as hard as the real anomaly + (retain_until unset despite a payments row existing), permanently wedging + the job on the very first successful sale (and, since leads purge runs from + the same function AFTER this check, silently stopping 180-day 152-ФЗ lead + retention too). See test_real_preflight_ignores_healthy_paid_row below for + the behavioural proof against a real DB.""" sql = task_mod._PREFLIGHT_PAID_CANDIDATES_SQL.text assert "expires_at < NOW()" in sql assert "created_by IS NULL" in sql - assert "retain_until" not in sql + assert "retain_until IS NULL" in sql assert "EXISTS (SELECT 1 FROM payments p WHERE p.estimate_id = e.id)" in sql assert not re.search(r":\w+::", sql) @@ -230,10 +237,10 @@ def test_preflight_zero_candidates_proceeds_as_before(monkeypatch: pytest.Monkey def test_leads_sql_unchanged_by_pr_d1() -> None: - """Snapshot: _DELETE_EXPIRED_LEADS_SQL byte-for-byte unchanged by PR-D1 — - leads have their own retention deadline (migration 231) and are explicitly - out of scope (mera-pr-d-spec.md §1.4: '_DELETE_EXPIRED_LEADS_SQL — оставить - дословно').""" + """Snapshot: _DELETE_EXPIRED_LEADS_SQL byte-for-byte unchanged by payments + retention (PR #2754) — leads have their own retention deadline (migration + 231, no created_by/B2B split, no payments concept) and are explicitly out + of scope for the payments-retention safeguards.""" expected = ( "\n DELETE FROM trade_in_leads\n WHERE id IN (\n" " SELECT id FROM trade_in_leads\n" @@ -443,3 +450,160 @@ def test_real_purge_deletes_only_anonymous_expired_estimates() -> None: ) db.commit() db.close() + + +@pytest.mark.skipif(_live_session() is None, reason="no reachable Postgres test DB") +def test_real_preflight_ignores_healthy_paid_row_flags_only_anomaly() -> None: + """Deep-review finding 2026-08-06 MEDIUM on PR #2754, reproduced exactly + against a real DB: a HEALTHY paid row (retain_until set, a payments row + exists) is the ordinary steady state one day after every sale and must NOT + raise the pre-flight count; a row where fulfillment failed to set + retain_until despite a payments row existing is the real ANOMALY and must. + Baseline-delta assertions (not absolute counts) so this is safe to run + against a dev DB that may already contain unrelated rows.""" + from sqlalchemy import text as _t + + db = _live_session() + assert db is not None + healthy_id = uuid4() + anomaly_id = uuid4() + healthy_order = f"pytest-healthy-{uuid4().hex[:12]}" + anomaly_order = f"pytest-anomaly-{uuid4().hex[:12]}" + try: + baseline = task_mod._preflight_paid_candidates(db) + + # Healthy: retain_until set (paid, safeguard 1 already protects it) + + # a payments row -- exactly what every successful sale looks like a day + # later. Must NOT move the pre-flight count. + db.execute( + _t( + "INSERT INTO trade_in_estimates " + "(id, address, area_m2, rooms, floor, total_floors, " + " median_price, range_low, range_high, median_price_per_m2, confidence, " + " expires_at, created_by, retain_until) VALUES " + "(CAST(:id AS uuid), 'purge-test здоровая оплаченная', 40, 1, 2, 5, " + " 5000000, 4500000, 5500000, 125000, 'low', " + " NOW() - interval '1 hour', NULL, NOW() + interval '363 days')" + ), + {"id": str(healthy_id)}, + ) + db.execute( + _t( + "INSERT INTO payments " + "(order_id, terminal_key, product_code, amount_kopecks, status, estimate_id) " + "VALUES (:order_id, 'pytest_terminal', 'trade_in_report', 15000, 'CONFIRMED', " + " CAST(:id AS uuid))" + ), + {"order_id": healthy_order, "id": str(healthy_id)}, + ) + db.commit() + assert ( + task_mod._preflight_paid_candidates(db) == baseline + ), "healthy paid row (retain_until set) must NOT raise the pre-flight count" + + # Anomaly: retain_until NULL despite a payments row existing -- exactly + # the case the two DELETE safeguards exist for. Must raise by exactly one. + db.execute( + _t( + "INSERT INTO trade_in_estimates " + "(id, address, area_m2, rooms, floor, total_floors, " + " median_price, range_low, range_high, median_price_per_m2, confidence, " + " expires_at, created_by, retain_until) VALUES " + "(CAST(:id AS uuid), 'purge-test настоящая аномалия', 40, 1, 2, 5, " + " 5000000, 4500000, 5500000, 125000, 'low', " + " NOW() - interval '1 hour', NULL, NULL)" + ), + {"id": str(anomaly_id)}, + ) + db.execute( + _t( + "INSERT INTO payments " + "(order_id, terminal_key, product_code, amount_kopecks, status, estimate_id) " + "VALUES (:order_id, 'pytest_terminal', 'trade_in_report', 15000, 'CONFIRMED', " + " CAST(:id AS uuid))" + ), + {"order_id": anomaly_order, "id": str(anomaly_id)}, + ) + db.commit() + assert task_mod._preflight_paid_candidates(db) == baseline + 1, ( + "anomaly row (retain_until unset, payments row exists) must raise " + "the pre-flight count by exactly one" + ) + finally: + db.execute( + _t("DELETE FROM payments WHERE order_id = ANY(CAST(:orders AS text[]))"), + {"orders": [healthy_order, anomaly_order]}, + ) + db.execute( + _t("DELETE FROM trade_in_estimates WHERE id = ANY(CAST(:ids AS uuid[]))"), + {"ids": [str(healthy_id), str(anomaly_id)]}, + ) + db.commit() + db.close() + + +@pytest.mark.skipif(_live_session() is None, reason="no reachable Postgres test DB") +def test_real_purge_not_wedged_by_healthy_paid_row() -> None: + """Deep-review finding 2026-08-06 MEDIUM on PR #2754: before the fix, a + healthy paid row anywhere in the table (retain_until set, has a payments + row) permanently wedged the job -- the very first successful sale would + have made every subsequent scheduled run abort in mark_failed with zero + deletions FOREVER, silently taking 180-day leads purge (152-ФЗ) down with + it (leads purge runs from the same function AFTER the pre-flight check). + This proves a real end-to-end run completes normally (mark_done) in the + presence of such a row.""" + from sqlalchemy import text as _t + + db = _live_session() + assert db is not None + healthy_id = uuid4() + healthy_order = f"pytest-wedge-{uuid4().hex[:12]}" + try: + db.execute( + _t( + "INSERT INTO trade_in_estimates " + "(id, address, area_m2, rooms, floor, total_floors, " + " median_price, range_low, range_high, median_price_per_m2, confidence, " + " expires_at, created_by, retain_until) VALUES " + "(CAST(:id AS uuid), 'purge-test не блокирует джобу', 40, 1, 2, 5, " + " 5000000, 4500000, 5500000, 125000, 'low', " + " NOW() - interval '1 hour', NULL, NOW() + interval '363 days')" + ), + {"id": str(healthy_id)}, + ) + db.execute( + _t( + "INSERT INTO payments " + "(order_id, terminal_key, product_code, amount_kopecks, status, estimate_id) " + "VALUES (:order_id, 'pytest_terminal', 'trade_in_report', 15000, 'CONFIRMED', " + " CAST(:id AS uuid))" + ), + {"order_id": healthy_order, "id": str(healthy_id)}, + ) + db.commit() + + # Must complete normally -- no RuntimeError, no mark_failed short-circuit + # (would raise before reaching this line if the bug were still present). + result = task_mod.purge_expired_trade_in_data( + db, run_id=999999998, batch_size=100, max_batches=1 + ) + assert set(result) == {"estimates_deleted", "leads_deleted"}, ( + "leads purge must also have run -- it is NOT reachable when the " + "pre-flight wrongly aborts first" + ) + + still_there = db.execute( + _t("SELECT id FROM trade_in_estimates WHERE id = CAST(:id AS uuid)"), + {"id": str(healthy_id)}, + ).fetchone() + assert still_there is not None, "healthy paid row must survive the run untouched" + finally: + db.execute( + _t("DELETE FROM payments WHERE order_id = :order_id"), {"order_id": healthy_order} + ) + db.execute( + _t("DELETE FROM trade_in_estimates WHERE id = CAST(:id AS uuid)"), + {"id": str(healthy_id)}, + ) + db.commit() + db.close() diff --git a/tradein-mvp/frontend/src/app/mera-public/content.ts b/tradein-mvp/frontend/src/app/mera-public/content.ts index 3e521b68..66772877 100644 --- a/tradein-mvp/frontend/src/app/mera-public/content.ts +++ b/tradein-mvp/frontend/src/app/mera-public/content.ts @@ -92,6 +92,25 @@ export const LEGAL_ENTITY: { /** Внутренний маршрут страницы про обработку персональных данных. */ export const PRIVACY_PATH = "/mera-public/privacy"; +/** + * Сколько месяцев на нашей стороне хранится ссылка/строка оплаченного отчёта + * после оплаты (`trade_in_estimates.retain_until`, migration 234) — НЕ срок + * действия самого расчёта (тот отдельный, `expires_at`, часы). + * + * ЕДИНСТВЕННОЕ место, где это число хардкодится на фронте — любой другой + * текст (оферта, экран после оплаты) обязан импортировать эту константу, а + * не писать «12 месяцев» заново (см. `test_paid_retention_text_consistency.py` + * — проверяет, что privacy-страница действительно использует эту константу, + * а не литерал). + * + * Источник истины на бэкенде — `settings.trade_in_paid_retention_days = 365` + * (`app/core/config.py`). 365 дней округляется до «12 месяцев» для + * человекочитаемого текста (12×30=360..12×31=372 — 365 попадает в диапазон); + * если бэкендовое число когда-нибудь изменится так, что «12» перестанет быть + * честным округлением — обновить оба места руками, тест это тоже проверяет. + */ +export const PAID_REPORT_RETENTION_MONTHS = 12; + // --------------------------------------------------------------------------- // География // --------------------------------------------------------------------------- diff --git a/tradein-mvp/frontend/src/app/mera-public/privacy/page.tsx b/tradein-mvp/frontend/src/app/mera-public/privacy/page.tsx index 86fd3cec..0cdd50cf 100644 --- a/tradein-mvp/frontend/src/app/mera-public/privacy/page.tsx +++ b/tradein-mvp/frontend/src/app/mera-public/privacy/page.tsx @@ -3,6 +3,7 @@ import Link from "next/link"; import { LEGAL_ENTITY, + PAID_REPORT_RETENTION_MONTHS, PUBLIC_ESTIMATE_ENABLED, SUPPORT_TELEGRAM_LABEL, SUPPORT_TELEGRAM_URL, @@ -42,11 +43,16 @@ import { safeUrl } from "@/lib/safeUrl"; * стирается (см. докстринг `data_erasure.py`) — сюда её не выносим * (излишняя техническая деталь для публичной страницы), но это ограничение * реальное и известное. - * - PR-D1 (`retain_until`, `trade_in_paid_retention_days`): срок «12 месяцев» - * ниже читается из той же настройки, что и оферта/SQL продления — - * см. `mera-pr-d-spec.md` §1.2 в корне репо. Платёжного кода в этом PR - * нет — срок описан на будущее, синхронно с privacy-обязательством #1.7 - * того же дока, а не «потом». + * - Срок хранения оплаченного отчёта (PR #2754, `retain_until` / + * `trade_in_paid_retention_days` в backend `config.py`): число месяцев + * ниже — константа `PAID_REPORT_RETENTION_MONTHS` из `../content.ts`, + * НЕ литерал здесь (см. докстринг константы — она единственный источник + * этого числа на фронте; `test_paid_retention_text_consistency.py` + * проверяет, что эта страница действительно её импортирует). Платёжного + * кода в этом PR нет — срок описан на будущее, вместе с честной правкой + * ниже про то, что удаление сегодня описывает установленный ПОРЯДОК, а + * не наблюдаемый на проде автоматический прогон (задача засеяна + * выключенной). * * Раздел «Что делает эта страница» УСЛОВЕН по `PUBLIC_ESTIMATE_ENABLED`: пока * расчёт выключен, адрес действительно не покидает браузер; после включения это @@ -149,12 +155,13 @@ export default function MeraPublicPrivacyPage() { Самостоятельной кнопки «удалить мои данные» в интерфейсе пока нет, но механизм удаления в сервисе есть: обращение в поддержку об удалении мы разбираем вручную и физически стираем телефон, адрес и расчёт из базы, - а не просто помечаем запись. Помимо запроса, у данных есть собственный - срок хранения, по истечении которого они удаляются без обращения с - вашей стороны. Если результат расчёта оплачен, ссылка на отчёт и - связанные с ним данные хранятся на нашей стороне 12 месяцев с даты - оплаты, после чего удаляются точно так же — на файл, который вы - скачали себе, это не влияет: мы его не отзываем, не изменяем и не + а не просто помечаем запись. Помимо запроса, для каждого типа данных + установлен срок хранения, по истечении которого они подлежат + удалению. Если результат расчёта оплачен, ссылка на отчёт и + связанные с ним данные хранятся на нашей стороне{" "} + {PAID_REPORT_RETENTION_MONTHS} месяцев с даты оплаты, а затем + подлежат удалению так же, как и остальные данные — на файл, который + вы скачали себе, это не влияет: мы его не отзываем, не изменяем и не имеем к нему доступа.