Skip to content

Commit 0490a45

Browse files
Claudeclaude
andcommitted
fix(objectql,rest,spec): DELETE_RESTRICTED stops showing developer copy to end users (#7307)
The 409 refusal is correct and its transport is fine; the message was not. REST ships `error.message` verbatim as `body.error` and Console renders it as-is in a toast, so a business user in a zh-CN app got an English sentence naming two tables and a column, ending in `set deleteBehavior:'cascade' on …`. The error now carries two messages because it has two audiences: - `message` — the user's half, rendered in `ExecutionContext.locale` from a new operation-message catalog against resolved LABELS (bundle → declared label → API name). No `deleteBehavior` in any locale. - `developerMessage` — the developer's half, the previous sentence byte for byte. Shipped by `mapDataError` as a sibling field and logged by the engine's delete path, so the guidance is moved, not lost. `code` / `status` / `object` / `dependentObject` / `dependentCount` unchanged; one `DELETE_RESTRICTED` (ADR-0112), two sentences. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017CZbGQYbdUMyzTMdqcQxp9
1 parent 7cdbcbb commit 0490a45

10 files changed

Lines changed: 763 additions & 7 deletions
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/objectql": minor
4+
"@objectstack/rest": minor
5+
---
6+
7+
fix(objectql,rest,spec): the `DELETE_RESTRICTED` 409 stops handing a business user a developer instruction
8+
9+
Deleting a record that other records reference is correctly refused with
10+
`409 DELETE_RESTRICTED`. The transport was never the problem — `status` is set
11+
and the structured fields survive the mapper. What reached the end user was:
12+
`error.message` is shipped verbatim as `body.error` by `mapDataError`, and
13+
Console renders that as-is in a toast. So an operator deleting a 部门 in a fully
14+
Chinese app read
15+
16+
```
17+
Cannot delete sys_business_unit (): 1 dependent os_tianshun_ehr_sporadic_application
18+
record(s) reference it via apply_dept (apply_dept is required, so it cannot be
19+
cleared). Delete or reassign them first, or set deleteBehavior:'cascade' on
20+
os_tianshun_ehr_sporadic_application.apply_dept.
21+
```
22+
23+
— an English sentence in a zh-CN UI, naming two tables and a column they have
24+
never seen (they know them as 「零星申请」 and 「申报部门」), ending in a
25+
metadata-authoring instruction a business user cannot act on and will open a
26+
support ticket about.
27+
28+
**The error now carries two messages, because it has two audiences.**
29+
30+
- `message` is the **user's** half: rendered in the caller's locale
31+
(`ExecutionContext.locale`) from a new built-in catalog, against resolved
32+
**labels** for the object, the dependent object and the referencing field —
33+
translation bundle → declared `label` → API name, so the API name is where the
34+
ladder ends rather than where it starts. The actionable half of the old advice
35+
("delete or reassign them first") stays; `deleteBehavior` does not appear in
36+
any locale.
37+
- `developerMessage` is the **developer's** half, and is the previous sentence
38+
byte for byte: English, API names, and the `deleteBehavior:'cascade'` remedy.
39+
The guidance is correct and useful — it is moved to a channel that reaches
40+
developers, not deleted. `@objectstack/rest` ships it as a sibling field of the
41+
409 body (it discloses nothing the envelope did not already carry: `object` and
42+
`dependentObject` are API names on the same body), and the engine's delete
43+
error log now carries it too, so a zh-CN deployment's server log does not lose
44+
its operator detail to the localized sentence.
45+
46+
`code`, `status`, `object`, `dependentObject` and `dependentCount` are
47+
unchanged, and the wire code does **not** split — one `DELETE_RESTRICTED`
48+
(ADR-0112), two sentences, exactly as the field catalog splits a message key
49+
without splitting `FieldErrorCode`.
50+
51+
**New in `@objectstack/spec/system`** (`operation-message.ts`): the operation
52+
message catalog — `renderOperationMessage`, `BUILTIN_OPERATION_MESSAGES`
53+
(`en` / `zh-CN` / `ja-JP` / `es-ES`), `operationMessageTranslationKey`, plus
54+
`objectLabelKey` in `i18n-resolver`. A deployment overrides any sentence with a
55+
`translation` item under `errors.<messageKey>`. It is a **separate** catalog from
56+
`validation-message.ts` deliberately: that one is addressed `validation.field.*`
57+
because every entry names a field and the constraint it broke, and a
58+
`DELETE_RESTRICTED` names neither — the offending field is on a different object
59+
from the one the caller acted on, and there is no `fields[]` entry to hang it
60+
off. Filing it there would give deployments an override key that lies about what
61+
it overrides.
62+
63+
`minor`, not `major`: nothing breaks. The structured fields clients match on are
64+
untouched, no test or doc ever pinned the message text, and both new fields are
65+
additive. `check-changeset-no-major.mjs` is the second reason — every publishable
66+
package is in the Changesets `fixed` group, so one `major` promotes all ~70
67+
packages, and the launch-window convention ships even genuinely breaking changes
68+
as `minor`.
69+
70+
This is #3957's fix reached from the operation side: same defect (platform copy
71+
composed in English with API names concatenated in), same machinery, one layer
72+
up.

packages/objectql/src/engine-cascade-delete.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,15 @@ describe('cascadeDeleteRelations — required FK escalates set_null → restrict
109109
await expect(engine.delete('acct', { where: { id: a.id } } as any))
110110
.rejects.toMatchObject({ code: 'DELETE_RESTRICTED', status: 409, dependentObject: 'opp', dependentCount: 1 });
111111

112+
// [#7307] The refusal's copy is now SPLIT in two. The structured fields
113+
// above are unchanged — this pins which half says what, so a later edit
114+
// cannot quietly put the API names back in front of an end user.
115+
const err = await engine.delete('acct', { where: { id: a.id } } as any).catch((e) => e);
116+
expect(err.message).toContain('Opportunity'); // the label, …
117+
expect(err.message).not.toContain('opp'); // … not the API name,
118+
expect(err.message).not.toMatch(/deleteBehavior/); // … and no authoring hint.
119+
expect(err.developerMessage).toContain("set deleteBehavior:'cascade' on opp.account");
120+
112121
// Nothing was deleted or mutated.
113122
expect(await engine.findOne('acct', { where: { id: a.id } })).toBeTruthy();
114123
expect((await engine.find('opp', {})).length).toBe(1);
Lines changed: 253 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,253 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #7307 — the CALL SITE of the `DELETE_RESTRICTED` copy, with a REAL
5+
* {@link ObjectQL} engine + stub driver.
6+
*
7+
* The refusal itself was never in doubt: `cascadeDeleteRelations` correctly
8+
* declines the delete with `409 DELETE_RESTRICTED` and the transport ships it
9+
* intact. What reached the end user was the problem. REST puts `error.message`
10+
* verbatim into the flat 409 envelope (`mapDataError`) and Console renders that
11+
* as-is in a toast, so an operator deleting a 部门 in a fully Chinese app read
12+
* an English sentence naming two tables and a column — ending in
13+
* `set deleteBehavior:'cascade' on …`, a metadata-authoring instruction they
14+
* cannot act on.
15+
*
16+
* These tests assert the SPLIT: `message` is the user's half (their locale,
17+
* labels, no developer vocabulary) and `developerMessage` is the developer's
18+
* half (English, API names, the remedy) — and the structured fields the wire
19+
* contract is built on are byte-identical to before.
20+
*
21+
* The catalog half is pinned in
22+
* `packages/spec/src/system/operation-message.test.ts`.
23+
*/
24+
25+
import { describe, it, expect, beforeEach } from 'vitest';
26+
import { ObjectQL } from './engine.js';
27+
28+
/** The reporter's shape: a business unit referenced by a REQUIRED lookup. */
29+
const businessUnit = {
30+
name: 'sys_business_unit',
31+
label: 'Business Unit',
32+
fields: {
33+
id: { name: 'id', type: 'text' as const, primaryKey: true },
34+
name: { name: 'name', type: 'text' as const },
35+
},
36+
};
37+
const sporadicApplication = {
38+
name: 'os_ehr_sporadic_application',
39+
label: 'Sporadic Application',
40+
fields: {
41+
id: { name: 'id', type: 'text' as const, primaryKey: true },
42+
title: { name: 'title', type: 'text' as const },
43+
apply_dept: {
44+
name: 'apply_dept', type: 'lookup' as const, reference: 'sys_business_unit',
45+
label: 'Applying Department', required: true,
46+
},
47+
},
48+
};
49+
/** An EXPLICIT restrict on a NULLABLE FK — the other sentence variant. */
50+
const archiveNote = {
51+
name: 'os_ehr_archive_note',
52+
label: 'Archive Note',
53+
fields: {
54+
id: { name: 'id', type: 'text' as const, primaryKey: true },
55+
body: { name: 'body', type: 'text' as const },
56+
dept: {
57+
name: 'dept', type: 'lookup' as const, reference: 'sys_business_unit',
58+
label: 'Department', deleteBehavior: 'restrict',
59+
},
60+
},
61+
};
62+
63+
/** Every API name that must never appear in a message a business user reads. */
64+
const API_NAMES = ['sys_business_unit', 'os_ehr_sporadic_application', 'apply_dept'];
65+
66+
function makeStubDriver() {
67+
const stores = new Map<string, Map<string, Record<string, unknown>>>();
68+
const storeFor = (o: string) => { let s = stores.get(o); if (!s) { s = new Map(); stores.set(o, s); } return s; };
69+
let nextId = 0;
70+
const matches = (row: Record<string, unknown>, where: any): boolean => {
71+
if (!where || typeof where !== 'object') return true;
72+
for (const [k, v] of Object.entries(where)) {
73+
if (k.startsWith('$')) continue;
74+
const exp = (v && typeof v === 'object' && '$eq' in (v as any)) ? (v as any).$eq : v;
75+
if ((row[k] ?? null) !== (exp ?? null)) return false;
76+
}
77+
return true;
78+
};
79+
const driver: any = {
80+
name: 'memory', version: '0.0.0', supports: {},
81+
async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; },
82+
async find(o: string, ast: any) { return Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)); },
83+
async findOne(o: string, ast: any) { for (const r of storeFor(o).values()) if (matches(r, ast?.where)) return r; return null; },
84+
async create(o: string, data: Record<string, unknown>) {
85+
nextId += 1; const id = (data.id as string) ?? `r_${nextId}`; const row = { ...data, id }; storeFor(o).set(id, row); return row;
86+
},
87+
async update(o: string, id: string, data: Record<string, unknown>) {
88+
const s = storeFor(o); const cur = s.get(id); if (!cur) throw new Error(`nf ${o}/${id}`);
89+
const up = { ...cur, ...data, id }; s.set(id, up); return up;
90+
},
91+
async upsert(o: string, data: Record<string, unknown>) { const id = data.id as string | undefined; return id && storeFor(o).has(id) ? this.update(o, id, data) : this.create(o, data); },
92+
async delete(o: string, id: string) { return storeFor(o).delete(id); },
93+
async count(o: string, ast: any) { return (await this.find(o, ast)).length; },
94+
async bulkCreate(o: string, rows: Record<string, unknown>[]) { return Promise.all(rows.map((r) => this.create(o, r))); },
95+
async bulkUpdate() { return []; }, async bulkDelete() {},
96+
async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, async commit() {}, async rollback() {},
97+
};
98+
return { driver };
99+
}
100+
101+
/** The zh-CN bundle the reporter's deployment ships, as an `II18nService`. */
102+
const ZH_BUNDLE: Record<string, string> = {
103+
'objects.sys_business_unit.label': '部门',
104+
'objects.os_ehr_sporadic_application.label': '零星申请',
105+
'objects.os_ehr_sporadic_application.fields.apply_dept.label': '申报部门',
106+
};
107+
// Locale-aware, like a real `II18nService`: a bundle it does not carry echoes
108+
// the key back, which is the contract every resolver here detects a miss by.
109+
const zhI18n = { t: (key: string, locale: string) => (locale?.startsWith('zh') ? ZH_BUNDLE[key] ?? key : key) };
110+
111+
async function makeEngine(i18n?: { t: (k: string, l: string) => string }) {
112+
const engine = new ObjectQL();
113+
const { driver } = makeStubDriver();
114+
engine.registerDriver(driver, true);
115+
await engine.init();
116+
for (const o of [businessUnit, sporadicApplication, archiveNote]) engine.registry.registerObject(o as any);
117+
if (i18n) engine.setI18nService(i18n);
118+
return engine;
119+
}
120+
121+
/** Seed one parent + one required-FK child and return the refusal it throws. */
122+
async function refuseDelete(engine: ObjectQL, locale?: string): Promise<any> {
123+
const bu = await engine.insert('sys_business_unit', { name: 'HR' });
124+
await engine.insert('os_ehr_sporadic_application', { title: '差旅', apply_dept: bu.id });
125+
try {
126+
await engine.delete('sys_business_unit', { where: { id: bu.id }, context: { locale } } as any);
127+
} catch (e) {
128+
return e;
129+
}
130+
throw new Error('expected the delete to be refused');
131+
}
132+
133+
describe('#7307 DELETE_RESTRICTED — user copy vs developer guidance', () => {
134+
let engine: ObjectQL;
135+
136+
describe('with no i18n service (the bare-kernel / programmatic caller)', () => {
137+
beforeEach(async () => { engine = await makeEngine(); });
138+
139+
it('still refuses the delete: 409, DELETE_RESTRICTED, structured fields unchanged', async () => {
140+
const err = await refuseDelete(engine);
141+
expect(err).toMatchObject({
142+
code: 'DELETE_RESTRICTED',
143+
status: 409,
144+
object: 'sys_business_unit',
145+
dependentObject: 'os_ehr_sporadic_application',
146+
dependentCount: 1,
147+
});
148+
});
149+
150+
it('names the objects by their DECLARED labels, never by API name', async () => {
151+
const err = await refuseDelete(engine);
152+
expect(err.message).toContain('Business Unit');
153+
expect(err.message).toContain('Sporadic Application');
154+
expect(err.message).toContain('Applying Department');
155+
for (const api of API_NAMES) expect(err.message).not.toContain(api);
156+
});
157+
158+
it('does not hand the user a metadata-authoring instruction', async () => {
159+
const err = await refuseDelete(engine);
160+
expect(err.message).not.toMatch(/deleteBehavior|cascade/i);
161+
// The half that IS actionable for a user survives.
162+
expect(err.message).toMatch(/Delete or reassign them first/);
163+
});
164+
});
165+
166+
describe('with a zh-CN deployment (the reported app)', () => {
167+
beforeEach(async () => { engine = await makeEngine(zhI18n); });
168+
169+
it('renders the toast sentence in the caller locale, with TRANSLATED labels', async () => {
170+
const err = await refuseDelete(engine, 'zh-CN');
171+
expect(err.message).toBe(
172+
'该部门正被 1 条零星申请记录通过「申报部门」引用,且该字段为必填、无法清空,请先删除或改派这些记录。',
173+
);
174+
});
175+
176+
it('leaks no API name and no developer vocabulary into the toast', async () => {
177+
const err = await refuseDelete(engine, 'zh-CN');
178+
for (const api of API_NAMES) expect(err.message).not.toContain(api);
179+
expect(err.message).not.toMatch(/deleteBehavior|cascade/i);
180+
});
181+
182+
it('an i18n service that THROWS still yields a 409, not a 500, and still no leak', async () => {
183+
const boom = await makeEngine({ t: () => { throw new Error('i18n down'); } });
184+
const err = await refuseDelete(boom, 'zh-CN');
185+
expect(err).toMatchObject({ code: 'DELETE_RESTRICTED', status: 409 });
186+
expect(err.message).not.toMatch(/deleteBehavior/i);
187+
});
188+
189+
it('an unresolved locale falls back to English rather than to the API names', async () => {
190+
const err = await refuseDelete(engine, 'fr-FR');
191+
expect(err.message).toContain('Business Unit');
192+
for (const api of API_NAMES) expect(err.message).not.toContain(api);
193+
});
194+
});
195+
196+
describe('developerMessage — the guidance is moved, not lost', () => {
197+
beforeEach(async () => { engine = await makeEngine(zhI18n); });
198+
199+
it('carries the API names and the deleteBehavior remedy, in English, even for a zh-CN caller', async () => {
200+
const err = await refuseDelete(engine, 'zh-CN');
201+
expect(err.developerMessage).toContain('sys_business_unit');
202+
expect(err.developerMessage).toContain('os_ehr_sporadic_application');
203+
expect(err.developerMessage).toContain('apply_dept');
204+
expect(err.developerMessage).toContain(
205+
"set deleteBehavior:'cascade' on os_ehr_sporadic_application.apply_dept",
206+
);
207+
});
208+
209+
it('is the pre-#7307 sentence verbatim, so nothing a developer relied on changed wording', async () => {
210+
const err = await refuseDelete(engine, 'zh-CN');
211+
expect(err.developerMessage).toMatch(
212+
/^Cannot delete sys_business_unit \(.+\): 1 dependent os_ehr_sporadic_application record\(s\) reference it via apply_dept \(apply_dept is required, so it cannot be cleared\)\. Delete or reassign them first, or set deleteBehavior:'cascade' on os_ehr_sporadic_application\.apply_dept\.$/,
213+
);
214+
});
215+
216+
it('is a SEPARATE field — the user-facing message never contains it', async () => {
217+
const err = await refuseDelete(engine, 'zh-CN');
218+
expect(err.message).not.toContain(err.developerMessage);
219+
expect(err.message).not.toBe(err.developerMessage);
220+
});
221+
222+
it('reaches the SERVER LOG, so a zh-CN deployment does not log its operator half in Chinese', async () => {
223+
const logged: Array<Record<string, unknown>> = [];
224+
const original = (engine as any).logger.error.bind((engine as any).logger);
225+
(engine as any).logger.error = (msg: string, err: unknown, meta: Record<string, unknown>) => {
226+
logged.push(meta ?? {});
227+
return original(msg, err, meta);
228+
};
229+
await refuseDelete(engine, 'zh-CN');
230+
expect(logged.some((m) => typeof m.developerMessage === 'string'
231+
&& (m.developerMessage as string).includes("deleteBehavior:'cascade'"))).toBe(true);
232+
});
233+
});
234+
235+
describe('an EXPLICIT restrict on a nullable FK gets the other sentence', () => {
236+
beforeEach(async () => { engine = await makeEngine(zhI18n); });
237+
238+
it('omits the "required, cannot be cleared" clause it has no right to claim', async () => {
239+
const bu = await engine.insert('sys_business_unit', { name: 'Finance' });
240+
await engine.insert('os_ehr_archive_note', { body: 'n', dept: bu.id });
241+
const err = await engine
242+
.delete('sys_business_unit', { where: { id: bu.id }, context: { locale: 'zh-CN' } } as any)
243+
.then(() => { throw new Error('expected refusal'); }, (e) => e);
244+
245+
expect(err).toMatchObject({ code: 'DELETE_RESTRICTED', status: 409, dependentCount: 1 });
246+
expect(err.message).not.toContain('必填');
247+
expect(err.message).toContain('请先删除或改派这些记录');
248+
// No label on the child object's translation entries → declared label.
249+
expect(err.message).toContain('Archive Note');
250+
expect(err.developerMessage).not.toContain('is required, so it cannot be cleared');
251+
});
252+
});
253+
});

0 commit comments

Comments
 (0)