Merge pull request #14186 from Budibase/budi-8436-handle-required-fields
[BUDI-8436] Make sure a field cannot both have a default value and be required.
This commit is contained in:
commit
b6f1396a01
|
@ -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<void, FetchTablesResponse>) {
|
||||
const internal = await sdk.tables.getAllInternalTables()
|
||||
|
@ -76,6 +91,8 @@ export async function save(ctx: UserCtx<SaveTableRequest, SaveTableResponse>) {
|
|||
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) {
|
||||
|
|
|
@ -563,9 +563,6 @@ describe.each([
|
|||
name: "description",
|
||||
type: FieldType.STRING,
|
||||
default: "default description",
|
||||
constraints: {
|
||||
presence: true,
|
||||
},
|
||||
},
|
||||
},
|
||||
})
|
||||
|
|
|
@ -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 () => {
|
||||
|
|
Loading…
Reference in New Issue