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>
308 lines
14 KiB
JavaScript
308 lines
14 KiB
JavaScript
import { test } from 'node:test'
|
|
import assert from 'node:assert/strict'
|
|
import {
|
|
formFromRule,
|
|
ruleToPayload,
|
|
audienceChoicesFor,
|
|
segmentChoicesFor,
|
|
describeReach,
|
|
describeRule,
|
|
describeExpression,
|
|
notPlacementError,
|
|
audienceWarning,
|
|
operatorWords,
|
|
conditionRowsFrom,
|
|
conditionsFromRows,
|
|
operatorsForType,
|
|
coerceLiteral,
|
|
humanSeconds,
|
|
} from '../src/lib/engagementRules.js'
|
|
|
|
// lib/engagementRules.js — what the two Engagement screens say and what they let
|
|
// an operator pick (ENGAGEMENT.md Phase 4b).
|
|
//
|
|
// None of this is a boundary: the server's `engagementRules.model` decides what
|
|
// may be saved and the engine re-checks the audience ceiling at send time. What
|
|
// is tested here is the part that would be wrong SILENTLY — a form that sends a
|
|
// string where the trigger declared an int, a composer that flattens a nested
|
|
// condition into one that fires on different events, an editor that offers an
|
|
// audience the save is going to refuse.
|
|
|
|
const CEILINGS = [
|
|
{ id: 'everyone', label: 'Everyone', permits: ['everyone', 'authenticated', 'subscribers', 'members', 'staff', 'owner'] },
|
|
{ id: 'authenticated', label: 'Signed-in users', permits: ['authenticated', 'subscribers', 'members', 'staff', 'owner'] },
|
|
{ id: 'subscribers', label: 'Subscribers', permits: ['subscribers'] },
|
|
{ id: 'members', label: 'A module list', permits: ['members'] },
|
|
{ id: 'staff', label: 'Staff', permits: ['staff'] },
|
|
{ id: 'owner', label: 'The person it is about', permits: ['owner'] },
|
|
]
|
|
|
|
const TRIGGER = {
|
|
id: 'uo.house.idoc_warning',
|
|
label: 'House approaching collapse',
|
|
ceiling: 'owner',
|
|
audience: 'owner',
|
|
subjectKey: 'house',
|
|
variables: [
|
|
{ name: 'house', type: 'string', required: true },
|
|
{ name: 'daysLeft', type: 'int', required: false },
|
|
{ name: 'insured', type: 'boolean', required: false },
|
|
],
|
|
}
|
|
|
|
const OPERATORS = [
|
|
{ cmp: 'eq', label: 'is', types: ['string', 'int', 'boolean'], arity: 1 },
|
|
{ cmp: 'gt', label: 'is greater than', types: ['int'], arity: 1 },
|
|
{ cmp: 'in', label: 'is one of', types: ['string', 'int'], arity: 'list' },
|
|
{ cmp: 'present', label: 'is present', types: ['string', 'int', 'boolean'], arity: 0 },
|
|
]
|
|
|
|
const row = (over = {}) => ({
|
|
id: 3,
|
|
trigger_id: 'uo.house.idoc_warning',
|
|
name: 'IDOC warning',
|
|
enabled: 1,
|
|
audience: 'owner',
|
|
audience_segment_id: null,
|
|
channels: ['email'],
|
|
template_keys: { email: 'idoc-warning' },
|
|
conditions: null,
|
|
cooldown_seconds: 86400,
|
|
delay_seconds: 0,
|
|
cancel_on: [],
|
|
max_sends_per_hour: 100,
|
|
...over,
|
|
})
|
|
|
|
// ── The form round trip ────────────────────────────────────────────────────
|
|
|
|
test('a rule row round-trips through the form without changing what it means', () => {
|
|
const payload = ruleToPayload(formFromRule(row()))
|
|
|
|
assert.equal(payload.triggerId, 'uo.house.idoc_warning')
|
|
assert.equal(payload.enabled, true)
|
|
assert.deepEqual(payload.channels, ['email'])
|
|
assert.deepEqual(payload.templateKeys, { email: 'idoc-warning' })
|
|
assert.equal(payload.cooldownSeconds, 86400)
|
|
assert.equal(payload.maxSendsPerHour, 100)
|
|
})
|
|
|
|
test('unticking a channel drops its template key, rather than sending one the server refuses', () => {
|
|
const form = formFromRule(row({ channels: ['email', 'push'], template_keys: { email: 'a', push: 'b' } }))
|
|
form.channels = ['email']
|
|
|
|
const payload = ruleToPayload(form)
|
|
|
|
// The server refuses `templateKeys` naming a channel the rule does not have.
|
|
// Leaving it in would produce an error about a field the operator cannot see.
|
|
assert.deepEqual(payload.templateKeys, { email: 'a' })
|
|
})
|
|
|
|
// ── The audience the editor may offer ──────────────────────────────────────
|
|
|
|
test('the editor offers only what the trigger ceiling permits', () => {
|
|
const choices = audienceChoicesFor(TRIGGER, CEILINGS).map((c) => c.id)
|
|
assert.deepEqual(choices, ['owner'])
|
|
})
|
|
|
|
test('a wider trigger offers more, in lattice order', () => {
|
|
const choices = audienceChoicesFor({ ...TRIGGER, ceiling: 'authenticated' }, CEILINGS).map((c) => c.id)
|
|
assert.deepEqual(choices, ['authenticated', 'subscribers', 'members', 'staff', 'owner'])
|
|
})
|
|
|
|
test('an unknown trigger offers nothing — failing closed, like the server', () => {
|
|
// This is a dormant rule, whose module has been uninstalled. Offering the full
|
|
// vocabulary would be the widening the whole ceiling design exists to prevent.
|
|
assert.deepEqual(audienceChoicesFor({ ...TRIGGER, ceiling: 'nonsense' }, CEILINGS), [])
|
|
assert.deepEqual(audienceChoicesFor(null, CEILINGS), [])
|
|
})
|
|
|
|
test('segments are filtered by their STORED ceiling, not re-derived', () => {
|
|
const segments = [
|
|
{ id: 1, name: 'Governors', ceiling: 'members' },
|
|
{ id: 2, name: 'Watchers', ceiling: 'authenticated' },
|
|
]
|
|
const wide = segmentChoicesFor({ ...TRIGGER, ceiling: 'authenticated' }, CEILINGS, segments)
|
|
assert.deepEqual(wide.map((s) => s.id), [1, 2])
|
|
|
|
const narrow = segmentChoicesFor({ ...TRIGGER, ceiling: 'members' }, CEILINGS, segments)
|
|
assert.deepEqual(narrow.map((s) => s.id), [1])
|
|
})
|
|
|
|
// ── The reach preview ──────────────────────────────────────────────────────
|
|
|
|
test('a capped count reads as a floor, never as a total', () => {
|
|
const said = describeReach({ count: 5000, capped: true, dormant: false, reason: null, permitted: true })
|
|
assert.match(said, /At least 5000/)
|
|
})
|
|
|
|
test('a count the trigger would refuse says so, instead of looking healthy', () => {
|
|
const said = describeReach({ count: 12, capped: false, dormant: false, reason: null, permitted: false })
|
|
assert.match(said, /will be refused/)
|
|
})
|
|
|
|
test('a dormant segment says why, rather than reading as "nobody"', () => {
|
|
const said = describeReach({ count: 0, dormant: true, reason: 'audience segment is dormant' })
|
|
assert.match(said, /dormant/)
|
|
})
|
|
|
|
test('an owner audience carries its reason forward', () => {
|
|
const said = describeReach({ count: 0, dormant: false, reason: 'event carries no ownerUserId', permitted: true })
|
|
assert.match(said, /ownerUserId/)
|
|
})
|
|
|
|
// ── Conditions ─────────────────────────────────────────────────────────────
|
|
|
|
test('operators narrow to the variable type that was picked', () => {
|
|
assert.deepEqual(operatorsForType(OPERATORS, 'boolean').map((o) => o.cmp), ['eq', 'present'])
|
|
assert.deepEqual(operatorsForType(OPERATORS, 'int').map((o) => o.cmp), ['eq', 'gt', 'in', 'present'])
|
|
})
|
|
|
|
test('a literal is coerced to the type the trigger DECLARED', () => {
|
|
// Every value in an HTML input is a string, and `{ cmp: 'gt', value: "5" }`
|
|
// against an int variable is refused by the server — rightly, because a
|
|
// comparison between a number and a string quietly never matches.
|
|
const built = conditionsFromRows('and', [{ variable: 'daysLeft', cmp: 'gt', value: '5' }], TRIGGER.variables)
|
|
assert.deepEqual(built, { variable: 'daysLeft', cmp: 'gt', value: 5 })
|
|
})
|
|
|
|
test('a value that does not parse is passed through, so the server names the field', () => {
|
|
// NOT NaN, and not 0: a rule that saves cleanly having silently compared
|
|
// against a number nobody typed is worse than a refusal that says which
|
|
// variable it was.
|
|
assert.equal(coerceLiteral('int', 'soon'), 'soon')
|
|
assert.equal(coerceLiteral('boolean', 'yes'), 'yes')
|
|
assert.equal(coerceLiteral('boolean', 'true'), true)
|
|
assert.equal(coerceLiteral('float', '1.5'), 1.5)
|
|
})
|
|
|
|
test('a list operator splits on commas and types each item', () => {
|
|
const built = conditionsFromRows('and', [{ variable: 'daysLeft', cmp: 'in', value: '1, 2, 3' }], TRIGGER.variables)
|
|
assert.deepEqual(built.value, [1, 2, 3])
|
|
})
|
|
|
|
test('present and absent carry no value at all', () => {
|
|
const built = conditionsFromRows('and', [{ variable: 'house', cmp: 'present', value: 'ignored' }], TRIGGER.variables)
|
|
assert.deepEqual(built, { variable: 'house', cmp: 'present' })
|
|
})
|
|
|
|
test('no rows means no conditions — not an empty group that matches nothing', () => {
|
|
assert.equal(conditionsFromRows('and', [], TRIGGER.variables), null)
|
|
assert.equal(conditionsFromRows('and', [{ variable: '', cmp: '' }], TRIGGER.variables), null)
|
|
})
|
|
|
|
test('a flat stored tree opens editable; a nested one opens read-only', () => {
|
|
const flat = conditionRowsFrom({
|
|
op: 'and',
|
|
nodes: [{ variable: 'house', cmp: 'eq', value: 'x' }, { variable: 'daysLeft', cmp: 'gt', value: 5 }],
|
|
})
|
|
assert.equal(flat.editable, true)
|
|
assert.equal(flat.rows.length, 2)
|
|
|
|
// `A AND (B OR C)` flattened to `A AND B AND C` fires on different events, and
|
|
// the operator would have no way to know the save had done it.
|
|
const nested = conditionRowsFrom({
|
|
op: 'and',
|
|
nodes: [
|
|
{ variable: 'house', cmp: 'eq', value: 'x' },
|
|
{ op: 'or', nodes: [{ variable: 'daysLeft', cmp: 'gt', value: 5 }] },
|
|
],
|
|
})
|
|
assert.equal(nested.editable, false)
|
|
assert.deepEqual(nested.rows, [])
|
|
})
|
|
|
|
test('a single stored comparison is one editable row', () => {
|
|
const one = conditionRowsFrom({ variable: 'house', cmp: 'eq', value: 'x' })
|
|
assert.equal(one.editable, true)
|
|
assert.deepEqual(one.rows, [{ variable: 'house', cmp: 'eq', value: 'x' }])
|
|
})
|
|
|
|
// ── Segment composition ────────────────────────────────────────────────────
|
|
|
|
test('a members audience with no saved audience is warned about BEFORE the save', () => {
|
|
// The trap the browser walk found: it is the default the moment a
|
|
// members-ceiling trigger is chosen, and the rule it produces saves, switches
|
|
// on and mails nobody. Nothing on the screen said so unless you pressed
|
|
// Preview.
|
|
assert.match(audienceWarning({ audience: 'members', audienceSegmentId: null }), /reaches nobody/)
|
|
assert.equal(audienceWarning({ audience: 'members', audienceSegmentId: 4 }), null)
|
|
assert.equal(audienceWarning({ audience: 'owner', audienceSegmentId: null }), null)
|
|
})
|
|
|
|
test('the server says "segment"; the screens say "saved audience"', () => {
|
|
// One word for one table in the API, the schema and the docs. But an operator
|
|
// meets the concept under a heading that says "Audiences", and a sentence that
|
|
// switches vocabulary mid-screen reads as being about something else.
|
|
assert.equal(operatorWords('audience segment is dormant'), 'audience saved audience is dormant')
|
|
assert.match(describeReach({ count: 0, dormant: true, reason: 'audience segment is dormant' }), /saved audience/)
|
|
// and it does not maul a word that merely contains it
|
|
assert.equal(operatorWords('segmented data'), 'segmented data')
|
|
})
|
|
|
|
test('a list of nothing but exclusions is refused before the round trip', () => {
|
|
// One checkbox away at all times, because the composer offers "exclude" on
|
|
// every row including the only one. The server refuses it correctly — but
|
|
// only after a save.
|
|
const err = notPlacementError({ op: 'and', nodes: [{ op: 'not', nodes: [{ audienceId: 'a' }] }] })
|
|
assert.match(err, /at least one audience/i)
|
|
})
|
|
|
|
test('a bare not is refused before it reaches the server', () => {
|
|
assert.ok(notPlacementError({ op: 'not', nodes: [{ audienceId: 'uo.governors' }] }))
|
|
assert.ok(notPlacementError({ op: 'or', nodes: [{ audienceId: 'a' }, { op: 'not', nodes: [{ audienceId: 'b' }] }] }))
|
|
})
|
|
|
|
test('a not under an "all of" is fine — that is the only universe that does not widen', () => {
|
|
assert.equal(
|
|
notPlacementError({
|
|
op: 'and',
|
|
nodes: [{ audienceId: 'uo.governors' }, { op: 'not', nodes: [{ audienceId: 'uo.flagged' }] }],
|
|
}),
|
|
null,
|
|
)
|
|
})
|
|
|
|
test('an expression describes itself with module labels where it has them', () => {
|
|
const byId = { 'uo.governors': { label: 'Governors' } }
|
|
const said = describeExpression(
|
|
{ op: 'and', nodes: [{ audienceId: 'uo.governors' }, { op: 'not', nodes: [{ audienceId: 'uo.flagged' }] }] },
|
|
byId,
|
|
)
|
|
assert.equal(said, 'Governors and not uo.flagged')
|
|
})
|
|
|
|
test('a leaf renders its parameters, so two rows built on the same audience are distinguishable', () => {
|
|
const said = describeExpression({ audienceId: 'uo.team.members', params: { teamId: 4 } }, {})
|
|
assert.equal(said, 'uo.team.members (teamId: 4)')
|
|
})
|
|
|
|
// ── The list summary ───────────────────────────────────────────────────────
|
|
|
|
test('a rule summarises to what it will do, and always names its hourly cap', () => {
|
|
const said = describeRule(row({ delay_seconds: 3600 }), { segmentsById: {} })
|
|
assert.match(said, /to owner/)
|
|
assert.match(said, /via email/)
|
|
assert.match(said, /after 1 hour/)
|
|
assert.match(said, /once per 1 day/)
|
|
assert.match(said, /100\/hour/)
|
|
})
|
|
|
|
test('a rule on a segment names the segment, not the ceiling column', () => {
|
|
// The `audience` column on such a rule holds the segment's ceiling, which is a
|
|
// fact about what it MAY reach and not about who it does.
|
|
const said = describeRule(row({ audience: 'members', audience_segment_id: 7 }), {
|
|
segmentsById: { 7: { name: 'Governors' } },
|
|
})
|
|
assert.match(said, /to Governors/)
|
|
})
|
|
|
|
test('humanSeconds picks the coarsest EXACT unit, and never rounds', () => {
|
|
assert.equal(humanSeconds(0), 'none')
|
|
assert.equal(humanSeconds(3600), '1 hour')
|
|
assert.equal(humanSeconds(86400), '1 day')
|
|
assert.equal(humanSeconds(7200), '2 hours')
|
|
assert.equal(humanSeconds(3660), '61 minutes')
|
|
assert.equal(humanSeconds(90), '90 seconds')
|
|
})
|