diff --git a/.changeset/6045-field-payload-sort-order.md b/.changeset/6045-field-payload-sort-order.md new file mode 100644 index 000000000..c18dc4bd9 --- /dev/null +++ b/.changeset/6045-field-payload-sort-order.md @@ -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. diff --git a/packages/app-shell/src/services/MetadataService.retiredFieldSortOrder.test.ts b/packages/app-shell/src/services/MetadataService.retiredFieldSortOrder.test.ts new file mode 100644 index 000000000..35abb69ed --- /dev/null +++ b/packages/app-shell/src/services/MetadataService.retiredFieldSortOrder.test.ts @@ -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> = []; + 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); + } + 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[] { + return puts[puts.length - 1].fields as Record[]; +} + +const unrecognizedKeys = (result: ReturnType): 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)); + // 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([]); + }); +}); diff --git a/packages/app-shell/src/services/MetadataService.specKeyObjectPayload.test.ts b/packages/app-shell/src/services/MetadataService.specKeyObjectPayload.test.ts index 2c698ea2b..1adda2ff1 100644 --- a/packages/app-shell/src/services/MetadataService.specKeyObjectPayload.test.ts +++ b/packages/app-shell/src/services/MetadataService.specKeyObjectPayload.test.ts @@ -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[]; - 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`. }); }); diff --git a/packages/app-shell/src/services/MetadataService.ts b/packages/app-shell/src/services/MetadataService.ts index f49ce1f23..19a8a1d46 100644 --- a/packages/app-shell/src/services/MetadataService.ts +++ b/packages/app-shell/src/services/MetadataService.ts @@ -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 @@ -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.) } // --------------------------------------------------------------------------- @@ -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, @@ -138,7 +155,6 @@ function toFieldPayload(field: DesignerFieldDefinition): FieldMetadataPayload { externalId: field.externalId, trackHistory: field.trackHistory, reference: field.referenceTo, - sortOrder: field.sortOrder, }; } diff --git a/packages/types/src/designer.ts b/packages/types/src/designer.ts index 3028e201f..60ed20ed8 100644 --- a/packages/types/src/designer.ts +++ b/packages/types/src/designer.ts @@ -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 */ diff --git a/scripts/check-designer-field-key-parity.mjs b/scripts/check-designer-field-key-parity.mjs index aac8644ac..5409469c5 100644 --- a/scripts/check-designer-field-key-parity.mjs +++ b/scripts/check-designer-field-key-parity.mjs @@ -260,17 +260,22 @@ export const KNOWN_UNPARSEABLE_KEYS = { // placeholder — bare field refs that evaluate to null under the `record` // scope — under a valid key name. The control was removed instead; the field // TYPE `formula` is unaffected and remains a valid spec `FieldType`. - sortOrder: { - card: "objectui#6045", - // Scoped to the FIELD oracle deliberately. `sortOrder` is refused at BOTH - // levels and the two are different cards with different resolutions - // (objectui#6223 removed the object-level one). An unscoped entry would let - // this card's entry absorb an object-level reappearance in silence, which - // is the ledger becoming the hiding place the header says it must not be. - oracle: "FieldSchema", - spec: null, // the spec has `sortable` (a boolean), and no field-level ordering key - note: "Latent: declared and written by `toFieldPayload`, but nothing populates it, so JSON drops the undefined. One reorder feature away from live.", - }, + // objectui#6045 `sortOrder` (FIELD level) was resolved and its entry removed. + // The resolution was objectui#4687's — delete the declaration — because the + // key had zero readers and zero writers: `toFieldPayload` copied it, nothing + // ever populated it, and `JSON.stringify` dropped the `undefined`. The `spec` + // column recorded no equivalent and there was none to take: `sortable` is a + // boolean ("whether field is sortable in list views"), and the spec models + // field order by DECLARATION ORDER in the object's `fields` record rather + // than by an index on the field. It was dropped from the wire shape + // (`FieldMetadataPayload`), from its writer, and from the UI model + // (`DesignerFieldDefinition`) in one go. + // + // The FIELD entry going away does not touch the OBJECT level: that spelling + // is still refused by `ObjectSchema` and still declared on `ObjectDefinition` + // (objectui#6223 kept it there as the Object Manager's display order), where + // the gate reports it as `uiOnly`. That is why this entry was oracle-scoped: + // removing it must not, and does not, quiet the other level. enabled: { card: "objectui#6238", oracle: "ObjectSchema",