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
15 changes: 15 additions & 0 deletions .changeset/29200-clone-empty-readonly-not-dropped.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
'@objectstack/metadata-protocol': patch
---

fix(metadata-protocol): a plain clone no longer reports the source's empty read-only columns as dropped fields

Clause-②: no
Metadata-protocol: none

`cloneData` copied every column of the source into the insert, read-only ones included, even where the source held no value. The engine's read-only strip judged each such column a caller-supplied key and reported it, so a clone with no `overrides` of a row whose read-only column was empty answered `droppedFields` naming that column, and the REST `X-ObjectStack-Dropped-Fields` header repeated it. Nothing had been sent and nothing was lost, but the console announced "fields dropped" on every such clone.

The clone now leaves a read-only column whose source value is empty (`null`) out of the copy. The stored row is the same as before: the strip removed that key from a non-system caller anyway, and the insert's defaults treat an explicit `null` exactly as an omitted key. What still reports:

- a read-only column the source holds a VALUE in is still copied, stripped and reported, body and header, so the caller reading the default back is told why;
- a read-only field set through `overrides` is still stripped and reported, whatever the source held.
12 changes: 12 additions & 0 deletions packages/metadata-protocol/src/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14665,7 +14665,19 @@ export class ObjectStackProtocolImplementation implements
|| (def as any).type === 'formula'
|| (def as any).type === 'summary') {
delete data[name];
continue;
}
// [#29200] A read-only column the source left EMPTY is not carried
// into the insert. Carried, it reached `engine.insert`'s strip as a
// present key and was reported in `droppedFields` (and the
// `X-ObjectStack-Dropped-Fields` header) although neither the caller
// nor the source supplied a value. Leaving it out stores the same
// row: the strip would have taken it from a non-system caller, and
// the insert's defaults treat an explicit `null` exactly as an
// omitted key. A read-only column that HOLDS a value is still
// carried, stripped and reported (#15703), and so is one the caller
// sets through `overrides`, which are applied below.
if ((def as any).readonly && data[name] == null) delete data[name];
}
// Caller overrides win (new name, cleared unique field, reset status…).
if (request.overrides && typeof request.overrides === 'object') {
Expand Down
16 changes: 13 additions & 3 deletions packages/qa/dogfood/test/showcase-static-readonly.dogfood.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,19 @@ describe('showcase: clone and createMany advertise the readonly strip in the hea
expect(res.status).toBe(201);
const body = await res.json();
expect(namesReadonlyLeadScore(body), JSON.stringify(body?.droppedFields)).toBe(true);
// A plain clone copies the source's other readonly columns too, so the
// header may name more than lead_score; it names what the body names.
expect(res.headers.get('X-ObjectStack-Dropped-Fields') ?? '').toContain('lead_score;reason=readonly');
// The source's readonly columns are all empty, so the clone carries none of
// them into the insert (#29200): the header names lead_score and nothing else.
expect(res.headers.get('X-ObjectStack-Dropped-Fields')).toBe('lead_score;reason=readonly');
});

// #29200: the source's lead_score is empty and the caller sent none, so a
// plain clone strips nothing and must not announce a dropped field.
it('a plain clone of a source whose lead_score is empty: no droppedFields, no header', async () => {
const res = await stack.apiAs(admin, 'POST', `${OBJ}/${sourceId}/clone`, {});
const body = await res.json();
expect(res.status, JSON.stringify(body)).toBe(201);
expect(body?.droppedFields, JSON.stringify(body?.droppedFields)).toBeUndefined();
expect(res.headers.get('X-ObjectStack-Dropped-Fields')).toBeNull();
});

it('a two-row createMany carrying lead_score: body and header name lead_score once', async () => {
Expand Down
41 changes: 38 additions & 3 deletions packages/rest/src/rest-dropped-fields-every-write-door.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,10 @@ const PROJ = {
},
};

// Clone copies every column of the source it does not reset, readonly ones
// included, so a clone of a PROJ row also reports the source's `ro2`. Clone is
// read on its own objects: one readonly column, and none.
// Clone copies the source's readonly columns that hold a value (#15703: the
// strip of a copied value is reported), so a clone of a PROJ row would also
// report any `ro2` the source carried. Clone is read on its own objects: one
// readonly column, and none.
const CLONE_OBJECT = 'hdr_drops_clone';
const PLAIN_OBJECT = 'hdr_drops_plain';
const CLONE_PROJ = {
Expand Down Expand Up @@ -116,6 +117,8 @@ async function boot() {
// `readonlyWhen` already true; seeded under `isSystem`.
await engine.insert(OBJECT, { id: 'closed1', title: 'closed', status: 'closed', locked: 'L' } as any, { context: { isSystem: true } } as any);
await engine.insert(CLONE_OBJECT, { id: 'src1', title: 'source', ro: 'seeded' } as any, { context: { isSystem: true } } as any);
// #29200: the same object, its readonly column left empty (null).
await engine.insert(CLONE_OBJECT, { id: 'src_empty', title: 'empty source' } as any);
await engine.insert(PLAIN_OBJECT, { id: 'plain1', title: 'source' } as any);

const protocol = new ObjectStackProtocolImplementation(engine as any);
Expand Down Expand Up @@ -147,6 +150,38 @@ describe('clone: the header names what the body reports', () => {
expect(r.header).toBe('ro;reason=readonly');
});

// #29200: the caller sent no readonly key and the copy carried none with a
// value, so the engine stripped nothing the caller or the source supplied.
it('a plain clone of a row whose readonly column is empty: no droppedFields, no header', async () => {
const b = await boot();
const r = await b.call('/api/v1/data/:object/:id/clone', {}, { object: CLONE_OBJECT, id: 'src_empty' });
expect(r.status, JSON.stringify(r.body)).toBe(201);
expect(r.body).not.toHaveProperty('droppedFields');
expect(r.header).toBeUndefined();
expect(r.body.record.ro ?? null).toBeNull();
expect(r.body.record.title).toBe('empty source');
});

it('overrides setting the readonly field on that empty source: still reported, body and header', async () => {
const b = await boot();
const r = await b.call('/api/v1/data/:object/:id/clone', { overrides: { ro: 'forged' } }, { object: CLONE_OBJECT, id: 'src_empty' });
expect(r.status, JSON.stringify(r.body)).toBe(201);
expect(r.body.droppedFields).toEqual([{ object: CLONE_OBJECT, fields: ['ro'], reason: 'readonly' }]);
expect(r.header).toBe('ro;reason=readonly');
expect(r.body.record.ro ?? null).toBeNull();
});

// #15703 ruled that a clone REPORTS the readonly value it copied and the
// engine stripped: the caller reads the default back and is told why.
it('a plain clone of a row whose readonly column holds a value: still reported, body and header', async () => {
const b = await boot();
const r = await b.call('/api/v1/data/:object/:id/clone', {}, { object: CLONE_OBJECT, id: 'src1' });
expect(r.status, JSON.stringify(r.body)).toBe(201);
expect(r.body.droppedFields).toEqual([{ object: CLONE_OBJECT, fields: ['ro'], reason: 'readonly' }]);
expect(r.header).toBe('ro;reason=readonly');
expect(r.body.record.ro ?? null).toBeNull();
});

it('a clone that strips nothing: no droppedFields, no header (the control)', async () => {
const b = await boot();
const r = await b.call('/api/v1/data/:object/:id/clone', { overrides: { title: 'plain copy' } }, { object: PLAIN_OBJECT, id: 'plain1' });
Expand Down
Loading