Catch a few more edge cases with column names, add tests for them.

packages/backend-core/src/db/constants.ts

@@ -8,3 +8,7 @@ export const CONSTANT_INTERNAL_ROW_COLS = [
 ] as const
 
 export const CONSTANT_EXTERNAL_ROW_COLS = ["_id", "_rev", "tableId"] as const
+
+export function isInternalColumnName(name: string): boolean {
+  return (CONSTANT_INTERNAL_ROW_COLS as readonly string[]).includes(name)
+}

packages/frontend-core/src/components/grid/controls/MigrationModal.svelte

@@ -17,6 +17,9 @@
   $: error = checkNewColumnName(newColumnName)
 
   const checkNewColumnName = newColumnName => {
+    if (newColumnName === "") {
+      return "Column name can't be empty."
+    }
     if (newColumnName in $definition.schema) {
       return "New column name can't be the same as an existing column name."
     }

packages/server/src/api/routes/tests/table.spec.ts

@@ -10,11 +10,13 @@ import {
   SaveTableRequest,
   Table,
   TableSourceType,
+  User,
   ViewCalculation,
 } from "@budibase/types"
 import { checkBuilderEndpoint } from "./utilities/TestFunctions"
 import * as setup from "./utilities"
 import sdk from "../../../sdk"
+import uuid from "uuid"
 
 const { basicTable } = setup.structures
 
@@ -426,13 +428,16 @@ describe("/tables", () => {
   })
 
   describe("migrate", () => {
-    it("should successfully migrate a one-to-many user relationship to a user column", async () => {
+    let users: User[]
-      const users = await Promise.all([
+    beforeAll(async () => {
-        config.createUser({ email: "1@example.com" }),
+      users = await Promise.all([
-        config.createUser({ email: "2@example.com" }),
+        config.createUser({ email: `${uuid.v4()}@example.com` }),
-        config.createUser({ email: "3@example.com" }),
+        config.createUser({ email: `${uuid.v4()}@example.com` }),
+        config.createUser({ email: `${uuid.v4()}@example.com` }),
       ])
+    })
 
+    it("should successfully migrate a one-to-many user relationship to a user column", async () => {
       const table = await config.api.table.create({
         name: "table",
         type: "table",
@@ -488,12 +493,6 @@ describe("/tables", () => {
     })
 
     it("should successfully migrate a many-to-many user relationship to a users column", async () => {
-      const users = await Promise.all([
-        config.createUser({ email: "1@example.com" }),
-        config.createUser({ email: "2@example.com" }),
-        config.createUser({ email: "3@example.com" }),
-      ])
-
       const table = await config.api.table.create({
         name: "table",
         type: "table",
@@ -551,12 +550,6 @@ describe("/tables", () => {
     })
 
     it("should successfully migrate a many-to-one user relationship to a users column", async () => {
-      const users = await Promise.all([
-        config.createUser({ email: "1@example.com" }),
-        config.createUser({ email: "2@example.com" }),
-        config.createUser({ email: "3@example.com" }),
-      ])
-
       const table = await config.api.table.create({
         name: "table",
         type: "table",
@@ -612,5 +605,102 @@ describe("/tables", () => {
         users[2]._id,
       ])
     })
+
+    describe("unhappy paths", () => {
+      let table: Table
+      beforeAll(async () => {
+        table = await config.api.table.create({
+          name: "table",
+          type: "table",
+          sourceId: INTERNAL_TABLE_SOURCE_ID,
+          sourceType: TableSourceType.INTERNAL,
+          schema: {
+            "user relationship": {
+              type: FieldType.LINK,
+              fieldName: "test",
+              name: "user relationship",
+              constraints: {
+                type: "array",
+                presence: false,
+              },
+              relationshipType: RelationshipType.MANY_TO_ONE,
+              tableId: InternalTable.USER_METADATA,
+            },
+            num: {
+              type: FieldType.NUMBER,
+              name: "num",
+              constraints: {
+                type: "number",
+                presence: false,
+              },
+            },
+          },
+        })
+      })
+
+      it("should fail if the new column name is blank", async () => {
+        await config.api.table.migrate(
+          table._id!,
+          {
+            oldColumn: table.schema["user relationship"],
+            newColumn: {
+              name: "",
+              type: FieldType.BB_REFERENCE,
+              subtype: FieldSubtype.USERS,
+            },
+          },
+          { expectStatus: 400 }
+        )
+      })
+
+      it("should fail if the new column name is a reserved name", async () => {
+        await config.api.table.migrate(
+          table._id!,
+          {
+            oldColumn: table.schema["user relationship"],
+            newColumn: {
+              name: "_id",
+              type: FieldType.BB_REFERENCE,
+              subtype: FieldSubtype.USERS,
+            },
+          },
+          { expectStatus: 400 }
+        )
+      })
+
+      it("should fail if the new column name is the same as an existing column", async () => {
+        await config.api.table.migrate(
+          table._id!,
+          {
+            oldColumn: table.schema["user relationship"],
+            newColumn: {
+              name: "num",
+              type: FieldType.BB_REFERENCE,
+              subtype: FieldSubtype.USERS,
+            },
+          },
+          { expectStatus: 400 }
+        )
+      })
+
+      it("should fail if the old column name isn't a column in the table", async () => {
+        await config.api.table.migrate(
+          table._id!,
+          {
+            oldColumn: {
+              name: "not a column",
+              type: FieldType.BB_REFERENCE,
+              subtype: FieldSubtype.USERS,
+            },
+            newColumn: {
+              name: "new column",
+              type: FieldType.BB_REFERENCE,
+              subtype: FieldSubtype.USERS,
+            },
+          },
+          { expectStatus: 400 }
+        )
+      })
+    })
   })
 })

packages/server/src/sdk/app/tables/migration.ts

@@ -16,6 +16,7 @@ import sdk from "../../../sdk"
 import { isExternalTableID } from "../../../integrations/utils"
 import { EventType, updateLinks } from "../../../db/linkedRows"
 import { cloneDeep } from "lodash"
+import { isInternalColumnName } from "@budibase/backend-core/src/db"
 
 export interface MigrationResult {
   tablesUpdated: Table[]
@@ -30,6 +31,14 @@ export async function migrate(
     throw new BadRequestError(`Column "${newColumn.name}" already exists`)
   }
 
+  if (newColumn.name === "") {
+    throw new BadRequestError(`Column name cannot be empty`)
+  }
+
+  if (isInternalColumnName(newColumn.name)) {
+    throw new BadRequestError(`Column name cannot be a reserved column name`)
+  }
+
   table.schema[newColumn.name] = newColumn
   table = await sdk.tables.saveTable(table)