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
59 changes: 59 additions & 0 deletions .changeset/6043-retire-designer-formula-control.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
---
'@object-ui/types': minor
'@object-ui/plugin-designer': minor
'@object-ui/app-shell': minor
---

The Field Designer no longer offers a formula-expression textarea, and no designer write
path emits a `formula` key (objectui#6043).

**This is a behaviour change on an authoring surface: a control is removed.** A field's
`type` may still be set to `formula` — that is a valid spec `FieldType` and stays in the
palette — but the expression itself is no longer authored here. Authors write formula
expressions in metadata-admin's field inspector, where they are checked.

The control wrote `formula`, which is not in `FieldSchema`'s accept set. Measured against
the installed `@objectstack/spec` 17.2.0:

```
FieldSchema.safeParse({ type:'formula', label:'Tax', formula:'price * quantity' })
=> success = false
=> unrecognized_keys ['formula'] "Did you mean `formula` -> `expression`?"
```

so `PUT /api/v1/meta/object/:name` returned a hard 422 `INVALID_METADATA` — and because
the key was then stored, it blocked **every later save of that object**, not just the one
that introduced it.

**The key was deliberately NOT renamed to the spec's `expression`.** `FieldSchema` judges
the key name and never the expression LANGUAGE — measured, it accepts
`expression: 'price * quantity'` and even `expression: '!!!not cel at all!!!'`; only the
empty string is refused. Spec `expression` is CEL rooted at `record`
(`record.amount * 0.1`), whereas this control's own placeholder taught `price * quantity`
— bare field refs, which under the scope formulas bind evaluate to null silently. A rename
would therefore have converted a loud, immediate 422 into a formula that saves clean and
then quietly computes nothing, which is strictly worse than the bug it appears to fix.

Making refusals loud *in the control* would need CEL lint, autocomplete and `returnType`
inference — that is `CelPredicateField`, which lives in `@object-ui/app-shell`, and
app-shell depends on `@object-ui/plugin-designer`, so it cannot be imported back without a
dependency cycle. Growing a second formula-authoring surface inside plugin-designer is a
feature, not this fix. `returnType` is likewise not authored here: it is only derivable by
inferring the CEL result type, and with no expression control there is nothing to infer
from.

`formula` joins the retired-key tombstone in `MetadataFieldsPage`, so an object already
carrying the key is stripped clean on its next save instead of staying blocked forever —
which matters more than usual here, because with the control gone an author would
otherwise have no way left to clear it. It is dropped rather than migrated to `expression`,
for the same reason the rename was refused. A `expression` authored in metadata-admin is
**not** touched: it is a real `FieldSchema` key and rides through the designer's
round-trip untouched.

Also removes the now-unreachable `formula` read/write from
`views/metadata-admin/previews/object-fields-bridge.ts`, which was a third emit site for
the key that neither the card nor the parity gate named.

The `formula` entry is removed from `check-designer-field-key-parity.mjs`'s
`KNOWN_UNPARSEABLE_KEYS` ledger, which ratchets in both directions — a resolved key that
left a stale entry behind would be as red as a new offender.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
/**
* 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#6043 — `MetadataService` never writes `formula`, and does not rename
* it to `expression` either.
*
* 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`.
*
* `formula` is not in `FieldSchema`'s accept set. Measured against the installed
* `@objectstack/spec` 17.2.0:
*
* FieldSchema.safeParse({ type:'formula', label:'Total', formula:'price * quantity' })
* => success = false
* => unrecognized_keys ['formula']
* "Did you mean `formula` -> `expression`?"
*
* 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.
*
* ## The rename this file proves was NOT taken
*
* The obvious repair — emit `expression` instead — was refused, and the control
* below is what keeps that decision legible. `FieldSchema` judges the KEY, never
* the expression LANGUAGE: it accepts `expression: '!!!not cel at all!!!'`.
* Spec `expression` is CEL rooted at `record`, whereas the designer control this
* card retired taught `price * quantity` — bare field refs that evaluate to null
* silently under the scope formulas bind. Renaming would therefore have replaced
* a loud 422 with a formula that saves clean and computes nothing.
*
* 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.
*
* This file names no `reference`/`referenceTo` key, so reverting objectui#6041
* cannot red it, and vice versa.
*/

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 FORMULA_FIELD: DesignerFieldDefinition = {
id: 'total',
name: 'total',
label: 'Total',
type: 'formula',
};

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 `formula` by name while accepting the field TYPE `formula`', () => {
expect(
unrecognizedKeys(FieldSchema.safeParse({ type: 'formula', label: 'Total', formula: 'price * quantity' })),
).toEqual(['formula']);
// Only the expression key was ever the defect. The type stays authorable.
expect(FieldSchema.safeParse({ type: 'formula', label: 'Total' }).success).toBe(true);
});

it('accepts `expression` without parsing the CEL in it — why the rename was refused', () => {
expect(FieldSchema.safeParse({ type: 'formula', label: 'T', expression: '!!!not cel at all!!!' }).success).toBe(
true,
);
});
});

describe('objectui#6043 · saveFields never PUTs a formula expression', () => {
it('emits neither `formula` nor `expression` for a formula field', async () => {
const { adapter, puts } = makeCapturingAdapter();

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

const [def] = savedFields(puts);
expect('formula' in def).toBe(false);
// Not renamed. `toFieldPayload` has no expression source to write and must
// not invent one — see this file's header for why that is the fix.
expect('expression' in def).toBe(false);
// Falsification: the field itself made the trip, so the two absences above
// are a payload builder that stopped copying the key, not an empty PUT.
expect(def.type).toBe('formula');
expect(def.label).toBe('Total');
});

it('drops a `formula` smuggled onto the field instead of copying it through', async () => {
// `DesignerFieldDefinition` no longer DECLARES `formula`, 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 a re-added designer control all arrive by exactly this route.
const { adapter, puts } = makeCapturingAdapter();
const smuggled = { ...FORMULA_FIELD, formula: 'price * quantity' } as DesignerFieldDefinition;

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

const [def] = savedFields(puts);
expect('formula' in def).toBe(false);
expect('expression' in def).toBe(false);
});

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

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

const [def] = savedFields(puts);
const result = FieldSchema.safeParse(def);
expect(unrecognizedKeys(result)).toEqual([]);
expect(result.success).toBe(true);
});
});
15 changes: 13 additions & 2 deletions packages/app-shell/src/services/MetadataService.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -79,7 +79,19 @@ export interface FieldMetadataPayload {
// 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;
// No `formula` (objectui#6043): the spec spells a formula field's expression
// `expression`, and it is CEL. `FieldSchema.safeParse` refuses `formula` BY
// NAME ("Did you mean `formula` -> `expression`?"), so a formula field
// authored in the designer made `PUT /api/v1/meta/object/:name` fail 422
// `INVALID_METADATA` and blocked every later save of that object.
//
// Deliberately NOT renamed to `expression`. `FieldSchema` validates the key
// but not the LANGUAGE: measured on 17.2.0 it accepts
// `expression: '!!!not cel at all!!!'`. Emitting the retired textarea's
// non-CEL contents under the accepted spelling would have turned a loud,
// 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;
}

Expand DownExpand Up@@ -126,7 +138,6 @@ function toFieldPayload(field: DesignerFieldDefinition): FieldMetadataPayload {
externalId: field.externalId,
trackHistory: field.trackHistory,
reference: field.referenceTo,
formula: field.formula,
sortOrder: field.sortOrder,
};
}
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -43,7 +43,9 @@ interface FrameworkFieldDef {
placeholder?: string;
options?: Array<{ label?: string; value: string; color?: string }>;
reference?: string;
formula?: string;
// No declared `formula` (objectui#6043) — nothing reads or writes it any
// more. A legacy def that still carries one reaches this type through the
// index signature below, and a QUARANTINED field round-trips verbatim.
group?: string;
[k: string]: unknown;
}
Expand DownExpand Up@@ -139,7 +141,6 @@ export function bridgeFromDraft(fieldsInput: unknown): FieldsBridgeResult {
}))
: undefined,
referenceTo: typeof def?.reference === 'string' ? def.reference : undefined,
formula: typeof def?.formula === 'string' ? def.formula : undefined,
});
}

Expand DownExpand Up@@ -207,6 +208,12 @@ function serializeDesignerField(f: DesignerFieldDefinition): FrameworkFieldDef {
if (f.group) out.group = f.group;
if (f.options && f.options.length > 0) out.options = f.options;
if (f.referenceTo) out.reference = f.referenceTo;
if (f.formula) out.formula = f.formula;
// No `formula` (objectui#6043). This bridge was a THIRD emit site for the
// key, named by neither the card nor `check-designer-field-key-parity.mjs` —
// `FrameworkFieldDef` is not one of that gate's declared `PAYLOAD_SHAPES`, so
// the gate was green over it. `FieldSchema` refuses `formula` BY NAME, and it
// is not renamed to `expression` here for the same reason as everywhere else
// on this card: the schema accepts the key without parsing the CEL, so a
// rename ships an unevaluatable expression under a valid name.
return out;
}
25 changes: 21 additions & 4 deletions packages/plugin-designer/src/FieldDesigner.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -219,7 +219,6 @@ export function FieldDesigner({
defaultValue: data.defaultValue ? String(data.defaultValue) : undefined,
placeholder: data.placeholder ? String(data.placeholder) : undefined,
referenceTo: data.referenceTo ? String(data.referenceTo) : undefined,
formula: data.formula ? String(data.formula) : undefined,
};
onFieldsChange?.(fields.map((f) => (f.id === editingField.id ? updated : f)));
} else {
Expand All@@ -240,7 +239,6 @@ export function FieldDesigner({
defaultValue: data.defaultValue ? String(data.defaultValue) : undefined,
placeholder: data.placeholder ? String(data.placeholder) : undefined,
referenceTo: data.referenceTo ? String(data.referenceTo) : undefined,
formula: data.formula ? String(data.formula) : undefined,
isSystem: false,
};
onFieldsChange?.([...fields, newField]);
Expand DownExpand Up@@ -309,7 +307,27 @@ export function FieldDesigner({
label: t('appDesigner.fieldDesigner.typeSpecificSection'),
fields: [
{ name: 'referenceTo', label: t('appDesigner.fieldDesigner.referenceTo'), type: 'text', placeholder: 'Referenced object', disabled: readOnly, visibleWhen: "record.type == 'lookup'" },
{ name: 'formula', label: t('appDesigner.fieldDesigner.formula'), type: 'textarea', placeholder: 'e.g. price * quantity', disabled: readOnly, visibleWhen: "record.type == 'formula'" },
// No `formula` control (objectui#6043). It wrote a key the spec
// refuses BY NAME — `FieldSchema` spells a formula field's expression
// `expression` — so typing in this box made
// `PUT /api/v1/meta/object/:name` fail 422 `INVALID_METADATA` and
// blocked every later save of the object.
//
// Renaming the key was considered and REFUSED. `FieldSchema` does not
// parse CEL at the key level (measured on 17.2.0: it accepts
// `expression: '!!!not cel at all!!!'`), and this control's own
// placeholder was `e.g. price * quantity` — bare field refs, which
// formulas' `record` scope evaluates to null silently. A rename would
// have traded a loud 422 for a silent wrong answer.
//
// Making it loud instead needs the CEL engine: lint, autocomplete and
// `returnType` inference. That is `CelPredicateField`, which lives in
// `@object-ui/app-shell` — and app-shell DEPENDS on this package, so
// it cannot be imported back without a cycle. Formula expressions are
// therefore authored in metadata-admin's `ObjectFieldInspector`,
// where they are checked against the real `@objectstack/formula`
// engine. The field TYPE `formula` is a valid spec `FieldType` and
// stays in the palette above; only this unvalidatable box is gone.
{ name: 'defaultValue', label: t('appDesigner.fieldDesigner.defaultValue'), type: 'text', placeholder: 'Default value', disabled: readOnly },
{ name: 'placeholder', label: t('appDesigner.fieldDesigner.placeholder'), type: 'text', placeholder: 'Placeholder text', disabled: readOnly },
{ name: 'group', label: t('appDesigner.fieldDesigner.fieldGroup'), type: 'text', placeholder: 'Field Group', disabled: readOnly },
Expand DownExpand Up@@ -348,7 +366,6 @@ export function FieldDesigner({
defaultValue: editingField.defaultValue != null ? String(editingField.defaultValue) : '',
placeholder: editingField.placeholder || '',
referenceTo: editingField.referenceTo || '',
formula: editingField.formula || '',
}
: { type: 'text', required: false, unique: false },
onSuccess: handleFormSuccess,
Expand Down
Loading
Loading