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
44 changes: 44 additions & 0 deletions .changeset/6041-designer-reference-key.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
---
'@object-ui/plugin-designer': patch
'@object-ui/app-shell': patch
---

The field designer now reads and writes a lookup field's relationship target under the
spec's spelling `reference` (objectui#6041), in both directions.

`referenceTo` is not in `FieldSchema`'s accept set. Measured against the installed
`@objectstack/spec` 17.2.0, through the whole object document that
`PUT /api/v1/meta/object/:name` validates:

```
ObjectSchema.safeParse({ …, fields: { rel: { type: 'lookup', label: 'Owner',
referenceTo: 'user' } } })
=> success = false
=> unrecognized_keys at ["fields","rel"] keys=["referenceTo"]
"Did you mean `referenceTo` -> `reference`?"
```

so authoring a lookup field through the designer returned a hard 422 `INVALID_METADATA`,
and — because the key is then stored — blocked **every subsequent save** of that object,
with nothing in the UI to say which key did it.

The read direction was broken symmetrically and is the half that would have survived a
write-only fix: `toDesignerField` read `raw.referenceTo` while a spec-parsed server sends
`reference`, so every already-saved lookup field loaded into the designer with an **empty
reference box**. Both wire-bound payload shapes move — `FieldMetadataPayload`
(`MetadataService.toFieldPayload`) and `ServerFieldSchema`
(`MetadataFieldsPage.fromDesignerField`).

`referenceTo` also joins `RETIRED_FIELD_KEYS`. Renaming the emit sites alone does not
unblock an object whose stored fields already carry the misspelling: `carryOver` spreads
the previous server def verbatim, so the key would ride straight back out to the same 422.
The designer's in-memory `DesignerFieldDefinition` keeps `referenceTo` — that is the
internal prop name every other UI surface in this repo already uses (`LookupField`,
`filter-builder`, `ObjectChart`, `ListView`, `UserFilters`), it reaches no wire-bound
shape, and the parity gate classifies it as `uiOnly` rather than a violation.

No behavioural change for a half-filled draft: the spec's prose calls `reference`
"required for relationship types", but that is not enforced by the zod parse at 17.2.0 —
`{ type: 'lookup', label: 'L' }` parses green at field level and through `ObjectSchema`,
and `undefined` is dropped by `JSON.stringify` under either spelling, so the wire bytes
are identical before and after.
36 changes: 36 additions & 0 deletions .changeset/6044-designer-system-key.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
---
'@object-ui/plugin-designer': patch
---

The field designer now reads the system-field marker under the spec's spelling `system`,
and never hands `isSystem` back to the metadata API (objectui#6044).

`isSystem` is not in `FieldSchema`'s accept set. Measured against the installed
`@objectstack/spec` 17.2.0:

```
FieldSchema.safeParse({ type: 'text', label: 'L', isSystem: true })
=> success = false
=> unrecognized_keys keys=["isSystem"] "Did you mean `isSystem` -> `system`?"
```

Two defects, one misspelling, and they are two different sites.

**The read was dead** — the quieter and worse half. `toDesignerField` read `raw.isSystem`
while a spec-parsed server sends `system`, so the flag was always `undefined`. Nothing went
red, because the flag is optional and `undefined` is a valid "not a system field". But it is
load-bearing: `FieldDesigner` refuses to delete a system field and disables its name and
type inputs, so with the read dead `organization_id`, `created_at` and friends presented as
ordinary editable, **deletable** business fields.

**The write had no emit site at all.** `fromDesignerField` never names `isSystem`; its only
route out is the verbatim `...carryOver(prev)` spread, so a stored misspelling round-tripped
back to `PUT /api/v1/meta/object/:name` as a hard 422 `INVALID_METADATA` that blocks every
later save. The repair is a `RETIRED_FIELD_KEYS` tombstone rather than a renamed line — and
it is deliberately paired with the read fix, never a substitute for it: stripping alone would
close the 422 and fossilize the dead detection. The spec spelling `system` is not stripped,
so a server-injected flag rides through untouched and feeds the read.

`app-shell`'s `FieldMetadataPayload` never declared the key, so `toFieldPayload` had nothing
to fix. The designer's in-memory `DesignerFieldDefinition` keeps `isSystem`: it reaches no
wire-bound shape and the parity gate classifies it as `uiOnly`.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
/**
* 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#6041 — `MetadataService` writes the relationship target under the
* spec's spelling `reference`, never `referenceTo`.
*
* 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 two `wire` shapes: `toFieldPayload` builds it and
* `saveFields` PUTs `fields.map(toFieldPayload)` to
* `PUT /api/v1/meta/object/:name`.
*
* `referenceTo` is not in `FieldSchema`'s accept set. Measured against the
* installed `@objectstack/spec` 17.2.0, both at field level and through the
* whole object document:
*
* ObjectSchema.safeParse({ …, fields: { rel: { type: 'lookup', label: 'Owner',
* referenceTo: 'user' } } })
* => success = false
* => unrecognized_keys at ["fields","rel"] keys=["referenceTo"]
* "Did you mean `referenceTo` -> `reference`?"
*
* 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 until it is
* cleared by hand.
*
* ## Why the negative controls are the deliverable
*
* A green parity assertion proves nothing on its own: `FieldSchema` could be
* resolved to a look-alike or loosened to a passthrough and every positive
* assertion here would stay green while the 422 still happened server-side. So
* the instrument is asserted first, and each positive claim is paired with a
* control that must fail.
*
* Assertions are made on the bytes the SDK actually PUT — `JSON.parse` of the
* captured request body — not on the object handed to the client. That
* distinction is load-bearing for this key: a property whose value is
* `undefined` is a key that zod's strict object COUNTS but that
* `JSON.stringify` DROPS, so an in-memory assertion and a wire assertion
* disagree exactly on the half-filled draft this card had to measure.
*/

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, keyed by field name. */
function savedFields(puts: Array<Record<string, unknown>>): Record<string, unknown>[] {
const last = puts[puts.length - 1];
return last.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 LOOKUP: DesignerFieldDefinition = {
id: 'owner_id',
name: 'owner_id',
label: 'Owner',
type: 'lookup',
referenceTo: 'account',
};

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 `referenceTo` by name and accepts `reference` — the two states this file distinguishes', () => {
expect(unrecognizedKeys(FieldSchema.safeParse({ type: 'lookup', label: 'Owner', referenceTo: 'account' })))
.toEqual(['referenceTo']);
expect(FieldSchema.safeParse({ type: 'lookup', label: 'Owner', reference: 'account' }).success).toBe(true);
});
});

describe('objectui#6041 · saveFields PUTs the relationship target as `reference`', () => {
it('carries `reference` and no `referenceTo` on the wire', async () => {
const { adapter, puts } = makeCapturingAdapter();

await new MetadataService(adapter).saveFields('account', [LOOKUP]);

const [def] = savedFields(puts);
expect(def.reference).toBe('account');
expect('referenceTo' in def).toBe(false);
});

it('the PUT body parses through the real FieldSchema', async () => {
const { adapter, puts } = makeCapturingAdapter();

await new MetadataService(adapter).saveFields('account', [LOOKUP]);

const [def] = savedFields(puts);
const result = FieldSchema.safeParse(def);
expect(unrecognizedKeys(result)).toEqual([]);
expect(result.success).toBe(true);
// Falsification: the target actually made the trip. A payload that simply
// dropped the key would also parse green, and that is not the fix.
expect(def.reference).toBe('account');
});

it('a HALF-FILLED draft — type `lookup`, target left empty — still saves, exactly as before', async () => {
// The behavioural edge this card had to measure. The spec's prose calls
// `reference` "Required for relationship types", but that requirement is
// NOT enforced by the zod parse at 17.2.0: `{ type: 'lookup', label: 'L' }`
// parses green at field level AND through `ObjectSchema`. `undefined` is
// dropped by `JSON.stringify` under either spelling, so the wire bytes are
// byte-identical before and after this fix.
//
// ⚠ This case would still pass on a revert, and says so deliberately: it
// is here to prove the rename did NOT newly block a draft, which is a
// claim about the unchanged half.
const { adapter, puts } = makeCapturingAdapter();

await new MetadataService(adapter).saveFields('account', [{ ...LOOKUP, referenceTo: undefined }]);

const [def] = savedFields(puts);
expect('reference' in def).toBe(false);
expect('referenceTo' in def).toBe(false);
expect(FieldSchema.safeParse(def).success).toBe(true);
});
});
9 changes: 7 additions & 2 deletions packages/app-shell/src/services/MetadataService.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -62,7 +62,12 @@ export interface FieldMetadataPayload {
// `FieldSchema.safeParse` rejects the key by name, so writing it made
// `PUT /api/v1/meta/object/:name` fail with 422 `INVALID_METADATA`.
// Object-level `indexes[]` is the real surface.
referenceTo?: string;
// No `referenceTo` (objectui#6041): the spec spells the relationship
// target `reference`. `FieldSchema.safeParse` refuses `referenceTo` BY NAME
// ("Did you mean `referenceTo` -> `reference`?"), so a lookup field authored
// in the designer made `PUT /api/v1/meta/object/:name` fail 422
// `INVALID_METADATA` and blocked every later save of that object.
reference?: string;
formula?: string;
sortOrder?: number;
}
Expand DownExpand Up@@ -103,7 +108,7 @@ function toFieldPayload(field: DesignerFieldDefinition): FieldMetadataPayload {
options: field.options,
externalId: field.externalId,
trackHistory: field.trackHistory,
referenceTo: field.referenceTo,
reference: field.referenceTo,
formula: field.formula,
sortOrder: field.sortOrder,
};
Expand Down
Loading
Loading