Merge pull request #14290 from Budibase/fix/reimporting_rows

Handle `_id` when reimporting internal rows

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

@@ -17,6 +17,7 @@ import {
   CsvToJsonRequest,
   CsvToJsonResponse,
   FetchTablesResponse,
+  FieldType,
   MigrateRequest,
   MigrateResponse,
   SaveTableRequest,
@@ -178,9 +179,17 @@ export async function validateExistingTableImport(
   const { rows, tableId } = ctx.request.body
 
   let schema = null
+
   if (tableId) {
     const table = await sdk.tables.getTable(tableId)
     schema = table.schema
+
+    if (!isExternalTable(table)) {
+      schema._id = {
+        name: "_id",
+        type: FieldType.STRING,
+      }
+    }
   } else {
     ctx.status = 422
     return

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

@@ -3,6 +3,7 @@ import { handleDataImport } from "./utils"
 import {
   BulkImportRequest,
   BulkImportResponse,
+  FieldType,
   RenameColumn,
   SaveTableRequest,
   SaveTableResponse,
@@ -69,10 +70,22 @@ export async function bulkImport(
 ) {
   const table = await sdk.tables.getTable(ctx.params.tableId)
   const { rows, identifierFields } = ctx.request.body
-  await handleDataImport(table, {
+  await handleDataImport(
+    {
+      ...table,
+      schema: {
+        _id: {
+          name: "_id",
+          type: FieldType.STRING,
+        },
+        ...table.schema,
+      },
+    },
+    {
       importRows: rows,
       identifierFields,
       user: ctx.user,
-  })
+    }
+  )
   return table
 }

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

@@ -122,13 +122,15 @@ export function makeSureTableUpToDate(table: Table, tableToSave: Table) {
 export async function importToRows(
   data: Row[],
   table: Table,
-  user?: ContextUser
+  user?: ContextUser,
+  opts?: { keepCouchId: boolean }
 ) {
-  let originalTable = table
+  const originalTable = table
-  let finalData: any = []
+  const finalData: Row[] = []
+  const keepCouchId = !!opts?.keepCouchId
   for (let i = 0; i < data.length; i++) {
     let row = data[i]
-    row._id = generateRowID(table._id!)
+    row._id = (keepCouchId && row._id) || generateRowID(table._id!)
     row.type = "row"
     row.tableId = table._id
 
@@ -180,7 +182,11 @@ export async function handleDataImport(
   const db = context.getAppDB()
   const data = parse(importRows, table)
 
-  let finalData: any = await importToRows(data, table, user)
+  const finalData = await importToRows(data, table, user, {
+    keepCouchId: identifierFields.includes("_id"),
+  })
+
+  let newRowCount = finalData.length
 
   //Set IDs of finalData to match existing row if an update is expected
   if (identifierFields.length > 0) {
@@ -203,12 +209,14 @@ export async function handleDataImport(
           if (match) {
             finalItem._id = doc._id
             finalItem._rev = doc._rev
+
+            newRowCount--
           }
         })
       })
   }
 
-  await quotas.addRows(finalData.length, () => db.bulkDocs(finalData), {
+  await quotas.addRows(newRowCount, () => db.bulkDocs(finalData), {
     tableId: table._id,
   })
 

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

@@ -1298,6 +1298,113 @@ describe.each([
       await assertRowUsage(isInternal ? rowUsage + 2 : rowUsage)
     })
 
+    isInternal &&
+      it("should be able to update existing rows on bulkImport", async () => {
+        const table = await config.api.table.save(
+          saveTableRequest({
+            schema: {
+              name: {
+                type: FieldType.STRING,
+                name: "name",
+              },
+              description: {
+                type: FieldType.STRING,
+                name: "description",
+              },
+            },
+          })
+        )
+
+        const existingRow = await config.api.row.save(table._id!, {
+          name: "Existing row",
+          description: "Existing description",
+        })
+
+        const rowUsage = await getRowUsage()
+
+        await config.api.row.bulkImport(table._id!, {
+          rows: [
+            {
+              name: "Row 1",
+              description: "Row 1 description",
+            },
+            { ...existingRow, name: "Updated existing row" },
+            {
+              name: "Row 2",
+              description: "Row 2 description",
+            },
+          ],
+          identifierFields: ["_id"],
+        })
+
+        const rows = await config.api.row.fetch(table._id!)
+        expect(rows.length).toEqual(3)
+
+        rows.sort((a, b) => a.name.localeCompare(b.name))
+        expect(rows[0].name).toEqual("Row 1")
+        expect(rows[0].description).toEqual("Row 1 description")
+        expect(rows[1].name).toEqual("Row 2")
+        expect(rows[1].description).toEqual("Row 2 description")
+        expect(rows[2].name).toEqual("Updated existing row")
+        expect(rows[2].description).toEqual("Existing description")
+
+        await assertRowUsage(rowUsage + 2)
+      })
+
+    isInternal &&
+      it("should create new rows if not identifierFields are provided", async () => {
+        const table = await config.api.table.save(
+          saveTableRequest({
+            schema: {
+              name: {
+                type: FieldType.STRING,
+                name: "name",
+              },
+              description: {
+                type: FieldType.STRING,
+                name: "description",
+              },
+            },
+          })
+        )
+
+        const existingRow = await config.api.row.save(table._id!, {
+          name: "Existing row",
+          description: "Existing description",
+        })
+
+        const rowUsage = await getRowUsage()
+
+        await config.api.row.bulkImport(table._id!, {
+          rows: [
+            {
+              name: "Row 1",
+              description: "Row 1 description",
+            },
+            { ...existingRow, name: "Updated existing row" },
+            {
+              name: "Row 2",
+              description: "Row 2 description",
+            },
+          ],
+        })
+
+        const rows = await config.api.row.fetch(table._id!)
+        expect(rows.length).toEqual(4)
+
+        rows.sort((a, b) => a.name.localeCompare(b.name))
+        expect(rows[0].name).toEqual("Existing row")
+        expect(rows[0].description).toEqual("Existing description")
+        expect(rows[1].name).toEqual("Row 1")
+        expect(rows[1].description).toEqual("Row 1 description")
+        expect(rows[2].name).toEqual("Row 2")
+        expect(rows[2].description).toEqual("Row 2 description")
+        expect(rows[3].name).toEqual("Updated existing row")
+        expect(rows[3].description).toEqual("Existing description")
+
+        await assertRowUsage(rowUsage + 3)
+      })
+
     // Upserting isn't yet supported in MSSQL, see:
     //   https://github.com/knex/knex/pull/6050
     !isMSSQL &&

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

@@ -1,4 +1,4 @@
-import { context, events } from "@budibase/backend-core"
+import { context, docIds, events } from "@budibase/backend-core"
 import {
   AutoFieldSubType,
   BBReferenceFieldSubType,
@@ -10,6 +10,7 @@ import {
   Row,
   SaveTableRequest,
   Table,
+  TableSchema,
   TableSourceType,
   User,
   ViewCalculation,
@@ -1022,4 +1023,92 @@ describe.each([
       })
     })
   })
+
+  describe("import validation", () => {
+    const basicSchema: TableSchema = {
+      id: {
+        type: FieldType.NUMBER,
+        name: "id",
+      },
+      name: {
+        type: FieldType.STRING,
+        name: "name",
+      },
+    }
+
+    describe("validateNewTableImport", () => {
+      it("can validate basic imports", async () => {
+        const result = await config.api.table.validateNewTableImport(
+          [{ id: generator.natural(), name: generator.first() }],
+          basicSchema
+        )
+
+        expect(result).toEqual({
+          allValid: true,
+          errors: {},
+          invalidColumns: [],
+          schemaValidation: {
+            id: true,
+            name: true,
+          },
+        })
+      })
+    })
+
+    describe("validateExistingTableImport", () => {
+      it("can validate basic imports", async () => {
+        const table = await config.api.table.save(
+          tableForDatasource(datasource, {
+            primary: ["id"],
+            schema: basicSchema,
+          })
+        )
+        const result = await config.api.table.validateExistingTableImport({
+          tableId: table._id,
+          rows: [{ id: generator.natural(), name: generator.first() }],
+        })
+
+        expect(result).toEqual({
+          allValid: true,
+          errors: {},
+          invalidColumns: [],
+          schemaValidation: {
+            id: true,
+            name: true,
+          },
+        })
+      })
+
+      isInternal &&
+        it("can reimport _id fields for internal tables", async () => {
+          const table = await config.api.table.save(
+            tableForDatasource(datasource, {
+              primary: ["id"],
+              schema: basicSchema,
+            })
+          )
+          const result = await config.api.table.validateExistingTableImport({
+            tableId: table._id,
+            rows: [
+              {
+                _id: docIds.generateRowID(table._id!),
+                id: generator.natural(),
+                name: generator.first(),
+              },
+            ],
+          })
+
+          expect(result).toEqual({
+            allValid: true,
+            errors: {},
+            invalidColumns: [],
+            schemaValidation: {
+              _id: true,
+              id: true,
+              name: true,
+            },
+          })
+        })
+    })
+  })
 })

packages/server/src/db/defaultData/datasource_bb_default.ts

@@ -651,10 +651,10 @@ export async function buildDefaultDocs() {
       return new LinkDocument(
         employeeData.table._id!,
         "Jobs",
-        employeeData.rows[index]._id,
+        employeeData.rows[index]._id!,
         jobData.table._id!,
         "Assigned",
-        jobData.rows[index]._id
+        jobData.rows[index]._id!
       )
     }
   )

packages/server/src/tests/utilities/api/table.ts

@@ -3,9 +3,13 @@ import {
   BulkImportResponse,
   MigrateRequest,
   MigrateResponse,
+  Row,
   SaveTableRequest,
   SaveTableResponse,
   Table,
+  TableSchema,
+  ValidateTableImportRequest,
+  ValidateTableImportResponse,
 } from "@budibase/types"
 import { Expectations, TestAPI } from "./base"
 
@@ -61,8 +65,38 @@ export class TableAPI extends TestAPI {
     revId: string,
     expectations?: Expectations
   ): Promise<void> => {
-    return await this._delete<void>(`/api/tables/${tableId}/${revId}`, {
+    return await this._delete(`/api/tables/${tableId}/${revId}`, {
       expectations,
     })
   }
+
+  validateNewTableImport = async (
+    rows: Row[],
+    schema: TableSchema,
+    expectations?: Expectations
+  ): Promise<ValidateTableImportResponse> => {
+    return await this._post<ValidateTableImportResponse>(
+      `/api/tables/validateNewTableImport`,
+      {
+        body: {
+          rows,
+          schema,
+        },
+        expectations,
+      }
+    )
+  }
+
+  validateExistingTableImport = async (
+    body: ValidateTableImportRequest,
+    expectations?: Expectations
+  ): Promise<ValidateTableImportResponse> => {
+    return await this._post<ValidateTableImportResponse>(
+      `/api/tables/validateExistingTableImport`,
+      {
+        body,
+        expectations,
+      }
+    )
+  }
 }