diff --git a/packages/server/src/api/controllers/table/index.ts b/packages/server/src/api/controllers/table/index.ts index eae5204c7e..3148e54af7 100644 --- a/packages/server/src/api/controllers/table/index.ts +++ b/packages/server/src/api/controllers/table/index.ts @@ -10,7 +10,7 @@ import { isExternalTableID, isSQL, } from "../../../integrations/utils" -import { events } from "@budibase/backend-core" +import { events, HTTPError } from "@budibase/backend-core" import { BulkImportRequest, BulkImportResponse, @@ -29,6 +29,7 @@ import sdk from "../../../sdk" import { jsonFromCsvString } from "../../../utilities/csv" import { builderSocket } from "../../../websockets" import { cloneDeep, isEqual } from "lodash" +import { helpers } from "@budibase/shared-core" function pickApi({ tableId, table }: { tableId?: string; table?: Table }) { if (table && isExternalTable(table)) { @@ -40,6 +41,20 @@ function pickApi({ tableId, table }: { tableId?: string; table?: Table }) { return internal } +function checkDefaultFields(table: Table) { + for (const [key, field] of Object.entries(table.schema)) { + if (!("default" in field) || field.default == null) { + continue + } + if (helpers.schema.isRequired(field.constraints)) { + throw new HTTPError( + `Cannot make field "${key}" required, it has a default value.`, + 400 + ) + } + } +} + // covers both internal and external export async function fetch(ctx: UserCtx) { const internal = await sdk.tables.getAllInternalTables() @@ -76,6 +91,8 @@ export async function save(ctx: UserCtx) { const isImport = table.rows const renaming = ctx.request.body._rename + checkDefaultFields(table) + const api = pickApi({ table }) let savedTable = await api.save(ctx, renaming) if (!table._id) { diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 5ce16a54d5..8cabdf5e0f 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -563,9 +563,6 @@ describe.each([ name: "description", type: FieldType.STRING, default: "default description", - constraints: { - presence: true, - }, }, }, }) diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index 8102966ad1..20c83549d2 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -86,6 +86,30 @@ describe.each([ expect(events.rows.imported).toHaveBeenCalledWith(res, 1) }) + it("should not allow a column to have a default value and be required", async () => { + await config.api.table.save( + tableForDatasource(datasource, { + schema: { + name: { + name: "name", + type: FieldType.STRING, + default: "default", + constraints: { + presence: true, + }, + }, + }, + }), + { + status: 400, + body: { + message: + 'Cannot make field "name" required, it has a default value.', + }, + } + ) + }) + it("should apply authorization to endpoint", async () => { await checkBuilderEndpoint({ config, @@ -225,6 +249,142 @@ describe.each([ }) }) + describe("default field validation", () => { + it("should error if an existing column is set to required and has a default value", async () => { + const table = await config.api.table.save( + tableForDatasource(datasource, { + schema: { + name: { + name: "name", + type: FieldType.STRING, + default: "default", + }, + }, + }) + ) + + await config.api.table.save( + { + ...table, + schema: { + ...table.schema, + name: { + name: "name", + type: FieldType.STRING, + default: "default", + constraints: { + presence: true, + }, + }, + }, + }, + { + status: 400, + body: { + message: + 'Cannot make field "name" required, it has a default value.', + }, + } + ) + }) + + it("should error if an existing column is given a default value and is required", async () => { + const table = await config.api.table.save( + tableForDatasource(datasource, { + schema: { + name: { + name: "name", + type: FieldType.STRING, + constraints: { + presence: true, + }, + }, + }, + }) + ) + + await config.api.table.save( + { + ...table, + schema: { + ...table.schema, + name: { + name: "name", + type: FieldType.STRING, + default: "default", + constraints: { + presence: true, + }, + }, + }, + }, + { + status: 400, + body: { + message: + 'Cannot make field "name" required, it has a default value.', + }, + } + ) + }) + + it("should be able to set an existing column to have a default value if it's not required", async () => { + const table = await config.api.table.save( + tableForDatasource(datasource, { + schema: { + name: { + name: "name", + type: FieldType.STRING, + }, + }, + }) + ) + + await config.api.table.save( + { + ...table, + schema: { + ...table.schema, + name: { + name: "name", + type: FieldType.STRING, + default: "default", + }, + }, + }, + { status: 200 } + ) + }) + + it("should be able to remove a default value if the column is not required", async () => { + const table = await config.api.table.save( + tableForDatasource(datasource, { + schema: { + name: { + name: "name", + type: FieldType.STRING, + default: "default", + }, + }, + }) + ) + + await config.api.table.save( + { + ...table, + schema: { + ...table.schema, + name: { + name: "name", + type: FieldType.STRING, + }, + }, + }, + { status: 200 } + ) + }) + }) + describe("external table validation", () => { !isInternal && it("should error if column is of type auto", async () => {