diff --git a/common/components/primitives.py b/common/components/primitives.py index 500211d..305c440 100644 --- a/common/components/primitives.py +++ b/common/components/primitives.py @@ -138,10 +138,16 @@ def _popover_html( ) popover_tooltip_class = ( - "absolute z-10 invisible inline-block text-sm text-white " - "transition-opacity duration-300 bg-white border border-purple-200 " - "rounded-lg shadow-xs opacity-0 dark:text-white dark:border-purple-600 " - "dark:bg-purple-800" + # `[&.invisible]:hidden`: while Flowbite keeps the popover hidden it + # carries the `invisible` class (visibility:hidden), which still + # occupies layout — an absolutely-positioned, Popper-transformed + # popover then expands its scroll container, producing a phantom + # scrollbar (issue #53 / #40). Removing it from layout while hidden + # fixes that; Flowbite drops `invisible` on show, restoring display. + "absolute z-10 invisible [&.invisible]:hidden inline-block text-sm " + "text-white transition-opacity duration-300 bg-white border " + "border-purple-200 rounded-lg shadow-xs opacity-0 dark:text-white " + "dark:border-purple-600 dark:bg-purple-800" ) div = Div( diff --git a/common/layout.py b/common/layout.py index 81f5c74..8a0f255 100644 --- a/common/layout.py +++ b/common/layout.py @@ -187,6 +187,25 @@ def _main_script(mastered: bool) -> str: return _MAIN_SCRIPT_A + ("true" if mastered else "false") + _MAIN_SCRIPT_B +def NavbarPlaytime( + today_played: str, last_7_played: str, *, oob: bool = False +) -> "Node": + """The navbar 'Today · Last 7 days' totals. Carries a stable id so + htmx endpoints can refresh it out-of-band after a session change.""" + from common.components import Safe + + oob_attr = ' hx-swap-oob="true"' if oob else "" + return Safe( + f'" + ) + + def Navbar( *, today_played: str, last_7_played: str, current_year: int, csrf_token: str ) -> "Node": @@ -225,10 +244,7 @@ def Navbar( -
  • - Today·Last 7 days - {today_played}·{last_7_played} -
    • + {NavbarPlaytime(today_played, last_7_played)}
  • Home
    • diff --git a/docs/superpowers/plans/2026-06-20-issue-53-session-row-fragment-rebuild.md b/docs/superpowers/plans/2026-06-20-issue-53-session-row-fragment-rebuild.md new file mode 100644 index 0000000..8b842d5 --- /dev/null +++ b/docs/superpowers/plans/2026-06-20-issue-53-session-row-fragment-rebuild.md @@ -0,0 +1,595 @@ +# Issue #53 — Rebuild session row fragment via shared builder — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make the htmx session-row fragment reuse the same row builder as the list table, and give the finish/reset actions a real in-place row swap with the navbar playtime totals kept correct via an out-of-band swap. + +**Architecture:** Extract the session row's content into one `session_row_data()` dict builder used by both `list_sessions` and a thin `session_row()` Node wrapper (`TableRow(session_row_data(...))`). The navbar's playtime `
  • ` becomes a `NavbarPlaytime` component with a stable id so endpoints can return it `hx-swap-oob`. `end_session`/`reset_session_start` return `Fragment(row, NavbarPlaytime(oob=True))`; clone keeps `HX-Refresh`. + +**Tech Stack:** Django 6, the in-house Python component system (`common/components`), HTMX, pytest / pytest-django, Playwright (e2e). + +## Global Constraints + +- Build UI with Python components from `common.components`; never raw HTML strings or Django templates. Builders return `Node`; stringify only at the `HttpResponse` boundary (Django str-encodes content). Do **not** return `SafeText`/`mark_safe` from row builders. +- Never write to `GeneratedField`s (`duration_calculated`, `duration_total`, `days_to_finish`). +- Name variables with complete words (`device_list`, `csrf_token`, `session`, not abbreviations). +- Name compound types explicitly: the row dict is a `TypedDict` (`SessionRowData`). +- Signals handle playtime recalculation — do not recompute `Game.playtime` by hand. +- Run tests with `uv run --with pytest-django pytest`. A bare `pytest` also collects `e2e/` (needs a browser); scope unit/view runs to `tests/...` paths. +- Spec: `docs/superpowers/specs/2026-06-20-issue-53-session-row-fragment-rebuild-design.md`. + +--- + +## File Structure + +- `games/views/session.py` — add `SessionRowData` (TypedDict), `session_row_data()`, `session_row()`; refactor `list_sessions` to use them; delete `_session_row_fragment()`; rewire `end_session`, `reset_session_start`, `new_session_from_existing_session`. +- `common/layout.py` — add `NavbarPlaytime()`; embed it inside `Navbar()`. +- `tests/test_session_row.py` — new: unit tests for `session_row_data` / `session_row`. +- `tests/test_navbar_playtime.py` — new: unit tests for `NavbarPlaytime`. +- `tests/test_session_endpoints.py` — new: view tests for the three rewired endpoints. +- `e2e/test_session_inplace_swap_e2e.py` — new: in-place finish swap + navbar update. + +--- + +### Task 1: Extract `session_row_data` + `session_row` (canonical row builder) + +**Files:** +- Modify: `games/views/session.py` (the `data["rows"]` comprehension at ~line 126-190, and the `list_sessions` body) +- Test: `tests/test_session_row.py` (create) + +**Interfaces:** +- Produces: + - `class SessionRowData(TypedDict)` with keys `row_id: str`, `hx_trigger: str`, `hx_get: str`, `hx_select: str`, `hx_swap: str`, `cell_data: list[Node]`. + - `session_row_data(session: Session, device_list, csrf_token: str) -> SessionRowData` — the 6-cell row dict (Name, Date, Duration, Device, Created, Actions) with `row_id="session-row-{pk}"` and the device-change self-refresh hx attrs. For a running session (`timestamp_end is None`) the Actions `ButtonGroup` includes Finish and Reset buttons wired for htmx row swap (`hx_get` to the end/reset URL, `hx_target=f"#session-row-{pk}"`, `hx_swap="outerHTML"`). + - `session_row(session: Session, device_list, csrf_token: str) -> Node` — `TableRow(session_row_data(...))`. + +- [ ] **Step 1: Write the failing test** + +```python +# tests/test_session_row.py +import pytest +from django.utils import timezone + +from games.models import Device, Game, Platform, Session +from games.views.session import session_row, session_row_data + + +@pytest.fixture +def running_session(db): + platform = Platform.objects.create(name="PC") + game = Game.objects.create(name="Celeste", platform=platform) + device = Device.objects.create(name="Desktop") + return Session.objects.create( + game=game, device=device, timestamp_start=timezone.now() + ) + + +def test_session_row_data_shape(running_session): + device_list = Device.objects.order_by("name") + data = session_row_data(running_session, device_list, "tok") + assert data["row_id"] == f"session-row-{running_session.pk}" + assert len(data["cell_data"]) == 6 + assert data["hx_select"] == f"#session-row-{running_session.pk}" + + +def test_session_row_renders_id_and_six_cells(running_session): + device_list = Device.objects.order_by("name") + html = str(session_row(running_session, device_list, "tok")) + assert f'id="session-row-{running_session.pk}"' in html + assert html.count(" SessionRowData: + """Canonical session-list row. Single source of truth shared by + list_sessions and the htmx finish/reset fragments.""" + row_selector = f"#session-row-{session.pk}" + end_url = reverse("games:list_sessions_end_session", args=[session.pk]) + reset_url = reverse( + "games:list_sessions_reset_session_start", args=[session.pk] + ) + actions = ButtonGroup( + [ + { + "href": end_url, + "hx_get": end_url, + "hx_target": row_selector, + "hx_swap": "outerHTML", + "slot": Icon("end"), + "title": "Finish session now", + "color": "green", + } + if session.timestamp_end is None + else {}, + { + "href": reset_url, + "hx_get": reset_url, + "hx_target": row_selector, + "hx_swap": "outerHTML", + "hx_confirm": "Reset this session's start time to now?", + "slot": Icon("reset"), + "title": "Reset start to now", + "color": "gray", + } + if session.timestamp_end is None + else {}, + { + "href": reverse("games:edit_session", args=[session.pk]), + "slot": Icon("edit"), + "title": "Edit", + }, + { + "href": reverse("games:delete_session", args=[session.pk]), + "slot": Icon("delete"), + "title": "Delete", + "color": "red", + }, + ] + ) + return SessionRowData( + row_id=f"session-row-{session.pk}", + hx_trigger="device-changed from:body", + hx_get="", + hx_select=row_selector, + hx_swap="outerHTML", + cell_data=[ + NameWithIcon(session=session), + f"{local_strftime(session.timestamp_start)}" + f"{f' — {local_strftime(session.timestamp_end, timeformat)}' if session.timestamp_end else ''}", + session.duration_formatted_with_mark(), + SessionDeviceSelector(session, device_list, csrf_token), + session.created_at.strftime(dateformat), + actions, + ], + ) + + +def session_row(session: Session, device_list, csrf_token: str) -> Node: + """The single-session node, rendered through the same TableRow + path the list table uses.""" + return TableRow(session_row_data(session, device_list, csrf_token)) +``` + +Add `TableRow` to the `from common.components import (...)` block (it currently imports `paginated_table_content` but not `TableRow`). + +- [ ] **Step 4: Run test to verify it passes** + +Run: `uv run --with pytest-django pytest tests/test_session_row.py -v` +Expected: PASS (3 tests). + +- [ ] **Step 5: Refactor `list_sessions` to consume the builder** + +Replace the inline `"rows": [ {...} for session in sessions]` in the `data` dict with the builder call. First compute the token once near the top of `list_sessions` (after `device_list`): add `csrf_token = get_token(request)`. Then: + +```python + "rows": [ + session_row_data(session, device_list, csrf_token) + for session in sessions + ], +``` + +Delete the now-removed inline row dict (the whole `{ "row_id": ..., ... }` comprehension body, ~line 127-189). Leave `header_action`/`columns` untouched. + +- [ ] **Step 6: Run the broader suite to confirm no regression** + +Run: `uv run --with pytest-django pytest tests/test_session_row.py tests/test_paths_return_200.py tests/test_rendered_pages.py -v` +Expected: PASS. + +- [ ] **Step 7: Commit** + +```bash +git add games/views/session.py tests/test_session_row.py +git commit -m "refactor(session): extract canonical session_row_data builder + +Co-Authored-By: Claude Opus 4.8 " +``` + +--- + +### Task 2: `NavbarPlaytime` component (OOB-swappable) + +**Files:** +- Modify: `common/layout.py` (`Navbar()` at ~line 190-231) +- Test: `tests/test_navbar_playtime.py` (create) + +**Interfaces:** +- Produces: `NavbarPlaytime(today_played: str, last_7_played: str, *, oob: bool = False) -> Node` — an `
  • `. + +- [ ] **Step 1: Write the failing test** + +```python +# tests/test_navbar_playtime.py +from common.layout import NavbarPlaytime + + +def test_navbar_playtime_has_stable_id_and_values(): + html = str(NavbarPlaytime("1 h 00 m", "7 h 00 m")) + assert 'id="navbar-playtime"' in html + assert "1 h 00 m" in html + assert "7 h 00 m" in html + assert "hx-swap-oob" not in html + + +def test_navbar_playtime_oob_flag(): + html = str(NavbarPlaytime("1 h 00 m", "7 h 00 m", oob=True)) + assert 'id="navbar-playtime"' in html + assert 'hx-swap-oob="true"' in html +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `uv run --with pytest-django pytest tests/test_navbar_playtime.py -v` +Expected: FAIL with `ImportError: cannot import name 'NavbarPlaytime'`. + +- [ ] **Step 3: Write minimal implementation** + +In `common/layout.py`, add above `Navbar()`: + +```python +def NavbarPlaytime( + today_played: str, last_7_played: str, *, oob: bool = False +) -> "Node": + """The navbar 'Today · Last 7 days' totals. Carries a stable id so + htmx endpoints can refresh it out-of-band after a session change.""" + from common.components import Safe + + oob_attr = ' hx-swap-oob="true"' if oob else "" + return Safe( + f'
    • " + ) +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `uv run --with pytest-django pytest tests/test_navbar_playtime.py -v` +Expected: PASS (2 tests). + +- [ ] **Step 5: Embed it inside `Navbar()`** + +In the `Navbar()` `Safe(f"""...""")` markup, replace the inline `
  • ` block: + +```html +
  • + Today·Last 7 days + {today_played}·{last_7_played} +
    • +``` + +with: + +```python + {NavbarPlaytime(today_played, last_7_played)} +``` + +(The surrounding string is already an f-string, so the `{NavbarPlaytime(...)}` call interpolates its rendered HTML.) + +- [ ] **Step 6: Run pages tests to confirm navbar still renders** + +Run: `uv run --with pytest-django pytest tests/test_navbar_playtime.py tests/test_rendered_pages.py tests/test_paths_return_200.py -v` +Expected: PASS. The navbar still shows the totals (now via the component). + +- [ ] **Step 7: Commit** + +```bash +git add common/layout.py tests/test_navbar_playtime.py +git commit -m "feat(layout): extract NavbarPlaytime as OOB-swappable component + +Co-Authored-By: Claude Opus 4.8 " +``` + +--- + +### Task 3: Rewire endpoints (in-place swap for end/reset, HX-Refresh for clone) + +**Files:** +- Modify: `games/views/session.py` (`_session_row_fragment` delete; `end_session`, `reset_session_start`, `new_session_from_existing_session`) +- Test: `tests/test_session_endpoints.py` (create) + +**Interfaces:** +- Consumes: `session_row` (Task 1), `NavbarPlaytime` (Task 2), `model_counts` (`games/views/general.py`). +- Produces: rewired views. `end_session`/`reset_session_start` htmx → `HttpResponse(str(Fragment(session_row(...), NavbarPlaytime(..., oob=True))))`; `new_session_from_existing_session` htmx → `HttpResponse(status=204)` with `HX-Refresh: true`. + +- [ ] **Step 1: Write the failing test** + +```python +# tests/test_session_endpoints.py +import pytest +from django.urls import reverse +from django.utils import timezone + +from games.models import Device, Game, Platform, Session + + +@pytest.fixture +def auth_client(client, django_user_model): + user = django_user_model.objects.create_user(username="u", password="p") + client.force_login(user) + return client + + +@pytest.fixture +def running_session(db): + platform = Platform.objects.create(name="PC") + game = Game.objects.create(name="Hades", platform=platform) + device = Device.objects.create(name="Deck") + return Session.objects.create( + game=game, device=device, timestamp_start=timezone.now() + ) + + +def test_end_session_htmx_returns_row_and_oob_navbar(auth_client, running_session): + url = reverse("games:list_sessions_end_session", args=[running_session.pk]) + response = auth_client.get(url, HTTP_HX_REQUEST="true") + body = response.content.decode() + assert response.status_code == 200 + assert f'id="session-row-{running_session.pk}"' in body + assert 'id="navbar-playtime"' in body + assert 'hx-swap-oob="true"' in body + running_session.refresh_from_db() + assert running_session.timestamp_end is not None + + +def test_reset_session_start_htmx_returns_row_no_refresh_header( + auth_client, running_session +): + original_start = running_session.timestamp_start + url = reverse( + "games:list_sessions_reset_session_start", args=[running_session.pk] + ) + response = auth_client.get(url, HTTP_HX_REQUEST="true") + body = response.content.decode() + assert response.status_code == 200 + assert f'id="session-row-{running_session.pk}"' in body + assert 'id="navbar-playtime"' in body + assert "HX-Refresh" not in response.headers + running_session.refresh_from_db() + assert running_session.timestamp_start > original_start + + +def test_clone_htmx_returns_hx_refresh(auth_client, running_session): + url = reverse( + "games:list_sessions_start_session_from_session", + args=[running_session.pk], + ) + before = Session.objects.count() + response = auth_client.get(url, HTTP_HX_REQUEST="true") + assert response.status_code == 204 + assert response.headers.get("HX-Refresh") == "true" + assert Session.objects.count() == before + 1 + + +def test_end_session_non_htmx_redirects(auth_client, running_session): + url = reverse("games:list_sessions_end_session", args=[running_session.pk]) + response = auth_client.get(url) + assert response.status_code == 302 + assert response.url == reverse("games:list_sessions") +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `uv run --with pytest-django pytest tests/test_session_endpoints.py -v` +Expected: FAIL — `end`/`reset` currently return the old fragment / `204+HX-Refresh`; clone returns the old fragment (200, not 204). + +- [ ] **Step 3: Delete `_session_row_fragment` and rewire the views** + +In `games/views/session.py`: + +1. Delete the entire `_session_row_fragment(session)` function (the hand-built 4-column `Tr`). +2. Add imports: at top, `from games.views.general import model_counts`. Ensure `Fragment` and `Node` are imported from `common.components` (they already are). Add `from common.layout import NavbarPlaytime` (the file already imports `render_page` from `common.layout`). +3. Add a small helper near the endpoints: + +```python +def _row_with_navbar(request: HttpRequest, session: Session) -> HttpResponse: + device_list = Device.objects.order_by("name") + counts = model_counts(request) + fragment = Fragment( + session_row(session, device_list, get_token(request)), + NavbarPlaytime( + counts["today_played"], counts["last_7_played"], oob=True + ), + ) + return HttpResponse(str(fragment)) +``` + +4. Rewrite the endpoints: + +```python +@login_required +def new_session_from_existing_session( + request: HttpRequest, session_id: int +) -> HttpResponse: + clone_session_by_id(session_id) + if request.htmx: + # Clone adds a new row whose position depends on sort + pagination, + # which a single-row swap cannot place — refresh the list instead. + response = HttpResponse(status=204) + response["HX-Refresh"] = "true" + return response + return redirect("games:list_sessions") + + +@login_required +def end_session(request: HttpRequest, session_id: int) -> HttpResponse: + session = get_object_or_404(Session, id=session_id) + session.timestamp_end = timezone.now() + session.save() + if request.htmx: + return _row_with_navbar(request, session) + return redirect("games:list_sessions") + + +@login_required +def reset_session_start(request: HttpRequest, session_id: int) -> HttpResponse: + session = get_object_or_404(Session, id=session_id) + session.timestamp_start = timezone.now() + session.save() + if request.htmx: + return _row_with_navbar(request, session) + return redirect("games:list_sessions") +``` + +Note: `clone_session_by_id` already returns the clone; we drop the unused local. Check for an import cycle when adding `from games.views.general import model_counts` at module top — if `general.py` imports from `session.py` it will cycle; in that case import `model_counts` lazily inside `_row_with_navbar` instead. + +- [ ] **Step 4: Run test to verify it passes** + +Run: `uv run --with pytest-django pytest tests/test_session_endpoints.py -v` +Expected: PASS (4 tests). + +- [ ] **Step 5: Switch the game-detail "Finish" button off the htmx path it never used** + +Confirm `games/views/game.py` `_sessions_section` still uses plain `href` for its end button (it does, and it stays full-nav per spec / #55). No change needed — just verify by reading; if it has any `hx_get` to `view_game_end_session`, leave it, since `end_session` still redirects for non-list contexts. (The game-detail button is `href`-only, so it triggers the non-htmx redirect branch.) + +- [ ] **Step 6: Run the full unit/view suite** + +Run: `uv run --with pytest-django pytest tests/ -v` +Expected: PASS (no regressions; old fragment tests, if any, are gone with the function). + +- [ ] **Step 7: Commit** + +```bash +git add games/views/session.py tests/test_session_endpoints.py +git commit -m "feat(session): in-place row swap for finish/reset with OOB navbar + +Delete stale _session_row_fragment; end_session and reset_session_start +return the canonical row plus an OOB navbar-playtime fragment. Clone keeps +HX-Refresh since it changes row count. Fixes #53. + +Co-Authored-By: Claude Opus 4.8 " +``` + +--- + +### Task 4: E2E — in-place finish swap + navbar update + +**Files:** +- Test: `e2e/test_session_inplace_swap_e2e.py` (create) + +**Interfaces:** +- Consumes: the rewired list UI (Tasks 1-3). No production code changes. + +- [ ] **Step 1: Write the test** + +Follow the existing `e2e/` patterns (`live_server`, login helper, Playwright `page`). Inspect `e2e/conftest.py` and an existing test (e.g. `e2e/test_widgets_e2e.py`) for the project's login fixture and `page.goto(live_server.url + ...)` style, and mirror them. + +```python +# e2e/test_session_inplace_swap_e2e.py +from django.urls import reverse +from django.utils import timezone + +from games.models import Device, Game, Platform, Session + + +def test_finish_session_swaps_row_in_place(live_server, page, logged_in): + platform = Platform.objects.create(name="PC") + game = Game.objects.create(name="Tunic", platform=platform) + device = Device.objects.create(name="Desktop") + session = Session.objects.create( + game=game, device=device, timestamp_start=timezone.now() + ) + + page.goto(live_server.url + reverse("games:list_sessions")) + row = page.locator(f"#session-row-{session.pk}") + row.get_by_title("Finish session now").click() + + # Row updates in place (still present, now shows an end time → em dash). + page.wait_for_selector(f"#session-row-{session.pk}") + assert "—" in page.locator(f"#session-row-{session.pk}").inner_text() + session.refresh_from_db() + assert session.timestamp_end is not None +``` + +If the repo has no shared `logged_in` fixture, replicate the login step used by the other e2e tests inline (they all authenticate the same way — copy that fixture/usage). + +- [ ] **Step 2: Build TS assets (custom elements served fresh) and run the test** + +Run: +```bash +make ts +uv run --with pytest-django --with pytest-playwright pytest e2e/test_session_inplace_swap_e2e.py -v +``` +Expected: PASS. (Requires a Chromium; `e2e/conftest.py` prefers a system Chrome, else run `uv run playwright install chromium` once.) + +- [ ] **Step 3: Commit** + +```bash +git add e2e/test_session_inplace_swap_e2e.py +git commit -m "test(e2e): in-place session-row finish swap + +Co-Authored-By: Claude Opus 4.8 " +``` + +--- + +### Task 5: Full check + cleanup + +**Files:** none (verification). + +- [ ] **Step 1: Lint + format + tests aggregate** + +Run: `make check` +Expected: PASS (ruff lint, format check, ts-check, tests). Fix any unused imports left in `session.py` — particularly `SafeText`, `mark_safe`, `date_filter`, `Span`, `Tr`, `Td` if the deleted `_session_row_fragment` was their only user. Verify with `make lint` and remove the dead imports. + +- [ ] **Step 2: Manual smoke (optional but recommended)** + +Run `make dev`, open the session list, finish a running session: the row should update in place (end time appears, duration fills) and the navbar "Today · Last 7 days" totals change, with no full-page reload. Reset start on a running session: start time jumps to now, duration resets, navbar updates. Clone ("play" button): list reloads. + +- [ ] **Step 3: Commit any cleanup** + +```bash +git add -A +git commit -m "chore(session): drop imports orphaned by fragment removal + +Co-Authored-By: Claude Opus 4.8 " +``` + +--- + +## Self-Review + +**Spec coverage:** +- Canonical builder (`session_row_data` + `session_row`, both Node) → Task 1. ✓ +- `NavbarPlaytime` OOB component → Task 2. ✓ +- end/reset in-place swap + OOB navbar; reset drops 204+HX-Refresh → Task 3. ✓ +- clone stays HX-Refresh → Task 3 (with documented reason). ✓ +- Return `Node`, stringify at HttpResponse boundary → Tasks 1/3 (`HttpResponse(str(Fragment(...)))`). ✓ +- List buttons switch to htmx row swap → Task 1 (Finish/Reset in `session_row_data`). ✓ +- Delete dead `_session_row_fragment` + old Tr → Task 3. ✓ +- game-detail out of scope (#55) → Task 3 Step 5 (verify, no change). ✓ +- Tests: unit (row, navbar), view (endpoints), e2e (in-place swap) → Tasks 1-4. ✓ + +**Placeholder scan:** No TBD/TODO; all steps carry concrete code or commands. The one judgement call (import-cycle on `model_counts`) is given an explicit fallback (lazy import). ✓ + +**Type consistency:** `session_row_data(session, device_list, csrf_token) -> SessionRowData` and `session_row(session, device_list, csrf_token) -> Node` used identically in Task 1, Task 3, and tests. `NavbarPlaytime(today_played, last_7_played, *, oob=False)` used consistently in Task 2 and Task 3. `_row_with_navbar(request, session) -> HttpResponse` used in both end/reset. ✓ diff --git a/docs/superpowers/specs/2026-06-20-issue-53-session-row-fragment-rebuild-design.md b/docs/superpowers/specs/2026-06-20-issue-53-session-row-fragment-rebuild-design.md new file mode 100644 index 0000000..e4fe424 --- /dev/null +++ b/docs/superpowers/specs/2026-06-20-issue-53-session-row-fragment-rebuild-design.md @@ -0,0 +1,223 @@ +# Design: Issue #53 — Rebuild `_session_row_fragment` via a shared row builder + +**Date:** 2026-06-20 +**Issue:** [#53](https://github.com/KucharczykL/timetracker/issues/53) +**Follow-on:** [#55](https://github.com/KucharczykL/timetracker/issues/55) (standardize all session tables on the canonical builder) + +## Problem + +`_session_row_fragment()` in `games/views/session.py` renders a **4-column** session +`` (Name, Start, End, Duration) with a hand-built `Tr`, no `id="session-row-{pk}"`. +The live `list_sessions` table is **6 columns** (Name, Date, Duration, Device, Created, +Actions) with a row id and htmx attributes. The fragment cannot be htmx-swapped into the +live table without producing a malformed, un-targetable row. + +In practice the fragment is **dead**: every session action button in the UI is a plain +`href` (full-page navigation). The only htmx caller, `reset_session_start`, returns +`204 + HX-Refresh` (the #33 workaround) rather than the fragment. The fragment's htmx +paths in `end_session` and `new_session_from_existing_session` are never exercised, which +is why the drift went unnoticed. + +Root cause: the fragment is an independent re-implementation of a session row. Fixed +properly, there must be exactly one source of truth for a session row, reused by both +the table and any htmx fragment. + +## Goal + +1. One canonical session-row builder shared by `list_sessions` and the htmx fragment — no + duplicated `` markup, so the two cannot drift. +2. Real in-place htmx row swap for **finish** and **reset-start** actions on the session + list, with the navbar playtime totals kept correct in the same request via an + out-of-band (OOB) swap. + +Non-goals (tracked in #55): migrating the game-detail sessions table (4-column, different +shape) onto the canonical builder. It keeps its current full-navigation buttons for now. + +## Architecture + +### Single source of truth for a session row + +`TableRow` (`common/components/primitives.py:894`) is the only place a `` is built. +The table reaches it through `list_sessions → row dict → paginated_table_content → +SimpleTable → TableRow(data=dict)`. The fix splits the row into two reused units: + +- **`session_row_data(session, device_list, csrf_token) -> SessionRowData`** — owns cell + content, `row_id`, and the row's htmx attributes (the dict currently inlined in + `list_sessions`). New function in `games/views/session.py`. +- **`TableRow`** — owns the `` markup. Unchanged, already shared. + +Both consumers go through the same dict builder and the same renderer: + +```python +# list_sessions +rows = [session_row_data(s, device_list, csrf_token) for s in sessions] +# → paginated_table_content → SimpleTable → TableRow(data=dict) + +# single-row htmx fragment — returns a Node, not a stringified SafeText +def session_row(session, device_list, csrf_token) -> Node: + return TableRow(session_row_data(session, device_list, csrf_token)) +``` + +The fragment is therefore the *same* row the table renders, for a single session. Change +a column once in `session_row_data` and list + fragment move together. The old hand-built +`Tr` (4-column, the `#last-session-start` toggle, the yellow "Finish now?" link) — and the +`_session_row_fragment` helper returning `SafeText` — are deleted entirely. + +**Return `Node`, not `SafeText`.** Per the component-system direction, builders return +`Node` objects and stringification happens only at the `HttpResponse` boundary (Django +str-encodes response content automatically — `HttpResponse(node)` already works across the +codebase, e.g. `purchase.py` `HttpResponse(_refund_confirmation_modal(...))`). `TableRow` +already returns an `Element` (a `Node`), so `session_row` returns it directly with no +`str()`/`mark_safe`. The endpoints combine the row and the OOB navbar with `Fragment` +(also a `Node`) and pass that straight to `HttpResponse`. + +`session_row_data` reproduces today's `list_sessions` dict exactly: + +- `row_id`: `f"session-row-{session.pk}"` +- `hx_trigger`: `"device-changed from:body"`, `hx_get`: `""`, `hx_select`: + `f"#session-row-{session.pk}"`, `hx_swap`: `"outerHTML"` (the existing self-refresh on + device change) +- `cell_data` (6): `NameWithIcon(session=session)`; start–end string via `local_strftime`; + `session.duration_formatted_with_mark()`; `SessionDeviceSelector(session, device_list, + csrf_token)`; `session.created_at.strftime(dateformat)`; the `ButtonGroup` of actions. + +The action `ButtonGroup` for a running session (`timestamp_end is None`) switches the +**Finish** and **Reset start** buttons from plain `href` to htmx (see below). `ButtonGroup` +already forwards `hx_get`/`hx_target`/`hx_swap`/`hx_confirm` (`primitives.py:367`). + +### Named type + +```python +class SessionRowData(TypedDict): + row_id: str + hx_trigger: str + hx_get: str + hx_select: str + hx_swap: str + cell_data: list[Node] +``` + +Defined in `games/views/session.py` (per the project convention to name compound types +passed between functions). + +### Navbar playtime as an OOB-swappable component + +The navbar's "Today · Last 7 days" totals live inline in the monolithic `Navbar()` +`Safe` f-string (`common/layout.py:228-231`). Finishing or resetting a session changes a +session's duration → game playtime → these totals, so an in-place row swap would leave +them stale. + +Extract the `
  • ` into a small component with a stable id: + +```python +# common/layout.py (or common/components) +def NavbarPlaytime(today_played: str, last_7_played: str, *, oob: bool = False) -> Node: + #
    • +``` + +- `Navbar()` embeds `NavbarPlaytime(today_played, last_7_played)` in place of the inline + markup (no visual change). +- htmx endpoints render `NavbarPlaytime(..., oob=True)`, which adds `hx-swap-oob="true"`, + and append it to their response body. htmx applies it to the matching `#navbar-playtime` + regardless of the primary target. + +Totals come from the existing `model_counts(request)` (`games/views/general.py:26`), which +already computes `today_played` / `last_7_played`. The endpoints call it after saving. + +### Endpoint behavior + +All three endpoints keep their non-htmx branch (`redirect("games:list_sessions")`). + +| Endpoint | htmx response | +|---|---| +| `end_session` | `HttpResponse(Fragment(session_row(...), NavbarPlaytime(..., oob=True)))` | +| `reset_session_start` | `HttpResponse(Fragment(session_row(...), NavbarPlaytime(..., oob=True)))` | +| `new_session_from_existing_session` (clone) | `204 + HX-Refresh: true` | + +- **end / reset** return a `Fragment` Node holding the fresh row plus the OOB navbar in one + response body, passed straight to `HttpResponse` (no manual stringification). The + triggering button targets `#session-row-{pk}` with `hx-swap="outerHTML"`; htmx extracts + the OOB `
  • ` and swaps the remainder (the ``) into the row. `reset_session_start` + drops its current `204 + HX-Refresh` workaround. +- **clone stays on `HX-Refresh`**: it creates a *new* session whose correct position + depends on sort + pagination, which a single-row `outerHTML` swap cannot place. Its htmx + branch returns `204 + HX-Refresh: true` (replacing the dead fragment return). This is a + deliberate, documented exception. + +Both `end_session` and `reset_session_start` need `device_list` and a CSRF token to build +the row (for the `SessionDeviceSelector` cell): `Device.objects.order_by("name")` and +`get_token(request)`, mirroring `list_sessions`. + +### List buttons → htmx + +In `session_row_data`, for a running session: + +- **Finish session now**: add `hx_get` = `list_sessions_end_session` URL, + `hx_target` = `f"#session-row-{session.pk}"`, `hx_swap` = `"outerHTML"`. Keep `href` as + a no-JS fallback. +- **Reset start to now**: same `hx_target`/`hx_swap`; keep existing `hx_confirm` and + `href` fallback. (Previously its `hx_get` hit the 204+refresh path; now it swaps the + row.) + +Edit, Delete, and the clone/"play" affordances are unchanged. + +## Components / files touched + +- `games/views/session.py` — add `SessionRowData`, `session_row_data() -> SessionRowData`, + `session_row() -> Node`; delete the old `_session_row_fragment() -> SafeText`; update + `list_sessions` to use the builder; rewire `end_session`, `reset_session_start`, + `new_session_from_existing_session`. Drop the now-unused `SafeText`/`Tr` imports if no + other references remain. +- `common/layout.py` — add `NavbarPlaytime`; use it inside `Navbar()`. +- (If `NavbarPlaytime` is placed in `common/components`, re-export via `__init__.py`.) + +## Data flow (finish from the list) + +``` +click Finish → hx-get end_session (htmx) + → session.timestamp_end = now; save() + → model_counts(request) (fresh totals) + → response body: (6 cells) + +
    • +htmx: OOB
  • → #navbar-playtime ; → #session-row-pk (outerHTML) + → row shows end time + duration; navbar totals update; no full reload + → swapped row keeps device-change self-refresh + device selector custom element +``` + +## Error handling + +- Missing session → `get_object_or_404` (unchanged). +- Non-htmx requests → full-page redirect (unchanged), so the feature degrades to the + current behavior without JS. +- `SessionDeviceSelector` custom element re-initializes on swap via its native + `connectedCallback`; its JS module is already loaded by the list page, so no extra + `scripts=` wiring is needed. + +## Testing + +Unit (`tests/`): +- `session_row_data` returns 6 `cell_data` entries and `row_id == "session-row-{pk}"`, + with the device/created/actions cells present. +- `session_row(...)` is a `Node`; `str(session_row(...))` contains `id="session-row-{pk}"` + and 6 `/` cells (regression against the 4-column drift). +- `NavbarPlaytime(oob=True)` emits `id="navbar-playtime"` and `hx-swap-oob="true"`; + `oob=False` omits the OOB attribute. + +View (`tests/`, htmx requests via `HTTP_HX_REQUEST=true`): +- `end_session` (htmx) response body contains `#session-row-{pk}` and an OOB + `#navbar-playtime`; sets `timestamp_end`. +- `reset_session_start` (htmx) likewise; sets `timestamp_start` to ~now; **no** + `HX-Refresh` header. +- `new_session_from_existing_session` (htmx) returns status 204 with `HX-Refresh: true` + and creates a session. +- Non-htmx variants of all three still redirect to the session list. + +E2E (`e2e/`): +- From the session list, finish a running session → its row updates in place (end time + + duration) and the navbar "Today · Last 7 days" totals change, with no full page reload. + +## Out of scope (→ #55) + +`games/views/game.py` `_sessions_section` (4-column game-detail table, different first +column, no Device/Created) keeps its full-navigation `href` buttons. Migrating it onto +`session_row_data` with configurable visible columns is tracked in #55. diff --git a/e2e/test_session_inplace_swap_e2e.py b/e2e/test_session_inplace_swap_e2e.py new file mode 100644 index 0000000..b5985de --- /dev/null +++ b/e2e/test_session_inplace_swap_e2e.py @@ -0,0 +1,103 @@ +"""Browser test for the session-list "Finish session now" in-place row swap (issue #53). + +Drives the real session list against pytest-django's ``live_server``: clicks the +finish button on a running session and asserts the row is updated in place via +htmx (the row still exists and now shows an end-time em dash separator). +""" + +import pytest +from django.urls import reverse +from django.utils import timezone +from playwright.sync_api import Page, expect + +from games.models import Device, Game, Platform, Session + + +@pytest.fixture +def authenticated_page(live_server, page: Page, django_user_model) -> Page: + django_user_model.objects.create_user(username="tester", password="secret123") + page.goto(f"{live_server.url}{reverse('login')}") + page.fill('input[name="username"]', "tester") + page.fill('input[name="password"]', "secret123") + page.click('button:has-text("Login")') + page.wait_for_url(f"{live_server.url}/tracker**") + return page + + +def test_finish_session_swaps_row_in_place(authenticated_page: Page, live_server): + page = authenticated_page + platform = Platform.objects.create(name="PC", icon="pc", group="PC") + game = Game.objects.create(name="Tunic", platform=platform) + device = Device.objects.create(name="Desktop") + session = Session.objects.create( + game=game, device=device, timestamp_start=timezone.now() + ) + + page.goto(f"{live_server.url}{reverse('games:list_sessions')}") + + row = page.locator(f"#session-row-{session.pk}") + expect(row).to_be_visible() + + row.locator('button[title="Finish session now"]').click() + + # htmx swaps the row in place; the row still exists and now shows an end + # time separated by an em dash. + expect(row).to_contain_text("—") + + session.refresh_from_db() + assert session.timestamp_end is not None + + +def test_finish_session_swap_does_not_add_scrollbar( + authenticated_page: Page, live_server +): + """Regression for the phantom horizontal scrollbar (issues #53 / #40). + + Flowbite re-initialises popovers on every htmx swap; a popover hidden via + Tailwind ``invisible`` (visibility:hidden) still occupies layout, so once + Popper parks it with a transform it expands the table's overflow-x-auto + wrapper and a spurious scrollbar appears. The popover must be removed from + layout while hidden. + """ + page = authenticated_page + page.set_viewport_size({"width": 1280, "height": 800}) + platform = Platform.objects.create(name="PC", icon="pc", group="PC") + # A long name guarantees a truncated NameWithIcon popover in the row. + game = Game.objects.create(name="A Very Long Game Title That Truncates") + game.platform = platform + game.save() + device = Device.objects.create(name="Desktop") + session = Session.objects.create( + game=game, device=device, timestamp_start=timezone.now() + ) + + page.goto(f"{live_server.url}{reverse('games:list_sessions')}") + + # The fix only removes the popover from layout while it is hidden; it must + # still display on hover. Verify on the freshly-loaded page. + trigger = page.locator(f"#session-row-{session.pk} [data-popover-target]").first + popover_id = trigger.get_attribute("data-popover-target") + trigger.hover() + page.wait_for_timeout(400) + shown_display = page.evaluate( + """(id) => getComputedStyle(document.querySelector(`[id="${id}"]`)).display""", + popover_id, + ) + assert shown_display != "none", "popover stayed display:none on hover" + page.mouse.move(0, 0) + + page.locator(f"#session-row-{session.pk}").locator( + 'button[title="Finish session now"]' + ).click() + expect(page.locator(f"#session-row-{session.pk}")).to_contain_text("—") + page.wait_for_timeout(500) # allow Flowbite afterSettle re-init + Popper + + # After the swap re-inits popovers, the table wrapper must not become + # horizontally scrollable (the phantom-scrollbar regression). + overflow = page.evaluate( + """() => { + const w = document.querySelector('.overflow-x-auto'); + return w.scrollWidth - w.clientWidth; + }""" + ) + assert overflow <= 0, f"table wrapper overflows by {overflow}px after swap" diff --git a/games/static/base.css b/games/static/base.css index 0c1bf77..ae4a8c3 100644 --- a/games/static/base.css +++ b/games/static/base.css @@ -1487,9 +1487,6 @@ .h-10 { height: calc(var(--spacing) * 10); } - .h-12 { - height: calc(var(--spacing) * 12); - } .h-\[calc\(100\%-1rem\)\] { height: calc(100% - 1rem); } @@ -2489,9 +2486,6 @@ .align-middle { vertical-align: middle; } - .align-top { - vertical-align: top; - } .font-mono { font-family: var(--font-mono); } @@ -2754,9 +2748,6 @@ .text-white { color: var(--color-white); } - .text-yellow-300 { - color: var(--color-yellow-300); - } .lowercase { text-transform: lowercase; } @@ -2904,106 +2895,6 @@ .ring-inset { --tw-ring-inset: inset; } - .group-hover\:absolute { - &:is(:where(.group):hover *) { - @media (hover: hover) { - position: absolute; - } - } - } - .group-hover\:-top-8 { - &:is(:where(.group):hover *) { - @media (hover: hover) { - top: calc(var(--spacing) * -8); - } - } - } - .group-hover\:-left-6 { - &:is(:where(.group):hover *) { - @media (hover: hover) { - left: calc(var(--spacing) * -6); - } - } - } - .group-hover\:max-w-none { - &:is(:where(.group):hover *) { - @media (hover: hover) { - max-width: none; - } - } - } - .group-hover\:min-w-60 { - &:is(:where(.group):hover *) { - @media (hover: hover) { - min-width: calc(var(--spacing) * 60); - } - } - } - .group-hover\:rounded-xs { - &:is(:where(.group):hover *) { - @media (hover: hover) { - border-radius: var(--radius-xs); - } - } - } - .group-hover\:bg-purple-600 { - &:is(:where(.group):hover *) { - @media (hover: hover) { - background-color: var(--color-purple-600); - } - } - } - .group-hover\:px-6 { - &:is(:where(.group):hover *) { - @media (hover: hover) { - padding-inline: calc(var(--spacing) * 6); - } - } - } - .group-hover\:py-3\.5 { - &:is(:where(.group):hover *) { - @media (hover: hover) { - padding-block: calc(var(--spacing) * 3.5); - } - } - } - .group-hover\:text-purple-100 { - &:is(:where(.group):hover *) { - @media (hover: hover) { - color: var(--color-purple-100); - } - } - } - .group-hover\:decoration-purple-900 { - &:is(:where(.group):hover *) { - @media (hover: hover) { - text-decoration-color: var(--color-purple-900); - } - } - } - .group-hover\:outline-4 { - &:is(:where(.group):hover *) { - @media (hover: hover) { - outline-style: var(--tw-outline-style); - outline-width: 4px; - } - } - } - .group-hover\:outline-purple-400 { - &:is(:where(.group):hover *) { - @media (hover: hover) { - outline-color: var(--color-purple-400); - } - } - } - .group-hover\:outline-dashed { - &:is(:where(.group):hover *) { - @media (hover: hover) { - --tw-outline-style: dashed; - outline-style: dashed; - } - } - } .group-data-\[search-select-highlighted\]\:border-white { &:is(:where(.group)[data-search-select-highlighted] *) { border-color: var(--color-white); @@ -3487,11 +3378,6 @@ outline-color: var(--color-brand-strong); } } - .sm\:table-cell { - @media (width >= 40rem) { - display: table-cell; - } - } .sm\:max-w-\(--breakpoint-sm\) { @media (width >= 40rem) { max-width: var(--breakpoint-sm); @@ -3647,11 +3533,6 @@ } } } - .lg\:table-cell { - @media (width >= 64rem) { - display: table-cell; - } - } .lg\:max-w-3xl { @media (width >= 64rem) { max-width: var(--container-3xl); @@ -4242,6 +4123,11 @@ } } } + .\[\&\.invisible\]\:hidden { + &.invisible { + display: none; + } + } .\[\&\:first-of-type_button\]\:rounded-s-lg { &:first-of-type button { border-start-start-radius: var(--radius-lg); diff --git a/games/views/session.py b/games/views/session.py index c93157f..de5bef0 100644 --- a/games/views/session.py +++ b/games/views/session.py @@ -1,14 +1,13 @@ -from typing import Any +from typing import Any, TypedDict from django.contrib.auth.decorators import login_required from django.db.models import Q from django.http import HttpRequest, HttpResponse from django.middleware.csrf import get_token from django.shortcuts import get_object_or_404, redirect -from django.template.defaultfilters import date as date_filter from django.urls import reverse from django.utils import timezone -from django.utils.safestring import SafeText, mark_safe +from django.utils.safestring import mark_safe from common.components import ( A, @@ -26,10 +25,11 @@ SessionDeviceSelector, SessionTimestampButtons, StyledButton, + TableRow, paginated_table_content, ) -from common.components.primitives import Span, Td, Tr -from common.layout import render_page +from common.layout import NavbarPlaytime, render_page +from games.views.general import model_counts from common.time import ( dateformat, local_strftime, @@ -40,6 +40,83 @@ from games.models import Device, Game, Session +class SessionRowData(TypedDict): + row_id: str + hx_trigger: str + hx_get: str + hx_select: str + hx_swap: str + cell_data: list[Node | str] + + +def session_row_data(session: Session, device_list, csrf_token: str) -> SessionRowData: + """Canonical session-list row. Single source of truth shared by + list_sessions and the htmx finish/reset fragments.""" + row_selector = f"#session-row-{session.pk}" + end_url = reverse("games:list_sessions_end_session", args=[session.pk]) + reset_url = reverse("games:list_sessions_reset_session_start", args=[session.pk]) + actions = ButtonGroup( + [ + { + "href": end_url, + "hx_get": end_url, + "hx_target": row_selector, + "hx_swap": "outerHTML", + "slot": Icon("end"), + "title": "Finish session now", + "color": "green", + } + if session.timestamp_end is None + else {}, + { + "href": reset_url, + "hx_get": reset_url, + "hx_target": row_selector, + "hx_swap": "outerHTML", + "hx_confirm": "Reset this session's start time to now?", + "slot": Icon("reset"), + "title": "Reset start to now", + "color": "gray", + } + if session.timestamp_end is None + else {}, + { + "href": reverse("games:edit_session", args=[session.pk]), + "slot": Icon("edit"), + "title": "Edit", + }, + { + "href": reverse("games:delete_session", args=[session.pk]), + "slot": Icon("delete"), + "title": "Delete", + "color": "red", + }, + ] + ) + return SessionRowData( + row_id=f"session-row-{session.pk}", + hx_trigger="device-changed from:body", + hx_get="", + hx_select=row_selector, + hx_swap="outerHTML", + cell_data=[ + NameWithIcon(session=session), + f"{local_strftime(session.timestamp_start)}" + f"{f' — {local_strftime(session.timestamp_end, timeformat)}' if session.timestamp_end else ''}", + session.duration_formatted_with_mark(), + SessionDeviceSelector(session, device_list, csrf_token), + session.created_at.strftime(dateformat), + actions, + ], + ) + + +def session_row(session: Session, device_list, csrf_token: str) -> Node: + """The single-session node, rendered through the same TableRow + path the list table uses.""" + return TableRow(session_row_data(session, device_list, csrf_token)) + + @login_required def list_sessions(request: HttpRequest, search_string: str = "") -> HttpResponse: sessions = Session.objects.order_by("-timestamp_start", "created_at") @@ -69,6 +146,7 @@ def list_sessions(request: HttpRequest, search_string: str = "") -> HttpResponse except Session.DoesNotExist: last_session = None sessions, page_obj, elided_page_range = paginate(request, sessions) + csrf_token = get_token(request) data = { "header_action": Div( @@ -120,68 +198,7 @@ def list_sessions(request: HttpRequest, search_string: str = "") -> HttpResponse "Actions", ], "rows": [ - { - "row_id": f"session-row-{session.pk}", - "hx_trigger": "device-changed from:body", - "hx_get": "", - "hx_select": f"#session-row-{session.pk}", - "hx_swap": "outerHTML", - "cell_data": [ - NameWithIcon(session=session), - f"{local_strftime(session.timestamp_start)}{f' — {local_strftime(session.timestamp_end, timeformat)}' if session.timestamp_end else ''}", - session.duration_formatted_with_mark(), - SessionDeviceSelector(session, device_list, get_token(request)), - session.created_at.strftime(dateformat), - ButtonGroup( - [ - { - "href": reverse( - "games:list_sessions_end_session", args=[session.pk] - ), - "slot": Icon("end"), - "title": "Finish session now", - "color": "green", - } - if session.timestamp_end is None - else {}, - { - "href": reverse( - "games:list_sessions_reset_session_start", - args=[session.pk], - ), - "hx_get": reverse( - "games:list_sessions_reset_session_start", - args=[session.pk], - ), - "hx_confirm": ( - "Reset this session's start time to now?" - ), - "slot": Icon("reset"), - "title": "Reset start to now", - "color": "gray", - } - if session.timestamp_end is None - else {}, - { - "href": reverse( - "games:edit_session", args=[session.pk] - ), - "slot": Icon("edit"), - "title": "Edit", - }, - { - "href": reverse( - "games:delete_session", args=[session.pk] - ), - "slot": Icon("delete"), - "title": "Delete", - "color": "red", - }, - ] - ), - ], - } - for session in sessions + session_row_data(session, device_list, csrf_token) for session in sessions ], } content = paginated_table_content( @@ -286,84 +303,14 @@ def edit_session(request: HttpRequest, session_id: int) -> HttpResponse: ) -def _session_row_fragment(session: Session) -> SafeText: - """A single session (the old list_sessions.html#session-row partial), - returned by the inline end/clone-session HTMX endpoints.""" - name_link = A( - href=reverse("games:view_game", args=[session.game.id]), - attributes=[ - ( - "class", - "underline decoration-slate-500 sm:decoration-2 inline-block " - "truncate max-w-20char group-hover:absolute group-hover:max-w-none " - "group-hover:-top-8 group-hover:-left-6 group-hover:min-w-60 " - "group-hover:px-6 group-hover:py-3.5 group-hover:bg-purple-600 " - "group-hover:rounded-xs group-hover:outline-dashed " - "group-hover:outline-purple-400 group-hover:outline-4 " - "group-hover:decoration-purple-900 group-hover:text-purple-100", - ), - ], - children=[session.game.name], - ) - name_td = Td( - attributes=[ - ( - "class", - "px-2 sm:px-4 md:px-6 md:py-2 purchase-name relative align-top " - "w-24 h-12 group", - ) - ], - children=[ - Span( - attributes=[("class", "inline-block relative")], - children=[name_link], - ) - ], - ) - start_td = Td( - attributes=[ - ("class", "px-2 sm:px-4 md:px-6 md:py-2 font-mono hidden sm:table-cell") - ], - children=[date_filter(session.timestamp_start, "d/m/Y H:i")], - ) - - if not session.timestamp_end: - end_url = reverse("games:list_sessions_end_session", args=[session.id]) - end_inner: SafeText | str = A( - href=end_url, - attributes=[ - ("hx-get", end_url), - ("hx-target", "closest tr"), - ("hx-swap", "outerHTML"), - ("hx-indicator", "#indicator"), - ( - "onClick", - "document.querySelector('#last-session-start')" - ".classList.remove('invisible')", - ), - ], - children=[ - Span( - attributes=[("class", "text-yellow-300")], - children=["Finish now?"], - ) - ], - ) - elif session.duration_manual: - end_inner = "--" - else: - end_inner = date_filter(session.timestamp_end, "d/m/Y H:i") - end_td = Td( - attributes=[ - ("class", "px-2 sm:px-4 md:px-6 md:py-2 font-mono hidden lg:table-cell") - ], - children=[end_inner], - ) - duration_td = Td( - attributes=[("class", "px-2 sm:px-4 md:px-6 md:py-2 font-mono")], - children=[session.duration_formatted()], +def _row_with_navbar(request: HttpRequest, session: Session) -> HttpResponse: + device_list = Device.objects.order_by("name") + counts = model_counts(request) + fragment = Fragment( + session_row(session, device_list, get_token(request)), + NavbarPlaytime(counts["today_played"], counts["last_7_played"], oob=True), ) - return Tr(children=[name_td, start_td, end_td, duration_td]) + return HttpResponse(str(fragment)) def clone_session_by_id(session_id: int) -> Session: @@ -381,9 +328,13 @@ def clone_session_by_id(session_id: int) -> Session: def new_session_from_existing_session( request: HttpRequest, session_id: int ) -> HttpResponse: - session = clone_session_by_id(session_id) + clone_session_by_id(session_id) if request.htmx: - return HttpResponse(_session_row_fragment(session)) + # Clone adds a new row whose position depends on sort + pagination, + # which a single-row swap cannot place — refresh the list instead. + response = HttpResponse(status=204) + response["HX-Refresh"] = "true" + return response return redirect("games:list_sessions") @@ -393,7 +344,7 @@ def end_session(request: HttpRequest, session_id: int) -> HttpResponse: session.timestamp_end = timezone.now() session.save() if request.htmx: - return HttpResponse(_session_row_fragment(session)) + return _row_with_navbar(request, session) return redirect("games:list_sessions") @@ -403,11 +354,7 @@ def reset_session_start(request: HttpRequest, session_id: int) -> HttpResponse: session.timestamp_start = timezone.now() session.save() if request.htmx: - # The list table is rebuilt server-side per request; a full refresh - # avoids swapping in a row fragment whose layout could drift from it. - response = HttpResponse(status=204) - response["HX-Refresh"] = "true" - return response + return _row_with_navbar(request, session) return redirect("games:list_sessions") diff --git a/tests/test_navbar_playtime.py b/tests/test_navbar_playtime.py new file mode 100644 index 0000000..6c26efd --- /dev/null +++ b/tests/test_navbar_playtime.py @@ -0,0 +1,15 @@ +from common.layout import NavbarPlaytime + + +def test_navbar_playtime_has_stable_id_and_values(): + html = str(NavbarPlaytime("1 h 00 m", "7 h 00 m")) + assert 'id="navbar-playtime"' in html + assert "1 h 00 m" in html + assert "7 h 00 m" in html + assert "hx-swap-oob" not in html + + +def test_navbar_playtime_oob_flag(): + html = str(NavbarPlaytime("1 h 00 m", "7 h 00 m", oob=True)) + assert 'id="navbar-playtime"' in html + assert 'hx-swap-oob="true"' in html diff --git a/tests/test_rendered_pages.py b/tests/test_rendered_pages.py index 40c101d..c8dec68 100644 --- a/tests/test_rendered_pages.py +++ b/tests/test_rendered_pages.py @@ -130,7 +130,7 @@ def test_add_game_form(self): self.assertIn("submit_and_redirect", html) self.assertIn("Submit & Create Purchase", html) # & correctly escaped self.assertIn("submit_and_create_session", html) - self.assertIn("Submit & Create Session", html) # & correctly escaped + self.assertIn("Submit & Create Session", html) # & correctly escaped # Fields self-style: label + control carry their own classes (no #add-form # / form CSS in input.css). self.assertIn("mb-2.5 text-sm font-medium text-heading", html) # _LABEL_CLASS @@ -255,19 +255,23 @@ def test_refund_confirmation_modal(self): self.assertNoEscapedTags(html) def test_session_row_fragment_via_htmx(self): - # The inline "finish session" endpoint returns a fragment. + # The inline "finish session" endpoint returns an in-place row swap + # () plus an OOB navbar-playtime update. resp = self.client.get( reverse("games:list_sessions_end_session", args=[self.session.id]), HTTP_HX_REQUEST="true", ) html = resp.content.decode() self.assertTrue(html.lstrip().startswith(" original_start + + +def test_clone_htmx_returns_hx_refresh(auth_client, running_session): + url = reverse( + "games:list_sessions_start_session_from_session", + args=[running_session.pk], + ) + before = Session.objects.count() + response = auth_client.get(url, HTTP_HX_REQUEST="true") + assert response.status_code == 204 + assert response.headers.get("HX-Refresh") == "true" + assert Session.objects.count() == before + 1 + + +def test_end_session_non_htmx_redirects(auth_client, running_session): + url = reverse("games:list_sessions_end_session", args=[running_session.pk]) + response = auth_client.get(url) + assert response.status_code == 302 + assert response.url == reverse("games:list_sessions") diff --git a/tests/test_session_row.py b/tests/test_session_row.py new file mode 100644 index 0000000..bb69b3a --- /dev/null +++ b/tests/test_session_row.py @@ -0,0 +1,37 @@ +import pytest +from django.utils import timezone + +from games.models import Device, Game, Platform, Session +from games.views.session import session_row, session_row_data + + +@pytest.fixture +def running_session(db): + platform = Platform.objects.create(name="PC") + game = Game.objects.create(name="Celeste", platform=platform) + device = Device.objects.create(name="Desktop") + return Session.objects.create( + game=game, device=device, timestamp_start=timezone.now() + ) + + +def test_session_row_data_shape(running_session): + device_list = Device.objects.order_by("name") + data = session_row_data(running_session, device_list, "tok") + assert data["row_id"] == f"session-row-{running_session.pk}" + assert len(data["cell_data"]) == 6 + assert data["hx_select"] == f"#session-row-{running_session.pk}" + + +def test_session_row_renders_id_and_six_cells(running_session): + device_list = Device.objects.order_by("name") + html = str(session_row(running_session, device_list, "tok")) + assert f'id="session-row-{running_session.pk}"' in html + assert html.count("