fix(tradein/payments): pre-flight должен ловить аномалию, не штатное состояние (review PR #2754)
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI / changes (pull_request) Successful in 8s
CI Trade-In / browser-tests (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Successful in 1m3s
CI Trade-In / backend-tests (pull_request) Successful in 3m49s
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI / changes (pull_request) Successful in 8s
CI Trade-In / browser-tests (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Successful in 1m3s
CI Trade-In / backend-tests (pull_request) Successful in 3m49s
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.
This commit is contained in:
parent
5ff06d25b4
commit
48664dfe0e
9 changed files with 401 additions and 75 deletions
|
|
@ -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())"
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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 (нельзя одним
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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 обнаруживается поздно.
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
)
|
||||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// География
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -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} месяцев с даты оплаты, а затем
|
||||
подлежат удалению так же, как и остальные данные — на файл, который
|
||||
вы скачали себе, это не влияет: мы его не отзываем, не изменяем и не
|
||||
имеем к нему доступа.
|
||||
</p>
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue