From 46dd6ce7cf72aa6028e104a4afa6b00eb22feec8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8C=85=E5=91=A8=E6=B6=9B?= Date: Sun, 19 Jul 2026 11:26:10 -0700 Subject: [PATCH] fix(plugin-audit): exclude computed fields from update diffs + treat undefined/null as equal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../plugin-audit/src/audit-writers.test.ts | 90 +++++++++++++++++++ .../plugins/plugin-audit/src/audit-writers.ts | 39 ++++++-- 2 files changed, 120 insertions(+), 9 deletions(-) diff --git a/packages/plugins/plugin-audit/src/audit-writers.test.ts b/packages/plugins/plugin-audit/src/audit-writers.test.ts index 11193bba72..c7d5d40550 100644 --- a/packages/plugins/plugin-audit/src/audit-writers.test.ts +++ b/packages/plugins/plugin-audit/src/audit-writers.test.ts @@ -440,6 +440,96 @@ describe('audit writers — enable.files server-side enforcement (#2727)', () => }); }); +describe('audit writers — update diff hygiene (objectui detail-history report)', () => { + // gantt_plan-shaped object: a formula helper alongside real fields. The + // `before` snapshot is read through the query path (formula computed), but + // `after` (ctx.result) is the raw write result (formula absent) — the diff + // must not record that asymmetry as a change. + const SCHEMA = { + sys_audit_log: SINGLE_TENANT.sys_audit_log, + sys_activity: SINGLE_TENANT.sys_activity, + gantt_plan: { + id: { type: 'text' }, + name: { type: 'text', label: 'Name' }, + plan_start: { type: 'datetime', label: 'Plan Start' }, + deps_rendered: { type: 'formula', label: 'Deps (rendered)' }, + }, + }; + + it('excludes computed (formula) fields from the update diff', async () => { + const { engine, fire, created } = makeEngine(SCHEMA); + installAuditWriters(engine as any, 'test.audit'); + + await fire('afterUpdate', { + object: 'gantt_plan', + input: { id: 'p-1', plan_start: '2026-08-04T12:00:00.000Z' }, + // before: query-path snapshot carries the computed formula value… + __previous: { id: 'p-1', name: 'Plan C', plan_start: '2026-07-26T00:00:00.000Z', deps_rendered: ['LnLJIsTwXbv1E2gF'] }, + // …after: raw write result does not. + result: { id: 'p-1', name: 'Plan C', plan_start: '2026-08-04T12:00:00.000Z' }, + session: { userId: 'user-1' }, + }); + + const audit = created.find((c) => c.object === 'sys_audit_log'); + expect(audit).toBeDefined(); + const oldValue = JSON.parse(audit!.row.old_value); + const newValue = JSON.parse(audit!.row.new_value); + expect(newValue).toEqual({ plan_start: '2026-08-04T12:00:00.000Z' }); + expect(oldValue).toEqual({ plan_start: '2026-07-26T00:00:00.000Z' }); + expect('deps_rendered' in oldValue).toBe(false); + expect('deps_rendered' in newValue).toBe(false); + }); + + it('writes NO audit row when only computed fields differ', async () => { + const { engine, fire, created } = makeEngine(SCHEMA); + installAuditWriters(engine as any, 'test.audit'); + + await fire('afterUpdate', { + object: 'gantt_plan', + input: { id: 'p-1' }, + __previous: { id: 'p-1', name: 'Plan C', deps_rendered: ['LnLJIsTwXbv1E2gF'] }, + result: { id: 'p-1', name: 'Plan C' }, + session: { userId: 'user-1' }, + }); + + expect(created.find((c) => c.object === 'sys_audit_log')).toBeUndefined(); + expect(created.find((c) => c.object === 'sys_activity')).toBeUndefined(); + }); + + it('treats an absent key (undefined) and an explicit null as equal — no noise row', async () => { + const { engine, fire, created } = makeEngine(SCHEMA); + installAuditWriters(engine as any, 'test.audit'); + + await fire('afterUpdate', { + object: 'gantt_plan', + input: { id: 'p-1', plan_start: null }, + // `plan_start` key absent before, explicit null after: not a change. + __previous: { id: 'p-1', name: 'Plan C' }, + result: { id: 'p-1', name: 'Plan C', plan_start: null }, + session: { userId: 'user-1' }, + }); + + expect(created.find((c) => c.object === 'sys_audit_log')).toBeUndefined(); + }); + + it('still records a real transition to null (value cleared)', async () => { + const { engine, fire, created } = makeEngine(SCHEMA); + installAuditWriters(engine as any, 'test.audit'); + + await fire('afterUpdate', { + object: 'gantt_plan', + input: { id: 'p-1', plan_start: null }, + __previous: { id: 'p-1', name: 'Plan C', plan_start: '2026-07-26T00:00:00.000Z' }, + result: { id: 'p-1', name: 'Plan C', plan_start: null }, + session: { userId: 'user-1' }, + }); + + const audit = created.find((c) => c.object === 'sys_audit_log'); + expect(JSON.parse(audit!.row.old_value)).toEqual({ plan_start: '2026-07-26T00:00:00.000Z' }); + expect(JSON.parse(audit!.row.new_value)).toEqual({ plan_start: null }); + }); +}); + // timeout: the FIRST localized case pays the one-off cost of dynamically // importing @objectstack/core + the shipped translation bundle (and, with the // #3071 src aliases, their vite transforms). On a shared 4-vCPU CI runner that diff --git a/packages/plugins/plugin-audit/src/audit-writers.ts b/packages/plugins/plugin-audit/src/audit-writers.ts index 994186f2d2..ca23ebdab1 100644 --- a/packages/plugins/plugin-audit/src/audit-writers.ts +++ b/packages/plugins/plugin-audit/src/audit-writers.ts @@ -152,23 +152,44 @@ function recordLabel(record: any, id: string): string { return id; } +/** + * Field types whose values the engine computes on READ (formula / summary / + * rollup / autonumber). The `before` snapshot is read back through the query + * path and therefore carries them, but `after` (`ctx.result`) is the raw + * write result and does not — so diffing them records a phantom + * "value → null" change on EVERY update (surfaced by the objectui record + * History tab). As derived values their changes are implied by their source + * fields anyway, so they are excluded from the audit diff. + */ +const COMPUTED_FIELD_TYPES = new Set(['formula', 'summary', 'rollup', 'autonumber', 'auto_number']); + /** * Compute a shallow JSON diff between two records. Returns only keys whose - * value changed (and ignores keys in `NOISE_FIELDS`). Both sides are - * serialisable via `JSON.stringify` — values that fail to serialise are - * coerced to `String(value)`. + * value changed (and ignores keys in `NOISE_FIELDS` plus computed field + * types per `fieldDefs`). Both sides are serialisable via `JSON.stringify` — + * values that fail to serialise are coerced to `String(value)`. */ -function diff(before: Record, after: Record): { old: Record; next: Record } { +function diff( + before: Record, + after: Record, + fieldDefs?: Record | null, +): { old: Record; next: Record } { const oldOut: Record = {}; const newOut: Record = {}; const keys = new Set([...Object.keys(before || {}), ...Object.keys(after || {})]); for (const k of keys) { if (NOISE_FIELDS.has(k)) continue; - const b = before?.[k]; - const a = after?.[k]; + const type = fieldDefs?.[k]?.type; + if (typeof type === 'string' && COMPUTED_FIELD_TYPES.has(type)) continue; + // `?? null` BEFORE comparing: a key absent on one side (undefined) must + // compare equal to an explicit null on the other. JSON.stringify(undefined) + // returns undefined (not a string), so the raw comparison saw + // undefined ≠ 'null' and wrote a noise row with old=new=null. + const b = before?.[k] ?? null; + const a = after?.[k] ?? null; if (safeStringify(b) !== safeStringify(a)) { - oldOut[k] = b ?? null; - newOut[k] = a ?? null; + oldOut[k] = b; + newOut[k] = a; } } return { old: oldOut, next: newOut }; @@ -455,7 +476,7 @@ export function installAuditWriters( if (action === 'create') { newValue = (after && typeof after === 'object') ? { ...after } : null; } else if (action === 'update') { - const d = diff(before || {}, after || {}); + const d = diff(before || {}, after || {}, getFieldDefs(ctx.object)); oldValue = d.old; newValue = d.next; // If nothing meaningfully changed, skip the audit row to avoid noise.