fix(admin): interpolate activity and notification message values

Users were shown raw "{{quoteNumber}}", "{{name}}" and "{{count}}" tokens.
Two distinct render-side causes; nothing is persisted as a rendered string
(messages are stored as type + metadata JSON and formatted client-side), so
no backend change was needed.

{{quoteNumber}} / {{name}} -- AdminDashboard's getActivityMessage built a
hardcoded five-value allowlist (eventName, email, count, template,
categoryName) and passed it to t('admin.activities.<type>'). The backend does
record quoteNumber (quoteService.js) and name (adminWebhooks.js); the values
just never reached i18next, so every activity string interpolating anything
outside that allowlist rendered its literal token. Spread activity.metadata
first, keeping the five derived entries as overrides since they resolve from
columns that are not in metadata. Extracted as buildActivityParams for
testability, mirroring the formatDayHeader extraction.

{{count}} -- different cause, the notification-bell path: archiveBulk.js logs
successfulCount, but the locale string expects count and
bulk_archive_completed had no explicit case, so the default branch spread a
metadata object without one. Added a case next to the existing
bulk_delete_completed, following that idiom.

Also fixes bulk_delete_completed, which has the identical mismatch: it reads
metadata.deleted || metadata.count while archiveBulk.js writes successfulCount,
so that notification always rendered "0 events deleted". It degrades to a wrong
number rather than a visible placeholder, which is why it was not among the
three reported instances -- but it is the same one-token bug.

Refs testplan REPORT.md #15b.
This commit is contained in:
Paul Nothaft
2026-09-01 16:48:24 +02:00
parent d8bd0cd449
commit 78b1ddd0db
3 changed files with 157 additions and 13 deletions
+27 -12
View File
@@ -19,6 +19,7 @@ import { parseISO } from 'date-fns';
import { useQueryClient } from '@tanstack/react-query';
import { useExpiryRefresh } from '../../hooks/useExpiryRefresh';
import { useTranslation } from 'react-i18next';
import i18n from '../../i18n/config';
import { useLocalizedDate } from '../../hooks/useLocalizedDate';
import { useMutationWithToast } from '../../hooks';
@@ -28,7 +29,7 @@ import { WhatsNewBanner } from '../../components/admin/WhatsNewBanner';
import { CrmOverviewSection } from '../../components/admin/CrmOverviewSection';
import { useQuery } from '@tanstack/react-query';
import { eventsService } from '../../services/events.service';
import { adminService, ActivityType } from '../../services/admin.service';
import { adminService, ActivityType, type Activity } from '../../services/admin.service';
import { workflowsService } from '../../services/workflows.service';
import { useFeatureFlags } from '../../contexts/FeatureFlagsContext';
@@ -40,6 +41,30 @@ interface StatCard {
color: string;
}
/**
* Interpolation values for an `admin.activities.*` line.
*
* The activity's own metadata is spread in first: the keys interpolate
* whatever the backend recorded for that type ({{name}} for
* webhook_created, {{quoteNumber}} for quote_created, {{contractNumber}},
* {{username}}, {{word}}, …). Before that, only a fixed five-value
* allowlist was passed, so every other key rendered its raw "{{…}}"
* placeholder in the activity feed (QA S7). The explicit entries below
* stay as derived/defaulted overrides — they resolve from columns that
* are not in metadata, or need a fallback when metadata is empty.
*/
export function buildActivityParams(activity: Activity): Record<string, unknown> {
const t = i18n.t;
return {
...activity.metadata,
eventName: activity.eventName || t('common.unknown'),
email: activity.metadata?.email || activity.actorName || '',
count: activity.metadata?.count || 0,
template: activity.metadata?.template_key || '',
categoryName: activity.metadata?.category_name || '',
};
}
export const AdminDashboard: React.FC = () => {
const { t } = useTranslation();
const navigate = useNavigate();
@@ -416,17 +441,7 @@ export const AdminDashboard: React.FC = () => {
// Format activity message with translations
const getActivityMessage = (): string => {
const params: Record<string, any> = {
eventName: activity.eventName || t('common.unknown'),
// Customer/account activity keys (customer_login,
// customer_invitation_*, customer_updated, …) interpolate
// {{email}}; without it the literal placeholder rendered.
// Sourced the same way formatActivityMessage does.
email: activity.metadata?.email || activity.actorName || '',
count: activity.metadata?.count || 0,
template: activity.metadata?.template_key || '',
categoryName: activity.metadata?.category_name || ''
};
const params = buildActivityParams(activity);
const translated = t(`admin.activities.${activity.type}`, params);
// Translate; if key missing i18n returns the key string itself
@@ -0,0 +1,121 @@
/**
* Raw `{{placeholder}}` tokens leaking into the admin UI (QA bug #15b).
*
* Three confirmed sightings, two distinct call sites:
*
* - Dashboard "recent activity" feed — `buildActivityParams` used to pass a
* fixed five-value allowlist (eventName / email / count / template /
* categoryName), so every `admin.activities.*` string interpolating
* anything else rendered its literal token: "Quote created: {{quoteNumber}}",
* "Webhook erstellt: {{name}}". The backend does record those values, they
* just never reached i18next.
*
* - Notification bell — `bulk_archive_completed` fell through to the generic
* default branch, which spreads `metadata`. The bulk routes log
* `successfulCount`, never `count`, so the bell showed
* "Bulk archive completed: {{count}} events archived".
*
* These assertions are deliberately written against the *rendered output*: any
* future regression that drops an interpolation value shows up as a surviving
* "{{" in the string, whatever the mechanism.
*/
import { describe, it, expect, beforeAll } from 'vitest';
import i18n from '../../../i18n/config';
import { notificationsService, type Notification } from '../../../services/notifications.service';
import { buildActivityParams } from '../AdminDashboard';
import type { Activity } from '../../../services/admin.service';
const activity = (type: string, metadata: Record<string, unknown>): Activity =>
({
id: 1,
type,
actorType: 'admin',
actorName: 'admin',
eventName: undefined,
metadata,
createdAt: '2026-09-01T12:00:00Z',
}) as Activity;
const notification = (type: string, metadata: Record<string, unknown>): Notification =>
({
id: 1,
type,
actorType: 'admin',
actorName: 'admin',
eventName: undefined,
metadata,
createdAt: '2026-09-01T12:00:00Z',
isRead: false,
}) as Notification;
const render = (a: Activity) =>
i18n.t(`admin.activities.${a.type}`, buildActivityParams(a)) as string;
describe('dashboard activity feed interpolation', () => {
beforeAll(async () => {
await i18n.changeLanguage('en');
});
it('interpolates {{name}} for webhook_created', () => {
const msg = render(activity('webhook_created', { name: 'n8n WhatsApp', events: ['event.published'] }));
expect(msg).toContain('n8n WhatsApp');
expect(msg).not.toContain('{{');
});
it('interpolates {{quoteNumber}} for quote_created', () => {
const msg = render(activity('quote_created', { quoteId: 7, quoteNumber: 'Q-2026-0007' }));
expect(msg).toContain('Q-2026-0007');
expect(msg).not.toContain('{{');
});
it('still honours the derived overrides that are not plain metadata', () => {
// `template` is read from metadata.template_key, `categoryName` from
// metadata.category_name — the spread must not shadow those mappings.
const params = buildActivityParams(
activity('email_template_created', { template_key: 'gallery_created', category_name: 'Ceremony' })
);
expect(params.template).toBe('gallery_created');
expect(params.categoryName).toBe('Ceremony');
});
it('leaves no raw placeholder on any German activity string either', async () => {
await i18n.changeLanguage('de');
const msg = render(activity('webhook_created', { name: 'ZZTEST-hook' }));
expect(msg).toContain('ZZTEST-hook');
expect(msg).not.toContain('{{');
await i18n.changeLanguage('en');
});
});
describe('notification bell interpolation', () => {
beforeAll(async () => {
await i18n.changeLanguage('en');
});
it('maps the bulk-archive metadata onto {{count}}', () => {
// Exactly what adminEvents/archiveBulk.js writes — no `count` key.
const msg = notificationsService.formatNotificationMessage(
notification('bulk_archive_completed', { totalEvents: 5, successfulCount: 4, failedCount: 1 })
);
expect(msg).toContain('4');
expect(msg).not.toContain('{{');
});
it('falls back to 0 rather than a placeholder when metadata is empty', () => {
const msg = notificationsService.formatNotificationMessage(
notification('bulk_archive_completed', {})
);
expect(msg).not.toContain('{{');
});
it('keeps interpolating the webhook/quote bell rows', () => {
expect(
notificationsService.formatNotificationMessage(notification('webhook_created', { name: 'n8n' }))
).not.toContain('{{');
expect(
notificationsService.formatNotificationMessage(
notification('quote_created', { quoteNumber: 'Q-2026-0007' })
)
).toContain('Q-2026-0007');
});
});
@@ -280,7 +280,15 @@ export const notificationsService = {
return t('admin.notificationMessages.eventLogoRemoved', { eventName: notification.eventName });
case 'bulk_delete_completed':
return t('admin.notificationMessages.bulkDeleteCompleted', {
count: notification.metadata.deleted || notification.metadata.count || 0,
count: notification.metadata.successfulCount ?? notification.metadata.deleted ?? notification.metadata.count ?? 0,
});
// The bulk routes log `successfulCount` (see adminEvents/archiveBulk.js),
// never `count` — without the mapping the default branch below spread a
// metadata object with no `count`, so i18next left the literal
// "{{count}}" in the bell (QA B.06b).
case 'bulk_archive_completed':
return t('admin.notificationMessages.bulkArchiveCompleted', {
count: notification.metadata.successfulCount ?? notification.metadata.count ?? 0,
});
case 'photo_replaced':
return t('admin.notificationMessages.photoReplaced', { eventName: notification.eventName });