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
54 changes: 54 additions & 0 deletions .changeset/6223-object-payload-spec-keys.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
---
'@object-ui/plugin-designer': minor
'@object-ui/app-shell': minor
---

Object-level metadata payloads no longer emit the three keys `ObjectSchema` refuses by
name — **group**, **sortOrder** and **relationships** (objectui#6223).

Measured against the installed `@objectstack/spec` 17.2.0, whose `ObjectSchema` accept set
is 42 keys:

```
const base = { name: 'account', label: 'Account', fields: { n: { type: 'text', label: 'N' } } };

ObjectSchema.safeParse(base) => success = true (control)
ObjectSchema.safeParse({ ...base, isSystem: true }) => success = true (control)
ObjectSchema.safeParse({ ...base, pluralLabel: 'A' }) => success = true (control)

ObjectSchema.safeParse({ ...base, group: 'Sales' }) => unrecognized_keys ["group"]
ObjectSchema.safeParse({ ...base, sortOrder: 3 }) => unrecognized_keys ["sortOrder"]
ObjectSchema.safeParse({ ...base, relationships: [ … ] }) => unrecognized_keys ["relationships"]
```

The two controls are what make that a key-by-key result rather than a schema refusing
everything. Each key was resolved on its own, as the objectui#5761 family ruling requires:

- **group** — the Object Manager's grouping is a UI-only display category. The spec has no
object-level grouping key (`fieldGroups` groups the fields *inside* one object), so the
grouping control and its column stay, and the value is now DERIVED from the spec key that
is accepted (`isSystem`) instead of round-tripped. `MetadataObjectsPage` also strips a
`group` already stored by an earlier build, because its save-back spreads the server
document verbatim and would otherwise keep re-sending it forever.
- **sortOrder** — what populated it was the array index the converter happened to be at,
i.e. the order the list was already in. The declaration is removed from the object
payload. The field-level `sortOrder` is a different key with a different card
(objectui#6045) and is untouched.
- **relationships** — the spec models relationships on the FIELD (`reference` /
`master_detail`, plus object-level `indexes`). The object payload stops declaring and
sending an object-level relationship array; what the designer should author for a
relationship is a data-model question this change does not settle.

**Breaking for TypeScript consumers of `ObjectMetadataPayload`** (exported from app-shell):
the three properties are gone from the published type, so code that set them stops
compiling. That is the point — setting any of them produced a payload the metadata route
refuses. `ObjectDefinition` (the designer's UI model) is unchanged and still carries all
three.

The parity gate built for objectui#5761 now has a **second oracle**: every shape in
`PAYLOAD_SHAPES` names the schema that judges it, `ObjectSchema` alongside `FieldSchema`,
and reach is resolved within an oracle rather than across one — `group` is a legal
`FieldSchema` key and a refused `ObjectSchema` key at the same time. That extension found a
fourth object-level key (`enabled`, objectui#6238) and a value-level rejection the key-name
check cannot see (`fields` sent as an array where the spec wants a map, objectui#6240);
both are filed and ledgered rather than fixed here.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,250 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* objectui#6223 — the object payload `MetadataService` PUTs carries no key
* `ObjectSchema` refuses BY NAME.
*
* Surfaced by the key-level parity gate built for objectui#5761
* (`scripts/check-designer-field-key-parity.mjs`), once objectui#6223 gave it a
* SECOND ORACLE. Until then the gate compared field shapes against
* `FieldSchema` and nothing at all checked the parent document those fields are
* nested in, so three object-level keys sat on the wire while the gate was
* green. `ObjectMetadataPayload` is now one of that gate's object-level `wire`
* shapes: `toObjectPayload` builds it and `saveObject` PUTs it whole to
* `PUT /api/v1/meta/object/:name`.
*
* Measured against the installed `@objectstack/spec` 17.2.0 (ESM build), whose
* `ObjectSchema` accept set is 42 keys:
*
* ObjectSchema.safeParse({ ...base, group: 'Sales' }) => unrecognized_keys ["group"]
* ObjectSchema.safeParse({ ...base, sortOrder: 3 }) => unrecognized_keys ["sortOrder"]
* ObjectSchema.safeParse({ ...base, relationships: … }) => unrecognized_keys ["relationships"]
*
* The controls are what make that a KEY-BY-KEY result rather than a schema that
* refuses everything: `isSystem` and `pluralLabel` parse green on the same base
* document. Both are asserted below, first, before any claim about the fix.
*
* ## What this file does NOT claim
*
* It does not claim a reproduced HTTP 422. The card was explicit that whether
* the deployed route rejects these today depends on what that route parses
* with; the schema fact is the ground for the fix and is all that is asserted.
*
* ## Why the assertions are on bytes
*
* `undefined` is a key zod's strict object COUNTS and `JSON.stringify` DROPS.
* An in-memory assertion on the object handed to the client and a wire
* assertion therefore disagree exactly on the half-filled case, so every
* assertion here reads `JSON.parse` of the captured request body.
*
* ## One `it` per key, deliberately
*
* Three keys were resolved. A suite that exercised them together would stay
* green on a fix that landed two of three, so each key is pinned by name in its
* own case: reverting one resolution reds only that case.
*/

import { describe, expect, it, vi } from 'vitest';
import { ObjectSchema } from '@objectstack/spec/data';
import { ObjectStackAdapter } from '@object-ui/data-objectstack';
import type { ObjectDefinition } from '@object-ui/types';
import { MetadataService } from './MetadataService';

/** The bodies of every PUT the SDK issued, exactly as they went over the wire. */
function makeCapturingAdapter() {
const puts: Array<Record<string, unknown>> = [];
const adapter = new ObjectStackAdapter({
baseUrl: 'http://test.local',
fetch: vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => {
if ((init?.method ?? 'GET').toUpperCase() === 'PUT') {
puts.push(JSON.parse(String(init?.body ?? '{}')) as Record<string, unknown>);
}
return new Response(JSON.stringify({ success: true }), {
status: 200,
headers: { 'content-type': 'application/json' },
});
}) as unknown as typeof fetch,
});
return { adapter, puts };
}

const unrecognizedKeys = (result: ReturnType<typeof ObjectSchema.safeParse>): string[] =>
result.success
? []
: result.error.issues
.filter((i) => i.code === 'unrecognized_keys')
.flatMap((i) => (i as unknown as { keys: string[] }).keys);

/** A base document `ObjectSchema` accepts, for probing one key at a time. */
const BASE = { name: 'account', label: 'Account', fields: { n: { type: 'text', label: 'N' } } };

/**
* An object as the Object Manager holds it, with all three UI-only keys
* populated — the state a designer save actually starts from.
*/
const MANAGED: ObjectDefinition = {
id: 'account',
name: 'account',
label: 'Account',
pluralLabel: 'Accounts',
description: 'Customer accounts',
icon: 'Building',
group: 'Custom Objects',
sortOrder: 3,
isSystem: false,
fieldCount: 1,
relationships: [
{ relatedObject: 'contact', type: 'one-to-many', label: 'Contacts', foreignKey: 'account_id' },
],
};

async function putFor(obj: ObjectDefinition = MANAGED): Promise<Record<string, unknown>> {
const { adapter, puts } = makeCapturingAdapter();
await new MetadataService(adapter).saveObject(obj, [{ name: 'name', type: 'text', label: 'Name' }]);
expect(puts).toHaveLength(1);
return puts[0];
}

describe('the instrument', () => {
it('is the installed spec schema and it is STRICT — unknown keys are refused, not stripped', () => {
// objectstack#4001 closed the silent-drop shape. Every parity assertion
// below depends on it: a stripping schema would make them all trivially
// green while the 422 still happened server-side.
const result = ObjectSchema.safeParse({ ...BASE, zzzDefinitelyNotAKey: 1 });
expect(result.success).toBe(false);
expect(unrecognizedKeys(result)).toContain('zzzDefinitelyNotAKey');
});

it('accepts the controls — this is a key-by-key result, not a schema refusing everything', () => {
// Without these two, "ObjectSchema refuses `group`" would be worthless: a
// schema that refused every object would produce the same evidence.
expect(ObjectSchema.safeParse(BASE).success).toBe(true);
expect(ObjectSchema.safeParse({ ...BASE, isSystem: true }).success).toBe(true);
expect(ObjectSchema.safeParse({ ...BASE, pluralLabel: 'Accounts' }).success).toBe(true);
});

it('refuses each of the three keys BY NAME, one at a time', () => {
expect(unrecognizedKeys(ObjectSchema.safeParse({ ...BASE, group: 'Sales' }))).toEqual(['group']);
expect(unrecognizedKeys(ObjectSchema.safeParse({ ...BASE, sortOrder: 3 }))).toEqual(['sortOrder']);
expect(
unrecognizedKeys(
ObjectSchema.safeParse({ ...BASE, relationships: [{ relatedObject: 'contact', type: 'one-to-many' }] }),
),
).toEqual(['relationships']);
});

it('has no near-spelling for any of them — unlike objectui#6041, nothing here is a rename', () => {
const accept = new Set(Object.keys(ObjectSchema.shape as Record<string, unknown>));
expect(accept.size).toBe(42);
// `fieldGroups` is the only grouping key on the object, and it groups the
// FIELDS INSIDE one object — it is not a category for objects themselves,
// so `group` has no mapping target here.
expect(accept.has('fieldGroups')).toBe(true);
for (const key of ['group', 'sortOrder', 'relationships', 'order', 'category', 'sortField']) {
expect(accept.has(key), `ObjectSchema unexpectedly accepts \`${key}\``).toBe(false);
}
});
});

describe('objectui#6223 · `group` — the manager’s display category, never the payload', () => {
it('does not put `group` on the wire even when the object carries one', async () => {
const put = await putFor();
expect('group' in put).toBe(false);
// Falsification: the save really happened and really described this object.
expect(put.name).toBe('account');
expect(put.label).toBe('Account');
});

it('and `ObjectSchema` reports no `group` among the refused keys of that body', async () => {
expect(unrecognizedKeys(ObjectSchema.safeParse(await putFor()))).not.toContain('group');
});
});

describe('objectui#6223 · `sortOrder` — list order, not object metadata', () => {
it('does not put `sortOrder` on the wire even when the object carries one', async () => {
const put = await putFor();
expect('sortOrder' in put).toBe(false);
expect(put.name).toBe('account');
});

it('and `ObjectSchema` reports no `sortOrder` among the refused keys of that body', async () => {
expect(unrecognizedKeys(ObjectSchema.safeParse(await putFor()))).not.toContain('sortOrder');
});

it('leaves the FIELD-level `sortOrder` alone — that key is objectui#6045 and is not this card', async () => {
// The two keys share a spelling and nothing else. Reverting the object-level
// resolution must not read as progress on the field-level one, and this
// assertion is what keeps the two cards independently measurable.
const { adapter, puts } = makeCapturingAdapter();
await new MetadataService(adapter).saveFields('account', [
{ id: 'name', name: 'name', label: 'Name', type: 'text', sortOrder: 7 },
]);
const fields = puts[puts.length - 1].fields as Record<string, unknown>[];
expect(fields[0].sortOrder).toBe(7);
});
});

describe('objectui#6223 · `relationships` — the spec models these on the FIELD', () => {
it('does not put `relationships` on the wire even when the object carries them', async () => {
const put = await putFor();
expect('relationships' in put).toBe(false);
expect(put.name).toBe('account');
});

it('and `ObjectSchema` reports no `relationships` among the refused keys of that body', async () => {
expect(unrecognizedKeys(ObjectSchema.safeParse(await putFor()))).not.toContain('relationships');
});
});

describe('objectui#6223 · the whole body, and the honest limit of this fix', () => {
it('carries NO key `ObjectSchema` refuses by name', async () => {
// The claim of this card, stated once over the whole document rather than
// key by key. Before the fix this was ["group", "sortOrder", "relationships"].
expect(unrecognizedKeys(ObjectSchema.safeParse(await putFor()))).toEqual([]);
});

it('still carries everything the spec DOES accept — the fix removed keys, it did not empty the payload', async () => {
// Falsification for the assertion above: a payload of `{}` would also carry
// no refused key, and that is not the fix.
const put = await putFor();
expect(put).toMatchObject({
name: 'account',
label: 'Account',
pluralLabel: 'Accounts',
description: 'Customer accounts',
icon: 'Building',
});
});

it('does NOT parse green as a whole — `fields` is an array where the spec wants a map (objectui#6240)', async () => {
// Stated as an assertion rather than left as prose, because a reader who
// saw only the `unrecognized_keys` case above would reasonably conclude the
// designer's object payload is now spec-valid. It is not: this fix closes
// the KEY-NAME class, which is the class objectui#6223 and its parity gate
// are about. What remains is a VALUE-level rejection — the gate's coverage
// note 4 — and it is filed separately as objectui#6240.
const result = ObjectSchema.safeParse(await putFor());
expect(result.success).toBe(false);
expect(unrecognizedKeys(result)).toEqual([]);
expect(result.error?.issues.map((i) => `${i.code} @ ${i.path.join('.')}`)).toEqual([
'invalid_type @ fields',
]);
});

it('a half-filled object — no group, no sortOrder, no relationships — puts identical bytes, as it always did', async () => {
// ⚠ This case would still pass on a revert, deliberately. `undefined` is
// dropped by `JSON.stringify`, so an object that never had these keys
// populated produced byte-identical output before and after this fix. It is
// here to prove the fix did not newly break the untouched half, which is a
// claim about what did NOT change.
const put = await putFor({ id: 'lead', name: 'lead', label: 'Lead' });
expect(Object.keys(put).sort()).toEqual(['fields', 'label', 'name']);
expect(unrecognizedKeys(ObjectSchema.safeParse(put))).toEqual([]);
});
});
41 changes: 29 additions & 12 deletions packages/app-shell/src/services/MetadataService.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -30,16 +30,27 @@ export interface ObjectMetadataPayload {
pluralLabel?: string;
description?: string;
icon?: string;
group?: string;
sortOrder?: number;
// No `group` (objectui#6223): `ObjectSchema` has no object-level grouping
// key — its 42-key accept set contains `fieldGroups`, which groups the FIELDS
// inside one object, and nothing that categorises objects against each other.
// The designer's grouping IS a real feature, but a UI-only one: the Object
// Manager's group column and its group select are display categories derived
// from the object itself (`sys_` prefix / `isSystem` -> `System Objects` vs
// `Custom Objects`), never authored data the server round-trips. Writing it
// made `PUT /api/v1/meta/object/:name` refuse the key by name.
// No `sortOrder` (objectui#6223): `ObjectSchema` has no object-level ordering
// key either. What populated it was the ARRAY INDEX the converter happened to
// be at (`sortOrder: index`), i.e. the order the list was already in — a
// display concern of the manager, not object metadata. (Distinct from the
// field-level `sortOrder`, objectui#6045, which is still declared below.)
enabled?: boolean;
fields?: FieldMetadataPayload[];
relationships?: Array<{
relatedObject: string;
type: string;
label?: string;
foreignKey?: string;
}>;
// No `relationships` (objectui#6223): the spec models relationships on the
// FIELD — `reference` / `master_detail` plus object-level `indexes` — and
// `ObjectSchema` refuses an object-level `relationships` array by name. What
// the designer should author for a relationship is a data-model question
// that this card does not settle; what it settles is that this shape must
// stop putting the key on the wire.
}

/** Shape written to the metadata API for a field definition. */
Expand DownExpand Up@@ -76,18 +87,24 @@ export interface FieldMetadataPayload {
// Converters: UI types → API payloads
// ---------------------------------------------------------------------------

/** Convert an `ObjectDefinition` (UI) to the API payload shape. */
/**
* Convert an `ObjectDefinition` (UI) to the API payload shape.
*
* `ObjectDefinition` carries three keys that deliberately do NOT cross into the
* payload (objectui#6223): `group` and `sortOrder` are the Object Manager's own
* display category and display order, and `relationships` has no object-level
* home in the spec. `ObjectSchema` refuses all three BY NAME, so copying them
* across is what turned a designer save into a 422. The UI model keeps them;
* the wire shape does not.
*/
function toObjectPayload(obj: ObjectDefinition, fields?: FieldMetadataPayload[]): ObjectMetadataPayload {
return {
name: obj.name,
label: obj.label,
pluralLabel: obj.pluralLabel,
description: obj.description,
icon: obj.icon,
group: obj.group,
sortOrder: obj.sortOrder,
fields,
relationships: obj.relationships,
};
}

Expand Down
6 changes: 6 additions & 0 deletions packages/app-shell/src/utils/metadataConverters.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -83,6 +83,12 @@ export function toObjectDefinition(obj: MetadataObject, index: number): ObjectDe
pluralLabel: obj.pluralLabel || obj.plural_label || undefined,
description: typeof obj.description === 'object' ? obj.description.defaultValue : (obj.description || undefined),
icon: obj.icon || undefined,
// `group` and `sortOrder` are DISPLAY values of the Object Manager, derived
// here and belonging to the UI model only (objectui#6223). `ObjectSchema`
// has no object-level grouping or ordering key and refuses both BY NAME, so
// they must never be copied into an object payload — see the tombstones on
// `ObjectMetadataPayload`. Note what populates them: the `sys_` prefix and
// the array index, i.e. facts about this list, not about the object.
group: obj.name?.startsWith('sys_') ? 'System Objects' : 'Custom Objects',
sortOrder: index,
isSystem: obj.name?.startsWith('sys_') || false,
Expand Down
Loading
Loading