Hartmut/HartOMat
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: per-position camera settings, material alias dialog, product delete, media browser links

Browse Source 
- 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>
This commit is contained in:
Hartmut Nörenberg
2026-03-14 12:16:37 +01:00
parent 0020376702
commit b583b0d7a2
48 changed files with 1827 additions and 376 deletions
Download Patch File Download Diff File Expand all files Collapse all files
review-report.md
+45 -43
View File
@@ -1,59 +1,61 @@
# Review Report: Draw Call Batching + Merge Dual STEP Parse
Date: 2026-03-13
# Review Report: Material Alias Completeness with Blocking Dialog
Date: 2026-03-14
## Result: ⚠️ Minor issues
## Problems Found
### [backend/app/services/step_processor.py:405] `dataclass` import at module middle
**Severity**: Low
**Recommendation**: Move `from dataclasses import dataclass, field` to the top of the file with other stdlib imports. Mid-file imports work but violate PEP 8.
### [frontend/src/components/cad/useGeometryMerge.ts:6] Unused import `forEachMeshMaterial`
**Severity**: Low
**Recommendation**: Remove `forEachMeshMaterial` from the import — it's imported but never called.
### [frontend/src/components/cad/useGeometryMerge.ts:148] `partMaterials` and `pbrMap` in useEffect deps
**Severity**: Medium
**Recommendation**: `partMaterials` and `pbrMap` are objects that get new references on re-render. The `stateRef.current` early return (line 55) mitigates unnecessary re-merge, BUT the cleanup function (line 141) runs on every deps change, setting `stateRef.current = null` and triggering a full re-merge next render. Consider comparing by stable serialization or using refs. **Not blocking** because `partMaterials` only changes on user save, and the re-merge is fast for typical assemblies.
### [backend/app/services/step_processor.py:487] `ReadFile` status unchecked
**Severity**: Low
**Recommendation**: `status` from `reader.ReadFile()` is captured but never validated. Not a regression — same pattern exists at lines 270 and 650 in existing code.
## Result: ✅ Approved
## Checklist Results
### Backend / Python
- [x] No new endpoints — no role check needed
- [x] No SQL injections
- [x] Async consistency: `extract_step_metadata` is sync (called from sync Celery tasks)
- [x] Uses `logger` (not `print()`)
- [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] Fallback preserves existing behavior
- [N/A] No new models or migrations
- [x] `storage_key` not touched
### Database
- [x] No migration needed — uses existing tables
### Frontend / TypeScript
- [x] `tsc --noEmit` passes with 0 errors
- [x] No new API interfaces needed (pure client-side optimization)
- [x] No Tailwind opacity syntax violations
- [x] Icons from `lucide-react` only
- [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 or secrets
- [x] No hardcoded tokens
- [x] English variable names and comments
## Positives
## Minor Notes (non-blocking)
1. **Excellent fallback strategy**: Both callers try unified read first, fall back to separate reads on failure. Zero-risk deployment.
2. **Geometry attribute compatibility check** (useGeometryMerge.ts:75-81): Correctly compares attribute sets before merging — prevents crash on mismatched attributes.
3. **Clean restore logic**: `_restore()` properly disposes merged geometry and materials, restores visibility and raycast. No memory leaks on toggle-off.
4. **OCC.Core vs OCP dual-import handling**: `_using_ocp` flag with lambda wrappers for `_s` suffix dispatch matches existing codebase pattern.
5. **World transform baking** (useGeometryMerge.ts:71-73): Correctly applies `matrixWorld` to cloned geometry before merging.
6. **Cloned geometry disposal** (line 131): Disposes intermediate buffers after merge.
### `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 with minor cleanup. The unused import and mid-file import are trivial fixes. The useEffect deps issue is mitigated by existing guards and only affects rapid material reassignment during perfMode — acceptable for now.
Review complete. Result: ⚠️
Approved — ready to commit.