From 4ac9b657e506a494f9ce5ba401f95ca662387cb3 Mon Sep 17 00:00:00 2001 From: melohagan <101575380+melohagan@users.noreply.github.com> Date: Tue, 25 Jun 2024 08:26:52 +0100 Subject: [PATCH 1/8] Remove deprecated properties (#13958) * Remove deprecated properties * Fix backend-core test * Don't run account-portal tests * Update account-portal ref * Run account portal unit tests * Revert "Run account portal unit tests" This reverts commit b509bf31a493890c7fe7773d531660e5cb2482b3. * Revert "Update account-portal ref" This reverts commit 186391fbb6fa0b7d9ff1641b0187c565b4a1370a. --- .github/workflows/budibase_ci.yml | 4 ++-- .../backend-core/tests/core/utilities/structures/accounts.ts | 1 - packages/types/src/documents/account/account.ts | 3 --- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/.github/workflows/budibase_ci.yml b/.github/workflows/budibase_ci.yml index b7dbcae771..64eadef072 100644 --- a/.github/workflows/budibase_ci.yml +++ b/.github/workflows/budibase_ci.yml @@ -117,9 +117,9 @@ jobs: - name: Test run: | if ${{ env.USE_NX_AFFECTED }}; then - yarn test --ignore=@budibase/worker --ignore=@budibase/server --since=${{ env.NX_BASE_BRANCH }} + yarn test --ignore=@budibase/worker --ignore=@budibase/server --since=${{ env.NX_BASE_BRANCH }} --ignore=@budibase/account-portal-ui --ignore @budibase/account-portal-server else - yarn test --ignore=@budibase/worker --ignore=@budibase/server + yarn test --ignore=@budibase/worker --ignore=@budibase/server --ignore=@budibase/account-portal-ui --ignore @budibase/account-portal-server fi test-worker: diff --git a/packages/backend-core/tests/core/utilities/structures/accounts.ts b/packages/backend-core/tests/core/utilities/structures/accounts.ts index 7dcc2de116..29453ad60a 100644 --- a/packages/backend-core/tests/core/utilities/structures/accounts.ts +++ b/packages/backend-core/tests/core/utilities/structures/accounts.ts @@ -24,7 +24,6 @@ export const account = (partial: Partial = {}): Account => { createdAt: Date.now(), verified: true, verificationSent: true, - tier: "FREE", // DEPRECATED authType: AuthType.PASSWORD, name: generator.name(), size: "10+", diff --git a/packages/types/src/documents/account/account.ts b/packages/types/src/documents/account/account.ts index 239d845722..61792a7f47 100644 --- a/packages/types/src/documents/account/account.ts +++ b/packages/types/src/documents/account/account.ts @@ -42,10 +42,7 @@ export interface Account extends CreateAccount { verified: boolean verificationSent: boolean // licensing - tier: string // deprecated planType?: PlanType - /** @deprecated */ - planTier?: number license?: License installId?: string installTenantId?: string From f3d466f25585f477b254441cb3784ed6876109e1 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Tue, 25 Jun 2024 08:51:35 +0100 Subject: [PATCH 2/8] fix issue where schema wasn't updating types when a query was run (#14004) * fix issue where schema wasn't updating types when a query was run * add tests for schema matching --- .../integration/RestQueryViewer.svelte | 6 +- .../server/src/api/controllers/query/index.ts | 4 +- .../routes/tests/queries/generic-sql.spec.ts | 61 +++++++++++++++++++ .../api/routes/tests/queries/mongodb.spec.ts | 61 +++++++++++++++++++ .../src/api/routes/tests/queries/rest.spec.ts | 55 +++++++++++++++++ 5 files changed, 182 insertions(+), 5 deletions(-) diff --git a/packages/builder/src/components/integration/RestQueryViewer.svelte b/packages/builder/src/components/integration/RestQueryViewer.svelte index b44831550e..0489167823 100644 --- a/packages/builder/src/components/integration/RestQueryViewer.svelte +++ b/packages/builder/src/components/integration/RestQueryViewer.svelte @@ -233,9 +233,9 @@ response.info = response.info || { code: 200 } // if existing schema, copy over what it is if (schema) { - for (let [name, field] of Object.entries(schema)) { - if (response.schema[name]) { - response.schema[name] = field + for (let [name, field] of Object.entries(response.schema)) { + if (!schema[name]) { + schema[name] = field } } } diff --git a/packages/server/src/api/controllers/query/index.ts b/packages/server/src/api/controllers/query/index.ts index b52cea553f..54f672c3f3 100644 --- a/packages/server/src/api/controllers/query/index.ts +++ b/packages/server/src/api/controllers/query/index.ts @@ -311,8 +311,8 @@ export async function preview( // if existing schema, update to include any previous schema keys if (existingSchema) { - for (let key of Object.keys(previewSchema)) { - if (existingSchema[key]) { + for (let key of Object.keys(existingSchema)) { + if (!previewSchema[key]) { previewSchema[key] = existingSchema[key] } } diff --git a/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts b/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts index e72a091688..3ed19f5eee 100644 --- a/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts +++ b/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts @@ -250,6 +250,67 @@ describe.each( expect(events.query.previewed).toHaveBeenCalledTimes(1) }) + it("should update schema when column type changes from number to string", async () => { + const tableName = "schema_change_test" + await client.schema.dropTableIfExists(tableName) + + await client.schema.createTable(tableName, table => { + table.increments("id").primary() + table.string("name") + table.integer("data") + }) + + await client(tableName).insert({ + name: "test", + data: 123, + }) + + const firstPreview = await config.api.query.preview({ + datasourceId: datasource._id!, + name: "Test Query", + queryVerb: "read", + fields: { + sql: `SELECT * FROM ${tableName}`, + }, + parameters: [], + transformer: "return data", + schema: {}, + readable: true, + }) + + expect(firstPreview.schema).toEqual( + expect.objectContaining({ + data: { type: "number", name: "data" }, + }) + ) + + await client.schema.alterTable(tableName, table => { + table.string("data").alter() + }) + + await client(tableName).update({ + data: "string value", + }) + + const secondPreview = await config.api.query.preview({ + datasourceId: datasource._id!, + name: "Test Query", + queryVerb: "read", + fields: { + sql: `SELECT * FROM ${tableName}`, + }, + parameters: [], + transformer: "return data", + schema: firstPreview.schema, + readable: true, + }) + + expect(secondPreview.schema).toEqual( + expect.objectContaining({ + data: { type: "string", name: "data" }, + }) + ) + }) it("should work with static variables", async () => { await config.api.datasource.update({ ...datasource, diff --git a/packages/server/src/api/routes/tests/queries/mongodb.spec.ts b/packages/server/src/api/routes/tests/queries/mongodb.spec.ts index c79ae68a36..4822729478 100644 --- a/packages/server/src/api/routes/tests/queries/mongodb.spec.ts +++ b/packages/server/src/api/routes/tests/queries/mongodb.spec.ts @@ -137,6 +137,67 @@ describe("/queries", () => { }) }) + it("should update schema when structure changes from object to array", async () => { + const name = generator.guid() + + await withCollection(async collection => { + await collection.insertOne({ name, field: { subfield: "value" } }) + }) + + const firstPreview = await config.api.query.preview({ + name: "Test Query", + datasourceId: datasource._id!, + fields: { + json: { name: { $eq: name } }, + extra: { + collection, + actionType: "findOne", + }, + }, + schema: {}, + queryVerb: "read", + parameters: [], + transformer: "return data", + readable: true, + }) + + expect(firstPreview.schema).toEqual( + expect.objectContaining({ + field: { type: "json", name: "field" }, + }) + ) + + await withCollection(async collection => { + await collection.updateOne( + { name }, + { $set: { field: ["value1", "value2"] } } + ) + }) + + const secondPreview = await config.api.query.preview({ + name: "Test Query", + datasourceId: datasource._id!, + fields: { + json: { name: { $eq: name } }, + extra: { + collection, + actionType: "findOne", + }, + }, + schema: firstPreview.schema, + queryVerb: "read", + parameters: [], + transformer: "return data", + readable: true, + }) + + expect(secondPreview.schema).toEqual( + expect.objectContaining({ + field: { type: "array", name: "field" }, + }) + ) + }) + it("should generate a nested schema based on all of the nested items", async () => { const name = generator.guid() const item = { diff --git a/packages/server/src/api/routes/tests/queries/rest.spec.ts b/packages/server/src/api/routes/tests/queries/rest.spec.ts index 1d5483017b..29bbbf3a61 100644 --- a/packages/server/src/api/routes/tests/queries/rest.spec.ts +++ b/packages/server/src/api/routes/tests/queries/rest.spec.ts @@ -92,6 +92,61 @@ describe("rest", () => { expect(cached.rows[0].name).toEqual("one") }) + it("should update schema when structure changes from JSON to array", async () => { + const datasource = await config.api.datasource.create({ + name: generator.guid(), + type: "test", + source: SourceName.REST, + config: {}, + }) + + nock("http://www.example.com") + .get("/") + .reply(200, [{ obj: {}, id: "1" }]) + + const firstResponse = await config.api.query.preview({ + datasourceId: datasource._id!, + name: "test query", + parameters: [], + queryVerb: "read", + transformer: "", + schema: {}, + readable: true, + fields: { + path: "www.example.com", + }, + }) + + expect(firstResponse.schema).toEqual({ + obj: { type: "json", name: "obj" }, + id: { type: "string", name: "id" }, + }) + + nock.cleanAll() + + nock("http://www.example.com") + .get("/") + .reply(200, [{ obj: [], id: "1" }]) + + const secondResponse = await config.api.query.preview({ + datasourceId: datasource._id!, + name: "test query", + parameters: [], + queryVerb: "read", + transformer: "", + schema: firstResponse.schema, + readable: true, + fields: { + path: "www.example.com", + }, + }) + + expect(secondResponse.schema).toEqual({ + obj: { type: "array", name: "obj" }, + id: { type: "string", name: "id" }, + }) + }) + it("should parse global and query level header mappings", async () => { const datasource = await config.api.datasource.create({ name: generator.guid(), From dca73dac83455f823ba8ac40957c564616300bca Mon Sep 17 00:00:00 2001 From: melohagan <101575380+melohagan@users.noreply.github.com> Date: Tue, 25 Jun 2024 09:14:46 +0100 Subject: [PATCH 3/8] Run account portal unit tests (#14013) --- .github/workflows/budibase_ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/budibase_ci.yml b/.github/workflows/budibase_ci.yml index 64eadef072..b7dbcae771 100644 --- a/.github/workflows/budibase_ci.yml +++ b/.github/workflows/budibase_ci.yml @@ -117,9 +117,9 @@ jobs: - name: Test run: | if ${{ env.USE_NX_AFFECTED }}; then - yarn test --ignore=@budibase/worker --ignore=@budibase/server --since=${{ env.NX_BASE_BRANCH }} --ignore=@budibase/account-portal-ui --ignore @budibase/account-portal-server + yarn test --ignore=@budibase/worker --ignore=@budibase/server --since=${{ env.NX_BASE_BRANCH }} else - yarn test --ignore=@budibase/worker --ignore=@budibase/server --ignore=@budibase/account-portal-ui --ignore @budibase/account-portal-server + yarn test --ignore=@budibase/worker --ignore=@budibase/server fi test-worker: From 95d97d87af6c17b75eda76a1c9fe6c26de15d38b Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Tue, 25 Jun 2024 08:47:29 +0000 Subject: [PATCH 4/8] Bump version to 2.29.2 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 0efaf75283..76d39a69b3 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.29.1", + "version": "2.29.2", "npmClient": "yarn", "packages": [ "packages/*", From 1fa18ccfcea0efff224084a9f6b023a343f1a94f Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Tue, 25 Jun 2024 11:17:14 +0100 Subject: [PATCH 5/8] Some automation fixes and refactoring (#13870) * fix issue where booleans were being parsed to null * refactor looping out of automations * clean out execute function in orchestrator of looping code * re-add accidentally deleted file * remove spec file * remove log * move code back into main automation thread * account portal update --------- Co-authored-by: Adria Navarro Co-authored-by: Michael Drury Co-authored-by: Sam Rose --- packages/account-portal | 2 +- packages/server/src/automations/loopUtils.ts | 36 +++++++ .../server/src/automations/steps/filter.ts | 7 +- packages/server/src/threads/automation.ts | 96 +++++++------------ .../types/src/documents/app/automation.ts | 2 +- 5 files changed, 76 insertions(+), 67 deletions(-) create mode 100644 packages/server/src/automations/loopUtils.ts diff --git a/packages/account-portal b/packages/account-portal index b600cca314..ff16525b73 160000 --- a/packages/account-portal +++ b/packages/account-portal @@ -1 +1 @@ -Subproject commit b600cca314a5cc9971e44d46047d1a0019b46b08 +Subproject commit ff16525b73c5751d344f5c161a682609c0a993f2 diff --git a/packages/server/src/automations/loopUtils.ts b/packages/server/src/automations/loopUtils.ts new file mode 100644 index 0000000000..5ee2559050 --- /dev/null +++ b/packages/server/src/automations/loopUtils.ts @@ -0,0 +1,36 @@ +import * as automationUtils from "./automationUtils" + +type ObjValue = { + [key: string]: string | ObjValue +} + +export function replaceFakeBindings( + originalStepInput: Record, + loopStepNumber: number +) { + for (const [key, value] of Object.entries(originalStepInput)) { + originalStepInput[key] = replaceBindingsRecursive(value, loopStepNumber) + } + return originalStepInput +} + +function replaceBindingsRecursive( + value: string | ObjValue, + loopStepNumber: number +) { + if (typeof value === "object") { + for (const [innerKey, innerValue] of Object.entries(value)) { + if (typeof innerValue === "string") { + value[innerKey] = automationUtils.substituteLoopStep( + innerValue, + `steps.${loopStepNumber}` + ) + } else if (typeof innerValue === "object") { + value[innerKey] = replaceBindingsRecursive(innerValue, loopStepNumber) + } + } + } else if (typeof value === "string") { + value = automationUtils.substituteLoopStep(value, `steps.${loopStepNumber}`) + } + return value +} diff --git a/packages/server/src/automations/steps/filter.ts b/packages/server/src/automations/steps/filter.ts index 6867809500..624619bb95 100644 --- a/packages/server/src/automations/steps/filter.ts +++ b/packages/server/src/automations/steps/filter.ts @@ -73,7 +73,12 @@ export async function run({ inputs }: AutomationStepInput) { try { let { field, condition, value } = inputs // coerce types so that we can use them - if (!isNaN(value) && !isNaN(field)) { + if ( + !isNaN(value) && + !isNaN(field) && + typeof field !== "boolean" && + typeof value !== "boolean" + ) { value = parseFloat(value) field = parseFloat(field) } else if (!isNaN(Date.parse(value)) && !isNaN(Date.parse(field))) { diff --git a/packages/server/src/threads/automation.ts b/packages/server/src/threads/automation.ts index 469d0845c9..a7cf71de4b 100644 --- a/packages/server/src/threads/automation.ts +++ b/packages/server/src/threads/automation.ts @@ -7,6 +7,8 @@ import { } from "../automations/utils" import * as actions from "../automations/actions" import * as automationUtils from "../automations/automationUtils" +import { replaceFakeBindings } from "../automations/loopUtils" + import { default as AutomationEmitter } from "../events/AutomationEmitter" import { generateAutomationMetadataID, isProdAppID } from "../db/utils" import { definitions as triggerDefs } from "../automations/triggerInfo" @@ -214,15 +216,15 @@ class Orchestrator { } updateContextAndOutput( - loopStepNumber: number | undefined, + currentLoopStepIndex: number | undefined, step: AutomationStep, output: any, result: { success: boolean; status: string } ) { - if (!loopStepNumber) { + if (!currentLoopStepIndex) { throw new Error("No loop step number provided.") } - this.executionOutput.steps.splice(loopStepNumber, 0, { + this.executionOutput.steps.splice(currentLoopStepIndex, 0, { id: step.id, stepId: step.stepId, outputs: { @@ -232,7 +234,7 @@ class Orchestrator { }, inputs: step.inputs, }) - this._context.steps.splice(loopStepNumber, 0, { + this._context.steps.splice(currentLoopStepIndex, 0, { ...output, success: result.success, status: result.status, @@ -256,7 +258,7 @@ class Orchestrator { let loopStep: LoopStep | undefined = undefined let stepCount = 0 - let loopStepNumber: any = undefined + let currentLoopStepIndex: number = 0 let loopSteps: LoopStep[] | undefined = [] let metadata let timeoutFlag = false @@ -290,7 +292,7 @@ class Orchestrator { }, }) - let input: any, + let input: LoopInput | undefined, iterations = 1, iterationCount = 0 @@ -309,19 +311,19 @@ class Orchestrator { stepCount++ if (step.stepId === LOOP_STEP_ID) { loopStep = step as LoopStep - loopStepNumber = stepCount + currentLoopStepIndex = stepCount continue } if (loopStep) { input = await processObject(loopStep.inputs, this._context) - iterations = getLoopIterations(loopStep as LoopStep) + iterations = getLoopIterations(loopStep) stepSpan?.addTags({ step: { iterations } }) } - for (let index = 0; index < iterations; index++) { + + for (let stepIndex = 0; stepIndex < iterations; stepIndex++) { let originalStepInput = cloneDeep(step.inputs) - // Handle if the user has set a max iteration count or if it reaches the max limit set by us - if (loopStep && input.binding) { + if (loopStep && input?.binding) { let tempOutput = { items: loopSteps, iterations: iterationCount, @@ -332,7 +334,7 @@ class Orchestrator { ) } catch (err) { this.updateContextAndOutput( - loopStepNumber, + currentLoopStepIndex, step, tempOutput, { @@ -353,55 +355,22 @@ class Orchestrator { } else if (Array.isArray(loopStep.inputs.binding)) { item = loopStep.inputs.binding } - this._context.steps[loopStepNumber] = { - currentItem: item[index], + this._context.steps[currentLoopStepIndex] = { + currentItem: item[stepIndex], } - // The "Loop" binding in the front end is "fake", so replace it here so the context can understand it - // Pretty hacky because we need to account for the row object - for (let [key, value] of Object.entries(originalStepInput)) { - if (typeof value === "object") { - for (let [innerKey, innerValue] of Object.entries( - originalStepInput[key] - )) { - if (typeof innerValue === "string") { - originalStepInput[key][innerKey] = - automationUtils.substituteLoopStep( - innerValue, - `steps.${loopStepNumber}` - ) - } else if (typeof value === "object") { - for (let [innerObject, innerValue] of Object.entries( - originalStepInput[key][innerKey] - )) { - if (typeof innerValue === "string") { - originalStepInput[key][innerKey][innerObject] = - automationUtils.substituteLoopStep( - innerValue, - `steps.${loopStepNumber}` - ) - } - } - } - } - } else { - if (typeof value === "string") { - originalStepInput[key] = - automationUtils.substituteLoopStep( - value, - `steps.${loopStepNumber}` - ) - } - } - } + originalStepInput = replaceFakeBindings( + originalStepInput, + currentLoopStepIndex + ) if ( - index === env.AUTOMATION_MAX_ITERATIONS || + stepIndex === env.AUTOMATION_MAX_ITERATIONS || (loopStep.inputs.iterations && - index === parseInt(loopStep.inputs.iterations)) + stepIndex === parseInt(loopStep.inputs.iterations)) ) { this.updateContextAndOutput( - loopStepNumber, + currentLoopStepIndex, step, tempOutput, { @@ -416,7 +385,7 @@ class Orchestrator { let isFailure = false const currentItem = - this._context.steps[loopStepNumber]?.currentItem + this._context.steps[currentLoopStepIndex]?.currentItem if (currentItem && typeof currentItem === "object") { isFailure = Object.keys(currentItem).some(value => { return currentItem[value] === loopStep?.inputs.failure @@ -428,7 +397,7 @@ class Orchestrator { if (isFailure) { this.updateContextAndOutput( - loopStepNumber, + currentLoopStepIndex, step, tempOutput, { @@ -453,7 +422,6 @@ class Orchestrator { continue } - // If it's a loop step, we need to manually add the bindings to the context let stepFn = await this.getStepFunctionality(step.stepId) let inputs = await processObject(originalStepInput, this._context) inputs = automationUtils.cleanInputValues( @@ -502,9 +470,9 @@ class Orchestrator { if (loopStep) { iterationCount++ - if (index === iterations - 1) { + if (stepIndex === iterations - 1) { loopStep = undefined - this._context.steps.splice(loopStepNumber, 1) + this._context.steps.splice(currentLoopStepIndex, 1) break } } @@ -515,7 +483,7 @@ class Orchestrator { if (loopStep && iterations === 0) { loopStep = undefined - this.executionOutput.steps.splice(loopStepNumber + 1, 0, { + this.executionOutput.steps.splice(currentLoopStepIndex + 1, 0, { id: step.id, stepId: step.stepId, outputs: { @@ -525,14 +493,14 @@ class Orchestrator { inputs: {}, }) - this._context.steps.splice(loopStepNumber, 1) + this._context.steps.splice(currentLoopStepIndex, 1) iterations = 1 } // Delete the step after the loop step as it's irrelevant, since information is included // in the loop step if (wasLoopStep && !loopStep) { - this._context.steps.splice(loopStepNumber + 1, 1) + this._context.steps.splice(currentLoopStepIndex + 1, 1) wasLoopStep = false } if (loopSteps && loopSteps.length) { @@ -541,13 +509,13 @@ class Orchestrator { items: loopSteps, iterations: iterationCount, } - this.executionOutput.steps.splice(loopStepNumber + 1, 0, { + this.executionOutput.steps.splice(currentLoopStepIndex + 1, 0, { id: step.id, stepId: step.stepId, outputs: tempOutput, inputs: step.inputs, }) - this._context.steps[loopStepNumber] = tempOutput + this._context.steps[currentLoopStepIndex] = tempOutput wasLoopStep = true loopSteps = [] diff --git a/packages/types/src/documents/app/automation.ts b/packages/types/src/documents/app/automation.ts index 5954a47151..6ea62ffffb 100644 --- a/packages/types/src/documents/app/automation.ts +++ b/packages/types/src/documents/app/automation.ts @@ -144,7 +144,7 @@ interface BaseIOStructure { required?: string[] } -interface InputOutputBlock { +export interface InputOutputBlock { properties: { [key: string]: BaseIOStructure } From 314d62bea0558f997e5df3d4770a2d76d0eea077 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 25 Jun 2024 14:29:56 +0100 Subject: [PATCH 6/8] Revert "Disallow prohibited columns" --- packages/backend-core/src/db/constants.ts | 19 +++++++++---- .../DataTable/modals/CreateEditColumn.svelte | 18 ++++-------- .../server/src/api/routes/tests/table.spec.ts | 28 ------------------- .../src/sdk/app/tables/internal/index.ts | 12 -------- packages/shared-core/src/constants/index.ts | 1 - packages/shared-core/src/constants/rows.ts | 14 ---------- packages/shared-core/src/table.ts | 22 +-------------- 7 files changed, 20 insertions(+), 94 deletions(-) delete mode 100644 packages/shared-core/src/constants/rows.ts diff --git a/packages/backend-core/src/db/constants.ts b/packages/backend-core/src/db/constants.ts index 69c98fe569..bfa7595d62 100644 --- a/packages/backend-core/src/db/constants.ts +++ b/packages/backend-core/src/db/constants.ts @@ -1,5 +1,14 @@ -export { - CONSTANT_INTERNAL_ROW_COLS, - CONSTANT_EXTERNAL_ROW_COLS, - isInternalColumnName, -} from "@budibase/shared-core" +export const CONSTANT_INTERNAL_ROW_COLS = [ + "_id", + "_rev", + "type", + "createdAt", + "updatedAt", + "tableId", +] as const + +export const CONSTANT_EXTERNAL_ROW_COLS = ["_id", "_rev", "tableId"] as const + +export function isInternalColumnName(name: string): boolean { + return (CONSTANT_INTERNAL_ROW_COLS as readonly string[]).includes(name) +} diff --git a/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte b/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte index d79eedd194..17ecd8f844 100644 --- a/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte +++ b/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte @@ -17,8 +17,6 @@ SWITCHABLE_TYPES, ValidColumnNameRegex, helpers, - CONSTANT_INTERNAL_ROW_COLS, - CONSTANT_EXTERNAL_ROW_COLS, } from "@budibase/shared-core" import { createEventDispatcher, getContext, onMount } from "svelte" import { cloneDeep } from "lodash/fp" @@ -54,6 +52,7 @@ const DATE_TYPE = FieldType.DATETIME const dispatch = createEventDispatcher() + const PROHIBITED_COLUMN_NAMES = ["type", "_id", "_rev", "tableId"] const { dispatch: gridDispatch, rows } = getContext("grid") export let field @@ -488,27 +487,20 @@ }) } const newError = {} - const prohibited = externalTable - ? CONSTANT_EXTERNAL_ROW_COLS - : CONSTANT_INTERNAL_ROW_COLS if (!externalTable && fieldInfo.name?.startsWith("_")) { newError.name = `Column name cannot start with an underscore.` } else if (fieldInfo.name && !fieldInfo.name.match(ValidColumnNameRegex)) { newError.name = `Illegal character; must be alpha-numeric.` - } else if ( - prohibited.some( - name => fieldInfo?.name?.toLowerCase() === name.toLowerCase() - ) - ) { - newError.name = `${prohibited.join( + } else if (PROHIBITED_COLUMN_NAMES.some(name => fieldInfo.name === name)) { + newError.name = `${PROHIBITED_COLUMN_NAMES.join( ", " - )} are not allowed as column names - case insensitive.` + )} are not allowed as column names` } else if (inUse($tables.selected, fieldInfo.name, originalName)) { newError.name = `Column name already in use.` } if (fieldInfo.type === FieldType.AUTO && !fieldInfo.subtype) { - newError.subtype = `Auto Column requires a type.` + newError.subtype = `Auto Column requires a type` } if (fieldInfo.fieldName && fieldInfo.tableId) { diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index e75e5e23e7..f23e0de6db 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -276,34 +276,6 @@ describe.each([ }) }) - isInternal && - it("shouldn't allow duplicate column names", async () => { - const saveTableRequest: SaveTableRequest = { - ...basicTable(), - } - saveTableRequest.schema["Type"] = { - type: FieldType.STRING, - name: "Type", - } - await config.api.table.save(saveTableRequest, { - status: 400, - body: { - message: - 'Column(s) "type" are duplicated - check for other columns with these name (case in-sensitive)', - }, - }) - saveTableRequest.schema.foo = { type: FieldType.STRING, name: "foo" } - saveTableRequest.schema.FOO = { type: FieldType.STRING, name: "FOO" } - - await config.api.table.save(saveTableRequest, { - status: 400, - body: { - message: - 'Column(s) "type, foo" are duplicated - check for other columns with these name (case in-sensitive)', - }, - }) - }) - it("should add a new column for an internal DB table", async () => { const saveTableRequest: SaveTableRequest = { ...basicTable(), diff --git a/packages/server/src/sdk/app/tables/internal/index.ts b/packages/server/src/sdk/app/tables/internal/index.ts index fc32708708..ea40d2bfe9 100644 --- a/packages/server/src/sdk/app/tables/internal/index.ts +++ b/packages/server/src/sdk/app/tables/internal/index.ts @@ -17,7 +17,6 @@ import { cloneDeep } from "lodash/fp" import isEqual from "lodash/isEqual" import { runStaticFormulaChecks } from "../../../../api/controllers/table/bulkFormula" import { context } from "@budibase/backend-core" -import { findDuplicateInternalColumns } from "@budibase/shared-core" import { getTable } from "../getters" import { checkAutoColumns } from "./utils" import * as viewsSdk from "../../views" @@ -45,17 +44,6 @@ export async function save( if (hasTypeChanged(table, oldTable)) { throw new Error("A column type has changed.") } - - // check for case sensitivity - we don't want to allow duplicated columns - const duplicateColumn = findDuplicateInternalColumns(table) - if (duplicateColumn.length) { - throw new Error( - `Column(s) "${duplicateColumn.join( - ", " - )}" are duplicated - check for other columns with these name (case in-sensitive)` - ) - } - // check that subtypes have been maintained table = checkAutoColumns(table, oldTable) diff --git a/packages/shared-core/src/constants/index.ts b/packages/shared-core/src/constants/index.ts index c9d1a8fc8f..82ac0d51c3 100644 --- a/packages/shared-core/src/constants/index.ts +++ b/packages/shared-core/src/constants/index.ts @@ -1,6 +1,5 @@ export * from "./api" export * from "./fields" -export * from "./rows" export const OperatorOptions = { Equals: { diff --git a/packages/shared-core/src/constants/rows.ts b/packages/shared-core/src/constants/rows.ts deleted file mode 100644 index bfa7595d62..0000000000 --- a/packages/shared-core/src/constants/rows.ts +++ /dev/null @@ -1,14 +0,0 @@ -export const CONSTANT_INTERNAL_ROW_COLS = [ - "_id", - "_rev", - "type", - "createdAt", - "updatedAt", - "tableId", -] as const - -export const CONSTANT_EXTERNAL_ROW_COLS = ["_id", "_rev", "tableId"] as const - -export function isInternalColumnName(name: string): boolean { - return (CONSTANT_INTERNAL_ROW_COLS as readonly string[]).includes(name) -} diff --git a/packages/shared-core/src/table.ts b/packages/shared-core/src/table.ts index 4b578a2aef..7706b78037 100644 --- a/packages/shared-core/src/table.ts +++ b/packages/shared-core/src/table.ts @@ -1,5 +1,4 @@ -import { FieldType, Table } from "@budibase/types" -import { CONSTANT_INTERNAL_ROW_COLS } from "./constants" +import { FieldType } from "@budibase/types" const allowDisplayColumnByType: Record = { [FieldType.STRING]: true, @@ -52,22 +51,3 @@ export function canBeDisplayColumn(type: FieldType): boolean { export function canBeSortColumn(type: FieldType): boolean { return !!allowSortColumnByType[type] } - -export function findDuplicateInternalColumns(table: Table): string[] { - // get the column names - const columnNames = Object.keys(table.schema) - .concat(CONSTANT_INTERNAL_ROW_COLS) - .map(colName => colName.toLowerCase()) - // there are duplicates - const set = new Set(columnNames) - let duplicates: string[] = [] - if (set.size !== columnNames.length) { - for (let key of set.keys()) { - const count = columnNames.filter(name => name === key).length - if (count > 1) { - duplicates.push(key) - } - } - } - return duplicates -} From 9554d8095386dba604e82e4a7a3cc21604b44781 Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Tue, 25 Jun 2024 13:40:38 +0000 Subject: [PATCH 7/8] Bump version to 2.29.3 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 76d39a69b3..32b7d8f766 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.29.2", + "version": "2.29.3", "npmClient": "yarn", "packages": [ "packages/*", From a13d60f7c9c4ba437761f349d005856448bfd1e9 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Wed, 26 Jun 2024 07:45:12 +0100 Subject: [PATCH 8/8] Don't show hidden settings in the settings bar --- packages/client/manifest.json | 26 ++++++++++--------- .../src/components/preview/SettingsBar.svelte | 2 +- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/packages/client/manifest.json b/packages/client/manifest.json index 00b503626f..7a9d1a5695 100644 --- a/packages/client/manifest.json +++ b/packages/client/manifest.json @@ -23,17 +23,21 @@ { "type": "bigint", "message": "stringAsNumber" }, { "type": "options", "message": "stringAsNumber" }, { "type": "formula", "message": "stringAsNumber" }, - { "type": "datetime", "message": "dateAsNumber"} + { "type": "datetime", "message": "dateAsNumber" } ], - "unsupported": [ - { "type": "json", "message": "jsonPrimitivesOnly" } - ] + "unsupported": [{ "type": "json", "message": "jsonPrimitivesOnly" }] }, "stringLike": { - "supported": ["string", "number", "bigint", "options", "longform", "boolean", "datetime"], - "unsupported": [ - { "type": "json", "message": "jsonPrimitivesOnly" } - ] + "supported": [ + "string", + "number", + "bigint", + "options", + "longform", + "boolean", + "datetime" + ], + "unsupported": [{ "type": "json", "message": "jsonPrimitivesOnly" }] }, "datetimeLike": { "supported": ["datetime"], @@ -43,11 +47,9 @@ { "type": "options", "message": "stringAsDate" }, { "type": "formula", "message": "stringAsDate" }, { "type": "bigint", "message": "stringAsDate" }, - { "type": "number", "message": "numberAsDate"} + { "type": "number", "message": "numberAsDate" } ], - "unsupported": [ - { "type": "json", "message": "jsonPrimitivesOnly" } - ] + "unsupported": [{ "type": "json", "message": "jsonPrimitivesOnly" }] } }, "layout": { diff --git a/packages/client/src/components/preview/SettingsBar.svelte b/packages/client/src/components/preview/SettingsBar.svelte index b69b8ce050..c5109c6bca 100644 --- a/packages/client/src/components/preview/SettingsBar.svelte +++ b/packages/client/src/components/preview/SettingsBar.svelte @@ -41,7 +41,7 @@ allSettings.push(setting) } }) - return allSettings.filter(setting => setting.showInBar) + return allSettings.filter(setting => setting.showInBar && !setting.hidden) } const updatePosition = () => {