docs(15-01): complete ux-driver-upload-feedback-fix plan

- SUMMARY.md created: one-line fix, two new tests, full suite green - STATE.md updated: Phase 15 complete, decisions logged - ROADMAP.md: Phase 15 marked 1/1 Complete 2026-04-16 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-16 16:35:16 +02:00
parent 409d43bd75
commit b8f10107d2
7 changed files with 722 additions and 17 deletions
@@ -0,0 +1,292 @@
+# Phase 15: UX Driver Upload Feedback Fix - Research
+
+**Researched:** 2026-04-16
+**Domain:** HTMX OOB swap, Jinja2 templates, driver upload flow
+**Confidence:** HIGH
+
+## Summary
+
+The integration gap is fully diagnosed by reading the actual source files. No speculative work is needed.
+
+Phase 9 (plan 09-01) wired the driver upload OOB flow correctly: `POST /drivers/upload?caller=printer_form` returns `driver_upload_with_oob.html`, which contains two swap targets: (1) the primary `#driver-list` `outerHTML` swap, and (2) an OOB `<select id="printer-form-driver-select">` that refreshes the driver dropdown. The OOB select refresh works as intended.
+
+The gap: Phase 11 (plan 11-01) created `/printers/new` using `printers_new.html` — a new template with the form markup inlined (not using `printer_form.html`). That template includes the HTMX upload sub-form (`hx-post="/drivers/upload"`, `hx-target="#driver-list"`) and a hidden `<div id="driver-list" style="display:none">`. When the upload returns the `driver_upload_with_oob.html` response, HTMX swaps the returned `<div id="driver-list">` (the driver list table) into the hidden target element. Because the target div has `style="display:none"`, the upload confirmation (driver name, table row, unused files notice) is invisible. The OOB select refresh still works because HTMX processes OOB elements independently of the primary swap.
+
+**Primary recommendation:** Remove `style="display:none"` from the `<div id="driver-list">` in `printers_new.html` so the primary swap content (upload confirmation) becomes visible. Optionally add a heading/label above it for clarity. Add a smoke test asserting visible confirmation content appears after upload.
+
+<phase_requirements>
+## Phase Requirements
+
+| ID | Description | Research Support |
+|----|-------------|-----------------|
+| UX-01 | After a new driver is uploaded on the printer form, the DriverDesc dropdown refreshes automatically (no manual page reload) — verified live in browser | OOB select refresh already works; fix is making the upload confirmation visible by un-hiding `#driver-list` on `/printers/new` |
+</phase_requirements>
+
+## Standard Stack
+
+### Core
+| Library | Version | Purpose | Why Standard |
+|---------|---------|---------|--------------|
+| HTMX | 1.x (CDN, project-standard) | OOB swap, partial HTML responses | Already in use; driver upload flow depends on it |
+| Jinja2 | project-standard (FastAPI) | Template rendering | Entire template layer uses Jinja2 |
+| pytest + httpx TestClient | project-standard | Integration tests | All existing tests use this pattern |
+
+No new dependencies are needed. This is a one-line template fix plus a test.
+
+**Installation:** No installation needed.
+
+## Architecture Patterns
+
+### How the Current OOB Upload Flow Works (Phase 9 implementation)
+
+```
+Browser                         Server
+  |                               |
+  |-- POST /drivers/upload ------->|
+  |   (multipart: file + caller=printer_form)
+  |                               |
+  |<-- 200 driver_upload_with_oob.html --|
+  |   Body contains TWO elements:
+  |   1. <div id="driver-list">...</div>   <- primary swap (hx-target="#driver-list", hx-swap="outerHTML")
+  |   2. <select id="printer-form-driver-select" hx-swap-oob="true">...</select>
+  |
+  HTMX processes response:
+  - Primary: replaces <div id="driver-list"> in DOM with the returned div
+  - OOB:     replaces <select id="printer-form-driver-select"> in DOM with the returned select
+```
+
+The primary swap target in `printers_new.html`:
+```html
+<!-- CURRENT (broken): hidden div — swap happens but content invisible -->
+<div id="driver-list" style="display:none"></div>
+
+<!-- FIX: remove display:none so swapped-in driver list table is visible -->
+<div id="driver-list"></div>
+```
+
+### Recommended Project Structure (unchanged)
+
+```
+imptune/templates/
+├── printers_new.html           # MODIFY: remove style="display:none" from #driver-list
+├── partials/
+│   ├── driver_list.html        # unchanged
+│   └── driver_upload_with_oob.html  # unchanged
+tests/
+├── test_printer_form.py        # ADD: smoke test for visible upload confirmation
+```
+
+### Pattern 1: HTMX OOB Swap (established in Phase 9)
+**What:** Server response body includes a primary fragment AND sibling elements with `hx-swap-oob="true"`. HTMX applies the primary swap to `hx-target`, then independently finds each OOB element by id and swaps it.
+**When to use:** When a single action must update multiple DOM regions.
+**Example:**
+```html
+<!-- Source: driver_upload_with_oob.html -->
+{% include "partials/driver_list.html" %}
+
+<select name="driver_id" id="printer-form-driver-select" hx-swap-oob="true">
+  ...
+</select>
+```
+The `driver_list.html` partial renders `<div id="driver-list">` — this is the primary swap content. It is returned as the response body. The `<select>` with `hx-swap-oob="true"` is the secondary update.
+
+### Anti-Patterns to Avoid
+- **Re-architecting the OOB flow:** The upload handler and `driver_upload_with_oob.html` work correctly. Do not change them.
+- **Using `style="display:none"` on HTMX swap targets:** HTMX outerHTML swap replaces the element including its style. The replacement element (`<div id="driver-list">` from `driver_list.html`) has no display:none. But the initial state of the div before upload is invisible, so users see nothing until the swap occurs — and once swapped in, content IS visible. The real issue is UX: users see no feedback that upload was received at all — the driver table appears out of thin air in a previously empty, invisible area. Making the `#driver-list` div visible (even empty) also makes the post-upload table appear in an expected location.
+- **Nesting upload form inside printer form:** Already handled in Phase 9. The upload form is a sibling. Do not change this.
+
+## Don't Hand-Roll
+
+| Problem | Don't Build | Use Instead | Why |
+|---------|-------------|-------------|-----|
+| Multi-region DOM update | Custom JS event dispatch | HTMX OOB swap | Already implemented; proven in Phase 9 |
+| Test HTTP responses | Manual request construction | httpx TestClient (conftest `client` fixture) | Standard pattern across all test files |
+
+## Common Pitfalls
+
+### Pitfall 1: Modifying the Wrong Template
+**What goes wrong:** Editing `printer_form.html` instead of `printers_new.html`.
+**Why it happens:** Phase 9 added the `#driver-list` hidden anchor to `printer_form.html`. Phase 11 created `printers_new.html` with its own inline copy of the form (not using `{% include "partials/printer_form.html" %}`). The two templates now diverge — `printers_new.html` has its own inline `<div id="driver-list" style="display:none"></div>` at line 98.
+**How to avoid:** The target file is `imptune/templates/printers_new.html`, line 98. `printer_form.html` still has its own hidden anchor for contexts where it is used as a partial (though it is no longer used on /printers/new).
+**Warning signs:** If the fix is in `printer_form.html` but the test checks `/printers/new`, the test will still fail.
+
+### Pitfall 2: Removing the #driver-list Anchor Entirely
+**What goes wrong:** Deleting `<div id="driver-list">` from `printers_new.html` causes HTMX to silently fail the primary swap (no target found in DOM).
+**Why it happens:** HTMX `hx-target="#driver-list"` requires the element to exist. If the anchor is absent, the outerHTML swap finds nothing and discards the response.
+**How to avoid:** Keep `<div id="driver-list"></div>` but remove `style="display:none"`.
+**Warning signs:** After upload, the OOB select refreshes but the driver list table never appears.
+
+### Pitfall 3: UX Without Label/Context
+**What goes wrong:** The driver list table appears without heading, making it confusing why a table of all uploaded drivers suddenly appears.
+**Why it happens:** `driver_list.html` just renders a `<div id="driver-list">` with a table — no surrounding context label.
+**How to avoid:** Add a heading or label above the `#driver-list` div in `printers_new.html` (e.g. "Available Drivers" or a localized key). This makes the feedback contextually clear.
+**Warning signs:** Technicians don't know what the table means after upload.
+
+### Pitfall 4: Test Asserts Wrong Route
+**What goes wrong:** Smoke test hits `/drivers/upload` without `caller=printer_form` and checks standalone behavior instead of the printer form integration.
+**Why it happens:** Confusion between the two upload paths.
+**How to avoid:** Test must POST to `/drivers/upload` with `data={"caller": "printer_form"}` and assert visible confirmation in the response fragment that gets swapped into `#driver-list`.
+
+## Code Examples
+
+### Current State of printers_new.html (lines 87-99)
+```html
+<!-- Source: imptune/templates/printers_new.html, lines 87-99 -->
+  <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>
+      <span x-text="$store.i18n.t('upload_new_driver')">Upload New Driver</span>
+      <input type="file" name="file" accept=".zip" required>
+    </label>
+    <button type="submit" class="secondary" x-text="$store.i18n.t('upload_driver')">Upload Driver</button>
+  </form>
+  <div id="driver-list" style="display:none"></div>   <!-- BUG: hidden -->
+</div>
+```
+
+### Fix (single-line change)
+```html
+  <div id="driver-list"></div>   <!-- FIX: visible, HTMX will outerHTML-swap driver list here -->
+```
+
+### Upload Handler (unchanged — already correct)
+```python
+# Source: imptune/api/drivers.py, lines 113-122
+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,
+        },
+    )
+```
+
+### OOB Template (unchanged — already correct)
+```html
+<!-- Source: imptune/templates/partials/driver_upload_with_oob.html -->
+{% 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>
+```
+
+### Test Pattern (existing style from test_printer_form.py and test_driver_upload.py)
+```python
+# Pattern: POST upload with caller, check HTML for confirmation content
+def test_upload_feedback_visible_on_printers_new(client: TestClient) -> None:
+    """After upload on /printers/new flow, driver name appears in the OOB response fragment."""
+    zip_bytes = _make_driver_zip_with_cat()
+    resp = client.post(
+        "/drivers/upload",
+        files={"file": ("driver.zip", zip_bytes, "application/zip")},
+        data={"caller": "printer_form"},
+    )
+    assert resp.status_code == 200
+    # The primary #driver-list fragment must contain the driver name (upload confirmation)
+    assert "Test LaserJet Pro" in resp.text
+    # OOB select still refreshes
+    assert 'hx-swap-oob="true"' in resp.text
+```
+
+Note: this test verifies server-side response content (what HTMX receives). The visibility fix (removing `style="display:none"`) is a template change verified by inspecting the rendered `/printers/new` HTML.
+
+```python
+def test_printers_new_driver_list_visible(client: TestClient) -> None:
+    """GET /printers/new: #driver-list anchor must NOT have display:none."""
+    resp = client.get("/printers/new")
+    assert resp.status_code == 200
+    assert 'id="driver-list"' in resp.text
+    # The anchor must exist but must NOT be hidden
+    assert 'id="driver-list" style="display:none"' not in resp.text
+```
+
+## State of the Art
+
+| Old Approach | Current Approach | When Changed | Impact |
+|--------------|------------------|--------------|--------|
+| Inline form on /printers | Dedicated /printers/new page | Phase 11 | printers_new.html is the only template to fix |
+| printer_form.html partial on /printers | Inlined form markup in printers_new.html | Phase 11 | Two diverged templates; fix goes in printers_new.html only |
+| No OOB upload flow | caller=printer_form sentinel + driver_upload_with_oob.html | Phase 9 | OOB mechanism works; visibility is the only gap |
+
+**Deprecated/outdated:**
+- The Phase 9 plan notes about adding a hidden anchor: correct at the time, but the hidden anchor is now the bug in the Phase 11 context.
+
+## Open Questions
+
+1. **Should `printer_form.html` also get its hidden anchor removed?**
+   - What we know: `printer_form.html` is no longer included in `/printers/new` (Phase 11 inlined the markup). It may still be used in other contexts (e.g. edit modal).
+   - What's unclear: whether any route still renders `printer_form.html` as a standalone partial.
+   - Recommendation: Check if `printer_form.html` is referenced anywhere besides the edit modal. Scope of Phase 15 is `printers_new.html` only — do not change `printer_form.html` unless the edit modal context also needs visible feedback.
+
+2. **i18n key for "Available Drivers" heading**
+   - What we know: Phase 12 added full i18n coverage; all strings use Alpine i18n store.
+   - What's unclear: whether an i18n key for a "Drivers uploaded" or "Available Drivers" label already exists.
+   - Recommendation: Check `imptune/static/i18n/` for existing keys before adding a new one. If a label is out of scope, omit it and just un-hide the div — the driver list table is self-explanatory.
+
+## Validation Architecture
+
+### Test Framework
+| Property | Value |
+|----------|-------|
+| Framework | pytest (project standard) |
+| Config file | `pyproject.toml` or `pytest.ini` (inferred from project) |
+| Quick run command | `pytest tests/test_printer_form.py tests/test_driver_upload.py -x -q` |
+| Full suite command | `pytest tests/ -x -q --ignore=tests/e2e` |
+
+### Phase Requirements -> Test Map
+| Req ID | Behavior | Test Type | Automated Command | File Exists? |
+|--------|----------|-----------|-------------------|-------------|
+| UX-01 | Upload confirmation visible after driver upload on /printers/new | integration | `pytest tests/test_printer_form.py -x -q` | Partial — new test needed |
+| UX-01 | #driver-list anchor not hidden on GET /printers/new | integration | `pytest tests/test_printer_form.py -x -q` | Partial — new assertion needed |
+| UX-01 | OOB select still refreshes (no regression) | integration | `pytest tests/test_driver_upload.py::test_upload_returns_oob_when_called_from_form -x` | YES (test_driver_upload.py) |
+
+### Sampling Rate
+- **Per task commit:** `pytest tests/test_printer_form.py tests/test_driver_upload.py -x -q`
+- **Per wave merge:** `pytest tests/ -x -q --ignore=tests/e2e`
+- **Phase gate:** Full suite green before marking phase complete
+
+### Wave 0 Gaps
+- [ ] `tests/test_printer_form.py` — add `test_printers_new_driver_list_visible` (asserts no `style="display:none"` on `#driver-list`)
+- [ ] `tests/test_printer_form.py` — add `test_upload_feedback_visible_on_printers_new` (asserts driver name appears in upload response)
+
+*(Existing test infrastructure covers everything else — only these two assertions are missing)*
+
+## Sources
+
+### Primary (HIGH confidence)
+- `imptune/templates/printers_new.html` — confirmed `<div id="driver-list" style="display:none">` at line 98
+- `imptune/templates/partials/driver_upload_with_oob.html` — confirmed correct OOB template
+- `imptune/api/drivers.py` — confirmed correct handler branching on `caller == "printer_form"`
+- `tests/test_driver_upload.py` — confirmed OOB contract tests already exist and pass
+- `tests/test_printer_form.py` — confirmed existing printer form tests
+
+### Secondary (MEDIUM confidence)
+- Phase 09-01 SUMMARY.md — decision to use hidden anchor: "Hidden `<div id="driver-list" style="display:none">` added to printer form to provide HTMX outerHTML swap target"
+- Phase 11-01 SUMMARY.md — confirmed `printers_new.html` inlines form markup (Option A), not `{% include "partials/printer_form.html" %}`
+
+### Tertiary (LOW confidence)
+- None
+
+## Metadata
+
+**Confidence breakdown:**
+- Root cause: HIGH — confirmed by reading `printers_new.html` line 98 directly
+- Fix: HIGH — single-line template change, no architectural risk
+- Test strategy: HIGH — established pattern in test_printer_form.py and test_driver_upload.py
+- Regression risk: LOW — OOB select path is unchanged; only the div visibility changes
+
+**Research date:** 2026-04-16
+**Valid until:** N/A — project-internal, stable until printers_new.html is changed