feat(engagement): Admin → Engagement → Rules and Audiences (engagement Phase 4b) #171
Reference in New Issue
Block a user
No description provided.
Delete Branch "feature/engagement-rules-admin"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Engagement Phase 4b — the admin surface over the Phase 4a engine. Companion docs PR: docs#184.
Two screens, twelve routes and a reach preview. Nothing in the engine changed; what changed is that an operator can reach it, which is what turns 4a's model into rules a person will actually meet.
enabledstill defaults to 0 and core still seeds no rules, so this PR does not start any mail — it makes starting some possible.Four decisions settled before any code
PATCH …/enabled, or a fullPUTQ4's answer from Phase 4a lands here too: Engagement is its own top-level nav group, beside Content / Moderation / System, with Rules and Audiences in it now and Triggers / Templates / the send log joining in Phase 5. Email Delivery stays a section of Settings.
The enable switch is a correctness argument, not a convenience
A
PUTre-validates against the registries as they are now. So the rules a re-validating toggle cannot switch off are exactly three: a rule whose module has been uninstalled, one naming a channel that is gone, and one whose trigger has since narrowed its ceiling underneath a saved audience. Those are the three rules an operator most urgently wants stopped — a toggle built onPUTis broken in precisely the case it is needed.PATCH /admin/engagement/rules/:id/enabledwrites one column and always works. Switching a rule on without re-validation is safe for a different reason:audiences.permittedruns again at send time, so an enabled-but-no-longer-permitted rule resolves to nobody rather than to the wrong people. There is a test for each half — the dormant rule that switches off, and the full update of that same rule being refused.The reach preview: a count, and three ways it refuses to lie
It calls the engine's own resolver (
audiences.resolveForRule) rather than a second query that agrees with it today — a preview built out of its own SQL can be wrong about the one thing it exists to say.It answers a count and nothing else. Not a sample, not names: the resolver's output for a module-declared segment is a set of players derived from game data, and an editor that rendered them would be a user-enumeration surface reached from a screen about mail scheduling. A test asserts the response carries no identities.
capped— every audience query is bounded at 5000, so a count landing on the bound is a floor. The screen says "at least 5000", never "5000".reason— anowneraudience resolves per event from an id the event carries, so it has no advance answer. It reports 0 with the reason; a bare 0 reads as "nobody".permitted— whether the trigger's G24 ceiling allows the reach just counted. Without it the editor shows a healthy number beside a save the server will refuse, which reads as a bug in the save rather than as the ceiling doing its job.Two defects found by walking the API against a live server — both in Phase 4a's code
1. A rule pointing at a dormant segment read as healthy.
listAnnotatedasked only whether the segment row still existed — §7.3's case, the segment was deleted. The other shape of the same failure is §5.1a rule 4's: the segment row is exactly where it was and every audience in it belongs to a module that has been uninstalled. Both leave the rule reaching nobody; only one leaves nothing behind. Uninstalling a module under an enabled rule produced a rule the new screen showed as on, healthy and firing, which is the one thing this screen exists not to do.The expression walk now lives in
engagement/segments.jsasmissingAudiences, and both the segment list and the rule list ask it, so the two cannot disagree about what dormant means. Verified live, and pinned by a test that leaves the segment row in place and takes the module away.2.
"1 rule still use this segment". The delete refusal pluralised the noun and not the verb — in the sentence an operator reads at the moment they are being told no.Six more from driving the screens in a browser
None of these is visible from a test or from
curl, and two of them cost an operator something real:audienceChoicesFor(null)answers[]— correctly, because without a trigger there is no ceiling and so nothing it may legitimately offer. But a<select>with zero options reads as a control that is broken, not one that is waiting. It now says "Choose a trigger first…" and is disabled.membersaudience with no saved audience reaches nobody, and only the preview button said so. That is the design (§5.1a) — and it is also the default the instant an operator picks anymembers-ceiling trigger, which turns a documented property into a trap: the rule saves, gets switched on, and mails nobody, with nothing on screen saying so unless the operator happens to press Preview. Said inline now, before the save, and it stands down once a preview has answered the same question more precisely.andwhose every child is a complement. The server refuses that correctly — "has nothing but complements, there is no set to exclude from" — but only after a save, and it is one checkbox away at all times. Refused inline now, in the operator's words.All six verified in the browser after the fix, and each is pinned by a test where there is something to pin.
Three smaller things the screens decided
engagementRules.db.updatenever carriedtrigger_id; this is where that becomes a stated rule rather than an omission. A rule's cooldown rows, its pending outbox rows and its send-log history are all about one trigger id, and re-pointing the rule silently re-attributes all three. The editor renders the field read-only and says why.A AND (B OR C)flattened toA AND B AND Cfires on different events and the operator would have no way to know the save had done it, so such a rule opens read-only with its JSON and one honest choice: leave it, or clear it.{cmp: 'gt', value: "5"}against anint. A value that does not parse is passed through unchanged rather than becomingNaNorfalse, so the refusal names the variable instead of the rule saving cleanly having compared against a number nobody typed.Where the logic lives
client/src/lib/engagementRules.js— plain JS, 28 tests — holds everything the screens decide: the form↔payload mapping, which audiences a trigger permits (derived from thepermitslist the server serves, never a second copy of the lattice), the reach sentence, thenot-placement check, the condition round trip. None of it is a boundary — the server decides and the engine re-checks — so the client holds the affordance (do not offer what will be refused) and the server holds the answer. It is a.jsbecause the test runner cannot reach a.jsx.Verification
test/engagementAdmin.test.js) and 28 client tests (client/test/engagementRules.test.js), all green. Client suite 316/316; client build clean.the committed manifest matches the declarations in the tree— is the known Windows CRLF artifact and fails identically on cleanedge.routes.manifestandroutes.guardswere the other two and are green here because this PR regenerates them.409with its count, theOR-takes-the-tighter-ceiling arithmetic,members AND NOT staffbeing allowed, two incomparable ceilings being refused, the dormancy path in both directions, and a rule composed, saved, summarised, switched on and deleted from the screens themselves.modules/rig/, gitignored) declaring four audiences at three ceilings is what made that walk possible; it is recorded in the docs PR for whoever needs it next. Its ids must be namespaced to the module's own id, and must contain a dot.modules/uomoved aside).Two environment facts the browser pass turned up, recorded in the docs PR: rebuilding
client/distwhile the server is running blanks the whole SPA (the HTML shell resolves core's hashed bundle name at boot, sowindow.__rgis never published andmodules/uo/entry.jsthrows MODULE_API.md §3.1's error into a blank page — the error names core, and core is not at fault); and awindow.confirmblocks CDP entirely, so the two destructive actions cannot be driven from a script.🤖 Generated with Claude Code
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>Driving the two screens in Chrome, after the API walk had already found the two in Phase 4a's code. None of these is visible from a test or from curl. Two cost an operator something real: - The Audience dropdown rendered EMPTY before a trigger was chosen. There is genuinely nothing it may offer without a ceiling, but a select with zero options reads as broken rather than as waiting. It now says "Choose a trigger first..." and is disabled. - A `members` audience with no saved audience reaches NOBODY, and only the preview button said so. That is the design, but it is also the default the instant a members-ceiling trigger is picked - so the rule saves, gets switched on, and mails nobody with nothing on screen saying so. The editor now says it inline, and stands down once a preview has answered the same question more precisely. One the server was already refusing, just too late: - The composer offered "exclude" on the only row, building an `and` whose every child is a complement. The server refuses it correctly but only after a save, and it is one checkbox away at all times. Now refused inline, in the operator's words. Three wording and layout: - the template-key input truncated its placeholder, and said "optional until Phase 5" - a sentence about the plan document, not about the deployment - "segment" leaked into a screen that says "saved audience" everywhere else. The API, schema and docs keep saying segment (one word for one table); translated at the point of display only - the composer repeated its AUDIENCE heading above every row Client only - no server change, so swagger and the route manifest are untouched. Client suite 316/316; all six verified in the browser after the fix. - [x] AI-assisted: written with Claude Code (Opus) Co-Authored-By: Claude <noreply@anthropic.com>