From 8bf79c5f107f75c4de1bf28b12d120ffe1de4c17 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 9 Feb 2024 18:04:40 +0000 Subject: [PATCH 01/25] Adding some basic cron validation to publishing, currently the error is not explained if it is hit - still need some frontend for this, but this now means that an error is provided to users when attempting to publish, and we can re-use this validation in the automation UI. Need to have both backend and frontend validation as invalid CRONs will already exist, backend makes sure these are error'd on. --- .../src/components/deploy/AppActions.svelte | 8 +++- packages/server/src/automations/utils.ts | 10 +++- packages/shared-core/package.json | 3 +- packages/shared-core/src/helpers/cron.ts | 47 +++++++++++++++++++ packages/shared-core/src/helpers/index.ts | 1 + packages/shared-core/src/tests/cron.test.ts | 22 +++++++++ yarn.lock | 36 +++++++++++++- 7 files changed, 122 insertions(+), 5 deletions(-) create mode 100644 packages/shared-core/src/helpers/cron.ts create mode 100644 packages/shared-core/src/tests/cron.test.ts diff --git a/packages/builder/src/components/deploy/AppActions.svelte b/packages/builder/src/components/deploy/AppActions.svelte index 7d14fd0e87..30f86a79d1 100644 --- a/packages/builder/src/components/deploy/AppActions.svelte +++ b/packages/builder/src/components/deploy/AppActions.svelte @@ -101,7 +101,13 @@ } catch (error) { console.error(error) analytics.captureException(error) - notifications.error("Error publishing app") + const baseMsg = "Error publishing app" + const message = error.message + if (message) { + notifications.error(`${baseMsg} - ${message}`) + } else { + notifications.error(baseMsg) + } } publishing = false } diff --git a/packages/server/src/automations/utils.ts b/packages/server/src/automations/utils.ts index 0c28787f67..b1f463e363 100644 --- a/packages/server/src/automations/utils.ts +++ b/packages/server/src/automations/utils.ts @@ -16,6 +16,7 @@ import { } from "@budibase/types" import sdk from "../sdk" import { automationsEnabled } from "../features" +import { helpers } from "@budibase/shared-core" import tracer from "dd-trace" const REBOOT_CRON = "@reboot" @@ -198,6 +199,13 @@ export async function enableCronTrigger(appId: any, automation: Automation) { !isRebootTrigger(automation) && trigger?.inputs.cron ) { + const cronExp = trigger.inputs.cron + const validation = helpers.cron.validate(cronExp) + if (!validation.valid) { + throw new Error( + `Invalid automation CRON "${cronExp}" - ${validation.err.join(", ")}` + ) + } // make a job id rather than letting Bull decide, makes it easier to handle on way out const jobId = `${appId}_cron_${newid()}` const job: any = await automationQueue.add( @@ -205,7 +213,7 @@ export async function enableCronTrigger(appId: any, automation: Automation) { automation, event: { appId, timestamp: Date.now() }, }, - { repeat: { cron: trigger.inputs.cron }, jobId } + { repeat: { cron: cronExp }, jobId } ) // Assign cron job ID from bull so we can remove it later if the cron trigger is removed trigger.cronJobId = job.id diff --git a/packages/shared-core/package.json b/packages/shared-core/package.json index edbbd1dc56..12a94f5a2f 100644 --- a/packages/shared-core/package.json +++ b/packages/shared-core/package.json @@ -14,7 +14,8 @@ "check:types": "tsc -p tsconfig.json --noEmit --paths null" }, "dependencies": { - "@budibase/types": "0.0.0" + "@budibase/types": "0.0.0", + "cron-validate": "^1.4.5" }, "devDependencies": { "rimraf": "3.0.2", diff --git a/packages/shared-core/src/helpers/cron.ts b/packages/shared-core/src/helpers/cron.ts new file mode 100644 index 0000000000..e83738f7cd --- /dev/null +++ b/packages/shared-core/src/helpers/cron.ts @@ -0,0 +1,47 @@ +import cronValidate from "cron-validate" + +const INPUT_CRON_START = "(Input cron: " +const ERROR_SWAPS = { + "smaller than lower limit": "less than", + "bigger than upper limit": "greater than", + daysOfMonth: "'days of the month'", + daysOfWeek: "'days of the week'", + years: "'years'", + months: "'months'", + hours: "'hours'", + minutes: "'minutes'", + seconds: "'seconds'", +} + +function improveErrors(errors: string[]): string[] { + const finalErrors: string[] = [] + + for (let error of errors) { + if (error.includes(INPUT_CRON_START)) { + error = error.split(INPUT_CRON_START)[0].trim() + } + for (let [key, value] of Object.entries(ERROR_SWAPS)) { + if (error.includes(key)) { + error = error.replace(new RegExp(key, "g"), value) + } + } + finalErrors.push(error) + } + return finalErrors +} + +export function validate( + cronExpression: string +): { valid: false; err: string[] } | { valid: true } { + const result = cronValidate(cronExpression, { + preset: "npm-node-cron", + override: { + useSeconds: false, + }, + }) + if (!result.isValid()) { + return { valid: false, err: improveErrors(result.getError()) } + } else { + return { valid: true } + } +} diff --git a/packages/shared-core/src/helpers/index.ts b/packages/shared-core/src/helpers/index.ts index fd185aa1e9..e76022b14b 100644 --- a/packages/shared-core/src/helpers/index.ts +++ b/packages/shared-core/src/helpers/index.ts @@ -1,2 +1,3 @@ export * from "./helpers" export * from "./integrations" +export * as cron from "./cron" diff --git a/packages/shared-core/src/tests/cron.test.ts b/packages/shared-core/src/tests/cron.test.ts new file mode 100644 index 0000000000..d56165b2b8 --- /dev/null +++ b/packages/shared-core/src/tests/cron.test.ts @@ -0,0 +1,22 @@ +import { expect, describe, it } from "vitest" +import { cron } from "../helpers" + +describe("check valid and invalid crons", () => { + it("invalid - 0 0 0 11 *", () => { + expect(cron.validate("0 0 0 11 *")).toStrictEqual({ + valid: false, + err: [expect.stringContaining("less than '1'")], + }) + }) + + it("invalid - 5 4 32 1 1", () => { + expect(cron.validate("5 4 32 1 1")).toStrictEqual({ + valid: false, + err: [expect.stringContaining("greater than '31'")], + }) + }) + + it("valid - * * * * *", () => { + expect(cron.validate("* * * * *")).toStrictEqual({ valid: true }) + }) +}) diff --git a/yarn.lock b/yarn.lock index 9e12ecad89..defee1e6e1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1980,6 +1980,13 @@ resolved "https://registry.yarnpkg.com/@babel/regjsgen/-/regjsgen-0.8.0.tgz#f0ba69b075e1f05fb2825b7fad991e7adbb18310" integrity sha512-x/rqGMdzj+fWZvCOYForTghzbtqPDZ5gPwaoNGHdgDfF2QA/XZbCBp4Moo5scrkAMPhB7z26XM/AaHuIJdgauA== +"@babel/runtime@^7.10.5": + version "7.23.9" + resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.23.9.tgz#47791a15e4603bb5f905bc0753801cf21d6345f7" + integrity sha512-0CX6F+BI2s9dkUqr08KFrAIZgNFj75rdBU/DjCyYLIaV/quFjkk6T+EJ2LkZHyZTbEV4L5p97mNkUsHl2wLFAw== + dependencies: + regenerator-runtime "^0.14.0" + "@babel/runtime@^7.12.5", "@babel/runtime@^7.13.10", "@babel/runtime@^7.15.4", "@babel/runtime@^7.21.0", "@babel/runtime@^7.8.4", "@babel/runtime@^7.9.2": version "7.23.8" resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.23.8.tgz#8ee6fe1ac47add7122902f257b8ddf55c898f650" @@ -5431,6 +5438,11 @@ resolved "https://registry.yarnpkg.com/@types/lodash/-/lodash-4.14.200.tgz#435b6035c7eba9cdf1e039af8212c9e9281e7149" integrity sha512-YI/M/4HRImtNf3pJgbF+W6FrXovqj+T+/HpENLTooK9PnkacBsDpeP3IpHab40CClUfhNmdM2WTNP2sa2dni5Q== +"@types/lodash@^4.14.165": + version "4.14.202" + resolved "https://registry.yarnpkg.com/@types/lodash/-/lodash-4.14.202.tgz#f09dbd2fb082d507178b2f2a5c7e74bd72ff98f8" + integrity sha512-OvlIYQK9tNneDlS0VN54LLd5uiPCBOp7gS5Z0f1mjoJYBrtStzgmJBxONW3U6OZqdtNzZPmn9BS/7WI7BFFcFQ== + "@types/long@^4.0.0": version "4.0.2" resolved "https://registry.yarnpkg.com/@types/long/-/long-4.0.2.tgz#b74129719fc8d11c01868010082d483b7545591a" @@ -8541,6 +8553,13 @@ cron-parser@^4.2.1: dependencies: luxon "^3.2.1" +cron-validate@^1.4.5: + version "1.4.5" + resolved "https://registry.yarnpkg.com/cron-validate/-/cron-validate-1.4.5.tgz#eceb221f7558e6302e5f84c7b3a454fdf4d064c3" + integrity sha512-nKlOJEnYKudMn/aNyNH8xxWczlfpaazfWV32Pcx/2St51r2bxWbGhZD7uwzMcRhunA/ZNL+Htm/i0792Z59UMQ== + dependencies: + yup "0.32.9" + cross-spawn@^6.0.0: version "6.0.5" resolved "https://registry.yarnpkg.com/cross-spawn/-/cross-spawn-6.0.5.tgz#4a5ec7c64dfae22c3a14124dbacdee846d80cbc4" @@ -14440,7 +14459,7 @@ lock@^1.1.0: resolved "https://registry.yarnpkg.com/lock/-/lock-1.1.0.tgz#53157499d1653b136ca66451071fca615703fa55" integrity sha512-NZQIJJL5Rb9lMJ0Yl1JoVr9GSdo4HTPsUEWsSFzB8dE8DSoiLCVavWZPi7Rnlv/o73u6I24S/XYc/NmG4l8EKA== -lodash-es@^4.17.21: +lodash-es@^4.17.15, lodash-es@^4.17.21: version "4.17.21" resolved "https://registry.yarnpkg.com/lodash-es/-/lodash-es-4.17.21.tgz#43e626c46e6591b7750beb2b50117390c609e3ee" integrity sha512-mKnC+QJ9pWVzv+C4/U3rRsHapFfHvQFoFB92e52xeyGMcX6/OlIl78je1u8vePzYZSkkogMPJ2yjxxsb89cxyw== @@ -14590,7 +14609,7 @@ lodash.xor@^4.5.0: resolved "https://registry.yarnpkg.com/lodash.xor/-/lodash.xor-4.5.0.tgz#4d48ed7e98095b0632582ba714d3ff8ae8fb1db6" integrity sha512-sVN2zimthq7aZ5sPGXnSz32rZPuqcparVW50chJQe+mzTYV+IsxSsl/2gnkWWE2Of7K3myBQBqtLKOUEHJKRsQ== -lodash@4.17.21, lodash@^4.17.11, lodash@^4.17.14, lodash@^4.17.15, lodash@^4.17.19, lodash@^4.17.21, lodash@^4.17.3, lodash@^4.7.0: +lodash@4.17.21, lodash@^4.17.11, lodash@^4.17.14, lodash@^4.17.15, lodash@^4.17.19, lodash@^4.17.20, lodash@^4.17.21, lodash@^4.17.3, lodash@^4.7.0: version "4.17.21" resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.21.tgz#679591c564c3bffaae8454cf0b3df370c3d6911c" integrity sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg== @@ -22020,6 +22039,19 @@ yocto-queue@^1.0.0: resolved "https://registry.yarnpkg.com/yocto-queue/-/yocto-queue-1.0.0.tgz#7f816433fb2cbc511ec8bf7d263c3b58a1a3c251" integrity sha512-9bnSc/HEW2uRy67wc+T8UwauLuPJVn28jb+GtJY16iiKWyvmYJRXVT4UamsAEGQfPohgr2q4Tq0sQbQlxTfi1g== +yup@0.32.9: + version "0.32.9" + resolved "https://registry.yarnpkg.com/yup/-/yup-0.32.9.tgz#9367bec6b1b0e39211ecbca598702e106019d872" + integrity sha512-Ci1qN+i2H0XpY7syDQ0k5zKQ/DoxO0LzPg8PAR/X4Mpj6DqaeCoIYEEjDJwhArh3Fa7GWbQQVDZKeXYlSH4JMg== + dependencies: + "@babel/runtime" "^7.10.5" + "@types/lodash" "^4.14.165" + lodash "^4.17.20" + lodash-es "^4.17.15" + nanoclone "^0.2.1" + property-expr "^2.0.4" + toposort "^2.0.2" + yup@^0.32.11: version "0.32.11" resolved "https://registry.yarnpkg.com/yup/-/yup-0.32.11.tgz#d67fb83eefa4698607982e63f7ca4c5ed3cf18c5" From c18a3d4abb92c0b33221c0dc52969ee642fccfd1 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 9 Feb 2024 15:27:31 +0100 Subject: [PATCH 02/25] Add creation tests --- .../server/src/api/controllers/table/index.ts | 2 - .../server/src/api/routes/tests/table.spec.ts | 47 +++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/controllers/table/index.ts b/packages/server/src/api/controllers/table/index.ts index db2bd672d0..40fc2aedb4 100644 --- a/packages/server/src/api/controllers/table/index.ts +++ b/packages/server/src/api/controllers/table/index.ts @@ -14,7 +14,6 @@ import { events } from "@budibase/backend-core" import { BulkImportRequest, BulkImportResponse, - DocumentType, FetchTablesResponse, MigrateRequest, MigrateResponse, @@ -25,7 +24,6 @@ import { TableResponse, TableSourceType, UserCtx, - SEPARATOR, } from "@budibase/types" import sdk from "../../../sdk" import { jsonFromCsvString } from "../../../utilities/csv" diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index 62efdda448..4da4e25ace 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -18,6 +18,12 @@ import * as setup from "./utilities" import sdk from "../../../sdk" import * as uuid from "uuid" +import tk from "timekeeper" +import { mocks } from "@budibase/backend-core/tests" +import { TableToBuild } from "src/tests/utilities/TestConfiguration" + +tk.freeze(mocks.date.MOCK_DATE) + const { basicTable } = setup.structures describe("/tables", () => { @@ -60,6 +66,47 @@ describe("/tables", () => { expect(events.table.created).toBeCalledWith(res.body) }) + it("creates all the passed fields", async () => { + const tableData: TableToBuild = { + name: "TestTable", + type: "table", + schema: { + autoId: { + name: "id", + type: FieldType.NUMBER, + subtype: AutoFieldSubType.AUTO_ID, + autocolumn: true, + constraints: { + type: "number", + presence: false, + }, + }, + }, + views: { + view1: { + id: "viewId", + version: 2, + name: "table view", + tableId: "tableId", + }, + }, + } + const testTable = await config.createTable(tableData) + + const expected: Table = { + ...tableData, + type: "table", + sourceType: TableSourceType.INTERNAL, + sourceId: expect.any(String), + _rev: expect.stringMatching(/^1-.+/), + updatedAt: mocks.date.MOCK_DATE.toISOString(), + } + expect(testTable).toEqual(expected) + + const persistedTable = await config.api.table.get(testTable._id!) + expect(persistedTable).toEqual(expected) + }) + it("creates a table via data import", async () => { const table: SaveTableRequest = basicTable() table.rows = [{ name: "test-name", description: "test-desc" }] From f1b31b4119486dfe42e038850f548e6f1fd9a553 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 9 Feb 2024 15:27:48 +0100 Subject: [PATCH 03/25] Export type --- packages/server/src/tests/utilities/TestConfiguration.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index d96655af43..53a4e3432c 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -84,7 +84,7 @@ type DefaultUserValues = { csrfToken: string } -interface TableToBuild extends Omit { +export interface TableToBuild extends Omit { sourceId?: string sourceType?: TableSourceType } From f1a75b84b4c268906ce32d03c2ebbfa5fe5112b1 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 9 Feb 2024 16:20:56 +0100 Subject: [PATCH 04/25] Add test --- .../server/src/api/routes/tests/table.spec.ts | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index 4da4e25ace..7fb4c0673a 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -199,6 +199,56 @@ describe("/tables", () => { expect(res.body.name).toBeUndefined() }) + it("updates only the passed fields", async () => { + const testTable = await config.createTable({ + name: "TestTable", + type: "table", + schema: { + autoId: { + name: "id", + type: FieldType.NUMBER, + subtype: AutoFieldSubType.AUTO_ID, + autocolumn: true, + constraints: { + type: "number", + presence: false, + }, + }, + }, + views: { + view1: { + id: "viewId", + version: 2, + name: "table view", + tableId: "tableId", + }, + }, + }) + + const response = await request + .post(`/api/tables`) + .send({ + ...testTable, + name: "UpdatedName", + }) + .set(config.defaultHeaders()) + .expect("Content-Type", /json/) + .expect(200) + + expect(response.body).toEqual({ + ...testTable, + name: "UpdatedName", + _rev: expect.stringMatching(/^2-.+/), + }) + + const persistedTable = await config.api.table.get(testTable._id!) + expect(persistedTable).toEqual({ + ...testTable, + name: "UpdatedName", + _rev: expect.stringMatching(/^2-.+/), + }) + }) + describe("user table", () => { it("should add roleId and email field when adjusting user table schema", async () => { const res = await request From 2c26b55a7c3fc87a0ea39c7d614043a34d4ee20c Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 12 Feb 2024 11:59:05 +0100 Subject: [PATCH 05/25] Handle view creation on new table requests --- .../server/src/api/controllers/table/index.ts | 3 ++- .../src/api/controllers/table/internal.ts | 7 +++--- .../server/src/api/routes/tests/table.spec.ts | 23 ++++++++++++++++++- .../src/sdk/app/tables/internal/index.ts | 12 ++++++---- 4 files changed, 35 insertions(+), 10 deletions(-) diff --git a/packages/server/src/api/controllers/table/index.ts b/packages/server/src/api/controllers/table/index.ts index 40fc2aedb4..55a896373f 100644 --- a/packages/server/src/api/controllers/table/index.ts +++ b/packages/server/src/api/controllers/table/index.ts @@ -75,9 +75,10 @@ export async function save(ctx: UserCtx) { const table = ctx.request.body const isImport = table.rows - const savedTable = await pickApi({ table }).save(ctx) + let savedTable = await pickApi({ table }).save(ctx) if (!table._id) { await events.table.created(savedTable) + savedTable = sdk.tables.enrichViewSchemas(savedTable) } else { await events.table.updated(savedTable) } diff --git a/packages/server/src/api/controllers/table/internal.ts b/packages/server/src/api/controllers/table/internal.ts index bb94f2bc01..34bc78b243 100644 --- a/packages/server/src/api/controllers/table/internal.ts +++ b/packages/server/src/api/controllers/table/internal.ts @@ -18,10 +18,11 @@ export async function save(ctx: UserCtx) { _rename?: RenameColumn } = { _id: generateTableID(), - ...rest, - type: "table", - sourceType: TableSourceType.INTERNAL, views: {}, + ...rest, + // Ensure these fields are populated, even if not sent in the request + type: rest.type || "table", + sourceType: rest.sourceType || TableSourceType.INTERNAL, } const renaming = tableToSave._rename delete tableToSave._rename diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index 7fb4c0673a..b5d0107981 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 { FieldType, INTERNAL_TABLE_SOURCE_ID, InternalTable, + NumberFieldMetadata, RelationshipType, Row, SaveTableRequest, @@ -83,7 +84,7 @@ describe("/tables", () => { }, }, views: { - view1: { + "table view": { id: "viewId", version: 2, name: "table view", @@ -96,9 +97,29 @@ describe("/tables", () => { const expected: Table = { ...tableData, type: "table", + views: { + "table view": { + ...tableData.views!["table view"], + schema: { + autoId: { + autocolumn: true, + constraints: { + presence: false, + type: "number", + }, + name: "id", + type: FieldType.NUMBER, + subtype: AutoFieldSubType.AUTO_ID, + visible: false, + } as NumberFieldMetadata, + }, + }, + }, sourceType: TableSourceType.INTERNAL, sourceId: expect.any(String), _rev: expect.stringMatching(/^1-.+/), + _id: expect.any(String), + createdAt: mocks.date.MOCK_DATE.toISOString(), updatedAt: mocks.date.MOCK_DATE.toISOString(), } expect(testTable).toEqual(expected) diff --git a/packages/server/src/sdk/app/tables/internal/index.ts b/packages/server/src/sdk/app/tables/internal/index.ts index 25fe145484..5d9feb5fe8 100644 --- a/packages/server/src/sdk/app/tables/internal/index.ts +++ b/packages/server/src/sdk/app/tables/internal/index.ts @@ -75,11 +75,13 @@ export async function save( if (!tableView) continue if (viewsSdk.isV2(tableView)) { - table.views[view] = viewsSdk.syncSchema( - oldTable!.views![view] as ViewV2, - table.schema, - renaming - ) + if (oldTable?.views && oldTable.views[view]) { + table.views[view] = viewsSdk.syncSchema( + oldTable.views[view] as ViewV2, + table.schema, + renaming + ) + } continue } From ffdfb731fb89fbad8f1b9e1925ece06fbbf4309f Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 12 Feb 2024 12:25:56 +0100 Subject: [PATCH 06/25] Fix tests --- .../server/src/api/routes/tests/table.spec.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index b5d0107981..efa9845fcb 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -348,6 +348,7 @@ describe("/tables", () => { describe("fetch", () => { let testTable: Table + const enrichViewSchemasMock = jest.spyOn(sdk.tables, "enrichViewSchemas") beforeEach(async () => { testTable = await config.createTable(testTable) @@ -357,6 +358,10 @@ describe("/tables", () => { delete testTable._rev }) + afterAll(() => { + enrichViewSchemasMock.mockRestore() + }) + it("returns all the tables for that instance in the response body", async () => { const res = await request .get(`/api/tables`) @@ -405,7 +410,7 @@ describe("/tables", () => { it("should enrich the view schemas for viewsV2", async () => { const tableId = config.table!._id! - jest.spyOn(sdk.tables, "enrichViewSchemas").mockImplementation(t => ({ + enrichViewSchemasMock.mockImplementation(t => ({ ...t, views: { view1: { @@ -413,7 +418,7 @@ describe("/tables", () => { name: "view1", schema: {}, id: "new_view_id", - tableId, + tableId: t._id!, }, }, })) @@ -480,11 +485,7 @@ describe("/tables", () => { let testTable: Table beforeEach(async () => { - testTable = await config.createTable(testTable) - }) - - afterEach(() => { - delete testTable._rev + testTable = await config.createTable() }) it("returns a success response when a table is deleted.", async () => { From 231c8ccaabb5d4f33fa095f72abfa1d432591beb Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 12 Feb 2024 12:33:16 +0100 Subject: [PATCH 07/25] Make code more readable --- packages/server/src/api/controllers/table/internal.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/controllers/table/internal.ts b/packages/server/src/api/controllers/table/internal.ts index 34bc78b243..8e90007d88 100644 --- a/packages/server/src/api/controllers/table/internal.ts +++ b/packages/server/src/api/controllers/table/internal.ts @@ -18,12 +18,16 @@ export async function save(ctx: UserCtx) { _rename?: RenameColumn } = { _id: generateTableID(), - views: {}, ...rest, // Ensure these fields are populated, even if not sent in the request type: rest.type || "table", sourceType: rest.sourceType || TableSourceType.INTERNAL, } + + if (!tableToSave.views) { + tableToSave.views = {} + } + const renaming = tableToSave._rename delete tableToSave._rename From 8651a836a5f0094c90f4aafa4aa2edad65651608 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 12 Feb 2024 12:34:39 +0100 Subject: [PATCH 08/25] Fix exports --- packages/server/src/tests/utilities/TestConfiguration.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index 53a4e3432c..90d50e2816 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -89,7 +89,7 @@ export interface TableToBuild extends Omit { sourceType?: TableSourceType } -class TestConfiguration { +export default class TestConfiguration { server: any request: supertest.SuperTest | undefined started: boolean @@ -911,5 +911,3 @@ class TestConfiguration { return await this._req(config, null, layoutController.save) } } - -export = TestConfiguration From 6cdfd4b621a23e433dec500d4fc599a87da57ac7 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 12 Feb 2024 12:36:29 +0100 Subject: [PATCH 09/25] Lint --- packages/server/src/api/routes/tests/table.spec.ts | 2 +- 1 file changed, 1 insertion(+), 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 efa9845fcb..c8cb3ef21b 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -21,7 +21,7 @@ import * as uuid from "uuid" import tk from "timekeeper" import { mocks } from "@budibase/backend-core/tests" -import { TableToBuild } from "src/tests/utilities/TestConfiguration" +import { TableToBuild } from "../../../tests/utilities/TestConfiguration" tk.freeze(mocks.date.MOCK_DATE) From 3ee555e72af8986caf683e68ed1221469c1c3dd5 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 12 Feb 2024 12:50:23 +0100 Subject: [PATCH 10/25] Fix js tests --- packages/server/src/tests/utilities/TestConfiguration.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index 90d50e2816..ea3204536a 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -911,3 +911,5 @@ export default class TestConfiguration { return await this._req(config, null, layoutController.save) } } + +module.exports = TestConfiguration From b27ca57e1aabfac5c9bd4d585ea2775a7b89f179 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 12 Feb 2024 14:00:32 +0100 Subject: [PATCH 11/25] Allow loging js execution errors --- packages/string-templates/src/helpers/javascript.js | 5 +++++ packages/string-templates/src/index.js | 1 + packages/string-templates/src/index.mjs | 1 + 3 files changed, 7 insertions(+) diff --git a/packages/string-templates/src/helpers/javascript.js b/packages/string-templates/src/helpers/javascript.js index 99d7df10f7..4a7f602690 100644 --- a/packages/string-templates/src/helpers/javascript.js +++ b/packages/string-templates/src/helpers/javascript.js @@ -8,6 +8,9 @@ const { getJsHelperList } = require("./list") let runJS module.exports.setJSRunner = runner => (runJS = runner) +let onErrorLog +module.exports.setOnErrorLog = delegate => (onErrorLog = delegate) + // Helper utility to strip square brackets from a value const removeSquareBrackets = value => { if (!value || typeof value !== "string") { @@ -56,6 +59,8 @@ module.exports.processJS = (handlebars, context) => { const res = { data: runJS(js, sandboxContext) } return `{{${LITERAL_MARKER} js_result-${JSON.stringify(res)}}}` } catch (error) { + onErrorLog && onErrorLog(error) + if (error.code === "ERR_SCRIPT_EXECUTION_TIMEOUT") { return "Timed out while executing JS" } diff --git a/packages/string-templates/src/index.js b/packages/string-templates/src/index.js index 3636c0a456..f370b67272 100644 --- a/packages/string-templates/src/index.js +++ b/packages/string-templates/src/index.js @@ -365,6 +365,7 @@ module.exports.doesContainString = (template, string) => { } module.exports.setJSRunner = javascript.setJSRunner +module.exports.setOnErrorLog = javascript.setOnErrorLog module.exports.convertToJS = hbs => { const blocks = exports.findHBSBlocks(hbs) diff --git a/packages/string-templates/src/index.mjs b/packages/string-templates/src/index.mjs index bdded04b02..5ac7981fee 100644 --- a/packages/string-templates/src/index.mjs +++ b/packages/string-templates/src/index.mjs @@ -20,6 +20,7 @@ export const disableEscaping = templates.disableEscaping export const findHBSBlocks = templates.findHBSBlocks export const convertToJS = templates.convertToJS export const setJSRunner = templates.setJSRunner +export const setOnErrorLog = templates.setOnErrorLog export const FIND_ANY_HBS_REGEX = templates.FIND_ANY_HBS_REGEX export const helpersToRemoveForJs = templates.helpersToRemoveForJs From 93eb9fc9c8e475ab028db20caf54875578d41811 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 12 Feb 2024 14:01:00 +0100 Subject: [PATCH 12/25] Setup error logging --- packages/server/src/environment.ts | 1 + packages/server/src/jsRunner/index.ts | 10 ++++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/server/src/environment.ts b/packages/server/src/environment.ts index ba3aa280e2..fd4c3d205b 100644 --- a/packages/server/src/environment.ts +++ b/packages/server/src/environment.ts @@ -97,6 +97,7 @@ const environment = { APP_MIGRATION_TIMEOUT: parseIntSafe(process.env.APP_MIGRATION_TIMEOUT), JS_RUNNER_MEMORY_LIMIT: parseIntSafe(process.env.JS_RUNNER_MEMORY_LIMIT) || 64, + LOG_JS_ERRORS: process.env.LOG_JS_ERRORS, } // threading can cause memory issues with node-ts in development diff --git a/packages/server/src/jsRunner/index.ts b/packages/server/src/jsRunner/index.ts index 1936b0ef45..38cd0dd0d6 100644 --- a/packages/server/src/jsRunner/index.ts +++ b/packages/server/src/jsRunner/index.ts @@ -1,7 +1,7 @@ import vm from "vm" import env from "../environment" -import { setJSRunner } from "@budibase/string-templates" -import { context, timers } from "@budibase/backend-core" +import { setJSRunner, setOnErrorLog } from "@budibase/string-templates" +import { context, logging, timers } from "@budibase/backend-core" import tracer from "dd-trace" type TrackerFn = (f: () => T) => T @@ -58,4 +58,10 @@ export function init() { ) }) }) + + if (env.LOG_JS_ERRORS) { + setOnErrorLog((error: Error) => { + logging.logWarn(JSON.stringify(error)) + }) + } } From 5546b8ffe6a5007ae9fee586a2bd0e30a373f4b9 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 12 Feb 2024 13:28:12 +0000 Subject: [PATCH 13/25] PR comments. --- packages/shared-core/src/helpers/cron.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/shared-core/src/helpers/cron.ts b/packages/shared-core/src/helpers/cron.ts index e83738f7cd..ca1f1badb7 100644 --- a/packages/shared-core/src/helpers/cron.ts +++ b/packages/shared-core/src/helpers/cron.ts @@ -20,9 +20,9 @@ function improveErrors(errors: string[]): string[] { if (error.includes(INPUT_CRON_START)) { error = error.split(INPUT_CRON_START)[0].trim() } - for (let [key, value] of Object.entries(ERROR_SWAPS)) { - if (error.includes(key)) { - error = error.replace(new RegExp(key, "g"), value) + for (let [oldErr, newErr] of Object.entries(ERROR_SWAPS)) { + if (error.includes(oldErr)) { + error = error.replace(new RegExp(oldErr, "g"), newErr) } } finalErrors.push(error) From cd2922308f739883d96ed303b150cb4c0e040a46 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 12 Feb 2024 16:01:02 +0100 Subject: [PATCH 14/25] Properly stringify errors --- packages/server/src/jsRunner/index.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/server/src/jsRunner/index.ts b/packages/server/src/jsRunner/index.ts index 38cd0dd0d6..e39dab1313 100644 --- a/packages/server/src/jsRunner/index.ts +++ b/packages/server/src/jsRunner/index.ts @@ -3,6 +3,7 @@ import env from "../environment" import { setJSRunner, setOnErrorLog } from "@budibase/string-templates" import { context, logging, timers } from "@budibase/backend-core" import tracer from "dd-trace" +import { serializeError } from "serialize-error" type TrackerFn = (f: () => T) => T @@ -61,7 +62,7 @@ export function init() { if (env.LOG_JS_ERRORS) { setOnErrorLog((error: Error) => { - logging.logWarn(JSON.stringify(error)) + logging.logWarn(JSON.stringify(serializeError(error))) }) } } From e3f803ef6bc1117974a36f5b8166ef3d7ec0efa2 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 12 Feb 2024 16:04:31 +0100 Subject: [PATCH 15/25] Install package --- packages/server/package.json | 1 + yarn.lock | 24 ++++++++++++++++++------ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/packages/server/package.json b/packages/server/package.json index 9d385c7664..2119ad05a1 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -103,6 +103,7 @@ "pouchdb-all-dbs": "1.1.1", "pouchdb-find": "7.2.2", "redis": "4", + "serialize-error": "^11.0.3", "server-destroy": "1.0.1", "snowflake-promise": "^4.5.0", "socket.io": "4.6.1", diff --git a/yarn.lock b/yarn.lock index 9e12ecad89..ae109ef864 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10840,9 +10840,9 @@ fn.name@1.x.x: integrity sha512-GRnmB5gPyJpAhTQdSZTSp9uaPSvl09KoYcMQtsB9rQoOmzs9dH6ffeccH+Z+cv6P68Hu5bC6JjRh4Ah/mHSNRw== follow-redirects@^1.15.0: - version "1.15.2" - resolved "https://registry.yarnpkg.com/follow-redirects/-/follow-redirects-1.15.2.tgz#b460864144ba63f2681096f274c4e57026da2c13" - integrity sha512-VQLG33o04KaQ8uYi2tVNbdrWp1QWxNNea+nmIB4EVM28v0hmP17z7aG1+wAkNzVq4KeXTq3221ye5qTJP91JwA== + version "1.15.5" + resolved "https://registry.yarnpkg.com/follow-redirects/-/follow-redirects-1.15.5.tgz#54d4d6d062c0fa7d9d17feb008461550e3ba8020" + integrity sha512-vSFWUON1B+yAw1VN4xMfxgn5fTUiaOzAJCKBwIIgT/+7CuGy9+r+5gITvP62j3RmaD5Ph65UaERdOSRGUzZtgw== for-each@^0.3.3: version "0.3.3" @@ -18918,9 +18918,9 @@ sax@1.2.1: integrity sha512-8I2a3LovHTOpm7NV5yOyO8IHqgVsfK4+UuySrXU8YXkSRX7k6hCV9b3HrkKCr3nMpgj+0bmocaJJWpvp1oc7ZA== sax@>=0.6.0: - version "1.2.4" - resolved "https://registry.yarnpkg.com/sax/-/sax-1.2.4.tgz#2816234e2378bddc4e5354fab5caa895df7100d9" - integrity sha512-NqVDv9TpANUjFm0N8uM5GxL36UgKi9/atZw+x7YFnQ8ckwFGKrl4xX4yWtrey3UJm5nP1kUbnYgLopqWNSRhWw== + version "1.3.0" + resolved "https://registry.yarnpkg.com/sax/-/sax-1.3.0.tgz#a5dbe77db3be05c9d1ee7785dbd3ea9de51593d0" + integrity sha512-0s+oAmw9zLl1V1cS9BtZN7JAd0cW5e0QH4W3LWEK6a4LaLEA2OTpGYWDY+6XasBLtz6wkm3u1xRw95mRuJ59WA== saxes@^5.0.1: version "5.0.1" @@ -19009,6 +19009,13 @@ seq-queue@^0.0.5: resolved "https://registry.yarnpkg.com/seq-queue/-/seq-queue-0.0.5.tgz#d56812e1c017a6e4e7c3e3a37a1da6d78dd3c93e" integrity sha512-hr3Wtp/GZIc/6DAGPDcV4/9WoZhjrkXsi5B/07QgX8tsdc6ilr7BFM6PM6rbdAX1kFSDYeZGLipIZZKyQP0O5Q== +serialize-error@^11.0.3: + version "11.0.3" + resolved "https://registry.yarnpkg.com/serialize-error/-/serialize-error-11.0.3.tgz#b54f439e15da5b4961340fbbd376b6b04aa52e92" + integrity sha512-2G2y++21dhj2R7iHAdd0FIzjGwuKZld+7Pl/bTU6YIkrC2ZMbVUjm+luj6A6V34Rv9XfKJDKpTWu9W4Gse1D9g== + dependencies: + type-fest "^2.12.2" + serialize-error@^7.0.1: version "7.0.1" resolved "https://registry.yarnpkg.com/serialize-error/-/serialize-error-7.0.1.tgz#f1360b0447f61ffb483ec4157c737fab7d778e18" @@ -20787,6 +20794,11 @@ type-fest@^0.8.1: resolved "https://registry.yarnpkg.com/type-fest/-/type-fest-0.8.1.tgz#09e249ebde851d3b1e48d27c105444667f17b83d" integrity sha512-4dbzIzqvjtgiM5rw1k5rEHtBANKmdudhGyBEajN01fEyhaAIhsoKNy6y7+IN93IfpFtwY9iqi7kD+xwKhQsNJA== +type-fest@^2.12.2: + version "2.19.0" + resolved "https://registry.yarnpkg.com/type-fest/-/type-fest-2.19.0.tgz#88068015bb33036a598b952e55e9311a60fd3a9b" + integrity sha512-RAH822pAdBgcNMAfWnCBU3CFZcfZ/i1eZjwFU/dsLKumyuuP3niueg2UAukXYF0E2AAoc82ZSSf9J0WQBinzHA== + type-is@^1.6.14, type-is@^1.6.16, type-is@^1.6.18: version "1.6.18" resolved "https://registry.yarnpkg.com/type-is/-/type-is-1.6.18.tgz#4e552cd05df09467dcbc4ef739de89f2cf37c131" From 36ea9b9df1882ad0aec513af0a2aa2096145f960 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 12 Feb 2024 16:20:01 +0100 Subject: [PATCH 16/25] Use existing version --- packages/server/package.json | 2 +- yarn.lock | 24 ++++++------------------ 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/packages/server/package.json b/packages/server/package.json index 2119ad05a1..e600e36bd3 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -103,7 +103,7 @@ "pouchdb-all-dbs": "1.1.1", "pouchdb-find": "7.2.2", "redis": "4", - "serialize-error": "^11.0.3", + "serialize-error": "^7.0.1", "server-destroy": "1.0.1", "snowflake-promise": "^4.5.0", "socket.io": "4.6.1", diff --git a/yarn.lock b/yarn.lock index ae109ef864..9e12ecad89 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10840,9 +10840,9 @@ fn.name@1.x.x: integrity sha512-GRnmB5gPyJpAhTQdSZTSp9uaPSvl09KoYcMQtsB9rQoOmzs9dH6ffeccH+Z+cv6P68Hu5bC6JjRh4Ah/mHSNRw== follow-redirects@^1.15.0: - version "1.15.5" - resolved "https://registry.yarnpkg.com/follow-redirects/-/follow-redirects-1.15.5.tgz#54d4d6d062c0fa7d9d17feb008461550e3ba8020" - integrity sha512-vSFWUON1B+yAw1VN4xMfxgn5fTUiaOzAJCKBwIIgT/+7CuGy9+r+5gITvP62j3RmaD5Ph65UaERdOSRGUzZtgw== + version "1.15.2" + resolved "https://registry.yarnpkg.com/follow-redirects/-/follow-redirects-1.15.2.tgz#b460864144ba63f2681096f274c4e57026da2c13" + integrity sha512-VQLG33o04KaQ8uYi2tVNbdrWp1QWxNNea+nmIB4EVM28v0hmP17z7aG1+wAkNzVq4KeXTq3221ye5qTJP91JwA== for-each@^0.3.3: version "0.3.3" @@ -18918,9 +18918,9 @@ sax@1.2.1: integrity sha512-8I2a3LovHTOpm7NV5yOyO8IHqgVsfK4+UuySrXU8YXkSRX7k6hCV9b3HrkKCr3nMpgj+0bmocaJJWpvp1oc7ZA== sax@>=0.6.0: - version "1.3.0" - resolved "https://registry.yarnpkg.com/sax/-/sax-1.3.0.tgz#a5dbe77db3be05c9d1ee7785dbd3ea9de51593d0" - integrity sha512-0s+oAmw9zLl1V1cS9BtZN7JAd0cW5e0QH4W3LWEK6a4LaLEA2OTpGYWDY+6XasBLtz6wkm3u1xRw95mRuJ59WA== + version "1.2.4" + resolved "https://registry.yarnpkg.com/sax/-/sax-1.2.4.tgz#2816234e2378bddc4e5354fab5caa895df7100d9" + integrity sha512-NqVDv9TpANUjFm0N8uM5GxL36UgKi9/atZw+x7YFnQ8ckwFGKrl4xX4yWtrey3UJm5nP1kUbnYgLopqWNSRhWw== saxes@^5.0.1: version "5.0.1" @@ -19009,13 +19009,6 @@ seq-queue@^0.0.5: resolved "https://registry.yarnpkg.com/seq-queue/-/seq-queue-0.0.5.tgz#d56812e1c017a6e4e7c3e3a37a1da6d78dd3c93e" integrity sha512-hr3Wtp/GZIc/6DAGPDcV4/9WoZhjrkXsi5B/07QgX8tsdc6ilr7BFM6PM6rbdAX1kFSDYeZGLipIZZKyQP0O5Q== -serialize-error@^11.0.3: - version "11.0.3" - resolved "https://registry.yarnpkg.com/serialize-error/-/serialize-error-11.0.3.tgz#b54f439e15da5b4961340fbbd376b6b04aa52e92" - integrity sha512-2G2y++21dhj2R7iHAdd0FIzjGwuKZld+7Pl/bTU6YIkrC2ZMbVUjm+luj6A6V34Rv9XfKJDKpTWu9W4Gse1D9g== - dependencies: - type-fest "^2.12.2" - serialize-error@^7.0.1: version "7.0.1" resolved "https://registry.yarnpkg.com/serialize-error/-/serialize-error-7.0.1.tgz#f1360b0447f61ffb483ec4157c737fab7d778e18" @@ -20794,11 +20787,6 @@ type-fest@^0.8.1: resolved "https://registry.yarnpkg.com/type-fest/-/type-fest-0.8.1.tgz#09e249ebde851d3b1e48d27c105444667f17b83d" integrity sha512-4dbzIzqvjtgiM5rw1k5rEHtBANKmdudhGyBEajN01fEyhaAIhsoKNy6y7+IN93IfpFtwY9iqi7kD+xwKhQsNJA== -type-fest@^2.12.2: - version "2.19.0" - resolved "https://registry.yarnpkg.com/type-fest/-/type-fest-2.19.0.tgz#88068015bb33036a598b952e55e9311a60fd3a9b" - integrity sha512-RAH822pAdBgcNMAfWnCBU3CFZcfZ/i1eZjwFU/dsLKumyuuP3niueg2UAukXYF0E2AAoc82ZSSf9J0WQBinzHA== - type-is@^1.6.14, type-is@^1.6.16, type-is@^1.6.18: version "1.6.18" resolved "https://registry.yarnpkg.com/type-is/-/type-is-1.6.18.tgz#4e552cd05df09467dcbc4ef739de89f2cf37c131" From 8e734b2d7245ce286af739720c6b96d90afde108 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Mon, 12 Feb 2024 15:25:00 +0000 Subject: [PATCH 17/25] frontend valiation for crons and fix preset null issue --- .../automation/SetupPanel/CronBuilder.svelte | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/packages/builder/src/components/automation/SetupPanel/CronBuilder.svelte b/packages/builder/src/components/automation/SetupPanel/CronBuilder.svelte index ee9eed51e9..97b5e3cfd4 100644 --- a/packages/builder/src/components/automation/SetupPanel/CronBuilder.svelte +++ b/packages/builder/src/components/automation/SetupPanel/CronBuilder.svelte @@ -2,15 +2,31 @@ import { Button, Select, Input, Label } from "@budibase/bbui" import { onMount, createEventDispatcher } from "svelte" import { flags } from "stores/backend" - + import { helpers } from "@budibase/shared-core" const dispatch = createEventDispatcher() export let value + let error + + $: { + const exists = CRON_EXPRESSIONS.some(cron => cron.value === value) + const customIndex = CRON_EXPRESSIONS.findIndex( + cron => cron.label === "Custom" + ) + + if (!exists && customIndex === -1) { + CRON_EXPRESSIONS[0] = { label: "Custom", value: value } + } else if (exists && customIndex !== -1) { + CRON_EXPRESSIONS.splice(customIndex, 1) + } + } const onChange = e => { - if (e.detail === value) { + error = helpers.cron.validate(e.detail).err + if (e.detail === value || error) { return } + value = e.detail dispatch("change", e.detail) } @@ -49,6 +65,7 @@
(touched = true)} @@ -64,7 +81,7 @@ {#if presets}