Hartmut
b583b0d7a2
- Per-render-position focal_length_mm/sensor_width_mm (DB → pipeline → Blender)
- FOV-based camera distance with min clamp fix for wide-angle lenses
- Unmapped materials blocking dialog on "Dispatch Renders" with batch alias creation
- Material check endpoint (GET /orders/{id}/check-materials)
- Batch alias endpoint (POST /materials/batch-aliases)
- Quick-map "No alias" badges on Materials page
- Full product hard-delete with storage cleanup (MinIO + disk files + orphaned CadFile)
- Delete button on ProductDetail page with confirmation
- Clickable product names in Media Browser (links to product page)
- Single-line render dispatch/retry (POST /orders/{id}/lines/{id}/dispatch-render)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
62 lines
3.4 KiB
Markdown
62 lines
3.4 KiB
Markdown
# Review Report: Material Alias Completeness with Blocking Dialog
|
|
Date: 2026-03-14
|
|
|
|
## Result: ✅ Approved
|
|
|
|
## Checklist Results
|
|
|
|
### Backend / Python
|
|
- [x] New endpoints have role checks: `check_materials` uses `get_current_user`, `batch_create_aliases` uses `require_admin_or_pm`
|
|
- [x] No SQL injections — all ORM usage
|
|
- [x] Pydantic input validation: `BatchAliasCreate` with `BatchAliasMapping` for POST body
|
|
- [x] Invalid IDs return 404: order not found → 404, material not found → 404
|
|
- [x] No new routers — endpoints added to existing `orders` and `materials` routers
|
|
- [x] No new models — uses existing `Material`, `MaterialAlias`
|
|
- [x] Async consistency: all handlers are `async def`
|
|
- [x] No `print()` in production code
|
|
- [x] No hardcoded paths
|
|
- [x] `storage_key` not touched
|
|
|
|
### Database
|
|
- [x] No migration needed — uses existing tables
|
|
|
|
### Frontend / TypeScript
|
|
- [x] New API interfaces in `frontend/src/api/materials.ts`: `MaterialSuggestion`, `UnmappedMaterial`, `UnmappedMaterialCheck`
|
|
- [x] No `as any` for API responses — correct types throughout
|
|
- [x] CSS vars use inline style where needed (`style={{ backgroundColor: 'var(--color-bg-surface)' }}`)
|
|
- [x] Loading states: `checkingMaterials` for button, `saving` in dialog, `quickMapMut.isPending`
|
|
- [x] Error feedback: error state in dialog, toast on quick-map success/failure
|
|
- [x] No new role-dependent UI elements (dispatch button already gated by `canDispatch`)
|
|
- [x] TypeScript compiles clean (`tsc --noEmit` passes)
|
|
|
|
### Render Pipeline
|
|
- [x] Material alias lookup order preserved: aliases FIRST, then exact name
|
|
- [x] `find_unmapped_materials()` follows same resolution logic as `resolve_material_map()`
|
|
|
|
### Security
|
|
- [x] No credentials in code
|
|
- [x] No hardcoded tokens
|
|
- [x] English variable names and comments
|
|
|
|
## Minor Notes (non-blocking)
|
|
|
|
### `UnmappedMaterialsDialog.tsx:56` — `bg-amber-500/10` opacity syntax
|
|
Uses Tailwind opacity on `bg-amber-500/10` and `border-amber-500/30`. This is fine because `amber-500` is a static Tailwind color (not a CSS variable), so the `/opacity` syntax works correctly here. No issue.
|
|
|
|
### `UnmappedMaterialsDialog.tsx:99` — `.sort()` mutates in render
|
|
The `libraryMaterials.sort(...)` inside JSX mutates the filtered array on every render. Functionally harmless since `libraryMaterials` is a fresh array from `.filter()`, but a `[...libraryMaterials].sort(...)` or `useMemo` would be cleaner. Non-blocking.
|
|
|
|
### `dispatch_single_line_render` endpoint in orders.py
|
|
This endpoint (and its frontend `dispatchLineRender` function + `dispatchLineMut` usage in `OrderLineRow`) appeared in the diff but is not part of the material alias plan. It's a separate per-line dispatch feature. It looks correct: proper auth (`require_admin_or_pm`), 404 checks, status validation, same dispatch pattern as the bulk dispatch endpoint.
|
|
|
|
## Positives
|
|
- Clean separation: service function (`find_unmapped_materials`) is reusable and follows the same resolution logic as `resolve_material_map()`
|
|
- Deduplication in material name collection (`seen` set, case-insensitive)
|
|
- Graceful fallback: if `checkOrderMaterials` fails, dispatch proceeds anyway (no blocking on network errors)
|
|
- Suggestions via `SequenceMatcher` give useful context without external dependencies
|
|
- Quick-map on Materials page provides a second entry point for the same workflow
|
|
- Batch alias endpoint reuses existing `MaterialAlias` model — no schema changes needed
|
|
|
|
## Recommendation
|
|
Approved — ready to commit.
|