Skip to content

Commit 5c31684

Browse files
baozhoutaoclaude
andauthored
fix(plugin-audit): exclude computed fields from update diffs + treat undefined/null as equal (#3293)
Two audit-writer diff bugs surfaced by the objectui record History tab (gantt QA report showed "紧前依赖(渲染用): [id] → —" on every drag): 1. The beforeUpdate snapshot is read back through the query path, which computes formula/summary/rollup/autonumber fields; ctx.result is the raw write result, which does not. diff() therefore recorded a phantom "value → null" change for every computed field on every update. Computed fields are now excluded from the diff via the engine schema (their changes are implied by their source fields); an update touching only computed fields no longer writes an audit/activity row at all. 2. safeStringify(undefined) returns undefined (JSON.stringify contract), not a string, so a key absent on one side compared unequal to an explicit null on the other and wrote a noise row with old=new=null. Values are normalized with `?? null` before comparison; a real value → null transition is still recorded (covered by a guard test). Verified against a live showcase stack: updating showcase_project.health now writes old_value {"health":"green"} / new_value {"health":"yellow"} with no task_count/total_estimate phantoms, and the three new regression tests fail against the pre-fix writer. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 64e0bbd commit 5c31684

2 files changed

Lines changed: 120 additions & 9 deletions

File tree

packages/plugins/plugin-audit/src/audit-writers.test.ts

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,96 @@ describe('audit writers — enable.files server-side enforcement (#2727)', () =>
440440
});
441441
});
442442

443+
describe('audit writers — update diff hygiene (objectui detail-history report)', () => {
444+
// gantt_plan-shaped object: a formula helper alongside real fields. The
445+
// `before` snapshot is read through the query path (formula computed), but
446+
// `after` (ctx.result) is the raw write result (formula absent) — the diff
447+
// must not record that asymmetry as a change.
448+
const SCHEMA = {
449+
sys_audit_log: SINGLE_TENANT.sys_audit_log,
450+
sys_activity: SINGLE_TENANT.sys_activity,
451+
gantt_plan: {
452+
id: { type: 'text' },
453+
name: { type: 'text', label: 'Name' },
454+
plan_start: { type: 'datetime', label: 'Plan Start' },
455+
deps_rendered: { type: 'formula', label: 'Deps (rendered)' },
456+
},
457+
};
458+
459+
it('excludes computed (formula) fields from the update diff', async () => {
460+
const { engine, fire, created } = makeEngine(SCHEMA);
461+
installAuditWriters(engine as any, 'test.audit');
462+
463+
await fire('afterUpdate', {
464+
object: 'gantt_plan',
465+
input: { id: 'p-1', plan_start: '2026-08-04T12:00:00.000Z' },
466+
// before: query-path snapshot carries the computed formula value…
467+
__previous: { id: 'p-1', name: 'Plan C', plan_start: '2026-07-26T00:00:00.000Z', deps_rendered: ['LnLJIsTwXbv1E2gF'] },
468+
// …after: raw write result does not.
469+
result: { id: 'p-1', name: 'Plan C', plan_start: '2026-08-04T12:00:00.000Z' },
470+
session: { userId: 'user-1' },
471+
});
472+
473+
const audit = created.find((c) => c.object === 'sys_audit_log');
474+
expect(audit).toBeDefined();
475+
const oldValue = JSON.parse(audit!.row.old_value);
476+
const newValue = JSON.parse(audit!.row.new_value);
477+
expect(newValue).toEqual({ plan_start: '2026-08-04T12:00:00.000Z' });
478+
expect(oldValue).toEqual({ plan_start: '2026-07-26T00:00:00.000Z' });
479+
expect('deps_rendered' in oldValue).toBe(false);
480+
expect('deps_rendered' in newValue).toBe(false);
481+
});
482+
483+
it('writes NO audit row when only computed fields differ', async () => {
484+
const { engine, fire, created } = makeEngine(SCHEMA);
485+
installAuditWriters(engine as any, 'test.audit');
486+
487+
await fire('afterUpdate', {
488+
object: 'gantt_plan',
489+
input: { id: 'p-1' },
490+
__previous: { id: 'p-1', name: 'Plan C', deps_rendered: ['LnLJIsTwXbv1E2gF'] },
491+
result: { id: 'p-1', name: 'Plan C' },
492+
session: { userId: 'user-1' },
493+
});
494+
495+
expect(created.find((c) => c.object === 'sys_audit_log')).toBeUndefined();
496+
expect(created.find((c) => c.object === 'sys_activity')).toBeUndefined();
497+
});
498+
499+
it('treats an absent key (undefined) and an explicit null as equal — no noise row', async () => {
500+
const { engine, fire, created } = makeEngine(SCHEMA);
501+
installAuditWriters(engine as any, 'test.audit');
502+
503+
await fire('afterUpdate', {
504+
object: 'gantt_plan',
505+
input: { id: 'p-1', plan_start: null },
506+
// `plan_start` key absent before, explicit null after: not a change.
507+
__previous: { id: 'p-1', name: 'Plan C' },
508+
result: { id: 'p-1', name: 'Plan C', plan_start: null },
509+
session: { userId: 'user-1' },
510+
});
511+
512+
expect(created.find((c) => c.object === 'sys_audit_log')).toBeUndefined();
513+
});
514+
515+
it('still records a real transition to null (value cleared)', async () => {
516+
const { engine, fire, created } = makeEngine(SCHEMA);
517+
installAuditWriters(engine as any, 'test.audit');
518+
519+
await fire('afterUpdate', {
520+
object: 'gantt_plan',
521+
input: { id: 'p-1', plan_start: null },
522+
__previous: { id: 'p-1', name: 'Plan C', plan_start: '2026-07-26T00:00:00.000Z' },
523+
result: { id: 'p-1', name: 'Plan C', plan_start: null },
524+
session: { userId: 'user-1' },
525+
});
526+
527+
const audit = created.find((c) => c.object === 'sys_audit_log');
528+
expect(JSON.parse(audit!.row.old_value)).toEqual({ plan_start: '2026-07-26T00:00:00.000Z' });
529+
expect(JSON.parse(audit!.row.new_value)).toEqual({ plan_start: null });
530+
});
531+
});
532+
443533
// timeout: the FIRST localized case pays the one-off cost of dynamically
444534
// importing @objectstack/core + the shipped translation bundle (and, with the
445535
// #3071 src aliases, their vite transforms). On a shared 4-vCPU CI runner that

packages/plugins/plugin-audit/src/audit-writers.ts

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -152,23 +152,44 @@ function recordLabel(record: any, id: string): string {
152152
return id;
153153
}
154154

155+
/**
156+
* Field types whose values the engine computes on READ (formula / summary /
157+
* rollup / autonumber). The `before` snapshot is read back through the query
158+
* path and therefore carries them, but `after` (`ctx.result`) is the raw
159+
* write result and does not — so diffing them records a phantom
160+
* "value → null" change on EVERY update (surfaced by the objectui record
161+
* History tab). As derived values their changes are implied by their source
162+
* fields anyway, so they are excluded from the audit diff.
163+
*/
164+
const COMPUTED_FIELD_TYPES = new Set<string>(['formula', 'summary', 'rollup', 'autonumber', 'auto_number']);
165+
155166
/**
156167
* Compute a shallow JSON diff between two records. Returns only keys whose
157-
* value changed (and ignores keys in `NOISE_FIELDS`). Both sides are
158-
* serialisable via `JSON.stringify` — values that fail to serialise are
159-
* coerced to `String(value)`.
168+
* value changed (and ignores keys in `NOISE_FIELDS` plus computed field
169+
* types per `fieldDefs`). Both sides are serialisable via `JSON.stringify` —
170+
* values that fail to serialise are coerced to `String(value)`.
160171
*/
161-
function diff(before: Record<string, any>, after: Record<string, any>): { old: Record<string, any>; next: Record<string, any> } {
172+
function diff(
173+
before: Record<string, any>,
174+
after: Record<string, any>,
175+
fieldDefs?: Record<string, any> | null,
176+
): { old: Record<string, any>; next: Record<string, any> } {
162177
const oldOut: Record<string, any> = {};
163178
const newOut: Record<string, any> = {};
164179
const keys = new Set<string>([...Object.keys(before || {}), ...Object.keys(after || {})]);
165180
for (const k of keys) {
166181
if (NOISE_FIELDS.has(k)) continue;
167-
const b = before?.[k];
168-
const a = after?.[k];
182+
const type = fieldDefs?.[k]?.type;
183+
if (typeof type === 'string' && COMPUTED_FIELD_TYPES.has(type)) continue;
184+
// `?? null` BEFORE comparing: a key absent on one side (undefined) must
185+
// compare equal to an explicit null on the other. JSON.stringify(undefined)
186+
// returns undefined (not a string), so the raw comparison saw
187+
// undefined ≠ 'null' and wrote a noise row with old=new=null.
188+
const b = before?.[k] ?? null;
189+
const a = after?.[k] ?? null;
169190
if (safeStringify(b) !== safeStringify(a)) {
170-
oldOut[k] = b ?? null;
171-
newOut[k] = a ?? null;
191+
oldOut[k] = b;
192+
newOut[k] = a;
172193
}
173194
}
174195
return { old: oldOut, next: newOut };
@@ -455,7 +476,7 @@ export function installAuditWriters(
455476
if (action === 'create') {
456477
newValue = (after && typeof after === 'object') ? { ...after } : null;
457478
} else if (action === 'update') {
458-
const d = diff(before || {}, after || {});
479+
const d = diff(before || {}, after || {}, getFieldDefs(ctx.object));
459480
oldValue = d.old;
460481
newValue = d.next;
461482
// If nothing meaningfully changed, skip the audit row to avoid noise.

0 commit comments

Comments
 (0)