diff --git a/.changeset/i18n-field-labels-emit-declared-shape.md b/.changeset/i18n-field-labels-emit-declared-shape.md new file mode 100644 index 0000000000..bec5351b96 --- /dev/null +++ b/.changeset/i18n-field-labels-emit-declared-shape.md @@ -0,0 +1,60 @@ +--- +"@objectstack/spec": minor +"@objectstack/runtime": patch +"@objectstack/service-i18n": patch +--- + +fix(i18n)!: `/i18n/labels/:object/:locale` emits the entry shape it declares — +and stops discarding `help`/`options` (#3847) + +`GetFieldLabelsResponseSchema` has always declared each label as an object: + +```ts +labels: z.record(z.string(), z.object({ + label: z.string(), + help: z.string().optional(), + options: z.record(z.string(), z.string()).optional(), +})) +``` + +Both serving surfaces emitted `Record` — a bare label per field. +A client typed against `GetFieldLabelsResponse` read `labels[field].label` and +got `undefined`, because the value was the string itself. The SDK's type was +right the whole time; the servers were wrong. + +The cost is not only the type mismatch. `FieldTranslationSchema` carries `help` +and `options`, bundles populate them, and the endpoint threw them away. objectui +needs exactly those — its `spec-translations.ts` transform reads `label` **and** +`options` (as `fieldOptions...`) — and gets them by pulling the +whole bundle from `/i18n/translations/:locale` and resolving client-side. The +per-object endpoint could not have served it even if it wanted to: the data was +being dropped at the emit site. + +Fixed at that emit site, `resolveObjectFieldLabels`, which both surfaces already +share as of #3833 — so one change covers both. `help` and `options` are attached +only when non-empty: an `options: {}` would claim a field has translated options +and hand back none, and a `help: ''` would erase a caller's source help text. +Fields with no non-empty `label` are still omitted entirely, which is what lets +`ResolvedFieldLabel.label` be a required string. + +**The response schema is unchanged** — this moves the implementation onto the +contract, not the contract onto the implementation. Generated docs are +byte-identical for that reason. + +`placeholder` is deliberately left out. `FieldTranslationSchema` has it and the +response schema does not, so emitting it would be widening the contract rather +than satisfying it — and adding an optional response field later is additive and +non-breaking, whereas guessing now is not. + +The regression guard is the part worth keeping: a test that builds the response +body from the shared helper and parses it with `GetFieldLabelsResponseSchema`. +Nothing had ever put the emitted value and the declared contract in one +assertion, which is precisely why a bare string could sit under an object schema +unnoticed. Third and last of the declared ≠ enforced gaps on this endpoint +family, after #3676 (request filters no server read) and #3833 (a derivation +scanning a retired dialect). + +BREAKING: `labels[field]` is now `{ label, help?, options? }` rather than a +string. No consumer in this repo or objectui read it — objectui never calls this +route, and in-repo use is the SDK method plus URL-shape tests — so the practical +blast radius is nil, and this is the cheap moment to align it. diff --git a/packages/runtime/src/http-dispatcher.test.ts b/packages/runtime/src/http-dispatcher.test.ts index 19076d5ceb..70e5d021c8 100644 --- a/packages/runtime/src/http-dispatcher.test.ts +++ b/packages/runtime/src/http-dispatcher.test.ts @@ -1932,6 +1932,7 @@ describe('HttpDispatcher', () => { fields: { first_name: { label: 'First Name' }, email: { label: 'Email', help: 'Primary address' }, + status: { label: 'Status', options: { open: 'Open' } }, // No label — partial translation is the normal // state, and a blank entry would overwrite the // caller's source label with an empty string. @@ -1945,9 +1946,13 @@ describe('HttpDispatcher', () => { const result = await dispatcher.handleI18n('/labels/contact/en', 'GET', {}, { request: {} }); expect(result.handled).toBe(true); expect(result.response?.status).toBe(200); + // Entries are objects carrying help/options, per + // `GetFieldLabelsResponseSchema` — not the bare strings both + // surfaces used to emit against it (#3847). expect(result.response?.body?.data?.labels).toEqual({ - first_name: 'First Name', - email: 'Email', + first_name: { label: 'First Name' }, + email: { label: 'Email', help: 'Primary address' }, + status: { label: 'Status', options: { open: 'Open' } }, }); }); diff --git a/packages/services/service-i18n/src/i18n-service-plugin.test.ts b/packages/services/service-i18n/src/i18n-service-plugin.test.ts index 3f2952e8ff..2ee4012cc3 100644 --- a/packages/services/service-i18n/src/i18n-service-plugin.test.ts +++ b/packages/services/service-i18n/src/i18n-service-plugin.test.ts @@ -259,12 +259,16 @@ describe('I18nServicePlugin', () => { const res = createMockRes(); await handler(req, res); + // Each entry is an object, not a bare string: that is what + // `GetFieldLabelsResponseSchema` has always declared, and emitting a + // string contradicted it while discarding the `help`/`options` the + // bundle carries (#3847). expect(res._data).toEqual({ success: true, data: { object: 'account', locale: 'en', - labels: { name: 'Account Name' }, + labels: { name: { label: 'Account Name' } }, }, }); }); diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index 60f37f30c3..917da82f62 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -1068,6 +1068,7 @@ "ResolveOptions (interface)", "ResolvedBook (interface)", "ResolvedEntry (interface)", + "ResolvedFieldLabel (interface)", "ResolvedGroup (interface)", "ResolvedSettingValue (type)", "ResolvedSettingValueSchema (const)", diff --git a/packages/spec/src/system/i18n-resolver.test.ts b/packages/spec/src/system/i18n-resolver.test.ts index 15d6ee981f..ac02889393 100644 --- a/packages/spec/src/system/i18n-resolver.test.ts +++ b/packages/spec/src/system/i18n-resolver.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect } from 'vitest'; import { ObjectTranslationDataSchema, TranslationDataSchema, type TranslationBundle } from './translation.zod'; +import { GetFieldLabelsResponseSchema } from '../api/protocol.zod'; import { resolveViewLabel, resolveViewDescription, @@ -1044,7 +1045,7 @@ describe('translateObject inline actions (objectstack#3370)', () => { // resolveObjectFieldLabels — the `/i18n/labels/:object/:locale` body // ========================================== -describe('resolveObjectFieldLabels (objectstack#3833)', () => { +describe('resolveObjectFieldLabels (objectstack#3833, #3847)', () => { const data = TranslationDataSchema.parse({ objects: { contact: { @@ -1052,6 +1053,10 @@ describe('resolveObjectFieldLabels (objectstack#3833)', () => { fields: { first_name: { label: 'First Name' }, email: { label: 'Email', help: 'Primary address' }, + status: { + label: 'Status', + options: { open: 'Open', closed: 'Closed' }, + }, phone: { help: 'Mobile preferred' }, }, }, @@ -1061,14 +1066,35 @@ describe('resolveObjectFieldLabels (objectstack#3833)', () => { it('enumerates the labels a locale actually translates', () => { expect(resolveObjectFieldLabels(data, 'contact')).toEqual({ - first_name: 'First Name', - email: 'Email', + first_name: { label: 'First Name' }, + email: { label: 'Email', help: 'Primary address' }, + status: { label: 'Status', options: { open: 'Open', closed: 'Closed' } }, }); }); + it('carries the help and options the bundle holds (#3847)', () => { + // These are the translations the endpoint used to discard by emitting a + // bare string per field. objectui needs exactly them, and had to read the + // full-bundle route to get them. + const out = resolveObjectFieldLabels(data, 'contact'); + expect(out.email?.help).toBe('Primary address'); + expect(out.status?.options).toEqual({ open: 'Open', closed: 'Closed' }); + }); + + it('omits help/options entirely rather than emitting empty ones', () => { + // An `options: {}` would claim the field has translated options and hand + // back none; `help: ''` would erase a caller's source help text. + const out = resolveObjectFieldLabels(data, 'contact'); + expect(out.first_name).toEqual({ label: 'First Name' }); + expect(Object.hasOwn(out.first_name!, 'help')).toBe(false); + expect(Object.hasOwn(out.first_name!, 'options')).toBe(false); + }); + it('omits fields carrying no label rather than emitting a blank one', () => { // Partial translation is the normal state (see ObjectTranslationDataSchema), // and callers merge this over their source labels — a '' would erase them. + // `phone` has help but no label, so it yields no entry at all — which is + // what lets `ResolvedFieldLabel.label` be a required string. expect(resolveObjectFieldLabels(data, 'contact')).not.toHaveProperty('phone'); }); @@ -1088,4 +1114,32 @@ describe('resolveObjectFieldLabels (objectstack#3833)', () => { } as unknown as Parameters[0]; expect(resolveObjectFieldLabels(flat, 'contact')).toEqual({}); }); + + /** + * The guard that would have caught #3847 on the day it was introduced: + * build the response the surfaces actually send and parse it with the schema + * that declares it. Both used to emit `Record` against a + * schema declaring `Record` — a + * mismatch no test compared, because none of them ever put the emitted + * value and the declared contract in the same assertion. + */ + it('produces a body that satisfies GetFieldLabelsResponseSchema (#3847)', () => { + const response = { + object: 'contact', + locale: 'en-US', + labels: resolveObjectFieldLabels(data, 'contact'), + }; + const parsed = GetFieldLabelsResponseSchema.safeParse(response); + expect(parsed.success).toBe(true); + expect(parsed.data?.labels.status?.options?.open).toBe('Open'); + }); + + it('an empty label map is still a valid response body', () => { + const parsed = GetFieldLabelsResponseSchema.safeParse({ + object: 'account', + locale: 'en-US', + labels: resolveObjectFieldLabels(data, 'account'), + }); + expect(parsed.success).toBe(true); + }); }); diff --git a/packages/spec/src/system/i18n-resolver.ts b/packages/spec/src/system/i18n-resolver.ts index 28af462cb8..cfbd9c6c1a 100644 --- a/packages/spec/src/system/i18n-resolver.ts +++ b/packages/spec/src/system/i18n-resolver.ts @@ -746,18 +746,44 @@ function lookupObjectFieldAttr( * * Fields with no non-empty `label` are omitted rather than emitted blank: * partial translation is the normal state (see `ObjectTranslationDataSchema`), - * and a caller merges what comes back over its source labels. + * and a caller merges what comes back over its source labels. That rule is + * also what lets `ResolvedFieldLabel.label` be required — an entry exists + * precisely because a label was found, so a field carrying only `help` yields + * no entry rather than one with a blank label. + * + * Each entry carries `help` and `options` alongside `label`, which is what + * `GetFieldLabelsResponseSchema` has always declared. Both surfaces used to + * emit a bare `Record` instead, so the endpoint contradicted + * its own response schema AND discarded translations the bundle already + * carried — `FieldTranslationSchema` populates `help` and `options`, and + * objectui reads exactly those (as `fieldOptions...`) off the + * full-bundle route, because this endpoint could not give them to it (#3847). */ +export interface ResolvedFieldLabel { + /** Translated field label. Required — an entry exists only because it has one. */ + label: string; + /** Translated help text, when the bundle carries one. */ + help?: string; + /** Option value → translated option label, when the bundle carries them. */ + options?: Record; +} + export function resolveObjectFieldLabels( data: TranslationData | undefined, objectName: string, -): Record { +): Record { const fields = data?.objects?.[objectName]?.fields; - const labels: Record = {}; + const labels: Record = {}; if (!fields) return labels; for (const [fieldName, field] of Object.entries(fields)) { const label = field?.label; - if (typeof label === 'string' && label.length > 0) labels[fieldName] = label; + if (typeof label !== 'string' || label.length === 0) continue; + const entry: ResolvedFieldLabel = { label }; + if (typeof field.help === 'string' && field.help.length > 0) entry.help = field.help; + // Only a non-empty map — an empty `options: {}` would claim the field has + // translated options and hand back none. + if (field.options && Object.keys(field.options).length > 0) entry.options = { ...field.options }; + labels[fieldName] = entry; } return labels; }