diff --git a/src/objects/catalog-item.object.ts b/src/objects/catalog-item.object.ts index 21fec27..26357ca 100644 --- a/src/objects/catalog-item.object.ts +++ b/src/objects/catalog-item.object.ts @@ -112,12 +112,41 @@ export const CatalogItem = ObjectSchema.create({ nameField: 'name', highlightFields: ['name', 'position_code', 'form', 'frequency'], - // Mirrors the three new `duly_duty` rules (#61) — not its full validation - // set. `duly_duty`'s `recurring_needs_frequency` and `effective_window_ordered` - // are pre-existing gaps on THIS object (no `effective_*` fields exist here - // at all, and nothing currently requires a recurring item to carry a - // frequency); left alone as out of this issue's scope and filed separately. + // Mirrors `duly_duty`'s cadence rules (#61, #65) — not its full validation + // set. `effective_window_ordered` has no equivalent here and never will: + // this object declares no `effective_from` / `effective_to` at all, so + // there is no window to order. That is the one deliberate divergence. validations: [ + { + // The gap #65 closes, and the LAST place a blank recurring frequency + // can be caught. Measured on @objectstack/runtime 17.2.0, not assumed: + // + // - On INSERT the conditional default masks it. `applyFieldDefaults` + // treats an explicit `frequency: null` as absent and re-stamps + // `"monthly"` from the CEL default two dozen lines up, so an insert + // could never have reached this rule blank in the first place. + // - On UPDATE nothing runs. `applyFieldDefaults` is INSERT-only, so + // `{ frequency: null }` on a still-recurring item landed with no + // refusal — the one write that produces the state, and the one this + // rule exists for. + // - Downstream does NOT catch it either. `applyCatalogHandler` copies + // `frequency` onto every new `duly_duty` verbatim, and that insert + // hits the SAME default-masking: the duty is stamped `"monthly"` + // and `duly_duty`'s own `recurring_needs_frequency` never fires. So + // a blank here does not fan out as N loud refusals — it fans out as + // N duties silently dispatching on a monthly cadence nobody chose. + // Pinned in `test/catalog-apply-cadence.test.ts`. + // + // Wording is `duly_duty`'s, verbatim, as the three rules below already + // are: a catalog item IS a duty template ("Duty" is even this object's + // `name` label), and one message per cadence rule is what lets the + // suites assert both objects against one constant. + name: 'recurring_needs_frequency', + type: 'script', + severity: 'error', + message: 'A recurring duty needs a frequency — otherwise nothing can dispatch it.', + condition: P`record.form == "recurring" && isBlank(record.frequency)`, + }, { name: 'standing_no_frequency', type: 'script', diff --git a/test/cadence-conditional-defaults.test.ts b/test/cadence-conditional-defaults.test.ts index 3363242..951f2ce 100644 --- a/test/cadence-conditional-defaults.test.ts +++ b/test/cadence-conditional-defaults.test.ts @@ -4,6 +4,7 @@ import { afterAll, beforeAll, describe, expect, it } from 'vitest'; import { AppPlugin, ObjectKernel, createStandaloneStack } from '@objectstack/runtime'; import stack from '../objectstack.config.js'; +import { CatalogItem, Duty } from '../src/objects/index.js'; import { DEFAULT_DUE_ANCHOR, DEFAULT_DUE_OFFSET_DAYS, DEFAULT_LEAD_DAYS } from '../src/jobs/dispatch.plan.js'; /** @@ -101,7 +102,15 @@ const insertCatalogItem = async (over: AnyRow): Promise => { const readDuty = async (id: string): Promise => (await data.find('duly_duty', { where: { id }, limit: 1 }))[0] as AnyRow; +const readCatalogItem = async (id: string): Promise => + (await data.find('duly_catalog_item', { where: { id }, limit: 1 }))[0] as AnyRow; + const CADENCE_MESSAGES = { + // Asserted against BOTH objects, which is the point of it being one + // constant: `duly_catalog_item`'s rules are `duly_duty`'s wording verbatim + // (a catalog item is a duty template), so a message that drifted on one + // object would fail here rather than shipping two answers for one rule. + recurring: 'A recurring duty needs a frequency — otherwise nothing can dispatch it.', frequency: 'A standing duty never dispatches — a frequency on it is meaningless. Remove it.', timing: 'Due anchor, due offset and lead time compute a period due date — only a recurring duty has one. Clear them for standing and one-off.', @@ -208,7 +217,7 @@ describe('negative control — recurring_needs_frequency must still fire (#61 mu const created = await insertDuty({ form: 'recurring' }); const { code, message } = await refusal(data.update('duly_duty', { id: created.id, frequency: null })); expect(code).toBe('VALIDATION_FAILED'); - expect(message).toBe('A recurring duty needs a frequency — otherwise nothing can dispatch it.'); + expect(message).toBe(CADENCE_MESSAGES.recurring); // And the row itself must be untouched by the refused write. expect((await readDuty(String(created.id))).frequency).toBe('monthly'); }); @@ -272,6 +281,90 @@ describe('duly_catalog_item — mirrors duly_duty field-for-field (#61)', () => }); }); +describe('duly_catalog_item — recurring_needs_frequency (#65)', () => { + // The rule `duly_duty` has carried all along and this object never had. + // #61 mirrored the three STANDING/non-recurring rules here and left this + // one — the converse direction — as a pre-existing gap; #65 closes it. + // + // Why it matters more here than on a duty: `applyCatalogHandler` copies + // `frequency` onto every duty it creates, so one blank catalog item is + // replicated onto every person who takes the role. What that replication + // actually does is pinned in `test/catalog-apply-cadence.test.ts`, and it + // is NOT "each of those duties trips the duty's own rule". + // + // The refusals below are asserted by ENVELOPE (ADR-0112). In-process the + // throw carries `code`, `name` and `fields` and no `status` — that is added + // at the HTTP boundary — so `code` plus the exact message is the whole + // pin available on this path, and `fields` is `_record` for every + // record-scoped rule, which discriminates nothing. + + it('refuses an update that blanks frequency on a still-recurring catalog item', async () => { + // THE gap, in the one write that can reach it. `applyFieldDefaults` runs + // on INSERT only, so before this rule the update below landed silently: + // an org-wide template for a role went blank with nothing to say so. + const created = await insertCatalogItem({ form: 'recurring' }); + expect(created.frequency).toBe('monthly'); + + const { code, message } = await refusal( + data.update('duly_catalog_item', { id: created.id, frequency: null }), + ); + expect(code).toBe('VALIDATION_FAILED'); + expect(message).toBe(CADENCE_MESSAGES.recurring); + + // The refused write left the row alone — a rule that reported and wrote + // anyway would be worse than no rule. + expect((await readCatalogItem(String(created.id))).frequency).toBe('monthly'); + }); + + it('an INSERT never reaches the rule blank — the conditional default masks it', async () => { + // Measured, and the reason this gap outlived #61: `applyFieldDefaults` + // treats an explicit `frequency: null` as ABSENT and re-stamps the CEL + // default, so no insert ever produced the state the rule refuses. This is + // an assumption the rule's scope rests on, so it is a test and not a + // comment: if defaults ever stopped masking null, this goes red instead + // of the insert path quietly starting to need the rule too. + const row = await insertCatalogItem({ form: 'recurring', frequency: null }); + expect(row.frequency).toBe('monthly'); + }); + + it('does not fire for the forms that legitimately carry no frequency', async () => { + // A standing item's blank frequency is REQUIRED by `standing_no_frequency` + // three rules up. A recurring-only rule that over-fired here would make + // the pair jointly unsatisfiable and lock standing items out of the + // catalog entirely — which is why this control is worth its line. + await expect(insertCatalogItem({ form: 'standing' })).resolves.toBeTruthy(); + + // A one-off is dispatched by hand and has no period, so a blank frequency + // on it is legal — `duly_duty`'s rule is scoped to `recurring` for the + // same reason (see the cadence block in duty.object.ts). + const oneOff = await insertCatalogItem({ form: 'one_off' }); + await expect( + data.update('duly_catalog_item', { id: oneOff.id, frequency: null }), + ).resolves.toBeTruthy(); + }); + + it('still admits a recurring item that states its own frequency', async () => { + const row = await insertCatalogItem({ form: 'recurring', frequency: 'annual' }); + expect(row.frequency).toBe('annual'); + await expect( + data.update('duly_catalog_item', { id: row.id, frequency: 'weekly' }), + ).resolves.toBeTruthy(); + }); + + it('is declared on both objects under one name with one message', () => { + // Structural, deliberately: this claim is about the DECLARATIONS agreeing, + // and the behavioural halves are the two suites either side of it. A + // divergence here is how "mirrored verbatim" quietly becomes two rules. + const rule = (o: typeof Duty | typeof CatalogItem) => + (o.validations ?? []).find((v) => v.name === 'recurring_needs_frequency'); + + expect(rule(Duty)?.message).toBe(CADENCE_MESSAGES.recurring); + expect(rule(CatalogItem)?.message).toBe(CADENCE_MESSAGES.recurring); + expect(rule(Duty)?.severity).toBe('error'); + expect(rule(CatalogItem)?.severity).toBe('error'); + }); +}); + describe('#5 instantiation: a blank catalog-side cadence stays blank on the duty it produces', () => { it('copying a standing catalog item verbatim onto a duty still yields no cadence fields', async () => { // `applyCatalogHandler` (catalog.handlers.ts) copies frequency/due_anchor/ diff --git a/test/catalog-apply-cadence.test.ts b/test/catalog-apply-cadence.test.ts new file mode 100644 index 0000000..943bcd5 --- /dev/null +++ b/test/catalog-apply-cadence.test.ts @@ -0,0 +1,267 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { AppPlugin, ObjectKernel, createStandaloneStack } from '@objectstack/runtime'; + +import stack from '../objectstack.config.js'; +import { registerDulyActionHandlers } from '../src/actions/register-handlers.js'; +import { + CADENCE_FIELDS, + CATALOG_APPLY_ACTION, + GLOBAL_ACTION_OBJECT, +} from '../src/actions/catalog.handlers.js'; +import type { CatalogApplyResult } from '../src/actions/catalog.handlers.js'; + +/** + * #65 — what the catalog APPLY path does with a cadence value. + * + * `duly_catalog_item` now carries `recurring_needs_frequency` + * (`catalog-item.object.ts`). `test/cadence-conditional-defaults.test.ts` + * proves the rule refuses the write that produces a blank. This suite covers + * the other half, and it is the half that decides whether the rule belongs on + * the catalog item at all: a rule that fires on a direct write but leaves the + * apply path unprotected is half a fix. + * + * ── Why a REAL booted engine and not `catalog-instantiate.test.ts`'s fake ── + * That suite's `FakeEngine` is a Map with a `where` matcher: it runs no + * validation rules and stamps no defaults, so every claim below would pass on + * it for the wrong reason. Validation and `applyFieldDefaults` are precisely + * what is under test here, so the handler is dispatched through the app's own + * `executeAction` against an in-memory ObjectQL engine — the same harness + * `task-actions.test.ts` uses, and for the same reason. + */ + +type AnyRow = Record; + +let kernel: any; +let data: any; + +beforeAll(async () => { + const { plugins } = await createStandaloneStack({ + databaseDriver: 'memory', + skipSeedData: true, + // Must not resolve to a real path: a local `pnpm build` would make this + // suite report on the last BUILD instead of on `src/`. See the sibling + // suites' identical note. + artifactPath: 'dist/objectstack.this-suite-must-not-load-an-artifact.json', + }); + kernel = new ObjectKernel(); + for (const plugin of plugins) await kernel.use(plugin); + await kernel.use(new AppPlugin(stack, undefined, { skipSeedData: true })); + await kernel.bootstrap(); + data = kernel.getService('data'); + + // Through the REAL registration function, so a handler dropped from + // `registerDulyActionHandlers` surfaces here as the 404 the console gets. + registerDulyActionHandlers(data); +}, 180_000); + +afterAll(async () => { + await kernel?.shutdown?.(); +}); + +// ── The two engine facades ────────────────────────────────────────────────── + +interface Facade { + insert(object: string, values: AnyRow): Promise<{ id: string }>; + update(object: string, id: string, values: AnyRow): Promise; + delete(object: string, id: string): Promise; + find(object: string, query: AnyRow): Promise; +} + +/** + * The facade `applyCatalogHandler` is WRITTEN against: `find(object, query)` + * takes ObjectQL's own query envelope, `where` and all. It is the convention + * `catalog-instantiate.test.ts`'s `FakeEngine` honours too, so this is the + * shape every existing assertion about the handler is made under. + * + * `catalogItems`, when given, replaces the catalog read with rows handed + * straight to the handler (unfiltered — the handler re-applies its own + * `active` filter). That is how a row the object's own rules now REFUSE can + * still be put in front of the handler, which one test below needs. + */ +function handlerFacade(catalogItems?: AnyRow[]): Facade { + return { + insert: async (object, values) => { + const row = await data.insert(object, values); + return { id: row?.id }; + }, + update: async (object, id, values) => { + await data.update(object, values, { where: { id } }); + }, + delete: async (object, id) => { + await data.delete(object, { where: { id } }); + }, + find: async (object, query) => { + if (catalogItems && object === 'duly_catalog_item') return catalogItems.map((r) => ({ ...r })); + return data.find(object, query); + }, + }; +} + +/** + * The facade the RUNTIME actually builds — `buildActionEngineFacade` in + * @objectstack/runtime 17.2.0, reproduced line for line: + * + * async find(object, query) { + * const where = query && Object.keys(query).length ? { where: query } : {}; + * const rows = await ql.find(object, { ...where, context }); + * ... + * + * It wraps whatever it is handed in a `where` of its own. Used by exactly one + * test, the tripwire at the bottom. + */ +function runtimeFacade(): Facade { + const base = handlerFacade(); + return { + ...base, + find: async (object, query) => { + const where = query && Object.keys(query).length ? { where: query } : {}; + return data.find(object, { ...where }); + }, + }; +} + +/** Dispatch `duly_catalog_apply` the way the platform dispatcher does. */ +async function apply( + engine: Facade, + params: { position_code: string; users: string[] }, +): Promise { + return (await data.executeAction(GLOBAL_ACTION_OBJECT, CATALOG_APPLY_ACTION, { + record: {}, + user: { id: 'admin_1' }, + session: { userId: 'admin_1' }, + engine, + params, + })) as CatalogApplyResult; +} + +const dutiesOf = async (owner: string): Promise => + data.find('duly_duty', { where: { owner } }); + +let seq = 0; +const insertItem = async (over: AnyRow): Promise => + data.insert('duly_catalog_item', { name: `Item ${++seq}`, ...over }); + +// ──────────────────────────────────────────────────────────────────────────── + +describe('duly_catalog_apply — the cadence it replicates (#65)', () => { + it('copies a recurring item\'s frequency onto every duty it creates', async () => { + // The positive: whatever the catalog item holds is what every person who + // takes the role gets. This is the mechanism that turns ONE bad catalog + // row into N bad duties, and the reason #65's rule is on this object. + const position = 'apply_positive'; + await insertItem({ position_code: position, form: 'recurring', frequency: 'quarterly' }); + + const result = await apply(handlerFacade(), { position_code: position, users: ['u_p1', 'u_p2'] }); + expect(result.created).toBe(2); + + for (const owner of ['u_p1', 'u_p2']) { + const [duty] = await dutiesOf(owner); + expect(duty?.form).toBe('recurring'); + expect(duty?.frequency).toBe('quarterly'); + expect(duty?.source).toBe('catalog'); + } + }); + + it('a blank recurring item is replicated as a SILENT monthly — not as N refusals', async () => { + // The claim #65's issue makes about the downstream half is that each + // person's duty would "individually trip `duly_duty`'s own + // `recurring_needs_frequency` on their first save". Measured on + // @objectstack/runtime 17.2.0, it does not: the handler's insert carries + // `frequency: null` explicitly, `applyFieldDefaults` reads an explicit + // null as ABSENT, and the duty is stamped `"monthly"` from its own CEL + // default before any rule sees it. `duly_duty`'s rule never fires. + // + // So the fan-out is not N loud refusals — it is N duties dispatching on a + // monthly cadence nobody chose, diverged from the catalog that is + // supposed to define them. That makes the catalog item the LAST place the + // blank can be caught, not merely the tidiest, which is the whole + // argument for the rule. + // + // The row below is REAL — inserted through the object, so `duly_duty`'s + // `catalog_item` lookup resolves (a synthetic id is refused outright by + // `assertReferencesResolve`, which is its own good news) — and only the + // value #65 now forbids is substituted in transit. That is exactly the + // state the unrefused UPDATE used to leave behind: same row, same id, + // frequency gone. + const real = await insertItem({ + position_code: 'apply_blank', + form: 'recurring', + frequency: 'monthly', + }); + const forbidden: AnyRow = { ...real, frequency: null }; + + const result = await apply(handlerFacade([forbidden]), { + position_code: 'apply_blank', + users: ['u_b1', 'u_b2'], + }); + + // Nothing refused anything. + expect(result.created).toBe(2); + expect(result.entries.every((e) => e.outcome === 'created')).toBe(true); + + for (const owner of ['u_b1', 'u_b2']) { + const [duty] = await dutiesOf(owner); + expect(duty?.form).toBe('recurring'); + // The silent divergence, pinned: 'monthly' was never in the catalog. + expect(duty?.frequency).toBe('monthly'); + } + }); + + it('a standing item applies to standing duties with no cadence at all', async () => { + // The control against over-firing THROUGH apply. A standing item's blank + // frequency is required (`standing_no_frequency`), it is copied onto the + // duty verbatim, and #65's recurring-only rule must stay silent on both + // objects — a rule that fired here would make the catalog's standing + // items un-appliable, which is a worse failure than the gap it closes. + const position = 'apply_standing'; + const item = await insertItem({ position_code: position, form: 'standing' }); + for (const field of CADENCE_FIELDS) expect(item[field] ?? null, field).toBeNull(); + + const result = await apply(handlerFacade(), { position_code: position, users: ['u_s1'] }); + expect(result.created).toBe(1); + + const [duty] = await dutiesOf('u_s1'); + expect(duty?.form).toBe('standing'); + for (const field of CADENCE_FIELDS) expect(duty?.[field] ?? null, field).toBeNull(); + }); +}); + +// ─────────────────────────────────────────────────────────────────────────── +// A tripwire on a filed defect — NOT an assertion that this is correct +// ─────────────────────────────────────────────────────────────────────────── +describe('the handler\'s query shape does not survive the runtime\'s own facade', () => { + /** + * Measured while covering the apply path for #65 and filed as #79 — a + * different defect from the missing validation rule, and not fixed here. + * + * `applyCatalogHandler` calls `engine.find('duly_catalog_item', { where: … + * })`. The runtime's `buildActionEngineFacade` wraps whatever it is given: + * `ql.find(object, { where: query })`. So through the real dispatcher the + * handler's own `where` becomes `{ where: { where: … } }`, no row has a + * field called `where`, and the read comes back EMPTY — with no error. The + * action then reports `{ created: 0 }` and a successful run. + * + * Pinned so the seam is visible rather than folklore. When #79 is fixed + * this goes red: delete this describe block — do not adjust it — and + * `handlerFacade` above becomes the only convention in the file. + */ + it('finds nothing, creates nothing, and reports success', async () => { + const position = 'apply_runtime_facade'; + await insertItem({ position_code: position, form: 'recurring', frequency: 'weekly' }); + + // Same item, same params, the only difference being which facade. + const viaHandlerConvention = await apply(handlerFacade(), { + position_code: position, + users: ['u_f1'], + }); + expect(viaHandlerConvention.catalog_items).toBe(1); + expect(viaHandlerConvention.created).toBe(1); + + const viaRuntime = await apply(runtimeFacade(), { position_code: position, users: ['u_f2'] }); + expect(viaRuntime.catalog_items).toBe(0); + expect(viaRuntime.created).toBe(0); + expect(await dutiesOf('u_f2')).toEqual([]); + }); +});