feat(tradein/cian): admin endpoints + UI for detail+newbuilding history scrape #525

Merged

lekss361 merged 3 commits from feat/cian-detail-nb-endpoints into main

2026-05-24 14:05:56 +00:00

lekss361 commented

2026-05-24 13:57:40 +00:00

Owner

What

Activates Cian historical-data ingestion. Parsers and DB writers were already complete (cian_detail.fetch_detail/save_detail_enrichment, cian_newbuilding.fetch_newbuilding/save_newbuilding_enrichment) — only the HTTP trigger layer + UI form were missing. Two endpoints + two functional forms.

Backend

POST /api/v1/admin/scrape/cian-detail?offer_url=...&listing_id=... — fetch+parse Cian offer detail. If listing_id provided → writes to offer_price_history and updates listings. Without it → debug-only.
POST /api/v1/admin/scrape/cian-newbuilding?zhk_url=...&house_id=... — fetch+parse Cian ЖК (newbuilding) landing. If house_id provided → writes houses_price_dynamics + management_companies + updates houses. Without it → debug-only.

Both mirror Yandex equivalent pattern (admin.py:829–933). save_newbuilding_enrichment properly awaited (async).

Frontend

/scrapers/cian page sections 2 + 3 (previously red banner "404 — planned") replaced with working forms reusing existing apiFetch / useMutation / <ResultPanel> patterns. Section 4 (Valuation Calculator) keeps placeholder text but loses red banner (Calculator is called automatically from /estimate flow Stage 9, separate admin trigger TBD).

Why now

Predecessor: PR #523 (047 UNIQUE + 026/044 column fixes) — without it offer_price_history ON CONFLICT DO NOTHING was silently inserting duplicates. Now both writers are safe.

Follow-up PRs:

PR 3: surface chart_data + rent + change-pct on /trade-in page (UI for already-cached estimator data)
PR 4: Celery backfill task — iterate existing houses+listings, call these endpoints for historical fill

Files

tradein-mvp/backend/app/api/v1/admin.py (+89, mirroring Yandex endpoints)
tradein-mvp/frontend/src/app/scrapers/cian/page.tsx (+140/-31, sections 2+3+4 rewrite)

Test plan

ruff clean on admin.py (verified)
typecheck clean on frontend (verified)
Pre-push review ⚠️ MINOR → fixed double URLSearchParams encoding (commit 82a78c3); re-review ✅
Manual smoke after merge: open /scrapers/cian → section 2 form → paste a real Cian offer URL without listing_id → expect 200 with saved: false, price_changes_count > 0
Same with listing_id of an existing Cian listing → expect saved: true and rows in offer_price_history
Same for section 3 with a ЖК URL

## What Activates Cian historical-data ingestion. Parsers and DB writers were already complete (cian_detail.fetch_detail/save_detail_enrichment, cian_newbuilding.fetch_newbuilding/save_newbuilding_enrichment) — only the HTTP trigger layer + UI form were missing. Two endpoints + two functional forms. ## Backend - `POST /api/v1/admin/scrape/cian-detail?offer_url=...&listing_id=...` — fetch+parse Cian offer detail. If `listing_id` provided → writes to `offer_price_history` and updates `listings`. Without it → debug-only. - `POST /api/v1/admin/scrape/cian-newbuilding?zhk_url=...&house_id=...` — fetch+parse Cian ЖК (newbuilding) landing. If `house_id` provided → writes `houses_price_dynamics` + `management_companies` + updates `houses`. Without it → debug-only. Both mirror Yandex equivalent pattern (admin.py:829–933). `save_newbuilding_enrichment` properly awaited (async). ## Frontend `/scrapers/cian` page sections 2 + 3 (previously red banner "404 — planned") replaced with working forms reusing existing `apiFetch` / `useMutation` / `<ResultPanel>` patterns. Section 4 (Valuation Calculator) keeps placeholder text but loses red banner (Calculator is called automatically from /estimate flow Stage 9, separate admin trigger TBD). ## Why now Predecessor: PR #523 (047 UNIQUE + 026/044 column fixes) — without it offer_price_history `ON CONFLICT DO NOTHING` was silently inserting duplicates. Now both writers are safe. Follow-up PRs: - PR 3: surface chart_data + rent + change-pct on /trade-in page (UI for already-cached estimator data) - PR 4: Celery backfill task — iterate existing houses+listings, call these endpoints for historical fill ## Files - `tradein-mvp/backend/app/api/v1/admin.py` (+89, mirroring Yandex endpoints) - `tradein-mvp/frontend/src/app/scrapers/cian/page.tsx` (+140/-31, sections 2+3+4 rewrite) ## Test plan - [x] ruff clean on admin.py (verified) - [x] typecheck clean on frontend (verified) - [x] Pre-push review ⚠️ MINOR → fixed double URLSearchParams encoding (commit 82a78c3); re-review ✅ - [ ] Manual smoke after merge: open /scrapers/cian → section 2 form → paste a real Cian offer URL without listing_id → expect 200 with `saved: false`, `price_changes_count > 0` - [ ] Same with listing_id of an existing Cian listing → expect `saved: true` and rows in `offer_price_history` - [ ] Same for section 3 with a ЖК URL

lekss361 added 3 commits 2026-05-24 13:57:41 +00:00

feat(tradein/cian): admin endpoints /scrape/cian-detail + /scrape/cian-newbuilding 9234cae932

feat(tradein-ui): wire /scrapers/cian sections 2+3 to new backend endpoints 2d7446a4a9

fix(tradein-ui): drop double URLSearchParams encoding for cian-detail/nb forms 82a78c3ead

lekss361 commented

2026-05-24 14:05:46 +00:00

Author

Owner

Deep Code Review — PR #525

Summary

Status: APPROVE
Files: 2 (P1 backend +89 / P1 frontend +140/-31)
Risk: low (admin debug endpoints, parity with merged Yandex analog)

Findings

Security

SSRF surface: fetch_detail(offer_url)/fetch_newbuilding(zhk_url) pass user-supplied URL directly to AsyncSession.get(..., allow_redirects=True). No domain allow-list. This is pre-existing class — identical posture in scrape_yandex_detail/scrape_yandex_newbuilding/scrape_yandex_valuation already in main. Gated only by Caddy basic_auth on /trade-in/api/v1/admin/* per file docstring. Not a regression; flag for a follow-up hardening pass across all admin scrape triggers (allow-list cian.ru / yandex.ru hosts).
Frontend XSS: clean. ResultPanel renders via <pre>{JSON.stringify(mut.data, null, 2)}</pre> and mut.error.message as text — no dangerouslySetInnerHTML, no <a href={userInput}> (safeUrl helper from #512 not relevant here).
CSRF: not protected at app layer, consistent with rest of admin surface (Caddy basic_auth + cookie context). Same posture as Yandex endpoints.
Query-string encoding: correct — URLSearchParams single-encodes; no double-encoding regression (test plan confirms this was the pre-push fix in 82a78c3).

Correctness

fetch_detail is async, save_detail_enrichment is sync, fetch_newbuilding async, save_newbuilding_enrichment async → all await/non-await calls in admin.py match signatures (verified at cian_detail.py:59,216 and cian_newbuilding.py:78,391).
Both writers commit internally (db.commit() at cian_detail.py:318, cian_newbuilding.py:541). Depends(get_db) session is fine — no nested transaction risk.
Idempotency: offer_price_history UPSERT relies on UNIQUE constraint from PR #523 (047_cian_history_sanitize.sql) — merged 2026-05-24 13:44 UTC, before this PR. houses_price_dynamics uses ON CONFLICT ON CONSTRAINT houses_price_dynamics_dim_key DO UPDATE. Both clean.
FK safety: offer_price_history.listing_id REFERENCES listings(id) — bad listing_id → IntegrityError → FastAPI 500 (not silent corruption). UX could be improved with a pre-check returning 404, but acceptable for an admin debug endpoint. Nit, not blocking.
psycopg v3 CAST rules: all params use CAST(:x AS type) — compliant.
No regression in existing admin routes: router decorator count 19 → 21, only two new routes added.

Project conventions

Function-local imports for cian_detail/cian_newbuilding symbols (admin.py:956,999) deviate from the file's top-level-import norm. Justified: top-level fetch_detail/save_detail_enrichment are already bound to the Avito versions at line 27 — function-scope rebinding cleanly avoids the symbol collision and keeps the Avito-detail endpoint at line 471 unaffected. Acceptable trade-off; alternative would be aliased top-level imports.
Pattern parity with scrape_yandex_detail/scrape_yandex_newbuilding (admin.py:829–933) is exact: same response-model style, same query-param trigger pattern, same optional-persist semantics.

Performance

N/A — single-page fetch + a handful of small SQL writes. No N+1.

Tests

No unit tests for the new endpoints. Consistent with existing admin trigger endpoints (Yandex equivalents also lack tests). Manual smoke per PR test plan is the established posture here.

Frontend UX

Numeric inputs use parseInt(value, 10) after value !== "" guard → NaN avoided for empty case; garbage from type="number" blocked by browser. Acceptable.
Section 4 placeholder no longer links to /scrapers/cian-cookies, but the top-of-page Cookies callout still does → no dangling-link regression.

Verdict: APPROVE

Clean, well-scoped, low-risk parity work. Merge unblocked.

Recommended follow-ups (non-blocking)

Centralize SSRF allow-list for all admin scrape trigger endpoints (cian + yandex + avito families).
Optional: return 404 from cian-detail/cian-newbuilding when listing_id/house_id not found in DB before invoking parsers (better UX, no integrity error surface).

## Deep Code Review — PR #525 ### Summary - Status: APPROVE - Files: 2 (P1 backend +89 / P1 frontend +140/-31) - Risk: low (admin debug endpoints, parity with merged Yandex analog) ### Findings **Security** - SSRF surface: `fetch_detail(offer_url)`/`fetch_newbuilding(zhk_url)` pass user-supplied URL directly to `AsyncSession.get(..., allow_redirects=True)`. No domain allow-list. This is **pre-existing class** — identical posture in `scrape_yandex_detail`/`scrape_yandex_newbuilding`/`scrape_yandex_valuation` already in main. Gated only by Caddy `basic_auth` on `/trade-in/api/v1/admin/*` per file docstring. Not a regression; flag for a follow-up hardening pass across all admin scrape triggers (allow-list `cian.ru` / `yandex.ru` hosts). - Frontend XSS: clean. `ResultPanel` renders via `<pre>{JSON.stringify(mut.data, null, 2)}</pre>` and `mut.error.message` as text — no `dangerouslySetInnerHTML`, no `<a href={userInput}>` (safeUrl helper from #512 not relevant here). - CSRF: not protected at app layer, consistent with rest of admin surface (Caddy basic_auth + cookie context). Same posture as Yandex endpoints. - Query-string encoding: correct — `URLSearchParams` single-encodes; no double-encoding regression (test plan confirms this was the pre-push fix in 82a78c3). **Correctness** - `fetch_detail` is async, `save_detail_enrichment` is sync, `fetch_newbuilding` async, `save_newbuilding_enrichment` async → all `await`/non-`await` calls in admin.py match signatures (verified at cian_detail.py:59,216 and cian_newbuilding.py:78,391). - Both writers commit internally (`db.commit()` at cian_detail.py:318, cian_newbuilding.py:541). `Depends(get_db)` session is fine — no nested transaction risk. - Idempotency: `offer_price_history` UPSERT relies on UNIQUE constraint from PR #523 (047_cian_history_sanitize.sql) — merged 2026-05-24 13:44 UTC, before this PR. `houses_price_dynamics` uses `ON CONFLICT ON CONSTRAINT houses_price_dynamics_dim_key DO UPDATE`. Both clean. - FK safety: `offer_price_history.listing_id REFERENCES listings(id)` — bad `listing_id` → IntegrityError → FastAPI 500 (not silent corruption). UX could be improved with a pre-check returning 404, but acceptable for an admin debug endpoint. **Nit, not blocking.** - psycopg v3 CAST rules: all params use `CAST(:x AS type)` — compliant. - No regression in existing admin routes: router decorator count 19 → 21, only two new routes added. **Project conventions** - Function-local imports for `cian_detail`/`cian_newbuilding` symbols (admin.py:956,999) deviate from the file's top-level-import norm. Justified: top-level `fetch_detail`/`save_detail_enrichment` are already bound to the **Avito** versions at line 27 — function-scope rebinding cleanly avoids the symbol collision and keeps the Avito-detail endpoint at line 471 unaffected. Acceptable trade-off; alternative would be aliased top-level imports. - Pattern parity with `scrape_yandex_detail`/`scrape_yandex_newbuilding` (admin.py:829–933) is exact: same response-model style, same query-param trigger pattern, same optional-persist semantics. **Performance** - N/A — single-page fetch + a handful of small SQL writes. No N+1. **Tests** - No unit tests for the new endpoints. Consistent with existing admin trigger endpoints (Yandex equivalents also lack tests). Manual smoke per PR test plan is the established posture here. **Frontend UX** - Numeric inputs use `parseInt(value, 10)` after `value !== ""` guard → NaN avoided for empty case; garbage from `type="number"` blocked by browser. Acceptable. - Section 4 placeholder no longer links to `/scrapers/cian-cookies`, but the top-of-page Cookies callout still does → no dangling-link regression. ### Verdict: APPROVE Clean, well-scoped, low-risk parity work. Merge unblocked. ### Recommended follow-ups (non-blocking) 1. Centralize SSRF allow-list for all admin scrape trigger endpoints (cian + yandex + avito families). 2. Optional: return 404 from cian-detail/cian-newbuilding when `listing_id`/`house_id` not found in DB before invoking parsers (better UX, no integrity error surface).