From ca31add0393e033f7a95edc2f5b321bcc70d9973 Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Tue, 16 Jul 2024 16:54:34 +0100
Subject: [PATCH] Don't allow a column to be both required and have a default
 value.

---
 .../server/src/api/controllers/table/index.ts |  18 +-
 .../server/src/api/routes/tests/table.spec.ts | 160 ++++++++++++++++++
 2 files changed, 177 insertions(+), 1 deletion(-)

diff --git a/packages/server/src/api/controllers/table/index.ts b/packages/server/src/api/controllers/table/index.ts
index eae5204c7e..8698e7eab2 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,
@@ -40,6 +40,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 (field.constraints?.presence) {
+      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 +90,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) {
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 () => {