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
56 changes: 56 additions & 0 deletions .changeset/6045-field-payload-sort-order.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
---
'@object-ui/app-shell': minor
'@object-ui/types': minor
---

The field metadata payload no longer emits `sortOrder`, the key `FieldSchema` refuses by
name (objectui#6045). Field-level sibling of objectui#6223, same objectui#5761 family.

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

```
FieldSchema.safeParse({ type:'text', label:'L' }) => success = true (control)
FieldSchema.safeParse({ type:'text', label:'L', sortOrder: 3 }) => unrecognized_keys ["sortOrder"]

FieldSchema.safeParse({ type:'text', label:'L', sortable: true }) => success = true (control)
FieldSchema.safeParse({ type:'text', label:'L', sortable: 3 }) => success = false
```

The control is what makes that a key-by-key result rather than a schema refusing
everything, and the `sortable` pair is what shows the near-spelling is a *different
concept* — a boolean ("whether field is sortable in list views"), not this key's spec
name.

**The resolution was deletion, not a rename**, which is objectui#4687's shape rather than
objectui#6041's. The spec has no field-level ordering key at all: it models field order by
**declaration order** in the object's `fields` record, so a designer that wants explicit
ordering reorders that record rather than carrying an index. There was nothing to map onto,
and nothing was invented to map onto.

**It was latent, and that is confirmed on today's tree.** Neither of the two sites that
construct a `DesignerFieldDefinition` — `FieldDesigner`'s create/update handlers and
`MetadataFieldsPage.toDesignerField` — ever named the key, so `toFieldPayload` emitted
`sortOrder: undefined` and `JSON.stringify` dropped it. The key never reached the wire. It
was one reorder feature away from doing so, which is the objectui#4644 shape: a hard 422
`INVALID_METADATA` that blocks every subsequent save of the object, with nothing in the UI
to say which key caused it.

Removed in one go from the wire shape (`FieldMetadataPayload`), its writer
(`toFieldPayload`) and the UI model (`DesignerFieldDefinition`), so no declaration is left
behind that no writer fills and no schema accepts.

**Breaking for TypeScript consumers**: `sortOrder` is gone from `DesignerFieldDefinition`
(`@object-ui/types`) and from `FieldMetadataPayload` (app-shell), so code that set either
stops compiling.

Two keys share this spelling and are untouched, which is why the census was on the *shape*
— a field-metadata payload key `FieldSchema` refuses — rather than on the identifier: the
**object-level** `sortOrder` (`ObjectSchema`'s, removed from the object wire shape by
objectui#6223 and deliberately kept on the `ObjectDefinition` UI model) and the
**saved-view** `sortOrder` in `ObjectView`, which is per-view display order on a different
document entirely.

The `KNOWN_UNPARSEABLE_KEYS` entry in `scripts/check-designer-field-key-parity.mjs` goes
with the fix — that ledger ratchets in both directions, so an entry left behind for a
resolved key is as red as a missing one.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
/**
* 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#6045 — `MetadataService` never writes a field-level `sortOrder`.
*
* Surfaced by the key-level parity gate built for objectui#5761
* (`scripts/check-designer-field-key-parity.mjs`). `FieldMetadataPayload` is one
* of that gate's field-level `wire` shapes: `toFieldPayload` builds it and
* `saveFields` PUTs `fields.map(toFieldPayload)` to
* `PUT /api/v1/meta/object/:name`.
*
* `sortOrder` is not in `FieldSchema`'s accept set. Measured against the
* installed `@objectstack/spec` 17.2.0:
*
* FieldSchema.safeParse({ type:'text', label:'L', sortOrder: 3 })
* => success = false
* => unrecognized_keys ['sortOrder']
*
* which the route returns as a hard 422 `INVALID_METADATA`. Because the key is
* then STORED, every later save of that object fails the same way.
*
* ## Why the resolution was deletion, not a rename
*
* This is objectui#4687's shape, not objectui#6041's. The spec has no
* field-level ordering key AT ALL — it models field order by DECLARATION ORDER
* in the object's `fields` record — so there was no spelling to move to. The
* near-spelling `sortable` is NOT a rename target and the control below proves
* it is a different concept: it is a BOOLEAN ("whether field is sortable in
* list views"), so `sortable: 3` does not even parse.
*
* ## What "latent" meant, and which assertion is the load-bearing one
*
* Nothing on the tree ever populated a field's `sortOrder`: neither of the two
* sites that construct a `DesignerFieldDefinition` (`FieldDesigner`'s
* create/update handlers and `MetadataFieldsPage.toDesignerField`) names the
* key, so `toFieldPayload` emitted `sortOrder: undefined` and
* `JSON.stringify` dropped it. That has two consequences for this file:
*
* - The plain "a normal field PUTs no `sortOrder`" case below WOULD STILL
* PASS if the copy were restored, exactly as objectui#6223's half-filled
* case would. It is here to show the removal did not break the untouched
* path — a claim about what did NOT change.
* - The SMUGGLED case is the one that reds on a revert, and it is the reason
* this file is not a pin on an assertion that cannot fail. Restoring
* `sortOrder: field.sortOrder` in `toFieldPayload` puts the key back on the
* wire and fails it.
*
* Assertions are on the bytes actually PUT — `JSON.parse` of the captured
* request body — not on the object handed to the client. A property whose value
* is `undefined` is a key zod's strict object COUNTS but `JSON.stringify`
* DROPS, so an in-memory assertion and a wire assertion disagree exactly here.
*
* ## The two keys that share this spelling and are NOT this card
*
* `sortOrder` names three unrelated concepts in this repo, which is why the
* census for this card was on the SHAPE (a field-metadata payload key
* `FieldSchema` refuses) rather than on the identifier:
*
* - OBJECT-level `sortOrder` — the Object Manager's display order, refused by
* `ObjectSchema`, removed from the object wire shape by objectui#6223 and
* deliberately KEPT on the `ObjectDefinition` UI model. Pinned by
* `MetadataService.specKeyObjectPayload.test.ts`.
* - SAVED-VIEW `sortOrder` — `ObjectView.tsx`'s per-view display order, a
* real persisted key on a different document entirely.
*
* This file names neither, so reverting either of those cannot red it.
*/

import { describe, expect, it, vi } from 'vitest';
import { FieldSchema } from '@objectstack/spec/data';
import { ObjectStackAdapter } from '@object-ui/data-objectstack';
import type { DesignerFieldDefinition } 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 };
}

/** The field defs of the last PUT, in wire order. */
function savedFields(puts: Array<Record<string, unknown>>): Record<string, unknown>[] {
return puts[puts.length - 1].fields as Record<string, unknown>[];
}

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

const PLAIN_FIELD: DesignerFieldDefinition = {
id: 'amount',
name: 'amount',
label: 'Amount',
type: 'number',
};

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 = FieldSchema.safeParse({ type: 'text', label: 'L', zzzDefinitelyNotAKey: 1 });
expect(result.success).toBe(false);
expect(unrecognizedKeys(result)).toContain('zzzDefinitelyNotAKey');
});

it('refuses `sortOrder` BY NAME on a document it otherwise accepts', () => {
// The control that makes this a key-by-key result rather than a schema
// refusing everything: the same base parses green without the key.
expect(FieldSchema.safeParse({ type: 'text', label: 'L' }).success).toBe(true);
expect(unrecognizedKeys(FieldSchema.safeParse({ type: 'text', label: 'L', sortOrder: 3 }))).toEqual([
'sortOrder',
]);
});

it('has no field-level ordering key to rename onto — and `sortable` is not one', () => {
const accept = new Set(Object.keys(FieldSchema.shape as Record<string, unknown>));
// The near-spelling exists and is a BOOLEAN, so it is a different concept
// rather than this key's spec name. Asserted, because "do not conflate
// `sortable`" is prose until something can fail on it.
expect(accept.has('sortable')).toBe(true);
expect(FieldSchema.safeParse({ type: 'text', label: 'L', sortable: 3 }).success).toBe(false);
expect(FieldSchema.safeParse({ type: 'text', label: 'L', sortable: true }).success).toBe(true);
// And nothing else in the accept set is an ordering index either.
for (const key of ['sortOrder', 'order', 'position', 'index', 'sequence', 'displayOrder']) {
expect(accept.has(key), `FieldSchema unexpectedly accepts \`${key}\``).toBe(false);
}
});
});

describe('objectui#6045 · saveFields never PUTs a field-level `sortOrder`', () => {
it('drops a `sortOrder` smuggled onto the field instead of copying it through', async () => {
// THE load-bearing case — see this file's header. `DesignerFieldDefinition`
// no longer DECLARES `sortOrder`, so this cast is the point rather than a
// workaround: it proves `toFieldPayload` is closed at RUNTIME, not merely
// that the type forbids the key. A stale build, a JS caller, or the
// drag-to-reorder control this card exists to get ahead of all arrive by
// exactly this route — as does `FieldDesigner`'s update handler, which
// spreads the previous field verbatim.
const { adapter, puts } = makeCapturingAdapter();
const smuggled = { ...PLAIN_FIELD, sortOrder: 7 } as DesignerFieldDefinition;

await new MetadataService(adapter).saveFields('invoice', [smuggled]);

const [def] = savedFields(puts);
expect('sortOrder' in def).toBe(false);
// Falsification: the field itself made the trip, so the absence above is a
// payload builder that stopped copying the key, not an empty PUT.
expect(def.name).toBe('amount');
expect(def.type).toBe('number');
});

it('and that smuggled body parses through the real FieldSchema', async () => {
const { adapter, puts } = makeCapturingAdapter();
const smuggled = { ...PLAIN_FIELD, sortOrder: 7 } as DesignerFieldDefinition;

await new MetadataService(adapter).saveFields('invoice', [smuggled]);

const result = FieldSchema.safeParse(savedFields(puts)[0]);
expect(unrecognizedKeys(result)).toEqual([]);
expect(result.success).toBe(true);
});

it('a field that never carried one PUTs identical bytes, as it always did', async () => {
// ⚠ This case would still pass on a revert, deliberately — the key was
// latent precisely because `JSON.stringify` drops the `undefined`. It is
// here to prove the removal did not newly break the untouched path.
const { adapter, puts } = makeCapturingAdapter();

await new MetadataService(adapter).saveFields('invoice', [PLAIN_FIELD]);

const [def] = savedFields(puts);
expect(Object.keys(def).sort()).toEqual(['label', 'name', 'type']);
expect(unrecognizedKeys(FieldSchema.safeParse(def))).toEqual([]);
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -177,16 +177,26 @@ describe('objectui#6223 · `sortOrder` — list order, not object metadata', ()
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);
it('is measured on the OBJECT document only — the field-level key is objectui#6045', async () => {
// The two keys share a spelling and nothing else, and this case is what
// keeps the two cards independently measurable.
//
// It used to assert the OPPOSITE — that `saveFields` still put a
// field-level `sortOrder` on the wire — because when objectui#6223 landed,
// objectui#6045 was still open and the object-level fix had to be provable
// WITHOUT quietly resolving the field-level one. objectui#6045 has since
// removed that key from `FieldMetadataPayload`, from `toFieldPayload` and
// from `DesignerFieldDefinition`, so the old assertion is a fixture that
// pinned exactly the branch that card deleted: it is replaced rather than
// respelled. What survives is the claim it was really making — the object
// half is judged on the object document, and a field's absence of the key
// is not evidence about it either way.
const put = await putFor();
expect('sortOrder' in put).toBe(false);
// The object-level resolution is still visible on the UI model it kept:
// reverting objectui#6045 cannot make this case green or red.
expect(MANAGED.sortOrder).toBe(3);
// Field-level coverage lives in `MetadataService.retiredFieldSortOrder.test.ts`.
});
});

Expand Down
24 changes: 20 additions & 4 deletions packages/app-shell/src/services/MetadataService.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -42,7 +42,8 @@ export interface ObjectMetadataPayload {
// 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.)
// field-level `sortOrder`, which objectui#6045 has since removed for its own
// reasons — `FieldSchema` refuses that spelling too, at the other level.)
enabled?: boolean;
fields?: FieldMetadataPayload[];
// No `relationships` (objectui#6223): the spec models relationships on the
Expand DownExpand Up@@ -92,7 +93,16 @@ export interface FieldMetadataPayload {
// immediate 422 into a formula that parses and then silently evaluates to
// null. Expressions are authored in metadata-admin's `ObjectFieldInspector`,
// which lints them against the real `@objectstack/formula` engine.
sortOrder?: number;
// No `sortOrder` (objectui#6045): `FieldSchema` refuses it BY NAME and the
// spec has no field-level ordering key at all. The near-spelling `sortable`
// is NOT it — that is a boolean ("whether field is sortable in list views"),
// a different concept, so this is objectui#4687's shape (a declaration with
// zero readers and zero writers) and not objectui#6041's rename. The spec
// models field order by DECLARATION ORDER in the object's `fields` record;
// a designer that wants explicit ordering reorders that record rather than
// carrying an index. (Distinct from the object-level `sortOrder` retired by
// objectui#6223, and from the saved-view `sortOrder` in `ObjectView.tsx`,
// which is per-view display order and untouched by this card.)
}

// ---------------------------------------------------------------------------
Expand DownExpand Up@@ -120,7 +130,14 @@ function toObjectPayload(obj: ObjectDefinition, fields?: FieldMetadataPayload[])
};
}

/** Convert a `DesignerFieldDefinition` (UI) to the API payload shape. */
/**
* Convert a `DesignerFieldDefinition` (UI) to the API payload shape.
*
* It no longer copies `sortOrder` (objectui#6045). `FieldSchema` refuses that
* key by name and nothing on the tree ever populated it, so the write was
* latent — `JSON.stringify` drops the `undefined` — but one reorder feature
* away from a hard 422 that blocks every later save of the object.
*/
function toFieldPayload(field: DesignerFieldDefinition): FieldMetadataPayload {
return {
name: field.name,
Expand All@@ -138,7 +155,6 @@ function toFieldPayload(field: DesignerFieldDefinition): FieldMetadataPayload {
externalId: field.externalId,
trackHistory: field.trackHistory,
reference: field.referenceTo,
sortOrder: field.sortOrder,
};
}

Expand Down
25 changes: 23 additions & 2 deletions packages/types/src/designer.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -780,8 +780,29 @@ export interface DesignerFieldDefinition {
type: DesignerFieldType;
/** Field group/section */
group?: string;
/** Sort order within group */
sortOrder?: number;
/*
* There is deliberately no `sortOrder` here (objectui#6045). `FieldSchema`
* rejects the key by name and the spec has NO field-level ordering key at
* all — field order is DECLARATION ORDER in the object's `fields` record, so
* a designer that wants explicit ordering reorders that record rather than
* carrying an index. The near-spelling `sortable` is not a rename target: it
* is a boolean ("whether field is sortable in list views"), a different
* concept entirely.
*
* Nothing ever populated it. `MetadataService.toFieldPayload` copied it onto
* the wire shape, so the key was one reorder feature away from the hard 422
* `INVALID_METADATA` that blocks every later save of an object; it stayed
* latent only because `JSON.stringify` drops the `undefined`. That is
* objectui#4687's shape — a declaration with zero readers and zero writers —
* and the resolution is the same one: delete it, rather than leave a key
* declared here that no writer fills and no schema accepts.
*
* The object-level `sortOrder` on `ObjectDefinition` above is a DIFFERENT
* key on a different schema (objectui#6223 removed it from the object wire
* shape and deliberately kept it on that UI model, where it is the Object
* Manager's display order). So is the saved-view `sortOrder` in
* `app-shell`'s `ObjectView`. Neither is this one.
*/
/** Field description / help text */
description?: string;
/** Whether field is required */
Expand Down
Loading
Loading