fix(tradein/estimate): не перетирать дату обращения при оживлении оценки
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
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 Trade-In / backend-tests (pull_request) Successful in 4m10s
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
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 Trade-In / backend-tests (pull_request) Successful in 4m10s
Оживление мёртвых оценок (#2826) копировало в оригинальную строку поле created_at временной строки, которую создаёт estimate_quality. На проде оценка ff421062, созданная 2026-08-10 12:54, после оживления получила created_at = 2026-08-11 04:30. created_at — дата обращения клиента, а не дата нашего пересчёта. Второй симптом того же бага: /history сортирует по created_at, и оживлённая старая запись прыгала в начало списка. - created_at убран из SET в persist-UPDATE, ответ отдаёт дату исходной строки, а не временной - revival_completed_at (миграция 256_*, идемпотентная) — отдельный аудит-след «когда успешно пересчитали»; revival_attempted_at из 255_* ставится на захвате и включает неудачи с троттлингом - проверены остальные поля исходного обращения (TTL, снимок входных данных, согласие 152-ФЗ) — они и раньше не входили в SET PDF не затронут: там печатается дата формирования отчёта, не created_at.
This commit is contained in:
parent
82d8db9f42
commit
f5a4b8ce01
3 changed files with 118 additions and 3 deletions
|
|
@ -299,6 +299,16 @@ async def _try_revive_dead_estimate(
|
||||||
# into the ORIGINAL row (id/link contract), then drop the throwaway one.
|
# into the ORIGINAL row (id/link contract), then drop the throwaway one.
|
||||||
# INPUT snapshot (address/area/rooms/...) is untouched — it did not change,
|
# INPUT snapshot (address/area/rooms/...) is untouched — it did not change,
|
||||||
# only the outputs were recomputed.
|
# only the outputs were recomputed.
|
||||||
|
# #incident-2026-08-11: created_at is DELIBERATELY excluded from this SET —
|
||||||
|
# it is the client's original request date (printed in /history and in
|
||||||
|
# AggregatedEstimate.created_at, see app/schemas/trade_in.py:317-318), NOT
|
||||||
|
# a recompute output. It previously got clobbered with the throwaway temp
|
||||||
|
# row's created_at (=NOW() at recompute time), which also silently
|
||||||
|
# re-sorted the row to the top of `GET /history ORDER BY created_at DESC`.
|
||||||
|
# revival_completed_at (migration 256) is the audit trail for "when did a
|
||||||
|
# revival LAST successfully rewrite this row" — distinct from
|
||||||
|
# revival_attempted_at (255), which is stamped on every claim regardless
|
||||||
|
# of outcome (throttle loss / recompute failure included).
|
||||||
db.execute(
|
db.execute(
|
||||||
text(
|
text(
|
||||||
"""
|
"""
|
||||||
|
|
@ -337,7 +347,7 @@ async def _try_revive_dead_estimate(
|
||||||
ratio_basis = :ratio_basis,
|
ratio_basis = :ratio_basis,
|
||||||
relaxations = CAST(:relaxations_json AS jsonb),
|
relaxations = CAST(:relaxations_json AS jsonb),
|
||||||
reliability = :reliability,
|
reliability = :reliability,
|
||||||
created_at = :created_at
|
revival_completed_at = NOW()
|
||||||
WHERE id = CAST(:id AS uuid)
|
WHERE id = CAST(:id AS uuid)
|
||||||
"""
|
"""
|
||||||
),
|
),
|
||||||
|
|
@ -371,7 +381,6 @@ async def _try_revive_dead_estimate(
|
||||||
"ratio_basis": result.ratio_basis,
|
"ratio_basis": result.ratio_basis,
|
||||||
"relaxations_json": json.dumps(result.relaxations, ensure_ascii=False),
|
"relaxations_json": json.dumps(result.relaxations, ensure_ascii=False),
|
||||||
"reliability": result.reliability,
|
"reliability": result.reliability,
|
||||||
"created_at": result.created_at,
|
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
db.execute(
|
db.execute(
|
||||||
|
|
@ -388,7 +397,9 @@ async def _try_revive_dead_estimate(
|
||||||
result.confidence,
|
result.confidence,
|
||||||
result.reliability,
|
result.reliability,
|
||||||
)
|
)
|
||||||
return result.model_copy(update={"estimate_id": estimate_id})
|
# created_at on the returned object must mirror the DB row (untouched
|
||||||
|
# original request date, NOT the temp row's NOW()) — see UPDATE above.
|
||||||
|
return result.model_copy(update={"estimate_id": estimate_id, "created_at": row.created_at})
|
||||||
|
|
||||||
|
|
||||||
@router.post("/estimate", response_model=AggregatedEstimate)
|
@router.post("/estimate", response_model=AggregatedEstimate)
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,48 @@
|
||||||
|
-- 256_trade_in_estimates_revival_completed_at.sql
|
||||||
|
-- Номер сверен по `ls data/sql | sort` (max applied = 255) непосредственно
|
||||||
|
-- перед коммитом — см. sql.md § file naming + tradein.md § collision trap
|
||||||
|
-- (108_*/084_* уже дублировались в прошлом).
|
||||||
|
--
|
||||||
|
-- ── fix/tradein-created (2026-08-11): created_at перезаписывался revival'ом ──
|
||||||
|
-- `_try_revive_dead_estimate` (app/api/v1/trade_in.py, migration 255) писал
|
||||||
|
-- пересчитанные поля в ИСХОДНУЮ строку trade_in_estimates, и вместе с ними —
|
||||||
|
-- `created_at`, скопированный из временной строки (estimate_quality() ставит
|
||||||
|
-- туда NOW() на момент пересчёта). Факт с прода: оценка
|
||||||
|
-- ff421062-cc38-4c4c-ad2e-0cfac52d14ff создана 2026-08-10 12:54:47, после
|
||||||
|
-- revival'а на GET её created_at стал 2026-08-11 04:30:03 — «дата обращения»
|
||||||
|
-- клиента (печатается в /history и в схеме AggregatedEstimate.created_at,
|
||||||
|
-- см. app/schemas/trade_in.py:317-318 «для метки «отчёт от DD.MM» в UI»)
|
||||||
|
-- подменилась датой служебного пересчёта. Заодно ломался ORDER BY created_at
|
||||||
|
-- DESC в GET /history — оживлённая старая заявка выпрыгивала в начало списка.
|
||||||
|
--
|
||||||
|
-- Фикс (app/api/v1/trade_in.py): created_at исключён из UPDATE SET revival'а,
|
||||||
|
-- исходная дата больше не трогается. Момент, когда revival РЕАЛЬНО пересчитал
|
||||||
|
-- строку (не просто "попытался" — revival_attempted_at из 255 ставится на
|
||||||
|
-- claim'е ДО вызова estimate_quality(), в том числе при throttle-проигрыше и
|
||||||
|
-- при неудачном пересчёте), нужен для аудита отдельно — новая колонка.
|
||||||
|
--
|
||||||
|
-- ── IDEMPOTENCY ───────────────────────────────────────────────────────────
|
||||||
|
-- ADD COLUMN IF NOT EXISTS — безопасный re-run. Бэкфилла нет: NULL = либо
|
||||||
|
-- строка живая и revival никогда успешно не пересчитывал, либо запись
|
||||||
|
-- создана до этой колонки.
|
||||||
|
--
|
||||||
|
-- Dependencies: 255_trade_in_estimates_revival_relaxations.sql. Apply after: 255_*.
|
||||||
|
|
||||||
|
BEGIN;
|
||||||
|
|
||||||
|
SET LOCAL lock_timeout = '5s';
|
||||||
|
|
||||||
|
ALTER TABLE trade_in_estimates
|
||||||
|
ADD COLUMN IF NOT EXISTS revival_completed_at timestamptz;
|
||||||
|
|
||||||
|
COMMENT ON COLUMN trade_in_estimates.revival_completed_at IS
|
||||||
|
'Момент УСПЕШНОГО пересчёта «мёртвой» (median_price<=0/NULL) строки '
|
||||||
|
'revival''ом (app/api/v1/trade_in.py::_try_revive_dead_estimate) — '
|
||||||
|
'выставляется, когда пересчёт реально записал новые значения в строку. '
|
||||||
|
'Отличается от revival_attempted_at (255): тот ставится на claim''е ДО '
|
||||||
|
'вызова estimate_quality() и фиксирует ЛЮБУЮ попытку (включая throttled-'
|
||||||
|
'проигрыш гонки и неудачный пересчёт), этот — только успех. created_at '
|
||||||
|
'строки при этом НЕ меняется (исходная дата обращения клиента, печатается '
|
||||||
|
'в /history) — см. fix/tradein-created 2026-08-11.';
|
||||||
|
|
||||||
|
COMMIT;
|
||||||
|
|
@ -335,6 +335,62 @@ def test_dead_row_revives_and_updates_db(
|
||||||
assert delete_calls[0].args[1]["id"] == _TEMP_ID
|
assert delete_calls[0].args[1]["id"] == _TEMP_ID
|
||||||
|
|
||||||
|
|
||||||
|
def test_dead_row_revival_preserves_created_at(
|
||||||
|
trade_in_app: FastAPI, _estimator_stub: SimpleNamespace
|
||||||
|
) -> None:
|
||||||
|
"""#incident-2026-08-11: created_at is the client's ORIGINAL request date
|
||||||
|
(printed on /history, see app/schemas/trade_in.py:317-318) — revival must
|
||||||
|
not clobber it with the throwaway temp row's NOW(). Regression for the
|
||||||
|
prod incident (estimate ff421062-...: created_at jumped from the original
|
||||||
|
2026-08-10 12:54:47 to the revival moment 2026-08-11 04:30:03).
|
||||||
|
|
||||||
|
Also asserts: (a) the persist UPDATE never sets created_at at all — the
|
||||||
|
fix removes the column from SET, it doesn't just overwrite it with the
|
||||||
|
right value; (b) migration 256's revival_completed_at IS stamped, as the
|
||||||
|
separate "when did revival last succeed" audit trail; (c) the JSON
|
||||||
|
response mirrors the original created_at, not the temp result's.
|
||||||
|
"""
|
||||||
|
_original_created_at = datetime(2026, 5, 29, tzinfo=UTC)
|
||||||
|
|
||||||
|
async def _fake_estimate_quality(payload, db, **kwargs):
|
||||||
|
# The temp row estimate_quality() mints internally always carries
|
||||||
|
# NOW() as its created_at — deliberately far from the original, so a
|
||||||
|
# regression (copying result.created_at through) is unmissable.
|
||||||
|
return _fake_revived_result(created_at=datetime(2026, 8, 11, 4, 30, 3, tzinfo=UTC))
|
||||||
|
|
||||||
|
_estimator_stub.estimate_quality = _fake_estimate_quality
|
||||||
|
|
||||||
|
row = _make_dead_row()
|
||||||
|
row.created_at = _original_created_at
|
||||||
|
db = _dispatch_db(row, claim_result=SimpleNamespace(id=_ESTIMATE_ID))
|
||||||
|
client = _client_with(trade_in_app, db)
|
||||||
|
resp = client.get(
|
||||||
|
f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}",
|
||||||
|
headers={"X-Authenticated-User": "kopylov"},
|
||||||
|
)
|
||||||
|
assert resp.status_code == 200
|
||||||
|
body = resp.json()
|
||||||
|
assert body["created_at"] == "2026-05-29T00:00:00Z"
|
||||||
|
|
||||||
|
persist_calls = _calls_containing(db, "UPDATE trade_in_estimates SET")
|
||||||
|
assert len(persist_calls) == 1
|
||||||
|
persist_sql = persist_calls[0].args[0].text
|
||||||
|
persist_params = persist_calls[0].args[1]
|
||||||
|
assert "created_at" not in persist_sql
|
||||||
|
assert "created_at" not in persist_params
|
||||||
|
assert "revival_completed_at = NOW()" in persist_sql
|
||||||
|
# Input-snapshot / TTL columns (what the client originally asked for and
|
||||||
|
# for how long the row is retained) are likewise not recompute outputs —
|
||||||
|
# untouched by the revival persist UPDATE.
|
||||||
|
# NB: "address" is checked via persist_params only (not persist_sql) —
|
||||||
|
# canonical_address (a legitimate recompute output) ends in "address =",
|
||||||
|
# which would false-positive a substring check against the raw SQL text.
|
||||||
|
for protected in ("expires_at", "retain_until", "created_by"):
|
||||||
|
assert f"{protected} =" not in persist_sql
|
||||||
|
assert protected not in persist_params
|
||||||
|
assert "address" not in persist_params
|
||||||
|
|
||||||
|
|
||||||
# ── Live row is never touched ────────────────────────────────────────────
|
# ── Live row is never touched ────────────────────────────────────────────
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue