From ef7281716acd7799b3c3c92a6eff411e86db35cf Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 5 Nov 2024 13:14:45 +0100 Subject: [PATCH 01/13] Wait for test data to be processed before running the test --- .../AutomationBuilder/FlowChart/TestDataModal.svelte | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/builder/src/components/automation/AutomationBuilder/FlowChart/TestDataModal.svelte b/packages/builder/src/components/automation/AutomationBuilder/FlowChart/TestDataModal.svelte index 21cdc4b893..4bc1e8d496 100644 --- a/packages/builder/src/components/automation/AutomationBuilder/FlowChart/TestDataModal.svelte +++ b/packages/builder/src/components/automation/AutomationBuilder/FlowChart/TestDataModal.svelte @@ -1,4 +1,5 @@ - +{#if responseType === FieldType.NUMBER} + +{:else if responseType === FieldType.BOOLEAN} + +{:else if responseType === FieldType.DATETIME} + +{:else} + +{/if} diff --git a/packages/server/src/utilities/rowProcessor/index.ts b/packages/server/src/utilities/rowProcessor/index.ts index 910e9d220f..e21cd81e3f 100644 --- a/packages/server/src/utilities/rowProcessor/index.ts +++ b/packages/server/src/utilities/rowProcessor/index.ts @@ -163,33 +163,33 @@ async function processDefaultValues(table: Table, row: Row) { /** * This will coerce a value to the correct types based on the type transform map - * @param row The value to coerce + * @param value The value to coerce * @param type The type fo coerce to * @returns The coerced value */ -export function coerce(row: any, type: string) { +export function coerce(value: unknown, type: string) { // no coercion specified for type, skip it if (!TYPE_TRANSFORM_MAP[type]) { - return row + return value } // eslint-disable-next-line no-prototype-builtins - if (TYPE_TRANSFORM_MAP[type].hasOwnProperty(row)) { + if (TYPE_TRANSFORM_MAP[type].hasOwnProperty(value)) { // @ts-ignore - return TYPE_TRANSFORM_MAP[type][row] + return TYPE_TRANSFORM_MAP[type][value] } else if (TYPE_TRANSFORM_MAP[type].parse) { // @ts-ignore - return TYPE_TRANSFORM_MAP[type].parse(row) + return TYPE_TRANSFORM_MAP[type].parse(value) } - return row + return value } /** * Given an input route this function will apply all the necessary pre-processing to it, such as coercion * of column values or adding auto-column values. - * @param user the user which is performing the input. + * @param userId the ID of the user which is performing the input. * @param row the row which is being created/updated. - * @param table the table which the row is being saved to. + * @param source the table/view which the row is being saved to. * @param opts some input processing options (like disabling auto-column relationships). * @returns the row which has been prepared to be written to the DB. */ diff --git a/packages/server/src/utilities/rowProcessor/utils.ts b/packages/server/src/utilities/rowProcessor/utils.ts index 33aba5eb3a..15c0612fae 100644 --- a/packages/server/src/utilities/rowProcessor/utils.ts +++ b/packages/server/src/utilities/rowProcessor/utils.ts @@ -10,11 +10,13 @@ import { FieldType, OperationFieldTypeEnum, AIOperationEnum, + AIFieldMetadata, } from "@budibase/types" import { OperationFields } from "@budibase/shared-core" import tracer from "dd-trace" import { context } from "@budibase/backend-core" import * as pro from "@budibase/pro" +import { coerce } from "./index" interface FormulaOpts { dynamic?: boolean @@ -67,7 +69,18 @@ export async function processFormulas( continue } + const responseType = schema.responseType const isStatic = schema.formulaType === FormulaType.STATIC + const formula = schema.formula + + // coerce static values + if (isStatic) { + rows.forEach(row => { + if (row[column] && responseType) { + row[column] = coerce(row[column], responseType) + } + }) + } if ( schema.formula == null || @@ -80,12 +93,17 @@ export async function processFormulas( for (let i = 0; i < rows.length; i++) { let row = rows[i] let context = contextRows ? contextRows[i] : row - let formula = schema.formula rows[i] = { ...row, [column]: tracer.trace("processStringSync", {}, span => { span?.addTags({ table_id: table._id, column, static: isStatic }) - return processStringSync(formula, context) + const result = processStringSync(formula, context) + try { + return responseType ? coerce(result, responseType) : result + } catch (err) { + // if the coercion fails, we return empty row contents + return undefined + } }), } } @@ -117,12 +135,13 @@ export async function processAIColumns( continue } + const operation = schema.operation + const aiSchema: AIFieldMetadata = schema const rowUpdates = rows.map((row, i) => { const contextRow = contextRows ? contextRows[i] : row // Check if the type is bindable and pass through HBS if so - const operationField = - OperationFields[schema.operation as AIOperationEnum] + const operationField = OperationFields[operation as AIOperationEnum] for (const key in schema) { const fieldType = operationField[key as keyof typeof operationField] if (fieldType === OperationFieldTypeEnum.BINDABLE_TEXT) { @@ -131,7 +150,10 @@ export async function processAIColumns( } } - const prompt = llm.buildPromptFromAIOperation({ schema, row }) + const prompt = llm.buildPromptFromAIOperation({ + schema: aiSchema, + row, + }) return tracer.trace("processAIColumn", {}, async span => { span?.addTags({ table_id: table._id, column }) diff --git a/packages/types/src/documents/app/table/schema.ts b/packages/types/src/documents/app/table/schema.ts index 7e79902a49..00e119669b 100644 --- a/packages/types/src/documents/app/table/schema.ts +++ b/packages/types/src/documents/app/table/schema.ts @@ -115,6 +115,11 @@ export interface FormulaFieldMetadata extends BaseFieldSchema { type: FieldType.FORMULA formula: string formulaType?: FormulaType + responseType?: + | FieldType.STRING + | FieldType.NUMBER + | FieldType.BOOLEAN + | FieldType.DATETIME } export interface AIFieldMetadata extends BaseFieldSchema { From 4c7103e5fded3d89fe19c0a166bdb614701384c5 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 7 Nov 2024 17:01:30 +0000 Subject: [PATCH 06/13] Updating typing. --- packages/types/src/documents/app/row.ts | 6 ++++++ packages/types/src/documents/app/table/schema.ts | 8 ++------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/types/src/documents/app/row.ts b/packages/types/src/documents/app/row.ts index b0c5267b37..6b6b38a5cf 100644 --- a/packages/types/src/documents/app/row.ts +++ b/packages/types/src/documents/app/row.ts @@ -134,6 +134,12 @@ export const JsonTypes = [ FieldType.ARRAY, ] +export type FormulaResponseType = + | FieldType.STRING + | FieldType.NUMBER + | FieldType.BOOLEAN + | FieldType.DATETIME + export const NumericTypes = [FieldType.NUMBER, FieldType.BIGINT] export function isNumeric(type: FieldType) { diff --git a/packages/types/src/documents/app/table/schema.ts b/packages/types/src/documents/app/table/schema.ts index 00e119669b..771192e2f5 100644 --- a/packages/types/src/documents/app/table/schema.ts +++ b/packages/types/src/documents/app/table/schema.ts @@ -1,6 +1,6 @@ // all added by grid/table when defining the // column size, position and whether it can be viewed -import { FieldType } from "../row" +import { FieldType, FormulaResponseType } from "../row" import { AutoFieldSubType, AutoReason, @@ -115,11 +115,7 @@ export interface FormulaFieldMetadata extends BaseFieldSchema { type: FieldType.FORMULA formula: string formulaType?: FormulaType - responseType?: - | FieldType.STRING - | FieldType.NUMBER - | FieldType.BOOLEAN - | FieldType.DATETIME + responseType?: FormulaResponseType } export interface AIFieldMetadata extends BaseFieldSchema { From db3c6c36fcb853a8bef1afd3d5b0c94aed67c1cb Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 7 Nov 2024 17:01:42 +0000 Subject: [PATCH 07/13] Adding test cases for all formula response types. --- .../server/src/api/routes/tests/row.spec.ts | 108 +++++++++++++++--- 1 file changed, 92 insertions(+), 16 deletions(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index bf8f5a2a1c..10b30466f3 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -11,9 +11,9 @@ import emitter from "../../../../src/events" import { outputProcessing } from "../../../utilities/rowProcessor" import { context, + features, InternalTable, tenancy, - features, utils, } from "@budibase/backend-core" import { quotas } from "@budibase/pro" @@ -21,27 +21,28 @@ import { AIOperationEnum, AttachmentFieldMetadata, AutoFieldSubType, + BBReferenceFieldSubType, Datasource, DateFieldMetadata, DeleteRow, FieldSchema, FieldType, - BBReferenceFieldSubType, + FormulaResponseType, FormulaType, INTERNAL_TABLE_SOURCE_ID, + JsonFieldSubType, NumberFieldMetadata, QuotaUsageType, + RelationSchemaField, RelationshipType, Row, + RowExportFormat, SaveTableRequest, StaticQuotaName, Table, + TableSchema, TableSourceType, UpdatedRowEventEmitter, - TableSchema, - JsonFieldSubType, - RowExportFormat, - RelationSchemaField, } from "@budibase/types" import { generator, mocks } from "@budibase/backend-core/tests" import _, { merge } from "lodash" @@ -50,6 +51,7 @@ import { Knex } from "knex" import { InternalTables } from "../../../db/utils" import { withEnv } from "../../../environment" import { JsTimeoutError } from "@budibase/string-templates" +import { isDate } from "../../../utilities" jest.mock("@budibase/pro", () => ({ ...jest.requireActual("@budibase/pro"), @@ -89,6 +91,10 @@ async function waitForEvent( return await p } +function encodeJS(binding: string) { + return `{{ js "${Buffer.from(binding).toString("base64")}"}}` +} + describe.each([ ["lucene", undefined], ["sqs", undefined], @@ -3476,7 +3482,7 @@ describe.each([ describe("Formula fields", () => { let table: Table let otherTable: Table - let relatedRow: Row + let relatedRow: Row, mainRow: Row beforeAll(async () => { otherTable = await config.api.table.save(defaultTable()) @@ -3504,7 +3510,7 @@ describe.each([ name: generator.word(), description: generator.paragraph(), }) - await config.api.row.save(table._id!, { + mainRow = await config.api.row.save(table._id!, { name: generator.word(), description: generator.paragraph(), tableId: table._id!, @@ -3512,6 +3518,25 @@ describe.each([ }) }) + async function updateFormulaColumn( + formula: string, + opts?: { responseType?: FormulaResponseType; formulaType?: FormulaType } + ) { + table = await config.api.table.save({ + ...table, + schema: { + ...table.schema, + formula: { + name: "formula", + type: FieldType.FORMULA, + formula: formula, + responseType: opts?.responseType, + formulaType: opts?.formulaType || FormulaType.DYNAMIC, + }, + }, + }) + } + it("should be able to search for rows containing formulas", async () => { const { rows } = await config.api.row.search(table._id!) expect(rows.length).toBe(1) @@ -3519,12 +3544,63 @@ describe.each([ const row = rows[0] expect(row.formula).toBe(relatedRow.name) }) + + it("should coerce - number response type", async () => { + await updateFormulaColumn(encodeJS("return 1"), { + responseType: FieldType.NUMBER, + }) + const { rows } = await config.api.row.search(table._id!) + expect(rows[0].formula).toBe(1) + }) + + it("should coerce - boolean response type", async () => { + await updateFormulaColumn(encodeJS("return true"), { + responseType: FieldType.BOOLEAN, + }) + const { rows } = await config.api.row.search(table._id!) + expect(rows[0].formula).toBe(true) + }) + + it("should coerce - datetime response type", async () => { + await updateFormulaColumn(encodeJS("return new Date()"), { + responseType: FieldType.DATETIME, + }) + const { rows } = await config.api.row.search(table._id!) + expect(isDate(rows[0].formula)).toBe(true) + }) + + it("should coerce - datetime with invalid value", async () => { + await updateFormulaColumn(encodeJS("return 'a'"), { + responseType: FieldType.DATETIME, + }) + const { rows } = await config.api.row.search(table._id!) + expect(rows[0].formula).toBeUndefined() + }) + + it("should coerce handlebars", async () => { + await updateFormulaColumn("{{ add 1 1 }}", { + responseType: FieldType.NUMBER, + }) + const { rows } = await config.api.row.search(table._id!) + expect(rows[0].formula).toBe(2) + }) + + it("should coerce a static handlebars formula", async () => { + await updateFormulaColumn(encodeJS("return 1"), { + responseType: FieldType.NUMBER, + formulaType: FormulaType.STATIC, + }) + // save the row to store the static value + await config.api.row.save(table._id!, mainRow) + const { rows } = await config.api.row.search(table._id!) + expect(rows[0].formula).toBe(1) + }) }) describe("Formula JS protection", () => { it("should time out JS execution if a single cell takes too long", async () => { await withEnv({ JS_PER_INVOCATION_TIMEOUT_MS: 40 }, async () => { - const js = Buffer.from( + const js = encodeJS( ` let i = 0; while (true) { @@ -3532,7 +3608,7 @@ describe.each([ } return i; ` - ).toString("base64") + ) const table = await config.api.table.save( saveTableRequest({ @@ -3544,7 +3620,7 @@ describe.each([ formula: { name: "formula", type: FieldType.FORMULA, - formula: `{{ js "${js}"}}`, + formula: js, formulaType: FormulaType.DYNAMIC, }, }, @@ -3567,7 +3643,7 @@ describe.each([ JS_PER_REQUEST_TIMEOUT_MS: 80, }, async () => { - const js = Buffer.from( + const js = encodeJS( ` let i = 0; while (true) { @@ -3575,7 +3651,7 @@ describe.each([ } return i; ` - ).toString("base64") + ) const table = await config.api.table.save( saveTableRequest({ @@ -3587,7 +3663,7 @@ describe.each([ formula: { name: "formula", type: FieldType.FORMULA, - formula: `{{ js "${js}"}}`, + formula: js, formulaType: FormulaType.DYNAMIC, }, }, @@ -3629,7 +3705,7 @@ describe.each([ }) it("should not carry over context between formulas", async () => { - const js = Buffer.from(`return $("[text]");`).toString("base64") + const js = encodeJS(`return $("[text]");`) const table = await config.api.table.save( saveTableRequest({ schema: { @@ -3640,7 +3716,7 @@ describe.each([ formula: { name: "formula", type: FieldType.FORMULA, - formula: `{{ js "${js}"}}`, + formula: js, formulaType: FormulaType.DYNAMIC, }, }, From 4076e98308671b37129a0c3ffc810f2b00a6a1a5 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 8 Nov 2024 16:01:05 +0000 Subject: [PATCH 08/13] Disable static formula tests in external DBs. --- .../server/src/api/routes/tests/row.spec.ts | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 10b30466f3..36428cf8f0 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -3585,16 +3585,17 @@ describe.each([ expect(rows[0].formula).toBe(2) }) - it("should coerce a static handlebars formula", async () => { - await updateFormulaColumn(encodeJS("return 1"), { - responseType: FieldType.NUMBER, - formulaType: FormulaType.STATIC, + isInternal && + it("should coerce a static handlebars formula", async () => { + await updateFormulaColumn(encodeJS("return 1"), { + responseType: FieldType.NUMBER, + formulaType: FormulaType.STATIC, + }) + // save the row to store the static value + await config.api.row.save(table._id!, mainRow) + const { rows } = await config.api.row.search(table._id!) + expect(rows[0].formula).toBe(1) }) - // save the row to store the static value - await config.api.row.save(table._id!, mainRow) - const { rows } = await config.api.row.search(table._id!) - expect(rows[0].formula).toBe(1) - }) }) describe("Formula JS protection", () => { From 4750e9faca0a3dd2796900b88520b4b08888a0f6 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 18 Nov 2024 13:18:45 +0100 Subject: [PATCH 09/13] Move atrament dependency --- packages/bbui/package.json | 1 + packages/client/package.json | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/bbui/package.json b/packages/bbui/package.json index 0830f8ab6f..aeb7418526 100644 --- a/packages/bbui/package.json +++ b/packages/bbui/package.json @@ -81,6 +81,7 @@ "@spectrum-css/typography": "3.0.1", "@spectrum-css/underlay": "2.0.9", "@spectrum-css/vars": "3.0.1", + "atrament": "^4.3.0", "dayjs": "^1.10.8", "easymde": "^2.16.1", "svelte-dnd-action": "^0.9.8", diff --git a/packages/client/package.json b/packages/client/package.json index d3b0f098cb..fb9851e0a1 100644 --- a/packages/client/package.json +++ b/packages/client/package.json @@ -33,8 +33,7 @@ "sanitize-html": "^2.13.0", "screenfull": "^6.0.1", "shortid": "^2.2.15", - "svelte-spa-router": "^4.0.1", - "atrament": "^4.3.0" + "svelte-spa-router": "^4.0.1" }, "devDependencies": { "@rollup/plugin-alias": "^5.1.0", From dd7259219bd352239cae23768bc89047eca12ba1 Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Mon, 18 Nov 2024 15:03:59 +0000 Subject: [PATCH 10/13] Bump version to 3.2.4 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 582f95b303..a214415508 100644 --- a/lerna.json +++ b/lerna.json @@ -1,6 +1,6 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", - "version": "3.2.3", + "version": "3.2.4", "npmClient": "yarn", "packages": [ "packages/*", From 149579cf5c6f14461be93be8cc7bbdedd7e69a19 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 18 Nov 2024 15:39:55 +0000 Subject: [PATCH 11/13] PR comments. --- .../backend/DataTable/modals/CreateEditColumn.svelte | 4 ++-- packages/server/src/api/routes/tests/row.spec.ts | 8 ++++++++ packages/server/src/utilities/rowProcessor/utils.ts | 3 ++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte b/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte index 96397490f8..271a82f16b 100644 --- a/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte +++ b/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte @@ -387,7 +387,7 @@ editableColumn.relationshipType = RelationshipType.MANY_TO_MANY } else if (editableColumn.type === FieldType.FORMULA) { editableColumn.formulaType = "dynamic" - editableColumn.responseType = FIELDS.STRING.type + editableColumn.responseType = field.responseType || FIELDS.STRING.type } } @@ -784,7 +784,7 @@ ]} getOptionLabel={option => option.name} getOptionValue={option => option.type} - tooltip="Formulas by default will return a string - however if you need a native type the response can be coerced." + tooltip="Formulas by default will return a string - however if you need a another type the response can be coerced." /> diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 9ea2b5572e..5bdd341beb 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -3308,6 +3308,14 @@ datasourceDescribe( expect(rows[0].formula).toBe(2) }) + it("should coerce handlebars to string (default)", async () => { + await updateFormulaColumn("{{ add 1 1 }}", { + responseType: FieldType.STRING, + }) + const { rows } = await config.api.row.search(table._id!) + expect(rows[0].formula).toBe("2") + }) + isInternal && it("should coerce a static handlebars formula", async () => { await updateFormulaColumn(encodeJS("return 1"), { diff --git a/packages/server/src/utilities/rowProcessor/utils.ts b/packages/server/src/utilities/rowProcessor/utils.ts index 15c0612fae..9dbeb8ebb2 100644 --- a/packages/server/src/utilities/rowProcessor/utils.ts +++ b/packages/server/src/utilities/rowProcessor/utils.ts @@ -100,8 +100,9 @@ export async function processFormulas( const result = processStringSync(formula, context) try { return responseType ? coerce(result, responseType) : result - } catch (err) { + } catch (err: any) { // if the coercion fails, we return empty row contents + span?.addTags({ coercionError: err.message }) return undefined } }), From a5a48889b02daf2d4c12e1de586552dbe000aa0c Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Mon, 18 Nov 2024 15:41:46 +0000 Subject: [PATCH 12/13] Bump version to 3.2.5 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index a214415508..e5b0013535 100644 --- a/lerna.json +++ b/lerna.json @@ -1,6 +1,6 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", - "version": "3.2.4", + "version": "3.2.5", "npmClient": "yarn", "packages": [ "packages/*", From 0053072d4f6f164f894b91181624368d89f402a0 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 18 Nov 2024 16:05:50 +0000 Subject: [PATCH 13/13] PR comment. --- .../components/backend/DataTable/modals/CreateEditColumn.svelte | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte b/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte index 271a82f16b..17069aa8c6 100644 --- a/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte +++ b/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte @@ -784,7 +784,7 @@ ]} getOptionLabel={option => option.name} getOptionValue={option => option.type} - tooltip="Formulas by default will return a string - however if you need a another type the response can be coerced." + tooltip="Formulas by default will return a string - however if you need another type the response can be coerced." />