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
13 changes: 13 additions & 0 deletions .changeset/20922-per-row-dropped-fields.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
'@objectstack/objectql': minor
'@objectstack/metadata-protocol': minor
---

The dry run and the partial-success batch insert now say which row lost which field. `ObjectQL.validate` (and `validateData`, which relays it) answers `droppedFields` on each accepted row of `results`, and `ObjectQL.insertMany` (and `insertManyData`, which passes it through) answers `droppedFields` on each `ok` outcome: the caller-supplied fields the engine legally strips from that row, one `DroppedFieldsEvent` per reason, in the engine's own reason vocabulary (`computed` for a `formula` value, `readonly` for a static `readonly` or runtime-owned field). The key is absent when nothing was taken from the row.

- **Recorded at the strips, never inferred from the union.** Each strip records what it takes from each row as it runs. A `beforeInsert` hook that assigns a protected key on one row keeps it there, so that row is not named, while a sibling row that supplied the same key and lost it is.
- **A row the write does not complete carries none.** A preview row the verdict refuses, and an `ok: false` outcome, carry no `droppedFields`: a drop means the write completed without the field.
- **The dry run and the commit agree.** On `insert` mode the preview runs the same strips the write runs, so a row's preview drops and its outcome drops are the same list. One gap is unchanged: the preview runs no hooks, so a key a `beforeInsert` hook assigns is reported by the preview and kept by the write. An `update`-mode preview does not run the `readonlyWhen` or primary-key strips, which judge a prior record the preview does not read.
- **Unchanged:** the `onFieldsDropped` listener on `insert`, `insertMany` and `validate` still reports the batch-level union, one event per reason, naming no row. So does `insertManyData`'s top-level `droppedFields`. `insert(object, rows[])` still returns the records, with no per-row slot. `strictReadonlyWrites` still refuses the whole batch before any outcome is built.

Graded `minor` in both packages: each widens a published method's declared answer with a new optional key (`InsertManyRowOutcome` gains `droppedFields`, and so does each outcome of `insertManyData`'s return type), which is an additive widening of the public surface. Nothing is removed, renamed or refused. The keys on the wire, `ValidateDataResponseSchema.results[].droppedFields` and `ImportRowResultSchema.droppedFields`, were already declared in `@objectstack/spec`. The REST import route does not copy the per-row report onto its row results yet.
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,11 @@ describe('insertManyData — BATCH-LEVEL droppedFields that names no row (#3455)
// so the cases are REPLACED rather than amended. The engine double below
// models the exemption, which the old one had no concept of; that is why the
// old case stayed green through exactly the shape it was written for.
//
// [#20922] Row precision now comes from the ENGINE, on each `ok` outcome's
// own `droppedFields` (pinned against the real engine in objectql's
// `engine-per-row-dropped-fields.test.ts`). The double below attributes no
// row, so these cases pin what THIS seam adds: nothing derived from the union.

/**
* `hookStamps` names the rows whose `beforeInsert` hook re-assigns
Expand Down Expand Up @@ -250,6 +255,29 @@ describe('insertManyData — BATCH-LEVEL droppedFields that names no row (#3455)
});
expect(res).not.toHaveProperty('droppedFields');
});

it('[#20922] an outcome the ENGINE attributed passes through as answered, beside the unchanged union', async () => {
// The per-row channel is the engine's (`InsertManyRowOutcome.droppedFields`,
// recorded at its strips). This seam relays it and derives nothing: the
// cases above, whose engine attributes no row, still get none.
const rowDrop = { object: 'approval_case', fields: ['approval_status'], reason: 'readonly' as const };
const insertMany = vi.fn(async (object: string, rows: any[], options?: any) => {
options?.onFieldsDropped?.({ object, fields: ['approval_status'], reason: 'readonly' });
return rows.map((r, i) => (i === 1
? { ok: true, record: { id: 'rec-2', title: r.title, approval_status: 'draft' }, droppedFields: [rowDrop] }
: { ok: true, record: { id: 'rec-1', title: r.title, approval_status: 'draft' } }));
});
const p = makeProtocol(insertMany as any);

const res: any = await p.insertManyData({
object: 'approval_case',
records: [{ title: 'A' }, { title: 'B', approval_status: 'approved' }],
});

expect(res.outcomes[0]).not.toHaveProperty('droppedFields');
expect(res.outcomes[1].droppedFields).toEqual([rowDrop]);
expect(res.droppedFields).toEqual([rowDrop]);
});
});

describe('batchData — per-row droppedFields + context threading (#3455)', () => {
Expand Down
15 changes: 13 additions & 2 deletions packages/metadata-protocol/src/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11842,6 +11842,12 @@ export class ObjectStackProtocolImplementation implements
* Same object-existence gate as every other data entry point (#3770), so
* an unknown object fails the same way here as it would on the real write
* — a preview that 404s differently from its write is a mirror again.
*
* [#20922] The per-row drops ride the same verdict: `engine.validate`
* answers each accepted row's `droppedFields` from its own strips, so this
* relays them as it relays `errors` / `warnings`. No listener is passed
* here: the engine's `onFieldsDropped` events are the batch-level union,
* and this response has no batch-level slot to put them in.
*/
async validateData(request: { object: string, data: any, mode?: 'insert' | 'update', context?: any }) {
this.assertObjectRegistered(request.object);
Expand Down Expand Up @@ -13615,7 +13621,7 @@ export class ObjectStackProtocolImplementation implements
* channel (an `onFieldsDropped` signature that carries the row), never a
* reconstruction at this call site.
*/
async insertManyData(request: { object: string, records: any[], context?: any }): Promise<{ object: string; outcomes: Array<{ ok: boolean; record?: any; error?: unknown }>; droppedFields?: DroppedFieldsEvent[] }> {
async insertManyData(request: { object: string, records: any[], context?: any }): Promise<{ object: string; outcomes: Array<{ ok: boolean; record?: any; error?: unknown; droppedFields?: DroppedFieldsEvent[] }>; droppedFields?: DroppedFieldsEvent[] }> {
this.assertObjectRegistered(request.object); // [#3770]
const engineInsertMany = (this.engine as any)?.insertMany;
if (typeof engineInsertMany !== 'function') {
Expand All @@ -13629,7 +13635,12 @@ export class ObjectStackProtocolImplementation implements
const dropped: DroppedFieldsEvent[] = [];
const opts: any = { onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); } };
if (request.context !== undefined) opts.context = request.context;
const outcomes: Array<{ ok: boolean; record?: any; error?: unknown }> = await engineInsertMany.call(
// [#20922] Each `ok` outcome carries the ENGINE's per-row report
// (`outcomes[i].droppedFields`), recorded at its strips — a separate
// channel from the batch-level union below, which still names no row.
// Passed through as the engine answers it; ⛔ nothing here derives a
// row's drops from the union.
const outcomes: Array<{ ok: boolean; record?: any; error?: unknown; droppedFields?: DroppedFieldsEvent[] }> = await engineInsertMany.call(
this.engine,
request.object,
request.records,
Expand Down
21 changes: 12 additions & 9 deletions packages/objectql/src/engine-autonumber-runtime-owned.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,15 +305,15 @@ describe('#5503 — autonumber is runtime-owned: bulk-create surfaces', () => {
expect((res.droppedFields ?? []).flatMap((e: DroppedFieldsEvent) => e.fields)).toContain('account_number');
});

it('insertManyData reports the union at BATCH level and names no row', async () => {
it('insertManyData reports the union at BATCH level, and the row that lost the value on its own outcome', async () => {
// The import runner prefers this partial-success surface, so it is the one
// that has to stay honest — and honest here means naming no row: the
// engine's event carries no row index, and the two facts that would let a
// caller resolve it are both unavailable at the protocol seam. The row
// records below are the second one: BOTH come back carrying
// `account_number`, because the strip is followed by `applyAutonumbers`.
// So "is the key still on the row?" answers the same for the row that was
// stripped and the row that was not.
// that has to stay honest. The batch-level union names no row: the
// engine's event carries no row index. Nor can the row records resolve it:
// BOTH come back carrying `account_number`, because the strip is followed
// by `applyAutonumbers`, so "is the key still on the row?" answers the
// same for the row that was stripped and the row that was not.
// [#20922] Row precision comes from the ENGINE instead: the strip records
// what it took per row, and the row's `ok` outcome carries it.
const rig = await makeEngine();
const res: any = await rig.protocol.insertManyData({
object: 'an_account',
Expand All @@ -323,7 +323,10 @@ describe('#5503 — autonumber is runtime-owned: bulk-create surfaces', () => {
],
});
expect(res.outcomes.map((o: any) => o.record.account_number)).toEqual(['ACC-0001', 'ACC-0002']);
for (const o of res.outcomes) expect(o).not.toHaveProperty('droppedFields');
expect(res.outcomes[0]).not.toHaveProperty('droppedFields');
expect(res.outcomes[1].droppedFields).toEqual([
{ object: 'an_account', fields: ['account_number'], reason: 'readonly' },
]);
expect((res.droppedFields ?? []).flatMap((e: DroppedFieldsEvent) => e.fields)).toEqual(['account_number']);
});
});
Expand Down
Loading
Loading