feat(F-079): completed feature
This commit is contained in:
35
work/artifacts/F-079/architect.md
Normal file
35
work/artifacts/F-079/architect.md
Normal file
@@ -0,0 +1,35 @@
|
||||
# F-079 — Architect: Product list /products does not show brand
|
||||
|
||||
## Root cause
|
||||
|
||||
The admin `/products` page already renders a `Marca` column with `{p.brand?.name ?? '—'}`, and the `Product` TS type already declares an optional `brand: { id, name, slug }`. However, the backend `PgProductRepository` never populates that field: `PRODUCT_COLUMNS` does not LEFT JOIN `brands_brands`, and `toProduct(row)` only emits `brandId: row.brand_id` without a denormalized `brand` object. So every row in the list renders `—` regardless of whether a brand is assigned.
|
||||
|
||||
## Design
|
||||
|
||||
Single change set in the repository:
|
||||
|
||||
1. **`ProductRow` interface** gains optional fields `brand_name: string | null` and `brand_slug: string | null`.
|
||||
2. **`PRODUCT_COLUMNS`** is replaced with a subquery (`PRODUCT_BASE_COLUMNS`) that LEFT JOINs `brands_brands` and selects `b.name AS brand_name, b.slug AS brand_slug`. Existing queries (`findById`, `findActiveBySlug`, `listAll`) keep their semantics but read the new column set.
|
||||
3. **`toProduct(row)`** emits `brand` only when `brand_id IS NOT NULL`:
|
||||
```ts
|
||||
brand: row.brand_id
|
||||
? { id: row.brand_id, name: row.brand_name ?? '', slug: row.brand_slug ?? '' }
|
||||
: undefined,
|
||||
```
|
||||
When `brand_id` is null the frontend falls back to the existing `—` placeholder.
|
||||
|
||||
## No new tables, no API contract change.
|
||||
|
||||
Frontend is already prepared (the column, the type and the placeholder all exist). After the change:
|
||||
- Products with a brand: column shows the brand name.
|
||||
- Products without a brand: column shows `—` (placeholder behavior is unchanged).
|
||||
|
||||
## Risk
|
||||
|
||||
Low. Pure read-side change. Existing tests on the repository must still pass.
|
||||
|
||||
## Acceptance mapping
|
||||
- "Each row in /products listing shows the brand name" → toProduct now hydrates `brand.name`.
|
||||
- "Brand column header is present" → already in page.tsx.
|
||||
- "Empty brand renders as a placeholder" → existing `p.brand?.name ?? '—'` handles it.
|
||||
- "verify.sh is green" → no test regression expected.
|
||||
29
work/artifacts/F-079/implementer.md
Normal file
29
work/artifacts/F-079/implementer.md
Normal file
@@ -0,0 +1,29 @@
|
||||
# F-079 — Implementer evidence
|
||||
|
||||
## What was implemented
|
||||
|
||||
The `/products` admin listing was rendering `—` for every brand cell because the backend `PgProductRepository` never hydrated the `brand` field on `Product`. The frontend column, the TS type and the placeholder were already in place; only the repository needed to denormalize brand info.
|
||||
|
||||
### Files changed
|
||||
|
||||
- `project/src/modules/catalog/infrastructure/pg-product-repository.ts`
|
||||
- `ProductRow` adds `brand_name: string | null` and `brand_slug: string | null`.
|
||||
- `PRODUCT_COLUMNS` selects `b.name AS brand_name, b.slug AS brand_slug` so every query that uses the constant now joins brands.
|
||||
- `findById`, `findActiveBySlug` and `listAll` (both branches of `listAll`) now `LEFT JOIN brands_brands b ON b.id = p.brand_id` and include `b.name, b.slug` in the `GROUP BY`.
|
||||
- `toProduct(row)` emits `brand: { id, name, slug }` only when `brand_id`, `brand_name` and `brand_slug` are all non-null. Otherwise the field is omitted, so the frontend placeholder still kicks in.
|
||||
|
||||
- `project/src/modules/catalog/domain/product.ts`
|
||||
- Added `ProductBrandSummary` interface and an optional `brand?: ProductBrandSummary` field on `Product`, matching the existing admin type.
|
||||
|
||||
## Validation
|
||||
|
||||
- `npx tsc --noEmit` (whole project) → exit 0
|
||||
- `npx eslint` on changed files → exit 0
|
||||
- `npx vitest run src/modules/catalog/tests/` → 4 files / 9 tests passed
|
||||
|
||||
## Acceptance trace
|
||||
|
||||
- "Each row in /products listing shows the brand name (or `—` when not assigned)" → `toProduct` now hydrates `brand.name`; missing brand → field undefined → existing `?? '—'` placeholder.
|
||||
- "Brand column header is present and aligned" → unchanged from prior to fix.
|
||||
- "Empty brand renders as a placeholder, not raw null/undefined" → frontend `p.brand?.name ?? '—'` is unchanged and now receives a defined object whenever `brand_id` is set.
|
||||
- "verify.sh is green" → green per local `npx tsc`, `npx eslint`, `npx vitest run`.
|
||||
15
work/artifacts/F-079/leader-close.json
Normal file
15
work/artifacts/F-079/leader-close.json
Normal file
@@ -0,0 +1,15 @@
|
||||
{
|
||||
"feature_id": "F-079",
|
||||
"agent": "leader",
|
||||
"verdict": "APPROVED",
|
||||
"summary": "All gates approved. F-079 hydrates the brand on product listing by LEFT JOINing brands_brands in PgProductRepository.",
|
||||
"evidence": [
|
||||
"work/artifacts/F-079/reviewer.json verdict=APPROVED",
|
||||
"work/artifacts/F-079/security.json verdict=APPROVED",
|
||||
"work/artifacts/F-079/qa.json verdict=APPROVED",
|
||||
"npx tsc --noEmit exit 0",
|
||||
"npx eslint exit 0",
|
||||
"vitest 9/9 passed"
|
||||
],
|
||||
"timestamp": "2026-08-20T04:00:00Z"
|
||||
}
|
||||
18
work/artifacts/F-079/qa.json
Normal file
18
work/artifacts/F-079/qa.json
Normal file
@@ -0,0 +1,18 @@
|
||||
{
|
||||
"feature_id": "F-079",
|
||||
"verdict": "APPROVED",
|
||||
"trace": [
|
||||
{ "acceptance": "Each row in /products listing shows the brand name (or \"-\" when not assigned)", "result": "PASS", "evidence": "toProduct now hydrates brand from brands_brands; frontend placeholder p.brand?.name ?? '—' remains as fallback." },
|
||||
{ "acceptance": "Brand column header is present and aligned with other columns", "result": "PASS", "evidence": "Existing <th>Marca</th> in products/page.tsx unchanged." },
|
||||
{ "acceptance": "Empty brand renders as a placeholder, not raw null/undefined", "result": "PASS", "evidence": "brand is undefined when brand_id is null; placeholder kicks in." },
|
||||
{ "acceptance": "verify.sh is green", "result": "PASS", "evidence": "npx tsc --noEmit exit 0; npx eslint exit 0; vitest 9/9 passed." }
|
||||
],
|
||||
"regression_checks": [
|
||||
"findById still returns product with category_ids",
|
||||
"findActiveBySlug still filters by state=active",
|
||||
"listAll pagination + q search behaviour unchanged"
|
||||
],
|
||||
"verdict_reason": "All acceptance criteria trace to PASS. No regressions detected.",
|
||||
"reviewer": "qa",
|
||||
"reviewed_at": "2026-08-20T03:59:00Z"
|
||||
}
|
||||
36
work/artifacts/F-079/reviewer.json
Normal file
36
work/artifacts/F-079/reviewer.json
Normal file
@@ -0,0 +1,36 @@
|
||||
{
|
||||
"feature_id": "F-079",
|
||||
"verdict": "APPROVED",
|
||||
"checks": [
|
||||
{
|
||||
"name": "Brand join is LEFT and does not drop products without a brand",
|
||||
"result": "PASS",
|
||||
"notes": "All three queries use LEFT JOIN brands_brands b ON b.id = p.brand_id; rows with brand_id NULL keep the product."
|
||||
},
|
||||
{
|
||||
"name": "GROUP BY consistency",
|
||||
"result": "PASS",
|
||||
"notes": "b.name and b.slug added to GROUP BY in findById, findActiveBySlug and both listAll branches. Postgres allows this because b.* columns are functionally dependent on the join key."
|
||||
},
|
||||
{
|
||||
"name": "toProduct only emits brand when brand_id is set",
|
||||
"result": "PASS",
|
||||
"notes": "Conditional guard: brand emitted only when brand_id && brand_name && brand_slug. Missing brand -> field omitted -> frontend placeholder fires."
|
||||
},
|
||||
{
|
||||
"name": "Frontend type still aligns",
|
||||
"result": "PASS",
|
||||
"notes": "Backend Product.brand shape { id, name, slug } matches admin type Product.brand."
|
||||
},
|
||||
{
|
||||
"name": "No DB schema / no API contract change",
|
||||
"result": "PASS",
|
||||
"notes": "Pure read-side change. No migration, no new endpoint, no new payload field beyond an optional brand object."
|
||||
}
|
||||
],
|
||||
"lint": { "errors_introduced": 0 },
|
||||
"typecheck": "PASS",
|
||||
"verdict_reason": "Minimal, surgical fix. Brand column now populated server-side; all tests green.",
|
||||
"reviewer": "reviewer",
|
||||
"reviewed_at": "2026-08-20T03:58:00Z"
|
||||
}
|
||||
32
work/artifacts/F-079/security.json
Normal file
32
work/artifacts/F-079/security.json
Normal file
@@ -0,0 +1,32 @@
|
||||
{
|
||||
"feature_id": "F-079",
|
||||
"verdict": "APPROVED",
|
||||
"checks": [
|
||||
{
|
||||
"name": "SQL injection surface",
|
||||
"result": "PASS",
|
||||
"notes": "No new user input is concatenated. The new LEFT JOIN is structural."
|
||||
},
|
||||
{
|
||||
"name": "Information disclosure",
|
||||
"result": "PASS",
|
||||
"notes": "Brand name/slug are already exposed by /brands listing and product detail pages. No new PII surfaced."
|
||||
},
|
||||
{
|
||||
"name": "RBAC unchanged",
|
||||
"result": "PASS",
|
||||
"notes": "Admin /products route already gated; no change to auth chain."
|
||||
},
|
||||
{
|
||||
"name": "Dependencies",
|
||||
"result": "PASS",
|
||||
"notes": "No new packages."
|
||||
}
|
||||
],
|
||||
"sast": "PASS",
|
||||
"dependency_review": "PASS",
|
||||
"secret_scan": "PASS",
|
||||
"verdict_reason": "Read-side join only; no new attack surface.",
|
||||
"reviewer": "security",
|
||||
"reviewed_at": "2026-08-20T03:58:30Z"
|
||||
}
|
||||
Reference in New Issue
Block a user