Feedback reply: light Markdown formatting (bold / bullets / heading)
Per Codex: a constrained Markdown-ish composer rather than contenteditable. * goodnews/markup.render_reply_html — escapes everything first, then introduces only a tiny whitelist (**bold**, - bullets, #/##/### headings, paragraphs, line breaks). No links, attributes, inline styles, or raw HTML passthrough. * feedback_replies.message_html column (+ live migration); replies store both the Markdown text and the rendered HTML. * email_send.send_feedback_reply now sends multipart text/plain + text/html (the sanitized render, wrapped in a trusted email template). * Frontend: textarea + a small toolbar (Bold / • List / H) that inserts Markdown; the reply thread renders the server-sanitized HTML. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -136,13 +136,41 @@
|
||||
}
|
||||
}
|
||||
|
||||
// In-site reply (plain text, sent via SMTP; only stored on successful send).
|
||||
// In-site reply (Markdown-ish: textarea + toolbar; server renders sanitized HTML).
|
||||
let replyingId = $state(null);
|
||||
let replyText = $state('');
|
||||
let replyBusy = $state(false);
|
||||
let replyErr = $state('');
|
||||
let replyTextarea = $state(null);
|
||||
function openReply(f) { replyingId = f.id; replyText = ''; replyErr = ''; }
|
||||
function cancelReply() { replyingId = null; replyText = ''; replyErr = ''; }
|
||||
|
||||
// Wrap the current selection (e.g. **bold**), restoring focus + selection.
|
||||
function mdWrap(token) {
|
||||
const el = replyTextarea;
|
||||
if (!el) return;
|
||||
const s = el.selectionStart, e = el.selectionEnd;
|
||||
const sel = replyText.slice(s, e) || 'text';
|
||||
replyText = replyText.slice(0, s) + token + sel + token + replyText.slice(e);
|
||||
queueMicrotask(() => {
|
||||
el.focus();
|
||||
el.selectionStart = s + token.length;
|
||||
el.selectionEnd = s + token.length + sel.length;
|
||||
});
|
||||
}
|
||||
// Prefix each selected line (e.g. "- " or "## ").
|
||||
function mdPrefix(prefix) {
|
||||
const el = replyTextarea;
|
||||
if (!el) return;
|
||||
const s = el.selectionStart, e = el.selectionEnd;
|
||||
const start = replyText.lastIndexOf('\n', s - 1) + 1;
|
||||
let end = replyText.indexOf('\n', e);
|
||||
if (end === -1) end = replyText.length;
|
||||
const block = replyText.slice(start, end) || 'item';
|
||||
const prefixed = block.split('\n').map((ln) => prefix + ln).join('\n');
|
||||
replyText = replyText.slice(0, start) + prefixed + replyText.slice(end);
|
||||
queueMicrotask(() => el.focus());
|
||||
}
|
||||
async function sendReply(f) {
|
||||
const msg = replyText.trim();
|
||||
if (!msg || replyBusy) return;
|
||||
@@ -434,7 +462,12 @@
|
||||
{#each f.replies as r (r.id)}
|
||||
<div class="rep">
|
||||
<span class="rl">↪ Replied · {fwhen(r.sent_at)}</span>
|
||||
<p class="repmsg">{r.message}</p>
|
||||
{#if r.message_html}
|
||||
<!-- server-sanitized HTML (tiny Markdown subset) -->
|
||||
<div class="repmsg">{@html r.message_html}</div>
|
||||
{:else}
|
||||
<p class="repmsg">{r.message}</p>
|
||||
{/if}
|
||||
</div>
|
||||
{/each}
|
||||
</div>
|
||||
@@ -442,7 +475,12 @@
|
||||
|
||||
{#if replyingId === f.id}
|
||||
<div class="composer">
|
||||
<textarea bind:value={replyText} rows="4" placeholder="Write a reply…"></textarea>
|
||||
<div class="mdbar">
|
||||
<button type="button" class="mdbtn" title="Bold (**…**)" onclick={() => mdWrap('**')}><b>B</b></button>
|
||||
<button type="button" class="mdbtn" title="Bullet list" onclick={() => mdPrefix('- ')}>• List</button>
|
||||
<button type="button" class="mdbtn" title="Heading" onclick={() => mdPrefix('## ')}>H</button>
|
||||
</div>
|
||||
<textarea bind:this={replyTextarea} bind:value={replyText} rows="4" placeholder="Write a reply… **bold**, - bullets, ## heading"></textarea>
|
||||
{#if replyErr}<p class="cerr">{replyErr}</p>{/if}
|
||||
<div class="cbtns">
|
||||
<button class="csend" onclick={() => sendReply(f)} disabled={replyBusy || !replyText.trim()}>
|
||||
@@ -624,8 +662,19 @@
|
||||
.thread { margin: 10px 0 0; display: flex; flex-direction: column; gap: 8px; }
|
||||
.rep { border-left: 2px solid var(--accent-soft); padding: 2px 0 2px 12px; }
|
||||
.rep .rl { font-size: 0.72rem; color: var(--muted); }
|
||||
.rep .repmsg { margin: 2px 0 0; font-size: 0.9rem; color: var(--ink); white-space: pre-wrap; }
|
||||
.rep .repmsg { margin: 2px 0 0; font-size: 0.9rem; color: var(--ink); }
|
||||
.rep .repmsg :global(p) { margin: 0 0 6px; white-space: pre-wrap; }
|
||||
.rep .repmsg :global(ul) { margin: 4px 0; padding-left: 20px; }
|
||||
.rep .repmsg :global(h3),
|
||||
.rep .repmsg :global(h4),
|
||||
.rep .repmsg :global(h5) { margin: 6px 0 4px; font-size: 0.95rem; }
|
||||
.composer { margin: 10px 0 0; }
|
||||
.mdbar { display: flex; gap: 6px; margin-bottom: 6px; }
|
||||
.mdbtn {
|
||||
font: inherit; font-size: 0.78rem; background: var(--surface); border: 1px solid var(--line);
|
||||
color: var(--ink); border-radius: 7px; padding: 3px 9px; cursor: pointer; line-height: 1.4;
|
||||
}
|
||||
.mdbtn:hover { border-color: var(--accent); color: var(--accent-deep); }
|
||||
.composer textarea {
|
||||
width: 100%; box-sizing: border-box; font: inherit; font-size: 0.9rem;
|
||||
padding: 10px 12px; border: 1px solid var(--line); border-radius: 10px;
|
||||
|
||||
+8
-5
@@ -32,6 +32,7 @@ from fastapi.staticfiles import StaticFiles
|
||||
from pydantic import BaseModel
|
||||
|
||||
from . import auth, email_send, feeds, oauth_google, queries, share, summarize
|
||||
from .markup import render_reply_html
|
||||
from .db import connect
|
||||
from .filters import filter_articles, prefs_from_json
|
||||
from .hero import safe_to_lead
|
||||
@@ -808,7 +809,7 @@ def create_app() -> FastAPI:
|
||||
ids = [it["id"] for it in items]
|
||||
ph = ",".join("?" * len(ids))
|
||||
reps = conn.execute(
|
||||
f"SELECT id, feedback_id, message, sent_to, sent_at FROM feedback_replies "
|
||||
f"SELECT id, feedback_id, message, message_html, sent_to, sent_at FROM feedback_replies "
|
||||
f"WHERE feedback_id IN ({ph}) ORDER BY sent_at",
|
||||
ids,
|
||||
).fetchall()
|
||||
@@ -855,24 +856,26 @@ def create_app() -> FastAPI:
|
||||
if not fb["contact_email"]:
|
||||
raise HTTPException(status_code=400, detail="No reply address for this feedback.")
|
||||
admin_id, to, original = admin["id"], fb["contact_email"], fb["message"]
|
||||
reply_html = render_reply_html(message) # tiny safe Markdown subset
|
||||
# 2. Send with no DB connection held; only record a reply that actually
|
||||
# went out, so the UI can keep the draft on failure.
|
||||
try:
|
||||
email_send.send_feedback_reply(to, message, original)
|
||||
email_send.send_feedback_reply(to, message, reply_html, original)
|
||||
except Exception as exc: # noqa: BLE001 — surface any SMTP failure to the admin
|
||||
raise HTTPException(status_code=502, detail=f"Could not send the reply: {exc}")
|
||||
# 3. Record the sent reply + mark the item read.
|
||||
with get_conn() as conn:
|
||||
cur = conn.execute(
|
||||
"INSERT INTO feedback_replies (feedback_id, user_id, message, sent_to) VALUES (?, ?, ?, ?)",
|
||||
(fid, admin_id, message, to),
|
||||
"INSERT INTO feedback_replies (feedback_id, user_id, message, message_html, sent_to) "
|
||||
"VALUES (?, ?, ?, ?, ?)",
|
||||
(fid, admin_id, message, reply_html, to),
|
||||
)
|
||||
conn.execute(
|
||||
"UPDATE feedback SET read_at = COALESCE(read_at, CURRENT_TIMESTAMP) WHERE id = ?", (fid,)
|
||||
)
|
||||
conn.commit()
|
||||
reply = conn.execute(
|
||||
"SELECT id, feedback_id, message, sent_to, sent_at FROM feedback_replies WHERE id = ?",
|
||||
"SELECT id, feedback_id, message, message_html, sent_to, sent_at FROM feedback_replies WHERE id = ?",
|
||||
(cur.lastrowid,),
|
||||
).fetchone()
|
||||
return {"ok": True, "reply": dict(reply)}
|
||||
|
||||
@@ -246,6 +246,7 @@ CREATE TABLE IF NOT EXISTS feedback_replies (
|
||||
feedback_id INTEGER NOT NULL REFERENCES feedback(id) ON DELETE CASCADE,
|
||||
user_id INTEGER REFERENCES users(id) ON DELETE SET NULL,
|
||||
message TEXT NOT NULL,
|
||||
message_html TEXT,
|
||||
sent_to TEXT NOT NULL,
|
||||
sent_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP
|
||||
);
|
||||
@@ -320,3 +321,8 @@ def _migrate(conn: sqlite3.Connection) -> None:
|
||||
fb_cols = {row["name"] for row in conn.execute("PRAGMA table_info(feedback)")}
|
||||
if fb_cols and "read_at" not in fb_cols:
|
||||
conn.execute("ALTER TABLE feedback ADD COLUMN read_at TEXT")
|
||||
|
||||
# feedback_replies.message_html (rendered Markdown subset) added later.
|
||||
rep_cols = {row["name"] for row in conn.execute("PRAGMA table_info(feedback_replies)")}
|
||||
if rep_cols and "message_html" not in rep_cols:
|
||||
conn.execute("ALTER TABLE feedback_replies ADD COLUMN message_html TEXT")
|
||||
|
||||
+16
-4
@@ -63,19 +63,31 @@ def send_feedback(to: str, category: str, message: str, contact: str | None, who
|
||||
send_email(to, subject, text)
|
||||
|
||||
|
||||
def send_feedback_reply(to: str, reply_message: str, original_message: str) -> None:
|
||||
"""Reply to a reader's feedback from the admin inbox (plain text). Quotes
|
||||
def send_feedback_reply(to: str, reply_text: str, reply_html: str | None, original_message: str) -> None:
|
||||
"""Reply to a reader's feedback from the admin inbox. Sends multipart
|
||||
text/plain + text/html (the HTML is the pre-sanitized Markdown render). Quotes
|
||||
their original note for context; exposes no analytics/account details."""
|
||||
subject = "Re: Your Upbeat Bytes feedback"
|
||||
quoted = "\n".join("> " + line for line in (original_message or "").splitlines())
|
||||
text = (
|
||||
f"{reply_message}\n\n"
|
||||
f"{reply_text}\n\n"
|
||||
"—\n"
|
||||
"In reply to your note to Upbeat Bytes:\n"
|
||||
f"{quoted}\n\n"
|
||||
"Thanks for reaching out.\n— Upbeat Bytes\n"
|
||||
)
|
||||
send_email(to, subject, text)
|
||||
body_html = None
|
||||
if reply_html:
|
||||
oq = escape(original_message or "").replace("\n", "<br>")
|
||||
body_html = (
|
||||
'<div style="font-family:-apple-system,Segoe UI,Roboto,Helvetica,Arial,sans-serif;'
|
||||
'color:#16263a;font-size:15px;line-height:1.5">'
|
||||
f"{reply_html}"
|
||||
'<p style="color:#5d6b78;font-size:13px;margin-top:20px">In reply to your note to Upbeat Bytes:</p>'
|
||||
f'<blockquote style="color:#5d6b78;border-left:3px solid #e8e3d8;margin:0;padding-left:12px">{oq}</blockquote>'
|
||||
"<p>Thanks for reaching out.<br>— Upbeat Bytes</p></div>"
|
||||
)
|
||||
send_email(to, subject, text, html=body_html)
|
||||
|
||||
|
||||
def send_magic_link(to: str, link: str) -> None:
|
||||
|
||||
@@ -0,0 +1,48 @@
|
||||
"""A tiny, safe Markdown subset → HTML, for admin feedback replies.
|
||||
|
||||
Everything is HTML-escaped FIRST, then a small whitelist of structures is
|
||||
introduced: bold (**…**), bullet lists (- / *), headings (#/##/###),
|
||||
paragraphs, and line breaks. No links, no attributes, no inline styles, no raw
|
||||
HTML passthrough — so the output is safe to render and to email. Authored only
|
||||
by admins, but sanitized on principle regardless.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import html
|
||||
import re
|
||||
|
||||
_BULLET_RE = re.compile(r"^\s*[-*]\s+")
|
||||
_HEADING_RE = re.compile(r"^\s*(#{1,3})\s+")
|
||||
_BOLD_RE = re.compile(r"\*\*(.+?)\*\*")
|
||||
_HEADING_TAG = {1: "h3", 2: "h4", 3: "h5"}
|
||||
|
||||
|
||||
def _inline(s: str) -> str:
|
||||
"""Escape, then apply the only inline transform we allow: **bold**."""
|
||||
s = html.escape(s, quote=False)
|
||||
return _BOLD_RE.sub(r"<strong>\1</strong>", s)
|
||||
|
||||
|
||||
def render_reply_html(text: str) -> str:
|
||||
"""Render the supported Markdown subset to sanitized HTML (or '' if empty)."""
|
||||
text = (text or "").replace("\r\n", "\n").replace("\r", "\n").strip()
|
||||
if not text:
|
||||
return ""
|
||||
out: list[str] = []
|
||||
for block in re.split(r"\n\s*\n", text):
|
||||
lines = [ln for ln in block.split("\n")]
|
||||
nonempty = [ln for ln in lines if ln.strip()]
|
||||
if not nonempty:
|
||||
continue
|
||||
if all(_BULLET_RE.match(ln) for ln in nonempty):
|
||||
items = "".join("<li>" + _inline(_BULLET_RE.sub("", ln)) + "</li>" for ln in nonempty)
|
||||
out.append("<ul>" + items + "</ul>")
|
||||
elif len(nonempty) == 1 and _HEADING_RE.match(nonempty[0]):
|
||||
level = len(_HEADING_RE.match(nonempty[0]).group(1))
|
||||
content = _HEADING_RE.sub("", nonempty[0])
|
||||
tag = _HEADING_TAG[level]
|
||||
out.append(f"<{tag}>" + _inline(content) + f"</{tag}>")
|
||||
else:
|
||||
out.append("<p>" + "<br>".join(_inline(ln) for ln in nonempty) + "</p>")
|
||||
return "".join(out)
|
||||
@@ -102,19 +102,22 @@ def test_feedback_reply_sends_stores_marks_read(tmp_path, monkeypatch):
|
||||
monkeypatch.setattr(api.email_send, "send_feedback", lambda *a, **k: None)
|
||||
sent = {}
|
||||
monkeypatch.setattr(api.email_send, "send_feedback_reply",
|
||||
lambda to, msg, orig: sent.update(to=to, msg=msg, orig=orig))
|
||||
lambda to, msg, html, orig: sent.update(to=to, msg=msg, html=html, orig=orig))
|
||||
TestClient(app).post("/api/feedback", json={"message": "is there an app?", "email": "reader@x.com", "visitor": "v"})
|
||||
tc = TestClient(app); link = {}
|
||||
monkeypatch.setattr(api.email_send, "send_magic_link", lambda to, l: link.update(l=l))
|
||||
tc.post("/api/auth/email/start", json={"email": "boss@x.com"})
|
||||
tc.post("/api/auth/email/verify", json={"token": link["l"].split("token=")[1]})
|
||||
fid = tc.get("/api/admin/feedback").json()[0]["id"]
|
||||
r = tc.post(f"/api/admin/feedback/{fid}/reply", json={"message": "Yes — a companion app is coming."})
|
||||
r = tc.post(f"/api/admin/feedback/{fid}/reply", json={"message": "Yes — a **companion app** is coming."})
|
||||
assert r.status_code == 200
|
||||
assert sent["to"] == "reader@x.com" and "companion app" in sent["msg"] and "is there an app?" in sent["orig"]
|
||||
assert "<strong>companion app</strong>" in sent["html"] # rendered markdown sent as HTML
|
||||
fb = tc.get("/api/admin/feedback").json()[0]
|
||||
assert fb["read_at"] is not None # marked read on reply
|
||||
assert len(fb["replies"]) == 1 and fb["replies"][0]["sent_to"] == "reader@x.com"
|
||||
rep = fb["replies"][0]
|
||||
assert len(fb["replies"]) == 1 and rep["sent_to"] == "reader@x.com"
|
||||
assert "<strong>companion app</strong>" in rep["message_html"] # stored HTML
|
||||
|
||||
|
||||
def test_feedback_reply_requires_address_and_message(tmp_path, monkeypatch):
|
||||
|
||||
@@ -0,0 +1,23 @@
|
||||
from goodnews.markup import render_reply_html as r
|
||||
|
||||
|
||||
def test_escapes_before_formatting():
|
||||
out = r("<script>alert(1)</script> and **bold**")
|
||||
assert "<script>" not in out and "<script>" in out
|
||||
assert "<strong>bold</strong>" in out
|
||||
|
||||
|
||||
def test_bullets_and_heading_and_paragraphs():
|
||||
assert r("- one\n- two") == "<ul><li>one</li><li>two</li></ul>"
|
||||
assert r("## Update") == "<h4>Update</h4>"
|
||||
assert r("a\nb\n\nc") == "<p>a<br>b</p><p>c</p>"
|
||||
|
||||
|
||||
def test_no_links_or_attrs_pass_through():
|
||||
out = r('[click](http://x) <a href="x" onclick="y">hi</a>')
|
||||
assert "<a" not in out # no anchor tag produced or passed through (escaped to <a)
|
||||
assert "[click]" in out # markdown links unsupported → left as literal text
|
||||
|
||||
|
||||
def test_empty():
|
||||
assert r(" ") == "" and r(None) == ""
|
||||
Reference in New Issue
Block a user