feat: expose unknown placeholders in macro API and document multi-tier implementation plans
This commit is contained in:
ashwin
parent
b9a11553e6
commit
d9350ac577
5 changed files with 221 additions and 2 deletions
|
|
@ -93,6 +93,28 @@ SUPPORTED_TOKENS: Iterable[str] = frozenset(
|
|||
)
|
||||
|
||||
|
||||
def find_unknown_placeholders(body: str | None) -> list[str]:
|
||||
"""Return the `%token%` placeholders in `body` that aren't supported.
|
||||
|
||||
Order-preserving and de-duplicated so callers can render them in the
|
||||
order they appear. Tokens are returned with their `%...%` wrapping so
|
||||
the UI / API consumer can show them verbatim. The result is non-empty
|
||||
only when the macro author has typoed (`%custmer_name%`) or used a
|
||||
token we don't expand server-side — same set as `render_macro`.
|
||||
"""
|
||||
if not body:
|
||||
return []
|
||||
seen: list[str] = []
|
||||
seen_set: set[str] = set()
|
||||
for match in _PLACEHOLDER_RE.finditer(body):
|
||||
token = match.group(1)
|
||||
if token in SUPPORTED_TOKENS or token in seen_set:
|
||||
continue
|
||||
seen.append(f"%{token}%")
|
||||
seen_set.add(token)
|
||||
return seen
|
||||
|
||||
|
||||
def render_macro(macro, case, profile) -> str:
|
||||
"""Substitute supported `%token%` placeholders in `macro.body`.
|
||||
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@
|
|||
from rest_framework import serializers
|
||||
|
||||
from macros.models import Macro
|
||||
from macros.render import find_unknown_placeholders
|
||||
|
||||
|
||||
class MacroSerializer(serializers.ModelSerializer):
|
||||
|
|
@ -13,6 +14,7 @@ class MacroSerializer(serializers.ModelSerializer):
|
|||
"""
|
||||
|
||||
owner_name = serializers.SerializerMethodField()
|
||||
unknown_placeholders = serializers.SerializerMethodField()
|
||||
|
||||
class Meta:
|
||||
model = Macro
|
||||
|
|
@ -25,6 +27,7 @@ class MacroSerializer(serializers.ModelSerializer):
|
|||
"owner_name",
|
||||
"is_active",
|
||||
"usage_count",
|
||||
"unknown_placeholders",
|
||||
"created_at",
|
||||
"updated_at",
|
||||
)
|
||||
|
|
@ -33,6 +36,7 @@ class MacroSerializer(serializers.ModelSerializer):
|
|||
"owner",
|
||||
"owner_name",
|
||||
"usage_count",
|
||||
"unknown_placeholders",
|
||||
"created_at",
|
||||
"updated_at",
|
||||
)
|
||||
|
|
@ -44,3 +48,9 @@ class MacroSerializer(serializers.ModelSerializer):
|
|||
if obj.owner is None or obj.owner.user is None:
|
||||
return None
|
||||
return obj.owner.user.email or None
|
||||
|
||||
def get_unknown_placeholders(self, obj):
|
||||
# Soft signal, not a save-blocker: unknown tokens render literally
|
||||
# on the ticket (see render_macro). Surfacing them here gives mobile
|
||||
# and API consumers the same warning the web UI computes client-side.
|
||||
return find_unknown_placeholders(obj.body)
|
||||
|
|
|
|||
|
|
@ -288,3 +288,76 @@ class TestRender:
|
|||
format="json",
|
||||
)
|
||||
assert resp.status_code == 404
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# unknown_placeholders serializer field
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestUnknownPlaceholders:
|
||||
"""Backend-authoritative warning surface for typoed/unsupported tokens.
|
||||
|
||||
The web UI does its own client-side check for live feedback, but mobile
|
||||
and API consumers rely on this field — and the workspace rule says
|
||||
every UX constraint must also exist server-side.
|
||||
"""
|
||||
|
||||
def test_clean_body_returns_empty_list(self, user_client, org_macro):
|
||||
resp = user_client.get(LIST_URL)
|
||||
rows = {row["id"]: row for row in resp.json()["results"]}
|
||||
assert rows[str(org_macro.id)]["unknown_placeholders"] == []
|
||||
|
||||
def test_typo_surfaced_on_create(self, admin_client):
|
||||
resp = admin_client.post(
|
||||
LIST_URL,
|
||||
{
|
||||
"title": "Greeting",
|
||||
"body": "Hi %custmer_name%, this is %agent_name%.",
|
||||
"scope": "org",
|
||||
},
|
||||
format="json",
|
||||
)
|
||||
assert resp.status_code == 201
|
||||
assert resp.json()["unknown_placeholders"] == ["%custmer_name%"]
|
||||
|
||||
def test_typo_surfaced_on_list_and_detail(self, user_client, org_a):
|
||||
macro = Macro.objects.create(
|
||||
org=org_a,
|
||||
title="Bad",
|
||||
body="Hi %custmer_name%, priority %priority%.",
|
||||
scope=Macro.SCOPE_ORG,
|
||||
)
|
||||
list_resp = user_client.get(LIST_URL)
|
||||
row = next(r for r in list_resp.json()["results"] if r["id"] == str(macro.id))
|
||||
assert row["unknown_placeholders"] == ["%custmer_name%", "%priority%"]
|
||||
|
||||
detail_resp = user_client.get(_detail_url(macro.id))
|
||||
assert detail_resp.json()["unknown_placeholders"] == [
|
||||
"%custmer_name%",
|
||||
"%priority%",
|
||||
]
|
||||
|
||||
def test_field_updates_after_patch(self, admin_client, org_macro):
|
||||
# Sanity: editing the body re-derives the field from the new text.
|
||||
resp = admin_client.patch(
|
||||
_detail_url(org_macro.id),
|
||||
{"body": "Now with %typo%"},
|
||||
format="json",
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.json()["unknown_placeholders"] == ["%typo%"]
|
||||
|
||||
def test_creation_is_not_blocked_by_unknown_placeholder(self, admin_client):
|
||||
# Soft warning, not a save-blocker — matches `render_macro`'s
|
||||
# leave-literal stance for unknown tokens.
|
||||
resp = admin_client.post(
|
||||
LIST_URL,
|
||||
{
|
||||
"title": "Future",
|
||||
"body": "Priority is %priority%",
|
||||
"scope": "org",
|
||||
},
|
||||
format="json",
|
||||
)
|
||||
assert resp.status_code == 201
|
||||
|
|
|
|||
|
|
@ -2,13 +2,43 @@
|
|||
|
||||
from types import SimpleNamespace
|
||||
|
||||
from macros.render import render_macro
|
||||
from macros.render import find_unknown_placeholders, render_macro
|
||||
|
||||
|
||||
def _macro(body: str):
|
||||
return SimpleNamespace(body=body)
|
||||
|
||||
|
||||
class TestFindUnknownPlaceholders:
|
||||
def test_empty_body_returns_empty_list(self):
|
||||
assert find_unknown_placeholders("") == []
|
||||
assert find_unknown_placeholders(None) == []
|
||||
|
||||
def test_no_placeholders_returns_empty_list(self):
|
||||
assert find_unknown_placeholders("Plain text, no tokens.") == []
|
||||
|
||||
def test_only_supported_returns_empty_list(self):
|
||||
body = "Hi %customer_name%, I'm %agent_name% from %org_name%."
|
||||
assert find_unknown_placeholders(body) == []
|
||||
|
||||
def test_typo_is_flagged(self):
|
||||
# The bug we're guarding against: missing 'o' in 'customer'.
|
||||
body = "Hi %custmer_name%, how can we help?"
|
||||
assert find_unknown_placeholders(body) == ["%custmer_name%"]
|
||||
|
||||
def test_mixed_known_and_unknown(self):
|
||||
body = "Hi %customer_name%, your priority %priority% case %case_id%."
|
||||
assert find_unknown_placeholders(body) == ["%priority%"]
|
||||
|
||||
def test_dedupes_repeated_unknown(self):
|
||||
body = "%foo% then %foo% again"
|
||||
assert find_unknown_placeholders(body) == ["%foo%"]
|
||||
|
||||
def test_preserves_first_seen_order(self):
|
||||
body = "%bar% comes after %foo%, then %baz%."
|
||||
assert find_unknown_placeholders(body) == ["%bar%", "%foo%", "%baz%"]
|
||||
|
||||
|
||||
class TestRenderPlaceholders:
|
||||
def test_empty_body(self):
|
||||
assert render_macro(_macro(""), case=None, profile=None) == ""
|
||||
|
|
|
|||
|
|
@ -8,7 +8,8 @@
|
|||
Plus,
|
||||
Trash2,
|
||||
Pencil,
|
||||
Power
|
||||
Power,
|
||||
AlertTriangle
|
||||
} from '@lucide/svelte';
|
||||
import { PageHeader } from '$lib/components/layout';
|
||||
import { Button } from '$lib/components/ui/button/index.js';
|
||||
|
|
@ -66,6 +67,34 @@
|
|||
'%agent_email%',
|
||||
'%org_name%'
|
||||
];
|
||||
|
||||
// Mirror of backend SUPPORTED_TOKENS in macros/render.py — unknown tokens
|
||||
// render literally on the ticket, so we surface them at edit-time before
|
||||
// they ship in a customer-facing reply.
|
||||
const SUPPORTED_TOKENS = new Set([
|
||||
'customer_name',
|
||||
'customer_email',
|
||||
'case_id',
|
||||
'case_subject',
|
||||
'agent_name',
|
||||
'agent_email',
|
||||
'org_name'
|
||||
]);
|
||||
|
||||
/** @param {string | undefined | null} body */
|
||||
function findUnknownPlaceholders(body) {
|
||||
if (!body) return [];
|
||||
const re = /%([a-zA-Z_][a-zA-Z0-9_]*)%/g;
|
||||
/** @type {Set<string>} */
|
||||
const found = new Set();
|
||||
for (const m of body.matchAll(re)) {
|
||||
if (!SUPPORTED_TOKENS.has(m[1])) found.add(`%${m[1]}%`);
|
||||
}
|
||||
return Array.from(found);
|
||||
}
|
||||
|
||||
const unknownInNew = $derived(findUnknownPlaceholders(newBody));
|
||||
const unknownInEdit = $derived(findUnknownPlaceholders(editBody));
|
||||
</script>
|
||||
|
||||
<svelte:head>
|
||||
|
|
@ -162,6 +191,24 @@
|
|||
required
|
||||
placeholder="Hi %customer_name%, this is %agent_name%..."
|
||||
/>
|
||||
{#if unknownInNew.length > 0}
|
||||
<div
|
||||
class="flex items-start gap-2 rounded-md border border-amber-200 bg-amber-50/70 p-2 text-xs text-amber-900 dark:border-amber-900/40 dark:bg-amber-900/15 dark:text-amber-200"
|
||||
role="alert"
|
||||
>
|
||||
<AlertTriangle class="mt-0.5 h-3.5 w-3.5 shrink-0" />
|
||||
<span>
|
||||
Unknown placeholder{unknownInNew.length > 1 ? 's' : ''}:
|
||||
{#each unknownInNew as token, i (token)}
|
||||
<code class="rounded bg-amber-100 px-1 dark:bg-amber-900/40"
|
||||
>{token}</code
|
||||
>{i < unknownInNew.length - 1 ? ', ' : ''}
|
||||
{/each}
|
||||
— these will render literally on the ticket. Check spelling
|
||||
against the supported list above.
|
||||
</span>
|
||||
</div>
|
||||
{/if}
|
||||
</div>
|
||||
<div class="flex justify-end">
|
||||
<Button type="submit" disabled={creating} class="gap-2">
|
||||
|
|
@ -202,6 +249,7 @@
|
|||
class="divide-y divide-[var(--border-default)] rounded-md border border-[var(--border-default)]"
|
||||
>
|
||||
{#each orgMacros as m (m.id)}
|
||||
{@const unknown = findUnknownPlaceholders(m.body)}
|
||||
<li class="flex items-start justify-between gap-3 px-3 py-2 text-sm">
|
||||
<div class="min-w-0 flex-1">
|
||||
<div class="flex items-center gap-2">
|
||||
|
|
@ -213,6 +261,15 @@
|
|||
Inactive
|
||||
</span>
|
||||
{/if}
|
||||
{#if unknown.length > 0}
|
||||
<span
|
||||
class="inline-flex items-center gap-1 rounded bg-amber-100 px-1.5 py-0.5 text-[10px] uppercase tracking-wide text-amber-900 dark:bg-amber-900/40 dark:text-amber-200"
|
||||
title={`Unknown placeholder${unknown.length > 1 ? 's' : ''}: ${unknown.join(', ')}`}
|
||||
>
|
||||
<AlertTriangle class="h-3 w-3" />
|
||||
Unknown placeholder{unknown.length > 1 ? 's' : ''}
|
||||
</span>
|
||||
{/if}
|
||||
</div>
|
||||
<p
|
||||
class="mt-0.5 line-clamp-2 whitespace-pre-wrap text-xs text-[var(--text-secondary)]"
|
||||
|
|
@ -284,10 +341,20 @@
|
|||
class="divide-y divide-[var(--border-default)] rounded-md border border-[var(--border-default)]"
|
||||
>
|
||||
{#each personalMacros as m (m.id)}
|
||||
{@const unknown = findUnknownPlaceholders(m.body)}
|
||||
<li class="flex items-start justify-between gap-3 px-3 py-2 text-sm">
|
||||
<div class="min-w-0 flex-1">
|
||||
<div class="flex items-center gap-2">
|
||||
<span class="font-medium">{m.title}</span>
|
||||
{#if unknown.length > 0}
|
||||
<span
|
||||
class="inline-flex items-center gap-1 rounded bg-amber-100 px-1.5 py-0.5 text-[10px] uppercase tracking-wide text-amber-900 dark:bg-amber-900/40 dark:text-amber-200"
|
||||
title={`Unknown placeholder${unknown.length > 1 ? 's' : ''}: ${unknown.join(', ')}`}
|
||||
>
|
||||
<AlertTriangle class="h-3 w-3" />
|
||||
Unknown placeholder{unknown.length > 1 ? 's' : ''}
|
||||
</span>
|
||||
{/if}
|
||||
</div>
|
||||
<p
|
||||
class="mt-0.5 line-clamp-2 whitespace-pre-wrap text-xs text-[var(--text-secondary)]"
|
||||
|
|
@ -390,6 +457,23 @@
|
|||
bind:value={editBody}
|
||||
required
|
||||
/>
|
||||
{#if unknownInEdit.length > 0}
|
||||
<div
|
||||
class="flex items-start gap-2 rounded-md border border-amber-200 bg-amber-50/70 p-2 text-xs text-amber-900 dark:border-amber-900/40 dark:bg-amber-900/15 dark:text-amber-200"
|
||||
role="alert"
|
||||
>
|
||||
<AlertTriangle class="mt-0.5 h-3.5 w-3.5 shrink-0" />
|
||||
<span>
|
||||
Unknown placeholder{unknownInEdit.length > 1 ? 's' : ''}:
|
||||
{#each unknownInEdit as token, i (token)}
|
||||
<code class="rounded bg-amber-100 px-1 dark:bg-amber-900/40"
|
||||
>{token}</code
|
||||
>{i < unknownInEdit.length - 1 ? ', ' : ''}
|
||||
{/each}
|
||||
— these will render literally on the ticket.
|
||||
</span>
|
||||
</div>
|
||||
{/if}
|
||||
</div>
|
||||
{#if editing.scope === 'org'}
|
||||
<div class="flex items-center gap-2">
|
||||
|
|
|
|||
Loading…
Reference in a new issue