From 1e6652dc396a341fa836e18b7c3cea4ccd672cee Mon Sep 17 00:00:00 2001 From: Dean Date: Wed, 10 May 2023 12:36:01 +0100 Subject: [PATCH] Fix for importing exported array/option fields. Fix to ensure lastid and inclusion updates persisted as a result of an import. Test updates for array and option fields --- .../server/src/api/controllers/table/index.ts | 2 - .../src/api/controllers/table/internal.ts | 5 ++ .../server/src/api/controllers/table/utils.ts | 2 - .../server/src/api/routes/tests/misc.spec.js | 89 +++++++++++++++++-- .../server/src/api/routes/tests/row.spec.js | 67 ++++++++++++-- .../src/utilities/rowProcessor/index.ts | 5 +- .../server/src/utilities/rowProcessor/map.ts | 34 +++---- 7 files changed, 167 insertions(+), 37 deletions(-) diff --git a/packages/server/src/api/controllers/table/index.ts b/packages/server/src/api/controllers/table/index.ts index 8e248d57ea..cbbda7b930 100644 --- a/packages/server/src/api/controllers/table/index.ts +++ b/packages/server/src/api/controllers/table/index.ts @@ -98,8 +98,6 @@ export async function bulkImport(ctx: UserCtx) { // can only be done in the builder, but in the future we may need to // think about events for bulk items - //const resp = pickApi({ tableId }).save(ctx) - ctx.status = 200 ctx.body = { message: `Bulk rows created.` } } diff --git a/packages/server/src/api/controllers/table/internal.ts b/packages/server/src/api/controllers/table/internal.ts index 628932bba1..d2a4de575e 100644 --- a/packages/server/src/api/controllers/table/internal.ts +++ b/packages/server/src/api/controllers/table/internal.ts @@ -184,8 +184,13 @@ export async function destroy(ctx: any) { } export async function bulkImport(ctx: any) { + const db = context.getAppDB() const table = await sdk.tables.getTable(ctx.params.tableId) const { rows } = ctx.request.body await handleDataImport(ctx.user, table, rows) + + // Ensure auto id and other table updates are persisted + await db.put(table) + return table } diff --git a/packages/server/src/api/controllers/table/utils.ts b/packages/server/src/api/controllers/table/utils.ts index 5681095df7..f088dbaa8e 100644 --- a/packages/server/src/api/controllers/table/utils.ts +++ b/packages/server/src/api/controllers/table/utils.ts @@ -138,10 +138,8 @@ export function importToRows( row[fieldName] ) { let merged = [...schema.constraints!.inclusion!, ...rowVal] - let superSet = new Set(merged) schema.constraints!.inclusion = Array.from(superSet) - schema.constraints!.inclusion.sort() } } diff --git a/packages/server/src/api/routes/tests/misc.spec.js b/packages/server/src/api/routes/tests/misc.spec.js index 6dd82df496..21ebea637f 100644 --- a/packages/server/src/api/routes/tests/misc.spec.js +++ b/packages/server/src/api/routes/tests/misc.spec.js @@ -73,18 +73,97 @@ describe("run misc tests", () => { type: "string", }, }, + e: { + name: "Auto ID", + type: "number", + subtype: "autoID", + icon: "ri-magic-line", + autocolumn: true, + constraints: { + type: "number", + presence: false, + numericality: { + greaterThanOrEqualTo: "", + lessThanOrEqualTo: "", + }, + }, + }, + f: { + type: "array", + constraints: { + type: "array", + presence: { + "allowEmpty": true + }, + inclusion: [ + "One", + "Two", + "Three", + ] + }, + name: "Sample Tags", + sortable: false + }, + g: { + type: "options", + constraints: { + type: "string", + presence: false, + inclusion: [ + "Alpha", + "Beta", + "Gamma" + ] + }, + name: "Sample Opts" + } }, }) - + + // Shift specific row tests to the row spec await tableUtils.handleDataImport( { userId: "test" }, table, - [{ a: '1', b: '2', c: '3', d: '4'}] + [ + { a: '1', b: '2', c: '3', d: '4', f: "['One']", g: "Alpha" }, + { a: '5', b: '6', c: '7', d: '8', f: "[]", g: undefined}, + { a: '9', b: '10', c: '11', d: '12', f: "['Two','Four']", g: ""}, + { a: '13', b: '14', c: '15', d: '16', g: "Omega"} + ] ) + + // 4 rows imported, the auto ID starts at 1 + // We expect the handleDataImport function to update the lastID + expect(table.schema.e.lastID).toEqual(4); + + // Array/Multi - should have added a new value to the inclusion. + expect(table.schema.f.constraints.inclusion).toEqual(['Four','One','Three','Two']); + + // Options - should have a new value in the inclusion + expect(table.schema.g.constraints.inclusion).toEqual(['Alpha','Beta','Gamma','Omega']); + const rows = await config.getRows() - expect(rows[0].a).toEqual("1") - expect(rows[0].b).toEqual("2") - expect(rows[0].c).toEqual("3") + expect(rows.length).toEqual(4); + + const rowOne = rows.find(row => row.e === 1) + expect(rowOne.a).toEqual("1") + expect(rowOne.f).toEqual(['One']) + expect(rowOne.g).toEqual('Alpha') + + const rowTwo = rows.find(row => row.e === 2) + expect(rowTwo.a).toEqual("5") + expect(rowTwo.f).toEqual([]) + expect(rowTwo.g).toEqual(undefined) + + const rowThree = rows.find(row => row.e === 3) + expect(rowThree.a).toEqual("9") + expect(rowThree.f).toEqual(['Two','Four']) + expect(rowThree.g).toEqual(null) + + const rowFour = rows.find(row => row.e === 4) + expect(rowFour.a).toEqual("13") + expect(rowFour.f).toEqual(undefined) + expect(rowFour.g).toEqual('Omega') }) }) }) diff --git a/packages/server/src/api/routes/tests/row.spec.js b/packages/server/src/api/routes/tests/row.spec.js index 4b835a1fb5..105dd21ae0 100644 --- a/packages/server/src/api/routes/tests/row.spec.js +++ b/packages/server/src/api/routes/tests/row.spec.js @@ -34,9 +34,9 @@ describe("/rows", () => { row = basicRow(table._id) }) - const loadRow = async (id, status = 200) => + const loadRow = async (id, tbl_Id, status = 200) => await request - .get(`/api/${table._id}/rows/${id}`) + .get(`/api/${tbl_Id}/rows/${id}`) .set(config.defaultHeaders()) .expect("Content-Type", /json/) .expect(status) @@ -182,8 +182,32 @@ describe("/rows", () => { type: "string", presence: false, datetime: { earliest: "", latest: "" }, - }, + } } + const arrayField = { + type: "array", + constraints: { + type: "array", + presence: false, + inclusion: [ + "One", + "Two", + "Three", + ] + }, + name: "Sample Tags", + sortable: false + } + const optsField = { + fieldName: "Sample Opts", + name: "Sample Opts", + type: "options", + constraints: { + type: "string", + presence: false, + inclusion: [ "Alpha", "Beta", "Gamma" ] + }, + }, table = await config.createTable({ name: "TestTable2", @@ -212,7 +236,15 @@ describe("/rows", () => { attachmentNull: attachment, attachmentUndefined: attachment, attachmentEmpty: attachment, - attachmentEmptyArrayStr: attachment + attachmentEmptyArrayStr: attachment, + arrayFieldEmptyArrayStr: arrayField, + arrayFieldArrayStrKnown: arrayField, + arrayFieldNull: arrayField, + arrayFieldUndefined: arrayField, + optsFieldEmptyStr: optsField, + optsFieldUndefined: optsField, + optsFieldNull: optsField, + optsFieldStrKnown: optsField }, }) @@ -241,11 +273,20 @@ describe("/rows", () => { attachmentUndefined: undefined, attachmentEmpty: "", attachmentEmptyArrayStr: "[]", + arrayFieldEmptyArrayStr: "[]", + arrayFieldUndefined: undefined, + arrayFieldNull: null, + arrayFieldArrayStrKnown: "['One']", + optsFieldEmptyStr: "", + optsFieldUndefined: undefined, + optsFieldNull: null, + optsFieldStrKnown: 'Alpha' } - const id = (await config.createRow(row))._id + const createdRow = await config.createRow(row); + const id = createdRow._id - const saved = (await loadRow(id)).body + const saved = (await loadRow(id, table._id)).body expect(saved.stringUndefined).toBe(undefined) expect(saved.stringNull).toBe("") @@ -270,7 +311,15 @@ describe("/rows", () => { expect(saved.attachmentNull).toEqual([]) expect(saved.attachmentUndefined).toBe(undefined) expect(saved.attachmentEmpty).toEqual([]) - expect(saved.attachmentEmptyArrayStr).toEqual([]) + expect(saved.attachmentEmptyArrayStr).toEqual([]) + expect(saved.arrayFieldEmptyArrayStr).toEqual([]) + expect(saved.arrayFieldNull).toEqual([]) + expect(saved.arrayFieldUndefined).toEqual(undefined) + expect(saved.optsFieldEmptyStr).toEqual(null) + expect(saved.optsFieldUndefined).toEqual(undefined) + expect(saved.optsFieldNull).toEqual(null) + expect(saved.arrayFieldArrayStrKnown).toEqual(['One']) + expect(saved.optsFieldStrKnown).toEqual('Alpha') }) }) @@ -299,7 +348,7 @@ describe("/rows", () => { expect(res.body.name).toEqual("Updated Name") expect(res.body.description).toEqual(existing.description) - const savedRow = await loadRow(res.body._id) + const savedRow = await loadRow(res.body._id, table._id) expect(savedRow.body.description).toEqual(existing.description) expect(savedRow.body.name).toEqual("Updated Name") @@ -401,7 +450,7 @@ describe("/rows", () => { .expect(200) expect(res.body.length).toEqual(2) - await loadRow(row1._id, 404) + await loadRow(row1._id, table._id, 404) await assertRowUsage(rowUsage - 2) await assertQueryUsage(queryUsage + 1) }) diff --git a/packages/server/src/utilities/rowProcessor/index.ts b/packages/server/src/utilities/rowProcessor/index.ts index a5bb352eeb..44cab4d18b 100644 --- a/packages/server/src/utilities/rowProcessor/index.ts +++ b/packages/server/src/utilities/rowProcessor/index.ts @@ -137,8 +137,7 @@ export function inputProcessing( opts?: AutoColumnProcessingOpts ) { let clonedRow = cloneDeep(row) - // need to copy the table so it can be differenced on way out - const copiedTable = cloneDeep(table) + const dontCleanseKeys = ["type", "_id", "_rev", "tableId"] for (let [key, value] of Object.entries(clonedRow)) { const field = table.schema[key] @@ -175,7 +174,7 @@ export function inputProcessing( } // handle auto columns - this returns an object like {table, row} - return processAutoColumn(user, copiedTable, clonedRow, opts) + return processAutoColumn(user, table, clonedRow, opts) } /** diff --git a/packages/server/src/utilities/rowProcessor/map.ts b/packages/server/src/utilities/rowProcessor/map.ts index 808b16178d..cf6823856c 100644 --- a/packages/server/src/utilities/rowProcessor/map.ts +++ b/packages/server/src/utilities/rowProcessor/map.ts @@ -2,6 +2,22 @@ import { FieldTypes } from "../../constants" import { logging } from "@budibase/backend-core" +const parseArrayString = value => { + if (typeof value === "string") { + if (value === "") { + return [] + } + let result + try { + result = JSON.parse(value.replace(/'/g, '"')) + return result + } catch (e) { + logging.logAlert("Could not parse row value", e) + } + } + return value +} + /** * A map of how we convert various properties in rows to each other based on the row type. */ @@ -26,9 +42,9 @@ export const TYPE_TRANSFORM_MAP: any = { [undefined]: undefined, }, [FieldTypes.ARRAY]: { - "": [], [null]: [], [undefined]: undefined, + parse: parseArrayString, }, [FieldTypes.STRING]: { "": "", @@ -70,21 +86,7 @@ export const TYPE_TRANSFORM_MAP: any = { [FieldTypes.ATTACHMENT]: { [null]: [], [undefined]: undefined, - parse: attachments => { - if (typeof attachments === "string") { - if (attachments === "") { - return [] - } - let result - try { - result = JSON.parse(attachments) - } catch (e) { - logging.logAlert("Could not parse attachments", e) - } - return result - } - return attachments - }, + parse: parseArrayString, }, [FieldTypes.BOOLEAN]: { "": null,