refactor: drawer delete-impact modal extracted to Jinja partial (T110.3)
The modal HTML was assembled via raw f-string concatenation in ``delete_preview``. Move it to a dedicated Jinja2 partial (``chat/templates/_delete_impact_modal.html``) and render via ``TEMPLATES.TemplateResponse``. Jinja2 autoescape now handles HTML safety automatically — the explicit ``html.escape()`` calls added in T110.2 (and the ``import html``) become redundant and are removed in this commit. Net behavioural change: attribute quoting style flips from single to double quotes (Jinja default) — the existing T98.4 substring-based assertions are unaffected, and the new T110.3 test pins the double-quoted shape so future regressions surface. Test: tests/test_drawer_phase4.py::test_delete_impact_modal_uses_jinja_partial.
This commit is contained in:
@@ -0,0 +1,34 @@
|
||||
{# T110.3: delete-impact modal partial.
|
||||
|
||||
Rendered from :func:`chat.web.drawer.delete_preview` via a Jinja2
|
||||
TemplateResponse so HTML autoescape covers user-controllable fields
|
||||
(item.kind, item.description, notes) automatically — the prior
|
||||
f-string assembly required explicit html.escape() calls (T110.2)
|
||||
which become redundant under autoescape.
|
||||
|
||||
Inputs:
|
||||
``chat_id`` — the URL chat id (used to build the confirm form action).
|
||||
``impact`` — an :class:`~chat.services.delete_impact.ImpactReport`.
|
||||
#}
|
||||
<div class="delete-impact-modal">
|
||||
<h3>Delete event {{ impact.target_event_id }}?</h3>
|
||||
<p>This will discard {{ impact.cascading|length }} events. Cascade:</p>
|
||||
<ul class="delete-impact-cascade">
|
||||
{% if impact.cascading %}
|
||||
{% for item in impact.cascading %}
|
||||
<li><strong>{{ item.kind }}</strong>: {{ item.description }}</li>
|
||||
{% endfor %}
|
||||
{% else %}
|
||||
<li>none</li>
|
||||
{% endif %}
|
||||
</ul>
|
||||
<ul class="delete-impact-notes">
|
||||
{% for note in impact.notes %}
|
||||
<li>{{ note }}</li>
|
||||
{% endfor %}
|
||||
</ul>
|
||||
<form hx-post="/chats/{{ chat_id }}/drawer/turn/delete/{{ impact.target_event_id }}"
|
||||
hx-target="#drawer" hx-swap="innerHTML">
|
||||
<button type="submit">Confirm delete</button>
|
||||
</form>
|
||||
</div>
|
||||
+11
-33
@@ -27,7 +27,6 @@ one so a later inverse edit can restore state (§6.4 final paragraph).
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import html
|
||||
import json
|
||||
import uuid
|
||||
from pathlib import Path
|
||||
@@ -1235,39 +1234,18 @@ async def delete_preview(
|
||||
|
||||
report = compute_delete_impact(conn, target_event_id=int(event_id))
|
||||
|
||||
# Build the modal HTML directly — the impact report is small and
|
||||
# reusing the drawer template would require a fragment include just
|
||||
# for this surface. Mirrors the rewind-preview style in
|
||||
# :func:`chat.web.turns.rewind_preview`.
|
||||
#
|
||||
# T110.2: ``item.kind``, ``item.description``, and the notes carry
|
||||
# user-controllable content (turn prose, scene timestamps, etc.).
|
||||
# Wrap them with :func:`html.escape` so a payload like
|
||||
# ``<script>alert(1)</script>`` renders as inert text. ``chat_id``
|
||||
# is matched against the projected ``chats`` table at request time
|
||||
# (404 above) so it isn't free-form, but we escape it for symmetry.
|
||||
items_html = "".join(
|
||||
f"<li><strong>{html.escape(item.kind)}</strong>: "
|
||||
f"{html.escape(item.description)}</li>"
|
||||
for item in report.cascading
|
||||
# T110.3: render via the ``_delete_impact_modal.html`` Jinja partial
|
||||
# so HTML autoescape covers user-controllable fields (item.kind,
|
||||
# item.description, notes) automatically. The prior implementation
|
||||
# built the modal HTML via raw f-string concatenation and required
|
||||
# explicit ``html.escape()`` calls (T110.2) on each interpolated
|
||||
# field; under autoescape those calls become redundant. Mirrors the
|
||||
# rewind-preview style in :func:`chat.web.turns.rewind_preview`.
|
||||
return TEMPLATES.TemplateResponse(
|
||||
request,
|
||||
"_delete_impact_modal.html",
|
||||
{"chat_id": chat_id, "impact": report},
|
||||
)
|
||||
notes_html = "".join(
|
||||
f"<li>{html.escape(note)}</li>" for note in report.notes
|
||||
)
|
||||
body = (
|
||||
"<div class='delete-impact-modal'>"
|
||||
f"<h3>Delete event {report.target_event_id}?</h3>"
|
||||
f"<p>This will discard {len(report.cascading)} events. Cascade:</p>"
|
||||
f"<ul class='delete-impact-cascade'>{items_html or '<li>none</li>'}</ul>"
|
||||
f"<ul class='delete-impact-notes'>{notes_html}</ul>"
|
||||
f"<form hx-post='/chats/{html.escape(chat_id)}/drawer/turn/delete/"
|
||||
f"{report.target_event_id}' "
|
||||
"hx-target='#drawer' hx-swap='innerHTML'>"
|
||||
"<button type='submit'>Confirm delete</button>"
|
||||
"</form>"
|
||||
"</div>"
|
||||
)
|
||||
return HTMLResponse(body)
|
||||
|
||||
|
||||
@router.post(
|
||||
|
||||
@@ -458,6 +458,35 @@ def test_t98_4_delete_invokes_rewind_and_drops_cascade(client, tmp_path):
|
||||
assert row is None, f"event {ev_id} should have been deleted"
|
||||
|
||||
|
||||
def test_delete_impact_modal_uses_jinja_partial(client, tmp_path):
|
||||
"""T110.3: the modal HTML is rendered from a Jinja partial
|
||||
(`_delete_impact_modal.html`) rather than f-string concatenation in
|
||||
Python. Verify the partial-rendered shape: the wrapping
|
||||
``delete-impact-modal`` div, the cascade list, and the confirm form.
|
||||
|
||||
The partial inherits Jinja2 autoescape so HTML safety follows
|
||||
automatically — the explicit ``html.escape()`` calls from T110.2
|
||||
become redundant once this lands.
|
||||
"""
|
||||
db = tmp_path / "test.db"
|
||||
_seed_chat(db)
|
||||
user_id, _bot_id = _seed_turns(db)
|
||||
|
||||
response = client.get(
|
||||
f"/chats/chat_bot_a/drawer/turn/delete-preview/{user_id}"
|
||||
)
|
||||
assert response.status_code == 200
|
||||
body = response.text
|
||||
|
||||
# Markup shape that the partial produces. Double-quoted attributes
|
||||
# signal Jinja rendering (the prior f-string used single quotes).
|
||||
assert '<div class="delete-impact-modal">' in body
|
||||
assert '<ul class="delete-impact-cascade">' in body
|
||||
# The confirm form still posts to the same delete route.
|
||||
assert f"/chats/chat_bot_a/drawer/turn/delete/{user_id}" in body
|
||||
assert "Confirm delete" in body
|
||||
|
||||
|
||||
def test_delete_impact_modal_escapes_user_controllable_strings(client, tmp_path):
|
||||
"""T110.2: defense-in-depth — fields embedded in the modal HTML come
|
||||
from event payloads (turn prose, scene timestamps, etc.) which are
|
||||
|
||||
Reference in New Issue
Block a user