Skip to content

Commit b88d592

Browse files
committed
fix(objectql,spec,rest): #2922 import automation chain — per-row batch hooks, effective skipAutomations, run-automations default ON
- engine.insert now triggers beforeInsert/afterInsert once per row with single-record hook contexts, so flat-input proxies, declarative hook conditions, audit writers and record-change triggers see real records instead of arrays. - New ExecutionContext.skipAutomations (mirrored into HookContext.session) suppresses metadata-bound automation hooks and implies skipTriggers for flow dispatch; code-registered system hooks (audit, security, sharing) still run. Makes the import wizard checkbox and import undo effective. - REST import defaults runAutomations to true unless the request explicitly opts out, matching historical behavior. Closes #2922
1 parent 77d9618 commit b88d592

8 files changed

Lines changed: 222 additions & 45 deletions

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
'@objectstack/spec': patch
3+
'@objectstack/objectql': patch
4+
'@objectstack/rest': patch
5+
---
6+
7+
Fix the data-import automation chain (#2922). Batch `engine.insert` now fires
8+
`beforeInsert`/`afterInsert` once **per row** with single-record hook contexts,
9+
so flat-input proxies, declarative hook conditions, audit writers, and
10+
record-change triggers see real records instead of arrays. A new
11+
`ExecutionContext.skipAutomations` flag (mirrored into `HookContext.session`)
12+
lets callers suppress metadata-bound automation hooks and flow dispatch while
13+
code-registered system hooks (audit, security, sharing) still run — making the
14+
import wizard's "run automations & triggers" checkbox and import undo actually
15+
effective. The REST import default flips to running automations unless the
16+
request explicitly opts out (`runAutomations: false`), matching historical
17+
behavior.

packages/objectql/src/engine.test.ts

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,89 @@ describe('ObjectQL Engine', () => {
400400
});
401401
});
402402

403+
describe('batch insert triggers hooks per row (#2922)', () => {
404+
beforeEach(async () => {
405+
engine.registerDriver(mockDriver, true);
406+
await engine.init();
407+
vi.mocked(SchemaRegistry.getObject).mockReturnValue({ name: 'task', fields: { title: { type: 'text' } } } as any);
408+
});
409+
410+
it('fires beforeInsert/afterInsert once per row with the single-record context shape', async () => {
411+
const beforeRows: any[] = [];
412+
const afterResults: any[] = [];
413+
engine.registerHook('beforeInsert', async (ctx: any) => {
414+
beforeRows.push(ctx.input.data);
415+
// Same mutation contract as single insert: write one row's field.
416+
ctx.input.data.stamped = ctx.input.data.title.toUpperCase();
417+
}, { object: 'task' });
418+
engine.registerHook('afterInsert', async (ctx: any) => {
419+
afterResults.push(ctx.result);
420+
}, { object: 'task' });
421+
(mockDriver.create as any).mockImplementation(async (_o: string, row: any) => ({ id: `id-${row.title}`, ...row }));
422+
423+
const result = await engine.insert('task', [{ title: 'a' }, { title: 'b' }]);
424+
425+
expect(beforeRows).toHaveLength(2);
426+
expect(Array.isArray(beforeRows[0])).toBe(false);
427+
expect(beforeRows[0]).toMatchObject({ title: 'a' });
428+
expect(beforeRows[1]).toMatchObject({ title: 'b' });
429+
// The per-row mutation reached the driver for each row.
430+
expect((mockDriver.create as any).mock.calls[0][1]).toMatchObject({ title: 'a', stamped: 'A' });
431+
expect((mockDriver.create as any).mock.calls[1][1]).toMatchObject({ title: 'b', stamped: 'B' });
432+
// afterInsert sees one record per row, never the whole array.
433+
expect(afterResults).toHaveLength(2);
434+
expect(Array.isArray(afterResults[0])).toBe(false);
435+
expect(afterResults[0]).toMatchObject({ id: 'id-a' });
436+
expect(afterResults[1]).toMatchObject({ id: 'id-b' });
437+
expect(result).toHaveLength(2);
438+
});
439+
440+
it('pairs each afterInsert result with its own row when the driver bulk-creates', async () => {
441+
(mockDriver as any).bulkCreate = vi.fn(async (_o: string, rows: any[]) =>
442+
rows.map((r: any) => ({ id: `id-${r.title}`, ...r })));
443+
const afterResults: any[] = [];
444+
engine.registerHook('afterInsert', async (ctx: any) => { afterResults.push(ctx.result); }, { object: 'task' });
445+
446+
await engine.insert('task', [{ title: 'a' }, { title: 'b' }]);
447+
448+
expect((mockDriver as any).bulkCreate).toHaveBeenCalledTimes(1);
449+
expect(afterResults.map((r) => r.id)).toEqual(['id-a', 'id-b']);
450+
});
451+
});
452+
453+
describe('skipAutomations suppresses metadata-bound hooks (#2922)', () => {
454+
beforeEach(async () => {
455+
engine.registerDriver(mockDriver, true);
456+
await engine.init();
457+
vi.mocked(SchemaRegistry.getObject).mockReturnValue({ name: 'task', fields: {} } as any);
458+
});
459+
460+
it('skips hooks bound from metadata but still runs code-registered system hooks', async () => {
461+
const calls: string[] = [];
462+
// Metadata-bound automation hook: `meta` present (bindHooksToEngine shape).
463+
engine.registerHook('beforeInsert', async () => { calls.push('automation'); },
464+
{ object: 'task', meta: { name: 'auto_hook', events: ['beforeInsert'] } });
465+
// Code-registered system hook (audit/security shape): no `meta`.
466+
engine.registerHook('beforeInsert', async () => { calls.push('system'); }, { object: 'task' });
467+
468+
await engine.insert('task', { title: 'x' }, { context: { skipAutomations: true } as any });
469+
expect(calls).toEqual(['system']);
470+
471+
calls.length = 0;
472+
await engine.insert('task', { title: 'y' });
473+
expect(calls.sort()).toEqual(['automation', 'system']);
474+
});
475+
476+
it('implies skipTriggers on the hook session so flow dispatch is suppressed too', async () => {
477+
let session: any;
478+
engine.registerHook('afterInsert', async (ctx: any) => { session = ctx.session; }, { object: 'task' });
479+
480+
await engine.insert('task', { title: 'x' }, { context: { userId: 'u1', skipAutomations: true } as any });
481+
482+
expect(session).toMatchObject({ skipAutomations: true, skipTriggers: true });
483+
});
484+
});
485+
403486
describe('execution context via the trailing options arg (read methods)', () => {
404487
// Regression: reads took context inside the query while writes took it in
405488
// a trailing options arg — so `find(obj, q, { context })` silently dropped

packages/objectql/src/engine.ts

Lines changed: 73 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -590,7 +590,16 @@ export class ObjectQL implements IDataEngine {
590590
}
591591

592592
this.logger.debug('Triggering hooks', { event, count: entries.length });
593-
593+
594+
// `session.skipAutomations` (set from ExecutionContext.skipAutomations —
595+
// import with "run automations & triggers" unchecked, import undo)
596+
// suppresses hooks bound FROM METADATA (`bindHooksToEngine` stamps
597+
// `entry.meta`). Hooks registered in code by plugins — audit, capability
598+
// gates, sharing projection — have no `meta` and always run: the opt-out
599+
// must never bypass security or audit (#2922).
600+
const skipAutomations =
601+
(context.session as { skipAutomations?: boolean } | undefined)?.skipAutomations === true;
602+
594603
for (const entry of entries) {
595604
// Per-object matching
596605
if (entry.object) {
@@ -599,6 +608,10 @@ export class ObjectQL implements IDataEngine {
599608
continue; // Skip non-matching hooks
600609
}
601610
}
611+
if (skipAutomations && entry.meta) {
612+
this.logger.debug('Skipping metadata-bound hook (skipAutomations)', { event, hook: entry.hookName });
613+
continue;
614+
}
602615
await entry.handler(context);
603616
}
604617
}
@@ -692,8 +705,13 @@ export class ObjectQL implements IDataEngine {
692705
...((execCtx as any).isSystem ? { isSystem: true } : {}),
693706
// Propagate the automation-suppression flag so the record-change trigger
694707
// can skip flow dispatch for seed/bulk writes (ADR: seed loads end-state
695-
// data, not user events).
696-
...((execCtx as any).skipTriggers ? { skipTriggers: true } : {}),
708+
// data, not user events). `skipAutomations` implies `skipTriggers` —
709+
// suppressing metadata hooks while still dispatching flows would leave
710+
// the "run automations & triggers" opt-out half-working (#2922).
711+
...((execCtx as any).skipTriggers || (execCtx as any).skipAutomations ? { skipTriggers: true } : {}),
712+
// Propagate the full automation opt-out so `triggerHooks` can skip
713+
// metadata-bound hooks (import with "run automations" unchecked, undo).
714+
...((execCtx as any).skipAutomations ? { skipAutomations: true } : {}),
697715
} as HookContext['session'];
698716
}
699717

@@ -2175,76 +2193,88 @@ export class ObjectQL implements IDataEngine {
21752193
// any defaulted field. `applyFieldDefaults` returns a fresh copy and only
21762194
// fills fields left `undefined`, so client-supplied values are untouched.
21772195
const nowSnap = new Date();
2178-
const defaultedData = Array.isArray(opCtx.data)
2196+
const isBatch = Array.isArray(opCtx.data);
2197+
const defaultedData = isBatch
21792198
? (opCtx.data as any[]).map((row) =>
21802199
this.applyFieldDefaults(object, row as Record<string, unknown>, opCtx.context, nowSnap),
21812200
)
21822201
: this.applyFieldDefaults(object, opCtx.data as Record<string, unknown>, opCtx.context, nowSnap);
21832202

2184-
const hookContext: HookContext = {
2203+
// Batch inserts trigger beforeInsert/afterInsert PER ROW, each with the
2204+
// exact single-record context shape (`input.data` = one row, `result` =
2205+
// its returned record). A single array-shaped context broke every
2206+
// consumer built for the single shape — the flat-input proxy read
2207+
// `undefined`s, declarative `condition`s evaluated against an array,
2208+
// audit rows and flow-trigger contexts came out mangled (#2922).
2209+
const rowHookContexts: HookContext[] = (isBatch ? (defaultedData as any[]) : [defaultedData]).map(
2210+
(row) => ({
21852211
object,
21862212
event: 'beforeInsert',
2187-
input: { data: defaultedData, options: opCtx.options },
2213+
input: { data: row, options: opCtx.options },
21882214
session: this.buildSession(opCtx.context),
21892215
api: this.buildHookApi(opCtx.context),
21902216
transaction: opCtx.context?.transaction,
2191-
ql: this
2192-
};
2193-
await this.triggerHooks('beforeInsert', hookContext);
2217+
ql: this,
2218+
}),
2219+
);
2220+
for (const rowCtx of rowHookContexts) {
2221+
await this.triggerHooks('beforeInsert', rowCtx);
2222+
}
21942223
// Thread the open transaction (if any) into the driver-facing
21952224
// options so that knex's `.transacting(trx)` is honoured. Without
21962225
// this, calls inside a `engine.transaction(...)` block would deadlock
21972226
// on SQLite's single-connection pool. Also propagates tenantId so
21982227
// the driver can enforce per-tenant isolation.
2199-
hookContext.input.options = this.buildDriverOptions(opCtx.context, hookContext.input.options as any);
2228+
// Base the merge on the first row context's options: hooks share the
2229+
// same underlying options object (in-place mutations are visible), and
2230+
// for single inserts this is exactly the pre-#2922 behaviour.
2231+
const driverOptions = this.buildDriverOptions(opCtx.context, rowHookContexts[0]?.input.options as any);
2232+
for (const rowCtx of rowHookContexts) {
2233+
rowCtx.input.options = driverOptions;
2234+
}
22002235

22012236
try {
22022237
let result;
22032238
const schemaForValidation = this._registry.getObject(object);
22042239
// When the driver generates autonumbers natively (persistent SQL
22052240
// sequence), the engine defers to it — see #1603.
22062241
const driverOwnsAutonumber = (driver as any)?.supports?.autonumber === true;
2207-
if (Array.isArray(hookContext.input.data)) {
2208-
// Defaults are already resolved above (pre-hook, #2703); the hook may
2209-
// have overridden or added fields — take its data as-is.
2210-
const rows = hookContext.input.data as Array<Record<string, unknown>>;
2211-
for (const r of rows) {
2212-
await this.applyAutonumbers(object, r as Record<string, unknown>, opCtx.context, driverOwnsAutonumber);
2213-
}
2214-
for (const r of rows) {
2215-
await this.encryptSecretFields(object, r, opCtx.context, hookContext.input.options);
2216-
}
2217-
for (const r of rows) {
2218-
normalizeMultiValueFields(schemaForValidation, r);
2219-
validateRecord(schemaForValidation, r, 'insert');
2220-
evaluateValidationRules(schemaForValidation as any, r, 'insert', { logger: this.logger, currentUser: this.buildEvalUser(opCtx.context) });
2221-
}
2242+
// Defaults are already resolved above (pre-hook, #2703); a hook may
2243+
// have overridden fields or replaced `input.data` — take its data as-is.
2244+
const rows = rowHookContexts.map((rowCtx) => rowCtx.input.data as Record<string, unknown>);
2245+
for (const r of rows) {
2246+
await this.applyAutonumbers(object, r, opCtx.context, driverOwnsAutonumber);
2247+
}
2248+
for (const r of rows) {
2249+
await this.encryptSecretFields(object, r, opCtx.context, driverOptions);
2250+
}
2251+
for (const r of rows) {
2252+
normalizeMultiValueFields(schemaForValidation, r);
2253+
validateRecord(schemaForValidation, r, 'insert');
2254+
evaluateValidationRules(schemaForValidation as any, r, 'insert', { logger: this.logger, currentUser: this.buildEvalUser(opCtx.context) });
2255+
}
2256+
if (isBatch) {
22222257
if (driver.bulkCreate) {
2223-
result = await driver.bulkCreate(object, rows, hookContext.input.options as any);
2258+
result = await driver.bulkCreate(object, rows, driverOptions);
22242259
} else {
22252260
// Fallback loop
2226-
result = await Promise.all(rows.map((item) => driver.create(object, item, hookContext.input.options as any)));
2261+
result = await Promise.all(rows.map((item) => driver.create(object, item, driverOptions)));
22272262
}
22282263
} else {
2229-
// Defaults already resolved pre-hook (#2703); use the hook's data.
2230-
const row = hookContext.input.data as Record<string, unknown>;
2231-
await this.applyAutonumbers(object, row, opCtx.context, driverOwnsAutonumber);
2232-
await this.encryptSecretFields(object, row, opCtx.context, hookContext.input.options);
2233-
normalizeMultiValueFields(schemaForValidation, row);
2234-
validateRecord(schemaForValidation, row, 'insert');
2235-
evaluateValidationRules(schemaForValidation as any, row, 'insert', { logger: this.logger, currentUser: this.buildEvalUser(opCtx.context) });
2236-
result = await driver.create(object, row, hookContext.input.options as any);
2264+
result = await driver.create(object, rows[0], driverOptions);
22372265
}
22382266

2239-
hookContext.event = 'afterInsert';
22402267
// Coerce `boolean` fields (SQLite/libsql return 0/1) to real booleans on
22412268
// the after-hook view so flow trigger conditions (`record.is_escalated
22422269
// != true`) and `{record.<bool>}` interpolation see JS booleans, not
22432270
// ints. A shallow copy — the value returned to the caller is untouched.
2244-
hookContext.result = Array.isArray(result)
2245-
? result.map((r) => coerceBooleanFields(schemaForValidation as any, r as any))
2246-
: coerceBooleanFields(schemaForValidation as any, result as any);
2247-
await this.triggerHooks('afterInsert', hookContext);
2271+
const resultRows: any[] = isBatch ? (Array.isArray(result) ? result : [result]) : [result];
2272+
for (let i = 0; i < rowHookContexts.length; i++) {
2273+
const rowCtx = rowHookContexts[i];
2274+
rowCtx.event = 'afterInsert';
2275+
rowCtx.result = coerceBooleanFields(schemaForValidation as any, resultRows[i] as any);
2276+
await this.triggerHooks('afterInsert', rowCtx);
2277+
}
22482278

22492279
// Roll-up: recompute parent summary fields that aggregate this object.
22502280
await this.recomputeSummaries(object, result, null, opCtx.context);
@@ -2285,7 +2315,9 @@ export class ObjectQL implements IDataEngine {
22852315
}
22862316
}
22872317

2288-
return hookContext.result;
2318+
// Return the (possibly hook-mutated) after-view: the array of per-row
2319+
// results for batch, the single record otherwise.
2320+
return isBatch ? rowHookContexts.map((rowCtx) => rowCtx.result) : rowHookContexts[0].result;
22892321
} catch (e) {
22902322
this.logger.error('Insert operation failed', e as Error, { object });
22912323
throw e;

packages/rest/src/import-integration.test.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -357,15 +357,19 @@ describe('import route — required-field dry-run fidelity', () => {
357357
{ id: 'm1', member_name: 'Alice' }, // status missing → must fail
358358
{ id: 'm2', member_name: 'Bob', status: 'active' }, // complete → ok
359359
];
360+
// The pre-check only runs with automations OFF (a beforeInsert hook may
361+
// populate a required field, so with automations on we defer to the
362+
// engine's own validation). runAutomations defaults to true since #2922,
363+
// so the opt-out is explicit here.
360364
// Dry run: no longer reports success for the row the insert will reject.
361-
const dry = await imp({ format: 'json', dryRun: true, rows });
365+
const dry = await imp({ format: 'json', dryRun: true, runAutomations: false, rows });
362366
expect(dry._json).toMatchObject({ dryRun: true, total: 2, ok: 1, errors: 1 });
363367
expect(dry._json.results.find((r: any) => !r.ok)).toMatchObject({ row: 1, field: 'status', code: 'required' });
364368
expect(await engine.findOne('member', { where: { id: 'm2' } })).toBeNull(); // dry run never writes
365369

366370
// Real insert: SAME verdict (parity), and a readable `status is required`
367371
// instead of a raw `NOT NULL constraint failed: member.status`.
368-
const real = await imp({ format: 'json', rows });
372+
const real = await imp({ format: 'json', runAutomations: false, rows });
369373
expect(real._json).toMatchObject({ total: 2, ok: 1, errors: 1, created: 1 });
370374
expect(real._json.results.find((r: any) => !r.ok)).toMatchObject({ field: 'status', code: 'required', error: 'status is required' });
371375
expect((await engine.findOne('member', { where: { id: 'm2' } }))?.status).toBe('active');
@@ -382,7 +386,7 @@ describe('import route — required-field dry-run fidelity', () => {
382386
});
383387

384388
it('flags a required text field too (not just selects); a blank cell counts as missing', async () => {
385-
const res = await imp({ format: 'json', dryRun: true, rows: [
389+
const res = await imp({ format: 'json', dryRun: true, runAutomations: false, rows: [
386390
{ id: 'm4', status: 'active' }, // member_name missing
387391
{ id: 'm5', member_name: ' ', status: 'active' }, // member_name blank
388392
] });

packages/rest/src/import-prepare.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,3 +99,27 @@ describe('prepareImportRequest — locale-translated option synonyms', () => {
9999
expect(matchOption('Backlog', prep.prepared.metaMap.get('status')!.options!)).toBe('backlog');
100100
});
101101
});
102+
103+
describe('prepareImportRequest — runAutomations default (#2922)', () => {
104+
const prepWith = async (body: Record<string, unknown>) => {
105+
const prep = await prepareImportRequest(
106+
{ format: 'json', rows: [{ title: 'a' }], ...body },
107+
{ p, objectName: 'task', maxRows: 10 },
108+
);
109+
expect(prep.ok).toBe(true);
110+
return prep.ok ? prep.prepared.runAutomations : undefined;
111+
};
112+
113+
it('defaults to true when the flag is omitted (automations always ran historically)', async () => {
114+
expect(await prepWith({})).toBe(true);
115+
});
116+
117+
it('honours an explicit opt-out', async () => {
118+
expect(await prepWith({ runAutomations: false })).toBe(false);
119+
});
120+
121+
it('treats any non-false value as true', async () => {
122+
expect(await prepWith({ runAutomations: true })).toBe(true);
123+
expect(await prepWith({ runAutomations: 'no' })).toBe(true);
124+
});
125+
});

packages/rest/src/import-prepare.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,10 @@ export async function prepareImportRequest(
252252
let matchFields: string[] = Array.isArray(body?.matchFields)
253253
? body.matchFields.filter((f: any) => typeof f === 'string' && f.length > 0)
254254
: [];
255-
const runAutomations = body?.runAutomations === true;
255+
// Default ON: automations always ran historically (the engine ignored the
256+
// flag until #2922), so opt-out must be explicit — matches platform
257+
// convention (Salesforce runs triggers on import by default).
258+
const runAutomations = body?.runAutomations !== false;
256259
const trimWhitespace = body?.trimWhitespace !== false;
257260
const nullValues: string[] | undefined = Array.isArray(body?.nullValues)
258261
? body.nullValues.filter((v: any) => typeof v === 'string')

0 commit comments

Comments
 (0)