feat(engagement): Admin - Engagement - Rules and Audiences (engagement Phase 4b)
All checks were successful
PR Checks / bot-tests (pull_request) Successful in 31s
PR Checks / client-build (pull_request) Successful in 32s
PR Checks / server-tests (pull_request) Successful in 10m36s

The admin surface over the Phase 4a engine: two screens, twelve routes and the
reach preview. Nothing in the engine changed; what changed is that an operator
can now reach it.

Four decisions settled by the org lead before any code:

  - segments get their OWN nav entry, "Audiences", not a tab of the rules screen
  - the on/off switch is its own PATCH route, not a full PUT
  - the reach preview is a count only, on demand
  - a rule can be hard-deleted; the send log survives it

The switch is the one with real content in it. A PUT re-validates against the
registries as they are NOW, so the rules a re-validating toggle cannot switch
off are exactly the three an operator most wants stopped: a rule whose module
was uninstalled, one naming a channel that is gone, and one whose trigger has
since narrowed its ceiling under a saved audience. PATCH .../enabled writes one
column and always works. Switching ON unvalidated is safe because the engine
re-checks the ceiling at send time.

The preview calls the engine's own resolver rather than a second query that
agrees with it today, and answers a count and nothing else - the resolver's
output for a module-declared segment is a set of players derived from game data.
It reports `capped` at the 5000-row bound (the count is a floor, not a total),
`reason` for an `owner` audience (which resolves per event and has no advance
answer), and `permitted` so the editor cannot show a healthy number beside a
save the server will refuse.

Two defects found by walking it against a live server, both in Phase 4a's code:

  1. A rule pointing at a DORMANT segment read as healthy. listAnnotated asked
     only whether the segment ROW existed. The other shape of the same failure
     is a segment sitting exactly where it was whose every audience belongs to
     an uninstalled module: same outcome, nothing deleted. Uninstalling a module
     under an enabled rule produced a rule the screen showed as on and firing.
     The expression walk now lives in engagement/segments.js as
     `missingAudiences` and both lists ask it.
  2. "1 rule still use this segment" - the delete refusal pluralised the noun
     and not the verb, in the sentence an operator reads when told no.

Also: a rule's trigger is now a stated rule rather than an omission in the
UPDATE statement (its cooldowns, queued sends and history are all about one
trigger id); a condition tree the editor cannot render is shown read-only rather
than flattened, because flattening changes which events fire the rule; and
literals are coerced client-side to the type the trigger declared, with anything
that does not parse passed through unchanged so the server's refusal names the
variable.

Tests: 21 new server tests (test/engagementAdmin.test.js) and 25 client ones
(client/test/engagementRules.test.js), all green. The single failure in the
server suite (`the committed manifest matches the declarations in the tree`) is
the known Windows CRLF artifact and fails identically on clean edge.

Companion docs PR: docs#184.

- [x] AI-assisted: written with Claude Code (Opus)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
2026-08-29 12:10:04 -05:00
parent 4d3f574480
commit 4b45eddb5d
17 changed files with 3777 additions and 30 deletions

View File

@@ -103,6 +103,27 @@ const update = (id, rule) =>
],
)
/**
* Flip `enabled` and nothing else (Phase 4b).
*
* Deliberately NOT a call through `validate`: turning a rule OFF is the panic
* button, and it has to work on a rule the registries would now refuse — one
* whose module was uninstalled, or whose trigger has since narrowed its ceiling
* underneath a saved audience. Re-validating on the way to `enabled = 0` would
* make exactly the rules an operator most wants to stop the ones they cannot.
*
* Turning a rule ON is safe without re-validation for a different reason: the
* engine re-runs the ceiling check at send time (audiences.permitted), so an
* enabled-but-no-longer-permitted rule resolves to nobody rather than to the
* wrong people.
*/
const setEnabled = (id, enabled, updatedBy = null) =>
query('UPDATE engagement_rules SET enabled = ?, updated_by = ? WHERE id = ?', [
enabled ? 1 : 0,
updatedBy,
id,
])
const remove = (id) => query('DELETE FROM engagement_rules WHERE id = ?', [id])
/** Does any rule still point at this segment? The check before a segment delete. */
@@ -121,6 +142,7 @@ module.exports = {
enabledCancelledBy,
insert,
update,
setEnabled,
remove,
countUsingSegment,
parseJson,

View File

@@ -23,6 +23,7 @@ const segmentsDb = require('./engagementSegments.db')
const registries = require('../../modules/registries')
const ceilings = require('../../modules/ceilings')
const channels = require('../../engagement/channels')
const segmentExpressions = require('../../engagement/segments')
const conditions = require('../../engagement/conditions')
// A day. Longer than this and "cooldown" is really "send once", which a rule
@@ -192,6 +193,48 @@ async function update(id, input) {
return { ok: true, rule: await db.getById(id) }
}
/**
* Why this rule cannot currently fire, as a list of sentences. Empty = it can.
*
* **Three ways, not two.** A rule can be dormant because its trigger is gone,
* because a channel it names is gone, or because its AUDIENCE is gone - and the
* audience case has two shapes that a screen must not collapse into one:
*
* • the segment row was deleted out from under it (§7.3), or
* • the segment still exists and every audience in it belongs to a module that
* has been uninstalled (§5.1a rule 4).
*
* Both leave the rule reaching nobody. Only the first leaves nothing behind, and
* a check that asks only "does the row exist" reports the first and misses the
* second - which shows an enabled, healthy-looking rule that cannot fire. Found
* by uninstalling a module under a live rule while building Phase 4b's screen.
*
* @param {Map<number, {expression: object}>} segments every segment, by id
*/
function dormancyReasons(rule, segments) {
const reasons = []
if (!registries.eventTrigger(rule.trigger_id)) reasons.push(`trigger "${rule.trigger_id}" is not registered`)
if (rule.audience_segment_id) {
const segment = segments.get(rule.audience_segment_id)
if (!segment) reasons.push('its audience segment no longer exists')
else {
const missing = segmentExpressions.missingAudiences(segment.expression)
if (missing.length) {
reasons.push(`its audience "${segment.name}" uses ${missing.join(', ')}, which nothing registers`)
}
}
}
for (const c of rule.channels || []) if (!channels.has(c)) reasons.push(`channel "${c}" is not registered`)
return reasons
}
const annotate = (rule, segments) => {
const reasons = dormancyReasons(rule, segments)
return { ...rule, dormant: reasons.length > 0, dormantReasons: reasons }
}
const segmentsById = async () => new Map((await segmentsDb.list()).map((s) => [s.id, s]))
/**
* List every rule, each annotated with whether it can currently fire.
*
@@ -202,23 +245,59 @@ async function update(id, input) {
*/
async function listAnnotated() {
const rows = await db.list()
const segments = new Map((await segmentsDb.list()).map((s) => [s.id, s]))
return rows.map((rule) => {
const reasons = []
if (!registries.eventTrigger(rule.trigger_id)) reasons.push(`trigger "${rule.trigger_id}" is not registered`)
if (rule.audience_segment_id && !segments.has(rule.audience_segment_id)) {
reasons.push('its audience segment no longer exists')
}
for (const c of rule.channels || []) if (!channels.has(c)) reasons.push(`channel "${c}" is not registered`)
return { ...rule, dormant: reasons.length > 0, dormantReasons: reasons }
})
const segments = await segmentsById()
return rows.map((rule) => annotate(rule, segments))
}
/** One rule with the same dormancy annotation the list carries, or null. */
async function getAnnotated(id) {
const rule = await db.getById(id)
if (!rule) return null
return annotate(rule, await segmentsById())
}
/**
* Turn one rule on or off, writing that column and no other (Phase 4b).
*
* This is the one write path that does NOT go through `validate`, and the
* asymmetry is deliberate. Switching a rule OFF must always be possible - a rule
* whose module has been uninstalled, or whose trigger has since narrowed its
* ceiling under a saved audience, is exactly the rule an operator most urgently
* wants stopped, and it is exactly the rule `validate` would now refuse. The
* full editor still re-validates on save, and the engine re-checks the ceiling at
* send time, so nothing is loosened by having a switch that is only a switch.
*/
async function setEnabled(id, enabled, updatedBy = null) {
const existing = await db.getById(id)
if (!existing) return { ok: false, errors: [`no rule ${id} exists`], notFound: true }
await db.setEnabled(id, enabled, updatedBy)
return { ok: true, rule: await getAnnotated(id) }
}
/**
* Delete a rule.
*
* Its cooldown rows and any still-pending outbox rows go with it (both carry an
* ON DELETE CASCADE), and that is the right blast radius: neither means anything
* without the rule. `engagement_sends` deliberately does NOT — its `rule_id`
* carries no foreign key — so the send log outlives the rule and the record of
* what was actually mailed survives an operator tidying up.
*/
async function remove(id) {
const existing = await db.getById(id)
if (!existing) return { ok: false, errors: [`no rule ${id} exists`], notFound: true }
await db.remove(id)
return { ok: true }
}
module.exports = {
validate,
create,
update,
setEnabled,
remove,
listAnnotated,
getAnnotated,
MAX_COOLDOWN_SECONDS,
MAX_DELAY_SECONDS,
MAX_SENDS_PER_HOUR,

View File

@@ -11,7 +11,6 @@
const db = require('./engagementSegments.db')
const rulesDb = require('./engagementRules.db')
const registries = require('../../modules/registries')
const segments = require('../../engagement/segments')
async function save(input, { id = null } = {}) {
@@ -56,7 +55,7 @@ async function remove(id) {
return {
ok: false,
inUse,
errors: [`${inUse} rule${inUse === 1 ? '' : 's'} still use this segment`],
errors: [`${inUse} rule${inUse === 1 ? ' still uses' : 's still use'} this segment`],
}
}
await db.remove(id)
@@ -73,14 +72,11 @@ async function remove(id) {
async function listAnnotated() {
const rows = await db.list()
return rows.map((segment) => {
const missing = []
const walk = (node) => {
if (!node || typeof node !== 'object') return
if (node.op) (node.nodes || []).forEach(walk)
else if (!registries.audience(node.audienceId)) missing.push(node.audienceId)
}
walk(segment.expression)
return { ...segment, dormant: missing.length > 0, missingAudiences: [...new Set(missing)] }
// The walk lives in segments.js so the rule list can ask the same question:
// a rule pointing at a DORMANT segment is dormant too, and asking only
// whether the segment row still exists misses that (§5.1a rule 4).
const missing = segments.missingAudiences(segment.expression)
return { ...segment, dormant: missing.length > 0, missingAudiences: missing }
})
}