Skip to content

Commit c36c1b2

Browse files
committed
Merge branch 'fix/w8-neutral' of https://github.com/simstudioai/sim into integrate/v2-w5
2 parents 4d1d130 + 9aaf865 commit c36c1b2

19 files changed

Lines changed: 456 additions & 190 deletions

File tree

apps/sim/app/api/v2/tables/[tableId]/rows/[rowId]/route.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ describe('/api/v2/tables/[tableId]/rows/[rowId]', () => {
131131
rowId: 'row-1',
132132
assertedWorkspaceId: WORKSPACE_ID,
133133
data: { name: 'Ada' },
134+
strictWrite: true,
134135
},
135136
request: req,
136137
})

apps/sim/app/api/v2/tables/[tableId]/rows/[rowId]/route.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ export const PATCH = defineV2JsonRoute({
4141
rowId: params.rowId,
4242
assertedWorkspaceId: body.workspaceId,
4343
data: body.data,
44+
strictWrite: true,
4445
}),
4546
useCase: updateTableRow,
4647
present: ({ table, row }) => ({

apps/sim/app/api/v2/tables/[tableId]/rows/route.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,10 @@ describe('/api/v2/tables/[tableId]/rows', () => {
154154
tableId: 'table-1',
155155
assertedWorkspaceId: WORKSPACE_ID,
156156
data: { name: 'Ada' },
157+
// v2 alone opts into the strict write contract: an unknown column name
158+
// or a value the column cannot hold is a 400, not a dropped key or a
159+
// nulled cell. Every first-party surface leaves this unset.
160+
strictWrite: true,
157161
},
158162
request: single,
159163
})
@@ -170,6 +174,7 @@ describe('/api/v2/tables/[tableId]/rows', () => {
170174
tableId: 'table-1',
171175
assertedWorkspaceId: WORKSPACE_ID,
172176
rows: [{ name: 'Ada' }],
177+
strictWrite: true,
173178
},
174179
request: batch,
175180
})

apps/sim/app/api/v2/tables/[tableId]/rows/route.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ export const POST = defineV2JsonRoute({
5454
tableId: params.tableId,
5555
assertedWorkspaceId: body.workspaceId,
5656
rows: body.rows,
57+
strictWrite: true,
5758
}
5859
: {
5960
kind: 'single' as const,
@@ -62,6 +63,7 @@ export const POST = defineV2JsonRoute({
6263
data: body.data,
6364
afterRowId: body.afterRowId,
6465
beforeRowId: body.beforeRowId,
66+
strictWrite: true,
6567
},
6668
useCase: createTableRows,
6769
present: (result) => {
@@ -89,6 +91,7 @@ export const PATCH = defineV2JsonRoute({
8991
filter: body.filter,
9092
data: body.data,
9193
limit: body.limit,
94+
strictWrite: true,
9295
}),
9396
useCase: updateTableRows,
9497
present: ({ affectedCount, affectedRowIds }) => ({

apps/sim/app/api/v2/tables/[tableId]/rows/upsert/route.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ describe('POST /api/v2/tables/[tableId]/rows/upsert', () => {
102102
assertedWorkspaceId: WORKSPACE_ID,
103103
data: { email: 'ada@example.com' },
104104
conflictTarget: 'email',
105+
strictWrite: true,
105106
},
106107
request,
107108
})

apps/sim/app/api/v2/tables/[tableId]/rows/upsert/route.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ export const POST = defineV2JsonRoute({
2020
assertedWorkspaceId: body.workspaceId,
2121
data: body.data,
2222
conflictTarget: body.conflictTarget,
23+
strictWrite: true,
2324
}),
2425
useCase: upsertTableRow,
2526
present: ({ table, row, operation }) => ({

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/utils.test.ts

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -156,21 +156,17 @@ describe('cleanCellValue', () => {
156156
expect(cleanCellValue('Bug, Docs', column)).toEqual(['opt_a', 'opt_b'])
157157
expect(cleanCellValue(['opt_b'], column)).toEqual(['opt_b'])
158158
expect(cleanCellValue('Bug, Bug', column)).toEqual(['opt_a'])
159-
// A part matching no option is refused rather than silently dropped: this helper
160-
// runs the same registry coercion the server does, and the server now rejects it.
161-
expect(cleanCellValue('Nope', column)).toBeNull()
162-
expect(cleanCellValue('Bug, Nope', column)).toBeNull()
159+
expect(cleanCellValue('Nope', column)).toEqual([])
163160
})
164161

165162
/**
166-
* The refusal above is `coerce`'s, not the last word the registry has on the
167-
* value: `salvage` reads the same paste as the one option that resolved. That
168-
* reading is reserved for writes with no caller to answer — a CSV row, a block
169-
* output — and a typed cell has one, so this helper must not reach for it. The
170-
* pairing is asserted rather than described so a future helper that "improves"
171-
* the paste by salvaging it fails here.
163+
* The grid writes through a first-party route, which runs the `null` policy —
164+
* a member the paste names that resolves to no option is dropped, and the ones
165+
* that do resolve are kept. Erasing the cell instead would lose two live
166+
* options over one deleted one. The registry pairing is asserted rather than
167+
* described so a helper that stops consulting `salvage` fails here.
172168
*/
173-
it('refuses a partial multiselect paste the registry could still salvage', () => {
169+
it('keeps the members of a partial multiselect paste that still resolve', () => {
174170
const column = {
175171
name: 'tags',
176172
type: 'select',
@@ -181,11 +177,12 @@ describe('cleanCellValue', () => {
181177
],
182178
} as const
183179

180+
expect(columnTypeOf(column).coerce('Bug, Nope', column)).toEqual({ ok: false })
184181
expect(columnTypeOf(column).salvage?.('Bug, Nope', column)).toEqual({
185182
ok: true,
186183
value: ['opt_a'],
187184
})
188-
expect(cleanCellValue('Bug, Nope', column)).toBeNull()
185+
expect(cleanCellValue('Bug, Nope', column)).toEqual(['opt_a'])
189186
})
190187
})
191188

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/utils.ts

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,14 @@ export function generateColumnName(columns: ReadonlyArray<{ name: string }>): st
2121
/**
2222
* Coerce a value a person typed or pasted into a cell to that column's type.
2323
* Throws on invalid JSON, and answers `null` for everything else the column
24-
* type refuses.
24+
* type can read nothing from.
2525
*
26-
* `null` is what the server would store for the same value, which is the point:
27-
* the optimistic cache and the row that comes back agree. It deliberately does
28-
* not consult `ColumnTypeDefinition.salvage`, which reads a refused value
29-
* lossily — a multiselect paste naming one option that no longer exists blanks
30-
* the cell here rather than storing the members that did resolve. Salvage is
31-
* reserved for writes with no caller to answer, and this one has one: a person
32-
* watching the cell, who is better served seeing the paste refused than seeing
33-
* part of it silently kept.
26+
* The result is what the server would store for the same value, which is the
27+
* point: the optimistic cache and the row that comes back agree. The grid
28+
* writes through a first-party route, which runs the `null` policy — so a
29+
* refused value falls back to `ColumnTypeDefinition.salvage` here exactly as it
30+
* does there, and a multiselect paste naming one live option and one deleted
31+
* one keeps the live one instead of erasing the cell.
3432
*/
3533
export function cleanCellValue(
3634
value: unknown,
@@ -56,8 +54,11 @@ export function cleanCellValue(
5654

5755
// Everything else runs the SAME coercion the server will run, so the
5856
// optimistic cache holds exactly the value that gets persisted.
59-
const coerced = columnTypeOf(column).coerce(value as JsonValue, column)
60-
return coerced.ok ? coerced.value : null
57+
const columnType = columnTypeOf(column)
58+
const coerced = columnType.coerce(value as JsonValue, column)
59+
if (coerced.ok) return coerced.value
60+
const salvaged = columnType.salvage?.(value as JsonValue, column)
61+
return salvaged?.ok ? salvaged.value : null
6162
}
6263

6364
/**

apps/sim/lib/table/__tests__/update-row.test.ts

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,14 +138,31 @@ describe('updateRow — partial merge', () => {
138138
expect(data?.values).not.toContain(JSON.stringify({ name: 'Alice', age: 31 }))
139139
})
140140

141-
it('holds only the patched keys to the strict policy when validating the merge', async () => {
141+
it('blanks an uncoercible cell for a first-party caller, as it always has', async () => {
142+
const { coerceRowToSchema } = await import('@/lib/table/validation')
143+
await updateRow(
144+
{ tableId: 'tbl-1', rowId: 'row-1', data: { age: 31 }, workspaceId: 'ws-1' },
145+
TABLE,
146+
'req-1'
147+
)
148+
149+
expect(coerceRowToSchema).toHaveBeenCalledWith(
150+
{ name: 'Alice', age: 31 },
151+
TABLE.schema,
152+
undefined,
153+
['age']
154+
)
155+
})
156+
157+
it('holds only the patched keys to the strict policy a v2 caller opts into', async () => {
142158
// The merged row carries cells this request never sent. A legacy value in one
143159
// of them belongs to an earlier write and must not decide this one.
144160
const { coerceRowToSchema } = await import('@/lib/table/validation')
145161
await updateRow(
146162
{ tableId: 'tbl-1', rowId: 'row-1', data: { age: 31 }, workspaceId: 'ws-1' },
147163
TABLE,
148-
'req-1'
164+
'req-1',
165+
{ uncoercibleValues: 'reject' }
149166
)
150167

151168
expect(coerceRowToSchema).toHaveBeenCalledWith(

apps/sim/lib/table/__tests__/validation.test.ts

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -358,15 +358,15 @@ describe('Validation', () => {
358358
expect(data.founded).toBe(1999)
359359
})
360360

361-
it('rejects an un-coercible value for an optional number column', () => {
361+
it('rejects an un-coercible value for an optional number column under `reject`', () => {
362362
const data = { name: 'Acme', founded: 2000, age: 'unknown' }
363-
const result = coerceRowToSchema(data, schema)
363+
const result = coerceRowToSchema(data, schema, 'reject')
364364
expect(result.valid).toBe(false)
365365
})
366366

367-
it('nulls an un-coercible optional value under the `null` policy', () => {
367+
it('nulls an un-coercible optional value by default', () => {
368368
const data = { name: 'Acme', founded: 2000, age: 'unknown' }
369-
const result = coerceRowToSchema(data, schema, 'null')
369+
const result = coerceRowToSchema(data, schema)
370370
expect(result.valid).toBe(true)
371371
expect(data.age).toBeNull()
372372
})
@@ -393,12 +393,20 @@ describe('Validation', () => {
393393
expect(data.active).toBe(false)
394394
})
395395

396-
it('refuses a bare epoch number, whose unit the value cannot state', () => {
396+
it('refuses a bare epoch number under `reject`, whose unit the value cannot state', () => {
397397
const data = { name: 'Acme', founded: 2000, created: Date.parse('2024-01-15T00:00:00Z') }
398-
const result = coerceRowToSchema(data, schema)
398+
const result = coerceRowToSchema(data, schema, 'reject')
399399
expect(result.valid).toBe(false)
400400
})
401401

402+
it('coerces an epoch number to an ISO date string by default', () => {
403+
const epoch = Date.parse('2024-01-15T00:00:00Z')
404+
const data = { name: 'Acme', founded: 2000, created: epoch }
405+
const result = coerceRowToSchema(data, schema)
406+
expect(result.valid).toBe(true)
407+
expect(data.created).toBe(new Date(epoch).toISOString())
408+
})
409+
402410
it('coerces a Date instance to an ISO date string', () => {
403411
const date = new Date('2024-01-15T00:00:00Z')
404412
const data = { name: 'Acme', founded: 2000, created: date }
@@ -407,16 +415,16 @@ describe('Validation', () => {
407415
expect(data.created).toBe(date.toISOString())
408416
})
409417

410-
it('nulls an out-of-range epoch number under the `null` policy without throwing', () => {
418+
it('nulls an out-of-range epoch number without throwing', () => {
411419
const data = { name: 'Acme', founded: 2000, created: 1e20 }
412-
const result = coerceRowToSchema(data, schema, 'null')
420+
const result = coerceRowToSchema(data, schema)
413421
expect(result.valid).toBe(true)
414422
expect(data.created).toBeNull()
415423
})
416424

417-
it('nulls an invalid Date instance under the `null` policy without throwing', () => {
425+
it('nulls an invalid Date instance without throwing', () => {
418426
const data = { name: 'Acme', founded: 2000, created: new Date('not-a-date') }
419-
const result = coerceRowToSchema(data, schema, 'null')
427+
const result = coerceRowToSchema(data, schema)
420428
expect(result.valid).toBe(true)
421429
expect(data.created).toBeNull()
422430
})
@@ -451,15 +459,15 @@ describe('Validation', () => {
451459
expect(patch.age).toBe(42)
452460
})
453461

454-
it('leaves an un-coercible optional patch value in place for downstream validation', () => {
462+
it('leaves an un-coercible optional patch value in place under `reject`', () => {
455463
const patch: { age: unknown } = { age: 'nope' }
456-
coerceRowValues(patch as never, schema)
464+
coerceRowValues(patch as never, schema, 'reject')
457465
expect(patch.age).toBe('nope')
458466
})
459467

460-
it('nulls an un-coercible optional patch value under the `null` policy', () => {
468+
it('nulls an un-coercible optional patch value by default', () => {
461469
const patch: { age: unknown } = { age: 'nope' }
462-
coerceRowValues(patch as never, schema, 'null')
470+
coerceRowValues(patch as never, schema)
463471
expect(patch.age).toBeNull()
464472
})
465473

@@ -533,15 +541,15 @@ describe('Validation', () => {
533541
expect(patch.price).toBe(42)
534542
})
535543

536-
it('leaves an unreadable amount in place on an optional column so validation reports it', () => {
544+
it('leaves an unreadable amount in place on an optional column under `reject`', () => {
537545
const patch: Record<string, unknown> = { price: 'ask sales' }
538-
coerceRowValues(patch as never, currencySchema)
546+
coerceRowValues(patch as never, currencySchema, 'reject')
539547
expect(patch.price).toBe('ask sales')
540548
})
541549

542-
it('nulls an unreadable amount on an optional column under the `null` policy', () => {
550+
it('nulls an unreadable amount on an optional column by default', () => {
543551
const patch: Record<string, unknown> = { price: 'ask sales' }
544-
coerceRowValues(patch as never, currencySchema, 'null')
552+
coerceRowValues(patch as never, currencySchema)
545553
expect(patch.price).toBeNull()
546554
})
547555

0 commit comments

Comments
 (0)