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
19 changes: 19 additions & 0 deletions .changeset/11692-writers-drop-served-options.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
'@object-ui/data-objectstack': minor
'@object-ui/app-shell': patch
'@object-ui/plugin-designer': patch
---

Objects with a picklist-bound field save again from the OWD overview, the Setup fields and objects pages, the metadata-admin embedded-item editor, and `MetadataService.saveFields` (objectui#11692).

The runtime serves a field that names a shared picklist (`picklist: 'NAME'`) with the options it resolved from the list. The authoring door refuses `options` beside `picklist`, with `422 INVALID_METADATA` at `fields.FIELD.options`, and the refusal covers the whole object. objectui#10202 fixed the two designers that host the select-field editor. These writers also seed their save from a served object read, so they sent the resolved options back and were refused, whatever had been edited:

- **Studio → Access → OWD overview.** Changing an object's sharing model and saving was refused.
- **The Setup fields page** (`MetadataFieldsPage`). Editing any field of such an object was refused.
- **The Setup objects page** (`MetadataObjectsPage`). Relabelling such an object was refused.
- **The metadata-admin embedded-item editor** (`EmbeddedItemEditor`, opened from an object's fields, indexes or validations). Saving any item of such an object was refused.
- **`MetadataService.saveFields`.** A field list built from the served object was refused.

Each now leaves `options` out of every field that names a picklist, before the PUT. A field without `picklist` keeps its inline `options` unchanged, and no other key is touched.

`dropServedPicklistOptions` is now exported from `@object-ui/data-objectstack`. It is the served-to-authored conversion these writers apply, and it moves here from `@object-ui/app-shell` (it was not exported there), because `@object-ui/plugin-designer` does not depend on `app-shell`. It is for code that builds an object PUT from a served read. `MetadataClient.save` does not apply it, and neither do `MetadataService.saveObject` and `MetadataService.saveMetadataItem`, which read nothing: a body that pairs `picklist` with `options` without a served read behind it is still refused by the server, with its prescription.
Original file line number Diff line number Diff 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#11692 — `MetadataService`'s three object writes, and a field that
* names a shared picklist.
*
* The runtime serves a picklist-bound field with `picklist` AND the options it
* resolved from the list; the authoring door refuses the two together for the
* whole object (`422 INVALID_METADATA` at `fields.FIELD.options`). The card's
* "Done when" asks each writer for one of two answers, and this file pins which
* one each method gives:
*
* - `saveFields` SEEDS from a served read (its GET, and the per-field
* carry-over that brings the binding back out), so it drops the served
* `options` on every bound field before its PUT.
* - `saveObject` and `saveMetadataItem` read NOTHING: every key they send is
* the caller's, so they cannot tell a served copy from an authored pair and
* must not strip — the authored pair stays the server's loud refusal, which
* is the authoring check objectui#10202 relies on. Pinned as "no read" plus
* "the pair still reaches the door and is refused".
*
* A real `ObjectStackAdapter` and SDK over a fetch double that serves the
* object and refuses a PUT body the way the authoring door does, so every
* claim is read off the WIRE.
*/

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

type Row = Record<string, unknown>;

const TIER_OPTIONS = [
{ label: 'Gold', value: 'gold' },
{ label: 'Platinum', value: 'platinum' },
];
const STATUS_OPTIONS = [
{ label: 'Open', value: 'open' },
{ label: 'Closed', value: 'closed' },
];

/** The object as the runtime SERVES it. */
const SERVED: Row = {
name: 'acme_account',
label: 'Account',
fields: {
tier: { type: 'select', label: 'Tier', picklist: 'acme_tier', options: TIER_OPTIONS },
status: { type: 'select', label: 'Status', options: STATUS_OPTIONS },
},
};

/** The authoring door's refusal for a field carrying both keys. */
function doorRefusal(body: Row): string[] {
return Object.entries((body.fields ?? {}) as Record<string, Row>)
.filter(([, def]) => def && def.picklist !== undefined && def.options !== undefined)
.map(([name]) => `fields.${name}.options`);
}

function json(body: unknown, status = 200): Response {
return new Response(JSON.stringify(body), { status, headers: { 'content-type': 'application/json' } });
}

/** A real adapter whose transport serves {@link SERVED} and refuses the pair. */
function makeAdapter() {
const puts: Array<{ body: Row; status: number }> = [];
let gets = 0;
const adapter = new ObjectStackAdapter({
baseUrl: 'http://test.local',
fetch: vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => {
const method = (init?.method ?? 'GET').toUpperCase();
if (method === 'PUT') {
const body = JSON.parse(String(init?.body ?? '{}')) as Row;
const refused = doorRefusal(body);
const status = refused.length > 0 ? 422 : 200;
puts.push({ body, status });
return status === 422
? json(
{
error: 'failed spec validation',
code: 'INVALID_METADATA',
issues: refused.map((path) => ({
path,
code: 'custom',
message: '`picklist` and `options` cannot both be declared. Keep `picklist` and delete `options`.',
})),
},
422,
)
: json({ success: true });
}
gets += 1;
return json({ item: SERVED });
}) as unknown as typeof fetch,
});
return { adapter, puts, getCount: () => gets };
}

describe('MetadataService — a picklist-bound field and the three object writes (objectui#11692)', () => {
it('saveFields seeds from a served read, so a field list built from that read and left untouched goes out accepted, the bound field without `options`', async () => {
const { adapter, puts, getCount } = makeAdapter();
// What a field designer holds after loading the served object: both fields,
// the bound one with the resolved options the read handed it.
const designerFields: DesignerFieldDefinition[] = [
{ id: 'tier', name: 'tier', label: 'Tier', type: 'select', options: TIER_OPTIONS },
{ id: 'status', name: 'status', label: 'Status', type: 'select', options: STATUS_OPTIONS },
];

await new MetadataService(adapter).saveFields('acme_account', designerFields);

expect(getCount(), 'the served read this writer seeds from').toBeGreaterThan(0);
expect(puts).toHaveLength(1);
const [put] = puts;
expect(put!.status, `refused at ${doorRefusal(put!.body).join(', ')}`).toBe(200);
const fields = put!.body.fields as Record<string, Row>;
// The binding the carry-over brought back stays; its served options do not.
expect(fields.tier).toMatchObject({ type: 'select', label: 'Tier', picklist: 'acme_tier' });
expect(fields.tier).not.toHaveProperty('options');
// A field that names no picklist keeps its options exactly.
expect(fields.status!.options).toEqual(STATUS_OPTIONS);
});

it('saveObject reads nothing, so it strips nothing: an authored pair reaches the door and comes back refused', async () => {
const { adapter, puts, getCount } = makeAdapter();
const obj: ObjectDefinition = { id: 'acme_account', name: 'acme_account', label: 'Account', isSystem: false, fieldCount: 1 };
// The binding is not a declared `FieldMetadataPayload` member, which is the
// point: only a caller holding a wider value can hand one in.
const authored = { name: 'tier', type: 'select', label: 'Tier', picklist: 'acme_tier', options: TIER_OPTIONS };

await expect(
new MetadataService(adapter).saveObject(obj, [authored as FieldMetadataPayload]),
).rejects.toMatchObject({ code: 'INVALID_METADATA', httpStatus: 422 });

expect(getCount(), 'saveObject issues no read').toBe(0);
expect(puts).toHaveLength(1);
expect((puts[0]!.body.fields as Record<string, Row>).tier).toMatchObject({ picklist: 'acme_tier', options: TIER_OPTIONS });
});

it('saveMetadataItem reads nothing, so it strips nothing: the pair it is handed reaches the door and comes back refused', async () => {
const { adapter, puts, getCount } = makeAdapter();

await expect(
new MetadataService(adapter).saveMetadataItem('object', 'acme_account', SERVED),
).rejects.toMatchObject({ code: 'INVALID_METADATA', httpStatus: 422 });

expect(getCount(), 'saveMetadataItem issues no read').toBe(0);
expect(puts).toHaveLength(1);
expect((puts[0]!.body.fields as Record<string, Row>).tier).toMatchObject({ picklist: 'acme_tier', options: TIER_OPTIONS });
});
});
35 changes: 34 additions & 1 deletion packages/app-shell/src/services/MetadataService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import { stripReadDecorations } from '@objectstack/spec/kernel';
import {
assertObjectMetadataWritable,
dropServedPicklistOptions,
RELATIONSHIP_TYPES_REQUIRING_REFERENCE,
viewItemObjectName,
type ObjectStackAdapter,
Expand Down Expand Up @@ -704,6 +705,14 @@ export class MetadataService {
* The object binding comes from the body being written, via the same
* accessor `listViewOverrides` narrows those rows by — not from a fourth
* private copy of "which object is this?".
*
* ⛔ No `dropServedPicklistOptions` here (objectui#11692). This method reads
* nothing: `data` is the caller's, so it cannot tell a served copy of a
* picklist-bound field from an author who wrote `picklist` and `options`
* together, and the second must stay the server's loud 422 with its
* prescription. A caller that seeded `data` from a served object read applies
* the conversion itself. Pinned (no read, the pair still refused) in
* `MetadataService.picklistServedOptions-11692.test.ts`.
*/
async saveMetadataItem(category: string, name: string, data: Record<string, unknown>): Promise<void> {
await this.putMetadataItem(category, name, data);
Expand Down Expand Up @@ -804,6 +813,14 @@ export class MetadataService {
* same way — but that builds capability for a path with zero measured
* pull and makes this parameter redundant, which then wants retiring on
* its own terms (ADR-0049 shape).
*
* ⛔ And, for the same reason it does not fetch, no `dropServedPicklistOptions`
* (objectui#11692): this method reads nothing, so every key it sends is one
* the caller handed it, and it cannot tell a served copy from an authored
* `picklist` + `options` pair — which must stay the server's loud 422.
* `FieldMetadataPayload` declares no `picklist`, so a fresh typed field
* literal cannot carry the binding here at all. Pinned (no read, the pair
* still refused) in `MetadataService.picklistServedOptions-11692.test.ts`.
*/
async saveObject(obj: ObjectDefinition, existingFields: FieldMetadataPayload[]): Promise<void> {
const payload = toObjectPayload(obj, existingFields);
Expand Down Expand Up @@ -894,6 +911,22 @@ export class MetadataService {
* previous entries ride in on the very document this method already
* fetched, so `toFieldPayload` merges onto them; see there for the mirror
* property that keeps a CLEARED designer property cleared.
*
* …and a fifth, pinned in `MetadataService.picklistServedOptions-11692.test.ts`:
*
* - **A picklist-bound field leaves without its served `options`**
* (objectui#11692). The fetched document is the SERVED one, so a field
* naming a shared picklist carries `picklist` beside the options the
* runtime resolved from the list, and the carry-over above brings the
* binding back out. The authoring door refuses `options` beside
* `picklist` for the WHOLE object, so the body passes through
* `dropServedPicklistOptions` before the PUT. The binding can only have
* come from that read: `DesignerFieldDefinition` declares no `picklist`,
* so this method has no way to author one and none to remove one. A
* bound field's `options` are therefore never half of an authored pair
* here; they are the served copy, or a designer-model value the door
* refuses beside the binding whatever it holds. Fields without a
* `picklist` keep their `options` exactly as before.
*/
async saveFields(objectName: string, fields: DesignerFieldDefinition[]): Promise<void> {
const client = this.adapter.getClient();
Expand Down Expand Up @@ -944,7 +977,7 @@ export class MetadataService {
fields: toFieldsMap(fields.map((field) => toFieldPayload(field, previousFieldEntry(previousFields, field.name)))),
}) as Record<string, unknown>;

await this.putMetadataItem('object', objectName, updatedObject);
await this.putMetadataItem('object', objectName, dropServedPicklistOptions(updatedObject));
this.adapter.invalidateCache(`object:${objectName}`);
}

Expand Down
5 changes: 4 additions & 1 deletion packages/app-shell/src/services/builtinComponents.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ import {
isAggregatedViewContainer,
viewDisplayType,
} from '../views/metadata-admin/view-item-normalize.js';
import { dropServedPicklistOptions } from '../views/metadata-admin/picklist-binding.js';
// objectui#11692 - the helper moved beside the metadata client, so the
// plugin-designer writers share it; `@object-ui/data-objectstack` is already on
// the console's eager path (its data source), so this adds no eager chunk.
import { dropServedPicklistOptions } from '@object-ui/data-objectstack';

/* -------------------------------------------------------------------------- */
/* 1) Top-level admin pages — bound to `metadata:directory` + `metadata:resource` */
Expand Down
Loading
Loading