Files
ImpTune/.planning/phases/09-ux-tech-debt-closure/09-01-driver-upload-fix-and-inline-oob-PLAN.md
T
2026-04-15 17:57:12 +02:00

263 lines
16 KiB
Markdown

---
phase: 09-ux-tech-debt-closure
plan: 01
type: execute
wave: 1
depends_on: []
files_modified:
- tests/test_driver_upload.py
- imptune/api/drivers.py
- imptune/templates/partials/driver_list.html
- imptune/templates/partials/driver_upload_with_oob.html
- imptune/templates/partials/printer_form.html
autonomous: true
requirements: [UX-01]
must_haves:
truths:
- "POST /drivers/upload never returns HTTP 500 for a valid driver ZIP"
- "Uploading a driver from the printer form refreshes the driver <select> via HTMX OOB swap without a page reload"
- "The newly uploaded driver is auto-selected in the refreshed <select>"
- "Uploading from the standalone /drivers page still returns only the #driver-list fragment (no OOB noise)"
artifacts:
- path: "tests/test_driver_upload.py"
provides: "Regression test for the 500 + OOB contract tests"
contains: "test_upload_500_regression"
- path: "imptune/api/drivers.py"
provides: "Fixed upload_driver handler with caller-aware OOB branch"
contains: "caller"
- path: "imptune/templates/partials/driver_upload_with_oob.html"
provides: "Template emitting primary driver_list fragment + OOB <select>"
contains: "hx-swap-oob"
- path: "imptune/templates/partials/printer_form.html"
provides: "Driver <select> has stable id + separate inline upload form"
contains: "printer-form-driver-select"
key_links:
- from: "imptune/templates/partials/printer_form.html"
to: "POST /drivers/upload"
via: "separate <form hx-post=/drivers/upload> with hidden caller=printer_form field"
pattern: 'name="caller"\s+value="printer_form"'
- from: "imptune/api/drivers.py (upload_driver)"
to: "partials/driver_upload_with_oob.html"
via: "TemplateResponse when caller == 'printer_form'"
pattern: 'driver_upload_with_oob\.html'
- from: "partials/driver_upload_with_oob.html"
to: "printer_form.html #printer-form-driver-select"
via: 'hx-swap-oob="true" on <select id="printer-form-driver-select">'
pattern: 'hx-swap-oob="true"'
---
<objective>
Fix the HTTP 500 on `POST /drivers/upload` (blocking UX-01), then add an inline driver upload form inside the printer form template that, on success, refreshes the driver `<select>` via HTMX Out-of-Band swap and auto-selects the newly uploaded driver.
Purpose: Closes UX-01 — technicians uploading a driver while creating/editing a printer see it appear in the dropdown and get it auto-selected, no manual reload.
Output: Green regression + OOB tests, a working inline upload control, and an OOB-capable upload handler.
</objective>
<execution_context>
@C:/Users/SebastienQUEROL/.claude/get-shit-done/workflows/execute-plan.md
@C:/Users/SebastienQUEROL/.claude/get-shit-done/templates/summary.md
</execution_context>
<context>
@.planning/PROJECT.md
@.planning/ROADMAP.md
@.planning/STATE.md
@.planning/phases/09-ux-tech-debt-closure/09-CONTEXT.md
@.planning/phases/09-ux-tech-debt-closure/09-RESEARCH.md
@.planning/phases/09-ux-tech-debt-closure/09-VALIDATION.md
@imptune/api/drivers.py
@imptune/templates/partials/printer_form.html
@imptune/templates/partials/driver_list.html
@tests/test_driver_upload.py
@tests/conftest.py
<interfaces>
<!-- Key contracts this plan operates on. Use these directly; no codebase exploration needed. -->
From imptune/api/drivers.py:
```python
router = APIRouter(prefix="/drivers") # mounted at /drivers in main.py
MAX_UPLOAD_BYTES = 100 * 1024 * 1024
def _error_response(message: str, status_code: int = 400) -> HTMLResponse: ...
@router.post("/upload", response_class=HTMLResponse)
def upload_driver(request: Request, file: UploadFile) -> HTMLResponse: ...
# Validates zip, parses INF via parse_inf, persists via DriverStore(_cfg.DRIVERS_DIR),
# upserts Driver via Driver.get_or_create(sha256=..., defaults={...}),
# returns TemplateResponse("partials/driver_list.html", {driver_data, parsed}).
# There is NO try/except around parse_inf / DriverStore.save / Driver.get_or_create —
# any of these can bubble into a FastAPI 500.
```
From imptune/db/models.Driver: fields include id, sha256, original_filename, driver_desc (JSON list), inf_filename, architecture, has_cat_file, uploaded_at.
From imptune/services/inf_parser: `parse_inf(inf_text, inf_filename, zip_names) -> ParsedInf` with attributes `driver_names: list[str]`, `inf_filename: str`, `architecture: str | None`, `has_cat_file: bool`.
Current printer_form.html driver select (lines 28-39):
```html
<label>
Driver
<select name="driver_id"> <!-- NO id attribute today -->
<option value="">-- No driver --</option>
{% for item in driver_data %}
<option value="{{ item.driver.id }}"
{% if printer and printer.driver_id == item.driver.id %}selected{% endif %}>
{{ item.driver.original_filename }} ({{ item.names | join(', ') }})
</option>
{% endfor %}
</select>
</label>
```
HTMX OOB contract: response body contains the primary swap fragment (targets `#driver-list`) PLUS one or more sibling elements with `hx-swap-oob="true"` whose `id` matches an element in the current page DOM. Out-of-band elements MUST be top-level in the response body (not nested inside the primary fragment).
</interfaces>
</context>
<tasks>
<task type="auto" tdd="true">
<name>Task 1: Wave 0 — write failing driver-upload regression + OOB contract tests</name>
<files>tests/test_driver_upload.py</files>
<behavior>
- test_upload_500_regression: POST /drivers/upload with a valid driver ZIP fixture MUST return status_code != 500. Use the existing test fixture pattern from tests/conftest.py (tmp_data_dir) and the same synthetic driver ZIP builder already used in this test module if present; otherwise create `_make_driver_zip()` helper that writes a minimal valid INF + `.cat` file into a ZIP. Assertion: `assert resp.status_code == 200, resp.text`.
- test_upload_returns_oob_when_called_from_form: POST /drivers/upload with multipart fields `{file: valid_zip, caller: "printer_form"}` MUST return 200 AND the response body MUST contain `hx-swap-oob="true"` AND `id="printer-form-driver-select"`.
- test_upload_oob_autoselects_new_driver: Same call as above — response body MUST contain the newly created driver's `<option value="{new_id}" selected>` inside the OOB `<select>`. Parse the new id from the response or query the DB post-upload and assert the selected marker appears on that option.
- test_upload_no_oob_from_standalone_drivers_page: POST /drivers/upload with `{file: valid_zip}` and NO `caller` field MUST return the existing `#driver-list` fragment and MUST NOT contain `hx-swap-oob`. This guards against OOB junk leaking into the standalone /drivers page.
</behavior>
<action>
Open `tests/test_driver_upload.py`. Add the four test functions above following the existing httpx TestClient pattern (see `tests/conftest.py` for `client` fixture). Reuse any existing driver-zip helper in the module; if none exists, create `_make_driver_zip() -> bytes` that builds a minimal ZIP with a real `.inf` body (encoding utf-8) whose `[Version]` section declares `Signature="$Windows NT$"` and a single `[Strings]` entry so `parse_inf` returns at least one driver name. Include a `.cat` sibling so `has_cat_file` is True.
Run the tests. At least `test_upload_500_regression` MAY pass or fail depending on synthetic fixture vs. real-world root cause — if it still passes with a synthetic ZIP, ALSO add a parametrized variant that feeds a ZIP containing an INF with a BOM + Windows-1252 encoded `[Strings]` section (the most likely real-world repro per 09-RESEARCH.md pitfall 1). At least one variant MUST go red before proceeding to Task 2.
The other three OOB tests MUST go red — the current handler has no `caller` support and no OOB template.
Commit: `test(09-01): add failing driver upload 500 regression + OOB contract tests`
</action>
<verify>
<automated>pytest tests/test_driver_upload.py::test_upload_500_regression tests/test_driver_upload.py::test_upload_returns_oob_when_called_from_form tests/test_driver_upload.py::test_upload_oob_autoselects_new_driver -x</automated>
</verify>
<done>All four new tests exist in tests/test_driver_upload.py. At least one test is RED (the intended failure). Failing test output captured in commit message or task notes so Task 2 has the traceback.</done>
</task>
<task type="auto" tdd="true">
<name>Task 2: Fix the 500 + extend upload_driver with caller-aware OOB branch</name>
<files>imptune/api/drivers.py, imptune/templates/partials/driver_upload_with_oob.html, imptune/templates/partials/driver_list.html</files>
<behavior>
- All four tests from Task 1 MUST go GREEN.
- `pytest tests/test_driver_upload.py -x` passes fully (no regressions on existing tests).
- `pytest tests/ -x -q --ignore=tests/e2e` passes.
</behavior>
<action>
Step 1 — Diagnose the 500 from the Task 1 red test traceback. Likely candidates per 09-RESEARCH.md pitfall 1: `parse_inf()` choking on encoding, `DriverStore.save()` on missing DRIVERS_DIR, or `Driver.get_or_create()` on constraint. Fix the SPECIFIC root cause only — do NOT wrap the whole handler in `try/except Exception`. If it's `parse_inf`, fix the parser; if it's `DriverStore.save`, ensure the dir exists before writing; if it's ORM, fix the field.
Step 2 — Add `caller: str = Form("")` parameter to `upload_driver(request, file, caller="")` (import `Form` from fastapi). FastAPI handles mixed multipart `UploadFile` + `Form` fields natively.
Step 3 — After the existing success path builds `driver_data`, capture `new_driver` from the `get_or_create` return tuple: `new_driver, _created = Driver.get_or_create(...)`. Currently the code discards this — fix it.
Step 4 — Branch on `caller`:
```python
if caller == "printer_form":
return templates.TemplateResponse(
request=request,
name="partials/driver_upload_with_oob.html",
context={"driver_data": driver_data, "new_driver_id": new_driver.id, "parsed": parsed},
)
# else: existing behavior unchanged
return templates.TemplateResponse(
request=request,
name="partials/driver_list.html",
context={"driver_data": driver_data, "parsed": parsed},
)
```
Step 5 — Create `imptune/templates/partials/driver_upload_with_oob.html`:
```jinja
{% include "partials/driver_list.html" %}
<select name="driver_id" id="printer-form-driver-select" hx-swap-oob="true">
<option value="">-- No driver --</option>
{% for item in driver_data %}
<option value="{{ item.driver.id }}"
{% if item.driver.id == new_driver_id %}selected{% endif %}>
{{ item.driver.original_filename }} ({{ item.names | join(', ') }})
</option>
{% endfor %}
</select>
```
Step 6 — Inspect `partials/driver_list.html`. Confirm its root element has `id="driver-list"` (hx-target from the inline upload form will swap it). If the partial currently wraps itself differently, leave as-is; the OOB template simply includes it. Do NOT refactor driver_list.html unless necessary.
Step 7 — Run the failing tests. Iterate until GREEN. Then run full non-e2e suite.
Commit: `fix(09-01): resolve driver upload 500 and add HTMX OOB refresh path`
</action>
<verify>
<automated>pytest tests/test_driver_upload.py -x -v && pytest tests/ -x -q --ignore=tests/e2e</automated>
</verify>
<done>All Task 1 tests green. Full non-e2e suite green. `upload_driver` accepts a `caller` form field and returns OOB-enabled response only when caller=="printer_form". New partial file exists.</done>
</task>
<task type="auto" tdd="true">
<name>Task 3: Wire inline driver upload form into printer_form.html</name>
<files>imptune/templates/partials/printer_form.html, tests/test_printer_form.py</files>
<behavior>
- printer_form.html renders a separate inline `<form hx-post="/drivers/upload">` OUTSIDE the main printer `<form>` but inside the Alpine x-data wrapper div.
- The driver `<select>` has `id="printer-form-driver-select"` (required OOB target).
- The inline upload form posts `caller=printer_form` as a hidden field and `file` as the upload.
- `pytest tests/test_printer_form.py` passes (add one assertion: rendered HTML contains `id="printer-form-driver-select"` and contains `name="caller" value="printer_form"` and an inline `hx-post="/drivers/upload"` NOT nested inside `<form hx-post="/printers"`).
</behavior>
<action>
Step 1 — Edit `imptune/templates/partials/printer_form.html`:
- Add `id="printer-form-driver-select"` attribute to the existing `<select name="driver_id">` (line 30).
- AFTER the closing `</form>` of the printer form (line 85) but BEFORE the closing `</div>` of the x-data wrapper (line 86), add a separate inline upload form:
```html
<form hx-post="/drivers/upload"
hx-target="#driver-list"
hx-encoding="multipart/form-data"
hx-swap="outerHTML">
<input type="hidden" name="caller" value="printer_form">
<label>
Upload New Driver
<input type="file" name="file" accept=".zip" required>
</label>
<button type="submit" class="secondary">Upload Driver</button>
</form>
```
- CRITICAL: Do NOT nest this form inside the printer `<form>` — HTML forbids nested forms and browsers silently drop the inner one. Place it as a sibling, still within the outer `<div x-data="...">` so visual grouping and Alpine scope are preserved.
- Also ensure `partials/driver_list.html` (or wherever the `#driver-list` anchor lives) is reachable from the page that renders printer_form.html. If the printer form page doesn't currently include a `<div id="driver-list">` anchor, add a hidden one next to the upload form: `<div id="driver-list" style="display:none"></div>` so the primary HTMX swap target exists even on the printer form page. Alternatively render the full driver_list partial for visibility (preferred if space allows — shows technician the uploaded driver landed).
Step 2 — Extend `tests/test_printer_form.py` with `test_printer_form_has_inline_driver_upload`:
- GET the printer form route (`/printers/new` or the HTMX partial route used by the existing tests — match the existing pattern in this test file).
- Assert response body contains `id="printer-form-driver-select"`.
- Assert response body contains `name="caller"` with value `printer_form`.
- Assert response body contains `hx-post="/drivers/upload"`.
- Assert nested form check: the substring between `<form hx-post="/printers"` and its matching `</form>` does NOT contain `hx-post="/drivers/upload"` (naive check is fine: split on `</form>` and verify the printer form chunk is clean).
Step 3 — Run the test. GREEN.
Commit: `feat(09-01): add inline driver upload to printer form with OOB refresh`
</action>
<verify>
<automated>pytest tests/test_printer_form.py -x -v && pytest tests/ -x -q --ignore=tests/e2e</automated>
</verify>
<done>Printer form template contains stable-id driver select + sibling inline upload form with caller sentinel. test_printer_form.py guards the wiring. Full non-e2e suite green.</done>
</task>
</tasks>
<verification>
- `pytest tests/ -x -q --ignore=tests/e2e` passes
- Manual eye check (captured in 09-VALIDATION.md manual section): start app, open printer form, upload a real driver ZIP, confirm driver list refreshes AND the new driver becomes the selected option in the dropdown without a page reload
</verification>
<success_criteria>
- UX-01 observable truth #1 achieved: technician uploading a driver on the printer form sees new DriverDesc in the dropdown and auto-selected, no reload
- No HTTP 500 from `POST /drivers/upload` for the captured repro case
- All 4 new tests (500 regression, OOB contract, auto-select, no-oob-on-standalone) green
</success_criteria>
<output>
After completion, create `.planning/phases/09-ux-tech-debt-closure/09-01-SUMMARY.md` documenting: actual root cause of the 500, files changed, test results, and link to commits.
</output>