From 4bda97d70ff4ac4d5da8ec98d2e6c5ae1850061f Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 11 Oct 2023 11:07:46 +0100 Subject: [PATCH 1/6] Create a failing test for BUDI-7552 --- .../server/src/api/routes/tests/table.spec.ts | 44 ++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index f56c6e4e44..874b3535cd 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -1,4 +1,3 @@ -import { generator } from "@budibase/backend-core/tests" import { events, context } from "@budibase/backend-core" import { FieldType, Table, ViewCalculation } from "@budibase/types" import { checkBuilderEndpoint } from "./utilities/TestFunctions" @@ -182,6 +181,49 @@ describe("/tables", () => { 1 ) }) + + it("should update Auto ID field after bulk import", async () => { + const table = await config.createTable({ + name: "TestTable", + type: "table", + schema: { + autoId: { + name: "id", + type: FieldType.NUMBER, + subtype: "autoID", + autocolumn: true, + constraints: { + type: "number", + presence: false, + }, + }, + }, + }) + + let response = await request + .post(`/api/${table._id}/rows`) + .send({}) + .set(config.defaultHeaders()) + .expect(200) + + expect(response.body.autoId).toEqual(1) + + await request + .post(`/api/tables/${table._id}/import`) + .send({ + rows: [{ autoId: 2 }], + }) + .set(config.defaultHeaders()) + .expect(200) + + response = await request + .post(`/api/${table._id}/rows`) + .send({}) + .set(config.defaultHeaders()) + .expect(200) + + expect(response.body.autoId).toEqual(3) + }) }) describe("fetch", () => { From 7f2ab8b1ae56c83517c4d42d62b21620fa00628d Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 11 Oct 2023 12:29:43 +0100 Subject: [PATCH 2/6] Make sure table gets saved after bulkImport if it has changed. This fixes auto ID columns having the wrong lastID. --- .../server/src/api/controllers/table/external.ts | 1 + .../server/src/api/controllers/table/index.ts | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/controllers/table/external.ts b/packages/server/src/api/controllers/table/external.ts index 327904666d..ad6a56ee27 100644 --- a/packages/server/src/api/controllers/table/external.ts +++ b/packages/server/src/api/controllers/table/external.ts @@ -376,6 +376,7 @@ export async function destroy(ctx: UserCtx) { export async function bulkImport(ctx: UserCtx) { const table = await sdk.tables.getTable(ctx.params.tableId) + const { rows }: { rows: unknown } = ctx.request.body const schema: unknown = table.schema diff --git a/packages/server/src/api/controllers/table/index.ts b/packages/server/src/api/controllers/table/index.ts index e7c6ae57b0..aa3f7cc702 100644 --- a/packages/server/src/api/controllers/table/index.ts +++ b/packages/server/src/api/controllers/table/index.ts @@ -6,7 +6,7 @@ import { validate as validateSchema, } from "../../../utilities/schema" import { isExternalTable, isSQL } from "../../../integrations/utils" -import { events } from "@budibase/backend-core" +import { context, events } from "@budibase/backend-core" import { FetchTablesResponse, SaveTableRequest, @@ -18,7 +18,7 @@ import { import sdk from "../../../sdk" import { jsonFromCsvString } from "../../../utilities/csv" import { builderSocket } from "../../../websockets" -import { cloneDeep } from "lodash" +import { cloneDeep, isEqual } from "lodash" function pickApi({ tableId, table }: { tableId?: string; table?: Table }) { if (table && !tableId) { @@ -99,7 +99,16 @@ export async function destroy(ctx: UserCtx) { export async function bulkImport(ctx: UserCtx) { const tableId = ctx.params.tableId - await pickApi({ tableId }).bulkImport(ctx) + let db = context.getAppDB() + let tableBefore = await sdk.tables.getTable(tableId) + let tableAfter = await pickApi({ tableId }).bulkImport(ctx) + + if (!isEqual(tableBefore, tableAfter)) { + await db.put(tableAfter) + ctx.eventEmitter && + ctx.eventEmitter.emitTable(`table:save`, ctx.appId, tableAfter) + } + // right now we don't trigger anything for bulk import because it // can only be done in the builder, but in the future we may need to // think about events for bulk items From 0d9f257cc2ac209e7f52c9ffddaaffc1fcfe92a1 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 11 Oct 2023 12:51:57 +0100 Subject: [PATCH 3/6] Remove whitespace-only change. --- packages/server/src/api/controllers/table/external.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/server/src/api/controllers/table/external.ts b/packages/server/src/api/controllers/table/external.ts index ad6a56ee27..327904666d 100644 --- a/packages/server/src/api/controllers/table/external.ts +++ b/packages/server/src/api/controllers/table/external.ts @@ -376,7 +376,6 @@ export async function destroy(ctx: UserCtx) { export async function bulkImport(ctx: UserCtx) { const table = await sdk.tables.getTable(ctx.params.tableId) - const { rows }: { rows: unknown } = ctx.request.body const schema: unknown = table.schema From a0e9abb95bff69074c729060b8c7b5000df1a6a5 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 11 Oct 2023 16:45:02 +0100 Subject: [PATCH 4/6] Responding to PR feedback. --- .../src/api/controllers/table/external.ts | 4 ++- .../server/src/api/controllers/table/index.ts | 11 ++++--- .../src/api/controllers/table/internal.ts | 4 ++- .../server/src/api/routes/tests/table.spec.ts | 29 +++++-------------- .../server/src/tests/utilities/api/row.ts | 15 ++++++++++ packages/types/src/api/web/app/rows.ts | 9 ++++++ 6 files changed, 43 insertions(+), 29 deletions(-) diff --git a/packages/server/src/api/controllers/table/external.ts b/packages/server/src/api/controllers/table/external.ts index 327904666d..a332247a26 100644 --- a/packages/server/src/api/controllers/table/external.ts +++ b/packages/server/src/api/controllers/table/external.ts @@ -18,6 +18,8 @@ import { AutoReason, Datasource, FieldSchema, + ImportRowsRequest, + ImportRowsResponse, Operation, QueryJson, RelationshipType, @@ -374,7 +376,7 @@ export async function destroy(ctx: UserCtx) { return tableToDelete } -export async function bulkImport(ctx: UserCtx) { +export async function bulkImport(ctx: UserCtx) { const table = await sdk.tables.getTable(ctx.params.tableId) const { rows }: { rows: unknown } = ctx.request.body const schema: unknown = table.schema diff --git a/packages/server/src/api/controllers/table/index.ts b/packages/server/src/api/controllers/table/index.ts index aa3f7cc702..69e1988def 100644 --- a/packages/server/src/api/controllers/table/index.ts +++ b/packages/server/src/api/controllers/table/index.ts @@ -6,9 +6,11 @@ import { validate as validateSchema, } from "../../../utilities/schema" import { isExternalTable, isSQL } from "../../../integrations/utils" -import { context, events } from "@budibase/backend-core" +import { events } from "@budibase/backend-core" import { FetchTablesResponse, + ImportRowsRequest, + ImportRowsResponse, SaveTableRequest, SaveTableResponse, Table, @@ -97,16 +99,13 @@ export async function destroy(ctx: UserCtx) { builderSocket?.emitTableDeletion(ctx, deletedTable) } -export async function bulkImport(ctx: UserCtx) { +export async function bulkImport(ctx: UserCtx) { const tableId = ctx.params.tableId - let db = context.getAppDB() let tableBefore = await sdk.tables.getTable(tableId) let tableAfter = await pickApi({ tableId }).bulkImport(ctx) if (!isEqual(tableBefore, tableAfter)) { - await db.put(tableAfter) - ctx.eventEmitter && - ctx.eventEmitter.emitTable(`table:save`, ctx.appId, tableAfter) + await sdk.tables.saveTable(tableAfter) } // right now we don't trigger anything for bulk import because it diff --git a/packages/server/src/api/controllers/table/internal.ts b/packages/server/src/api/controllers/table/internal.ts index e468848c57..7765e630db 100644 --- a/packages/server/src/api/controllers/table/internal.ts +++ b/packages/server/src/api/controllers/table/internal.ts @@ -10,6 +10,8 @@ import { } from "../../../utilities/rowProcessor" import { runStaticFormulaChecks } from "./bulkFormula" import { + ImportRowsRequest, + ImportRowsResponse, RenameColumn, SaveTableRequest, SaveTableResponse, @@ -206,7 +208,7 @@ export async function destroy(ctx: any) { return tableToDelete } -export async function bulkImport(ctx: any) { +export async function bulkImport(ctx: UserCtx) { const table = await sdk.tables.getTable(ctx.params.tableId) const { rows, identifierFields } = ctx.request.body await handleDataImport(ctx.user, table, rows, identifierFields) diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index 874b3535cd..12472eaa21 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -200,29 +200,16 @@ describe("/tables", () => { }, }) - let response = await request - .post(`/api/${table._id}/rows`) - .send({}) - .set(config.defaultHeaders()) - .expect(200) + let row = await config.api.row.save(table._id!, {}) + expect(row.autoId).toEqual(1) - expect(response.body.autoId).toEqual(1) + await config.api.row.bulkImport(table._id!, { + rows: [{ autoId: 2 }], + identifierFields: [], + }) - await request - .post(`/api/tables/${table._id}/import`) - .send({ - rows: [{ autoId: 2 }], - }) - .set(config.defaultHeaders()) - .expect(200) - - response = await request - .post(`/api/${table._id}/rows`) - .send({}) - .set(config.defaultHeaders()) - .expect(200) - - expect(response.body.autoId).toEqual(3) + row = await config.api.row.save(table._id!, {}) + expect(row.autoId).toEqual(3) }) }) diff --git a/packages/server/src/tests/utilities/api/row.ts b/packages/server/src/tests/utilities/api/row.ts index adeb96a593..ce95bf2835 100644 --- a/packages/server/src/tests/utilities/api/row.ts +++ b/packages/server/src/tests/utilities/api/row.ts @@ -4,6 +4,8 @@ import { Row, ValidateResponse, ExportRowsRequest, + ImportRowsRequest, + ImportRowsResponse, } from "@budibase/types" import TestConfiguration from "../TestConfiguration" import { TestAPI } from "./base" @@ -123,6 +125,19 @@ export class RowAPI extends TestAPI { return request } + bulkImport = async ( + tableId: string, + body: ImportRowsRequest, + { expectStatus } = { expectStatus: 200 } + ): Promise => { + let request = this.request + .post(`/api/tables/${tableId}/import`) + .send(body) + .set(this.config.defaultHeaders()) + .expect(expectStatus) + return (await request).body + } + search = async ( sourceId: string, { expectStatus } = { expectStatus: 200 } diff --git a/packages/types/src/api/web/app/rows.ts b/packages/types/src/api/web/app/rows.ts index 62ea90a6a4..d695075ac0 100644 --- a/packages/types/src/api/web/app/rows.ts +++ b/packages/types/src/api/web/app/rows.ts @@ -37,3 +37,12 @@ export interface ExportRowsRequest { } export type ExportRowsResponse = ReadStream + +export type ImportRowsRequest = { + rows: Row[] + identifierFields: string[] +} + +export type ImportRowsResponse = { + message: string +} From c76e11035021b6fed7cb37457e052c4d7ee20e08 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 11 Oct 2023 16:58:24 +0100 Subject: [PATCH 5/6] Add BulkImportResponse as a type after merging Adri's type changes. --- packages/server/src/api/controllers/table/external.ts | 3 ++- packages/server/src/api/controllers/table/index.ts | 3 ++- packages/server/src/api/controllers/table/internal.ts | 3 ++- packages/server/src/api/routes/tests/table.spec.ts | 3 ++- packages/server/src/tests/utilities/api/row.ts | 8 ++++---- packages/types/src/api/web/app/table.ts | 4 ++++ 6 files changed, 16 insertions(+), 8 deletions(-) diff --git a/packages/server/src/api/controllers/table/external.ts b/packages/server/src/api/controllers/table/external.ts index 75318a3462..49dbc855f7 100644 --- a/packages/server/src/api/controllers/table/external.ts +++ b/packages/server/src/api/controllers/table/external.ts @@ -16,6 +16,7 @@ import { context, events } from "@budibase/backend-core" import { isRows, isSchema, parse } from "../../../utilities/schema" import { BulkImportRequest, + BulkImportResponse, Datasource, FieldSchema, ManyToManyRelationshipFieldMetadata, @@ -386,7 +387,7 @@ export async function destroy(ctx: UserCtx) { return tableToDelete } -export async function bulkImport(ctx: UserCtx) { +export async function bulkImport(ctx: UserCtx) { const table = await sdk.tables.getTable(ctx.params.tableId) const { rows } = ctx.request.body const schema = table.schema diff --git a/packages/server/src/api/controllers/table/index.ts b/packages/server/src/api/controllers/table/index.ts index 4e63686bcb..8d81e6d168 100644 --- a/packages/server/src/api/controllers/table/index.ts +++ b/packages/server/src/api/controllers/table/index.ts @@ -9,6 +9,7 @@ import { isExternalTable, isSQL } from "../../../integrations/utils" import { events } from "@budibase/backend-core" import { BulkImportRequest, + BulkImportResponse, FetchTablesResponse, SaveTableRequest, SaveTableResponse, @@ -98,7 +99,7 @@ export async function destroy(ctx: UserCtx) { builderSocket?.emitTableDeletion(ctx, deletedTable) } -export async function bulkImport(ctx: UserCtx) { +export async function bulkImport(ctx: UserCtx) { const tableId = ctx.params.tableId let tableBefore = await sdk.tables.getTable(tableId) let tableAfter = await pickApi({ tableId }).bulkImport(ctx) diff --git a/packages/server/src/api/controllers/table/internal.ts b/packages/server/src/api/controllers/table/internal.ts index 11be19a8a7..3d957b8771 100644 --- a/packages/server/src/api/controllers/table/internal.ts +++ b/packages/server/src/api/controllers/table/internal.ts @@ -11,6 +11,7 @@ import { import { runStaticFormulaChecks } from "./bulkFormula" import { BulkImportRequest, + BulkImportResponse, RenameColumn, SaveTableRequest, SaveTableResponse, @@ -207,7 +208,7 @@ export async function destroy(ctx: any) { return tableToDelete } -export async function bulkImport(ctx: UserCtx) { +export async function bulkImport(ctx: UserCtx) { const table = await sdk.tables.getTable(ctx.params.tableId) const { rows, identifierFields } = ctx.request.body await handleDataImport(ctx.user, table, rows, identifierFields) diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index f8f78561dd..ded54729b9 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -5,6 +5,7 @@ import { RelationshipType, Table, ViewCalculation, + AutoFieldSubTypes, } from "@budibase/types" import { checkBuilderEndpoint } from "./utilities/TestFunctions" import * as setup from "./utilities" @@ -196,7 +197,7 @@ describe("/tables", () => { autoId: { name: "id", type: FieldType.NUMBER, - subtype: "autoID", + subtype: AutoFieldSubTypes.AUTO_ID, autocolumn: true, constraints: { type: "number", diff --git a/packages/server/src/tests/utilities/api/row.ts b/packages/server/src/tests/utilities/api/row.ts index ce95bf2835..bb880bb7da 100644 --- a/packages/server/src/tests/utilities/api/row.ts +++ b/packages/server/src/tests/utilities/api/row.ts @@ -4,8 +4,8 @@ import { Row, ValidateResponse, ExportRowsRequest, - ImportRowsRequest, - ImportRowsResponse, + BulkImportRequest, + BulkImportResponse, } from "@budibase/types" import TestConfiguration from "../TestConfiguration" import { TestAPI } from "./base" @@ -127,9 +127,9 @@ export class RowAPI extends TestAPI { bulkImport = async ( tableId: string, - body: ImportRowsRequest, + body: BulkImportRequest, { expectStatus } = { expectStatus: 200 } - ): Promise => { + ): Promise => { let request = this.request .post(`/api/tables/${tableId}/import`) .send(body) diff --git a/packages/types/src/api/web/app/table.ts b/packages/types/src/api/web/app/table.ts index 8fb0297a9e..cb5faaa9ea 100644 --- a/packages/types/src/api/web/app/table.ts +++ b/packages/types/src/api/web/app/table.ts @@ -29,3 +29,7 @@ export interface BulkImportRequest { rows: Row[] identifierFields?: Array } + +export interface BulkImportResponse { + message: string +} From b310d7c5a7f09abbf79d8dd3ce0ce73e94ca7738 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 11 Oct 2023 17:12:28 +0100 Subject: [PATCH 6/6] Linting. --- packages/server/src/api/controllers/table/external.ts | 4 +++- packages/server/src/api/controllers/table/index.ts | 4 +++- packages/server/src/api/controllers/table/internal.ts | 4 +++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/controllers/table/external.ts b/packages/server/src/api/controllers/table/external.ts index 49dbc855f7..967176c2e4 100644 --- a/packages/server/src/api/controllers/table/external.ts +++ b/packages/server/src/api/controllers/table/external.ts @@ -387,7 +387,9 @@ export async function destroy(ctx: UserCtx) { return tableToDelete } -export async function bulkImport(ctx: UserCtx) { +export async function bulkImport( + ctx: UserCtx +) { const table = await sdk.tables.getTable(ctx.params.tableId) const { rows } = ctx.request.body const schema = table.schema diff --git a/packages/server/src/api/controllers/table/index.ts b/packages/server/src/api/controllers/table/index.ts index 8d81e6d168..44f673f284 100644 --- a/packages/server/src/api/controllers/table/index.ts +++ b/packages/server/src/api/controllers/table/index.ts @@ -99,7 +99,9 @@ export async function destroy(ctx: UserCtx) { builderSocket?.emitTableDeletion(ctx, deletedTable) } -export async function bulkImport(ctx: UserCtx) { +export async function bulkImport( + ctx: UserCtx +) { const tableId = ctx.params.tableId let tableBefore = await sdk.tables.getTable(tableId) let tableAfter = await pickApi({ tableId }).bulkImport(ctx) diff --git a/packages/server/src/api/controllers/table/internal.ts b/packages/server/src/api/controllers/table/internal.ts index 3d957b8771..eeb4a9eb5f 100644 --- a/packages/server/src/api/controllers/table/internal.ts +++ b/packages/server/src/api/controllers/table/internal.ts @@ -208,7 +208,9 @@ export async function destroy(ctx: any) { return tableToDelete } -export async function bulkImport(ctx: UserCtx) { +export async function bulkImport( + ctx: UserCtx +) { const table = await sdk.tables.getTable(ctx.params.tableId) const { rows, identifierFields } = ctx.request.body await handleDataImport(ctx.user, table, rows, identifierFields)