feat(tradein-ui): placement history card #529

Merged
lekss361 merged 1 commit from feat/tradein-ui-placement-history-card into main 2026-05-24 14:22:17 +00:00
Owner

Context

New backend endpoint GET /estimate/{id}/placement-history (PR 11a) exposes historical lots from house_placement_history linked to target house after PR #527 bootstrap.

Changes

  • types/trade-in.ts — added PlacementHistoryItem interface mirroring backend schema
  • lib/trade-in-api.tsuseEstimatePlacementHistory hook (TanStack Query, staleTime: 10 min, enabled guard)
  • PlacementHistoryCard.tsx (new) — table: title / start→last price / last date / exposure_days. Slices at 20 с «показано N из M» note. Returns null on loading/error/empty (quiet fail — supplementary card).
  • page.tsx — imports PlacementHistoryCard, renders after HouseInfoCard wrapped in {currentEstimateId && …} guard

HouseInfoCard.tsx already handled non-empty arrays correctly — no changes.

Dependencies

Deploys AFTER PR #527 (DB backfill) и PR 11a (backend endpoint). Card будет пустой / hidden пока зависимости не merged.

Verify

After deploy chain (527 → 11a → this):

  • Open https://gendsgn.ru/trade-in?id= for any estimate
  • HouseInfoCard shows real house data (year_built, total_floors, source)
  • PlacementHistoryCard shows historical lots если у дома есть записи в house_placement_history
## Context New backend endpoint `GET /estimate/{id}/placement-history` (PR 11a) exposes historical lots from `house_placement_history` linked to target house after PR #527 bootstrap. ## Changes - **`types/trade-in.ts`** — added `PlacementHistoryItem` interface mirroring backend schema - **`lib/trade-in-api.ts`** — `useEstimatePlacementHistory` hook (TanStack Query, `staleTime: 10 min`, `enabled` guard) - **`PlacementHistoryCard.tsx`** (new) — table: title / start→last price / last date / exposure_days. Slices at 20 с «показано N из M» note. Returns `null` on loading/error/empty (quiet fail — supplementary card). - **`page.tsx`** — imports `PlacementHistoryCard`, renders after `HouseInfoCard` wrapped in `{currentEstimateId && …}` guard `HouseInfoCard.tsx` already handled non-empty arrays correctly — no changes. ## Dependencies Deploys AFTER PR #527 (DB backfill) и PR 11a (backend endpoint). Card будет пустой / hidden пока зависимости не merged. ## Verify After deploy chain (527 → 11a → this): - Open https://gendsgn.ru/trade-in?id=<uuid> for any estimate - HouseInfoCard shows real house data (year_built, total_floors, source) - PlacementHistoryCard shows historical lots если у дома есть записи в `house_placement_history`
lekss361 added 1 commit 2026-05-24 14:16:58 +00:00
New /estimate/{id}/placement-history endpoint surfaces historical lots
from house_placement_history for the target house (linked after PR #527
bootstrap). Card hidden when no data.

- PlacementHistoryItem type added to trade-in.ts
- useEstimatePlacementHistory hook added to trade-in-api.ts
- PlacementHistoryCard component uses TanStack Query (no raw fetch)
- Wired into page.tsx after HouseInfoCard
Author
Owner

Deep Code Review #529 — placement history card

Verdict: APPROVE. Small focused UI addition (1 new component + 1 hook + 1 type + 4-line page wiring). Safe to deploy independently of PR #527 / 11a: card silently renders null on isError/empty per PR description.

What was checked

  • Security: no dangerouslySetInnerHTML, no URL rendering — safeUrl not applicable here
  • Correctness: key={it.id} (numeric PK, unique); items.slice(0, 20) only after non-empty guard; date try/catch; page wraps <PlacementHistoryCard estimateId={currentEstimateId} /> in {currentEstimateId && ...} so the string-typed prop is always truthy at call site
  • Hook contract: useEstimatePlacementHistory follows established pattern (queryKey, staleTime 10min, enabled guard) — matches useEstimate / useEstimateHouses / useEstimateImvBenchmark in lib/trade-in-api.ts
  • Types: PlacementHistoryItem interface mirrors house_placement_history schema from PR #527 / decisions/Schema_Avito_Houses_Listings_History
  • Design tokens: uses var(--muted, ...) / var(--border, ...) from trade-in.css (OKLCH palette) with hex fallbacks
  • A11y: semantic <article> / <header> / <h3> / <table> with proper <thead> / <tbody> / <th> / <td>
  • Tests: trade-in card siblings have no tests either — no regression

Minor cosmetic notes (non-blocking)

  • Pluralization "лот" / "лотов" only handles 1 vs not-1. Russian needs 1/2-4/5+ tier: 2 renders "лотов" (should be "лота"); 21 renders "лотов" (should be "лот"). Same simplistic pattern exists in other parts of the app — acceptable.
  • Missing aria-label on <article> — sibling HouseInfoCard.tsx uses aria-label="Информация о доме". Consistency nit, not a blocker.
  • Inline styles vs CSS classes — new card uses inline style={{ padding: "6px 8px" }} etc., while DealsCard.tsx / ListingsCard.tsx lean on semantic CSS classes. HouseInfoCard.tsx mixes both. Mild drift; acceptable given supplementary card status.
  • 404 noise pre-deploy chain — until PR 11a backend lands, every /trade-in?id=... load fires a 404 GET on /api/v1/trade-in/estimate/{id}/placement-history. apiFetch throws → isError=true → renders null (no UI break), but log noise. Acceptable per PR description ordering.

Cross-file impact

  • types/trade-in.ts + lib/trade-in-api.ts are additive — no breaking changes to existing AggregatedEstimate / HouseInfoForEstimate consumers
  • page.tsx insertion ordered after HouseInfoCard, before PhotoUpload — wrapped in currentEstimateId guard; no impact on form-only view (!resultData branch unchanged)
  • No backend changes in this PR (endpoint deferred to 11a) — frontend gracefully degrades

Positive

  • Lazy null render strategy keeps the card invisible until backend is wired — clean staged rollout pattern
  • Consistent hook signature with the rest of trade-in-api.ts
  • Good defensive try/catch around new Date(d).toLocaleDateString for malformed ISO strings

Complexity / blast radius

  • Risk: very low (additive, isolated, graceful degradation)
  • Reversibility: trivial (revert 4-file commit)
  • Merge window: any time
<!-- gendesign-review-bot: sha=11b242d verdict=approve --> ## Deep Code Review #529 — placement history card **Verdict: APPROVE.** Small focused UI addition (1 new component + 1 hook + 1 type + 4-line page wiring). Safe to deploy independently of PR #527 / 11a: card silently renders `null` on isError/empty per PR description. ### What was checked - Security: no `dangerouslySetInnerHTML`, no URL rendering — `safeUrl` not applicable here - Correctness: `key={it.id}` (numeric PK, unique); `items.slice(0, 20)` only after non-empty guard; date `try/catch`; page wraps `<PlacementHistoryCard estimateId={currentEstimateId} />` in `{currentEstimateId && ...}` so the `string`-typed prop is always truthy at call site - Hook contract: `useEstimatePlacementHistory` follows established pattern (queryKey, staleTime 10min, `enabled` guard) — matches `useEstimate` / `useEstimateHouses` / `useEstimateImvBenchmark` in `lib/trade-in-api.ts` - Types: `PlacementHistoryItem` interface mirrors `house_placement_history` schema from PR #527 / decisions/Schema_Avito_Houses_Listings_History - Design tokens: uses `var(--muted, ...)` / `var(--border, ...)` from `trade-in.css` (OKLCH palette) with hex fallbacks - A11y: semantic `<article>` / `<header>` / `<h3>` / `<table>` with proper `<thead>` / `<tbody>` / `<th>` / `<td>` - Tests: trade-in card siblings have no tests either — no regression ### Minor cosmetic notes (non-blocking) - **Pluralization** `"лот" / "лотов"` only handles 1 vs not-1. Russian needs 1/2-4/5+ tier: `2` renders "лотов" (should be "лота"); `21` renders "лотов" (should be "лот"). Same simplistic pattern exists in other parts of the app — acceptable. - **Missing `aria-label`** on `<article>` — sibling `HouseInfoCard.tsx` uses `aria-label="Информация о доме"`. Consistency nit, not a blocker. - **Inline styles vs CSS classes** — new card uses inline `style={{ padding: "6px 8px" }}` etc., while `DealsCard.tsx` / `ListingsCard.tsx` lean on semantic CSS classes. `HouseInfoCard.tsx` mixes both. Mild drift; acceptable given supplementary card status. - **404 noise pre-deploy chain** — until PR 11a backend lands, every `/trade-in?id=...` load fires a 404 GET on `/api/v1/trade-in/estimate/{id}/placement-history`. `apiFetch` throws → `isError=true` → renders null (no UI break), but log noise. Acceptable per PR description ordering. ### Cross-file impact - `types/trade-in.ts` + `lib/trade-in-api.ts` are additive — no breaking changes to existing `AggregatedEstimate` / `HouseInfoForEstimate` consumers - `page.tsx` insertion ordered after `HouseInfoCard`, before `PhotoUpload` — wrapped in `currentEstimateId` guard; no impact on form-only view (`!resultData` branch unchanged) - No backend changes in this PR (endpoint deferred to 11a) — frontend gracefully degrades ### Positive - Lazy `null` render strategy keeps the card invisible until backend is wired — clean staged rollout pattern - Consistent hook signature with the rest of `trade-in-api.ts` - Good defensive `try/catch` around `new Date(d).toLocaleDateString` for malformed ISO strings ### Complexity / blast radius - Risk: very low (additive, isolated, graceful degradation) - Reversibility: trivial (revert 4-file commit) - Merge window: any time
lekss361 merged commit f2f9e5b79b into main 2026-05-24 14:22:17 +00:00
lekss361 deleted branch feat/tradein-ui-placement-history-card 2026-05-24 14:22:17 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lekss361/gendesign#529
No description provided.