docs(phase-07): research dashboard and navigation polish phase

2026-04-10 17:02:49 +02:00
parent 9ba8743144
commit 3e3d4860aa
1 changed files with 256 additions and 0 deletions
@@ -0,0 +1,256 @@
+# Phase 7: Dashboard & Navigation Polish - Research
+
+**Researched:** 2026-04-10
+**Domain:** FastAPI/Jinja2 routing + Peewee ORM query patterns
+**Confidence:** HIGH
+
+## Summary
+
+Phase 7 closes two integration gaps that are visible to users immediately after Phase 3 was complete: a broken `/packages` nav link that returns 404, and a dashboard that always shows empty lists despite the database having real data. Both are small, surgical fixes in existing files with no new models and no schema changes.
+
+The `/packages` nav link in `base.html` points to `/packages`, but no route at that path exists. The packages router (`imptune/api/packages.py`) uses prefix `/printers` and only exposes download endpoints under `/printers/{id}/packages/ninja` and `/printers/{id}/packages/intunewin`. The fix is either: (a) add a `/packages` page route in `pages.py` that lists all printers with package links, or (b) remove/redirect the nav entry. Given the success criteria say "clicking the /packages nav link does not produce a 404", a real page is the right answer.
+
+The dashboard route in `pages.py` hardcodes `recent_printers=[]` and `recent_packages=[]`. The `Printer` model has a `created_at` field suitable for ordering. There is no `ExportLog`/`PackageLog` model — "recent packages" must be derived from printers that have a driver assigned (i.e., are package-ready), unless a lightweight export-log table is added. The simplest interpretation consistent with the success criteria is to show recently-created printers and recently-created printers that have a driver (are exportable), which requires no schema change.
+
+**Primary recommendation:** Add a `/packages` page route that lists all printers with download links. Fix the dashboard route to query `Printer.select().order_by(Printer.created_at.desc()).limit(5)` for recent printers and the same filtered by `Printer.driver.is_null(False)` for recent packages.
+
+## Standard Stack
+
+### Core (already in place — no new installs)
+
+| Library | Version | Purpose | Why Standard |
+|---------|---------|---------|--------------|
+| FastAPI | 0.115+ | Route registration | Already used for all pages |
+| Peewee | 3.x | ORM query builder | Already used for all DB access |
+| Jinja2 | 3.x | Template rendering | Already used for all pages |
+
+**Installation:** No new packages required.
+
+## Architecture Patterns
+
+### Existing Route Pattern (pages.py)
+
+All full-page routes live in `imptune/api/pages.py`. They:
+1. Import ORM models inside the function body (deferred import pattern, prevents circular imports at module load time)
+2. Query with `list(Model.select()...)` — never `Model.get()` (established decision from Phase 3)
+3. Return `templates.TemplateResponse(request=request, name="...", context={...})`
+
+```python
+# Source: imptune/api/pages.py — established pattern
+@router.get("/packages", response_class=HTMLResponse)
+def packages_page(request: Request):
+    from imptune.db.models import Client, Driver, Printer
+    from peewee import JOIN
+
+    printers = list(
+        Printer.select(Printer, Client, Driver)
+        .join(Client, JOIN.LEFT_OUTER)
+        .switch(Printer)
+        .join(Driver, JOIN.LEFT_OUTER)
+        .order_by(Printer.name)
+    )
+    return templates.TemplateResponse(
+        request=request,
+        name="packages.html",
+        context={"printers": printers},
+    )
+```
+
+### Dashboard Query Pattern
+
+```python
+# Source: imptune/db/models.py — Printer has created_at DateTimeField
+@router.get("/", response_class=HTMLResponse)
+def dashboard(request: Request):
+    from imptune.db.models import Printer
+
+    recent_printers = list(
+        Printer.select().order_by(Printer.created_at.desc()).limit(5)
+    )
+    recent_packages = list(
+        Printer.select()
+        .where(Printer.driver.is_null(False))
+        .order_by(Printer.created_at.desc())
+        .limit(5)
+    )
+    return templates.TemplateResponse(
+        request=request,
+        name="dashboard.html",
+        context={
+            "recent_printers": recent_printers,
+            "recent_packages": recent_packages,
+        },
+    )
+```
+
+### Recommended Project Structure
+
+No structural changes. Two files modified, one new template added:
+
+```
+imptune/
+├── api/
+│   └── pages.py          # Add /packages route + fix dashboard route
+└── templates/
+    ├── base.html          # No change needed (link already points to /packages)
+    ├── dashboard.html     # Update to link printer names to detail pages
+    └── packages.html      # NEW — list printers with package download links
+```
+
+### Anti-Patterns to Avoid
+
+- **Don't add an ExportLog model for "recent packages"**: No schema change is warranted. Printers with drivers assigned are the natural proxy for "package-ready" items. Adding a new table would require a migration path and is disproportionate to the fix.
+- **Don't use `Model.get()` in page routes**: Phase 3 decision — use `list(Model.select().where(...))` to avoid Peewee cursor caching issues across DB re-inits in tests.
+- **Don't import models at module level in pages.py**: All existing routes use deferred imports inside function bodies. Match this pattern.
+- **Don't create a separate router for /packages**: The existing `packages.py` router already uses prefix `/printers`. A `/packages` page belongs in `pages.py` alongside other full-page routes.
+
+## Don't Hand-Roll
+
+| Problem | Don't Build | Use Instead | Why |
+|---------|-------------|-------------|-----|
+| Listing printers with LEFT JOIN client/driver | Raw SQL | `Peewee.select(Printer, Client, Driver).join(...)` | Already used in `printer_detail` and `printers_page` — exact same query shape |
+| Template rendering | String concatenation | `templates.TemplateResponse(request=request, ...)` | Starlette 0.40+ kwarg signature — already correct in all routes |
+| 404 detection for missing printer | Custom try/except | `Printer.get_or_none()` | Already established pattern in packages.py |
+
+## Common Pitfalls
+
+### Pitfall 1: Forgetting `request.url.path` active state in base.html
+
+**What goes wrong:** After adding `/packages` route, the nav item won't highlight as active unless `base.html` already has the conditional. Check line 23 of `base.html` — it already does: `{% if request.url.path == "/packages" %}class="active"{% endif %}`. No change needed to base.html.
+
+**How to avoid:** Verified — nav link and active-class logic for `/packages` are already in `base.html`.
+
+### Pitfall 2: `Printer.driver.is_null(False)` vs `Printer.driver != None`
+
+**What goes wrong:** Using Python `!= None` in a Peewee `.where()` clause does not generate valid SQL.
+**How to avoid:** Use `Printer.driver.is_null(False)` for "driver is assigned" filtering.
+
+```python
+# Correct
+Printer.select().where(Printer.driver.is_null(False))
+# Wrong — silent bug
+Printer.select().where(Printer.driver != None)
+```
+
+### Pitfall 3: Peewee deferred FK access in templates (N+1 / unresolved FK)
+
+**What goes wrong:** If dashboard only queries `Printer.select()` without joining `Client`, accessing `printer.client.name` in the template triggers an extra query per row or raises `DoesNotExist` if the FK is null. For dashboard with `.limit(5)` this is tolerable, but client name display must guard on null.
+
+**How to avoid:** Either join eagerly or guard in the template with `{% if printer.client_id %}{{ printer.client.name }}{% endif %}`.
+
+### Pitfall 4: packages.html template file missing
+
+**What goes wrong:** Adding the `/packages` route without creating `packages.html` raises a `TemplateNotFound` error at runtime.
+**How to avoid:** Create the template before or alongside the route.
+
+### Pitfall 5: dashboard.html links printers by name only
+
+**What goes wrong:** Current dashboard template renders `{{ printer.name }}` in a plain `<li>`. After the fix passes real printer objects, users can't navigate to a printer from the dashboard.
+**How to avoid:** Wrap with `<a href="/printers/{{ printer.id }}">{{ printer.name }}</a>` for better UX.
+
+## Code Examples
+
+### Existing Peewee JOIN pattern (from pages.py printer_detail route)
+
+```python
+# Source: imptune/api/pages.py lines 81-87
+printer = (
+    Printer.select(Printer, Client, Driver)
+    .join(Client, JOIN.LEFT_OUTER)
+    .switch(Printer)
+    .join(Driver, JOIN.LEFT_OUTER)
+    .where(Printer.id == printer_id)
+    .first()
+)
+```
+
+### Confirmed Peewee null FK filtering
+
+```python
+# Source: Peewee docs — .is_null(False) generates "IS NOT NULL"
+Printer.select().where(Printer.driver.is_null(False)).order_by(Printer.created_at.desc()).limit(5)
+```
+
+### TemplateResponse (Starlette 0.40+ kwarg signature)
+
+```python
+# Source: imptune/api/pages.py — established pattern across all routes
+return templates.TemplateResponse(
+    request=request,
+    name="template_name.html",
+    context={"key": value},
+)
+```
+
+## State of the Art
+
+| Old Approach | Current Approach | Impact |
+|--------------|------------------|--------|
+| `@app.on_event("startup")` | `asynccontextmanager lifespan` | Already migrated in Phase 1 |
+| `TemplateResponse("name", {"request": request})` positional dict | `TemplateResponse(request=request, name="name")` kwargs | Already migrated in Phase 1 |
+
+No outdated patterns need attention for this phase.
+
+## Open Questions
+
+1. **What does "recent packages" mean without an export log table?**
+   - What we know: There is no `ExportLog` model. The DB schema has `Client`, `Driver`, `Printer`, `Icon`.
+   - What's unclear: Does "recently exported" mean recently created printers with a driver, or should we track actual export events?
+   - Recommendation: Show printers with a driver assigned, ordered by `created_at desc`. This satisfies success criterion 3 ("recently exported packages from the database") without schema change. The planner may choose to note this interpretation in the plan.
+
+2. **Should `packages.html` be minimal (list only) or show download links?**
+   - What we know: The nav link currently goes nowhere. Success criterion 1 just says "does not produce a 404".
+   - Recommendation: Render a simple table of printers with driver assigned, linking to their detail pages and package download endpoints. Mirrors the information visible on the printer detail page.
+
+## Validation Architecture
+
+### Test Framework
+
+| Property | Value |
+|----------|-------|
+| Framework | pytest |
+| Config file | none — discovered via convention |
+| Quick run command | `pytest tests/test_static.py -x` |
+| Full suite command | `pytest tests/ -x` |
+
+### Phase Requirements to Test Map
+
+| Req ID | Behavior | Test Type | Automated Command | File Exists? |
+|--------|----------|-----------|-------------------|-------------|
+| SC-1 | GET /packages returns 200 | integration | `pytest tests/test_static.py::test_packages_returns_200 -x` | Wave 0 |
+| SC-2 | Dashboard recent_printers populated from DB | integration | `pytest tests/test_static.py::test_dashboard_shows_recent_printers -x` | Wave 0 |
+| SC-3 | Dashboard recent_packages populated from DB | integration | `pytest tests/test_static.py::test_dashboard_shows_recent_packages -x` | Wave 0 |
+
+### Sampling Rate
+
+- **Per task commit:** `pytest tests/test_static.py -x`
+- **Per wave merge:** `pytest tests/ -x`
+- **Phase gate:** Full suite green before `/gsd:verify-work`
+
+### Wave 0 Gaps
+
+- [ ] `tests/test_static.py` — add `test_packages_returns_200`, `test_dashboard_shows_recent_printers`, `test_dashboard_shows_recent_packages` (file exists, add to it)
+- [ ] `imptune/templates/packages.html` — new template file needed
+
+## Sources
+
+### Primary (HIGH confidence)
+
+- Direct code inspection: `imptune/api/pages.py` — confirmed dashboard hardcodes `[]`, confirmed all route patterns
+- Direct code inspection: `imptune/templates/base.html` — confirmed `/packages` nav link exists, active-class logic already present
+- Direct code inspection: `imptune/db/models.py` — confirmed `Printer.created_at` field, confirmed no `ExportLog` model
+- Direct code inspection: `imptune/api/packages.py` — confirmed router prefix is `/printers`, no `/packages` route exists
+
+### Secondary (MEDIUM confidence)
+
+- Peewee docs pattern for `.is_null()` — consistent with project usage observed in codebase
+
+## Metadata
+
+**Confidence breakdown:**
+- Gap identification (404, empty dashboard): HIGH — directly verified by reading source files
+- Fix approach (add route in pages.py, query Printer ORM): HIGH — consistent with all established project decisions
+- "Recent packages" interpretation: MEDIUM — no explicit requirement; derived from available schema
+
+**Research date:** 2026-04-10
+**Valid until:** Stable — no fast-moving dependencies; valid until codebase structural changes