Skip to content

Commit c2fdbf9

Browse files
authored
fix(objectql): surface human validation messages in ValidationError.message (#2577)
An object-level validation rule's author-written message (often localized, e.g. 最小水深不能大于最大水深。) was buried in fields[] while the top-level Error.message — what the console toast, CLI, and SDK callers display — only said "Validation failed for 1 field(s): _record (rule_violation)". Build the top-level message from the per-field human messages (joined with '; '), falling back to "field (code)" when a message is blank. code/fields[] stay unchanged for programmatic consumers.
1 parent 9693a36 commit c2fdbf9

5 files changed

Lines changed: 86 additions & 15 deletions

File tree

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
"@objectstack/objectql": patch
3+
---
4+
5+
fix(objectql): surface the human validation message in `ValidationError.message`, not a `field (code)` digest
6+
7+
When an object-level validation rule (ADR-0020 `validations[]`) rejected a
8+
save, the console toast showed the generic English string
9+
`Validation failed for 1 field(s): _record (rule_violation)` instead of the
10+
rule author's own `message` (often localized, e.g. 最小水深不能大于最大水深。).
11+
12+
The author's message was always transported in `ValidationError.fields[].message`
13+
through the whole chain (rule-validator → REST envelope `fields[]` → client SDK
14+
`error.details`), but every generic UI surface displays the top-level
15+
`Error.message`, which only contained the `field (code)` pairs.
16+
17+
Fix at the single choke point — the `ValidationError` constructor now builds its
18+
top-level message from the per-field human messages (joined with `; `), falling
19+
back to `field (code)` only when a field error has no message. Machine-readable
20+
`code` and `fields[]` are unchanged, so programmatic consumers and the REST
21+
envelope shape are unaffected; every client (console toast, CLI, SDK callers)
22+
now sees the author-written message with no client-side change needed.

packages/objectql/src/engine-multivalue-normalize.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,15 +149,15 @@ describe('engine write pipeline — multi-value scalar normalization (#2552)', (
149149
const ql = await makeEngine(driver);
150150
await expect(
151151
ql.update('project', { id: 'r1', labels: { nested: true } }),
152-
).rejects.toThrow(/invalid_type/i);
152+
).rejects.toThrow(/must be an array/i);
153153
expect(updated).toHaveLength(0);
154154
});
155155

156156
it('insert: invalid option inside a wrapped scalar still 400s', async () => {
157157
const { driver, created } = makeDriver();
158158
const ql = await makeEngine(driver);
159159
await expect(ql.insert('project', { name: 'P2', labels: 'nope' })).rejects.toThrow(
160-
/invalid_option/i,
160+
/is not one of/i,
161161
);
162162
expect(created).toHaveLength(0);
163163
});

packages/objectql/src/validation/record-validator.test.ts

Lines changed: 51 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
22

33
import { describe, it, expect } from 'vitest';
4-
import { validateRecord, normalizeMultiValueFields } from './record-validator.js';
4+
import { validateRecord, normalizeMultiValueFields, ValidationError } from './record-validator.js';
55

66
/**
77
* Required-field validation, with the autonumber exemption (#1603).
@@ -55,14 +55,14 @@ describe('validateRecord — time field accepts time-of-day', () => {
5555

5656
for (const v of ['25:00', '14:60', 'not-a-time', '14']) {
5757
it(`rejects ${v}`, () => {
58-
expect(() => validateRecord(schema, { at: v }, 'insert')).toThrow(/invalid_time/i);
58+
expect(() => validateRecord(schema, { at: v }, 'insert')).toThrow(/must be a valid time/i);
5959
});
6060
}
6161

6262
it('does NOT regress date/datetime (still ISO-parsed)', () => {
6363
const ds = { fields: { d: { type: 'date' }, dt: { type: 'datetime' } } };
6464
expect(() => validateRecord(ds, { d: '2026-06-17', dt: '2026-06-17T10:00:00Z' }, 'insert')).not.toThrow();
65-
expect(() => validateRecord(ds, { d: 'not-a-date' }, 'insert')).toThrow(/invalid_date/i);
65+
expect(() => validateRecord(ds, { d: 'not-a-date' }, 'insert')).toThrow(/must be a valid date/i);
6666
});
6767
});
6868

@@ -160,13 +160,13 @@ describe('validateRecord — multi-value fields must be arrays', () => {
160160
{ watchers: 'user-1' },
161161
{ attachments: 'file-key-1' },
162162
]) {
163-
expect(() => validateRecord(schema, payload, 'update')).toThrow(/invalid_type/i);
163+
expect(() => validateRecord(schema, payload, 'update')).toThrow(/must be an array/i);
164164
}
165165
});
166166

167167
it('rejects a plain-object shape with invalid_type', () => {
168-
expect(() => validateRecord(schema, { labels: { nested: true } }, 'update')).toThrow(/invalid_type/i);
169-
expect(() => validateRecord(schema, { team_members: { id: 'u1' } }, 'update')).toThrow(/invalid_type/i);
168+
expect(() => validateRecord(schema, { labels: { nested: true } }, 'update')).toThrow(/must be an array/i);
169+
expect(() => validateRecord(schema, { team_members: { id: 'u1' } }, 'update')).toThrow(/must be an array/i);
170170
});
171171

172172
it('accepts arrays (including for select+multiple, previously mis-rejected)', () => {
@@ -180,12 +180,54 @@ describe('validateRecord — multi-value fields must be arrays', () => {
180180
});
181181

182182
it('still validates array ELEMENTS against options', () => {
183-
expect(() => validateRecord(schema, { labels: ['nope'] }, 'update')).toThrow(/invalid_option/i);
184-
expect(() => validateRecord(schema, { channels: ['fax'] }, 'update')).toThrow(/invalid_option/i);
183+
expect(() => validateRecord(schema, { labels: ['nope'] }, 'update')).toThrow(/is not one of/i);
184+
expect(() => validateRecord(schema, { channels: ['fax'] }, 'update')).toThrow(/is not one of/i);
185185
});
186186

187187
it('does NOT regress single select / radio', () => {
188188
expect(() => validateRecord(schema, { status: 'active' }, 'update')).not.toThrow();
189-
expect(() => validateRecord(schema, { status: 'nope' }, 'update')).toThrow(/invalid_option/i);
189+
expect(() => validateRecord(schema, { status: 'nope' }, 'update')).toThrow(/must be one of/i);
190+
});
191+
});
192+
193+
/**
194+
* The top-level `ValidationError.message` is what generic UI surfaces (the
195+
* console's save-error toast, CLI output) display verbatim — it must carry
196+
* the HUMAN per-field messages, not a `field (code)` digest. Regression for
197+
* the rule-violation case: an author-written localized rule `message`
198+
* ("最小水深不能大于最大水深。") used to be buried in `fields[]` while the
199+
* toast showed "Validation failed for 1 field(s): _record (rule_violation)".
200+
*/
201+
describe('ValidationError — top-level message is human-readable', () => {
202+
it('uses each field error message verbatim', () => {
203+
const err = new ValidationError([
204+
{ field: '_record', code: 'rule_violation', message: '最小水深不能大于最大水深。' },
205+
]);
206+
expect(err.message).toBe('最小水深不能大于最大水深。');
207+
});
208+
209+
it('joins multiple field messages', () => {
210+
const err = new ValidationError([
211+
{ field: 'title', code: 'required', message: 'title is required' },
212+
{ field: '_record', code: 'rule_violation', message: '最小水深不能大于最大水深。' },
213+
]);
214+
expect(err.message).toBe('title is required; 最小水深不能大于最大水深。');
215+
});
216+
217+
it('falls back to `field (code)` when a message is blank', () => {
218+
const err = new ValidationError([
219+
{ field: '_record', code: 'rule_violation', message: '' },
220+
]);
221+
expect(err.message).toBe('_record (rule_violation)');
222+
});
223+
224+
it('still exposes machine-readable fields[] for programmatic handling', () => {
225+
const err = new ValidationError([
226+
{ field: '_record', code: 'rule_violation', message: 'boom' },
227+
]);
228+
expect(err.code).toBe('VALIDATION_FAILED');
229+
expect(err.fields).toEqual([
230+
{ field: '_record', code: 'rule_violation', message: 'boom' },
231+
]);
190232
});
191233
});

packages/objectql/src/validation/record-validator.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,16 @@ export class ValidationError extends Error {
8383
readonly code = 'VALIDATION_FAILED';
8484
readonly fields: FieldValidationError[];
8585
constructor(fields: FieldValidationError[]) {
86+
// The top-level message is what generic UI surfaces (toasts, CLI output)
87+
// display verbatim, so it must carry the HUMAN messages — most notably a
88+
// validation rule's author-written `message` (often localized), which used
89+
// to be buried in `fields[]` while the toast showed only
90+
// "Validation failed for 1 field(s): _record (rule_violation)".
91+
// Machine-readable field/code pairs remain available on `.fields`.
8692
super(
87-
`Validation failed for ${fields.length} field(s): ` +
88-
fields.map((f) => `${f.field} (${f.code})`).join(', '),
93+
fields
94+
.map((f) => (f.message?.trim() ? f.message : `${f.field} (${f.code})`))
95+
.join('; ') || 'Validation failed',
8996
);
9097
this.name = 'ValidationError';
9198
this.fields = fields;

packages/objectql/src/validation/rule-null-omitted.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,12 @@ const schema = {
2828
describe('validation: `field == null` on insert with omitted field (#1871)', () => {
2929
it('fires when due_date is OMITTED from the insert payload', () => {
3030
expect(() => evaluateValidationRules(schema, { priority: 'urgent' }, 'insert', {}))
31-
.toThrow(/rule_violation|_record|Validation failed/);
31+
.toThrow(/Urgent tasks require a due date/);
3232
});
3333

3434
it('fires when due_date is explicitly null (already worked)', () => {
3535
expect(() => evaluateValidationRules(schema, { priority: 'urgent', due_date: null }, 'insert', {}))
36-
.toThrow(/rule_violation|_record|Validation failed/);
36+
.toThrow(/Urgent tasks require a due date/);
3737
});
3838

3939
it('does NOT fire when due_date is present', () => {

0 commit comments

Comments
 (0)