Fix updating rows in external tables where the primary key is an autocolumn.

packages/server/src/api/controllers/row/ExternalRequest.ts

@@ -289,7 +289,13 @@ export class ExternalRequest<T extends Operation> {
       manyRelationships: ManyRelationship[] = []
     for (let [key, field] of Object.entries(table.schema)) {
       // if set already, or not set just skip it
-      if (row[key] === undefined || newRow[key] || !isEditableColumn(field)) {
+      if (row[key] === undefined || newRow[key]) {
+        continue
+      }
+      if (
+        !(this.operation === Operation.BULK_UPSERT) &&
+        !isEditableColumn(field)
+      ) {
         continue
       }
       // parse floats/numbers

packages/server/src/api/controllers/table/external.ts

@@ -86,7 +86,11 @@ export async function bulkImport(
   const { rows, identifierFields } = ctx.request.body
   const schema = table.schema
 
-  if (identifierFields && !_.isEqual(identifierFields, table.primary)) {
+  if (
+    identifierFields &&
+    identifierFields.length > 0 &&
+    !_.isEqual(identifierFields, table.primary)
+  ) {
     // This is becuse we make use of the ON CONFLICT functionality in SQL
     // databases, which only triggers when there's a conflict against a unique
     // index. The only unique index we can count on atm in Budibase is the
@@ -102,7 +106,7 @@ export async function bulkImport(
   }
 
   const parsedRows = []
-  for (const row of parse(rows, schema)) {
+  for (const row of parse(rows, table)) {
     const processed = await inputProcessing(ctx.user?._id, table, row, {
       noAutoRelationships: true,
     })

packages/server/src/api/controllers/table/utils.ts

@@ -178,7 +178,7 @@ export async function handleDataImport(
   }
 
   const db = context.getAppDB()
-  const data = parse(importRows, schema)
+  const data = parse(importRows, table)
 
   let finalData: any = await importToRows(data, table, user)
 

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

@@ -315,13 +315,13 @@ describe.each([
         // as quickly as possible.
         await Promise.all(
           sequence.map(async () => {
-            const attempts = 20
+            const attempts = 30
             for (let attempt = 0; attempt < attempts; attempt++) {
               try {
                 await config.api.row.save(table._id!, {})
                 return
               } catch (e) {
-                await new Promise(r => setTimeout(r, Math.random() * 15))
+                await new Promise(r => setTimeout(r, Math.random() * 50))
               }
             }
             throw new Error(`Failed to create row after ${attempts} attempts`)
@@ -919,24 +919,12 @@ describe.each([
       await assertRowUsage(isInternal ? rowUsage - 1 : rowUsage)
     })
 
-    it("Should ignore malformed/invalid delete requests", async () => {
+    it.each([{ not: "valid" }, { rows: 123 }, "invalid"])(
+      "Should ignore malformed/invalid delete request: %s",
+      async (request: any) => {
         const rowUsage = await getRowUsage()
 
-      await config.api.row.delete(table._id!, { not: "valid" } as any, {
-        status: 400,
-        body: {
-          message: "Invalid delete rows request",
-        },
-      })
-
-      await config.api.row.delete(table._id!, { rows: 123 } as any, {
-        status: 400,
-        body: {
-          message: "Invalid delete rows request",
-        },
-      })
-
-      await config.api.row.delete(table._id!, "invalid" as any, {
+        await config.api.row.delete(table._id!, request, {
           status: 400,
           body: {
             message: "Invalid delete rows request",
@@ -944,7 +932,8 @@ describe.each([
         })
 
         await assertRowUsage(rowUsage)
-    })
+      }
+    )
   })
 
   describe("bulkImport", () => {
@@ -1162,6 +1151,52 @@ describe.each([
         expect(rows[2].name).toEqual("Row 3")
         expect(rows[2].description).toEqual("Row 3 description")
       })
+
+    // Upserting isn't yet supported in MSSQL, see:
+    //   https://github.com/knex/knex/pull/6050
+    !isMSSQL &&
+      !isInternal &&
+      it("should be able to update existing rows an autoID primary key", async () => {
+        const tableName = uuid.v4()
+        await client!.schema.createTable(tableName, table => {
+          table.increments("userId").primary()
+          table.string("name")
+        })
+
+        const resp = await config.api.datasource.fetchSchema({
+          datasourceId: datasource!._id!,
+        })
+        const table = resp.datasource.entities![tableName]
+
+        const row1 = await config.api.row.save(table._id!, {
+          name: "Clare",
+        })
+
+        const row2 = await config.api.row.save(table._id!, {
+          name: "Jeff",
+        })
+
+        await config.api.row.bulkImport(table._id!, {
+          identifierFields: ["userId"],
+          rows: [
+            {
+              userId: row1.userId,
+              name: "Clare updated",
+            },
+            {
+              userId: row2.userId,
+              name: "Jeff updated",
+            },
+          ],
+        })
+
+        const rows = await config.api.row.fetch(table._id!)
+        expect(rows.length).toEqual(2)
+
+        rows.sort((a, b) => a.name.localeCompare(b.name))
+        expect(rows[0].name).toEqual("Clare updated")
+        expect(rows[1].name).toEqual("Jeff updated")
+      })
   })
 
   describe("enrich", () => {

packages/server/src/utilities/rowProcessor/index.ts

@@ -9,6 +9,7 @@ import {
   Row,
   RowAttachment,
   Table,
+  TableSourceType,
 } from "@budibase/types"
 import { cloneDeep } from "lodash/fp"
 import {

packages/server/src/utilities/schema.ts

@@ -4,6 +4,7 @@ import {
   TableSchema,
   FieldSchema,
   Row,
+  Table,
 } from "@budibase/types"
 import { ValidColumnNameRegex, helpers, utils } from "@budibase/shared-core"
 import { db } from "@budibase/backend-core"
@@ -118,16 +119,26 @@ export function validate(rows: Rows, schema: TableSchema): ValidationResults {
   return results
 }
 
-export function parse(rows: Rows, schema: TableSchema): Rows {
+export function parse(rows: Rows, table: Table): Rows {
   return rows.map(row => {
     const parsedRow: Row = {}
 
     Object.entries(row).forEach(([columnName, columnData]) => {
-      if (!(columnName in schema) || schema[columnName]?.autocolumn) {
+      const schema = table.schema
+      if (!(columnName in schema)) {
         // Objects can be present in the row data but not in the schema, so make sure we don't proceed in such a case
         return
       }
 
+      if (
+        schema[columnName].autocolumn &&
+        !table.primary?.includes(columnName)
+      ) {
+        // Don't want the user specifying values for autocolumns unless they're updating
+        // a row through its primary key.
+        return
+      }
+
       const columnSchema = schema[columnName]
       const { type: columnType } = columnSchema
       if (columnType === FieldType.NUMBER) {