263 lines
16 KiB
Markdown
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>
|