From 0b8199be746240670bafd3cfff247304a8498054 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Thu, 31 Oct 2024 12:19:57 +0000 Subject: [PATCH 01/13] Use existing _id values for non DS+ in tables, trusting that they are unique --- .../src/components/grid/lib/constants.js | 2 ++ .../src/components/grid/lib/utils.js | 19 +++++++++++++------ .../src/components/grid/stores/rows.js | 17 +++++++++++------ 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/packages/frontend-core/src/components/grid/lib/constants.js b/packages/frontend-core/src/components/grid/lib/constants.js index 6ea7a98178..81b4a2f823 100644 --- a/packages/frontend-core/src/components/grid/lib/constants.js +++ b/packages/frontend-core/src/components/grid/lib/constants.js @@ -10,6 +10,8 @@ export const DefaultColumnWidth = 200 export const MinColumnWidth = 80 export const NewRowID = "new" export const BlankRowID = "blank" +export const GeneratedIDPrefix = "‽‽" +export const CellIDSeparator = "‽‽" export const RowPageSize = 100 export const FocusedCellMinOffset = ScrollBarSize * 3 export const ControlsHeight = 50 diff --git a/packages/frontend-core/src/components/grid/lib/utils.js b/packages/frontend-core/src/components/grid/lib/utils.js index b5485abc36..f1f33d9950 100644 --- a/packages/frontend-core/src/components/grid/lib/utils.js +++ b/packages/frontend-core/src/components/grid/lib/utils.js @@ -1,18 +1,17 @@ -// We can't use "-" as a separator as this can be present in the ID -// or column name, so we use something very unusual to avoid this problem -const JOINING_CHARACTER = "‽‽" +import { GeneratedIDPrefix, CellIDSeparator } from "./constants" +import { Helpers } from "@budibase/bbui" export const parseCellID = cellId => { if (!cellId) { return { rowId: undefined, field: undefined } } - const parts = cellId.split(JOINING_CHARACTER) + const parts = cellId.split(CellIDSeparator) const field = parts.pop() - return { rowId: parts.join(JOINING_CHARACTER), field } + return { rowId: parts.join(CellIDSeparator), field } } export const getCellID = (rowId, fieldName) => { - return `${rowId}${JOINING_CHARACTER}${fieldName}` + return `${rowId}${CellIDSeparator}${fieldName}` } export const parseEventLocation = e => { @@ -21,3 +20,11 @@ export const parseEventLocation = e => { y: e.clientY ?? e.touches?.[0]?.clientY, } } + +export const generateRowID = () => { + return `${GeneratedIDPrefix}${Helpers.uuid()}` +} + +export const isGeneratedRowID = id => { + return id?.startsWith(GeneratedIDPrefix) +} diff --git a/packages/frontend-core/src/components/grid/stores/rows.js b/packages/frontend-core/src/components/grid/stores/rows.js index 4cfaa1cfd5..122bbc295a 100644 --- a/packages/frontend-core/src/components/grid/stores/rows.js +++ b/packages/frontend-core/src/components/grid/stores/rows.js @@ -1,7 +1,12 @@ import { writable, derived, get } from "svelte/store" import { fetchData } from "../../../fetch" import { NewRowID, RowPageSize } from "../lib/constants" -import { getCellID, parseCellID } from "../lib/utils" +import { + generateRowID, + getCellID, + isGeneratedRowID, + parseCellID, +} from "../lib/utils" import { tick } from "svelte" import { Helpers } from "@budibase/bbui" import { sleep } from "../../../utils/utils" @@ -634,10 +639,10 @@ export const createActions = context => { newRow = newRows[i] // Ensure we have a unique _id. - // This means generating one for non DS+, overwriting any that may already - // exist as we cannot allow duplicates. - if (!$hasBudibaseIdentifiers) { - newRow._id = Helpers.uuid() + // We generate one for non DS+ where required, but trust that any existing + // _id values are unique (e.g. Mongo) + if (!$hasBudibaseIdentifiers && !newRow._id?.length) { + newRow._id = generateRowID() } if (!rowCacheMap[newRow._id]) { @@ -674,7 +679,7 @@ export const createActions = context => { let clone = { ...row } delete clone.__idx delete clone.__metadata - if (!get(hasBudibaseIdentifiers)) { + if (!get(hasBudibaseIdentifiers) && isGeneratedRowID(clone._id)) { delete clone._id } return clone From afbec934f31e132f637aa944f04599da2c06395e Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Thu, 31 Oct 2024 12:28:52 +0000 Subject: [PATCH 02/13] Clean table data before using it as additional data context to better reflect usable structure --- packages/client/src/components/app/GridBlock.svelte | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/client/src/components/app/GridBlock.svelte b/packages/client/src/components/app/GridBlock.svelte index c6b08e3869..3d9dcc028e 100644 --- a/packages/client/src/components/app/GridBlock.svelte +++ b/packages/client/src/components/app/GridBlock.svelte @@ -60,8 +60,10 @@ // Provide additional data context for live binding eval export const getAdditionalDataContext = () => { - const rows = get(grid?.getContext()?.rows) - const goldenRow = generateGoldenSample(rows) + const gridContext = grid?.getContext() + const rows = get(gridContext?.rows) || [] + const cleaned = rows.map(gridContext?.rows.actions.cleanRow || (x => x)) + const goldenRow = generateGoldenSample(cleaned) const id = get(component).id return { // Not sure what this one is for... From b9ba76f639aefaed40c829910bbf3aa8523737db Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Thu, 31 Oct 2024 12:33:07 +0000 Subject: [PATCH 03/13] Avoid creating new function every iteration of array.map --- packages/client/src/components/app/GridBlock.svelte | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/client/src/components/app/GridBlock.svelte b/packages/client/src/components/app/GridBlock.svelte index 3d9dcc028e..4a8dfb4fcb 100644 --- a/packages/client/src/components/app/GridBlock.svelte +++ b/packages/client/src/components/app/GridBlock.svelte @@ -62,7 +62,8 @@ export const getAdditionalDataContext = () => { const gridContext = grid?.getContext() const rows = get(gridContext?.rows) || [] - const cleaned = rows.map(gridContext?.rows.actions.cleanRow || (x => x)) + const clean = gridContext?.rows.actions.cleanRow || (x => x) + const cleaned = rows.map(clean) const goldenRow = generateGoldenSample(cleaned) const id = get(component).id return { From c85dd1f6f89070c338f3e8487ee61bbd5f42ba0d Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Thu, 31 Oct 2024 14:30:54 +0000 Subject: [PATCH 04/13] Allow counting on non-numeric fields --- .../buttons/grid/GridViewCalculationButton.svelte | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/builder/src/components/backend/DataTable/buttons/grid/GridViewCalculationButton.svelte b/packages/builder/src/components/backend/DataTable/buttons/grid/GridViewCalculationButton.svelte index f6ef6e6099..851dfb6777 100644 --- a/packages/builder/src/components/backend/DataTable/buttons/grid/GridViewCalculationButton.svelte +++ b/packages/builder/src/components/backend/DataTable/buttons/grid/GridViewCalculationButton.svelte @@ -90,8 +90,15 @@ const getFieldOptions = (self, calculations, schema) => { return Object.entries(schema) .filter(([field, fieldSchema]) => { - // Only allow numeric fields that are not calculations themselves - if (fieldSchema.calculationType || !isNumeric(fieldSchema.type)) { + // Don't allow other calculation columns + if (fieldSchema.calculationType) { + return false + } + // Only allow numeric columns for most calculation types + if ( + self.type !== CalculationType.COUNT && + !isNumeric(fieldSchema.type) + ) { return false } // Don't allow duplicates @@ -257,6 +264,4 @@ .group-by { grid-column: 2 / 5; } - span { - } From 76a68a12f0b92f1c29903dd5a6cbd5a85ebcdd75 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 31 Oct 2024 14:33:37 +0000 Subject: [PATCH 05/13] Fix handling of the Postgres money field. --- packages/backend-core/src/sql/sql.ts | 9 ++-- .../src/integration-test/postgres.spec.ts | 52 ++++++++++++++++++- 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index d94b817ba4..6043a8713e 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -300,11 +300,9 @@ class InternalBuilder { const columnSchema = schema[column] if (this.SPECIAL_SELECT_CASES.POSTGRES_MONEY(columnSchema)) { - // TODO: figure out how to express this safely without string - // interpolation. - return this.knex.raw(`??::money::numeric as "${field}"`, [ + return this.knex.raw(`??::money::numeric as ??`, [ this.rawQuotedIdentifier([table, column].join(".")), - field, + this.knex.raw(this.quote(field)), ]) } @@ -314,8 +312,9 @@ class InternalBuilder { // TODO: figure out how to express this safely without string // interpolation. - return this.knex.raw(`CONVERT(varchar, ??, 108) as "${field}"`, [ + return this.knex.raw(`CONVERT(varchar, ??, 108) as ??`, [ this.rawQuotedIdentifier(field), + this.knex.raw(this.quote(field)), ]) } diff --git a/packages/server/src/integration-test/postgres.spec.ts b/packages/server/src/integration-test/postgres.spec.ts index f2e0382deb..d6978e743f 100644 --- a/packages/server/src/integration-test/postgres.spec.ts +++ b/packages/server/src/integration-test/postgres.spec.ts @@ -1,5 +1,5 @@ import * as setup from "../api/routes/tests/utilities" -import { Datasource, FieldType } from "@budibase/types" +import { Datasource, FieldType, Table } from "@budibase/types" import _ from "lodash" import { generator } from "@budibase/backend-core/tests" import { @@ -229,4 +229,54 @@ describe("postgres integrations", () => { ).toBeUndefined() }) }) + + describe.only("money field 💰", () => { + const tableName = "moneytable" + let table: Table + + beforeAll(async () => { + await client.raw(` + CREATE TABLE ${tableName} ( + id serial PRIMARY KEY, + price money + ) + `) + const response = await config.api.datasource.fetchSchema({ + datasourceId: datasource._id!, + }) + table = response.datasource.entities![tableName] + }) + + it("should be able to import a money field", async () => { + expect(table).toBeDefined() + expect(table?.schema.price.type).toBe(FieldType.NUMBER) + }) + + it("should be able to search a money field", async () => { + await config.api.row.bulkImport(table._id!, { + rows: [{ price: 200 }, { price: 300 }], + }) + + const { rows } = await config.api.row.search(table._id!, { + query: { + equal: { + price: 200, + }, + }, + }) + expect(rows).toHaveLength(1) + expect(rows[0].price).toBe("200.00") + }) + + it("should be able to update a money field", async () => { + let row = await config.api.row.save(table._id!, { price: 200 }) + expect(row.price).toBe("200.00") + + row = await config.api.row.save(table._id!, { ...row, price: 300 }) + expect(row.price).toBe("300.00") + + row = await config.api.row.save(table._id!, { ...row, price: "400.00" }) + expect(row.price).toBe("400.00") + }) + }) }) From 986cef6b25ebdcbb7260bd69004983908a8ad24d Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Thu, 31 Oct 2024 14:35:00 +0000 Subject: [PATCH 06/13] Update view calculation explanation text --- .../DataTable/buttons/grid/GridViewCalculationButton.svelte | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/builder/src/components/backend/DataTable/buttons/grid/GridViewCalculationButton.svelte b/packages/builder/src/components/backend/DataTable/buttons/grid/GridViewCalculationButton.svelte index 851dfb6777..7b279d3948 100644 --- a/packages/builder/src/components/backend/DataTable/buttons/grid/GridViewCalculationButton.svelte +++ b/packages/builder/src/components/backend/DataTable/buttons/grid/GridViewCalculationButton.svelte @@ -241,7 +241,7 @@
From 7026a35e046e02aa5d93cfc8f12f16a5879c2d0c Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 31 Oct 2024 14:46:57 +0000 Subject: [PATCH 07/13] Remove focus test. --- packages/server/src/integration-test/postgres.spec.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/server/src/integration-test/postgres.spec.ts b/packages/server/src/integration-test/postgres.spec.ts index d6978e743f..401e182578 100644 --- a/packages/server/src/integration-test/postgres.spec.ts +++ b/packages/server/src/integration-test/postgres.spec.ts @@ -230,7 +230,7 @@ describe("postgres integrations", () => { }) }) - describe.only("money field 💰", () => { + describe("money field 💰", () => { const tableName = "moneytable" let table: Table @@ -277,6 +277,9 @@ describe("postgres integrations", () => { row = await config.api.row.save(table._id!, { ...row, price: "400.00" }) expect(row.price).toBe("400.00") + + row = await config.api.row.save(table._id!, { ...row, price: "400.123" }) + expect(row.price).toBe("400.123") }) }) }) From 91dfa9a3780739e4933494300447e7cef6bba931 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 31 Oct 2024 14:47:14 +0000 Subject: [PATCH 08/13] Remove failing test. --- packages/server/src/integration-test/postgres.spec.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/server/src/integration-test/postgres.spec.ts b/packages/server/src/integration-test/postgres.spec.ts index 401e182578..7654d84551 100644 --- a/packages/server/src/integration-test/postgres.spec.ts +++ b/packages/server/src/integration-test/postgres.spec.ts @@ -277,9 +277,6 @@ describe("postgres integrations", () => { row = await config.api.row.save(table._id!, { ...row, price: "400.00" }) expect(row.price).toBe("400.00") - - row = await config.api.row.save(table._id!, { ...row, price: "400.123" }) - expect(row.price).toBe("400.123") }) }) }) From 9a2d5049345383a30787ed9d4d69d78a2ed46da6 Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Thu, 31 Oct 2024 15:31:31 +0000 Subject: [PATCH 09/13] Bump version to 2.33.14 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index ae6faef23d..7c47d0b3df 100644 --- a/lerna.json +++ b/lerna.json @@ -1,6 +1,6 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", - "version": "2.33.13", + "version": "2.33.14", "npmClient": "yarn", "packages": [ "packages/*", From 78554bdc7c1926306428f7d913787366f4929f1c Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 31 Oct 2024 17:16:06 +0000 Subject: [PATCH 10/13] Fix flaky export test. --- packages/server/package.json | 1 + .../src/api/controllers/view/exporters.ts | 30 +++++++++------- .../server/src/api/routes/tests/row.spec.ts | 34 +++++++++++++++++++ packages/server/src/utilities/csv.ts | 6 ++-- 4 files changed, 56 insertions(+), 15 deletions(-) diff --git a/packages/server/package.json b/packages/server/package.json index 76dd03b5a8..da770a3ed9 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -75,6 +75,7 @@ "content-disposition": "^0.5.4", "cookies": "0.8.0", "csvtojson": "2.0.10", + "csv": "6.3.10", "curlconverter": "3.21.0", "dd-trace": "5.2.0", "dotenv": "8.2.0", diff --git a/packages/server/src/api/controllers/view/exporters.ts b/packages/server/src/api/controllers/view/exporters.ts index 3269133d4b..87e4bfc056 100644 --- a/packages/server/src/api/controllers/view/exporters.ts +++ b/packages/server/src/api/controllers/view/exporters.ts @@ -9,29 +9,33 @@ function getHeaders( return headers.map(header => `"${customHeaders[header] || header}"`) } +function escapeCsvString(str: string) { + return str.replace(/"/g, '""') +} + export function csv( headers: string[], rows: Row[], delimiter: string = ",", customHeaders: { [key: string]: string } = {} ) { - let csv = getHeaders(headers, customHeaders).join(delimiter) + let csvRows = [getHeaders(headers, customHeaders)] for (let row of rows) { - csv = `${csv}\n${headers - .map(header => { - let val = row[header] - val = - typeof val === "object" && !(val instanceof Date) - ? `"${JSON.stringify(val).replace(/"/g, "'")}"` - : val !== undefined - ? `"${val}"` - : "" - return val.trim() + csvRows.push( + headers.map(header => { + const val = row[header] + if (typeof val === "object" && !(val instanceof Date)) { + return `"${JSON.stringify(val).replace(/"/g, "'")}"` + } + if (val !== undefined) { + return `"${escapeCsvString(val.toString())}"` + } + return "" }) - .join(delimiter)}` + ) } - return csv + return csvRows.map(row => row.join(delimiter)).join("\n") } export function json(rows: Row[]) { diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index cf3c87c430..252731bc16 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -2597,6 +2597,40 @@ describe.each([ }) }) + it("can handle csv-special characters in strings", async () => { + const badString = 'test":, wow", "test": "wow"' + const table = await config.api.table.save( + saveTableRequest({ + schema: { + string: { + type: FieldType.STRING, + name: "string", + }, + }, + }) + ) + + await config.api.row.save(table._id!, { string: badString }) + + const exportedValue = await config.api.row.exportRows( + table._id!, + { query: {} }, + RowExportFormat.CSV + ) + + const json = await config.api.table.csvToJson( + { + csvString: exportedValue, + }, + { + status: 200, + } + ) + + expect(json).toHaveLength(1) + expect(json[0].string).toEqual(badString) + }) + it("exported data can be re-imported", async () => { // export all const exportedValue = await config.api.row.exportRows( diff --git a/packages/server/src/utilities/csv.ts b/packages/server/src/utilities/csv.ts index 2fab1d11a4..43d712165a 100644 --- a/packages/server/src/utilities/csv.ts +++ b/packages/server/src/utilities/csv.ts @@ -5,8 +5,10 @@ export async function jsonFromCsvString(csvString: string) { csvString ) - // By default the csvtojson library casts empty values as empty strings. This is causing issues on conversion. - // ignoreEmpty will remove the key completly if empty, so creating this empty object will ensure we return the values with the keys but empty values + // By default the csvtojson library casts empty values as empty strings. This + // is causing issues on conversion. ignoreEmpty will remove the key completly + // if empty, so creating this empty object will ensure we return the values + // with the keys but empty values const result = await csv({ ignoreEmpty: false }).fromString(csvString) result.forEach((r, i) => { for (const [key] of Object.entries(r).filter(([, value]) => value === "")) { From b4e758d62971b29c8d4de58273c73561e4e53ae3 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 31 Oct 2024 17:23:51 +0000 Subject: [PATCH 11/13] No longer using the csv package. --- packages/server/package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/server/package.json b/packages/server/package.json index da770a3ed9..76dd03b5a8 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -75,7 +75,6 @@ "content-disposition": "^0.5.4", "cookies": "0.8.0", "csvtojson": "2.0.10", - "csv": "6.3.10", "curlconverter": "3.21.0", "dd-trace": "5.2.0", "dotenv": "8.2.0", From c72ca658b9b949442ec0b0980a125e91ae63f486 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 1 Nov 2024 14:30:14 +0000 Subject: [PATCH 12/13] Fixing an issue with sample data being added to an app due to the changes to bulk import. --- .../server/src/api/controllers/table/utils.ts | 7 +++---- .../src/api/routes/tests/application.spec.ts | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/packages/server/src/api/controllers/table/utils.ts b/packages/server/src/api/controllers/table/utils.ts index 743cce410b..4ea11c7043 100644 --- a/packages/server/src/api/controllers/table/utils.ts +++ b/packages/server/src/api/controllers/table/utils.ts @@ -123,7 +123,7 @@ export async function importToRows( data: Row[], table: Table, userId?: string, - opts?: { keepCouchId: boolean } + opts?: { keepCouchId?: boolean; allowRelationships?: boolean } ) { const originalTable = table const finalData: Row[] = [] @@ -136,16 +136,15 @@ export async function importToRows( // We use a reference to table here and update it after input processing, // so that we can auto increment auto IDs in imported data properly - const processed = await inputProcessing(userId, table, row, { + row = await inputProcessing(userId, table, row, { noAutoRelationships: true, }) - row = processed // However here we must reference the original table, as we want to mutate // the real schema of the table passed in, not the clone used for // incrementing auto IDs for (const [fieldName, schema] of Object.entries(originalTable.schema)) { - if (schema.type === FieldType.LINK) { + if (schema.type === FieldType.LINK && data.find(row => row[fieldName])) { throw new HTTPError( `Can't bulk import relationship fields for internal databases, found value in field "${fieldName}"`, 400 diff --git a/packages/server/src/api/routes/tests/application.spec.ts b/packages/server/src/api/routes/tests/application.spec.ts index 729f899379..6d85cdbda9 100644 --- a/packages/server/src/api/routes/tests/application.spec.ts +++ b/packages/server/src/api/routes/tests/application.spec.ts @@ -1,3 +1,5 @@ +import { DEFAULT_TABLES } from "../../../db/defaultData/datasource_bb_default" + jest.mock("../../../utilities/redis", () => ({ init: jest.fn(), getLocksById: () => { @@ -447,4 +449,18 @@ describe("/applications", () => { expect(devLogs.data.length).toBe(0) }) }) + + describe("POST /api/applications/:appId/sample", () => { + it("should be able to add sample data", async () => { + await config.api.application.addSampleData(config.getAppId()) + for (let table of DEFAULT_TABLES) { + const res = await config.api.row.search( + table._id!, + { query: {} }, + { status: 200 } + ) + expect(res.rows.length).not.toEqual(0) + } + }) + }) }) From 23f2df938e2583770d1909a4affaa36139907a14 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 1 Nov 2024 14:31:33 +0000 Subject: [PATCH 13/13] Removing un-necessary statement. --- packages/server/src/api/controllers/table/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/api/controllers/table/utils.ts b/packages/server/src/api/controllers/table/utils.ts index 4ea11c7043..04e77fbe62 100644 --- a/packages/server/src/api/controllers/table/utils.ts +++ b/packages/server/src/api/controllers/table/utils.ts @@ -123,7 +123,7 @@ export async function importToRows( data: Row[], table: Table, userId?: string, - opts?: { keepCouchId?: boolean; allowRelationships?: boolean } + opts?: { keepCouchId: boolean } ) { const originalTable = table const finalData: Row[] = []