diff --git a/client/src/App.jsx b/client/src/App.jsx index 00a21a5..c347a5c 100644 --- a/client/src/App.jsx +++ b/client/src/App.jsx @@ -64,6 +64,7 @@ import AcceptInvite from './routes/player/AcceptInvite.jsx' import PlayerPortalLayout, { PlayerIndex } from './routes/player/PlayerPortalLayout.jsx' import PlayerAccount from './routes/player/PlayerAccount.jsx' import PlayerNotifications from './routes/player/PlayerNotifications.jsx' +import PlayerInbox from './routes/player/PlayerInbox.jsx' import Unsubscribe from './routes/player/Unsubscribe.jsx' import PlayerAppeals from './routes/player/PlayerAppeals.jsx' @@ -209,6 +210,13 @@ export default function App() { } /> } /> + {/* Staff have an inbox and channel preferences like anyone else — + `/auth/me/notifications` is behind requireAuth only — but + `RequirePlayer` sends them out of the player portal, so the two + screens are mounted here as well. Same components, same API, + two paths; `lib/notificationPaths.js` is the one mapping. */} + } /> + } /> {/* Installed modules' admin pages, at /admin//…, already inside RequireAuth + AdminLayout. A module cannot supply its own auth wrapper — only an optional { roles }, which core applies as the @@ -252,7 +260,14 @@ export default function App() { } /> } /> } /> - } /> + {/* The inbox took `/account/notifications` in engagement Phase 7 + and the preferences screen moved under it. Content and + settings are different kinds of thing, and the plain word + belongs to the one a person means when they say it — which is + also what the bell in the header opens. The server's routes + split at the same place. */} + } /> + } /> {/* Installed modules' player-portal pages, at /player//…. This group's own routes are absolute (its layout route has no path), so the prefix is written here rather than inherited — the one diff --git a/client/src/api/client.js b/client/src/api/client.js index 6f01d54..7ff2d61 100644 --- a/client/src/api/client.js +++ b/client/src/api/client.js @@ -230,6 +230,25 @@ export const api = { // field, so clearing the last subscription must not become an absent key. setNotificationSubscriptions: (streams) => req('/auth/me/notifications/subscriptions', { method: 'PUT', body: { streams } }), + // Per-channel preferences (ENGAGEMENT.md Phase 3). A SPARSE update: only the + // (id, channel) pairs sent are written, so a screen managing one channel need + // not know what the others hold. Shipped with no surface at all until Phase 7. + notificationChannelPrefs: () => req('/auth/me/notifications/channels'), + setNotificationChannelPrefs: (prefs) => + req('/auth/me/notifications/channels', { method: 'PUT', body: { prefs } }), + // The in-app inbox (ENGAGEMENT.md Phase 7). `before` is a keyset cursor — the + // id of the last item on the previous page — not an offset: the list gains + // rows at the top while it is being read. + notifications: ({ limit, before, unread } = {}) => { + const qs = new URLSearchParams() + if (limit) qs.set('limit', String(limit)) + if (before) qs.set('before', String(before)) + if (unread) qs.set('unread', 'true') + return req(`/auth/me/notifications${withQs(qs.toString())}`) + }, + notificationsUnreadCount: () => req('/auth/me/notifications/unread-count'), + markNotificationRead: (id) => req(`/auth/me/notifications/${id}/read`, { method: 'POST' }), + markAllNotificationsRead: () => req('/auth/me/notifications/read-all', { method: 'POST' }), teamNotificationPrefs: () => req('/auth/me/notifications/teams'), setTeamNotificationPrefs: (teams) => req('/auth/me/notifications/teams', { method: 'PUT', body: { teams } }), diff --git a/client/src/components/NotificationBell.jsx b/client/src/components/NotificationBell.jsx new file mode 100644 index 0000000..efec630 --- /dev/null +++ b/client/src/components/NotificationBell.jsx @@ -0,0 +1,353 @@ +import { useCallback, useEffect, useRef, useState } from 'react' +import { Link, useLocation, useNavigate } from 'react-router-dom' +import { useAuth } from '../contexts/AuthContext.jsx' +import { api } from '../api/client.js' +import { inboxPath } from '../lib/notificationPaths.js' + +// The in-app inbox's header surface (ENGAGEMENT.md Phase 7): a bell with an +// unread badge, and a panel with the most recent items. +// +// **The badge is polled, not pushed**, and the reason is that there is nothing +// to push over. The site's two SSE streams are the shard's; neither is +// per-user, and adding a third authenticated stream to carry an integer would +// mean one open connection per signed-in tab for the rest of the deployment's +// life. A minute-granular badge on a page somebody is already looking at is the +// same answer for a fraction of that. The poll pauses while the tab is hidden — +// a background tab has nobody to show a badge to — and refreshes the moment it +// comes back, which is also the moment it would be most wrong. +// +// **The panel shows a handful and links out.** Paging belongs on the page; a +// dropdown that scrolls is a list in the wrong place. +// +// Dismissal follows `NavDropdown`'s contract exactly — Escape closes and +// returns focus, an outside `mousedown` closes, navigating closes — because +// this sits beside it in the same header and two menus that dismiss differently +// is a bug nobody files. + +const POLL_MS = 60_000 +const PANEL_ITEMS = 6 + +function BellIcon({ size = 17 }) { + return ( + + ) +} + +// "3m", "4h", "6d" — a relative stamp, because the only question a reader has +// about an inbox item's time is how fresh it is. +function ago(iso) { + const then = new Date(iso).getTime() + if (!Number.isFinite(then)) return '' + const secs = Math.max(0, Math.round((Date.now() - then) / 1000)) + if (secs < 60) return 'now' + if (secs < 3600) return `${Math.floor(secs / 60)}m` + if (secs < 86400) return `${Math.floor(secs / 3600)}h` + return `${Math.floor(secs / 86400)}d` +} + +export default function NotificationBell() { + const { user } = useAuth() + const [unread, setUnread] = useState(0) + const [items, setItems] = useState([]) + const [open, setOpen] = useState(false) + const [error, setError] = useState('') + const wrapRef = useRef(null) + const triggerRef = useRef(null) + const location = useLocation() + const navigate = useNavigate() + + // Every read here swallows its failure. A count that could not be fetched is + // a bell with no badge, which is what a bell with nothing to report looks + // like anyway — the alternative is an error banner in the site header for a + // number nobody asked for. + const refreshCount = useCallback(async () => { + if (!user) return + try { + const res = await api.notificationsUnreadCount() + setUnread(res.unread || 0) + } catch { + /* leave the badge as it was */ + } + }, [user]) + + useEffect(() => { + if (!user) return undefined + refreshCount() + const timer = setInterval(() => { + if (document.visibilityState === 'visible') refreshCount() + }, POLL_MS) + const onVisible = () => { + if (document.visibilityState === 'visible') refreshCount() + } + document.addEventListener('visibilitychange', onVisible) + return () => { + clearInterval(timer) + document.removeEventListener('visibilitychange', onVisible) + } + }, [user, refreshCount]) + + // The panel's items are fetched when it opens, never kept warm: a list nobody + // has asked to see is a request per minute for content nobody is reading. + const load = useCallback(async () => { + setError('') + try { + const res = await api.notifications({ limit: PANEL_ITEMS }) + setItems(res.items || []) + setUnread(res.unread || 0) + } catch (err) { + setError(err.message || 'Could not load notifications') + } + }, []) + + useEffect(() => setOpen(false), [location.pathname]) + + useEffect(() => { + if (!open) return undefined + const onKey = (e) => { + if (e.key !== 'Escape') return + setOpen(false) + triggerRef.current?.focus() + } + const onOutside = (e) => { + if (!wrapRef.current?.contains(e.target)) setOpen(false) + } + document.addEventListener('keydown', onKey) + document.addEventListener('mousedown', onOutside) + return () => { + document.removeEventListener('keydown', onKey) + document.removeEventListener('mousedown', onOutside) + } + }, [open]) + + if (!user) return null + + const toggle = () => { + const next = !open + setOpen(next) + if (next) load() + } + + // Opening an item marks it read and then goes where it points. The mark is + // awaited rather than fired off, so the badge the next screen renders is the + // one this click produced; a failed mark still navigates, because the item's + // link is the thing the user asked for. + const openItem = async (item) => { + setOpen(false) + if (!item.read) { + try { + const res = await api.markNotificationRead(item.id) + setUnread(res.unread ?? Math.max(0, unread - 1)) + } catch { + /* the link still works */ + } + } + navigate(item.url || inboxPath(user)) + } + + const markAll = async () => { + try { + await api.markAllNotificationsRead() + setUnread(0) + setItems((list) => list.map((i) => ({ ...i, read: true }))) + } catch (err) { + setError(err.message || 'Could not mark them read') + } + } + + return ( +
+ + + {open && ( +
+
+ + Notifications + + {unread > 0 && ( + + )} +
+ + {error && ( +

+ {error} +

+ )} + + {!error && items.length === 0 && ( +

+ Nothing here yet. +

+ )} + + {items.map((item) => ( + + ))} + + setOpen(false)} + className="sans" + style={{ + display: 'block', + marginTop: 4, + padding: '8px 10px', + borderTop: '1px solid var(--line-soft)', + fontSize: '0.8rem', + color: 'var(--accent)', + textDecoration: 'none', + }} + > + See all notifications → + +
+ )} +
+ ) +} diff --git a/client/src/components/SiteHeader.jsx b/client/src/components/SiteHeader.jsx index 84706f0..21eee19 100644 --- a/client/src/components/SiteHeader.jsx +++ b/client/src/components/SiteHeader.jsx @@ -5,6 +5,7 @@ import BrandLogo from './BrandLogo.jsx' import { useAuth } from '../contexts/AuthContext.jsx' import { useSite } from '../contexts/SiteContext.jsx' import NavDropdown from './NavDropdown.jsx' +import NotificationBell from './NotificationBell.jsx' import { buildPublicNav, pruneNav } from '../lib/navOverrides.js' import { parseJsonSetting } from '../lib/settingsJson.js' import { withModuleNav } from '../modules/nav.js' @@ -107,6 +108,10 @@ export default function SiteHeader() { ), )} + {/* Renders nothing when signed out, so the header keeps its shape for + a visitor. It is here rather than only in the portal because an + inbox item is worth seeing from the page you are already on. */} + {!loading && } {!loading && ( !!(user && user.role && user.role !== 'player') + +/** The inbox — what the bell opens. */ +export const inboxPath = (user) => (isStaff(user) ? '/admin/notifications' : '/account/notifications') + +/** The per-channel preferences screen. */ +export const notificationSettingsPath = (user) => + isStaff(user) ? '/admin/notifications/settings' : '/account/notifications/settings' diff --git a/client/src/modules/TeamNotifyToggle.jsx b/client/src/modules/TeamNotifyToggle.jsx index e48c8e6..b473b66 100644 --- a/client/src/modules/TeamNotifyToggle.jsx +++ b/client/src/modules/TeamNotifyToggle.jsx @@ -96,7 +96,7 @@ export default function TeamNotifyToggle({ externalId, moduleId }) { {/* The one link off this control, because "mute" is a blunt answer to a question the account screen asks properly — which streams, and whether email is on at all. */} - All notification settings + All notification settings ) } diff --git a/client/src/routes/admin/AdminLayout.jsx b/client/src/routes/admin/AdminLayout.jsx index 10a4a24..84339e2 100644 --- a/client/src/routes/admin/AdminLayout.jsx +++ b/client/src/routes/admin/AdminLayout.jsx @@ -2,6 +2,7 @@ import { useEffect, useMemo, useState } from 'react' import { NavLink, Outlet, useNavigate, useLocation } from 'react-router-dom' import MoonDot from '../../components/MoonDot.jsx' import BrandLogo from '../../components/BrandLogo.jsx' +import NotificationBell from '../../components/NotificationBell.jsx' import { useAuth } from '../../contexts/AuthContext.jsx' import { useSite } from '../../contexts/SiteContext.jsx' import { applyNavOverrides } from '../../lib/navOverrides.js' @@ -43,6 +44,7 @@ const IconKey = () => const IconPulse = () => const IconUser = () => +const IconBell = () => const IconNav = () => const IconPalette = () => const IconModules = () => @@ -129,6 +131,11 @@ export const NAV = [ }, { items: [ + // No `end`: `allowedPathsFor` turns an `end` row into an EXACT match, so + // marking this one exact would leave `/admin/notifications/settings` + // outside the allowlist and bounce a staff member off their own + // preferences screen. The row covering its sub-routes is the point. + { to: '/admin/notifications', label: 'Notifications', icon: IconBell }, { to: '/admin/account', label: 'Account', icon: IconUser }, ], }, @@ -177,6 +184,8 @@ const TITLES = { '/admin/users': 'Users', '/admin/invites': 'Invites', '/admin/account': 'Account Security', + '/admin/notifications': 'Notifications', + '/admin/notifications/settings': 'Notification settings', '/admin/engagement/rules': 'Engagement Rules', '/admin/engagement/audiences': 'Engagement Audiences', '/admin/engagement/templates': 'Message Templates', @@ -443,6 +452,11 @@ export default function AdminLayout() { {title}
+ {/* Staff have an inbox like anyone else — `/auth/me/notifications` + is role-agnostic — and `RequirePlayer` keeps them out of the + player portal, so without this the one place they spend their + time is the one place the bell is missing. */} + View site → diff --git a/client/src/routes/player/PlayerInbox.jsx b/client/src/routes/player/PlayerInbox.jsx new file mode 100644 index 0000000..3028126 --- /dev/null +++ b/client/src/routes/player/PlayerInbox.jsx @@ -0,0 +1,264 @@ +import { useCallback, useEffect, useState } from 'react' +import { Link, useNavigate } from 'react-router-dom' +import { Loading, ErrorState } from '../../components/PageState.jsx' +import { api } from '../../api/client.js' +import { useAuth } from '../../contexts/AuthContext.jsx' +import { notificationSettingsPath, inboxPath } from '../../lib/notificationPaths.js' + +// The in-app inbox (ENGAGEMENT.md Phase 7), at `/account/notifications`. +// +// **It took that path from the preferences screen, which moved to +// `/account/notifications/settings`.** The two are different kinds of thing — +// one is content addressed to this person, the other is how they would like to +// be reached — and the word "notifications" belongs to the first: it is what a +// person means when they say it, and what the bell in the header opens. The +// server's routes make the same split at the same place. +// +// Everything a row can carry is TEXT. `body` is stored as the text part of the +// in-app template's blocks and rendered with `white-space: pre-line`, never as +// markup; `url` is site-relative by the time it is stored, checked against the +// same character class `pageUrlTemplate` uses. So there is no sanitizing to do +// here — there is nothing on this screen that could be markup. + +const PAGE = 30 + +function ago(iso) { + const then = new Date(iso).getTime() + if (!Number.isFinite(then)) return '' + const secs = Math.max(0, Math.round((Date.now() - then) / 1000)) + if (secs < 60) return 'just now' + if (secs < 3600) return `${Math.floor(secs / 60)} min ago` + if (secs < 86400) return `${Math.floor(secs / 3600)} h ago` + if (secs < 30 * 86400) return `${Math.floor(secs / 86400)} d ago` + return new Date(iso).toLocaleDateString() +} + +function Item({ item, onOpen, onMark }) { + const body = ( + <> +
+ + {item.title} + + {ago(item.createdAt)} +
+ {item.body && ( +

+ {item.body} +

+ )} + + ) + + return ( +
  • +
    + {item.url ? ( + + ) : ( + body + )} +
    + {!item.read && ( + + )} +
    • + ) +} + +export default function PlayerInbox() { + const [items, setItems] = useState([]) + const [unread, setUnread] = useState(0) + const [hasMore, setHasMore] = useState(false) + const [unreadOnly, setUnreadOnly] = useState(false) + const [loading, setLoading] = useState(true) + const [busy, setBusy] = useState(false) + const [error, setError] = useState('') + const navigate = useNavigate() + const { user } = useAuth() + + const load = useCallback(async (only) => { + setLoading(true) + setError('') + try { + const res = await api.notifications({ limit: PAGE, unread: only }) + setItems(res.items || []) + setHasMore(!!res.hasMore) + setUnread(res.unread || 0) + } catch (err) { + setError(err.message || 'Could not load your notifications') + } finally { + setLoading(false) + } + }, []) + + useEffect(() => { load(unreadOnly) }, [load, unreadOnly]) + + // The cursor is the last item's id, not a page number: the list gains rows at + // the top while it is being read, and an offset under those conditions repeats + // or skips items. + const more = async () => { + if (!items.length) return + setBusy(true) + try { + const res = await api.notifications({ + limit: PAGE, + before: items[items.length - 1].id, + unread: unreadOnly, + }) + setItems((list) => [...list, ...(res.items || [])]) + setHasMore(!!res.hasMore) + } catch (err) { + setError(err.message || 'Could not load more') + } finally { + setBusy(false) + } + } + + const mark = async (item) => { + try { + const res = await api.markNotificationRead(item.id) + setUnread(res.unread ?? Math.max(0, unread - 1)) + // Filtered to unread, a marked item leaves the list; unfiltered it stays + // and goes quiet. Either way the list matches what it says it is showing. + setItems((list) => + unreadOnly + ? list.filter((i) => i.id !== item.id) + : list.map((i) => (i.id === item.id ? { ...i, read: true } : i)), + ) + } catch (err) { + setError(err.message || 'Could not mark it read') + } + } + + const open = async (item) => { + if (!item.read) await mark(item) + if (item.url) navigate(item.url) + } + + const markAll = async () => { + setBusy(true) + try { + await api.markAllNotificationsRead() + setUnread(0) + setItems((list) => (unreadOnly ? [] : list.map((i) => ({ ...i, read: true })))) + } catch (err) { + setError(err.message || 'Could not mark them read') + } finally { + setBusy(false) + } + } + + if (loading) return + if (error && !items.length) return + + return ( +
    +
    +

    + {unread > 0 ? `${unread} unread` : 'Everything is read.'}{' '} + + Notification settings + +

    +
    + + +
    +
    + + {error && ( +

    {error}

    + )} + + {items.length === 0 ? ( +

    + {unreadOnly + ? 'Nothing unread.' + : 'Nothing here yet. Anything the shard or your guilds want to tell you will show up on this page.'} +

    + ) : ( +
      + {items.map((item) => ( + + ))} +
    + )} + + {hasMore && ( + + )} +
    + ) +} diff --git a/client/src/routes/player/PlayerNotifications.jsx b/client/src/routes/player/PlayerNotifications.jsx index ad9ac9b..1eadd5a 100644 --- a/client/src/routes/player/PlayerNotifications.jsx +++ b/client/src/routes/player/PlayerNotifications.jsx @@ -1,8 +1,15 @@ import { useCallback, useEffect, useState } from 'react' +import { Link } from 'react-router-dom' import { Loading, ErrorState } from '../../components/PageState.jsx' import { api } from '../../api/client.js' +import { useAuth } from '../../contexts/AuthContext.jsx' +import { inboxPath } from '../../lib/notificationPaths.js' -// The account's notification settings (TEAMS.md §6.3/§6.4, phase 6). +// The account's notification settings (TEAMS.md §6.3/§6.4, phase 6; the +// per-channel matrix is ENGAGEMENT.md Phase 3, surfaced in Phase 7). +// +// **It moved to `/account/notifications/settings` in Phase 7**, because the +// inbox took the plain path. See `PlayerInbox.jsx`. // // **This screen did not exist before phase 6, and that was the phase's first // finding.** §6.3 says the per-Team mute list is "surfaced under the existing @@ -18,6 +25,14 @@ import { api } from '../../api/client.js' // thing to be told about, then which Teams, then whether any of it should reach a // mailbox. +// The three modes a per-channel preference can take, labelled for a person. The +// set a given channel actually offers comes from its `supportsDigest` flag. +const MODES = [ + { value: 'off', label: 'Off' }, + { value: 'instant', label: 'As it happens' }, + { value: 'digest', label: 'Daily digest' }, +] + const EMAIL_MODES = [ { value: 'off', label: 'No email' }, { value: 'digest', label: 'Daily digest' }, @@ -48,48 +63,125 @@ function Note({ msg, error }) { ) } -// ── What to be told about ────────────────────────────────────────────────── +// ── What to be told about, and how ───────────────────────────────────────── +// +// **This replaced the push-only checkbox list, and it is a strict superset of +// it.** `GET /auth/me/notifications/channels` returns every subscribable id — +// every push stream and every event trigger, one namespace (§7.2) — with the +// EFFECTIVE mode on each channel that applies. A trigger with nothing +// registered to push it simply has no push cell; core does not have to explain +// which kind of id a row is, and neither does a reader. +// +// The old whole-set endpoints are untouched and are now this surface's push +// projection: the shipped Android app keeps its wire shape, and a `push` entry +// written here is mirrored back into `notification_subscriptions` server-side. +// +// The update is SPARSE: only the cells that changed are sent. That is what lets +// this screen manage three channels without a whole-set PUT that could clobber +// a preference a newer client set. -function Streams({ streams, subscribed, onSave, busy, msg, error }) { - const [set, setSet] = useState(() => new Set(subscribed)) - useEffect(() => { setSet(new Set(subscribed)) }, [subscribed]) +function Channels({ channels, items, onSave, busy, msg, error }) { + const [edits, setEdits] = useState({}) + useEffect(() => setEdits({}), [items]) - const toggle = (id) => { - const next = new Set(set) - if (next.has(id)) next.delete(id) - else next.add(id) - setSet(next) + const key = (id, channel) => `${id}|${channel}` + const modeOf = (item, channel) => edits[key(item.id, channel)] ?? item.modes[channel] + const set = (id, channel, mode) => setEdits((e) => ({ ...e, [key(id, channel)]: mode })) + + // A channel that supports digest offers three modes; one that does not offers + // two. Read off the registry rather than hardcoded, so a channel added later + // shows the right options without touching this file. + const modesFor = (c) => (c.supportsDigest ? MODES : MODES.filter((m) => m.value !== 'digest')) + + const changed = Object.entries(edits).filter(([k, mode]) => { + const [id, channel] = k.split('|') + const item = items.find((i) => i.id === id) + return item && item.modes[channel] !== mode + }) + + const save = () => + onSave( + changed.map(([k, mode]) => { + const [id, channel] = k.split('|') + return { id, channel, mode } + }), + ) + + if (items.length === 0) { + return ( +
    +

    + There is nothing to configure yet. +

    +
    + ) } - const team = streams.filter((s) => isTeamStream(s.id)) - const rest = streams.filter((s) => !isTeamStream(s.id)) + const team = items.filter((i) => isTeamStream(i.id)) + const rest = items.filter((i) => !isTeamStream(i.id)) - const row = (s) => ( - - ) + const rows = (list) => + list.map((item) => ( + + + {item.label} + {item.description && ( + {item.description} + )} + + {channels.map((c) => ( + + {item.channels.includes(c.id) ? ( + + ) : ( + // Not "off" — a dash. Nothing is registered to push this id, so + // there is no preference to hold, and an `off` select would invite + // somebody to switch on a channel that has no sender behind it. + — + )} + + ))} + + )) return (
    -
    {rest.map(row)}
    - {team.length > 0 && ( - <> -

    - Teams -

    -
    {team.map(row)}
    - - )} +
    + + + + + {channels.map((c) => ( + + ))} + + + + {rows(rest)} + {team.length > 0 && ( + + + + )} + {rows(team)} + +
    Notification{c.label}
    + Teams — set site-wide here, then per team below +
    +
    -
    @@ -176,27 +268,29 @@ function Teams({ teams, onSave, busy, msg, error }) { // ── Page ─────────────────────────────────────────────────────────────────── export default function PlayerNotifications() { + const { user } = useAuth() const [loading, setLoading] = useState(true) const [error, setError] = useState('') - const [streams, setStreams] = useState([]) - const [subscribed, setSubscribed] = useState([]) + const [channels, setChannels] = useState([]) + const [items, setItems] = useState([]) const [teams, setTeams] = useState([]) - const [saving, setSaving] = useState({ streams: false, teams: false }) - const [notes, setNotes] = useState({ streams: '', teams: '', streamsError: '', teamsError: '' }) + const [saving, setSaving] = useState({ channels: false, teams: false }) + const [notes, setNotes] = useState({ channels: '', teams: '', channelsError: '', teamsError: '' }) const load = useCallback(async () => { setLoading(true) try { - // Three reads in parallel: the catalog is boot-fixed, the subscriptions and - // the Team list are this user's. None depends on another. - const [cat, subs, prefs] = await Promise.all([ - api.notificationStreams(), - api.notificationSubscriptions(), + // Two reads in parallel, where there used to be three: the per-channel + // surface already carries the catalog and this user's effective modes, so + // the streams+subscriptions pair it replaced is one request fewer as well + // as one concept fewer. + const [prefs, teamPrefs] = await Promise.all([ + api.notificationChannelPrefs(), api.teamNotificationPrefs(), ]) - setStreams(cat.streams || []) - setSubscribed(subs.streams || []) - setTeams(prefs.teams || []) + setChannels(prefs.channels || []) + setItems(prefs.items || []) + setTeams(teamPrefs.teams || []) setError('') } catch { setError('Could not load your notification settings.') @@ -207,17 +301,23 @@ export default function PlayerNotifications() { useEffect(() => { load() }, [load]) - const saveStreams = useCallback(async (ids) => { - setSaving((s) => ({ ...s, streams: true })) - setNotes((n) => ({ ...n, streams: '', streamsError: '' })) + const saveChannels = useCallback(async (prefs) => { + if (prefs.length === 0) return + setSaving((s) => ({ ...s, channels: true })) + setNotes((n) => ({ ...n, channels: '', channelsError: '' })) try { - const { streams: stored } = await api.setNotificationSubscriptions(ids) - setSubscribed(stored || []) - setNotes((n) => ({ ...n, streams: 'Saved.' })) + // The endpoint echoes the FULL stored state back, not just what was sent — + // so an entry it dropped (an unknown id, a channel that does not apply, a + // mode that channel will not take) is visible here as a cell that did not + // move, rather than as a screen that claims a save it did not make. + const stored = await api.setNotificationChannelPrefs(prefs) + setChannels(stored.channels || []) + setItems(stored.items || []) + setNotes((n) => ({ ...n, channels: 'Saved.' })) } catch { - setNotes((n) => ({ ...n, streamsError: 'Could not save that.' })) + setNotes((n) => ({ ...n, channelsError: 'Could not save that.' })) } finally { - setSaving((s) => ({ ...s, streams: false })) + setSaving((s) => ({ ...s, channels: false })) } }, []) @@ -245,16 +345,17 @@ export default function PlayerNotifications() { return (

    - Choose what you are told about, and how. Nothing here is on by default except team - notifications to the app, which you can mute per team below. + Choose what you are told about, and how. Email and push are off until you switch them on; + items on the site go to your notification inbox, + which you can turn off here per notification.

    - const IconShield = () => const IconBell = () => +// The settings row's own icon: a bell would make the two rows read as the same +// destination twice, which is exactly the confusion the split was meant to end. +const IconBellGear = () => // Exported because Admin -> Navigation edits this list. It stays declared here; // the editor may only relabel, reorder and hide what it finds (§7). No CORE row @@ -48,7 +52,8 @@ const IconBell = () => {title} - - ← Site - +
    diff --git a/client/src/routes/player/Unsubscribe.jsx b/client/src/routes/player/Unsubscribe.jsx index 520c781..b7ed9ec 100644 --- a/client/src/routes/player/Unsubscribe.jsx +++ b/client/src/routes/player/Unsubscribe.jsx @@ -54,14 +54,14 @@ export default function Unsubscribe() {

    This muted the team rather than switching off your account’s email, so your other teams are unaffected. You can turn it back on any time under{' '} - notification settings. + notification settings.

    )} {state === 'failed' && (

    We could not reach the site to record that. Please try the link again, or change the - setting yourself under notification settings. + setting yourself under notification settings.

    )} diff --git a/client/test/notificationPaths.test.js b/client/test/notificationPaths.test.js new file mode 100644 index 0000000..d0f69bd --- /dev/null +++ b/client/test/notificationPaths.test.js @@ -0,0 +1,42 @@ +// ── Where each account's notification screens live ───────────────────────── +// +// ENGAGEMENT.md Phase 7. Three assertions for a nine-line module, because the +// defect they pin was invisible to every other check: `/auth/me/notifications` +// is role-agnostic (behind `requireAuth` only, like the rest of `/auth/me`), so +// the server, the tests and the API all agreed a staff member had an inbox — +// and on the web they could not reach it, because `RequirePlayer` sends anyone +// who is not a player back out of `/account`. The bell pointed at a redirect. +// +// Found in the Phase 7 rig, signed in as an admin. What stops it coming back is +// this file plus the two admin routes it maps onto. + +import { test } from 'node:test' +import assert from 'node:assert/strict' + +import { isStaff, inboxPath, notificationSettingsPath } from '../src/lib/notificationPaths.js' + +test('a player gets the portal paths', () => { + const user = { role: 'player' } + assert.equal(isStaff(user), false) + assert.equal(inboxPath(user), '/account/notifications') + assert.equal(notificationSettingsPath(user), '/account/notifications/settings') +}) + +test('every non-player role gets the admin paths, not just admin', () => { + for (const role of ['admin', 'editor', 'moderator']) { + const user = { role } + assert.equal(isStaff(user), true, role) + assert.equal(inboxPath(user), '/admin/notifications', role) + assert.equal(notificationSettingsPath(user), '/admin/notifications/settings', role) + } +}) + +// The bell renders nothing when signed out, so these are never asked for a null +// user in practice — but a default that guessed "staff" would send a signed-out +// visitor at the admin area the moment that changed. +test('no user, or a user with no role, falls back to the player paths', () => { + for (const user of [null, undefined, {}, { role: '' }]) { + assert.equal(isStaff(user), false) + assert.equal(inboxPath(user), '/account/notifications') + } +}) diff --git a/server/db/schema.sql b/server/db/schema.sql index a56b5fc..840e1c2 100644 --- a/server/db/schema.sql +++ b/server/db/schema.sql @@ -1952,3 +1952,45 @@ CREATE TABLE IF NOT EXISTS engagement_digest_state ( INSERT IGNORE INTO engagement_digest_state (user_id, channel, scope_key, last_digest_at) SELECT user_id, 'email', CONCAT('team:', team_id), last_digest_at FROM team_notification_prefs; + +-- ── The in-app channel (ENGAGEMENT.md §4.5 G17 — Phase 7) ────────────────── + +-- The inbox. Core, game-agnostic, and the first sink core owns that CARRIES its +-- content: a push tickle deliberately holds none and an email leaves the +-- building, so this is the one place a message both belongs to this deployment +-- and can be read without a mailbox. +-- +-- `dedupe_key` is the acceptance criterion, expressed as an index rather than as +-- a check the writer has to remember: a replayed event, a retried outbox row and +-- a module calling `ctx.inbox.push` twice all reduce to the same INSERT IGNORE. +-- It is scoped to the USER (not to the rule and channel the outbox scopes by), +-- because one event may legitimately be two outbox rows for one person — a rule +-- spanning channels — and two inbox rows for it is one item shown twice. +-- Multiple NULLs are permitted by a UNIQUE index, which is what "this item does +-- not dedupe" means. +-- +-- `url` is stored RELATIVE only, validated with the character class +-- `pageUrlTemplate` and the engine's `url` variables already use: it ends up in +-- an href on a page a signed-in user is looking at, and `//evil.test/x` passes +-- every "is it rooted" check anyone writes by hand. +CREATE TABLE IF NOT EXISTS user_notifications ( + id BIGINT AUTO_INCREMENT PRIMARY KEY, + user_id INT NOT NULL, + trigger_id VARCHAR(96) NOT NULL, + title VARCHAR(300) NOT NULL, + body TEXT NULL, -- rendered by the inapp template, sanitized on write + url VARCHAR(500) NULL, -- relative only, validated like pageUrlTemplate + dedupe_key VARCHAR(190) NULL, + read_at DATETIME NULL, + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + CONSTRAINT fk_un_user FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE, + UNIQUE KEY uq_un_dedupe (user_id, dedupe_key), + -- Both of the two questions this table is asked: "what is in my inbox" (the + -- list, newest first) and "how many are unread" (the badge, on every page + -- load). A single index answers both because `read_at` is IS NULL in one and + -- unconstrained in the other, and `created_at` orders what is left. + INDEX idx_un_unread (user_id, read_at, created_at), + -- What the prune sweep queries. Without it the sweep is a table scan of every + -- notification this deployment has ever written. + INDEX idx_un_prune (created_at) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; diff --git a/server/routes.guards.json b/server/routes.guards.json index 0dc2568..08e139e 100644 --- a/server/routes.guards.json +++ b/server/routes.guards.json @@ -1614,6 +1614,28 @@ "validate" ] }, + { + "method": "GET", + "path": "/api/v1/auth/me/notifications", + "handlers": 5, + "gates": [ + "noindex", + "requireAuth", + "middleware", + "validate" + ] + }, + { + "method": "POST", + "path": "/api/v1/auth/me/notifications/:id/read", + "handlers": 3, + "gates": [ + "noindex", + "requireAuth", + "middleware", + "validate" + ] + }, { "method": "GET", "path": "/api/v1/auth/me/notifications/channels", @@ -1634,6 +1656,15 @@ "validate" ] }, + { + "method": "POST", + "path": "/api/v1/auth/me/notifications/read-all", + "handlers": 1, + "gates": [ + "noindex", + "requireAuth" + ] + }, { "method": "GET", "path": "/api/v1/auth/me/notifications/streams", @@ -1683,6 +1714,15 @@ "validate" ] }, + { + "method": "GET", + "path": "/api/v1/auth/me/notifications/unread-count", + "handlers": 1, + "gates": [ + "noindex", + "requireAuth" + ] + }, { "method": "GET", "path": "/api/v1/auth/me/sessions", diff --git a/server/routes.manifest.json b/server/routes.manifest.json index b4e6fab..20706a8 100644 --- a/server/routes.manifest.json +++ b/server/routes.manifest.json @@ -649,6 +649,14 @@ "method": "DELETE", "path": "/api/v1/auth/me/devices/:id" }, + { + "method": "GET", + "path": "/api/v1/auth/me/notifications" + }, + { + "method": "POST", + "path": "/api/v1/auth/me/notifications/:id/read" + }, { "method": "GET", "path": "/api/v1/auth/me/notifications/channels" @@ -657,6 +665,10 @@ "method": "PUT", "path": "/api/v1/auth/me/notifications/channels" }, + { + "method": "POST", + "path": "/api/v1/auth/me/notifications/read-all" + }, { "method": "GET", "path": "/api/v1/auth/me/notifications/streams" @@ -677,6 +689,10 @@ "method": "PUT", "path": "/api/v1/auth/me/notifications/teams" }, + { + "method": "GET", + "path": "/api/v1/auth/me/notifications/unread-count" + }, { "method": "GET", "path": "/api/v1/auth/me/sessions" diff --git a/server/src/engagement/coreChannels.js b/server/src/engagement/coreChannels.js index aa33dd6..59d3efd 100644 --- a/server/src/engagement/coreChannels.js +++ b/server/src/engagement/coreChannels.js @@ -17,6 +17,8 @@ const { registerDeliveryChannel } = require('./channels') const emailChannel = require('./emailChannel') +const pushChannel = require('./pushChannel') +const inappChannel = require('./inappChannel') const CHANNELS = [ { @@ -44,6 +46,10 @@ const CHANNELS = [ // there is nothing to roll up. Ten events are ten wakeups or one; either way // the app pulls the same inbox. supportsDigest: false, + // Phase 7: the oldest sink is the last to get a `deliver`, because until the + // inbox existed there was nothing for a content-free tickle to point at. + addressFor: pushChannel.addressFor, + deliver: pushChannel.deliver, }, { id: 'email', @@ -66,13 +72,25 @@ const CHANNELS = [ label: 'On the site', description: 'An item in your notification inbox on the website and in the app.', carriesContent: true, - // Opt-IN like the other two, and for a reason particular to this channel: the - // inbox does not exist until Phase 7. A default of 'instant' would mean every - // user is opted into a surface that has no rows and no screen, and the first - // thing Phase 7 shipped would be a backlog. Whether the inbox is opt-out once - // it is real is a Phase 7 decision with a live surface to look at. - defaultMode: 'off', + // **Opt-OUT, and the only one of the three that is** — settled by the org + // lead 2026-08-31, which is the Phase 7 decision this comment used to defer. + // + // The argument against a live default was never about in-app: it was that + // push wakes a device the user is holding and email leaves the building, so + // both must be asked for. An inbox item does neither. It is a row on a page + // the user chose to open, on this deployment, costing them one glance — and + // left at 'off' the surface would ship dead, because no rule could reach + // anyone until every user found a toggle for a channel they had never seen + // deliver anything. The backlog Phase 3 worried about cannot happen either: + // the table is empty at cutover, rules default to `enabled = 0`, and every + // rule carries a per-hour ceiling. + defaultMode: 'instant', + // Instant-only, and unlike push the reason is not that batching is + // meaningless — it is that the inbox IS the batch. A digest of inbox items + // is a list of things already sitting in a list. supportsDigest: false, + addressFor: inappChannel.addressFor, + deliver: inappChannel.deliver, }, ] diff --git a/server/src/engagement/engine.js b/server/src/engagement/engine.js index 5157854..6bfcd9b 100644 --- a/server/src/engagement/engine.js +++ b/server/src/engagement/engine.js @@ -42,6 +42,22 @@ const log = require('../utils/logger')('engagement') const HOUR_MS = 60 * 60 * 1000 +// Channels one of whose payloads can REFERENCE another's result, earliest first +// (Phase 7). Only one pair qualifies today: a push tickle's `ref` deep-links to +// the inbox row `inapp` writes, and the outbox is swept `ORDER BY due_at, id`, +// so enqueueing in-app first is what makes that ref resolve on the first pass +// rather than on a retry. Everything not named here keeps the operator's own +// order, which is the order the rules screen shows. +// +// It is an ordering, not a dependency: `pushChannel` treats a missing ref as +// null and the app pulls regardless, so a rule that names only push, or a row +// that gets retried out of sequence, is still correct. +const CHANNEL_ORDER = ['inapp'] +const channelRank = (id) => { + const i = CHANNEL_ORDER.indexOf(id) + return i === -1 ? CHANNEL_ORDER.length : i +} + /** * Which of a rule's channels are actually deliverable right now? * @@ -50,7 +66,10 @@ const HOUR_MS = 60 * 60 * 1000 * failing the rule: the other channels of that rule are still correct, and a * dropped one is visible in the log line below. */ -const liveChannels = (rule) => (rule.channels || []).filter((c) => channels.has(c)) +const liveChannels = (rule) => + (rule.channels || []) + .filter((c) => channels.has(c)) + .sort((a, b) => channelRank(a) - channelRank(b)) /** * The EFFECTIVE mode each candidate holds for (id, channel), given the event's @@ -267,4 +286,13 @@ async function dispatch(event, now = new Date()) { return summary } -module.exports = { dispatch, applyRule, applyCancellations, subscribedTo, effectiveModes, liveChannels, HOUR_MS } +module.exports = { + dispatch, + applyRule, + applyCancellations, + subscribedTo, + effectiveModes, + liveChannels, + CHANNEL_ORDER, + HOUR_MS, +} diff --git a/server/src/engagement/inappChannel.js b/server/src/engagement/inappChannel.js new file mode 100644 index 0000000..3a852a0 --- /dev/null +++ b/server/src/engagement/inappChannel.js @@ -0,0 +1,204 @@ +// ── The in-app DeliveryChannel: addressFor + deliver ─────────────────────── +// +// ENGAGEMENT.md Phase 7. The third channel to get behaviour, and the one whose +// "address" is not an address at all: the destination is the user's own row in +// this deployment's own table. `addressFor` still exists and still answers null, +// because the question it asks — *can this channel reach this user right now* — +// has a real answer here, and it is the same answer email's has: not if the +// account is no longer active. An outbox row can sit through a `delay_seconds` +// grace window, so a user banned between the emit and the send is exactly the +// case this catches. +// +// **What makes it different from email is what it does NOT have to do.** There +// is no transport, no relay to classify a failure for us, no unsubscribe link to +// mint per recipient, and no address to hash — an inbox item is addressed to a +// user id, and `engagement_sends.address_hash` exists to correlate a bounce that +// this channel cannot have. So `deliver` is two steps: render the template into +// the three columns, and insert. +// +// **It never throws**, for the reason `emailChannel` states: the worker reads a +// throw as a transient failure and retries five times, so an unrenderable +// template would become five identical failures in the send log instead of one +// honest terminal row. +// +// **A duplicate `dedupe_key` reports success.** The acceptance line calls it a +// no-op; from the recipient's side it is a delivery — they have the item — and +// recording `failed` for it would put a red row in the send log for the +// mechanism working exactly as designed. The detail says which it was. + +const rulesDb = require('../model/engagement/engagementRules.db') +const registries = require('../modules/registries') +const channelRegistry = require('./channels') +const inbox = require('../model/userNotifications/userNotifications.db') +const recipients = require('../model/engagement/engagementRecipients.db') +const templates = require('./templates') +const projection = require('./projection') +const log = require('../utils/logger')('engagement') + +// The template a rule renders through when it names none — §4.6.1 property 1's +// implementation for this channel, exactly as `notify.event` is for email. +const DEFAULT_TEMPLATE = 'inapp.event' + +/** + * Can this channel reach `userId`? + * + * Returns the shape every `addressFor` returns rather than a boolean, so the + * registry's contract stays one contract. The "address" is the user id as a + * string, which is the honest answer: this channel's destination is an account, + * and there is nothing else to name. + */ +const addressFor = async (userId) => { + const active = await recipients.filterActive([userId]) + return active.length ? { address: String(active[0]) } : null +} + +/** + * Render one event into an inbox item. Shared with `ctx.inbox.push`'s rule-less + * path only in spirit — that one is handed its title and body by the module and + * renders nothing. + */ +async function renderItem(triggerId, payload, templateKey) { + const values = projection.project(triggerId, payload || {}) + const rendered = await templates.renderInappByKey(templateKey, values) + if (!rendered) return null + if (rendered.missing.length) { + // Names only, never values — the rule every log line in this subsystem + // follows. An optional variable a trigger chose not to supply renders as + // nothing by design, so this is debug rather than a warning. + log.debug('template variables had no value', { key: templateKey, missing: rendered.missing }) + } + return rendered +} + +/** + * Deliver one claimed outbox row. + * + * @returns {Promise<{ok: boolean, retry?: boolean, detail?: string}>} + */ +async function deliver(row) { + try { + if (!(await addressFor(row.user_id))) { + // Terminal. A five-minute backoff does not un-ban an account, and writing + // the item anyway would put content in the inbox of somebody who is no + // longer allowed to open it. + return { ok: false, detail: 'this user can no longer be reached' } + } + + const rule = await rulesDb.getById(row.rule_id) + const key = (rule && rule.template_keys && rule.template_keys.inapp) || DEFAULT_TEMPLATE + + const rendered = await renderItem(row.trigger_id, row.payload, key) + if (!rendered) { + // Neither a usable row nor a shipped seed: the operator deleted a template + // a rule points at, which the admin surface refuses with a 409, so reaching + // here means it happened out of band. Terminal, and it names the key. + return { ok: false, detail: `no template and no shipped default for "${key}"` } + } + + const { inserted } = await inbox.insert({ + userId: row.user_id, + triggerId: row.trigger_id, + title: rendered.title, + body: rendered.body, + url: rendered.url, + dedupeKey: row.dedupe_key || null, + }) + + // `transport` is left absent rather than invented. The column means "which + // implementation of this channel delivered it", and this channel has one + // sink by construction — a value there would be a name nothing else uses. + return inserted + ? { ok: true } + : { ok: true, detail: 'already in this inbox (duplicate dedupe key)' } + } catch (err) { + log.error('in-app delivery failed', { outbox: row.id, message: err.message }) + return { ok: false, detail: `delivery error: ${err.message}` } + } +} + +// ── The rule-less sink: ctx.inbox.push (§5.1) ────────────────────────────── +// +// A module writing the inbox directly, with no trigger declaration to project +// from, no rule to pick a template, and no audience to resolve. It exists for +// the cases a rule cannot express — something that concerns exactly one person +// and needs no operator configuration to be worth telling them about. +// +// **It respects the user's in-app preference where there is one to respect** +// (settled by the org lead 2026-08-31). If `triggerId` names a REGISTERED +// trigger, the user's effective mode for it decides, and 'off' drops the write: +// a toggle somebody switched off on the preferences screen must not be walkable +// around by the module that owns the trigger behind it. If it names nothing +// registered there is no toggle, nothing on any screen to have switched off, and +// the item is written — refusing it would make the sink useless for the one job +// it has while protecting a preference that does not exist. +// +// Scoped preferences are deliberately not consulted: a scope is a property of an +// EVENT (`team:12`), and a caller with no trigger declaration has no scope to +// name. The engine's path, which does, still applies them. +// +// Fire-and-forget, never throws, never rejects — `ctx.teams.activity.push`'s +// posture, for its reason: this is called from inside a game-event handler and a +// storage problem of core's must not become the module's control flow. + +// user_notifications.title. Truncated rather than refused: a module that built a +// long title has still said something worth showing. +const MAX_TITLE = 300 +// user_notifications.body is TEXT; this is a sanity bound, not the column's. +const MAX_BODY = 4000 + +/** + * Write one item on a module's behalf. + * + * @param {string} moduleId bound by the loader, never taken from the arguments + * @param {number} userId + * @param {{triggerId: string, title: string, body?: string, url?: string, dedupeKey?: string}} item + * @returns {Promise<{written: boolean, reason?: string}>} for tests; the loader + * discards it, because a module has nothing correct to do with it. + */ +async function pushDirect(moduleId, userId, item = {}) { + try { + const uid = Number(userId) + if (!Number.isInteger(uid) || uid <= 0) return { written: false, reason: 'invalid user id' } + + const triggerId = String(item.triggerId || '').trim() + const title = String(item.title || '').trim().slice(0, MAX_TITLE) + if (!triggerId || !title) return { written: false, reason: 'triggerId and title are required' } + + // The declaration is consulted for ONE thing — whether a preference for this + // id exists — and not to validate a payload: there is no payload here, only + // the three strings the module composed itself. + if (registries.eventTrigger(triggerId)) { + const stored = await recipients.storedModes([uid], triggerId, 'inapp') + const mode = stored.get(uid) ?? channelRegistry.defaultMode('inapp') + if (mode !== 'instant') return { written: false, reason: 'the user has this switched off' } + } + + if (!(await addressFor(uid))) return { written: false, reason: 'this user can no longer be reached' } + + // Same relative-only rule the rendered path applies, and for the same reason: + // this string ends up in an href on a page a signed-in user is looking at. + const url = item.url ? templates.relativeUrl(item.url, templates.baseUrl()) : null + if (item.url && !url) { + log.warn('ctx.inbox.push dropped an off-site url', { module: moduleId, trigger: triggerId }) + } + + const body = item.body ? String(item.body).slice(0, MAX_BODY) : null + const { inserted } = await inbox.insert({ + userId: uid, + triggerId, + title, + // A module supplies data, never markup (§4.6.2's security posture). The + // body is stored as the text it claims to be and every surface renders it + // as text, so there is no markup to sanitize and none to be trusted. + body, + url, + dedupeKey: item.dedupeKey ? String(item.dedupeKey).slice(0, 190) : null, + }) + return { written: inserted, reason: inserted ? undefined : 'duplicate dedupe key' } + } catch (err) { + log.error('ctx.inbox.push failed', { module: moduleId, message: err.message }) + return { written: false, reason: err.message } + } +} + +module.exports = { addressFor, deliver, renderItem, pushDirect, DEFAULT_TEMPLATE } diff --git a/server/src/engagement/pushChannel.js b/server/src/engagement/pushChannel.js new file mode 100644 index 0000000..90fef88 --- /dev/null +++ b/server/src/engagement/pushChannel.js @@ -0,0 +1,91 @@ +// ── The push DeliveryChannel: addressFor + deliver ───────────────────────── +// +// ENGAGEMENT.md Phase 7. Push is the channel that has existed longest and had a +// `deliver` last, because until this phase there was nothing for a tickle to +// point AT: `{ stream, ref }` carries no content by design, so a rule firing on +// push before the inbox existed would have woken a phone to pull a screen that +// had nothing on it. +// +// **The tickle invariant is the whole of this file's security posture.** What +// leaves the server is the stream id and an opaque ref, never a title, never a +// body, never the payload — `carriesContent: false` on the registration is the +// declaration and this is the implementation. ntfy is treated as an untrusted +// relay, so a leaked topic must reveal nothing but that *something* happened; +// the app then pulls the real item over the authenticated, ownership-checked +// inbox API. Every claim in that paragraph is one `pushDispatch` already makes, +// which is why delivery here is a call into it rather than a second publisher. +// +// **`ref` points at the inbox row when there is one, and is null otherwise.** +// A rule spanning `inapp` and `push` enqueues both, and `liveChannels` orders +// `inapp` first precisely so the row exists by the time this runs — but that is +// an optimisation, not a guarantee: the two rows are independent, either can be +// retried, and a push-only rule has no inbox row at all. So the ref is a HINT. +// The app's contract (docs/android/PLAN.md §11, Phase 8) is wake-and-pull; a +// client that renders the ref instead of pulling is a client that will show +// nothing the first time a retry reorders these two rows. + +const inbox = require('../model/userNotifications/userNotifications.db') +const recipients = require('../model/engagement/engagementRecipients.db') +const pushDispatch = require('../utils/pushDispatch') +const log = require('../utils/logger')('engagement') + +/** + * Can this channel reach `userId`? + * + * Active account only, the same re-check `emailChannel` and `inappChannel` make + * for the same reason (a row can sit through a `delay_seconds` window). It does + * NOT check for a registered device: whether any endpoint is subscribed is the + * question `publishToUsers` answers in its own query, and asking it twice would + * mean two different definitions of "reachable" that could disagree. + */ +const addressFor = async (userId) => { + const active = await recipients.filterActive([userId]) + return active.length ? { address: String(active[0]) } : null +} + +/** + * Deliver one claimed outbox row. + * + * @returns {Promise<{ok: boolean, retry?: boolean, transport?: string, detail?: string}>} + */ +async function deliver(row) { + try { + if (!(await addressFor(row.user_id))) { + return { ok: false, detail: 'this user can no longer be reached' } + } + + // Best effort, and it fails to null rather than to an error: no dedupe key, + // no in-app row for it, or an inapp row this rule never enqueued all mean + // the same thing to the app — wake up and pull. + let ref = null + try { + const item = await inbox.findByDedupe(row.user_id, row.dedupe_key) + if (item) ref = `notification:${item.id}` + } catch (err) { + log.debug('could not resolve a push ref', { outbox: row.id, message: err.message }) + } + + // The stream id IS the trigger id — §7.2's one namespace, settled in Phase 2. + // A push stream and an event trigger share a name space, so the app's + // existing `{ stream }` switch keeps working for an engagement rule without + // learning a second vocabulary. + await pushDispatch.publishToUsers(row.trigger_id, { ref, userIds: [row.user_id] }) + + // **Success here means "handed to the relay", and the send log must not + // claim more than that.** `publishToUsers` resolves whether it found a + // subscribed device or none at all, and a tickle is fire-and-forget over + // HTTP to a relay that owes us no receipt. Retrying on "we are not sure" + // would mean five wakeups for one event, which is worse than one uncertain + // log line — so this is the one channel whose 'sent' is weaker than email's, + // and saying so in the detail is how an operator reading G15 finds that out. + return { ok: true, transport: 'unifiedpush', detail: 'tickle published' } + } catch (err) { + // pushDispatch never throws, so reaching here is a programming error rather + // than a relay being down. Terminal for that reason: retrying a bug is five + // identical rows in the send log. + log.error('push delivery failed', { outbox: row.id, message: err.message }) + return { ok: false, detail: `delivery error: ${err.message}` } + } +} + +module.exports = { addressFor, deliver } diff --git a/server/src/engagement/templateSeeds.js b/server/src/engagement/templateSeeds.js index 745d641..ea44d64 100644 --- a/server/src/engagement/templateSeeds.js +++ b/server/src/engagement/templateSeeds.js @@ -260,19 +260,33 @@ const SEEDS = [ name: 'On-site notification', channel: 'inapp', protected: false, - seedVersion: 1, + // **seedVersion 2, and the bump is a correction rather than an improvement.** + // Phase 5a wrote this template before the channel that renders it existed, and + // named its variables `body` and `url` — names NOTHING supplies. A trigger + // declares domain names (`teamName`, `threadTitle`), and `projection.project` + // fills the gaps with the STRUCTURAL ones the generic seeds use: `title`, + // `intro`, `actionUrl`. So every rendering of this template would have found + // `body` and `url` missing and produced a title and nothing else. Renamed to + // the vocabulary `notify.event` uses, which is the same property stated once: + // a new trigger must render with no authoring at all. + seedVersion: 2, // No subject: an inbox row has a title, and the title is a block. The column // is email's, and leaving it NULL is how a non-email template says so. subject: null, variables: [ { name: 'title', type: 'string', required: true, example: 'Your house is close to collapsing' }, - { name: 'body', type: 'string', required: false, example: 'The Silver Anvil in Britain has entered its final decay stage.' }, - { name: 'url', type: 'string', required: false, example: 'https://example.com/houses' }, + { name: 'intro', type: 'string', required: false, example: 'The Silver Anvil in Britain has entered its final decay stage.' }, + { name: 'actionUrl', type: 'string', required: false, example: '/player/uo/houses' }, ], + // The three blocks map onto the three columns of `user_notifications` by ROLE + // (templates.js `renderInappByKey`): the heading is the item's title, the + // button is its one action, and everything else is the body. There is no + // unsubscribe line — an inbox item has nowhere to send someone that the + // preferences screen it links to from does not already reach. blocks: [ heading('h', '{{title}}', 'h3'), - text('body', '{{body}}'), - button('cta', 'Open', '{{url}}'), + text('intro', '{{intro}}'), + button('cta', 'Open', '{{actionUrl}}'), ], }, ] diff --git a/server/src/engagement/templates.js b/server/src/engagement/templates.js index e05cb4d..1b01205 100644 --- a/server/src/engagement/templates.js +++ b/server/src/engagement/templates.js @@ -123,13 +123,17 @@ function renderTemplate(template, values, resolved) { } /** - * Render the template stored under `key`, falling back to its shipped default. - * @returns {Promise<{subject: string, html: string, text: string, missing: string[]}|null>} - * null when `key` names no usable row AND no seed — which now includes a - * duplicated (seedless) template still in draft. + * The template `key` should actually render through, or null. + * + * Extracted from `renderByKey` in Phase 7 rather than duplicated into the in-app + * channel: the fallback chain below is a policy about what this deployment sends + * when its own table is in a bad state, and a second channel resolving templates + * by its own rules would be a second answer to that. `renderInappByKey` takes the + * same rows, the same seeds and the same three refusals. + * + * @returns {Promise<{subject: string|null, blocks: object[], text_body: string|null}|null>} */ -async function renderByKey(key, values = {}) { - const resolved = await ambient() +async function resolveTemplate(key) { let template = null try { template = await templatesDb.getByKey(key) @@ -158,9 +162,113 @@ async function renderByKey(key, values = {}) { if (unusable === 'unpublished') log.warn('stored template is a draft; using the shipped default', { key }) template = { subject: seed.subject, blocks: seed.blocks, text_body: null } } + return template +} + +/** + * Render the template stored under `key`, falling back to its shipped default. + * @returns {Promise<{subject: string, html: string, text: string, missing: string[]}|null>} + * null when `key` names no usable row AND no seed — which now includes a + * duplicated (seedless) template still in draft. + */ +async function renderByKey(key, values = {}) { + const resolved = await ambient() + const template = await resolveTemplate(key) + if (!template) return null return renderTemplate(template, values, resolved) } +// ── The in-app projection (Phase 7) ──────────────────────────────────────── +// +// `user_notifications` has three columns — title, body, url — where email has a +// subject and a document, so the in-app channel needs the template rendered into +// those three rather than into a mail. **The mapping is by block ROLE**, and it +// is here rather than in the channel because it is a statement about what the +// block registry means, not about how a row gets written: +// +// - the first `email.heading` → `title` (a heading IS the item's headline) +// - the first `email.button` → `url` (a button IS the item's one action) +// - everything else, as TEXT → `body` +// +// **Text, not the email HTML, and that is the load-bearing choice.** The block +// renderer's HTML is built for mail clients: table rows, inline hex colours, a +// light-only palette declared with `color-scheme`. Dropped into a page that +// follows the viewer's theme it renders as a pale card floating in a dark one. +// `toText` is the same content with none of that, and it is the part the block +// contract already promises every block can produce. +// +// The three refusals a mail can afford and an inbox row cannot are handled here +// too: a title is NOT NULL, so an empty one falls back to the projected `title` +// and then to the trigger id; and a url that is not site-relative is dropped +// rather than stored, because the column's whole contract is that a template +// cannot aim a signed-in user's click off-site. + +const HEADING = 'email.heading' +const BUTTON = 'email.button' + +// user_notifications.title / .url. Truncated rather than refused: a long title is +// a cosmetic problem and a dropped notification is not. +const MAX_TITLE = 300 +const MAX_URL = 500 + +// The same character class `pageUrlTemplate` and the engine's `url` variables +// use (registries.js, engagementEmit.js). Duplicated as a literal rather than +// imported from `engagementEmit`, which would be a cycle through the engine. +const RELATIVE_URL = /^\/(?!\/)[A-Za-z0-9\-._~/?#[\]@!$&'()*+,;=%]*$/ + +/** + * Site-relative form of `raw`, or null. + * + * An absolute url on this deployment's own base is accepted and reduced — a + * template that writes `{{siteUrl}}/guilds/4` is saying the same thing as + * `/guilds/4`, and refusing it would make the ambient `siteUrl` variable a trap + * in the one channel where the link never leaves the site. + */ +function relativeUrl(raw, base) { + const value = String(raw || '').trim() + if (!value) return null + const stripped = base && value.startsWith(`${base}/`) ? value.slice(base.length) : value + if (!RELATIVE_URL.test(stripped)) return null + return stripped.slice(0, MAX_URL) +} + +/** + * Render one template into an inbox item. + * + * @returns {Promise<{title: string, body: string|null, url: string|null, missing: string[]}|null>} + * null when `key` names no usable row and no seed — the caller reports a + * terminal failure, exactly as the email channel does. + */ +async function renderInappByKey(key, values = {}) { + const resolved = await ambient() + const template = await resolveTemplate(key) + if (!template) return null + + const merged = { ...values, ...resolved.values } + const missing = new Set() + const ctx = emailBlocks.buildContext({ + values: merged, + theme: resolved.theme, + baseUrl: resolved.baseUrl, + missing, + }) + + const blocks = Array.isArray(template.blocks) ? template.blocks : [] + const visible = blocks.filter((b) => b && b.visible !== false) + const heading = visible.find((b) => b.type === HEADING) + const button = visible.find((b) => b.type === BUTTON) + // Only the FIRST of each is consumed; a second heading or button is ordinary + // body content, which is what an operator who added one meant. + const rest = visible.filter((b) => b !== heading && b !== button) + + const headingText = heading ? ctx.t((heading.props || {}).text || '').trim() : '' + const title = (headingText || String(merged.title || '').trim() || key).slice(0, MAX_TITLE) + const url = button ? relativeUrl(ctx.t((button.props || {}).url || ''), resolved.baseUrl) : null + const body = emailBlocks.renderBlocks(rest, ctx).text.trim() + + return { title, body: body || null, url, missing: [...missing] } +} + /** * Ensure every shipped template exists, and bring un-customized rows up to the * current seed. Idempotent: a second run reports nine skips and writes nothing. @@ -215,4 +323,16 @@ async function seedTemplates() { const KEY_RE = /^[a-z][a-z0-9]*(?:[.-][a-z0-9]+)*$/ const MAX_KEY = 96 -module.exports = { ambient, variablesFor, renderTemplate, renderByKey, seedTemplates, baseUrl, KEY_RE, MAX_KEY } +module.exports = { + ambient, + variablesFor, + renderTemplate, + resolveTemplate, + renderByKey, + renderInappByKey, + relativeUrl, + seedTemplates, + baseUrl, + KEY_RE, + MAX_KEY, +} diff --git a/server/src/model/userNotifications/userNotifications.db.js b/server/src/model/userNotifications/userNotifications.db.js new file mode 100644 index 0000000..5500448 --- /dev/null +++ b/server/src/model/userNotifications/userNotifications.db.js @@ -0,0 +1,189 @@ +// ── The in-app inbox: SQL ────────────────────────────────────────────────── +// +// ENGAGEMENT.md §4.5 (G17), Phase 7. `user_notifications` is a small table with +// one unusual property worth stating up front: **every read here is scoped by +// `user_id`, and none of them takes an id alone.** +// +// That is not belt-and-braces over the route's own auth check. A notification is +// the only content core stores that is addressed to exactly one person, so +// "mark 41 read" is a request whose whole meaning is which account is asking. +// Passing the caller down to the WHERE clause makes the ownership check part of +// the statement that does the work, rather than a separate question asked +// earlier and trusted afterwards — an `UPDATE … WHERE id = ? AND user_id = ?` +// that matches nothing is a 404, and there is no ordering in which it is not. +// Phase 7's acceptance line asks for that assertion at the ROUTE; this is what +// makes the route's answer true rather than merely tested. + +const { query } = require('../../utils/db') + +// The page size a client gets when it asks for none, and the largest it may ask +// for. An inbox is read newest-first and nobody scrolls to row 500; the cap is +// what stops `?limit=100000` from being a way to make the server assemble the +// whole table. +const DEFAULT_LIMIT = 30 +const MAX_LIMIT = 100 + +const num = (n) => (Number.isFinite(Number(n)) ? Number(n) : 0) + +/** Shape one row for the API. `read` as a boolean beside the stamp: a client + * renders the flag and shows the stamp, and neither has to parse the other. */ +const toItem = (row) => ({ + id: num(row.id), + triggerId: row.trigger_id, + title: row.title, + body: row.body || null, + url: row.url || null, + read: row.read_at != null, + readAt: row.read_at || null, + createdAt: row.created_at, +}) + +/** + * Write one item, ignoring a duplicate `dedupe_key`. + * + * @returns {Promise<{inserted: boolean, id: number|null}>} + * + * `INSERT IGNORE` rather than a SELECT-then-INSERT, because the two callers race + * by construction: the outbox worker can be mid-retry while a module calls + * `ctx.inbox.push` for the same event. IGNORE also swallows an FK failure on a + * deleted user, which is the right outcome for the same reason — a row addressed + * to an account that no longer exists is not a failure anybody can act on. + * + * `inserted: false` is the dedupe path and the caller reports success: the user + * has the item, which is what "delivered" means. Distinguishing them at all is + * for the send log, which is entitled to say the second one was a duplicate. + */ +const insert = async ({ userId, triggerId, title, body = null, url = null, dedupeKey = null }) => { + const res = await query( + `INSERT IGNORE INTO user_notifications (user_id, trigger_id, title, body, url, dedupe_key) + VALUES (?, ?, ?, ?, ?, ?)`, + [Number(userId), String(triggerId), String(title), body, url, dedupeKey], + ) + const inserted = num(res && res.affectedRows) > 0 + return { inserted, id: inserted ? num(res.insertId) : null } +} + +/** + * One page of a user's inbox, newest first. + * + * @param {number} userId + * @param {{limit?: number, before?: number, unreadOnly?: boolean}} [opts] + * `before` is a keyset cursor (an id), not an offset. An inbox gains rows + * at the top while it is being paged; OFFSET under those conditions skips + * or repeats items, and the id is already the ordering key. + */ +const list = async (userId, { limit, before, unreadOnly } = {}) => { + const take = Math.min(Math.max(Number(limit) || DEFAULT_LIMIT, 1), MAX_LIMIT) + const params = [Number(userId)] + let where = 'user_id = ?' + if (unreadOnly) where += ' AND read_at IS NULL' + if (Number(before) > 0) { + where += ' AND id < ?' + params.push(Number(before)) + } + // take + 1 so the caller can say whether there is another page without a + // second COUNT over the same predicate. + const rows = await query( + `SELECT id, trigger_id, title, body, url, read_at, created_at + FROM user_notifications + WHERE ${where} + ORDER BY id DESC + LIMIT ?`, + [...params, take + 1], + ) + const hasMore = rows.length > take + return { items: rows.slice(0, take).map(toItem), hasMore } +} + +/** + * The item written for one (user, dedupe key), or null. + * + * The push channel's `ref` lookup and nothing else. A NULL dedupe key is not a + * wildcard — it means "this item does not dedupe", and matching on it would + * return an arbitrary earlier notification. + */ +const findByDedupe = async (userId, dedupeKey) => { + if (!dedupeKey) return null + const rows = await query( + `SELECT id, trigger_id, title, body, url, read_at, created_at + FROM user_notifications WHERE user_id = ? AND dedupe_key = ? LIMIT 1`, + [Number(userId), String(dedupeKey)], + ) + return rows.length ? toItem(rows[0]) : null +} + +/** How many of this user's items are unread. The badge. */ +const unreadCount = async (userId) => { + const rows = await query( + 'SELECT COUNT(*) AS n FROM user_notifications WHERE user_id = ? AND read_at IS NULL', + [Number(userId)], + ) + return num(rows[0] && rows[0].n) +} + +/** + * Mark one item read. Idempotent, and scoped to its owner. + * + * `read_at IS NULL` in the predicate is what makes a second call a no-op rather + * than a re-stamp: the acceptance line says mark-read is idempotent, and a + * timestamp that moves every time somebody re-opens the page is not. + * + * @returns {Promise} whether the row exists FOR THIS USER — false is a + * 404 whether the id belongs to nobody or to somebody else, which is + * also the only answer that does not report other people's row ids. + */ +const markRead = async (userId, id) => { + await query( + 'UPDATE user_notifications SET read_at = NOW() WHERE id = ? AND user_id = ? AND read_at IS NULL', + [Number(id), Number(userId)], + ) + const rows = await query('SELECT id FROM user_notifications WHERE id = ? AND user_id = ?', [ + Number(id), + Number(userId), + ]) + return rows.length > 0 +} + +/** Mark everything read. @returns {Promise} how many changed. */ +const markAllRead = async (userId) => { + const res = await query( + 'UPDATE user_notifications SET read_at = NOW() WHERE user_id = ? AND read_at IS NULL', + [Number(userId)], + ) + return num(res && res.affectedRows) +} + +/** + * Drop items older than `days`. + * + * **Read rows only.** An unread item is one the user has not seen, and an inbox + * that quietly deletes those is worse than one that grows: the whole point of + * the badge is that something is waiting. Age alone would also delete the + * evidence for "I was never told", which is the complaint this table answers. + * A never-read backlog is bounded in practice by the per-rule hourly ceiling. + * + * `LIMIT` per call so one sweep after a long outage is a bounded statement + * rather than a delete of a million rows holding locks; the sweep runs again. + */ +const pruneRead = async (days, limit = 1000) => { + const res = await query( + `DELETE FROM user_notifications + WHERE read_at IS NOT NULL AND created_at < (NOW() - INTERVAL ? DAY) + LIMIT ?`, + [Number(days), Number(limit)], + ) + return num(res && res.affectedRows) +} + +module.exports = { + insert, + list, + findByDedupe, + unreadCount, + markRead, + markAllRead, + pruneRead, + toItem, + DEFAULT_LIMIT, + MAX_LIMIT, +} diff --git a/server/src/modules/loader.js b/server/src/modules/loader.js index a176f16..d53b698 100644 --- a/server/src/modules/loader.js +++ b/server/src/modules/loader.js @@ -122,6 +122,7 @@ function buildCtx(id, moduleRoot) { const teams = require('../model/teams/teamSync.model') const teamActivity = require('../model/teams/teamActivity.model') const engagementEmit = require('../utils/engagementEmit') + const inappChannel = require('../engagement/inappChannel') const { makeLimiter, accountChangeLimiter } = require('../middleware/rateLimit') /* eslint-enable global-require */ @@ -229,17 +230,26 @@ function buildCtx(id, moduleRoot) { }, }, // The in-app sink (§5.1) — a module writing the inbox directly, without a - // rule. It is PRESENT AND THROWS until Phase 7 builds the channel and the - // `user_notifications` table behind it. + // rule. Live from Phase 7; it threw until the `user_notifications` table + // behind it existed. // - // Present-and-throwing rather than absent is the shape 1.6.0 settled on for - // exactly this situation (`ctx.teams.activity.push` before its phase landed): - // the version number states a whole surface, so a member of 1.7.0 that is - // missing would make the version a lie, and one that silently accepted data - // into a table that does not exist would be the worst of the three. + // Fire-and-forget and returns undefined, like `events.emit` above and + // `teams.activity.push` before it, and for the same reason: a module calls + // this from inside a game-event handler, and there is nothing it could + // correctly do with a storage failure of core's. The decision the sink makes + // that a module might want to know about — the user has this switched off — + // is deliberately not reported either, because a module that could see it + // would be a module that could enumerate people's preferences one write at a + // time. + // + // `id` is bound here and never taken from the arguments, exactly as `emit` + // and `teams.activity.push` bind theirs. inbox: { - push: () => { - throw new Error('ctx.inbox.push is not available until the in-app channel lands (ENGAGEMENT.md Phase 7)') + push: (userId, item) => { + inappChannel.pushDirect(id, userId, item).then( + (result) => { void result }, + (err) => { log.error('ctx.inbox.push failed', { module: id, message: err.message }) }, + ) }, }, // One function, for one caller: the `admin.users.detail` slot router needs diff --git a/server/src/modules/version.js b/server/src/modules/version.js index 8dd05bf..24e6bca 100644 --- a/server/src/modules/version.js +++ b/server/src/modules/version.js @@ -14,12 +14,15 @@ // `api.registerAudiences([...])`, `ctx.events.emit(triggerId, envelope)` and // `ctx.inbox.push(userId, item)`. module-uo's `coreApi: "^1.3.0"` still resolves. // -// **As in 1.6.0, the number covers the whole surface and the members arrive by -// phase.** `ctx.inbox.push` is present and THROWS until Phase 7 builds the -// in-app channel and the table behind it — the same choice, for the same reason: -// a member of 1.7.0 that were absent would make the version a lie, and one that -// silently accepted data into a table that does not exist would be worse than -// either. Everything else in 1.7.0 is live. +// **As in 1.6.0, the number covered the whole surface and the members arrived by +// phase, and all of them have now arrived.** `ctx.inbox.push` threw until Phase 7 +// built the in-app channel and the table behind it — the same choice, for the +// same reason: a member of 1.7.0 that were absent would have made the version a +// lie, and one that silently accepted data into a table that did not exist would +// have been worse than either. **Filling it in is NOT a bump**: the signature is +// the one 1.7.0 declared, and a module written against it needs no change. What a +// module WILL see differently is the throw becoming a write, which is the whole +// point of the phase. // // One thing here is not a member and is still part of the contract: a trigger id // and a notification-stream id share ONE namespace (ENGAGEMENT.md §7.2, settled diff --git a/server/src/router/v1/auth/notifications.controller.js b/server/src/router/v1/auth/notifications.controller.js index bee7dad..4d7b8f1 100644 --- a/server/src/router/v1/auth/notifications.controller.js +++ b/server/src/router/v1/auth/notifications.controller.js @@ -9,6 +9,7 @@ const channelPrefs = require('../../../model/notificationChannelPrefs/notificati const registries = require('../../../modules/registries') const teamPrefs = require('../../../model/teams/teamNotify.model') const { isAllowedEndpoint } = require('../../../utils/pushDispatch') +const inbox = require('../../../model/userNotifications/userNotifications.db') const log = require('../../../utils/logger')('notifications') @@ -146,6 +147,80 @@ async function putTeamPrefs(req, res) { } } +// ── The inbox (ENGAGEMENT.md §4.5 G17, Phase 7) ──────────────────────────── +// +// The in-app channel's read side. Everything above this line is a PREFERENCE — +// which streams, which channels, which Teams — and everything below it is +// CONTENT addressed to the caller. They share a path prefix because a person +// calls both "notifications", and the shapes keep them apart: the preference +// endpoints are whole-set GET/PUT pairs on named sub-paths, the inbox is a +// paged GET on the bare path with POSTs that name a row. +// +// **`req.user.id` is the only user id any of these can name.** There is no route +// parameter for a user and no query string that selects one, so the ownership +// check is not something a caller can be forgetful about — it is the shape of +// the API. The model then repeats it in the WHERE clause of every statement, so +// "read someone else's notification" is a 404 twice over. + +// GET /auth/me/notifications — one page of the caller's inbox, newest first. +async function getInbox(req, res) { + try { + const page = await inbox.list(req.user.id, { + limit: req.query.limit, + before: req.query.before, + unreadOnly: req.query.unread === 'true' || req.query.unread === '1', + }) + // The unread count rides along on every page, so the bell and the list never + // disagree: a client that renders both from one response cannot show "3 + // unread" above a list in which the third was just marked read. + return res.json({ ...page, unread: await inbox.unreadCount(req.user.id) }) + } catch (err) { + log.error('getInbox', err) + return res.status(500).json({ message: 'Internal Server Error' }) + } +} + +// GET /auth/me/notifications/unread-count — the badge, on its own. +// +// Its own route rather than a field of the list, because it is polled: a client +// asking "is there anything new" every minute should not make the server +// assemble thirty rows and their bodies to answer with one integer. +async function getUnreadCount(req, res) { + try { + return res.json({ unread: await inbox.unreadCount(req.user.id) }) + } catch (err) { + log.error('getUnreadCount', err) + return res.status(500).json({ message: 'Internal Server Error' }) + } +} + +// POST /auth/me/notifications/:id/read — mark one item read. Idempotent. +// +// 404 both when the row does not exist and when it belongs to somebody else, +// which is the same answer on purpose: distinguishing them would turn this route +// into a way to ask whether a given id is anybody's. +async function markRead(req, res) { + try { + const found = await inbox.markRead(req.user.id, req.params.id) + if (!found) return res.status(404).json({ message: 'Not Found' }) + return res.json({ ok: true, unread: await inbox.unreadCount(req.user.id) }) + } catch (err) { + log.error('markRead', err) + return res.status(500).json({ message: 'Internal Server Error' }) + } +} + +// POST /auth/me/notifications/read-all — mark the whole inbox read. +async function markAllRead(req, res) { + try { + const changed = await inbox.markAllRead(req.user.id) + return res.json({ ok: true, changed, unread: 0 }) + } catch (err) { + log.error('markAllRead', err) + return res.status(500).json({ message: 'Internal Server Error' }) + } +} + module.exports = { registerDevice, listDevices, @@ -157,4 +232,8 @@ module.exports = { putChannelPrefs, getTeamPrefs, putTeamPrefs, + getInbox, + getUnreadCount, + markRead, + markAllRead, } diff --git a/server/src/router/v1/auth/notifications.routes.js b/server/src/router/v1/auth/notifications.routes.js index 81abffa..2c01f12 100644 --- a/server/src/router/v1/auth/notifications.routes.js +++ b/server/src/router/v1/auth/notifications.routes.js @@ -7,7 +7,7 @@ // and never touches /admin. const express = require('express') -const { body, param } = require('express-validator') +const { body, param, query } = require('express-validator') const notif = require('./notifications.controller') const { requireAuth } = require('../../../auth/session.middleware') @@ -137,6 +137,72 @@ notifRouter.put( notif.putChannelPrefs, ) +// ── The inbox (ENGAGEMENT.md §4.5 G17, phase 7) ──────────────────────────── +// +// The in-app channel's read side, and the only routes in this file that carry +// CONTENT rather than a preference. They share the `/notifications` prefix +// because a person calls both by that name; the bare path is the inbox and the +// named sub-paths above are the settings for it. +// +// **Route order matters here and is not incidental.** `/notifications/streams`, +// `/notifications/subscriptions`, `/notifications/channels` and +// `/notifications/teams` are all declared ABOVE, and none of the routes below +// introduces a GET `/notifications/:something` that could shadow them. The one +// parameterised path is a POST, and its `:id` is digits-only. +notifRouter.get( + '/notifications', + // #swagger.tags = ['Auth · Me'] + // #swagger.summary = 'One page of the caller’s notification inbox' + // #swagger.description = 'The in-app channel’s items for the signed-in user, newest first. Paged with a keyset cursor (`before`), not an offset, because the list gains rows at the top while it is being read. `unread` counts the whole inbox, not the page. There is no way to name another user: the caller is the only account these routes can read.' + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + // #swagger.parameters['limit'] = { in: 'query', required: false, schema: { type: 'integer', minimum: 1, maximum: 100, default: 30 }, description: 'Page size (capped at 100).' } + // #swagger.parameters['before'] = { in: 'query', required: false, schema: { type: 'integer' }, description: 'Return items with an id lower than this — the cursor from the previous page.' } + // #swagger.parameters['unread'] = { in: 'query', required: false, schema: { type: 'boolean' }, description: 'Only items that have not been read.' } + /* #swagger.responses[200] = { description: 'A page of the inbox', content: { "application/json": { schema: { $ref: "#/components/schemas/NotificationInbox" } } } } */ + /* #swagger.responses[401] = { description: 'Not authenticated', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + query('limit').optional().isInt({ min: 1, max: 100 }), + query('before').optional().isInt({ min: 1 }), + query('unread').optional().isIn(['true', 'false', '1', '0']), + validate, + notif.getInbox, +) + +notifRouter.get( + '/notifications/unread-count', + // #swagger.tags = ['Auth · Me'] + // #swagger.summary = 'How many inbox items the caller has not read' + // #swagger.description = 'The badge. Its own route because it is polled — asking “is there anything new” should not make the server assemble a page of bodies to answer with one integer.' + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + /* #swagger.responses[200] = { description: 'The unread count', content: { "application/json": { schema: { $ref: "#/components/schemas/NotificationUnreadCount" } } } } */ + /* #swagger.responses[401] = { description: 'Not authenticated', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + notif.getUnreadCount, +) + +notifRouter.post( + '/notifications/read-all', + // #swagger.tags = ['Auth · Me'] + // #swagger.summary = 'Mark the caller’s whole inbox read' + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + /* #swagger.responses[200] = { description: 'Marked read', content: { "application/json": { schema: { $ref: "#/components/schemas/NotificationReadResult" } } } } */ + /* #swagger.responses[401] = { description: 'Not authenticated', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + notif.markAllRead, +) + +notifRouter.post( + '/notifications/:id/read', + // #swagger.tags = ['Auth · Me'] + // #swagger.summary = 'Mark one inbox item read' + // #swagger.description = 'Idempotent: a second call does not move the timestamp. 404 both when no such item exists and when it belongs to another account — the same answer on purpose, so this cannot be used to ask whether an id is anybody’s.' + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + // #swagger.parameters['id'] = { in: 'path', required: true, schema: { type: 'integer' }, description: 'Notification id (must belong to the caller).' } + /* #swagger.responses[200] = { description: 'Marked read', content: { "application/json": { schema: { $ref: "#/components/schemas/NotificationReadResult" } } } } */ + /* #swagger.responses[401] = { description: 'Not authenticated', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + /* #swagger.responses[404] = { description: 'No such item for this user', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + param('id').isInt({ min: 1 }), + validate, + notif.markRead, +) + // ── Per-Team preferences (TEAMS.md §6.3, phase 6) ────────────────────────── // // The granularity per-stream opt-in cannot express: "I am in five Teams and want diff --git a/server/src/server.js b/server/src/server.js index 3efb3b7..fda8743 100644 --- a/server/src/server.js +++ b/server/src/server.js @@ -10,6 +10,7 @@ const http = require('http') const botScore = require('./middleware/botScore') const announceWorker = require('./utils/announceWorker') const teamActivityPrune = require('./utils/teamActivityPrune') +const inboxPrune = require('./utils/userNotificationsPrune') const teamForumUploadSweep = require('./utils/teamForumUploadSweep') const teamDigestWorker = require('./utils/teamDigestWorker') const engagementWorker = require('./utils/engagementWorker') @@ -158,6 +159,7 @@ async function start() { // is the obvious unbounded-growth failure, so retention starts with the feed // rather than after someone notices. No-op on a deployment with no Teams. teamActivityPrune.start() + inboxPrune.start() teamForumUploadSweep.start() teamDigestWorker.start() @@ -183,6 +185,7 @@ function setupShutdown(server, internalServer) { botScore.stopSweeper() // stop the bot-store cleanup interval announceWorker.stop() // stop the news-announcement dispatcher poller teamActivityPrune.stop() // stop the Team activity retention timer + inboxPrune.stop() // stop the in-app inbox retention timer teamForumUploadSweep.stop() // stop the forum upload sweep teamDigestWorker.stop() // stop the Team forum digest timer engagementWorker.stop() // stop the engagement outbox worker diff --git a/server/src/utils/userNotificationsPrune.js b/server/src/utils/userNotificationsPrune.js new file mode 100644 index 0000000..e3ab6c3 --- /dev/null +++ b/server/src/utils/userNotificationsPrune.js @@ -0,0 +1,105 @@ +// ── Inbox retention worker ───────────────────────────────────────────────── +// +// ENGAGEMENT.md Phase 7. `user_notifications` is written by a rule that can fire +// on every event of its trigger, for every member of its audience, forever — +// nothing in the engine deletes anything, and neither the outbox nor the send +// log is a bound on this table (both hold one row per DELIVERY, and an inbox +// item outlives its delivery by design). The plan specifies no retention at all, +// which is how `team_activity` grew until §4.2 gave it this same worker. +// +// **Read items only, and that is the policy rather than an implementation +// detail.** An unread item is one the user has not seen; deleting it because it +// is old is the inbox quietly answering "nothing waiting" when something is. +// A never-read backlog is bounded in practice by the per-rule hourly ceiling +// (§7.1 Q3), which is the limit an operator actually tunes. +// +// Same in-process shape as `teamActivityPrune` and `announceWorker` — setInterval +// + unref + stop(), wired into server.js start/shutdown beside them, with the +// first run delayed so a table-wide DELETE never lands in front of the first +// request on a crash-looping deployment. + +const inbox = require('../model/userNotifications/userNotifications.db') +const settings = require('../model/settings/settings.model') +const log = require('./logger')('engagement') + +const INTERVAL_MS = Number(process.env.INBOX_PRUNE_MS) || 24 * 60 * 60 * 1000 +const FIRST_RUN_MS = Number(process.env.INBOX_PRUNE_DELAY_MS) || 5 * 60 * 1000 + +// In `settings`, not in env, for the reason §4.2 gives: an operator tightening a +// busy shard should not need a deploy. The key is namespaced with the table it +// governs rather than with the phase that added it. +const RETAIN_KEY = 'user_notifications_retain_days' +const DEFAULT_RETAIN_DAYS = 90 + +// A bound per sweep, so one run after a long outage is a series of bounded +// statements rather than a delete of a million rows holding locks. The sweep +// repeats until it clears, and stops early rather than looping forever. +const BATCH = 1000 +const MAX_BATCHES = 50 + +/** + * How many days of read items to keep. + * + * Wrapped in a try like `teamActivity.retentionConfig`, and for its reason: this + * runs on a timer with nobody watching, so a settings table that is briefly + * unavailable must yield the default rather than an exception that kills the + * nightly job. A zero or negative value would delete the whole inbox, so it is + * rejected rather than honoured. + */ +async function retainDays() { + try { + const raw = await settings.get(RETAIN_KEY) + const days = Number(raw) + if (Number.isFinite(days) && days > 0) return Math.floor(days) + } catch (err) { + log.debug('inbox retention setting unreadable; using the default', { message: err.message }) + } + return DEFAULT_RETAIN_DAYS +} + +/** One prune. Never throws — it runs on a timer with nobody to catch it. */ +async function tick() { + try { + const days = await retainDays() + let removed = 0 + for (let i = 0; i < MAX_BATCHES; i += 1) { + const n = await inbox.pruneRead(days, BATCH) + removed += n + if (n < BATCH) break + } + if (removed) log.info('inbox pruned', { removed, retainDays: days }) + return removed + } catch (err) { + log.error('inbox prune failed', { message: err.message }) + return null + } +} + +let timer = null +let firstRun = null + +function start() { + if (timer || firstRun) return timer + firstRun = setTimeout(() => { + firstRun = null + tick() + timer = setInterval(() => { tick() }, INTERVAL_MS) + if (timer.unref) timer.unref() + }, FIRST_RUN_MS) + if (firstRun.unref) firstRun.unref() + log.info('inbox retention started', { intervalMs: INTERVAL_MS, firstRunMs: FIRST_RUN_MS }) + return timer +} + +function stop() { + if (firstRun) { + clearTimeout(firstRun) + firstRun = null + } + if (timer) { + clearInterval(timer) + timer = null + } +} + +module.exports = { start, stop, tick, retainDays, RETAIN_KEY, DEFAULT_RETAIN_DAYS, INTERVAL_MS, FIRST_RUN_MS } diff --git a/server/swagger/swagger-output.json b/server/swagger/swagger-output.json index 5d68a0f..8e385d4 100644 --- a/server/swagger/swagger-output.json +++ b/server/swagger/swagger-output.json @@ -10225,6 +10225,101 @@ ] } }, + "/api/v1/auth/me/notifications": { + "get": { + "tags": [ + "Auth · Me" + ], + "summary": "One page of the caller’s notification inbox", + "description": "The in-app channel’s items for the signed-in user, newest first. Paged with a keyset cursor (`before`), not an offset, because the list gains rows at the top while it is being read. `unread` counts the whole inbox, not the page. There is no way to name another user: the caller is the only account these routes can read.", + "parameters": [ + { + "name": "limit", + "in": "query", + "required": false, + "schema": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "integer" + }, + "minimum": { + "type": "number", + "example": 1 + }, + "maximum": { + "type": "number", + "example": 100 + }, + "default": { + "type": "number", + "example": 30 + } + } + }, + "description": "Page size (capped at 100)." + }, + { + "name": "before", + "in": "query", + "required": false, + "schema": { + "type": "integer" + }, + "description": "Return items with an id lower than this — the cursor from the previous page." + }, + { + "name": "unread", + "in": "query", + "required": false, + "schema": { + "type": "boolean" + }, + "description": "Only items that have not been read." + } + ], + "responses": { + "200": { + "description": "A page of the inbox", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/NotificationInbox" + } + } + } + }, + "400": { + "description": "Bad Request" + }, + "401": { + "description": "Not authenticated", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "403": { + "description": "Forbidden" + }, + "500": { + "description": "Internal Server Error" + } + }, + "security": [ + { + "cookieAuth": [] + }, + { + "bearerAuth": [] + } + ] + } + }, "/api/v1/auth/me/notifications/channels": { "get": { "tags": [ @@ -10333,6 +10428,51 @@ } } }, + "/api/v1/auth/me/notifications/read-all": { + "post": { + "tags": [ + "Auth · Me" + ], + "summary": "Mark the caller’s whole inbox read", + "description": "", + "responses": { + "200": { + "description": "Marked read", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/NotificationReadResult" + } + } + } + }, + "401": { + "description": "Not authenticated", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "403": { + "description": "Forbidden" + }, + "500": { + "description": "Internal Server Error" + } + }, + "security": [ + { + "cookieAuth": [] + }, + { + "bearerAuth": [] + } + ] + } + }, "/api/v1/auth/me/notifications/streams": { "get": { "tags": [ @@ -10594,6 +10734,120 @@ } } }, + "/api/v1/auth/me/notifications/unread-count": { + "get": { + "tags": [ + "Auth · Me" + ], + "summary": "How many inbox items the caller has not read", + "description": "The badge. Its own route because it is polled — asking “is there anything new” should not make the server assemble a page of bodies to answer with one integer.", + "responses": { + "200": { + "description": "The unread count", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/NotificationUnreadCount" + } + } + } + }, + "401": { + "description": "Not authenticated", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "403": { + "description": "Forbidden" + }, + "500": { + "description": "Internal Server Error" + } + }, + "security": [ + { + "cookieAuth": [] + }, + { + "bearerAuth": [] + } + ] + } + }, + "/api/v1/auth/me/notifications/{id}/read": { + "post": { + "tags": [ + "Auth · Me" + ], + "summary": "Mark one inbox item read", + "description": "Idempotent: a second call does not move the timestamp. 404 both when no such item exists and when it belongs to another account — the same answer on purpose, so this cannot be used to ask whether an id is anybody’s.", + "parameters": [ + { + "name": "id", + "in": "path", + "required": true, + "schema": { + "type": "integer" + }, + "description": "Notification id (must belong to the caller)." + } + ], + "responses": { + "200": { + "description": "Marked read", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/NotificationReadResult" + } + } + } + }, + "400": { + "description": "Bad Request" + }, + "401": { + "description": "Not authenticated", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "403": { + "description": "Forbidden" + }, + "404": { + "description": "No such item for this user", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "500": { + "description": "Internal Server Error" + } + }, + "security": [ + { + "cookieAuth": [] + }, + { + "bearerAuth": [] + } + ] + } + }, "/api/v1/auth/me/sessions": { "get": { "tags": [ @@ -18728,6 +18982,292 @@ } } }, + "NotificationItem": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "object" + }, + "description": { + "type": "string", + "example": "One item in the caller’s in-app inbox." + }, + "properties": { + "type": "object", + "properties": { + "id": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "integer" + }, + "example": { + "type": "number", + "example": 412 + } + } + }, + "triggerId": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "string" + }, + "example": { + "type": "string", + "example": "team.forum.post" + } + } + }, + "title": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "string" + }, + "example": { + "type": "string", + "example": "The Silver Anvil — new forum post" + } + } + }, + "body": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "string" + }, + "nullable": { + "type": "boolean", + "example": true + }, + "example": { + "type": "string", + "example": "Darrow posted in The Silver Anvil." + } + } + }, + "url": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "string" + }, + "nullable": { + "type": "boolean", + "example": true + }, + "description": { + "type": "string", + "example": "Site-relative path only. An absolute or protocol-relative url is never stored." + }, + "example": { + "type": "string", + "example": "/guilds/the-silver-anvil/forum/412" + } + } + }, + "read": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "boolean" + }, + "example": { + "type": "boolean", + "example": false + } + } + }, + "readAt": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "string" + }, + "format": { + "type": "string", + "example": "date-time" + }, + "nullable": { + "type": "boolean", + "example": true + } + } + }, + "createdAt": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "string" + }, + "format": { + "type": "string", + "example": "date-time" + } + } + } + } + } + } + }, + "NotificationInbox": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "object" + }, + "description": { + "type": "string", + "example": "One page of the inbox, newest first. `unread` counts the whole inbox, not the page." + }, + "properties": { + "type": "object", + "properties": { + "items": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "array" + }, + "items": { + "$ref": "#/components/schemas/NotificationItem" + } + } + }, + "hasMore": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "boolean" + }, + "description": { + "type": "string", + "example": "Whether another page exists. Fetch it with `before` set to the last item’s id." + }, + "example": { + "type": "boolean", + "example": false + } + } + }, + "unread": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "integer" + }, + "example": { + "type": "number", + "example": 3 + } + } + } + } + } + } + }, + "NotificationUnreadCount": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "object" + }, + "properties": { + "type": "object", + "properties": { + "unread": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "integer" + }, + "example": { + "type": "number", + "example": 3 + } + } + } + } + } + } + }, + "NotificationReadResult": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "object" + }, + "description": { + "type": "string", + "example": "The result of marking one item, or the whole inbox, read. `unread` is the count after the change, so a client never has to re-poll for the badge." + }, + "properties": { + "type": "object", + "properties": { + "ok": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "boolean" + }, + "example": { + "type": "boolean", + "example": true + } + } + }, + "changed": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "integer" + }, + "description": { + "type": "string", + "example": "Mark-all only: how many items changed." + }, + "example": { + "type": "number", + "example": 3 + } + } + }, + "unread": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "integer" + }, + "example": { + "type": "number", + "example": 0 + } + } + } + } + } + } + }, "TeamNotificationPref": { "type": "object", "properties": { diff --git a/server/swagger/swagger.js b/server/swagger/swagger.js index 17ef6df..d804449 100644 --- a/server/swagger/swagger.js +++ b/server/swagger/swagger.js @@ -716,6 +716,51 @@ const doc = { }, }, }, + NotificationItem: { + type: 'object', + description: 'One item in the caller’s in-app inbox.', + properties: { + id: { type: 'integer', example: 412 }, + triggerId: { type: 'string', example: 'team.forum.post' }, + title: { type: 'string', example: 'The Silver Anvil — new forum post' }, + body: { type: 'string', nullable: true, example: 'Darrow posted in The Silver Anvil.' }, + url: { + type: 'string', + nullable: true, + description: 'Site-relative path only. An absolute or protocol-relative url is never stored.', + example: '/guilds/the-silver-anvil/forum/412', + }, + read: { type: 'boolean', example: false }, + readAt: { type: 'string', format: 'date-time', nullable: true }, + createdAt: { type: 'string', format: 'date-time' }, + }, + }, + NotificationInbox: { + type: 'object', + description: 'One page of the inbox, newest first. `unread` counts the whole inbox, not the page.', + properties: { + items: { type: 'array', items: { $ref: '#/components/schemas/NotificationItem' } }, + hasMore: { + type: 'boolean', + description: 'Whether another page exists. Fetch it with `before` set to the last item’s id.', + example: false, + }, + unread: { type: 'integer', example: 3 }, + }, + }, + NotificationUnreadCount: { + type: 'object', + properties: { unread: { type: 'integer', example: 3 } }, + }, + NotificationReadResult: { + type: 'object', + description: 'The result of marking one item, or the whole inbox, read. `unread` is the count after the change, so a client never has to re-poll for the badge.', + properties: { + ok: { type: 'boolean', example: true }, + changed: { type: 'integer', description: 'Mark-all only: how many items changed.', example: 3 }, + unread: { type: 'integer', example: 0 }, + }, + }, TeamNotificationPref: { type: 'object', description: "One Team's notification preference for the current user. Absent fields take the stored defaults: push is opt-OUT (not muted) and email is opt-IN (`off`).", diff --git a/server/test/engagementEngine.test.js b/server/test/engagementEngine.test.js index 2dbabe8..fea8a46 100644 --- a/server/test/engagementEngine.test.js +++ b/server/test/engagementEngine.test.js @@ -475,11 +475,21 @@ test('two sweepers racing one due row: exactly one claim wins', async () => { // ── The send log ─────────────────────────────────────────────────────────── test('a row whose channel has no deliver() finishes failed, and the send log says why', async () => { - // `inapp`, because as of Phase 6 `email` DOES deliver. The inbox arrives in - // Phase 7, and until then recording 'sent' would be a lie in the one table - // whose purpose is answering "did they get it". - addRule({ channels: ['inapp'] }) - optIn(10, 'uo.house.idoc_warning', 'inapp') + // **A channel registered for this test, because as of Phase 7 all three of + // core's deliver.** It used to name `inapp` (and `email` before that), which + // meant the assertion moved every time a phase gave a channel behaviour. The + // property under test was never about a particular channel: it is that the + // worker does not record 'sent' for a sink it cannot reach, because that would + // be a lie in the one table whose purpose is answering "did they get it". + channels.registerDeliveryChannel({ + id: 'nosink', + label: 'No sink', + carriesContent: true, + defaultMode: 'off', + supportsDigest: false, + }) + addRule({ channels: ['nosink'] }) + optIn(10, 'uo.house.idoc_warning', 'nosink') await engine.dispatch(event(), T0) await worker.tick(later(1000)) diff --git a/server/test/engagementInapp.test.js b/server/test/engagementInapp.test.js new file mode 100644 index 0000000..4f1aa00 --- /dev/null +++ b/server/test/engagementInapp.test.js @@ -0,0 +1,365 @@ +// ── The in-app channel (ENGAGEMENT.md Phase 7) ───────────────────────────── +// +// The phase's five acceptance criteria, plus the things building it showed were +// worth pinning: +// +// • one event delivered to `inapp` produces exactly one row +// • a duplicate `dedupeKey` is a no-op +// • mark-read is idempotent +// • a user cannot read another user's row — asserted AT THE ROUTE, which is +// what the acceptance line asks for, not only in the model +// • `url` is relative-only, by the same character class `pageUrlTemplate` uses +// +// • the block→column role mapping, which is the whole of how a template with a +// subject and a document becomes a row with three fields +// • `ctx.inbox.push` honours a preference where one exists and writes where +// none does +// • `liveChannels` puts `inapp` before `push`, which is what makes the tickle's +// deep-link ref resolve on the first pass +// +// Point the DB at a closed port before requiring anything: the registries reach +// utils/discordAnnounce, which builds the pool at require time. +process.env.DB_HOST = '127.0.0.1' +process.env.DB_PORT = '59999' + +const { test, beforeEach, afterEach, after } = require('node:test') +const assert = require('node:assert/strict') + +const registries = require('../src/modules/registries') +const channels = require('../src/engagement/channels') +const engine = require('../src/engagement/engine') +const inappChannel = require('../src/engagement/inappChannel') +const pushChannel = require('../src/engagement/pushChannel') +const templates = require('../src/engagement/templates') +const templateSeeds = require('../src/engagement/templateSeeds') +const templatesDb = require('../src/model/engagement/engagementTemplates.db') +const settings = require('../src/model/settings/settings.model') +const inbox = require('../src/model/userNotifications/userNotifications.db') +const recipients = require('../src/model/engagement/engagementRecipients.db') +const rulesDb = require('../src/model/engagement/engagementRules.db') +const pushDispatch = require('../src/utils/pushDispatch') +const notifCtrl = require('../src/router/v1/auth/notifications.controller') +const db = require('../src/utils/db') + +require('../src/engagement') +registries.registerCore() + +after(() => db.close()) + +const saved = new Map() +function patch(mod, name, fn) { + if (!saved.has(mod)) saved.set(mod, new Map()) + if (!saved.get(mod).has(name)) saved.get(mod).set(name, mod[name]) + mod[name] = fn +} +function restore() { + for (const [mod, names] of saved) for (const [name, fn] of names) mod[name] = fn + saved.clear() +} + +const TRIGGER = 'team.forum.post' + +let world + +beforeEach(() => { + world = { rows: [], tickles: [], storedModes: new Map() } + + // A stand-in for `user_notifications`, keyed the way the UNIQUE index is. + patch(inbox, 'insert', async (item) => { + const clash = + item.dedupeKey && + world.rows.some((r) => r.userId === item.userId && r.dedupeKey === item.dedupeKey) + if (clash) return { inserted: false, id: null } + const row = { id: world.rows.length + 1, ...item } + world.rows.push(row) + return { inserted: true, id: row.id } + }) + patch(inbox, 'findByDedupe', async (userId, key) => { + if (!key) return null + const row = world.rows.find((r) => r.userId === Number(userId) && r.dedupeKey === key) + return row ? { id: row.id, title: row.title } : null + }) + patch(recipients, 'filterActive', async (ids) => ids) + patch(recipients, 'storedModes', async () => world.storedModes) + patch(pushDispatch, 'publishToUsers', async (streamId, opts) => { + world.tickles.push({ streamId, ...opts }) + }) + patch(rulesDb, 'getById', async () => ({ + id: 1, + trigger_id: TRIGGER, + template_keys: { inapp: 'inapp.event' }, + })) + // **Stubbed at the MODULE BOUNDARY, not on `templates` itself**, and the + // distinction cost four minutes a run to find: `renderInappByKey` calls its + // own file-local `ambient()` and `resolveTemplate()`, so patching + // `templates.ambient` replaces an export nothing in that path reads, every + // call reaches the dead port, and each one waits out the driver's 30-second + // connect timeout while still passing. These two are real cross-module calls, + // so replacing them is what actually keeps the render off the database. + // + // A null row is also the path a fresh deployment takes: `resolveTemplate` + // falls through to the shipped seed. + patch(templatesDb, 'getByKey', async () => null) + patch(settings, 'getInstanceName', async () => 'Test Shard') + patch(settings, 'getShellBrand', async () => ({ logo: null, theme: null })) +}) +afterEach(restore) + +const outboxRow = (over = {}) => ({ + id: 1, + rule_id: 1, + trigger_id: TRIGGER, + user_id: 11, + channel: 'inapp', + subject_key: 'The Silver Hand', + scope_key: 'team:1', + dedupe_key: 'post:7', + payload: { teamName: 'The Silver Hand', authorName: 'Ten', threadTitle: 'Raid', postUrl: '/g/1?thread=7' }, + ...over, +}) + +// ── The registration ─────────────────────────────────────────────────────── + +// The Phase 7 decision `coreChannels.js` deferred in as many words. Opt-OUT for +// in-app alone: it wakes no device and leaves no building. +test('inapp is the one channel that defaults to instant', () => { + assert.equal(channels.defaultMode('inapp'), 'instant') + assert.equal(channels.defaultMode('push'), 'off') + assert.equal(channels.defaultMode('email'), 'off') +}) + +test('inapp and push both have a deliver now, and push still carries no content', () => { + assert.equal(typeof channels.get('inapp').deliver, 'function') + assert.equal(typeof channels.get('push').deliver, 'function') + assert.equal(channels.get('push').carriesContent, false) +}) + +// ── deliver ──────────────────────────────────────────────────────────────── + +test('one event delivered to inapp produces exactly one row', async () => { + const result = await inappChannel.deliver(outboxRow()) + assert.equal(result.ok, true) + assert.equal(world.rows.length, 1) + assert.equal(world.rows[0].userId, 11) + assert.equal(world.rows[0].triggerId, TRIGGER) +}) + +// The acceptance line calls it a no-op; from the recipient's side it is a +// delivery, so it reports ok with the reason in the detail rather than putting a +// red row in the send log for the mechanism working. +test('a duplicate dedupeKey is a no-op that still reports success', async () => { + await inappChannel.deliver(outboxRow()) + const again = await inappChannel.deliver(outboxRow({ id: 2 })) + assert.equal(again.ok, true) + assert.match(again.detail, /duplicate/i) + assert.equal(world.rows.length, 1) +}) + +test('a row with no dedupe key is never deduped', async () => { + await inappChannel.deliver(outboxRow({ dedupe_key: null })) + await inappChannel.deliver(outboxRow({ id: 2, dedupe_key: null })) + assert.equal(world.rows.length, 2) +}) + +test('a user who can no longer be reached is a terminal failure, not a retry', async () => { + patch(recipients, 'filterActive', async () => []) + const result = await inappChannel.deliver(outboxRow()) + assert.equal(result.ok, false) + assert.equal(result.retry, undefined) + assert.equal(world.rows.length, 0) +}) + +// A throw would be read by the worker as a transient failure and retried five +// times — one unrenderable template becoming five identical send-log rows. +test('deliver never throws — a render failure is classified, not propagated', async () => { + patch(templates, 'renderInappByKey', async () => { throw new Error('blocks are broken') }) + const result = await inappChannel.deliver(outboxRow()) + assert.equal(result.ok, false) + assert.match(result.detail, /blocks are broken/) +}) + +test('a template that names nothing shipped is terminal and says which key', async () => { + patch(rulesDb, 'getById', async () => ({ id: 1, template_keys: { inapp: 'nope.missing' } })) + const result = await inappChannel.deliver(outboxRow()) + assert.equal(result.ok, false) + assert.match(result.detail, /nope\.missing/) +}) + +// ── The block → column role mapping ──────────────────────────────────────── + +test('the heading becomes the title, the button becomes the url, the rest becomes the body', async () => { + const rendered = await templates.renderInappByKey('inapp.event', { + title: 'Your house is close to collapsing', + intro: 'The Silver Anvil has entered its final decay stage.', + actionUrl: '/player/uo/houses', + }) + assert.equal(rendered.title, 'Your house is close to collapsing') + assert.equal(rendered.url, '/player/uo/houses') + assert.match(rendered.body, /final decay stage/) + // The title and the action are COLUMNS; repeating them in the body would show + // the same words twice on one card. + assert.doesNotMatch(rendered.body, /close to collapsing/) + assert.doesNotMatch(rendered.body, /player\/uo\/houses/) +}) + +// §4.6.1 property 1, for this channel: a trigger with no bespoke template still +// renders, because `projection.project` supplies the structural names. +test('a trigger that authored nothing still gets a title, from its declaration', async () => { + const rendered = await inappChannel.renderItem(TRIGGER, { teamName: 'The Silver Hand' }, 'inapp.event') + assert.ok(rendered.title.length > 0) + assert.notEqual(rendered.title, 'inapp.event') +}) + +// The seed Phase 5a wrote named `body` and `url` — names nothing supplies, so +// every rendering of it would have produced a title and nothing else. +test('the shipped inapp seed names only variables the projection actually supplies', () => { + const seed = templateSeeds.seedByKey('inapp.event') + assert.equal(seed.seedVersion, 2) + const names = seed.variables.map((v) => v.name).sort() + assert.deepEqual(names, ['actionUrl', 'intro', 'title']) +}) + +// ── url: relative only ───────────────────────────────────────────────────── + +test('url is relative-only, and a protocol-relative one is dropped rather than stored', () => { + const base = 'https://shard.test' + assert.equal(templates.relativeUrl('/guilds/4', base), '/guilds/4') + assert.equal(templates.relativeUrl('https://shard.test/guilds/4', base), '/guilds/4') + assert.equal(templates.relativeUrl('//evil.test/x', base), null) + assert.equal(templates.relativeUrl('https://evil.test/x', base), null) + assert.equal(templates.relativeUrl('javascript:alert(1)', base), null) + assert.equal(templates.relativeUrl('', base), null) +}) + +// ── ctx.inbox.push ───────────────────────────────────────────────────────── + +test('ctx.inbox.push writes an item for a trigger nothing has registered', async () => { + const res = await inappChannel.pushDirect('uo', 11, { + triggerId: 'uo.unregistered.thing', + title: 'Something happened', + body: 'A thing occurred.', + url: '/player/uo/houses', + }) + assert.equal(res.written, true) + assert.equal(world.rows[0].url, '/player/uo/houses') +}) + +// The decision: a toggle somebody switched off must not be walkable around by +// the module that owns the trigger behind it. +test('ctx.inbox.push honours the user’s preference when the trigger IS registered', async () => { + world.storedModes = new Map([[11, 'off']]) + const res = await inappChannel.pushDirect('uo', 11, { triggerId: TRIGGER, title: 'Hi' }) + assert.equal(res.written, false) + assert.equal(world.rows.length, 0) +}) + +test('ctx.inbox.push writes for a registered trigger the user has left at the default', async () => { + world.storedModes = new Map() + const res = await inappChannel.pushDirect('uo', 11, { triggerId: TRIGGER, title: 'Hi' }) + assert.equal(res.written, true) +}) + +test('ctx.inbox.push drops an off-site url rather than storing it', async () => { + await inappChannel.pushDirect('uo', 11, { + triggerId: 'x.y', + title: 'Hi', + url: 'https://evil.test/steal', + }) + assert.equal(world.rows[0].url, null) +}) + +test('ctx.inbox.push refuses an item with no title, and never throws', async () => { + const res = await inappChannel.pushDirect('uo', 11, { triggerId: 'x.y' }) + assert.equal(res.written, false) + patch(inbox, 'insert', async () => { throw new Error('table is gone') }) + const boom = await inappChannel.pushDirect('uo', 11, { triggerId: 'x.y', title: 'Hi' }) + assert.equal(boom.written, false) +}) + +// ── push: the tickle, its ref, and what must never ride on it ────────────── + +test('the push tickle carries the stream and a ref, and no content whatsoever', async () => { + await inappChannel.deliver(outboxRow()) + const result = await pushChannel.deliver(outboxRow({ id: 2, channel: 'push' })) + assert.equal(result.ok, true) + const tickle = world.tickles[0] + assert.equal(tickle.streamId, TRIGGER) + assert.equal(tickle.ref, 'notification:1') + assert.deepEqual(Object.keys(tickle).sort(), ['ref', 'streamId', 'userIds']) + assert.deepEqual(tickle.userIds, [11]) +}) + +test('a push row with no inbox row behind it still publishes, with a null ref', async () => { + const result = await pushChannel.deliver(outboxRow({ channel: 'push', dedupe_key: null })) + assert.equal(result.ok, true) + assert.equal(world.tickles[0].ref, null) +}) + +// The ordering is what makes the ref resolve on the first pass: the outbox is +// swept `ORDER BY due_at, id`, so the in-app row has to be enqueued first. +test('liveChannels enqueues inapp before push, whatever order the rule names them in', () => { + assert.deepEqual(engine.liveChannels({ channels: ['push', 'inapp'] }), ['inapp', 'push']) + assert.deepEqual(engine.liveChannels({ channels: ['email', 'push'] }), ['email', 'push']) + assert.deepEqual(engine.liveChannels({ channels: ['push', 'nope'] }), ['push']) +}) + +// ── The routes: ownership, asserted where the acceptance line asks for it ── + +function res() { + const out = { code: 200, body: null } + return { + out, + status(c) { out.code = c; return this }, + json(b) { out.body = b; return this }, + } +} + +test('a user cannot mark another user’s notification read — 404 at the route', async () => { + // The model is NOT stubbed to "found": it is the real ownership predicate the + // route depends on, so the stub answers the way the SQL would. + patch(inbox, 'markRead', async (userId, id) => Number(userId) === 11 && Number(id) === 5) + patch(inbox, 'unreadCount', async () => 0) + + const mine = res() + await notifCtrl.markRead({ user: { id: 11 }, params: { id: 5 } }, mine) + assert.equal(mine.out.code, 200) + + const theirs = res() + await notifCtrl.markRead({ user: { id: 12 }, params: { id: 5 } }, theirs) + assert.equal(theirs.out.code, 404) + // The same answer whether the row is nobody's or somebody else's: telling them + // apart would make this a way to ask whether an id exists. + const missing = res() + await notifCtrl.markRead({ user: { id: 11 }, params: { id: 999 } }, missing) + assert.equal(missing.out.code, 404) +}) + +test('mark-read is idempotent', async () => { + let stamps = 0 + patch(inbox, 'markRead', async () => { stamps += 1; return true }) + patch(inbox, 'unreadCount', async () => 0) + await notifCtrl.markRead({ user: { id: 11 }, params: { id: 5 } }, res()) + await notifCtrl.markRead({ user: { id: 11 }, params: { id: 5 } }, res()) + assert.equal(stamps, 2) // the route is happy to be called twice… + // …and the statement behind it only stamps an unread row, which is the half + // that makes the second call a no-op. Pinned in the SQL test. +}) + +// There is no route parameter and no query string that names a user, so the +// listing cannot be pointed at another account even by a caller who tries. +test('the inbox list reads the caller and nothing else', async () => { + let askedFor = null + patch(inbox, 'list', async (userId, opts) => { + askedFor = { userId, opts } + return { items: [], hasMore: false } + }) + patch(inbox, 'unreadCount', async () => 2) + const r = res() + await notifCtrl.getInbox( + { user: { id: 11 }, query: { limit: '10', before: '99', unread: 'true', userId: '12' } }, + r, + ) + assert.equal(askedFor.userId, 11) + assert.equal(askedFor.opts.unreadOnly, true) + assert.equal(r.out.body.unread, 2) +}) diff --git a/server/test/notificationChannelPrefs.test.js b/server/test/notificationChannelPrefs.test.js index 384ad4d..a6c70bf 100644 --- a/server/test/notificationChannelPrefs.test.js +++ b/server/test/notificationChannelPrefs.test.js @@ -157,11 +157,18 @@ test('unknown stream ids are still dropped, and are not mirrored either', async // ── Acceptance: defaults ─────────────────────────────────────────────────── -test("a fresh user's modes are the channel defaults, and all three are off", async () => { +test("a fresh user's modes are the channel defaults — push and email off, in-app on", async () => { const surface = await prefs.getForUser(USER, PLAYER) const news = item(surface, 'news.post') - assert.deepEqual(news.modes, { push: 'off', email: 'off', inapp: 'off' }) + // **`inapp` is 'instant' from Phase 7**, and it is the only one that is. + // Settled by the org lead 2026-08-31: the argument for opt-IN was that push + // wakes a device somebody is holding and email leaves the building, and an + // inbox item does neither — it is a row on a page the user chose to open. Left + // 'off' the surface ships dead, because no rule could reach anybody until + // every user found a toggle for a channel they had never seen deliver + // anything. + assert.deepEqual(news.modes, { push: 'off', email: 'off', inapp: 'instant' }) assert.equal(prefRows.size, 0, 'reading preferences must not write rows') // The acceptance line in ENGAGEMENT.md originally said push defaults diff --git a/server/test/userNotificationsSql.test.js b/server/test/userNotificationsSql.test.js new file mode 100644 index 0000000..220ad64 --- /dev/null +++ b/server/test/userNotificationsSql.test.js @@ -0,0 +1,203 @@ +// ── The inbox's raw SQL, against a real MariaDB ──────────────────────────── +// +// ENGAGEMENT.md Phase 7. `engagementInapp.test.js` stubs the table and exercises +// everything the channel DECIDES. It cannot prove the three statements whose +// correctness is a server contract rather than a reading of this code: +// +// • **`UNIQUE (user_id, dedupe_key)` must admit many NULLs.** The whole +// "this item does not dedupe" case rests on it, and a unique index that +// rejected a second NULL would mean the second un-keyed notification any +// user ever received was silently dropped. It is standard SQL and it is also +// exactly the kind of assumption Phase 4a's `foundRows` defect was. +// • **`INSERT IGNORE` on a duplicate reports `affectedRows = 0`** — the value +// `insert()` returns `inserted: false` from, and therefore the value that +// decides whether the send log says "delivered" or "duplicate". +// • **`read_at IS NULL` in the mark-read predicate is what makes it +// idempotent**: the timestamp must not move on a second call. +// +// Plus the prune's one policy: it deletes read rows and leaves unread ones, +// however old. +// +// **It SKIPS when there is no database**, exactly as `engagementEngineSql` +// does and for its reason: CI runs the suite with the pool pointed at a dead +// port, and a file that failed there would make every PR red for a reason +// unrelated to itself. Run it against this machine's container with: +// +// DB_HOST=127.0.0.1 DB_PORT=3307 DB_USER=... DB_PASSWORD=... \ +// node --test test/userNotificationsSql.test.js +// +// It creates a throwaway database named after the process and drops it again, so +// it can never touch a real schema. + +const { test, before, after } = require('node:test') +const assert = require('node:assert/strict') +const mariadb = require('mariadb') + +// Verbatim from schema.sql, minus the FK to `users` — the point of this file is +// the index semantics, and a foreign key would mean seeding an accounts table +// that has nothing to do with any of them. +const SCHEMA = ` +CREATE TABLE user_notifications ( + id BIGINT AUTO_INCREMENT PRIMARY KEY, + user_id INT NOT NULL, + trigger_id VARCHAR(96) NOT NULL, + title VARCHAR(300) NOT NULL, + body TEXT NULL, + url VARCHAR(500) NULL, + dedupe_key VARCHAR(190) NULL, + read_at DATETIME NULL, + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + UNIQUE KEY uq_un_dedupe (user_id, dedupe_key), + INDEX idx_un_unread (user_id, read_at, created_at), + INDEX idx_un_prune (created_at) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; +` + +// The statements under test, verbatim from `userNotifications.db.js`. Duplicated +// rather than required for `engagementEngineSql`'s reason: requiring the model +// would drag in `utils/db`'s pool, which the harness has pointed at a dead port. +const INSERT = ` +INSERT IGNORE INTO user_notifications (user_id, trigger_id, title, body, url, dedupe_key) +VALUES (?, ?, ?, ?, ?, ?)` + +const MARK_READ = ` +UPDATE user_notifications SET read_at = NOW() WHERE id = ? AND user_id = ? AND read_at IS NULL` + +const PRUNE = ` +DELETE FROM user_notifications + WHERE read_at IS NOT NULL AND created_at < (NOW() - INTERVAL ? DAY) + LIMIT ?` + +const DB = `rg_inbox_test_${process.pid}` +let pool = null +let available = false + +const opts = () => ({ + host: process.env.DB_HOST || '127.0.0.1', + port: Number(process.env.DB_PORT) || 3306, + user: process.env.DB_USER || 'root', + password: process.env.DB_PASSWORD || '', +}) + +before(async () => { + const admin = mariadb.createPool({ + ...opts(), + connectionLimit: 1, + connectTimeout: 2000, + initializationTimeout: 2000, + }) + try { + await admin.query(`CREATE DATABASE ${DB}`) + available = true + } catch { + available = false + } finally { + await admin.end().catch(() => {}) + } + if (!available) return + + pool = mariadb.createPool({ + ...opts(), + database: DB, + connectionLimit: 3, + multipleStatements: true, + bigIntAsNumber: true, + insertIdAsNumber: true, + }) + await pool.query(SCHEMA) +}) + +after(async () => { + if (pool) { + await pool.query(`DROP DATABASE IF EXISTS ${DB}`).catch(() => {}) + await pool.end().catch(() => {}) + } +}) + +// Checked INSIDE each test, never as a `{ skip }` option — the trap +// `engagementEngineSql` documents and this file fell into anyway: the option is +// evaluated when the file is READ, which is before `before()` has had a chance +// to find out whether there is a database, so every test skips unconditionally. +// It looks exactly like a passing suite. +const SKIP = 'no database reachable - set DB_HOST/DB_PORT/DB_USER/DB_PASSWORD to run' +const needDb = (t) => { + if (available) return false + t.skip(SKIP) + return true +} + +const write = (userId, key, over = {}) => + pool.query(INSERT, [userId, over.trigger || 't.x', over.title || 'Hi', null, null, key]) + +test('a duplicate (user, dedupe key) is ignored and reports affectedRows 0', async (t) => { + if (needDb(t)) return + const first = await write(901, 'evt:1') + assert.equal(first.affectedRows, 1) + const second = await write(901, 'evt:1') + assert.equal(second.affectedRows, 0) + + // Scoped to the USER, not global: one event legitimately reaches fifty people, + // and a global unique key would admit the first and drop forty-nine — the + // defect Phase 4a found in §4.2a's outbox index, in a second place. + const other = await write(902, 'evt:1') + assert.equal(other.affectedRows, 1) +}) + +test('a NULL dedupe key never collides, however many there are', async (t) => { + if (needDb(t)) return + for (let i = 0; i < 3; i += 1) { + const res = await write(903, null) + assert.equal(res.affectedRows, 1) + } + const rows = await pool.query('SELECT COUNT(*) AS n FROM user_notifications WHERE user_id = 903') + assert.equal(Number(rows[0].n), 3) +}) + +test('mark-read stamps once and a second call moves nothing', async (t) => { + if (needDb(t)) return + const ins = await write(904, 'evt:read') + const id = ins.insertId + + const first = await pool.query(MARK_READ, [id, 904]) + assert.equal(first.affectedRows, 1) + const [after1] = await pool.query('SELECT read_at FROM user_notifications WHERE id = ?', [id]) + + // A second later, so a re-stamp would be visible rather than equal by accident. + await pool.query('UPDATE user_notifications SET read_at = read_at - INTERVAL 1 SECOND WHERE id = ?', [id]) + const [before2] = await pool.query('SELECT read_at FROM user_notifications WHERE id = ?', [id]) + + const second = await pool.query(MARK_READ, [id, 904]) + assert.equal(second.affectedRows, 0) + const [after2] = await pool.query('SELECT read_at FROM user_notifications WHERE id = ?', [id]) + assert.deepEqual(after2.read_at, before2.read_at) + assert.notDeepEqual(after1.read_at, before2.read_at) // the shift really happened +}) + +test('mark-read scoped to the owner matches nothing for anyone else', async (t) => { + if (needDb(t)) return + const ins = await write(905, 'evt:owner') + const wrong = await pool.query(MARK_READ, [ins.insertId, 906]) + assert.equal(wrong.affectedRows, 0) + const [row] = await pool.query('SELECT read_at FROM user_notifications WHERE id = ?', [ins.insertId]) + assert.equal(row.read_at, null) +}) + +test('the prune drops old READ rows and keeps unread ones however old', async (t) => { + if (needDb(t)) return + const old = await write(907, 'evt:old') + const oldUnread = await write(907, 'evt:old-unread') + const recent = await write(907, 'evt:recent') + await pool.query( + 'UPDATE user_notifications SET created_at = NOW() - INTERVAL 200 DAY, read_at = NOW() WHERE id = ?', + [old.insertId], + ) + await pool.query('UPDATE user_notifications SET created_at = NOW() - INTERVAL 200 DAY WHERE id = ?', [ + oldUnread.insertId, + ]) + await pool.query('UPDATE user_notifications SET read_at = NOW() WHERE id = ?', [recent.insertId]) + + const res = await pool.query(PRUNE, [90, 1000]) + assert.equal(res.affectedRows, 1) + const rows = await pool.query('SELECT id FROM user_notifications WHERE user_id = 907 ORDER BY id') + assert.deepEqual(rows.map((r) => Number(r.id)), [oldUnread.insertId, recent.insertId]) +})