diff --git a/.changeset/29200-clone-empty-readonly-not-dropped.md b/.changeset/29200-clone-empty-readonly-not-dropped.md new file mode 100644 index 00000000000..5427a889630 --- /dev/null +++ b/.changeset/29200-clone-empty-readonly-not-dropped.md @@ -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. diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 8c383a7cc58..3ff88bdc6ab 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -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') { diff --git a/packages/qa/dogfood/test/showcase-static-readonly.dogfood.test.ts b/packages/qa/dogfood/test/showcase-static-readonly.dogfood.test.ts index 19a9eae2bbc..807f1117057 100644 --- a/packages/qa/dogfood/test/showcase-static-readonly.dogfood.test.ts +++ b/packages/qa/dogfood/test/showcase-static-readonly.dogfood.test.ts @@ -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 () => { diff --git a/packages/rest/src/rest-dropped-fields-every-write-door.test.ts b/packages/rest/src/rest-dropped-fields-every-write-door.test.ts index c8baefb2e94..be197e31fdf 100644 --- a/packages/rest/src/rest-dropped-fields-every-write-door.test.ts +++ b/packages/rest/src/rest-dropped-fields-every-write-door.test.ts @@ -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 = { @@ -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); @@ -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' });