diff --git a/.planning/phases/07-dashboard-nav-polish/07-RESEARCH.md b/.planning/phases/07-dashboard-nav-polish/07-RESEARCH.md new file mode 100644 index 0000000..441e0b4 --- /dev/null +++ b/.planning/phases/07-dashboard-nav-polish/07-RESEARCH.md @@ -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 `
  • `. After the fix passes real printer objects, users can't navigate to a printer from the dashboard. +**How to avoid:** Wrap with `{{ printer.name }}` 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