Skip to content

Commit 7bce934

Browse files
fix(tables): validate a conversion against the required flag the request sets
A single PATCH converting an optional column to select while also setting required: true checked blanks against the column's CURRENT required flag, so the conversion committed and the constraint write then failed — an error response with the type change already persisted. updateColumnType now takes the requested `required` and validates against the constraint the column ends up with. Empty rows are also counted when the target is required (they were skipped outright), so the null case fails up-front too rather than at the constraint write.
1 parent 7c0c24d commit 7bce934

4 files changed

Lines changed: 32 additions & 3 deletions

File tree

apps/sim/app/api/table/[tableId]/columns/route.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,9 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu
144144
newType: updates.type as NonNullable<typeof updates.type>,
145145
...(updates.options !== undefined ? { options: updates.options } : {}),
146146
...(updates.multiple !== undefined ? { multiple: updates.multiple } : {}),
147+
// Forwarded so the conversion validates against the constraint this
148+
// same request is about to set, not the column's current one.
149+
...(updates.required !== undefined ? { required: updates.required } : {}),
147150
},
148151
requestId
149152
)

apps/sim/app/api/v1/tables/[tableId]/columns/route.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,9 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu
173173
newType: updates.type as NonNullable<typeof updates.type>,
174174
...(updates.options !== undefined ? { options: updates.options } : {}),
175175
...(updates.multiple !== undefined ? { multiple: updates.multiple } : {}),
176+
// Forwarded so the conversion validates against the constraint this
177+
// same request is about to set, not the column's current one.
178+
...(updates.required !== undefined ? { required: updates.required } : {}),
176179
},
177180
requestId
178181
)

apps/sim/lib/table/columns/service.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -532,12 +532,21 @@ export async function updateColumnType(
532532
// once the column is text/number/etc. Check compatibility against the option
533533
// NAME — that's what the cell will actually become (migrated below).
534534
const convertingAwayFromSelect = column.type === 'select' && !isSelectType
535+
// The constraint the column ends up with, which may be arriving in this same
536+
// request. `updateColumnConstraints` runs as its own transaction afterwards,
537+
// so validating against the column's CURRENT flag would let the conversion
538+
// commit and only then fail the constraint.
539+
const targetRequired = !!(data.required ?? column.required)
535540

536541
let incompatibleCount = 0
542+
let blankCount = 0
537543
for (const row of rows) {
538544
const rowData = row.data as RowData
539545
const value = rowData[columnKey]
540-
if (value === null || value === undefined) continue
546+
if (value === null || value === undefined) {
547+
if (targetRequired) blankCount++
548+
continue
549+
}
541550

542551
const effective = convertingAwayFromSelect ? selectValueForConversion(column, value) : value
543552

@@ -547,13 +556,20 @@ export async function updateColumnType(
547556
data.newType,
548557
targetOptions,
549558
!!targetMultiple,
550-
!!column.required
559+
targetRequired
551560
)
552561
) {
553-
incompatibleCount++
562+
if (effective === null || effective === '') blankCount++
563+
else incompatibleCount++
554564
}
555565
}
556566

567+
if (blankCount > 0) {
568+
throw new Error(
569+
`Cannot change column "${column.name}" to a required "${data.newType}": ${blankCount} row(s) are empty. Fill them first, or apply the type change without making the column required.`
570+
)
571+
}
572+
557573
if (incompatibleCount > 0) {
558574
throw new Error(
559575
`Cannot change column "${column.name}" to type "${data.newType}": ${incompatibleCount} row(s) have incompatible values. Fix or remove the incompatible values first.`

apps/sim/lib/table/types.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -664,6 +664,13 @@ export interface UpdateColumnTypeData {
664664
options?: SelectOption[]
665665
/** Whether the `select` column accepts multiple options per cell. */
666666
multiple?: boolean
667+
/**
668+
* The `required` value the same request is about to set, when it changes type
669+
* and constraints together. Those are separate transactions, so the
670+
* conversion has to validate against the constraint the column will END UP
671+
* with — otherwise it commits and the constraint write then fails.
672+
*/
673+
required?: boolean
667674
}
668675

669676
export interface UpdateColumnOptionsData {

0 commit comments

Comments
 (0)