docs(state): Phase E complete (E1-E5), next is Phase F markers
This commit is contained in:
+50
-36
@@ -1,6 +1,6 @@
|
|||||||
# Premium Mechanic PWA — Implementation State
|
# Premium Mechanic PWA — Implementation State
|
||||||
|
|
||||||
> Last updated: 2026-05-17 (end of session 2 — Phase B + Phase C + Phase D done)
|
> Last updated: 2026-05-17 (end of session 3 — Phase E COMPLETE)
|
||||||
|
|
||||||
## What is done
|
## What is done
|
||||||
|
|
||||||
@@ -23,12 +23,17 @@
|
|||||||
| D | D2: `GET /inspections/{id}` with photos+markers | ✅ DONE | commit `1776a81` — service `get_inspection_by_id_with_relations` (selectinload both relationships, `scalar_one_or_none`); endpoint raises 404 with `"inspection not found"` parallel to vehicle's 404. D1's inline re-query block refactored to call the same helper (DRY). 6 new tests (4 endpoint + 2 service introspecting `stmt._with_options`). |
|
| D | D2: `GET /inspections/{id}` with photos+markers | ✅ DONE | commit `1776a81` — service `get_inspection_by_id_with_relations` (selectinload both relationships, `scalar_one_or_none`); endpoint raises 404 with `"inspection not found"` parallel to vehicle's 404. D1's inline re-query block refactored to call the same helper (DRY). 6 new tests (4 endpoint + 2 service introspecting `stmt._with_options`). |
|
||||||
| D | D3: `PATCH /inspections/{id}` status/notes/finished_at | ✅ DONE | commit `aefe117` — service `get_inspection_by_id` (thin `session.get` wrapper) + `patch_inspection` (auto-sets `finished_at=now(UTC)` when status→completed AND finished_at not in payload; `setattr` loop applies all `model_dump(exclude_unset=True)` fields). Endpoint re-loads via `get_inspection_by_id_with_relations` for the response. 8 new tests (6 endpoint + 2 service). Endpoint and service both named `patch_inspection` — disambiguated by module path in code, by `as svc_patch` alias in tests. |
|
| D | D3: `PATCH /inspections/{id}` status/notes/finished_at | ✅ DONE | commit `aefe117` — service `get_inspection_by_id` (thin `session.get` wrapper) + `patch_inspection` (auto-sets `finished_at=now(UTC)` when status→completed AND finished_at not in payload; `setattr` loop applies all `model_dump(exclude_unset=True)` fields). Endpoint re-loads via `get_inspection_by_id_with_relations` for the response. 8 new tests (6 endpoint + 2 service). Endpoint and service both named `patch_inspection` — disambiguated by module path in code, by `as svc_patch` alias in tests. |
|
||||||
| D | Housekeeping: extract test fakes to `mechanic_fakes.py` | ✅ DONE | commit `eedbf23` — pure refactor. Moved `_FakeResult`, `_FakeSession`, `_fake_user`, `_fake_car`, `_fake_inspection`, `_fake_inspection_orm`, `_fake_marker_orm` from `test_mechanic_endpoints.py` (was 899 lines) to new `backend/tests/mechanic_fakes.py` (207 lines). Test file dropped to 697 lines. Pytest fixtures (`client`, `override_mechanic`, `override_db`) stayed in the test file. Phase E tests will import from `mechanic_fakes` directly. |
|
| D | Housekeeping: extract test fakes to `mechanic_fakes.py` | ✅ DONE | commit `eedbf23` — pure refactor. Moved `_FakeResult`, `_FakeSession`, `_fake_user`, `_fake_car`, `_fake_inspection`, `_fake_inspection_orm`, `_fake_marker_orm` from `test_mechanic_endpoints.py` (was 899 lines) to new `backend/tests/mechanic_fakes.py` (207 lines). Test file dropped to 697 lines. Pytest fixtures (`client`, `override_mechanic`, `override_db`) stayed in the test file. Phase E tests will import from `mechanic_fakes` directly. |
|
||||||
|
| E | E1: MinIO async presigned PUT URL wrapper | ✅ DONE | commit `fe38981` — added `presigned_put_url(bucket, key, *, content_type, expires_sec=300)` to `app/services/storage.py`; `PresignedUploadResponse.fields` defaults to `{}`; 3 storage tests. |
|
||||||
|
| E | E2: `POST /inspections/{id}/photos/upload-url` | ✅ DONE | commit `111661b` — service `create_pending_photo` + `_storage_key_for_photo` helper in `mechanic_service.py`; endpoint in `mechanic.py` with 415-before-404 guard; UUID4 hex storage keys at `mechanic/inspections/{id}/<uuid>.<ext>`; 9 new tests (7 endpoint + 2 service). |
|
||||||
|
| E | E3: `POST /photos/{id}/confirm` | ✅ DONE | commit `724b209` — service `get_photo_by_id` + `confirm_photo` (None-safe dim update, status→ready); endpoint verifies `storage.exists`, 409 if missing, 404 covers both photo-missing and inspection-mismatch; 9 new tests (7 endpoint + 2 service). `_fake_photo_orm` factory added to `mechanic_fakes.py`. |
|
||||||
|
| E | E4: `GET /photos/{id}` + `/thumb` (302 redirect) | ✅ DONE | commit `8361ee1` — auth-guarded 302 to presigned MinIO GET URLs (1h expiry). Thumb endpoint falls back to `storage_key` when `thumb_key is None` so galleries degrade gracefully if E5 task hasn't run yet. 8 endpoint tests. |
|
||||||
|
| E | E5: async thumbnail generation | ✅ DONE | commit `b37a068` — `generate_and_store_thumbnail` worker (download → Pillow resize to 480px → upload → DB update) with try/except at each step (failures logged, return None, never crashes event loop). Triggered via `asyncio.create_task` from E3 confirm endpoint. `_thumb_key_for` predictable naming (`<key-base>-thumb.jpg`). `pillow_heif.register_heif_opener()` wrapped in try/except for dev-env tolerance. 6 new tests (5 storage unit + 1 endpoint trigger). |
|
||||||
|
|
||||||
**Branch state:** `feat/mechanic-mvp` at `eedbf23`. Full stack: `be0553e` (A4) → `d2113a0` (B1) → `2739cc8` + `2fb978d` (B3) → `f0d7e83` (B4) → `a0bdcaa` + `1e5b822` (C1) → `7be8783` (C2) → `9109383` + `90b8c66` (C3) → `a187a92` (B3-vendor-align) → `39a6579` + `ee3d370` (D1) → `1776a81` (D2) → `aefe117` (D3) → `eedbf23` (test-fakes-refactor). All pushed to `origin` (Gitea).
|
**Branch state:** `feat/mechanic-mvp` at `b37a068`. Full stack: A4 → B1 → B3 + fix → B4 → C1 + fix → C2 → C3 + fix → B3-vendor-align → D1 + fix → D2 → D3 → test-refactor → E1 → E2 → E3 → E4 → E5. All pushed to `origin` (Gitea). See git log for full SHAs.
|
||||||
|
|
||||||
**Test count on branch:** **108 mechanic tests pass** (38 models + 17 schemas + 9 deps + 44 endpoints across C1-C3 + D1-D3) in ~2s, no real DB needed.
|
**Test count on branch:** **144 mechanic tests pass** (38 models + 17 schemas + 9 deps + 9 storage + 71 endpoints) in ~2.5s, no real DB needed. Pillow image-resize tests use real PIL in-memory (verify JPEG magic header + dimensions); thumbnail task end-to-end uses mocked I/O.
|
||||||
|
|
||||||
**Live mechanic API surface (7 routes):**
|
**Live mechanic API surface (11 routes):**
|
||||||
- `GET /api/v1/mechanic/healthz`
|
- `GET /api/v1/mechanic/healthz`
|
||||||
- `GET /api/v1/mechanic/me`
|
- `GET /api/v1/mechanic/me`
|
||||||
- `GET /api/v1/mechanic/vehicles?q=&limit=`
|
- `GET /api/v1/mechanic/vehicles?q=&limit=`
|
||||||
@@ -36,6 +41,22 @@
|
|||||||
- `POST /api/v1/mechanic/inspections` — create with carry-over for type=return
|
- `POST /api/v1/mechanic/inspections` — create with carry-over for type=return
|
||||||
- `GET /api/v1/mechanic/inspections/{inspection_id}` — full detail with photos+markers
|
- `GET /api/v1/mechanic/inspections/{inspection_id}` — full detail with photos+markers
|
||||||
- `PATCH /api/v1/mechanic/inspections/{inspection_id}` — partial update + auto finished_at
|
- `PATCH /api/v1/mechanic/inspections/{inspection_id}` — partial update + auto finished_at
|
||||||
|
- `POST /api/v1/mechanic/inspections/{inspection_id}/photos/upload-url` — create pending photo row + presigned PUT URL (5min expiry)
|
||||||
|
- `POST /api/v1/mechanic/inspections/{inspection_id}/photos/{photo_id}/confirm` — verify upload, mark ready, schedule thumbnail task
|
||||||
|
- `GET /api/v1/mechanic/photos/{photo_id}` — auth-guarded 302 to presigned GET (1h)
|
||||||
|
- `GET /api/v1/mechanic/photos/{photo_id}/thumb` — same with thumb_key, falls back to original
|
||||||
|
|
||||||
|
### Phase E architecture decisions (RESOLVED 2026-05-17)
|
||||||
|
|
||||||
|
| Decision | Choice | Rationale |
|
||||||
|
|---|---|---|
|
||||||
|
| Upload pattern | PUT (single presigned URL) not POST/FormData | Simpler PWA code: `fetch(url, {method:'PUT', body:blob})`. `PresignedUploadResponse.fields` defaults to `{}` for future POST migration. |
|
||||||
|
| Storage key naming | `mechanic/inspections/{insp_id}/<uuid4hex>.<ext>` | Human-readable path + unguessable suffix. UUID4 avoids enumeration. |
|
||||||
|
| Allowed content types | `image/{jpeg,png,heic,webp}` | Whitelist; 415 on others. |
|
||||||
|
| Confirm status semantics | `status="ready"` set immediately on confirm; thumb generated async | Frontend doesn't need to poll. Galleries fall back to original via E4 endpoint. |
|
||||||
|
| Thumbnail backend | `asyncio.create_task` + Pillow (no Celery) | Pillow + 15+ existing fire-and-forget call sites; no broker setup needed. Thumb resize is ~50ms for typical photo — acceptable on event loop. |
|
||||||
|
| Thumb size | 480px max-edge, JPEG q=85 | Matches `services/download_wazzup_media.py` precedent. |
|
||||||
|
| Auth model for read URLs | Auth-required on `/photos/{id}` endpoint; presigned URLs themselves bearer-free (1h TTL) | Same as existing `media_proxy.py` convention. |
|
||||||
|
|
||||||
### Vendor data reality (from recon 2026-05-17 — 14,013 inspections, 2 years)
|
### Vendor data reality (from recon 2026-05-17 — 14,013 inspections, 2 years)
|
||||||
|
|
||||||
@@ -50,12 +71,6 @@
|
|||||||
| Carry-over in vendor | **None — denormalized snapshot** | Our `prev_inspection_id`+`carried_over_from_id` FK approach is net improvement |
|
| Carry-over in vendor | **None — denormalized snapshot** | Our `prev_inspection_id`+`carried_over_from_id` FK approach is net improvement |
|
||||||
| Vendor inspection status field | **None — write-once when completed** | Our `in_progress`/`completed`/`cancelled` is greenfield, no conflicts |
|
| Vendor inspection status field | **None — write-once when completed** | Our `in_progress`/`completed`/`cancelled` is greenfield, no conflicts |
|
||||||
|
|
||||||
**Live mechanic API surface (4 routes):**
|
|
||||||
- `GET /api/v1/mechanic/healthz` — liveness
|
|
||||||
- `GET /api/v1/mechanic/me` — current user + permissions
|
|
||||||
- `GET /api/v1/mechanic/vehicles?q=&limit=` — list with search (1≤limit≤200)
|
|
||||||
- `GET /api/v1/mechanic/vehicles/{vehicle_id}` — detail with recent inspections
|
|
||||||
|
|
||||||
## Where things live
|
## Where things live
|
||||||
|
|
||||||
### Repos (Gitea: `http://100.64.0.9:3001/tremble7681`)
|
### Repos (Gitea: `http://100.64.0.9:3001/tremble7681`)
|
||||||
@@ -111,25 +126,26 @@ If Phase D/E/F needs a more complex query shape (e.g. `.scalars().one()` / `.fir
|
|||||||
|
|
||||||
## Next task
|
## Next task
|
||||||
|
|
||||||
### **Task E1 — MinIO async client wrapper** (Phase E starts)
|
### **Task F1 — Markers CRUD** (Phase F starts; only 2 tasks)
|
||||||
|
|
||||||
(From `docs/superpowers/plans/2026-05-16-premium-mechanic-phase1.md` Phase E, 5 tasks total: E1 MinIO wrapper, E2 presigned upload generation, E3 photo confirm endpoint, E4 photo-read 302 redirect, E5 celery thumbnail.)
|
(From `docs/superpowers/plans/2026-05-16-premium-mechanic-phase1.md` Phase F.)
|
||||||
|
|
||||||
**Working dir:** `c:\NewProject\taxi-dashboard` (branch `feat/mechanic-mvp`, HEAD `eedbf23`)
|
**Working dir:** `c:\NewProject\taxi-dashboard` (branch `feat/mechanic-mvp`, HEAD `b37a068`)
|
||||||
|
|
||||||
⚠️ **Heads up — Phase E introduces a new external dependency layer (MinIO/S3) and possibly Celery.** Qualitatively new domain:
|
Phase F adds the marker endpoints — finally exposes the carry-over data we've been carefully copying since D1. After F, backend Phase 1 MVP is feature-complete and Phase G begins frontend work.
|
||||||
- **S3 client (`aioboto3` or `boto3` async wrapper)** — needs a thin wrapper module so endpoints don't import the SDK directly. Check what's already in `requirements.txt` (production has `media_proxy.py` and the existing `taxi-minio` container — there may be a usable abstraction already).
|
|
||||||
- **Presigned URL generation** for client-side direct uploads — credentials, expiration, content-type contract.
|
|
||||||
- **Celery for thumbnail generation** — this is a **major new dependency**. The TaxiDashboard backend may already have Celery (check `app/services/` or `app/tasks/`). If not, adding it is a big infrastructure step that warrants brainstorming + maybe a separate prep task.
|
|
||||||
- **Photo confirm flow** — client uploads to presigned URL → calls our `POST /photos/{id}/confirm` → backend updates `status='ready'` (matching the `MechanicInspectionPhoto.status` enum from B3).
|
|
||||||
- **302 redirect for reads** — `GET /photos/{id}` issues a 302 with a presigned download URL (avoids streaming binary through FastAPI).
|
|
||||||
|
|
||||||
**Recommendation for the next session:** before dispatching E1 implementer, **brainstorm**:
|
**Two endpoints:**
|
||||||
1. **Existing MinIO integration audit** — check `app/api/media_proxy.py`, look for existing S3 client setup, env vars (`MINIO_*` likely already in `.env`), bucket naming (the `pp-inspections` bucket is already set up per Phase A3).
|
- `POST /inspections/{inspection_id}/markers` — create a new damage marker (request: `MarkerCreate` from B3)
|
||||||
2. **Celery audit** — is Celery already running? If yes, where; if no, do we want it for Phase 1 or can thumbnails be sync-on-confirm (faster ship, slightly slower confirm endpoint).
|
- `PATCH /markers/{marker_id}` — update marker (request: `MarkerPatch` from B3)
|
||||||
3. **Auth model for presigned URLs** — they're time-bounded but anyone with the URL can hit MinIO directly. Are we OK with that for inspection photos? Or do we want signed URLs to also encode a JWT claim?
|
|
||||||
|
|
||||||
Read the E1 task definition in the plan before deciding. The B3 `PresignedUploadRequest` / `PresignedUploadResponse` schemas are already shipped and define the API contract — the endpoints just need to be wired up.
|
The plan likely also wants `DELETE /markers/{marker_id}` — verify in the plan. There may also be a "resolve marker" endpoint that's just a PATCH with `resolved=True`.
|
||||||
|
|
||||||
|
**Important context:**
|
||||||
|
- `MechanicDamageMarker` has a stale comment at line 54 (refs old DamageType values like `rust`/`glass`/`paint`) — fix in F1 while touching this model area.
|
||||||
|
- `MarkerCreate` and `MarkerOut` schemas already exist in `mechanic_schemas.py` from B3.
|
||||||
|
- The carry-over markers (`carried_over_from_id IS NOT NULL`) deserve a thought: PATCH should be allowed to update them (mechanic confirms the existing damage is still there with current photo); DELETE should probably be NOT allowed on carry-overs (only on freshly-created markers) to preserve audit trail. Verify this in the plan.
|
||||||
|
|
||||||
|
**Phase F implementer pattern:** apply the same Phase D path overrides (flat `app/api/`, `MechanicDamageMarker` not `DamageMarker`, etc.). Reuse `_FakeSession` + `_fake_marker_orm` from `mechanic_fakes.py`.
|
||||||
|
|
||||||
### Phase D architecture decisions (RESOLVED 2026-05-17)
|
### Phase D architecture decisions (RESOLVED 2026-05-17)
|
||||||
|
|
||||||
@@ -179,33 +195,31 @@ In a new Claude Code chat in `c:\NewProject\`, paste this prompt:
|
|||||||
|
|
||||||
Прочитай PremiumDriverApp/docs/superpowers/STATE.md — там полное состояние и следующая задача.
|
Прочитай PremiumDriverApp/docs/superpowers/STATE.md — там полное состояние и следующая задача.
|
||||||
|
|
||||||
Phase D полностью завершён (D1–D3, последний коммит eedbf23, 108/108 тестов зелёных).
|
Phase E полностью завершён (E1-E5, последний коммит b37a068, 144/144 тестов зелёных).
|
||||||
Backend read+write API готов: 7 routes. Следующая фаза — Phase E (MinIO storage layer:
|
Backend MVP API готов: 11 routes (полный flow inspection + photos + thumbnails).
|
||||||
presigned upload + confirm + 302 read + celery thumbnail) из плана
|
Следующая фаза — Phase F (markers CRUD, 2 задачи) из плана.
|
||||||
PremiumDriverApp/docs/superpowers/plans/2026-05-16-premium-mechanic-phase1.md.
|
|
||||||
|
|
||||||
⚠️ Перед спавном E1 implementer сделай 3 проверки:
|
После F backend Phase 1 MVP feature-complete и начинается Phase G (frontend PWA scaffold).
|
||||||
1. Audit existing MinIO integration: app/api/media_proxy.py, env vars, S3 client.
|
|
||||||
2. Audit Celery: уже установлен? Где? Иначе — sync thumbnails в Phase 1.
|
|
||||||
3. Решить про auth для presigned URLs (см. секцию "Next task" в STATE.md).
|
|
||||||
|
|
||||||
Используй superpowers:brainstorming для решений, потом superpowers:subagent-driven-development.
|
Используй superpowers:subagent-driven-development.
|
||||||
```
|
```
|
||||||
|
|
||||||
That prompt is the **single entry point** for the next session — Claude will read STATE.md, audit existing MinIO/Celery state, resolve the auth-for-URLs decision, then dispatch the E1 implementer.
|
That prompt is the **single entry point** for the next session — Claude will read STATE.md, ground itself, and dispatch the F1 implementer.
|
||||||
|
|
||||||
## Remaining roadmap (Phase 1 MVP)
|
## Remaining roadmap (Phase 1 MVP)
|
||||||
|
|
||||||
- ~~**Phase B** (4 tasks)~~ ✅ done — module skeleton, models, schemas, auth deps
|
- ~~**Phase B** (4 tasks)~~ ✅ done — module skeleton, models, schemas, auth deps
|
||||||
- ~~**Phase C** (3 tasks)~~ ✅ done — GET /me, GET /vehicles, GET /vehicles/{id}
|
- ~~**Phase C** (3 tasks)~~ ✅ done — GET /me, GET /vehicles, GET /vehicles/{id}
|
||||||
- ~~**Phase D** (3 tasks)~~ ✅ done — POST /inspections (+ carry-over), GET /inspections/{id}, PATCH /inspections/{id}
|
- ~~**Phase D** (3 tasks)~~ ✅ done — POST /inspections (+ carry-over), GET /inspections/{id}, PATCH /inspections/{id}
|
||||||
- **Phase E** (5 tasks): MinIO storage layer + presigned upload/confirm + 302 read + celery thumbnail ← **NEXT**
|
- ~~**Phase E** (5 tasks)~~ ✅ done — presigned PUT URL, photo upload-url, confirm, GET /photos/{id}+thumb, async thumb generation
|
||||||
- **Phase F** (2 tasks): markers create/update/delete
|
- **Phase F** (2 tasks): markers create/update/delete ← **NEXT**
|
||||||
- **Phase G** (5 tasks): Vite + React + Tailwind + shadcn + PWA scaffold
|
- **Phase G** (5 tasks): Vite + React + Tailwind + shadcn + PWA scaffold
|
||||||
- **Phase H** (3 tasks): Login + Home + VehicleCard pages
|
- **Phase H** (3 tasks): Login + Home + VehicleCard pages
|
||||||
- **Phase I** (6 tasks): CameraCapture + PhotoSlot + VehicleScheme + InspectionEditor + InspectionReview
|
- **Phase I** (6 tasks): CameraCapture + PhotoSlot + VehicleScheme + InspectionEditor + InspectionReview
|
||||||
- **Phase J** (4 tasks): e2e test + deploy backend + deploy frontend + smoke test
|
- **Phase J** (4 tasks): e2e test + deploy backend + deploy frontend + smoke test
|
||||||
|
|
||||||
|
≈ 20 tasks left until MVP live on `mechanic.pptaxi.ru`. Backend MVP is feature-complete after F.
|
||||||
|
|
||||||
≈ 25 tasks left until MVP live on `mechanic.pptaxi.ru`.
|
≈ 25 tasks left until MVP live on `mechanic.pptaxi.ru`.
|
||||||
|
|
||||||
## Known external dependencies
|
## Known external dependencies
|
||||||
|
|||||||
Reference in New Issue
Block a user