diff --git a/client/src/modules/version.js b/client/src/modules/version.js index 44d4942..e23e393 100644 --- a/client/src/modules/version.js +++ b/client/src/modules/version.js @@ -11,6 +11,11 @@ // that the two files can drift, so a test asserts they agree // (client/test/moduleRegistry.test.js) rather than trusting a bump to remember // both. +// 1.8.0 - the ceiling lattice gains `admin` (ENGAGEMENT.md Phase 11). Nothing on +// this half changed: a ceiling is declared on the server's `api` and enforced +// there, and the admin screens that render one read the vocabulary from +// `GET /admin/engagement/triggers` rather than holding a copy. This file bumps +// anyway, for the reason at the top - the two halves state ONE version. // 1.7.0 — the engagement contract (docs/website/ENGAGEMENT.md Phase 2). Nothing // on this half changed: every member the version adds is on the server's `api` // and `ctx` (registerEventTriggers, registerAudiences, ctx.events.emit, @@ -53,4 +58,4 @@ // but the two halves state ONE version: a module declares a single coreApi range // and is served one chunk, so a client that claimed 1.0.0 while the server // answered 1.1.0 would be two answers to one question. -export const MODULE_API_VERSION = '1.7.0' +export const MODULE_API_VERSION = '1.8.0' diff --git a/client/src/routes/admin/views/EngagementRules.jsx b/client/src/routes/admin/views/EngagementRules.jsx index 482f578..0995149 100644 --- a/client/src/routes/admin/views/EngagementRules.jsx +++ b/client/src/routes/admin/views/EngagementRules.jsx @@ -494,6 +494,15 @@ function RuleEditor({ catalog, segments, rule, onSaved, onCancel }) { // It reads the RULES rather than a flag, so it disappears the moment one is // switched on and comes back if every one is switched off again. A deployment // that deleted them all sees nothing, which is right: they made that choice. +// +// **Phase 11 added a second notice of exactly the same shape, for news** +// (ENGAGEMENT.md §7.1 Q9). Publishing a news post used to tickle every subscriber +// directly, and that call is now an emit through the engine, so news push stops +// on upgrade until the seeded `news.post` rule is switched on. Two notices rather +// than one generalised "some rules are off" banner, deliberately: each names a +// capability that USED to work without configuration and now does not, which is +// a different statement from "you have a disabled rule" — and a rule an operator +// created and disabled themselves must never produce a warning. const TEAM_TRIGGERS = [ 'team.forum.post', 'team.announcement', @@ -501,11 +510,32 @@ const TEAM_TRIGGERS = [ 'team.leadership.changed', ] -function teamRulesAllOff(rules) { - const team = rules.filter((r) => TEAM_TRIGGERS.includes(r.triggerId || r.trigger_id)) - return team.length > 0 && team.every((r) => !r.enabled) +const NEWS_TRIGGERS = ['news.post'] + +// One style for both notices, so the pair reads as one kind of message rather +// than two that happen to look alike. +const NOTICE_STYLE = { + fontSize: '0.85rem', + borderRadius: 8, + padding: '10px 12px', + marginBottom: 16, + border: '1px solid #7a6440', + color: '#e0b070', } +const triggerOf = (rule) => rule.triggerId || rule.trigger_id + +// True only when rules for these triggers EXIST and every one of them is off. +// Zero matching rules means the operator deleted them, which is a choice, not a +// regression to warn about. +function allOff(rules, triggers) { + const group = rules.filter((r) => triggers.includes(triggerOf(r))) + return group.length > 0 && group.every((r) => !r.enabled) +} + +const teamRulesAllOff = (rules) => allOff(rules, TEAM_TRIGGERS) +const newsRulesAllOff = (rules) => allOff(rules, NEWS_TRIGGERS) + export default function EngagementRules() { const [catalog, setCatalog] = useState(null) const [segments, setSegments] = useState([]) @@ -583,13 +613,7 @@ export default function EngagementRules() { return (
{teamRulesAllOff(rules) && ( -
+
Team notification emails are off. They used to be sent automatically; they are now rules, and the four below arrived switched off so that nothing starts mailing on its own. Switch on the ones this deployment wants — per-member preferences and per-Team mutes @@ -597,6 +621,16 @@ export default function EngagementRules() {
)} + {newsRulesAllOff(rules) && ( +
+ News notifications are off. Publishing a news post used to send a push + notification to everyone subscribed to it. That is now the “News posts” rule below, and it + arrived switched off for the same reason the Team rules did. Switch it on to resume news + push — it also carries email and the in-app inbox, each still subject to each person’s own + preferences. The in-game town crier and the Discord announcement are unaffected either way. +
+ )} +

A rule turns an event into mail: which event, who hears about it, on which channels, and how diff --git a/client/src/routes/admin/views/EngagementTriggers.jsx b/client/src/routes/admin/views/EngagementTriggers.jsx index da69fb9..de89807 100644 --- a/client/src/routes/admin/views/EngagementTriggers.jsx +++ b/client/src/routes/admin/views/EngagementTriggers.jsx @@ -24,6 +24,7 @@ const CEILING_NOTE = { members: 'only members of the thing it is about', subscribers: 'only people who opted in', staff: 'only staff', + admin: 'only administrators', authenticated: 'any signed-in account', everyone: 'anyone', } diff --git a/server/db/seed.js b/server/db/seed.js index 1b1a93b..e84ac21 100644 --- a/server/db/seed.js +++ b/server/db/seed.js @@ -5,7 +5,7 @@ const wikiDb = require('../src/model/wiki/wiki.db') const users = require('../src/model/users/users.model') const { ensureSchema, close } = require('../src/utils/db') const { seedTemplates } = require('../src/engagement/templates') -const { seedTeamRules } = require('../src/engagement/coreRules') +const { seedCoreRules } = require('../src/engagement/coreRules') const brand = require('../src/config/brand') const log = require('../src/utils/logger')('seed') @@ -82,10 +82,13 @@ async function seedDefaults() { // failed to seed costs the shipped default, which `renderByKey` falls back to // anyway, and must not stop a boot. await seedTemplates() - // The four Team rules, seeded ONCE and all disabled (ENGAGEMENT.md Phase 6). - // Guarded by a settings key rather than re-ensured, so a rule an operator - // deleted stays deleted and one they enabled stays enabled. - await seedTeamRules() + // Core's five rules — the four Team ones (Phase 6) and news (Phase 11) — + // seeded ONCE and all disabled. Each GROUP carries its own settings-key guard + // rather than re-ensured, so a rule an operator deleted stays deleted and one + // they enabled stays enabled; and so the news rule reaches the deployments that + // were already stamped for Teams, which are exactly the ones that lose their + // raw news push to the engine (ENGAGEMENT.md §7.1 Q9). + await seedCoreRules() log.info('settings and wiki defaults ensured') } diff --git a/server/engagement-triggers.json b/server/engagement-triggers.json index 80bf010..f873359 100644 --- a/server/engagement-triggers.json +++ b/server/engagement-triggers.json @@ -1,6 +1,6 @@ { "_comment": "Generated event-trigger inventory - the authoritative freeze of CORE's engagement contract (docs/website/ENGAGEMENT.md 4.3). Regenerate with `npm run engagement:manifest` in website/server. A renamed variable, a changed type or a widened ceiling breaks stored templates and rules, so the diff here is the review signal. A module ships its own copy in its bundle; this file never contains one.", - "moduleApiVersion": "1.7.0", + "moduleApiVersion": "1.8.0", "triggers": [ { "id": "news.post", @@ -38,8 +38,8 @@ "name": "postUrl", "type": "url", "required": true, - "example": "/news/five-on-friday-yew-invasion", - "description": "Site-relative path to the post." + "example": "/site/news", + "description": "Site-relative path to the post. The news list today — the site has no per-post route." } ] }, diff --git a/server/src/config/coreTriggers.js b/server/src/config/coreTriggers.js index 8fa2db1..d9ff10a 100644 --- a/server/src/config/coreTriggers.js +++ b/server/src/config/coreTriggers.js @@ -49,8 +49,15 @@ const TRIGGERS = [ description: 'A plain-text summary, already stripped of markup.' }, { name: 'category', type: 'string', required: false, example: 'Five on Friday', description: 'The post category, when it has one.' }, - { name: 'postUrl', type: 'url', required: true, example: '/news/five-on-friday-yew-invasion', - description: 'Site-relative path to the post.' }, + // **`/site/news`, the LIST, and not a per-post path.** The example said + // `/news/` when this was declared with no caller; Phase 11 gave it + // one and the path turned out not to exist — `App.jsx` mounts `/site/news` + // and nothing under it, which is why `announceJobs.logic.js` links the list + // from the Discord and town-crier announcements too. An `example` is what + // the template editor previews and test-sends with (§4.3 property 3), so an + // example naming a 404 is a preview that looks right and a mail that is not. + { name: 'postUrl', type: 'url', required: true, example: '/site/news', + description: 'Site-relative path to the post. The news list today — the site has no per-post route.' }, ], }, diff --git a/server/src/engagement/audiences.js b/server/src/engagement/audiences.js index d7721f4..29a9285 100644 --- a/server/src/engagement/audiences.js +++ b/server/src/engagement/audiences.js @@ -1,7 +1,7 @@ // ── Resolving a rule's audience to recipients ────────────────────────────── // // ENGAGEMENT.md §5.1a / §4.5, Phase 4a. A rule names an audience two ways and -// only ever one at a time: a **plain ceiling name** (`owner`, `staff`, +// only ever one at a time: a **plain ceiling name** (`owner`, `staff`, `admin`, // `subscribers`, `authenticated`, `everyone`) resolved from core's own tables, or // an **`audience_segment_id`** pointing at an operator-composed tree of // module-declared audiences (segments.js). This file turns either into user ids. @@ -88,9 +88,14 @@ async function resolveForRule(rule, event) { } } case 'staff': + case 'admin': + // Both role-gated, and resolved through the ONE query rather than two. + // `ceilings.ROLE_CEILINGS` holds which roles each names, so the day a + // third is added the resolver does not need a third case — and, more to + // the point, cannot get one of them wrong while the others stay right. return { - userIds: await recipients.staff(ceilings.STAFF_CEILING_ROLES), - ceiling: 'staff', + userIds: await recipients.staff(ceilings.ROLE_CEILINGS[rule.audience].roles), + ceiling: rule.audience, dormant: false, reason: null, } diff --git a/server/src/engagement/coreRules.js b/server/src/engagement/coreRules.js index 54deade..8d159b8 100644 --- a/server/src/engagement/coreRules.js +++ b/server/src/engagement/coreRules.js @@ -1,4 +1,4 @@ -// ── The four Team rules core ships, all of them OFF ──────────────────────── +// ── The five rules core ships, all of them OFF ───────────────────────────── // // ENGAGEMENT.md Phase 6, decision 3. Before this phase the Team pipeline mailed // people with no operator configuration at all: the code decided who was mailed @@ -24,6 +24,17 @@ // that has seen this seed never sees it again — deleted rules stay deleted, and // an enabled rule stays enabled rather than being reset to off. +// **Phase 11 added a fifth, for `news.post`, and it needed its OWN one-shot key +// rather than an entry in the list above.** The Team key is already stamped on +// every deployment that has booted since Phase 6, and the guard reads its +// presence — so appending to `RULES` would have seeded the news rule on fresh +// installs only, and on exactly the upgrades that need it, never. Those are the +// deployments where `pushDispatch.publish('news.post', …)` used to run and no +// longer does (§7.1 Q9): they would have lost news push with no rule to switch +// on and no way to tell why. One key per seed GROUP is the rule this establishes; +// a sixth rule for a new trigger takes a sixth key, and a rule added to an +// existing group is a rule that only fresh installs will ever see. + const rulesDb = require('../model/engagement/engagementRules.db') const settingsDb = require('../model/settings/settings.db') const log = require('../utils/logger')('engagement') @@ -32,6 +43,9 @@ const log = require('../utils/logger')('engagement') // the settings table can tell when it ran; only its presence is read. const SEEDED_KEY = 'engagement_team_rules_seeded' +// Phase 11's, and separate for the reason above. Same shape, same semantics. +const NEWS_SEEDED_KEY = 'engagement_news_rule_seeded' + const RULES = [ { trigger_id: 'team.forum.post', @@ -98,20 +112,57 @@ const RULES = [ }, ] +// Phase 11's one rule, in its own list so it can carry its own one-shot key. +const NEWS_RULES = [ + { + trigger_id: 'news.post', + name: 'News posts', + // `subscribers`, which is the trigger's declared default and the population + // `pushDispatch.publish('news.post', …)` used to reach directly: users who + // opted into this id on at least one channel. Not `authenticated`, even + // though the trigger's ceiling permits it — a news post is worth telling + // people who asked to be told, and mailing the whole user table on every + // publish is how a notification feature earns a spam complaint. + audience: 'subscribers', + // **All three channels, unlike the Team rules' `email` alone**, and that is + // the continuity half of §7.1 Q9's answer. Push is on this rule because push + // is what the raw tickle did; leaving it off would mean an operator who + // enabled the rule to restore news push got mail instead. In-app rides along + // because the inbox is the surface a tickle deep-links into (Phase 7). + channels: ['email', 'inapp', 'push'], + // The generic body plus the structural projection (§4.6.1 property 1): + // `news.post` declares its own `title` and `postUrl`, which the projection + // leaves exactly as emitted, so an unauthored mail already names the post and + // links it. `inapp.event` is the in-app renderer's; push carries no content + // by construction and needs no template. + template_keys: { email: 'notify.event', inapp: 'inapp.event', digest: 'notify.digest' }, + // An hour, per USER — `news.post` declares no `subjectKey`, so the cooldown + // subject is the recipient. "Do not tell me about news more than once an + // hour" is the useful rule; keying it per post would make it a no-op, since + // every post is a new subject. + cooldown_seconds: 3600, + max_sends_per_hour: 1000, + }, +] + /** - * Seed the four rules, once. Returns a small summary for the boot log. + * Seed one group of rules, once, under its own guard key. * * Never throws: it is on the boot path beside `seedTemplates`, and a rule that * failed to seed costs an operator one visit to the "new rule" form, not a * deployment. + * + * @param {string} key the one-shot settings guard for THIS group + * @param {object[]} rules + * @param {string} note what the boot log should say when it inserts */ -async function seedTeamRules() { +async function seedGroup(key, rules, note) { const summary = { inserted: 0, skipped: 0 } try { - const seen = await settingsDb.get(SEEDED_KEY) - if (seen) return { ...summary, skipped: RULES.length } + const seen = await settingsDb.get(key) + if (seen) return { ...summary, skipped: rules.length } - for (const rule of RULES) { + for (const rule of rules) { try { await rulesDb.insert({ audience_segment_id: null, @@ -133,17 +184,46 @@ async function seedTeamRules() { // Stamped even on a partial run. Re-running would duplicate the rules that // did insert, and a duplicate rule is two mails per event — a worse outcome // than the one missing rule an operator can add from the screen. - await settingsDb.set(SEEDED_KEY, new Date().toISOString()) + await settingsDb.set(key, new Date().toISOString()) if (summary.inserted) { - log.info('seeded Team engagement rules, all disabled', { - rules: summary.inserted, - note: 'Team email stays off until an operator enables one', - }) + log.info('seeded engagement rules, all disabled', { rules: summary.inserted, note }) } } catch (err) { - log.error('team rule seeding failed', { message: err.message }) + log.error('rule seeding failed', { key, message: err.message }) } return summary } -module.exports = { seedTeamRules, RULES, SEEDED_KEY } +/** The four Team rules (Phase 6). */ +const seedTeamRules = () => + seedGroup(SEEDED_KEY, RULES, 'Team email stays off until an operator enables one') + +/** The one news rule (Phase 11). */ +const seedNewsRule = () => + seedGroup(NEWS_SEEDED_KEY, NEWS_RULES, 'News notifications stay off until an operator enables this rule') + +/** + * Both groups, which is what the boot path calls. + * + * Sequential rather than concurrent, and not for correctness — each group has its + * own guard key and its own rows — but so the boot log reads in a fixed order and + * a failure names one group rather than an interleaving of two. + */ +async function seedCoreRules() { + const team = await seedTeamRules() + const news = await seedNewsRule() + return { + inserted: team.inserted + news.inserted, + skipped: team.skipped + news.skipped, + } +} + +module.exports = { + seedCoreRules, + seedTeamRules, + seedNewsRule, + RULES, + NEWS_RULES, + SEEDED_KEY, + NEWS_SEEDED_KEY, +} diff --git a/server/src/model/notificationChannelPrefs/notificationChannelPrefs.model.js b/server/src/model/notificationChannelPrefs/notificationChannelPrefs.model.js index 18e5c20..cb5b2fa 100644 Binary files a/server/src/model/notificationChannelPrefs/notificationChannelPrefs.model.js and b/server/src/model/notificationChannelPrefs/notificationChannelPrefs.model.js differ diff --git a/server/src/modules/ceilings.js b/server/src/modules/ceilings.js index ae5f809..4969d4f 100644 --- a/server/src/modules/ceilings.js +++ b/server/src/modules/ceilings.js @@ -20,16 +20,29 @@ // ├── subscribers logged-in users who opted into this id // ├── members a module-declared list (a Team, the governors) // ├── staff admin / editor / moderator +// │ └── admin admins only // └── owner the one user the event is about // -// The four leaves are mutually INCOMPARABLE, deliberately. `owner` is not a +// The four branches are mutually INCOMPARABLE, deliberately. `owner` is not a // subset of `subscribers` (an owner need not have subscribed), `staff` is not a // subset of `members`, and no pair of them has a common descendant. That is what // makes `meet()` below return null rather than guessing, and a null meet is a // refused save (§5.1a rule 3) rather than a silent widening. // +// **`admin` was added in Phase 11 and it is the one genuine refinement in the +// tree.** Phase 2 shipped six values, and §8.6 then turned out to describe three +// triggers as admin-audience — `uo.audit.staff_action`, `uo.economy.milestone`, +// `uo.world.saved` — for which the narrowest available value was `staff`, i.e. +// admin / editor / moderator. Ceilinging them there would have permitted a rule +// that mails the staff audit digest to every editor, which is the same class of +// mistake the whole file exists to prevent. It is a CHILD rather than a seventh +// leaf because every admin is staff — the containment the other pairs lack — and +// that is why `permits`, `meet` and `meetAll` needed no change at all beyond the +// new `PARENT` entry. It is a module-contract change (a module may now declare +// `ceiling: 'admin'`) and took MODULE_API_VERSION to 1.8.0. +// // Nothing here reaches the database, the network or a user record. It is -// arithmetic over six constants, so it is safe to require anywhere. +// arithmetic over seven constants, so it is safe to require anywhere. // child → parent. A tree, which is what makes `permits` a walk to the root and // `meet` a comparison rather than a search: two nodes in a tree have a greatest @@ -40,6 +53,7 @@ const PARENT = { subscribers: 'authenticated', members: 'authenticated', staff: 'authenticated', + admin: 'staff', owner: 'authenticated', } @@ -51,6 +65,7 @@ const LABELS = { subscribers: 'Signed-in users subscribed to this event', members: 'Members of a module-declared list', staff: 'Staff only', + admin: 'Administrators only', owner: 'Only the user the event is about', } @@ -66,9 +81,50 @@ const LABELS = { // TOLD, which is the tier gate's population. const STAFF_CEILING_ROLES = ['admin', 'editor', 'moderator'] +// And which the `admin` ceiling names. One role, and it is written as a list for +// the same reason `STAFF_CEILING_ROLES` is: `recipients.staff(roles)` takes a +// list, so the two ceilings resolve through one query rather than through two +// that could drift. +const ADMIN_CEILING_ROLES = ['admin'] + /** Does this user fall inside the `staff` ceiling? */ const isStaffRole = (role) => STAFF_CEILING_ROLES.includes(role) +/** Does this user fall inside the `admin` ceiling? */ +const isAdminRole = (role) => ADMIN_CEILING_ROLES.includes(role) + +// The ceilings that name a ROLE, and the test for each. Two consumers read this +// rather than asking about `staff` by name: the audience resolver, and the +// preferences catalog's `visibleTo`. +// +// **`visibleTo` is why this is a table and not two `if`s.** Its rule is that an +// id nobody outside a role can ever be reached by must not appear by name in a +// player's preferences screen — `uo.cheat.detected` is the case the lattice was +// written for. That rule was expressed as `ceiling !== 'staff'` when `staff` was +// the only role-gated value; the day `admin` was added, that spelling would have +// silently published every admin-ceilinged id to every player. A table cannot +// drift the same way: adding a role-gated ceiling without adding it here is a +// registration that fails its own test, not a leak. +const ROLE_CEILINGS = { + staff: { roles: STAFF_CEILING_ROLES, test: isStaffRole }, + admin: { roles: ADMIN_CEILING_ROLES, test: isAdminRole }, +} + +/** Is this ceiling one that only certain roles can ever be reached by? */ +const isRoleCeiling = (ceiling) => Object.prototype.hasOwnProperty.call(ROLE_CEILINGS, ceiling) + +/** + * Could a viewer with this role EVER be reached by an id ceilinged here? + * + * The question a catalog asks before offering a toggle, and it fails closed: a + * role-gated ceiling with no role, or an unknown role, is a no. + */ +function reachableBy(ceiling, role) { + const gate = ROLE_CEILINGS[ceiling] + if (!gate) return true + return gate.test(role) +} + const CEILINGS = Object.keys(PARENT) /** Is this one of the six? The gate every registration and every rule save runs. */ @@ -119,4 +175,18 @@ function meetAll(list) { return list.reduce((acc, next) => (acc === null ? null : meet(acc, next)), list[0]) } -module.exports = { CEILINGS, LABELS, STAFF_CEILING_ROLES, isStaffRole, isCeiling, permits, meet, meetAll } +module.exports = { + CEILINGS, + LABELS, + STAFF_CEILING_ROLES, + ADMIN_CEILING_ROLES, + ROLE_CEILINGS, + isStaffRole, + isAdminRole, + isRoleCeiling, + reachableBy, + isCeiling, + permits, + meet, + meetAll, +} diff --git a/server/src/modules/version.js b/server/src/modules/version.js index 24e6bca..157b28e 100644 --- a/server/src/modules/version.js +++ b/server/src/modules/version.js @@ -9,6 +9,26 @@ // Deliberately separate from PROTOCOL_VERSION (which versions the shard wire and // has nothing to say about a website module) and from any module's own version. +// 1.8.0 - a seventh value in the audience ceiling lattice: `admin`, a child of +// `staff` (docs/website/ENGAGEMENT.md Phase 11, decision 1). A module may now +// declare `ceiling: 'admin'` on a trigger or an audience, so the set of values +// `registerEventTriggers` and `registerAudiences` accept grew. Additions only, +// so minor: every declaration valid before is valid now, no stored value +// changes, and module-uo's `coreApi: "^1.3.0"` still resolves. +// +// It exists because Phase 11's operator-facing triggers - `uo.audit.staff_action`, +// `uo.economy.milestone`, `uo.world.saved` - are described everywhere as +// admin-audience, and the narrowest value the lattice had was `staff`, which +// means admin / editor / moderator. Ceilinging them there would have permitted a +// rule that mails the staff audit digest to every editor. +// +// **What a module has to know about it beyond the new name.** `admin` is the one +// pair in the tree with real containment - every admin is staff - so it is the +// only place `permits` is true between two non-`authenticated` values: +// `permits('staff', 'admin')` holds and nothing else of that shape does. A +// trigger ceilinged `staff` therefore accepts an `admin` audience, which is the +// intended narrowing, and the reverse is refused as it should be. + // 1.7.0 — the engagement contract (docs/website/ENGAGEMENT.md Phase 2). // Additions only, so minor: `api.registerEventTriggers([...])`, // `api.registerAudiences([...])`, `ctx.events.emit(triggerId, envelope)` and @@ -81,6 +101,6 @@ // an admin action a module performs belongs in core's one audit log, the // extension slot needs the user its prefix names, and §2.7 forbids a module // reading core's `APP_BASE_URL` for itself. Additions only, so minor. -const MODULE_API_VERSION = '1.7.0' +const MODULE_API_VERSION = '1.8.0' module.exports = { MODULE_API_VERSION } diff --git a/server/src/router/v1/admin/admin.controller.js b/server/src/router/v1/admin/admin.controller.js index 8447d3a..04bed7e 100644 --- a/server/src/router/v1/admin/admin.controller.js +++ b/server/src/router/v1/admin/admin.controller.js @@ -12,12 +12,12 @@ const announceJobs = require('../../../model/announceJobs/announceJobs.model') const emailConfig = require('../../../model/emailConfig/emailConfig.model') const emailDedupe = require('../../../model/emailDedupe/emailDedupe.model') const forumSettings = require('../../../model/teams/teamForumSettings.model') -const pushDispatch = require('../../../utils/pushDispatch') const { cleanBody } = require('../../../utils/sanitizeHtml') const { parseJsonSetting } = require('../../../utils/settingsJson') const { validateThemeVisual } = require('../../../utils/themeResolve') const { validateBrandAssets, resolveBrandAssets } = require('../../../utils/brandAssets') const { validateNavOverrides, resolveNavOverrides, NAV_KEYS } = require('../../../utils/navOverrides') +const newsEmit = require('../../../utils/newsNotify') const htmlShell = require('../../../utils/htmlShell') const log = require('../../../utils/logger')('admin') @@ -47,13 +47,36 @@ async function announceIfNewlyPublished(post, transition) { // sidecar hiccup cannot break saving a post: the same guarantee the enqueue // above gives. await registries.dispatchPostHook('onSaved', { post, transition }) - // Opt-in push tickle to news.post subscribers, on the same transition. - // Fire-and-forget + self-guarding, so a dead ntfy relay never breaks saving. - if (jobId) { - Promise.resolve(pushDispatch.publish('news.post', { ref: String(post.id) })).catch((err) => - log.warn('news push failed', { postId: post.id, message: err.message }), - ) - } + // **The engagement engine, and it REPLACES the raw push tickle that used to be + // here** (ENGAGEMENT.md §7.1 Q9, decided 2026-08-31 at the start of Phase 11). + // + // `news.post` has been a declared payload contract with no caller since Phase 2 + // — a rule naming it could never fire — so on a real deployment the only mail + // or inbox item a rule could produce came from Teams. This is the call that + // fixes that, and it is deliberately the only thing about this function that + // changed: the announce legs above (a one-shot DELIVERY to a channel of the + // deployment, with retry) and the post hooks (idempotent STATE mirroring, which + // also runs on delete) are different KINDS of thing and both still fire exactly + // as they did. `registries.js` already states those two apart; this adds a third + // distinction of the same kind rather than replacing either. + // + // **What it replaced, and what that costs.** `pushDispatch.publish('news.post', + // …)` stood here and tickled every subscriber directly. It is gone, so push now + // rides the engine like every other channel — which means it goes nowhere until + // an operator enables the `news.post` rule core seeds `enabled = 0` beside the + // four Team ones (engagement/coreRules.js). That IS a behaviour change on + // upgrade and it is the org lead's decision, taken over keeping the raw call + // beside the emit "for one release": that is an exception with a deadline + // nobody owns, and Phase 6 refused the analogous carve-out for Teams. The admin + // Rules screen says so, and the release note names it. + // + // **Gated on `jobId`, the same value the push was gated on.** That is the single + // "newly published news" transition test and re-deriving it here would be a + // second chance to disagree with the first — an edit or a re-publish must not + // re-fire. Fire-and-forget, like everything else in this function: `emit` does + // not await delivery by design, and a rule lookup must not be able to fail + // saving a post. + if (jobId) newsEmit.emitNewsPost(post) } // ── Dashboard & site mode ───────────────────────────────────────────── diff --git a/server/src/utils/newsNotify.js b/server/src/utils/newsNotify.js new file mode 100644 index 0000000..4a5c663 --- /dev/null +++ b/server/src/utils/newsNotify.js @@ -0,0 +1,112 @@ +// ── The `news.post` emitter (ENGAGEMENT.md §7.1 Q9, Phase 11) ────────────── +// +// Core's own trigger, and until this phase the only one of its five with no +// caller at all: `config/coreTriggers.js` declared the payload contract in Phase +// 2 and said in as many words that nothing emitted yet, and Phase 6 migrated +// only the four `team.*` ones onto the engine. So an operator could write a rule +// on `news.post` and it could never fire. +// +// It is one call, and every line of care here is about what it must NOT disturb. +// `announceIfNewlyPublished` fans one publish three ways now, and they are +// different in kind: +// +// • `announceJobs.enqueueIfNeeded` — a one-shot DELIVERY to a channel of the +// deployment (the in-game town crier, Discord #news), with retry and +// classification. Not per-person. Not the engine's. +// • `registries.dispatchPostHook('onSaved')` — idempotent STATE mirroring, +// which also runs on delete and refreshes silently on an edit. Not the +// engine's either. +// • this — a PER-PERSON notification, subject to a rule, a preference, a +// suppression and a channel. The engine's, and the only one that was missing. +// +// A module keeps both of its doors onto a publish (the announce leg and the post +// hook) and gains no third: `news.post` is core's id, `ctx.events.emit` binds the +// owner at the call and never reads it from the arguments, and §7.2's one +// namespace means an id has exactly one owner across both facets. A module that +// wants a person-facing notification of its own declares its own trigger. +// +// **Nothing here throws.** It is called from a path that has already saved the +// post; a notification is a courtesy and a courtesy that can fail the write +// behind it is a defect. Same posture as `teamNotify.js`, for the same reason. + +const engagementEmit = require('./engagementEmit') +const log = require('./logger')('news-notify') + +// How much of a post body the mail carries when the post has no excerpt of its +// own. Same length as the Team excerpt: long enough to tell whether it is worth +// opening, short enough that the mail is not a copy of the article. +const EXCERPT_CHARS = 200 + +// **The site has no per-post route.** `App.jsx` mounts `/site/news` (the list) +// and nothing under it, which is why `announceJobs.logic.js` links the list from +// the Discord and town-crier announcements too. So the mail links what exists. +// Declared `required: true` on the trigger, so this is a constant rather than an +// optional — a variable that is sometimes absent is a template that sometimes +// renders a dead button. +// +// Site-RELATIVE, because `engagementEmit.RELATIVE_URL` validates `url` variables +// that way: a payload value that ends up in an href must not be able to carry an +// absolute one somewhere else. The mail renderer absolutizes it against the +// deployment's base. +const NEWS_PATH = '/site/news' + +/** Markup out, whitespace collapsed, truncated. The mail is plain text. */ +function excerptFrom(post) { + const source = post.excerpt || post.body || '' + const text = String(source) + .replace(/<[^>]*>/g, ' ') + .replace(/ /g, ' ') + .replace(/&/g, '&') + .replace(/</g, '<') + .replace(/>/g, '>') + .replace(/"/g, '"') + .replace(/\s+/g, ' ') + .trim() + if (!text) return null + return text.length > EXCERPT_CHARS ? `${text.slice(0, EXCERPT_CHARS - 1)}…` : text +} + +/** + * Emit `news.post` for a post that has just transitioned into published news. + * + * The caller gates on `announceJobs.enqueueIfNeeded`'s job id — the single + * transition signal — so this does not re-derive it. Passing a post that did not + * transition would produce a duplicate mail, which is why this function does not + * take the transition and cannot be tempted to read it differently. + * + * @param {object} post the saved post row + * @returns {boolean} whether the emit was accepted (for tests; callers ignore it) + */ +function emitNewsPost(post) { + try { + if (!post || post.id == null) return false + const excerpt = excerptFrom(post) + const result = engagementEmit.emit('core', 'news.post', { + data: { + title: post.title || 'A new post', + // Omitted rather than sent empty when the post has neither an excerpt nor + // a body worth quoting. `excerpt` is `required: false`, and an absent + // optional renders as absent; an empty string renders as a blank line + // where a summary should be. + ...(excerpt ? { excerpt } : {}), + ...(post.category ? { category: String(post.category) } : {}), + postUrl: NEWS_PATH, + }, + // **The cooldown subject is the USER, not the post**, which is why there is + // no `subject` here and no `subjectKey` on the declaration. "Do not mail me + // about news more than once an hour" is the useful rule; keying it per post + // would make every cooldown a no-op, since every post is a new subject. + // + // `dedupeKey` IS per post, and that is the other half of the same thought: + // the engine must not write two outbox rows for one publish if this is ever + // reached twice, and the post id is the only stable name for "this publish". + dedupeKey: `news.post:${post.id}`, + }) + return Boolean(result && result.ok) + } catch (err) { + log.warn('news event not emitted', { postId: post && post.id, message: err.message }) + return false + } +} + +module.exports = { emitNewsPost, excerptFrom, NEWS_PATH, EXCERPT_CHARS } diff --git a/server/test/engagementCeilings.test.js b/server/test/engagementCeilings.test.js index a0a58b4..cd299fe 100644 --- a/server/test/engagementCeilings.test.js +++ b/server/test/engagementCeilings.test.js @@ -16,10 +16,10 @@ const assert = require('node:assert/strict') const ceilings = require('../src/modules/ceilings') -test('the six ceilings are the vocabulary, and nothing else is', () => { +test('the seven ceilings are the vocabulary, and nothing else is', () => { assert.deepEqual( [...ceilings.CEILINGS].sort(), - ['authenticated', 'everyone', 'members', 'owner', 'staff', 'subscribers'], + ['admin', 'authenticated', 'everyone', 'members', 'owner', 'staff', 'subscribers'], ) for (const id of ceilings.CEILINGS) assert.ok(ceilings.LABELS[id], `${id} has an operator label`) assert.equal(ceilings.isCeiling('nobody'), false) @@ -33,9 +33,9 @@ test('everyone permits every ceiling; every ceiling permits itself', () => { } }) -test('authenticated permits the four leaves but not everyone', () => { - for (const leaf of ['subscribers', 'members', 'staff', 'owner']) { - assert.equal(ceilings.permits('authenticated', leaf), true) +test('authenticated permits every branch and admin beneath staff, but not everyone', () => { + for (const below of ['subscribers', 'members', 'staff', 'owner', 'admin']) { + assert.equal(ceilings.permits('authenticated', below), true) } assert.equal(ceilings.permits('authenticated', 'everyone'), false) }) @@ -55,6 +55,70 @@ test('a staff ceiling does NOT permit owner — fewer people is not less exposur } }) +// ── `admin`, added in Phase 11 ───────────────────────────────────────────── +// +// The one genuine refinement in the tree: every admin is staff, which is the +// containment no other pair has. These assert that it is a NARROWING and not a +// second way to widen — the failure this file exists to keep out, in its newest +// possible costume. + +test('staff permits admin and admin does not permit staff — the one true refinement', () => { + assert.equal(ceilings.permits('staff', 'admin'), true) + assert.equal(ceilings.permits('admin', 'staff'), false) + assert.equal(ceilings.meet('staff', 'admin'), 'admin') + assert.equal(ceilings.meet('admin', 'staff'), 'admin') +}) + +test('admin is incomparable with every branch that is not staff', () => { + for (const other of ['subscribers', 'members', 'owner']) { + assert.equal(ceilings.permits('admin', other), false, `admin must not permit ${other}`) + assert.equal(ceilings.permits(other, 'admin'), false, `${other} must not permit admin`) + assert.equal(ceilings.meet('admin', other), null, `admin ∧ ${other} has no bound`) + } +}) + +test('an admin-ceilinged trigger refuses a staff audience', () => { + // The acceptance criterion in as many words: a rule cannot give an + // admin-ceiling trigger a `staff` audience. `permits` is what both the save + // check and the send-time re-check call. + assert.equal(ceilings.permits('admin', 'staff'), false) + // …and the narrowing direction is allowed, which is what makes the node useful + // rather than merely restrictive. + assert.equal(ceilings.permits('staff', 'admin'), true) +}) + +test('the role ceilings are a table, so a new one cannot be forgotten', () => { + // `visibleTo` used to ask `ceiling !== 'staff'`. That spelling was correct + // while `staff` was the only role-gated value and would have silently published + // every admin-ceilinged id to every player's preferences screen the day `admin` + // arrived. The table is what makes that impossible to get wrong quietly. + assert.deepEqual(Object.keys(ceilings.ROLE_CEILINGS).sort(), ['admin', 'staff']) + for (const id of Object.keys(ceilings.ROLE_CEILINGS)) { + assert.ok(ceilings.isRoleCeiling(id), `${id} is a role ceiling`) + assert.ok(ceilings.ROLE_CEILINGS[id].roles.length, `${id} names at least one role`) + } + assert.equal(ceilings.isRoleCeiling('subscribers'), false) +}) + +test('reachableBy gates the role ceilings and lets everything else through', () => { + assert.equal(ceilings.reachableBy('admin', 'admin'), true) + assert.equal(ceilings.reachableBy('admin', 'editor'), false) + assert.equal(ceilings.reachableBy('admin', 'moderator'), false) + assert.equal(ceilings.reachableBy('admin', 'user'), false) + assert.equal(ceilings.reachableBy('staff', 'editor'), true) + assert.equal(ceilings.reachableBy('staff', 'user'), false) + // Fails closed on a missing viewer, which is how a signed-out catalog read + // reaches it. + assert.equal(ceilings.reachableBy('staff', undefined), false) + assert.equal(ceilings.reachableBy('admin', undefined), false) + // Everything that is not role-gated is visible to anyone, including the `null` + // a stream-only catalog item carries. + for (const open of ['everyone', 'authenticated', 'subscribers', 'members', 'owner']) { + assert.equal(ceilings.reachableBy(open, 'user'), true, `${open} is not role-gated`) + } + assert.equal(ceilings.reachableBy(null, 'user'), true) +}) + test('an unknown ceiling is permitted by nothing, on either side', () => { assert.equal(ceilings.permits('everyone', 'god'), false) assert.equal(ceilings.permits('god', 'owner'), false) @@ -70,6 +134,7 @@ test('A OR B takes the NARROWER of the two ceilings, not the wider', () => { test('incomparable ceilings have no meet — the save is refused, not guessed', () => { assert.equal(ceilings.meet('staff', 'members'), null) + assert.equal(ceilings.meet('admin', 'owner'), null) assert.equal(ceilings.meet('owner', 'subscribers'), null) assert.equal(ceilings.meet('staff', 'nonsense'), null) }) diff --git a/server/test/engagementTriggers.test.js b/server/test/engagementTriggers.test.js index 63c59c1..88be5c8 100644 --- a/server/test/engagementTriggers.test.js +++ b/server/test/engagementTriggers.test.js @@ -419,10 +419,14 @@ test('GET /admin/engagement/triggers serves core\'s declarations and the ceiling assert.ok(news.variables.some((v) => v.name === 'title' && v.example)) // The lattice travels with the catalog so the rule editor never offers an // audience the server will refuse. + // `staff` permits itself and `admin` beneath it — the one refinement in the + // tree (Phase 11). Every other branch permits itself alone. const staff = res.body.ceilings.find((c) => c.id === 'staff') - assert.deepEqual(staff.permits, ['staff']) + assert.deepEqual(staff.permits, ['staff', 'admin']) + const admin = res.body.ceilings.find((c) => c.id === 'admin') + assert.deepEqual(admin.permits, ['admin']) const everyone = res.body.ceilings.find((c) => c.id === 'everyone') - assert.equal(everyone.permits.length, 6) + assert.equal(everyone.permits.length, 7) }) test('GET /admin/engagement/audiences never serves a resolver', () => { diff --git a/server/test/newsNotify.test.js b/server/test/newsNotify.test.js new file mode 100644 index 0000000..672925d --- /dev/null +++ b/server/test/newsNotify.test.js @@ -0,0 +1,127 @@ +// ── Core's `news.post` emitter (ENGAGEMENT.md §7.1 Q9, Phase 11) ─────────── +// +// `news.post` was a declared payload contract with NO CALLER from Phase 2 until +// this phase — a rule naming it could never fire, so on a real deployment the +// only mail or inbox item a rule could produce came from Teams. This is the test +// for the call that fixes it, and for the two things about it that are decisions +// rather than plumbing: +// +// 1. The emit REPLACED `pushDispatch.publish('news.post', …)`, so news push now +// rides a rule. Core seeds that rule DISABLED, which is why news push stops +// on upgrade — deliberately, on the Phase 6 Team precedent. +// 2. The seed carries its OWN one-shot key. The Team key is already stamped on +// every deployment that has booted since Phase 6, and those are exactly the +// deployments that lose their raw push — so joining that group would have +// seeded the news rule on fresh installs only. + +const { test } = require('node:test') +const assert = require('node:assert/strict') + +const registries = require('../src/modules/registries') +const newsNotify = require('../src/utils/newsNotify') +const coreRules = require('../src/engagement/coreRules') + +registries.registerCore() + +const POST = { + id: 412, + title: 'Five on Friday — the Yew invasion', + excerpt: 'Four new champion spawns, and the fate of the Yew moongate.', + category: 'Five on Friday', +} + +test('a publish emits news.post with the declaration\'s own variables', () => { + const ok = newsNotify.emitNewsPost(POST) + assert.equal(ok, true) +}) + +test('the emitted payload satisfies the declared contract', () => { + // `emit` throws in dev on a payload that misses a required variable, so the + // assertion that it did not throw above is already most of this. This says + // which variables, so a future declaration change breaks here with a name. + const declaration = registries.eventTrigger('news.post') + const required = declaration.variables.filter((v) => v.required).map((v) => v.name) + assert.deepEqual(required.sort(), ['postUrl', 'title']) +}) + +test('postUrl is the news LIST, because the site has no per-post route', () => { + // `App.jsx` mounts `/site/news` and nothing under it, which is why + // `announceJobs.logic.js` links the list from the Discord and town-crier + // announcements too. The declaration's `example` used to name `/news/`, + // a path that 404s — and an example is what the template editor previews and + // test-sends with, so a wrong one is a preview that looks right. + assert.equal(newsNotify.NEWS_PATH, '/site/news') + assert.ok(newsNotify.NEWS_PATH.startsWith('/'), 'site-relative, because it ends up in an href') + assert.ok(!newsNotify.NEWS_PATH.startsWith('//'), 'not protocol-relative') + const declaration = registries.eventTrigger('news.post') + const postUrl = declaration.variables.find((v) => v.name === 'postUrl') + assert.equal(postUrl.example, newsNotify.NEWS_PATH, 'the example is the value the emitter sends') +}) + +test('a post with no excerpt falls back to the body, stripped of markup', () => { + const text = newsNotify.excerptFrom({ body: '

Hello world

' }) + assert.equal(text, 'Hello world') +}) + +test('an empty post omits the excerpt rather than sending a blank one', () => { + // `excerpt` is declared optional. An absent optional renders as absent; an + // empty string renders as a blank line where a summary should be. + assert.equal(newsNotify.excerptFrom({}), null) + assert.equal(newsNotify.excerptFrom({ body: '

' }), null) +}) + +test('a long body is truncated rather than reproduced in the mail', () => { + const text = newsNotify.excerptFrom({ body: 'x'.repeat(500) }) + assert.equal(text.length, newsNotify.EXCERPT_CHARS) + assert.ok(text.endsWith('…')) +}) + +test('a malformed post is a no-op, never an exception on the publish path', () => { + // This runs inside `announceIfNewlyPublished`, after the post has been saved. + // A notification that can fail the write behind it is a defect. + assert.equal(newsNotify.emitNewsPost(null), false) + assert.equal(newsNotify.emitNewsPost({}), false) +}) + +// ── The seed, and why it is its own group ────────────────────────────────── + +test('the news rule is seeded, disabled, on all three channels', () => { + assert.equal(coreRules.NEWS_RULES.length, 1) + const [rule] = coreRules.NEWS_RULES + assert.equal(rule.trigger_id, 'news.post') + assert.equal(rule.audience, 'subscribers') + // **Push is on this rule** because push is what the raw tickle did. Leaving it + // off would mean an operator who enabled the rule to restore news push got + // mail instead. + assert.deepEqual(rule.channels, ['email', 'inapp', 'push']) + // Nothing core seeds is ever enabled — `seedGroup` stamps `enabled: 0` over + // every entry, so the rule cannot ship on even by accident. + assert.equal(rule.enabled, undefined) +}) + +test('the news seed has its own one-shot key, separate from the Team group\'s', () => { + // The failure this prevents: the Team key is already stamped on every + // deployment that has booted since Phase 6, and the guard reads its presence. + // Appending to the Team list would have seeded the news rule on fresh installs + // only — and on exactly the upgrades that lose their raw news push, never. + assert.notEqual(coreRules.NEWS_SEEDED_KEY, coreRules.SEEDED_KEY) + assert.equal(coreRules.SEEDED_KEY, 'engagement_team_rules_seeded') + assert.equal(coreRules.NEWS_SEEDED_KEY, 'engagement_news_rule_seeded') +}) + +test('the audience the rule names is one the trigger\'s ceiling permits', () => { + const ceilings = require('../src/modules/ceilings') + const declaration = registries.eventTrigger('news.post') + for (const rule of [...coreRules.RULES, ...coreRules.NEWS_RULES]) { + const d = registries.eventTrigger(rule.trigger_id) + assert.ok(d, `${rule.trigger_id} is declared`) + assert.ok( + ceilings.permits(d.ceiling, rule.audience), + `${rule.trigger_id}: audience "${rule.audience}" is within ceiling "${d.ceiling}"`, + ) + } + // Named explicitly, because it is the one seeded rule whose ceiling is wider + // than its audience: a rule editor may widen news to `authenticated`, and + // deliberately may not widen a Team event past `members`. + assert.equal(declaration.ceiling, 'authenticated') +}) diff --git a/server/test/notificationChannelPrefs.test.js b/server/test/notificationChannelPrefs.test.js index a6c70bf..dad54d8 100644 --- a/server/test/notificationChannelPrefs.test.js +++ b/server/test/notificationChannelPrefs.test.js @@ -35,6 +35,9 @@ after(() => db.close()) const USER = 7 const PLAYER = { id: USER, role: 'player' } const ADMIN = { id: USER, role: 'admin' } +// Phase 11 added the `admin` ceiling beneath `staff`, so the interesting viewer +// is no longer "player vs staff" but the one INSIDE `staff` and outside `admin`. +const EDITOR = { id: USER, role: 'editor' } // ── In-memory stand-ins for the two tables ───────────────────────────────── // @@ -338,6 +341,60 @@ test('a staff-ceilinged trigger is not offered to a player, and is to staff', as assert.equal(prefRows.size, 0) }) + +// **The Phase 11 ceiling, and the reason `visibleTo` stopped asking about `staff` +// by name.** Its rule used to be `ceiling !== 'staff'`, which was correct while +// `staff` was the only role-gated value and would have silently published every +// admin-ceilinged id to every player the day `admin` arrived. An editor is the +// viewer that tells the two apart: inside `staff`, outside `admin`. +test('an admin-ceilinged trigger is hidden from a player AND from an editor', async () => { + const api = registries.stage('uo') + api.registerEventTriggers([{ + id: 'uo.audit.staff_action', + label: 'A staff member acted in game', + ceiling: 'admin', + audience: 'admin', + variables: [{ name: 'action', type: 'string', required: true, example: 'set' }], + }]) + registries.apply(api.staged) + + const asPlayer = await prefs.getForUser(USER, PLAYER) + assert.equal(item(asPlayer, 'uo.audit.staff_action'), undefined, 'a player is not told it exists') + + // The one an `!== staff` test would have got wrong: an editor IS staff, and a + // digest of what staff did in game is not for them. + const asEditor = await prefs.getForUser(USER, EDITOR) + assert.equal(item(asEditor, 'uo.audit.staff_action'), undefined, 'an editor is not told either') + + const asAdmin = await prefs.getForUser(USER, ADMIN) + assert.ok(item(asAdmin, 'uo.audit.staff_action'), 'an admin is') + + // A gate, not a display rule: an editor who knows the id still cannot store a + // preference for it. + await notifCtrl.putChannelPrefs( + { user: EDITOR, body: { prefs: [{ id: 'uo.audit.staff_action', channel: 'email', mode: 'instant' }] } }, + mockRes(), + ) + assert.equal(prefRows.size, 0) +}) + +// The counterpart, so the generalisation did not quietly hide more than it should. +test('a staff-ceilinged trigger is still offered to an editor', async () => { + const api = registries.stage('uo') + api.registerEventTriggers([{ + id: 'uo.page.new', + label: 'A player opened a help page', + ceiling: 'staff', + audience: 'staff', + variables: [{ name: 'pageType', type: 'string', required: true, example: 'Stuck' }], + }]) + registries.apply(api.staged) + + const asEditor = await prefs.getForUser(USER, EDITOR) + assert.ok(item(asEditor, 'uo.page.new'), 'an editor is inside the staff ceiling') + assert.equal(item(await prefs.getForUser(USER, PLAYER), 'uo.page.new'), undefined) +}) + // ── The channel registry itself ──────────────────────────────────────────── test('the registry refuses a channel that under-declares', async () => {