Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions .changeset/i18n-field-labels-emit-declared-shape.md
Original file line number Diff line number Diff line change
@@ -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<string, string>` — 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.<obj>.<fld>.<value>`) — 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.
9 changes: 7 additions & 2 deletions packages/runtime/src/http-dispatcher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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' } },
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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' } },
},
});
});
Expand Down
1 change: 1 addition & 0 deletions packages/spec/api-surface.json
Original file line number Diff line number Diff line change
Expand Up @@ -1068,6 +1068,7 @@
"ResolveOptions (interface)",
"ResolvedBook (interface)",
"ResolvedEntry (interface)",
"ResolvedFieldLabel (interface)",
"ResolvedGroup (interface)",
"ResolvedSettingValue (type)",
"ResolvedSettingValueSchema (const)",
Expand Down
60 changes: 57 additions & 3 deletions packages/spec/src/system/i18n-resolver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -1044,14 +1045,18 @@ 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: {
label: 'Contact',
fields: {
first_name: { label: 'First Name' },
email: { label: 'Email', help: 'Primary address' },
status: {
label: 'Status',
options: { open: 'Open', closed: 'Closed' },
},
phone: { help: 'Mobile preferred' },
},
},
Expand All @@ -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');
});

Expand All @@ -1088,4 +1114,32 @@ describe('resolveObjectFieldLabels (objectstack#3833)', () => {
} as unknown as Parameters<typeof resolveObjectFieldLabels>[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<string, string>` against a
* schema declaring `Record<string, { label, help?, options? }>` — 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);
});
});
34 changes: 30 additions & 4 deletions packages/spec/src/system/i18n-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string>` 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.<obj>.<fld>.<value>`) 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<string, string>;
}

export function resolveObjectFieldLabels(
data: TranslationData | undefined,
objectName: string,
): Record<string, string> {
): Record<string, ResolvedFieldLabel> {
const fields = data?.objects?.[objectName]?.fields;
const labels: Record<string, string> = {};
const labels: Record<string, ResolvedFieldLabel> = {};
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;
}
Expand Down
Loading