From d9350ac5772cbb8852e799ee98e01c19f216bacf Mon Sep 17 00:00:00 2001 From: ashwin Date: Sun, 10 May 2026 16:53:11 +0530 Subject: [PATCH] feat: expose unknown placeholders in macro API and document multi-tier implementation plans --- backend/macros/render.py | 22 +++++ backend/macros/serializers.py | 10 +++ backend/macros/tests/test_api.py | 73 ++++++++++++++++ backend/macros/tests/test_render.py | 32 ++++++- .../routes/(app)/settings/macros/+page.svelte | 86 ++++++++++++++++++- 5 files changed, 221 insertions(+), 2 deletions(-) diff --git a/backend/macros/render.py b/backend/macros/render.py index 3970549c..9036bcd4 100644 --- a/backend/macros/render.py +++ b/backend/macros/render.py @@ -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`. diff --git a/backend/macros/serializers.py b/backend/macros/serializers.py index 66f9b3e0..4fe1da72 100644 --- a/backend/macros/serializers.py +++ b/backend/macros/serializers.py @@ -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) diff --git a/backend/macros/tests/test_api.py b/backend/macros/tests/test_api.py index de0946b7..20d65cbf 100644 --- a/backend/macros/tests/test_api.py +++ b/backend/macros/tests/test_api.py @@ -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 diff --git a/backend/macros/tests/test_render.py b/backend/macros/tests/test_render.py index f153a2c4..ae3d0d96 100644 --- a/backend/macros/tests/test_render.py +++ b/backend/macros/tests/test_render.py @@ -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) == "" diff --git a/frontend/src/routes/(app)/settings/macros/+page.svelte b/frontend/src/routes/(app)/settings/macros/+page.svelte index 2c66cdb5..1cc4f0f1 100644 --- a/frontend/src/routes/(app)/settings/macros/+page.svelte +++ b/frontend/src/routes/(app)/settings/macros/+page.svelte @@ -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} */ + 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)); @@ -162,6 +191,24 @@ required placeholder="Hi %customer_name%, this is %agent_name%..." /> + {#if unknownInNew.length > 0} + + {/if}